fix(rag-ui): distinguish a failed load from genuinely empty data (F-19) - #278
Merged
Conversation
loadDocuments/loadUsage/loadInsights on the RAG module detail page did
`if (res.ok) { ...update state... }` with no else and no persistent
error state -- a failed fetch left whatever was already in state (on
first load, the empty initial array) with no visible sign anything
went wrong. The failure was only visible in devtools; an operator
looking at the page saw "no documents" / "no queries logged" / "every
query found something" and had no way to tell that from a real empty
tenant.
All three now throw on a non-ok response, set a persistent error
message on catch (cleared on the next successful load), and the
affected sections render a distinct error state (red icon, the actual
error, a Retry button that re-runs the same load) instead of falling
through to their empty-state branch. Applies to both insights
sub-panels (score distribution chart, zero-result queries) since both
come from the same loadInsights failure.
Verification for this PR: npx tsc --noEmit, eslint, and a full
production `next build` all clean (the changed route still compiles
and is included in the build output). Did not verify in a live browser
against a real tenant/module -- this repo has no React component test
harness (@testing-library/react is not a dependency) to exercise the
new branches in an automated test either, so this change is verified
at the type/build level only, not behaviorally in a running app.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KQq6TnVNHRNU6Wz1eQzPpD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
P2 finding from the 2026-09-05 finance-institution assessment.
loadDocuments/loadUsage/loadInsightson the RAG module detail page didif (res.ok) { ...update state... }with noelseand no persistent error state — a failed fetch left whatever was already in state (on first load, the empty initial array) with no visible sign anything went wrong. The failure was only visible in devtools; an operator looking at the page saw "no documents" / "no queries logged" / "every query found something" and had no way to tell that apart from a genuinely empty tenant.All three now throw on a non-ok response, set a persistent error message on catch (cleared on the next successful load), and the affected sections render a distinct error state (red icon, the actual error message, a Retry button that re-runs the same load) instead of falling through to their empty-state branch. Applies to both insights sub-panels (score distribution chart, zero-result queries) since both come from the same
loadInsightsfailure.Explicitly out of scope for this PR
The same finding also names a
.catch(() => [])pattern inconsole-ee's cluster admin listings, a missing in-flight lock on the token-delete confirm button, and a missing provider-connectivity test flow. Each is a separate location/component with its own scope — bundling all of F-19 into one diff would make this harder to review, not easier. Flagging as follow-ups.Test plan
npx tsc --noEmitcleannpx eslintcleannext buildsucceeds — the changed route (/dashboard/rag/[key]) compiles and is included in the build outputvitest run: 5046 passed, 0 failed, 5 skipped (unrelated to this change; no regressions)@testing-library/reactisn't a dependency) to exercise the new branches in an automated test, and I did not stand up a live tenant/module to click through it manually. This change is verified at the type/lint/build level only, not behaviorally in a running app — flagging explicitly rather than claiming a level of confidence I don't have.🤖 Generated with Claude Code
https://claude.ai/code/session_01KQq6TnVNHRNU6Wz1eQzPpD