[allenhutchison]

Human overview

Against a server without GET /api/v1/groups/unread, the rail made a full room-list 200 plus a failing request on every poll and never kept the ETag. A 404 now holds for ten minutes, keeps the ETag so unchanged polls are 304 again, and then one full poll asks once more. 401 and transport errors behave as before.

Closes #154

AI reviewer

Change (tui/poller.py, RailPoller):

• _fetch_unread sets _unread_retry_at = monotonic() + UNREAD_MISSING_RETRY_SECONDS (600) on a 404 and clears it on a success. 401 still raises; transport errors still propagate; other statuses still return None (asked again next poll, ETag dropped).
• step: while the back-off is active the unread request is not made (asking is false). A 404 from this very request counts as an answer: complete is true, so the ETag is kept, seen is advanced and a pending refetch_all is answered. Once the back-off expires the step is forced full (a 200), so the route is retried even if no room moved. If an expired back-off meets an empty room list, nothing is asked, so the back-off is cleared instead of forcing a 200 on every poll.
• complete uses _unread_retry_at != retry_at (changed during this request), so a stale deadline cannot excuse a 500 on a retry.

Alternative not taken: a terminal "never ask again this session". Ten minutes lets a TUI left open across a server upgrade pick the route up without a restart, at the price of one extra failing request per interval.

Known limits: _unread_retry_at is written inside step (worker thread), not in commit, unlike the ETag and seen. Steps do not overlap, and the state is only a "do not ask yet" hint, so a discarded result at worst skips or repeats one request. Moving it into RailUpdate would be a wider change. The back-off is per process and in memory.

Docs: README unread paragraph and CHANGELOG entry updated.

Verification:

• New tests in tests/test_tui_poller.py: 404 keeps the ETag and answers refetch_all, no request during the window, retry after expiry (success and 404), expired back-off with no rooms ends, 500 after a 404 is not an answer. They use a fake clock.
• uv run ruff format --check, ruff check, ty check src/pepper_cli/ clean; uv run pytest --cov --cov-report=json:coverage-summary.json: 2660 passed, 2 skipped.
• Simplify: diff reviewed directly, nothing to change. code-review (medium): 4 findings, 2 fixed (stale deadline excusing a 500; endless 200s on an empty list after expiry, both now tested), 1 test-clock finding fixed (clock patched on the module reference only), 1 not fixed (write outside commit, above).

Checklist (repo-specific)

• uv run ruff format --check and uv run ruff check clean (no new lint ignores)
• uv run ty check src/pepper_cli/ clean
• uv run pytest passes; coverage floor in .claude/maintainerd.json not lowered
• Server-contract changes: none; the route and its 404 are unchanged
• No token or credential is logged, printed, or put in a fixture
• README and the bundled SKILL.md: README and CHANGELOG updated; SKILL.md has no TUI behavior

🤖 Generated with Claude Code

https://claude.ai/code/session_01Lk5Upqk2oCRiV3ZENbLyLL

Confidence Score: 5/5

The PR appears safe to merge.

Summary

Adds a ten-minute pause after GET /api/v1/groups/unread returns 404.

• The TUI rail retries a missing unread route after ten minutes.

No actionable issues found. allenhutchison explicitly acknowledged that _unread_retry_at changes inside step, rather than commit, and deferred moving it because steps do not overlap and discarded results only affect request timing. Tests were inspected, not run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Poll room list] --> B{Unread pause active?}
  B -->|Yes| C[Skip unread request and keep ETag]
  B -->|No, deadline expired| D[Force full room-list poll]
  D --> E{Any rooms?}
  E -->|No| F[Clear deadline]
  E -->|Yes| G[Request unread rows]
  G --> H{Response}
  H -->|404| I[Pause for ten minutes and keep ETag]
  H -->|Success| J[Clear deadline and apply unread rows]
  H -->|Other nonfatal error| K[Drop ETag and retry next poll]

Loading

_{Reviews (1) · Last reviewed commit: "fix(tui): back off the unread route when..." · Reviewed by Greptile}

[vycari-review]

Vycari Review (shadow)

Shadow round 1 on 0f7855d. 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

The back-off works for servers that really lack the unread route. One narrow P1 remains: a transient 404 on a route that exists is taken as a final answer. The pending refetch is then recorded as done, and unread markers can stay stale for up to ten minutes.

P1 · VR-7f685da2 · A transient 404 on an existing unread route stops unread refresh for ten minutes and answers a pending refetch_all

src/pepper_cli/tui/poller.py lines 166–176 · symbol RailPoller._fetch_unread · lens fail-open · status new

_fetch_unread treats any 404 from GET /api/v1/groups/unread as 'route missing' and sets _unread_retry_at for 600 s. step() then treats that 404 as an answer (complete=True via answered_404), commits seen for every room, and returns full=asked, so a pending refetch_all (the catch-up that re-reads every room's marks after a read on another device) is marked done without any marks having been read. For the next ten minutes, in-window steps never call the unread route (asking is False), so marks that moved on another device are not refreshed until the expiry-forced full poll.

Evidence: poller.py:166-176 (404 sets retry deadline and returns None), 120-129 (asking=False in window; answered_404 makes complete True), 145-147 (etag kept and full=asked when complete). api_client.py:3335-3336 documents 404 as the no-route answer only. The PR body says 'Only a 404 from this very request counts', so the transient case is not guarded. No check distinguishes a missing route from a transient 404 (no retry, no body inspection). Severity is limited: the stale state is bounded to ten minutes and only arises when a server returns 404 on an existing route.

Failure scenario

Server has the route. A read happens on another device, the app calls refetch_all(). The unread request hits a transient 404 (gateway or deploy hiccup, or a server 404 for a reason other than a missing route). step() returns complete with full=asked and the ETag kept. The rail's unread markers and server marks stay as before for up to 600 s; room-list changes during the window update seen without reading unread state. The user sees stale unread markers for up to ten minutes with no indication, and the refetch request is recorded as satisfied.

How this was verified (read-only-reasoning)

I read the diff of _fetch_unread and step. I did not open the checkout files or the api_client lines the candidate cites. The behaviour is deliberate and documented in the PR, and the only open question is how likely a spurious 404 is.

Proving test

Fake server with the unread route failing 404 once then succeeding with a changed mark for a topic. Set a fake clock. rail.refetch_all(); first=rail.step(); commit(first). Assert first.full == asked (refetch recorded done) and first.server_marks is empty. Advance the clock by 60 s and call rail.step() with no refetch; assert the unread route is not requested and the new mark is absent. The assertion that the mark appears within one poll fails on the PR head.