Repository navigation
feat(tui): read unread from the server's rows, retire seed_first_sight - #148
Conversation
Read-state-sync phase 5c. The TUI shows the per-topic `unread` rows from
GET /api/v1/groups/{gid}/read (unread_count, has_unread_mention) instead of
the local "activity later than the mark" rule. ReadMarks becomes a cache of
the server's answer; read_state.advanced and the poll fallback still update
it. seed_first_sight, the open-topic exception and the per-topic mention
fetch are removed.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Lk5Upqk2oCRiV3ZENbLyLL
Vycari Review (shadow)Shadow round 2 on 469f869. 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 4 · P2 0 · confirmed fixed 0 · withdrawn 0 Judge's notes Incomplete: 7 stage call(s) failed (hunter:concurrency-time, hunter:cross-file-contract, hunter:fail-open, hunter:security-identity-privacy, hunter:spec-adversary, judge, refuter:1). 4 finding(s) kept without a judge pass. P1 · VR-022754d6 · Crossing reads leave consumed mentions flagged indefinitelysrc/pepper_cli/tui/state.py lines 160–163 · symbol ReadMarks.merge_unread · lens concurrency-time · status new Keeping an answer counted from an older cursor preserves unread messages, but also preserves mention flags for messages already read. When the local cursor only partially covers that answer, merging it does not request a fresh answer. Completion of the local PUT and its equal-cursor stream event do not trigger that refresh either. Evidence: merge_unread unconditionally installs the stale flag at state.py:160-163, while settle leaves partial reads untouched at state.py:196-198. The only partial-read refetch requires merge_advanced to advance the cursor at app.py:644-657; equal cursors return None at state.py:179. _published_ok merges the cursor without requesting unread state at app.py:1076-1082. RailPoller.step skips unchanged signatures at poller.py:102. This differs from the prior discarded-row defect: accepting the row restores unread visibility but now leaves already-consumed mention information indefinitely. Failure scenario The server cursor is T9. The user views a mention at T10, raising the local cursor and clearing its marker while the PUT is pending, then leaves. An unmentioned message T11 arrives. The rail fetches activity T11 and an unread row counted from T9 containing both messages with the mention flag set. settle cannot clear it because T10 is below T11. The PUT then commits T10, but its event cannot advance the existing local cursor. Subsequent polls skip the unchanged room, leaving a false mention marker and potentially a false bell until another activity or full catch-up. How this was verified (read-only-reasoning) Not verified: refuter call failed. Proving test P1 · VR-159c012c · Rejected unread rows can stay missing indefinitelysrc/pepper_cli/tui/state.py lines 155–159 · symbol ReadMarks.merge_unread · lens concurrency-time · status still-open The stale-cursor guard discards the unread answer without requesting another fetch. The rail still commits that room's signature and ETag, so subsequent polls need not fetch its read state again. Evidence: Local viewing advances the cursor before publication at src/pepper_cli/tui/app.py:1039. The answer is skipped at src/pepper_cli/tui/state.py:158-159, but app.py:698 commits the poller result. poller.py:102 and 123-124 retain the activity signature and ETag even when the app rejects a row. Publication success only merges the cursor at app.py:1076-1078; an equal stream cursor returns no topic at state.py:176, bypassing the refetch branch at app.py:645-657. Thus neither publication nor ordinary polling guarantees the promised next answer. Failure scenario Viewing a topic raises the local cursor to message T10 while its PUT remains in flight. The user leaves, and another person posts T11. A rail fetch sees activity T11 but reads the server's older cursor T9, so its positive unread row is discarded and the cached zero remains. The PUT then completes at T10. Its stream event cannot advance the already-held cursor, and further polls skip the unchanged room. T11 remains unbadged until another activity or full catch-up occurs. How this was verified (read-only-reasoning) Not verified: refuter call failed. Proving test P1 · VR-691f341c · Stale activity clears a genuinely unread messagesrc/pepper_cli/tui/state.py lines 196–202 · symbol ReadMarks.settle · lens concurrency-time · status new A cursor newer than counted_from still cannot establish that it covers the unread answer: the topic activity was fetched before that answer. When a local read crosses the request, settle can clear messages posted after that read. Evidence: RailPoller.step fetches groups before read state at src/pepper_cli/tui/poller.py:96-104. PepperApp._apply_groups immediately settles the combined result at src/pepper_cli/tui/app.py:711-713. The guard at state.py:199-200 excludes only an equal server cursor; an older server cursor permits the erroneous clear despite the same stale-activity race. tests/test_tui_state.py:102-116 covers the equal-cursor case, not this crossing local read. Failure scenario With polling fallback active, the user reads through T10 while its PUT remains pending, then leaves the topic. A rail request fetches topic activity T10. Another person posts T11 before the subsequent read-state request, which returns a positive count computed from server cursor T9. The local cursor T10 exceeds T9 and equals the stale activity, so settle zeros the entire row, hiding T11 until another rail poll. How this was verified (read-only-reasoning) Not verified: refuter call failed. Proving test P1 · VR-bdc0a18b · Archived topics trigger invisible mention alertssrc/pepper_cli/tui/app.py lines 711–715 · symbol PepperApp._apply_groups · lens cross-file-contract · status still-open The bell compares raw mentioned-topic sets, which include archived topics. It bypasses the archive filtering used for sidebar markers and the terminal title. Evidence: src/pepper_cli/tui/state.py:215-217 includes every true mention flag in mentioned_topics, while state.py:198-207 explicitly excludes archived topics from visible unread and mention state. app.py:714 uses the unfiltered set to ring. settle does not remove the flag when the cursor precedes activity (state.py:190-194). Before this change, mention checks iterated topics_by_recency, which excludes archived topics; the new server-row path makes this alert reachable. Failure scenario With the bell enabled, a read-state response includes an unread mention in an archived topic whose cursor is behind its activity. The application rings even though that topic is excluded from unread navigation and produces no unread or mention marker. How this was verified (read-only-reasoning) Not verified: refuter call failed. Proving test |
…_marks Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Lk5Upqk2oCRiV3ZENbLyLL
The rail reads /groups before /read, so a message landing between the two left the room's activity at the cursor while the answer counted it; settle then zeroed the fresh row. A row now remembers the cursor the server counted from, and only a cursor that moved after it can clear the row. The same rule replaces merge_unread's stale-cursor skip, which dropped a fresh row (and a message newer than the local read) until the room moved again: every row is kept, and the newer local cursor settles it. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Lk5Upqk2oCRiV3ZENbLyLL
mentioned_topics read raw rows, so an unread mention in an archived topic rang the bell with no marker on screen. It now goes through is_mentioned. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Lk5Upqk2oCRiV3ZENbLyLL
The fake server's unread rule parses timestamps with parse_ts like the client does; merge_server takes a Mapping so merge_unread passes marks through; the bell's before-snapshot is skipped when the bell is off. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Lk5Upqk2oCRiV3ZENbLyLL
|
Round 2 summary (head 469f869):
|
Human overview
The terminal UI no longer decides what is unread. Dots and
@markers come from the server'sunreadrows (the same answer the web uses), so a fresh profile matches the web and your own messages never badge. The local read marks are now a cache of that answer;read_state.advancedand the polling fallback are unchanged.Closes #145
Human required (optional)
unreadrows (older than phase 5b) now shows no markers at all, where the local rule used to light them. Phase 5b is live, so I treated this as acceptable.AI reviewer
What changed
api_client.py:get_group_read_marksbecomesget_group_read_state, returning marks plusGroupUnreadRows parsed tolerantly (missing key, malformed rows dropped).tui/state.py:ReadMarksholds the server's rows.is_unreadisunread_count > 0and not archived; a topic with no row is not unread. Removed:seed_first_sight,scrolled_up, theopen_topic_idparameters, andMentions(the per-topic message fetch).has_unread_mentionreplaces it.ReadMarks.settledrops a cached row when a cursor (own mark orread_state.advanced) is at or after the topic'slast_activity_atand newer than the cursor the server counted that row from (ReadMarks.counted_from). The rail reads/groupsbefore/read, so the activity can be older than the answer; a cursor the server already counted from adds nothing. A partial advance leaves the count and callsrefetch_all.merge_unreadkeeps every row, including one counted from a cursor older than a local one (the local cursor settles it).mentioned_topics(rooms)goes throughis_mentioned, so archived topics never ring).tui/poller.py,tui/app.py: rows flow throughRailUpdate.unread; thementionsloop is gone; the bell rings when a non-open topic gainshas_unread_mention.planning/terminal-ui.mdsection 6.3, CHANGELOG (Unreleased).Alternatives rejected. Using
GET /api/v1/groups/unread(one call plusunread_total): the vendored contract snapshot does not list the path yet, and CLAUDE.md orders server, snapshot refresh, then CLI. Filed as #147. The TUI shows no count today, so there is no total to source from the server yet.Look hardest at. (1) The open topic is no longer exempt from the rule: while open at the bottom it shows the server's count until
_note_viewedsettles it (a local clear, before the PUT answers). (2) A partialread_state.advancedcallsrefetch_all, which re-reads every room's/read. (3)settle's answer-relative guard:tests/test_tui_state.pycovers the/groups-then-/readrace, an answer that crossed a local read (kept, settled by it), and one that still counts a message past that read.code-review (medium, worktree target): 8 findings; fixed: unused
currentonany_unread, and the stale-row guard inmerge_unread(it dropped a fresh row until the room moved again). Left as design choices or accepted behaviour: stale-row handling when the server omits a topic (golden/readalways sends a row per topic), the cost in (2), open-topic dot in (1),you_person_idplumbing still used for thread rows, and float counts being dropped. Bell re-fire after a local clear no longer happens within one rail answer (settle runs before the bell compares)./simplifyrun in round 2: fake server usesparse_ts,merge_servertakes aMapping, bell snapshot skipped when off; skipped droppingGroupUnreadRow.floor(a mirror carries every server field) and a per-room refetch on partial reads (the cost in (2)).Verification (actual).
ruff format --check,ruff check,ty check src/pepper_cli/clean.uv run pytest --cov(round 2 head): 2484 passed, 2 skipped, 1 unrelated flake intest_claude_channel.pythat passes alone (filed #149); coverage 95.02% against floor 88. New tests: fresh profile matches server (tests/test_tui_unread.py), no self-badge (newest message is yours, activity past the cursor, no dot; plus a send), an advance clears (full read clears from the frame, partial read asks again, viewing clears before the PUT answers), mentions from the flag, API client parsing, poller rows. The fake server intests/tui_support.pynow computesunreadthe way groups does.Deferred. Adopt
GET /api/v1/groups/unreadandunread_total: #147.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 lowered/api/v1usage stays additive-only,# MIRROR:comments updated, unknown fields and frames still toleratedSKILL.mdupdated if commands or output changed (README updated; SKILL.md does not mention unread, so unchanged)🤖 Generated with Claude Code
https://claude.ai/code/session_01Lk5Upqk2oCRiV3ZENbLyLL


Confidence Score: 5/5
The PR appears safe to merge, with a non-blocking unread-marker race still partly unresolved.Fix with agent prompt
Summary
The terminal UI now shows unread dots and mention markers from the server’s unread rows instead of deriving them locally.
In the PR description, allenhutchison accepts the older-server fallback, omitted-row behavior, stale-guard edge, full-room refetch cost, temporary open-topic dots, bell repeats after a local clear, retained person-ID plumbing, and dropping float counts as design choices or accepted behavior. allenhutchison also defers the aggregate unread endpoint to #147.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A["Rail reads rooms"] --> B["Read each changed room's marks and unread rows"] B --> C["Merge server rows and keep cursors moving forward"] D["Local viewing or read_state.advanced"] --> C C --> E["Settle cached rows against room activity"] E --> F["Draw dots, mentions, and terminal title"] F --> G["Ring for newly displayed mentions outside the open topic"]Reviews (2) · Last reviewed commit: "refactor(tui): simplify pass on the unre..." · Reviewed by Greptile