From 73a82fb767101682c1c44cbf47ec67231c982e88 Mon Sep 17 00:00:00 2001 From: Antoine Marot Date: Mon, 3 Aug 2026 08:19:44 +0000 Subject: [PATCH] fix(game): survive the pdf-only analysis result window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Game Mode crashed with "TypeError: Cannot convert undefined or null to object" whenever the operator ran Analyze & Suggest, right as the overflow graph became available. Step 2 streams a ``pdf`` event (emitted as soon as the overflow-graph file is written) seconds before the ``result`` event that carries the actions. The pdf branch merged into the previous result: setResult(p => ({ ...(p || {}), pdf_url, pdf_path, ... })) On a first analysis ``p`` is null, so the resulting object had NO ``actions`` key — despite ``AnalysisResult`` declaring it required (the ``as AnalysisResult`` cast hid it from the type checker). The Game Mode snapshot effect then ran ``Object.keys(result.actions)`` on that intermediate state and threw, taking down the app through the error boundary. The classic workspace was unaffected because that effect is behind ``gameBridge.isGameMode()``. Seed ``actions`` on the pdf merge so a partial result is always structurally valid, and harden the two Game Mode consumers that read the map unguarded (the snapshot effect and ``buildChosenActionRecord`` / ``combinedBeatsUnderlying``, which would have thrown next once an action was starred). Both regression tests fail without the fix. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01FSWwhkaPiAsxasECiBws8k Signed-off-by: Antoine Marot --- frontend/src/App.tsx | 2 +- frontend/src/game/solutionLog.test.ts | 10 +++++++++ frontend/src/game/solutionLog.ts | 4 ++-- frontend/src/hooks/useAnalysis.test.ts | 29 ++++++++++++++++++++++++++ frontend/src/hooks/useAnalysis.ts | 8 +++++++ 5 files changed, 50 insertions(+), 3 deletions(-) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index e0750155..efe12c3a 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1202,7 +1202,7 @@ function App() { // Every action that has a materialised result — recommender-suggested, // manually simulated, or lever-driven. The hints panel marks these // levers "simulated" and blocks a redundant re-run. - simulatedActionIds: result ? Object.keys(result.actions) : [], + simulatedActionIds: result?.actions ? Object.keys(result.actions) : [], }); }, [result, selectedActionIds, selectedContingency, n1Diagram]); diff --git a/frontend/src/game/solutionLog.test.ts b/frontend/src/game/solutionLog.test.ts index d5d1188b..4d729db8 100644 --- a/frontend/src/game/solutionLog.test.ts +++ b/frontend/src/game/solutionLog.test.ts @@ -146,6 +146,16 @@ describe('buildChosenActionRecord', () => { expect(rec.effective).toBe(false); }); + it('tolerates a partial result with no actions map (pdf-only stream window)', () => { + // The step-2 stream publishes the overflow-graph `pdf` event before the + // `result` event, so a non-null result with no `actions` map is a real + // intermediate state the Game Mode snapshot effect renders through. + const partial = { pdf_path: null, pdf_url: '/results/pdf/g.html' } as unknown as AnalysisResult; + expect(() => buildChosenActionRecord('a1', partial, 1.2)).not.toThrow(); + expect(() => buildChosenActionRecord('a1+a2', partial, 1.2)).not.toThrow(); + expect(buildChosenActionRecord('a1', partial, 1.2).maxRho).toBeNull(); + }); + it('marks an action ineffective when it does not beat the baseline', () => { const result = analysisResult({ // Still overloaded AND worse than doing nothing. diff --git a/frontend/src/game/solutionLog.ts b/frontend/src/game/solutionLog.ts index cd7aaa10..38a86333 100644 --- a/frontend/src/game/solutionLog.ts +++ b/frontend/src/game/solutionLog.ts @@ -107,7 +107,7 @@ function combinedBeatsUnderlying( ): boolean { if (!actionId.includes('+')) return true; const partRhos = actionId.split('+') - .map((part) => result?.actions[part.trim()]?.max_rho) + .map((part) => result?.actions?.[part.trim()]?.max_rho) .filter((rho): rho is number => typeof rho === 'number'); if (!partRhos.length) return true; return maxRho <= Math.min(...partRhos) - COMBINED_MIN_RHO_GAIN; @@ -119,7 +119,7 @@ export function buildChosenActionRecord( result: AnalysisResult | null, baselineMaxRho: number | null, ): ChosenActionRecord { - const detail = result?.actions[actionId]; + const detail = result?.actions?.[actionId]; const maxRho = detail?.max_rho ?? null; const after = detail?.lines_overloaded_after; const solved = maxRho != null && maxRho < 1.0 && (!after || after.length === 0); diff --git a/frontend/src/hooks/useAnalysis.test.ts b/frontend/src/hooks/useAnalysis.test.ts index 2fc7a6e9..6abff799 100644 --- a/frontend/src/hooks/useAnalysis.test.ts +++ b/frontend/src/hooks/useAnalysis.test.ts @@ -299,6 +299,35 @@ describe('useAnalysis', () => { expect(result.current.result?.pdf_url).toBe('/results/pdf/graph.pdf'); }); + it('keeps an `actions` map on the intermediate pdf-only result', async () => { + // The ``pdf`` event lands seconds before the ``result`` event (the + // overflow graph is written while the recommender still filters + // actions). On a first analysis the previous result is null, so the + // merged object used to carry NO `actions` key — and every consumer + // reading `result.actions` unguarded (Game Mode's snapshot effect + // published `Object.keys(result.actions)`) crashed on that window. + mockRunAnalysisStep1.mockResolvedValue({ + can_proceed: true, message: '', lines_overloaded: ['LINE_A'], + }); + + const pdfEvent = JSON.stringify({ + type: 'pdf', pdf_url: '/results/pdf/graph.html', pdf_path: '/tmp/graph.html', + }); + mockRunAnalysisStep2Stream.mockResolvedValue({ + ok: true, body: makeStream(`${pdfEvent}\n`), + }); + + const { result } = renderHook(() => useAnalysis()); + + await act(async () => { + await result.current.handleRunAnalysis(['LINE_X'], vi.fn(), vi.fn()); + }); + + expect(result.current.result?.pdf_url).toBe('/results/pdf/graph.html'); + expect(result.current.result?.actions).toEqual({}); + expect(() => Object.keys(result.current.result!.actions)).not.toThrow(); + }); + it('sets error on stream error event', async () => { const detected = ['LINE_A']; mockRunAnalysisStep1.mockResolvedValue({ diff --git a/frontend/src/hooks/useAnalysis.ts b/frontend/src/hooks/useAnalysis.ts index c537b852..5d074fb9 100644 --- a/frontend/src/hooks/useAnalysis.ts +++ b/frontend/src/hooks/useAnalysis.ts @@ -206,6 +206,14 @@ export function useAnalysis(): AnalysisState { if (event.type === 'pdf') { setResult((p: AnalysisResult | null) => ({ ...(p || {}), + // The ``pdf`` event lands BEFORE the ``result`` event, so on a + // first analysis `p` is null and the merged object would carry + // no `actions` map at all — even though `AnalysisResult` declares + // it required. Every consumer that reads `result.actions` without + // a guard (the Game Mode snapshot effect, `buildChosenActionRecord`, + // `buildSessionResult`) then crashes on this intermediate state. + // Seed it so a partial result is always structurally valid. + actions: p?.actions ?? {}, pdf_url: event.pdf_url, pdf_path: event.pdf_path, // The overflow-graph build time is emitted on the