fix(mock): stop scoring timing out on longer sessions, and let a failed report be retried

[alpha5611331]

Closes #133

Mock interview scoring fails on longer sessions with "The request timed out", and the failure is terminal: the candidate has already been billed for the report and has no way left to obtain one.

The timeout

MOCK_REPORT_TIMEOUT_MS was a flat 60_000 passed to AbortSignal.timeout, which is a total wall-clock deadline rather than a time-to-first-byte one. The reply is a single non-streaming JSON body, so nothing arrives until the whole report is generated.

The report is the one call whose duration scales with the session. MockReport carries a score, a justification and a full stronger_answer rewrite for every question asked - follow-ups included - over a body that also carries the profile and context. A twelve-turn session asks for several times the generation a three-turn one does, so one number is right for neither: sized for the short one it truncates the long one. That is the "sometimes" - it tracks interview length rather than anything intermittent. The abort is client-side only, so the backend goes on to finish the report and charge for it.

mockReportTimeoutMs() derives it from data.questions.length - the turns actually being sent, rather than the session's configured length, so a session ended early is not held to a deadline for questions it never asked and one that ran long on follow-ups gets the time they cost. Bounded at both ends: a floor so a short session still gets a real deadline, and a ceiling so a genuinely hung request cannot hold the Scoring spinner indefinitely just because the session was long. Wall clock rather than a stall timer, unlike the streaming paths - there is one body and no progress to detect - and End stays live throughout Scoring as the manual way out.

The recovery

generateNextQuestion retries once; the report did not, and it is the one charged call whose failure is terminal.

retryScoring() re-runs it, and the report screen offers Score again inside the alert that reports the failure. Deliberately user-driven rather than automatic: a client-side timeout says nothing about whether the backend finished and billed the attempt that timed out, so a silent second attempt can spend twice on the candidate's behalf - and at this call's deadline it would also double the wait before they are told anything. The answers are already on screen; the spend is theirs to make. It is the same reasoning the 402 path already applies to question generation.

It stays on Finished rather than returning to Scoring. Scoring is an active session, so it would re-arm the navigation lock and swap the report screen the candidate is looking at for the session screen, which has no question left to show. A rescoring flag on the session carries the in-flight state instead, mirrored in both type files.

Two things that fall out of the split

• A missing setup in requestReport is now a failure rather than an early return. By that point the state is already Scoring, which has no control that ends it but End, so returning stranded the session on a spinner instead of holding the terminal-state invariant mock-interview-state.test.mjs pins.
• Scoring failures are logged, the way the question path logs its own. This is the end of a session the candidate paid for, and the reason it produced nothing was the one thing neither the screen nor the log recorded.

Testing

test/mock-report-retry.test.mjs pins the deadline scaling and staying bounded, and the retry: the in-flight shape staying on Finished, a success clearing the error, a failure landing back where the first one did, a retry with nothing to recover not reaching the backend at all, and a retry abandoned by clear() writing nothing onto the session that replaced it.

Run locally, all green: pnpm lint, both tsc configs, pnpm build, pnpm test:main (20 new checks, no regressions).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Kt5ucR5XPzzzxqPYzSpXaL

[gitar-bot]

Gitar is working

_Gitar

[alpha5611331]

Review pass

Walked the diff for errors and side effects. One real defect found and fixed in 58cfdc0; the rest below is what was checked and cleared.

Fixed: a retried report left exported stale

markExported() sets exported: true, and hasUnsavedMockContent is hasContent && !exported. The report screen keeps Export beside the failure alert, so saving the answers and then pressing Score again is an ordinary sequence - and the successful retry spread ...this.session, carrying exported: true forward onto a session that now holds a score the file does not contain. Done and Practise again would then have waved the candidate past it without asking.

This is the rule appendAnswer already documents for a new answer, and it was unreachable before this PR: a report only ever arrived before there was anything to export it from. The success branch retires the flag now, and the test exports before retrying so the assertion means something.

Checked and clear

• withMockContent's fast path. It keys on answers identity and exported. Setting rescoring changes neither, so the flip takes the fast path and carries the derived flags forward - correct, and it does not add a rescan to any hot path.
• The navigation lock. isMockInterviewSessionActive is false at Finished, and the retry stays there, so the lock does not re-arm and the report screen is not swapped for the session screen mid-retry. Pinned by and stays on Finished rather than re-entering Scoring.
• clear() racing a retry. Practise again and Done both clear(), which bumps sessionSeq; requestReport's seq check then drops the result. Pinned by a retry abandoned by clear() writes nothing back.
• endSession() during a retry. Returns early on !isActive(), and Finished is not active - no interaction, and End is not reachable from the report screen anyway.
• The unmount fallback in the route tests state !== Idle && !== Finished, so a retry in flight does not trigger endSession.
• reportError's wording is unchanged for the cases that occur. generateReport goes through post, which returns {status: 0, error} for a timeout rather than throwing, so the thrown value is a plain Error and describeApiError returns error.message - the same string as before. The ApiRequestError branch only adds a status prefix, and nothing on this path produces one.
• A 402 on the retry surfaces the backend's own message, which already names the price and the balance, straight into the alert.
• AlertDescription is a <div>, not a <p>, so the nested button is valid; it is also a grid, so the original space-y-3 was fighting its gap-1. Replaced with a gap-3 override.
• No other ReportScreen caller and no other construction of MockInterviewSessionState outside initialSession(), so the new field cannot be missed anywhere.

Note

Prettier reformatted three spots in report.tsx unrelated to this change (the Accordion import, one <p>, one <Button>). report.tsx is a file this PR touches, which is the rule the repo sets, so they are kept rather than reverted.

Verification

pnpm lint, both tsc configs, pnpm build and pnpm test:main green locally; CI green on 58cfdc0. 22 checks in the new file, no regressions in the existing suite.

[alpha5611331]

Second review pass

Re-walked the diff looking specifically for errors and side effects. One defect found and fixed in 5b1f9ff; everything else below was traced and cleared.

Fixed: a successful retry dropped keyboard focus

The retry unmounts the failure alert, and with it the Score again button that was just pressed. Focus falls to <body>, losing the candidate's place in a screen that has at that moment filled up with the score they were waiting for.

This is the same replacement the existing mount effect exists for, one step further in - the comment there already notes that this screen is never reached through a real navigation, so nothing else moves focus on its own. The effect is now keyed on the report arriving. It can only fire twice at most: a retry requires reportError, which is only ever set while report is null, so hasReport flips false → true once per session and never back. Nothing steals focus while the candidate is reading.

Traced and clear

• rescoring cannot get stuck true. Both write paths in requestReport clear it, and both are behind a seq check - so the question is what bumps sessionSeq while a retry is in flight. clear() replaces the session with initialSession(); start() replaces it with {...initialSession(), setup, state: Starting}; endSession() returns early on !isActive(), and Finished is not active. Every route either clears the flag or is unreachable. The reachable one is real - leaving the report screen via the titlebar without pressing Done, then starting a new mock while the old retry is still running - and it lands correctly.
• Double-click on Score again cannot bill twice. rescoring gates the button, but it is only true after an IPC round trip, so two clicks can both dispatch. Main is the authority: retryScoring sets the flag synchronously, before its first await, and each ipcMain.handle invocation is a separate task - so the second call sees rescoring === true and returns without a request.
• Leaving during a retry is deliberately not blocked. Practise again and Done stay enabled, and clear() orphans the request. Disabling them would trap the candidate behind an up-to-ceiling wait with no cancel, which is the failure mode End-during-Scoring is already written to avoid.
• endSession() during Scoring now surfaces Score again, under its "ended before scoring finished" message. That does not contradict the comment there: what it guards against is falling through and silently billing a second report. An explicit button afterwards is the same user-driven choice this PR argues for everywhere else.
• retryScoring's hasRealAnswers() guard is unreachable, so it cannot produce a dead button: finishToScoring resets to Idle rather than Finished when it is false. Kept as the mirror of the guard before the first attempt.
• describeApiError does not change any message that actually occurs. generateReport goes through post, which returns {status: 0, error} for a timeout rather than throwing, so the thrown value is a plain Error and the function returns error.message unchanged. Its ApiRequestError branch is not reachable from this path.
• this.language and the config read are the session's own, frozen at start() - a retry cannot produce the half-translated report getLanguage() documents.
• Build artifacts carry both new symbols (preload.cjs, ipc/mock-interview.js), so the channel is live in a packaged build, not only in source.
• Deadline sizing re-checked against the work: ~220 output tokens per question for score + justification + stronger_answer, so 20s per question is roughly 3x headroom, and the 45s base covers connect, the up-to-256k-char profile and context upload, and the overall/strengths/gaps section. Ceiling reached at twelve turns.

Constant placement

MOCK_REPORT_* stays in api/mock-interview.ts beside the four sibling request timeouts rather than moving to consts.ts. The immediate neighbours are the better precedent here; moving all five would be a separate change.

Verification

pnpm lint, both tsc configs, pnpm build, pnpm test:main green locally. CI green on 5b1f9ff. 22 checks in the new file, no regressions.