Failed evidence fetch renders as "No evidence yet."

claude-bot commented

2026-07-28 06:00:26 +00:00

Severity: HIGH

The bug

frontend/src/pages/ReviewWorkspacePage.tsx:87-118 destructures only data from useQuery, then:

{!data || data.data.length === 0 ? "No evidence yet." : ...}

Loading and error states are collapsed into the empty state. If /evidence returns a 500, or the
session has expired, the panel tells the reviewer there is no evidence — and they decide on
that basis.

The same pattern appears in CommentsTab (:295-310) and HistoryTab (:337-344).

Why it matters

"Evidence-first" is the product's stated first principle, and docs/circa-ui-spec.md §9.3/§9.5
explicitly forbid collapsing these states. A reviewer who trusts a false "No evidence yet" will
approve or dispute on wrong grounds — and the resulting decision looks completely legitimate in
the history.

This is the most dangerous five-line bug in the frontend.

Fix

Destructure isLoading and error in all three sections and render distinct states:
"Loading evidence…" and "Couldn't load evidence — Retry".
Add a global 401 handler (a QueryCache.onError hook in the client) that redirects to login.
Today an expired session mid-review just makes every panel quietly fail.

Done when

Loading, error, and empty are three visually distinct states in evidence, comments, and history
An API failure never renders as an assertion that data does not exist
An expired session redirects to login rather than failing silently
Tests cover the error state for each panel

References

frontend/src/pages/ReviewWorkspacePage.tsx:87-118,295-310,337-344
docs/circa-ui-spec.md §9.3, §9.5

## Severity: HIGH ## The bug `frontend/src/pages/ReviewWorkspacePage.tsx:87-118` destructures only `data` from `useQuery`, then: ```jsx {!data || data.data.length === 0 ? "No evidence yet." : ...} ``` Loading and error states are collapsed into the empty state. If `/evidence` returns a 500, or the session has expired, the panel tells the reviewer **there is no evidence** — and they decide on that basis. The same pattern appears in `CommentsTab` (`:295-310`) and `HistoryTab` (`:337-344`). ## Why it matters "Evidence-first" is the product's stated first principle, and `docs/circa-ui-spec.md` §9.3/§9.5 explicitly forbid collapsing these states. A reviewer who trusts a false "No evidence yet" will approve or dispute on wrong grounds — and the resulting decision looks completely legitimate in the history. This is the most dangerous five-line bug in the frontend. ## Fix - Destructure `isLoading` and `error` in all three sections and render distinct states: "Loading evidence…" and "Couldn't load evidence — Retry". - Add a global 401 handler (a `QueryCache.onError` hook in the client) that redirects to login. Today an expired session mid-review just makes every panel quietly fail. ## Done when - [ ] Loading, error, and empty are three visually distinct states in evidence, comments, and history - [ ] An API failure never renders as an assertion that data does not exist - [ ] An expired session redirects to login rather than failing silently - [ ] Tests cover the error state for each panel ## References - `frontend/src/pages/ReviewWorkspacePage.tsx:87-118,295-310,337-344` - `docs/circa-ui-spec.md` §9.3, §9.5

claude-bot added this to the v0.2.0 milestone

2026-07-28 06:00:26 +00:00

claude-bot added the

bug

frontend

labels

2026-07-28 06:00:26 +00:00

rbrooks referenced this issue from a commit

2026-08-01 06:44:57 +00:00

Stop the UI asserting absences it has not established

rbrooks closed this issue

2026-08-01 06:44:57 +00:00

claude-bot commented

2026-08-01 06:45:08 +00:00

Author

Done in 88044bc. All four "done when" items covered.

Your description of this as "the most dangerous five-line bug in the frontend" is right, and I'd add one thing: it was in four places, not three — NotesHistory (added with #66) had inherited the same shape. That's the argument for fixing it as a shared AsyncPanel rather than four corrected conditionals. The bug existed four times because the shape was duplicated four times, and a fifth panel written next month would have inherited it too.

Loading / error / empty are now genuinely distinct, error carries a Retry (the failure is usually transient and a reviewer shouldn't have to reload the workspace to find out), and error uses role="alert" so it is announced rather than merely coloured.

The 401 handler is a QueryCache + MutationCache onError, guarded against redirect loops. I also turned off retries for 4xx generally: a 401 is a settled answer, and retrying it delays the redirect while spending the auth rate limit from #68 on a session that is already gone. Worth noting this stopped being an exotic case when #61 shortened sessions to a 12-hour idle window — an expired session mid-review is now routine.

Tests: this needed frontend tooling, which didn't exist, so vitest + Testing Library + jsdom came with it and are wired into CI. That's the tooling half of #11 — I've left #11 open for the review-workspace coverage it also asks for. 10 tests, including the exact failure mode: items === undefined with no error must not render the empty state.

One related fix rides along in the dashboard (#83): a dash now means "could not find out" and zero means zero, which is the same principle applied to a different panel.

Done in 88044bc. All four "done when" items covered. Your description of this as "the most dangerous five-line bug in the frontend" is right, and I'd add one thing: it was in **four** places, not three — `NotesHistory` (added with #66) had inherited the same shape. That's the argument for fixing it as a shared `AsyncPanel` rather than four corrected conditionals. The bug existed four times because the shape was duplicated four times, and a fifth panel written next month would have inherited it too. Loading / error / empty are now genuinely distinct, error carries a **Retry** (the failure is usually transient and a reviewer shouldn't have to reload the workspace to find out), and error uses `role="alert"` so it is announced rather than merely coloured. **The 401 handler is a `QueryCache` + `MutationCache` `onError`**, guarded against redirect loops. I also turned off retries for 4xx generally: a 401 is a settled answer, and retrying it delays the redirect while spending the auth rate limit from #68 on a session that is already gone. Worth noting this stopped being an exotic case when #61 shortened sessions to a 12-hour idle window — an expired session mid-review is now routine. **Tests:** this needed frontend tooling, which didn't exist, so vitest + Testing Library + jsdom came with it and are wired into CI. That's the tooling half of #11 — I've left #11 open for the review-workspace coverage it also asks for. 10 tests, including the exact failure mode: `items === undefined` with no error must **not** render the empty state. One related fix rides along in the dashboard (#83): a dash now means "could not find out" and zero means zero, which is the same principle applied to a different panel.

claude-bot referenced this issue

2026-08-01 06:45:20 +00:00

Dashboard stat cards read a field the API never returns #83

rbrooks referenced this issue from a commit

2026-08-01 07:48:31 +00:00

Check the API schema without depending on git

claude-bot referenced this issue

2026-08-01 07:54:34 +00:00

Add Pydantic response models and generate frontend types from OpenAPI #76

rbrooks referenced this issue from a commit

2026-08-01 21:22:32 +00:00

Read the back of the photograph