Repository navigation
fix(tui): drop cached unread rows the server no longer lists - #175
allenhutchison wants to merge 1 commit into
Conversation
merge_unread only upserted, so a topic absent from a later unread answer kept its old row. When the server answered the unread request, its rows are now the whole snapshot and the rest are dropped (and a snapshot without a total clears the cached total). A poll that did not ask, or whose request failed, still keeps what is cached. The rail shows dots only; the unread total is not displayed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lk5Upqk2oCRiV3ZENbLyLL
Vycari Review (shadow)Shadow round 1 on 9d68355. 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 0 · withdrawn 0 Judge's notes I found two P1 problems where a bad answer from the server is treated as "nothing unread". First, a 200 answer with no usable topics list clears every cached unread dot. Second, a topic whose row has a malformed count is dropped from the parsed list, so the snapshot erases its dot. In both cases the user is told rooms are caught up when they still have unread messages. P1 · VR-09d47854 · A 200 unread answer with no usable topics list clears every cached unread dotsrc/pepper_cli/tui/state.py lines 183–187 · symbol ReadMarks.merge_unread · lens fail-open · status new The snapshot prune treats any successful unread answer as the server's complete list. api_client.get_groups_unread (api_client.py:3339-3340) builds the rows with body.get('topics') or [], so a 200 body that has no 'topics' key, or a non-list value, parses to an empty list. poller.step then sets unread_answered=True (poller.py:129), and merge_unread(snapshot=True) deletes every cached row and counted_from entry (state.py:183-187) and clears unread_total (state.py:191-192). The client cannot tell 'nothing unread' from 'the body was not the expected shape'. Before this PR the same body left the cache untouched. Evidence: poller.py:113-129: unread_answered=fetched is not None, and _fetch_unread returns any 200 body. api_client.py:3339-3349: body.get('topics') or [] and no check that the body has the expected shape; the response is untyped (MIRROR comment, no snapshot schema). state.py:183-192: snapshot deletes rows not in the parsed list and clears unread_total. I looked for a degraded or shape guard on the unread answer in poller.py, app.py _apply_groups and api_client.py and found none. The PR body's own assumption ('the server lists every topic with unread messages') is unverified in this repo, so the trigger is not proven, but the harm when it happens is wrong dots on every room. Also reported here: Malformed unread rows erase existing unread dots: Snapshot pruning treats absence from the parsed rows as absence from the server response. However, get_groups_unread silently discards rows with malformed counts. A topic explicitly present in the response can therefore lose its cached unread state without any successful count or read establishing that it is read. Evidence: api_client.py:1300-1304 rejects non-integer counts; api_client.py:3339-3348 silently filters rejected rows while returning GroupsUnread successfully. This behavior is explicitly exercised by tests/test_api_client_groups.py:820-852. poller.py:123-131 treats that return as complete and sets unread_answered=true; app.py:761-765 forwards it as snapshot=true. state.py:183-187 then deletes the rejected topic's cached row. This is distinct from an HTTP or transport failure, which preserves the cache, and requires no assumption that the server legitimately omits unread topics. Failure scenario The server answers GET /api/v1/groups/unread with 200 and a body without a 'topics' array (a different envelope, a wrapper or proxy body, or a degraded answer that omits rows). The rail's room signature moved, so the poll requests the unread route. The answer parses to no rows. Every topic in every room loses its dot in the sidebar and the terminal title's unread marker clears, although the server still lists unread messages. The dots come back only when the next room moves and a parseable answer arrives. The user sees 'all caught up' with unread messages present. How this was verified (traced-callers) Read api_client.py:3339-3350, which uses body.get('topics') or [] with no shape check. Read poller.py:113-129 and found no guard between a 200 answer and unread_answered=True. Reachable from: RailPoller.step called from the app rail poll. Proving test |
|
On the open question in the body (does the server list every unread topic?): yes. Groups' |
Human overview
The TUI cached unread rows only ever grew: a topic missing from a later
GET /api/v1/groups/unreadanswer kept its old dot. When the server answers, its rows are now the whole snapshot and unlisted topics are dropped. Per the decision on the issue, the rail shows dots only andunread_totalis not displayed.Closes #155
AI reviewer
Change:
RailUpdate.unread_answered(poller.py) is true only when the unread request came back. An emptyunreadotherwise means "not asked" (no room moved) or "failed", not "nothing unread".ReadMarks.merge_unread(..., snapshot=False)(state.py): withsnapshot=True, cached rows (and theircounted_from) for topics not in the answer are deleted before the upsert; a snapshot without a total also clearsunread_totalrather than leaving a stale one. Default behavior is unchanged, so partial callers and existing tests are unaffected.app._apply_groupspassessnapshot=update.unread_answered.Not touched: the display of
unread_total(declined on the issue, #155);settle().Relation to #174 (the 404 back-off): independent. Both edit
RailPoller.step/RailUpdate, but this branch is cut from main and does not need that change. During a 404 back-offfetchedisNone, sounread_answeredis false and no rows are pruned, which is the wanted result; the two PRs touch different lines, but whichever merges second may need a trivial rebase.Assumption to check: the server lists every topic with unread messages (and may omit read topics). I could not confirm this from this repo (the route is mirrored from pepper with no schema). If the server ever caps or omits topics it could not evaluate, pruning would clear their dots.
Also: a row the client rejects as malformed (non-int count) is absent from the parsed list, so a snapshot would drop that topic's cached row instead of leaving it.
Verification:
counted_from, a partial answer keeps them, snapshot clears a stale total, andunread_answeredis true/false in the right polls.uv run ruff format --check,ruff check,ty check src/pepper_cli/clean;uv run pytest --cov --cov-report=json:coverage-summary.json: 2659 passed, 2 skipped.RailUpdate, which I kept becauseunread=[]cannot say "not asked".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.md: CHANGELOG updated; no command or README-visible change🤖 Generated with Claude Code
https://claude.ai/code/session_01Lk5Upqk2oCRiV3ZENbLyLL


Confidence Score: 5/5
The PR appears safe to merge; no new actionable issue was found.Summary
Successful unread answers now replace the cached topic list instead of only adding rows.
Acknowledged by allenhutchison: the server's answer is assumed to list every unread topic; malformed rows are dropped before pruning; and the extra
RailUpdateflag is intentional because an empty list cannot distinguish “not asked” from “nothing unread.” These were not reported again.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Rail poll] --> B{Unread request answered?} B -->|No| C[Keep cached rows] B -->|Yes| D[Remove topics missing from answer] D --> E[Save returned rows and total] C --> F[Apply existing read rules] E --> FReviews (1) · Last reviewed commit: "fix(tui): drop cached unread rows the se..." · Reviewed by Greptile