The staging E2E gate for #11 failed on the new mobile compare-chart test: the chart canvas never appeared on staging. Root cause is not the chart code but a pre-existing fragility it exposed.
Mechanism
app/compare/page.tsx fetches comparison data server-side (internal FASTAPI_URL, which works on staging) and passes it as initialData, so the chart has data to render.
ComparisonView's client useEffect then refetches /api/compare from the browser. On staging the external /api edge proxy 500s (known issue), and the catch did setComparisonData(null) — wiping the good SSR data, so the chart section disappeared.
On production the refetch succeeds, so this was invisible there — but any transient network failure would blank a working comparison for a real user too.
Fix
On refetch failure, keep whatever data we already have (SSR data or a previous successful fetch) instead of nulling it. Strictly better UX, and it lets the chart render on staging so the promotion gate passes.
Verified: production compare renders the canvas with zero console errors; staging returns 500 on the client /api/compare and (before this fix) drops the chart. Typecheck + 33 unit tests pass.
The staging E2E gate for #11 failed on the new mobile compare-chart test: the chart canvas never appeared on staging. Root cause is not the chart code but a pre-existing fragility it exposed.
**Mechanism**
- `app/compare/page.tsx` fetches comparison data server-side (internal `FASTAPI_URL`, which works on staging) and passes it as `initialData`, so the chart has data to render.
- `ComparisonView`'s client `useEffect` then refetches `/api/compare` from the browser. On staging the external `/api` edge proxy 500s (known issue), and the catch did `setComparisonData(null)` — wiping the good SSR data, so the chart section disappeared.
- On production the refetch succeeds, so this was invisible there — but any transient network failure would blank a working comparison for a real user too.
**Fix**
On refetch failure, keep whatever data we already have (SSR data or a previous successful fetch) instead of nulling it. Strictly better UX, and it lets the chart render on staging so the promotion gate passes.
Verified: production compare renders the canvas with zero console errors; staging returns 500 on the client `/api/compare` and (before this fix) drops the chart. Typecheck + 33 unit tests pass.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
The compare view refetches /api/compare on the client after SSR; on any
failure the catch nulled comparisonData, destroying the working
SSR-provided chart. A transient error (or staging's broken external /api
proxy) should not blank a comparison the user is already viewing — keep
the existing data instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This small diff changes ComparisonView's refetch error handler to keep the existing comparisonData instead of clearing it to null, avoiding a flash-to-empty when a transient refetch fails. The intent (preserve a working chart across a blip) is reasonable, but it introduces a staleness edge case with no user-visible feedback.
🟡 Minor
nextjs-app/components/ComparisonView.tsx: When the school selection itself changes (add/remove) and the refetch then fails, the component silently keeps rendering comparisonData keyed to the previous selection with no error indicator to the user. E.g. adding a school while a fetch fails leaves that school's card visible but absent from the chart/table with no explanation, and there is no retry/error UI — the failure is only logged to console.
## 🤖 AI Code Review (Claude Code)
This small diff changes ComparisonView's refetch error handler to keep the existing comparisonData instead of clearing it to null, avoiding a flash-to-empty when a transient refetch fails. The intent (preserve a working chart across a blip) is reasonable, but it introduces a staleness edge case with no user-visible feedback.
### 🟡 Minor
- **nextjs-app/components/ComparisonView.tsx**: When the school selection itself changes (add/remove) and the refetch then fails, the component silently keeps rendering comparisonData keyed to the previous selection with no error indicator to the user. E.g. adding a school while a fetch fails leaves that school's card visible but absent from the chart/table with no explanation, and there is no retry/error UI — the failure is only logged to console.
tudor
merged commit 694b6013b3 into main2026-07-06 07:27:04 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The staging E2E gate for #11 failed on the new mobile compare-chart test: the chart canvas never appeared on staging. Root cause is not the chart code but a pre-existing fragility it exposed.
Mechanism
app/compare/page.tsxfetches comparison data server-side (internalFASTAPI_URL, which works on staging) and passes it asinitialData, so the chart has data to render.ComparisonView's clientuseEffectthen refetches/api/comparefrom the browser. On staging the external/apiedge proxy 500s (known issue), and the catch didsetComparisonData(null)— wiping the good SSR data, so the chart section disappeared.Fix
On refetch failure, keep whatever data we already have (SSR data or a previous successful fetch) instead of nulling it. Strictly better UX, and it lets the chart render on staging so the promotion gate passes.
Verified: production compare renders the canvas with zero console errors; staging returns 500 on the client
/api/compareand (before this fix) drops the chart. Typecheck + 33 unit tests pass.🤖 Generated with Claude Code
🤖 AI Code Review (Claude Code)
This small diff changes ComparisonView's refetch error handler to keep the existing comparisonData instead of clearing it to null, avoiding a flash-to-empty when a transient refetch fails. The intent (preserve a working chart across a blip) is reasonable, but it introduces a staleness edge case with no user-visible feedback.
🟡 Minor