Skip to content

fix: stop aborting live answer streams at a flat 60s timeout#466

Merged
BigSimmo merged 10 commits into
mainfrom
claude/search-timeout-failure-s6aiuj
Jul 11, 2026
Merged

fix: stop aborting live answer streams at a flat 60s timeout#466
BigSimmo merged 10 commits into
mainfrom
claude/search-timeout-failure-s6aiuj

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

  • Fixes the live failure where Answer mode streams part of a response, shows "Fast answer was unsupported, retrying with the strong model," and then dies with "Answer generation timed out. Please try again."
  • Root cause: the dashboard aborted every answer request at a flat 60s wall clock (answerRequestTimeoutMs), even while the stream was alive and delivering tokens. The server pipeline legitimately exceeds 60s whenever the fast answer fails a quality gate and escalates: fast generation (≤30s OPENAI_ANSWER_TIMEOUT_MS) + strong retry (≤30s) + optional strong quality repair (≤30s) + retrieval/finalization.
  • Client: replaced the flat timer with a stall watchdog (createAnswerRequestWatchdog in search-utils.ts) — the 60s inactivity window resets on every received stream chunk (progress events, token deltas, heartbeats), so a slow-but-live fast→strong escalation is never aborted mid-stream; a 180s absolute ceiling still bounds runaway requests. Document searches never touch the watchdog and keep the previous flat 60s behavior.
  • Server: /api/answer/stream now emits SSE comment heartbeats (: heartbeat) every 15s via a new src/lib/sse-heartbeat.ts, so silent generation windows (strong-route reasoning before the first output token) stay visibly alive to proxies and to the client's stall detector. Also guards controller.close() against an already-cancelled stream.
  • Tests: watchdog behavior (stall fire, activity reset past the old 60s, absolute ceiling, cancel semantics, custom budgets) and heartbeat frame/stop-on-close behavior.

Verification

  • npm run verify:pr-local

During development, use npm run verify:cheap as the faster iteration gate before the final PR-local preflight.

  • npm run verify:cheap — check:runtime, check:github-actions, sitemap:check, lint, typecheck, vitest all green (155 files, 1445 tests passed)
  • Focused targets: tests/clinical-dashboard-search-utils.test.ts, tests/sse-heartbeat.test.ts, tests/private-access-routes.test.ts, tests/private-rag-access.test.ts, tests/api-route-coverage.test.ts, tests/architecture-boundaries.test.ts
  • npm run verify:ui when UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed
  • npm run verify:release before release or handoff confidence claims
  • npm run eval:retrieval:quality — not run: no retrieval, ranking, selection, chunking, or scoring behavior changed (transport/timeout handling only)
  • npm run eval:rag -- --limit 15 + npm run eval:quality -- --rag-only — not run: the synthesis prompt, generation routing, and answer post-processing are unchanged; only stream liveness and client abort behavior changed, and these evals need live keys unavailable in this environment
  • npm run check:production-readiness — only failures are missing Supabase/OpenAI secrets, expected in the sandbox per repo Cloud Agent notes
  • npm run check:deployment-readiness — deployment startup/hosting behavior unchanged

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use — answer content, gating, and citation behavior are untouched
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy) — no env or project changes
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked — no clinical decision-support behavior changed; the same quality-gated pipeline now simply gets to finish instead of being cut off

Notes

  • The user-visible timeout message is unchanged but should now only appear when a stream genuinely goes silent for 60s (4 missed heartbeats) or exceeds the 180s ceiling.
  • Heartbeats are SSE comment lines, which the existing client parser (and any standards-compliant SSE consumer of the public stream route) already ignores — no contract change.

🤖 Generated with Claude Code

https://claude.ai/code/session_017TAXfecBdBBCdrnYcPj5cd


Generated by Claude Code

Live answer generation failed with 'Answer generation timed out. Please
try again.' whenever the fast answer failed a quality gate and escalated
to the strong model: the pipeline (fast <=30s + strong <=30s + optional
strong quality repair <=30s, plus retrieval) legitimately exceeds 60s,
but the dashboard aborted every answer request at a flat 60s wall clock
even while tokens were still streaming.

- Replace the flat timer with a stall watchdog: the 60s window now
  resets on every received stream chunk, so a slow-but-live generation
  is never aborted mid-stream; a 180s absolute ceiling still bounds
  runaway requests. Document searches keep the old flat 60s behavior.
- Emit SSE comment heartbeats every 15s from /api/answer/stream so
  silent generation windows (strong-route reasoning before the first
  output token) stay visibly alive to proxies and the stall detector.
- Guard controller.close() against an already-cancelled stream.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017TAXfecBdBBCdrnYcPj5cd
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The answer stream now emits SSE heartbeat frames, guards stream closure after cancellation, and replaces the dashboard’s fixed timeout with an activity-based watchdog supporting stall and maximum-duration limits. Differential search also tracks the active evidence query and updates smoke-test expectations.

Changes

Answer stream liveness

Layer / File(s) Summary
SSE heartbeat lifecycle
src/lib/sse-heartbeat.ts, src/app/api/answer/stream/route.ts, tests/sse-heartbeat.test.ts, docs/branch-review-ledger.md
Heartbeat frames are emitted periodically, stopped during teardown, and terminated when enqueueing fails; stream closure is guarded against cancellation errors. The review ledger records the associated review result and checks.
Answer request watchdog
src/components/clinical-dashboard/search-utils.ts, tests/clinical-dashboard-search-utils.test.ts
Adds configurable inactivity and maximum-duration timers with touch, cancellation, single-fire behavior, and fake-timer coverage.
Dashboard stream activity integration
src/components/ClinicalDashboard.tsx
Stream activity resets the per-request watchdog, which aborts stalled requests and is cancelled when execution completes.

Differential evidence state

Layer / File(s) Summary
Evidence query propagation
src/components/differentials/differentials-home-page.tsx, tests/ui-smoke.spec.ts
Differential searches clear and then store the normalized evidence query, pass it to the home component, and update smoke-test assertions for current routing, source status, and document results.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ClinicalDashboard
  participant AnswerStreamRoute
  participant StreamController
  participant AnswerRequestWatchdog
  ClinicalDashboard->>AnswerRequestWatchdog: create watchdog
  ClinicalDashboard->>AnswerStreamRoute: request answer stream
  AnswerStreamRoute->>StreamController: enqueue tokens, progress, and heartbeats
  StreamController-->>ClinicalDashboard: streamed bytes
  ClinicalDashboard->>AnswerRequestWatchdog: touch on activity
  AnswerRequestWatchdog->>ClinicalDashboard: timeout and abort on stall or max duration
  ClinicalDashboard->>AnswerStreamRoute: cancel watchdog in finally
  AnswerStreamRoute->>StreamController: close with cancellation guard
Loading

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Verification Claims ❌ Error docs/branch-review-ledger.md row for PR #466 lists checks like “Focused SSE and search utility Vitest (13/13)” without the exact command/result format required. Rewrite each verification item as exact command/result pairs, e.g. “Ran npm run verify:cheap: passed” or “Not run: reason”, and spell out any rerun status similarly.
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 70.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (9 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: replacing the flat 60s timeout for live answer streams.
Description check ✅ Passed The description matches the required template with Summary, Verification, Clinical Governance Preflight, and Notes sections filled in.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Generated And Sensitive Files ✅ Passed Changed files are only TS/TSX, tests, and a markdown ledger; scans found no real secrets, keys, env files, or build/artifact outputs.
Risky Git Or Deployment Actions ✅ Passed PASS: The PR’s touched files add no force-push, reset/clean, branch-deletion, shared-branch-rebase, prod-mutation, or deploy-without-confirmation guidance.
Supabase Project And Schema Safety ✅ Passed PR diff changes only app/docs/tests; no Supabase env, project-ref, schema, migration, RLS, or policy files were touched, and diff grep found no project/schema updates.
Runtime And Package Manager Integrity ✅ Passed PR leaves package-manager/runtime config untouched: no lockfile or install-script changes, and repo still enforces npm@11/Node24 with engine-strict.
Api Route Failure Handling ✅ Passed PASS: The SSE route and client watchdog handle cancellation, timeouts, provider/env errors, empty results, and malformed streams with deterministic fallbacks/errors.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/search-timeout-failure-s6aiuj
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/search-timeout-failure-s6aiuj

Comment @coderabbitai help to get the list of available commands.

@supabase

supabase Bot commented Jul 10, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

Copy link
Copy Markdown
Owner Author

Advisory UI regression check: failures are pre-existing on main, not introduced by this PR.

All three failing Playwright tests reproduce identically on unmodified main (829eebc, run locally with the same Chromium project):

  • ui-tools.spec.ts:236 — hero global-search-input not found inside tools-home at mobile width
  • ui-tools.spec.ts:411 — hero global-search-input not found inside answer-empty-state at mobile width
  • ui-smoke.spec.ts:2023 — documents-mode search input renders at the bottom composer (y≈764) instead of above the "start here" panel

They all point at the same thing: on phone-width mode-home pages the shared search renders as the bottom composer dock rather than in the hero, while these tests still assert hero placement — likely fallout from the #456/#464 mobile composer work rather than anything in this diff (which only touches stream timeout/heartbeat logic; 116 other UI tests pass). The job is continue-on-error: true, so it doesn't block this PR. Worth deciding separately whether the bottom-dock composer is the intended mobile UX (→ update the three tests) or a regression (→ fix the hero placement on main).


Generated by Claude Code

Resolves conflicts with main's stream-boundary payload validation
(isAnswerPayload) and executeSearch scope/query-mode threading; the
stall-watchdog wiring and SSE heartbeat are unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017TAXfecBdBBCdrnYcPj5cd

Copy link
Copy Markdown
Owner Author

Advisory UI regression on the merged head (54d5229): still pre-existing main failures, not from this PR.

After merging main (b7d877b), the three earlier composer failures are gone — fixed by #470 as hoped. The advisory job now fails on two different tests, and both reproduce identically on unmodified main (verified locally, same Chromium project):

  • ui-smoke.spec.ts:1891 — "newer routed differential context wins over an older response": navigating to /differentials?q=…&run=1 fires 2 search requests where the test expects 1 (duplicate auto-run, likely from the new auto-run signature/replaceExistingAnswer threading in the recent search wiring).
  • ui-tools.spec.ts:489 — "all mode home heroes share identical sizing on mobile": hero heading renders at 26px vs the asserted 25.6px baseline (style/test drift from the design-review commit).

This PR's diff (stream timeout/heartbeat only) doesn't touch either area; 122 other UI tests pass. Both belong to main's recent search-wiring and design-review commits and are worth a small follow-up fix there.


Generated by Claude Code

@BigSimmo
BigSimmo marked this pull request as ready for review July 11, 2026 07:48

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/app/api/answer/stream/route.ts`:
- Around line 147-149: Update the send function in the SSE stream route to wrap
controller.enqueue in a defensive try/catch, silently ignoring enqueue failures
after the stream is closed or canceled. Preserve the existing encodeSse behavior
for active streams and ensure error handling cannot fail while attempting to
emit an error event.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 04eea357-7f87-4835-be69-62f5c9e462ba

📥 Commits

Reviewing files that changed from the base of the PR and between b7d877b and 54d5229.

📒 Files selected for processing (6)
  • src/app/api/answer/stream/route.ts
  • src/components/ClinicalDashboard.tsx
  • src/components/clinical-dashboard/search-utils.ts
  • src/lib/sse-heartbeat.ts
  • tests/clinical-dashboard-search-utils.test.ts
  • tests/sse-heartbeat.test.ts

Comment thread src/app/api/answer/stream/route.ts
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 11, 2026
# Conflicts:
#	docs/branch-review-ledger.md

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e33a148dc1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tests/ui-smoke.spec.ts Outdated
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 11, 2026
# Conflicts:
#	docs/branch-review-ledger.md

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f29b9488a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/components/differentials/differentials-home-page.tsx
@BigSimmo
BigSimmo enabled auto-merge (squash) July 11, 2026 09:19
@BigSimmo
BigSimmo merged commit 3d784f4 into main Jul 11, 2026
15 checks passed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/differentials/differentials-home-page.tsx`:
- Line 56: Update the evidenceQuery state flow around setEvidenceQuery so it
uses the repository’s shared query normalizer, not only trimming via normalized.
Apply that same normalization to the active query used by the downstream
evidence gate before string comparison, ensuring casing and repeated whitespace
are handled consistently.

In `@tests/ui-smoke.spec.ts`:
- Around line 1886-1892: Strengthen the request-count assertions around the
history.pushState navigation in the smoke test: retain the initial request
baseline, then assert the navigation produces the expected single request delta
rather than merely any increase. Add an upper-bound or exact-count check so
duplicate requests fail while preserving the existing wait for at least one
initial request.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 55bbffb5-a743-46d8-8ec5-4fdb7e149afb

📥 Commits

Reviewing files that changed from the base of the PR and between 072adf9 and b341fed.

📒 Files selected for processing (4)
  • docs/branch-review-ledger.md
  • src/components/ClinicalDashboard.tsx
  • src/components/differentials/differentials-home-page.tsx
  • tests/ui-smoke.spec.ts
✅ Files skipped from review due to trivial changes (1)
  • docs/branch-review-ledger.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/ClinicalDashboard.tsx

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/differentials/differentials-home-page.tsx`:
- Line 56: Update the evidenceQuery state flow around setEvidenceQuery so it
uses the repository’s shared query normalizer, not only trimming via normalized.
Apply that same normalization to the active query used by the downstream
evidence gate before string comparison, ensuring casing and repeated whitespace
are handled consistently.

In `@tests/ui-smoke.spec.ts`:
- Around line 1886-1892: Strengthen the request-count assertions around the
history.pushState navigation in the smoke test: retain the initial request
baseline, then assert the navigation produces the expected single request delta
rather than merely any increase. Add an upper-bound or exact-count check so
duplicate requests fail while preserving the existing wait for at least one
initial request.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 55bbffb5-a743-46d8-8ec5-4fdb7e149afb

📥 Commits

Reviewing files that changed from the base of the PR and between 072adf9 and b341fed.

📒 Files selected for processing (4)
  • docs/branch-review-ledger.md
  • src/components/ClinicalDashboard.tsx
  • src/components/differentials/differentials-home-page.tsx
  • tests/ui-smoke.spec.ts
✅ Files skipped from review due to trivial changes (1)
  • docs/branch-review-ledger.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/ClinicalDashboard.tsx
🛑 Comments failed to post (2)
src/components/differentials/differentials-home-page.tsx (1)

56-56: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the same normalization contract for evidenceQuery and the active query.

evidenceQuery receives normalized, but that value is only trimmed at Line 35. The repository normalizer also lowercases and collapses repeated whitespace, while the downstream evidence gate uses string equality. Queries such as "acute confusion" can therefore return matches but remain marked as pending. Normalize both sides with the shared helper before storing/comparing this state.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/differentials/differentials-home-page.tsx` at line 56, Update
the evidenceQuery state flow around setEvidenceQuery so it uses the repository’s
shared query normalizer, not only trimming via normalized. Apply that same
normalization to the active query used by the downstream evidence gate before
string comparison, ensuring casing and repeated whitespace are handled
consistently.
tests/ui-smoke.spec.ts (1)

1886-1892: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve the duplicate-request regression guard.

toBeGreaterThanOrEqual(1) and toBeGreaterThan(baselineRequestCount) only prove that requests occurred; they also pass when navigation triggers duplicate requests. Assert the expected exact count/delta, or add an upper-bound assertion.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/ui-smoke.spec.ts` around lines 1886 - 1892, Strengthen the
request-count assertions around the history.pushState navigation in the smoke
test: retain the initial request baseline, then assert the navigation produces
the expected single request delta rather than merely any increase. Add an
upper-bound or exact-count check so duplicate requests fail while preserving the
existing wait for at least one initial request.

@BigSimmo
BigSimmo deleted the claude/search-timeout-failure-s6aiuj branch July 13, 2026 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants