Repository navigation
fix(tui): dropped DM turn takes the reply after its own user row - #170
Conversation
After a stream drop the turn took the first new assistant row as its reply, so an unrelated row (another device's reply, a proactive message) settled it and showed in the reply strip. It now identifies its own user row (first new user row whose text is what it sent) and takes the first new assistant row created after it. With no such user row it stays "checking". Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QNDeh8M8jKfHhtWGJwkxTm
Vycari Review (shadow)Shadow round 2 on 109d231. This comment is non-gating and needs no response: it is collected to measure agreement with the current reviewer. The round's verdict is the Vycari Review (shadow) check run. Open: P0 0 · P1 1 · P2 0 · confirmed fixed 1 · withdrawn 0 Judge's notes The pending-bubble finding is fixed, and the trajectory check now stops replies to other turns from being taken. One narrow P1 is left, "An unlinked turn still settles on a reminder": before the real reply links the user row, a reminder can still be shown as the answer. The PR discloses this as an accepted limit. P1 · VR-a4d21b20 · An unlinked turn still settles on a remindersrc/pepper_cli/tui/state.py lines 997–1000 · symbol DmTurn.reply_id · lens concurrency-time · status fix-incomplete The new trajectory guard treats two absent trajectory IDs as a match. While the user's row remains unlinked, an unrelated assistant row with no trajectory still satisfies every predicate and permanently settles the dropped turn. Evidence: state.py:999 allows None == None; tests/test_tui_state.py:568-578 explicitly exercises an unlinked user row. The history schema permits null trajectory_id (contracts/api-v1.openapi.json:2127-2138), so field presence does not exclude this state. app.py:813-814 considers the selected row seen, and app.py:2349-2355 removes the checking turn and stores the selected text and assistant ID. A later transaction linking the user row cannot correct the retired turn. The added guard therefore fixes the prior unrelated-trajectory case but leaves the same harm reachable before linkage. Re-review (fix-incomplete): The trajectory_id guard now rejects replies to other turns that carry their own trajectory. But the server links the user row only when it writes the reply. Until then both ids are None, so a reminder that lands between the user row and the real reply (the prior's exact scenario) still matches and settles the turn. Failure scenario The terminal sends 'q' and loses its stream. History contains its user row with trajectory_id=None while Pepper is still working. A reminder then lands with trajectory_id=None. The next poll selects the reminder as this turn's answer and removes the turn from reconciliation. When the real reply subsequently lands with its trajectory, the pane's reply strip continues showing the reminder as the answer. How this was verified (read-only-reasoning) Read state.py reply_id lines 989-1001. The trajectory_id equality accepts None == None, as the candidate says. Did not open the app.py settle path. Proving test Confirmed fixed
|
…ajectory The server links a turn's user row to its trajectory and writes the reply with the same trajectory_id in one transaction, and /chat/history already sends the field. Mirror it on ChatHistoryEntry and require a dropped turn's candidate reply to share its user row's trajectory_id, so a reminder or a reply to another device created after the user row is no longer taken. The reply_id docstring now states the guarantee as it is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QNDeh8M8jKfHhtWGJwkxTm
While a dropped turn is checking, user_landed took any new user row, so another device's message hid this turn's provisional bubble. Use the same own-row match as the reply. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QNDeh8M8jKfHhtWGJwkxTm
|
Round 1 summary (head 109d231):
|
|
Round 2 (head 109d231): Greptile 5/5 on the head, checks green, no open threads.
|
|
Maintainer decision (2026-10-29): keep e8872e8's trajectory_id match; leave the no-trajectory window open (VR-a4d21b20 accepted as-is, no follow-up). Ready to merge. |
Human overview
When a Pepper DM stream drops, the TUI re-reads history to find the reply. It used to take any new assistant row, so a reply to another device's message or a proactive message could be shown as the answer. It now takes only the first assistant row created after the turn's own user row and in the same trajectory (the server already sends
trajectory_idon history rows and links a turn's user row and reply in one transaction), and does not guess when it can't find that row. Client-only, per the decision on the issue.Fixes #168
AI reviewer
DmTurninsrc/pepper_cli/tui/state.py: new_own_user_rowfinds the earliest user row not in the turn's snapshot whose stripped text equals the stripped text sent.reply_id(dropped stream, phasechecking) returns the earliest assistant row not in the snapshot withcreated_atstrictly after that user row andtrajectory_idequal to the user row's, elseNone.ChatHistoryEntryinapi_client.pynow mirrorstrajectory_id(already in the vendored schema as a required nullable field; defaults toNonewhen absent). Server side,chat_service.finalize_turnlinks the user row and writes the assistant row with the same id in one transaction, and reminders/scheduled announcements are written withtrajectory_id=None. So a linked user row matches only its own reply. Residual: while the user row is unlinked (reply not written yet, the error path, or a server without the field), a later row with no trajectory can still match. That is the limit accepted on tui: a dropped DM turn takes any new assistant row as its reply #168.reply_landednow delegates toreply_id, andfound_replyalready did, so the match is single-sourced.app._on_dm(seen set for the Pepper dot),_settle_turns(assistant_idformark_seen) and the P1b strip all usereply_id/found_reply, so they follow the new match with no app changes.checkinguntil the existing give-up window, then the existing "no reply yet" outcome. Strict>oncreated_atmeans a timestamp tie also does not match; this errs toward not guessing.user_landed: for acheckingturn it now uses the same own-row match, so another device's new user row no longer hides this turn's pending bubble.streamingkeeps the old rule; the reply frame names the user row there.tests/test_tui_state.pyadds the unrelated-row-first case, the unidentifiable-user-row case, the same-trajectory case (a reminder and another device's reply between the user row and the answer), the unlinked-user-row case, and the pending-bubble case;tests/test_api_client.pycheckstrajectory_idparsing; both fail on the old code (checked by stashing the source change). The existing tests intest_tui_state.pyandtest_tui_apps_state.pywere updated for realistic timestamps and a user row.origin/main...HEADdiff: no change-worthy findings. It re-raised the accepted limits (text-equality match, midnight, identical earlier text, the unlinked window), the decided-against ids-first option, a mootuser_id-on-settle point (a settled turn leavesturns), and a minor repeated scan per poll. /simplify: one pass, replaced sorts withmin.ruff format --check,ruff check,ty check src/pepper_cli/clean;pytest --cov: 2656 passed, 2 skipped, 95.10% coverage (floor 88).Checklist (repo-specific)
uv run ruff format --checkanduv run ruff checkclean (no new lint ignores)uv run ty check src/pepper_cli/cleanuv run pytestpasses; coverage floor in.claude/maintainerd.jsonnot loweredSKILL.mdupdated if commands or output changed: no command or output change🤖 Generated with Claude Code
https://claude.ai/code/session_01QNDeh8M8jKfHhtWGJwkxTm


Confidence Score: 5/5
The PR appears safe to merge; no new actionable issues remain.Summary
Dropped DM streams now find the reply after their own user row and require a matching
trajectory_id.In the PR description, allenhutchison accepts the remaining limits: rows without a trajectory can still match an unlinked user row, altered text or timestamp ties can prevent a match, and an earlier identical message outside the snapshot can be mistaken for the current one.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A[DM stream drops] --> B[Read history] B --> C{Matching new user row?} C -->|No| F[Keep checking until timeout] C -->|Yes| D{Later assistant row with same trajectory?} D -->|No| F D -->|Yes| E[Use reply and finish turn]Reviews (2) · Last reviewed commit: "fix(tui): a dropped turn's sending bubbl..." · Reviewed by Greptile