Skip to content

fix: abort cancelled answer streams#757

Closed
BigSimmo wants to merge 4 commits into
mainfrom
codex/harden-high-value-api-route-family
Closed

fix: abort cancelled answer streams#757
BigSimmo wants to merge 4 commits into
mainfrom
codex/harden-high-value-api-route-family

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Harden the SSE answer streaming route so client-side cancellation of the response body reliably aborts downstream retrieval and provider generation work.
  • The existing route forwarded only the incoming Request signal, allowing a ReadableStream consumer cancel to leave expensive work running and to produce stale SSE error frames.

Description

  • Combine request aborts and stream-body cancellation into a route-owned AbortController and pass its signal to both summarizeDocument and answerQuestionWithScope so retrieval/generation stop when the client cancels. (edited src/app/api/answer/stream/route.ts)
  • Suppress enqueueing SSE error frames when the combined signal is aborted so a cancelled consumer does not see duplicate or misleading errors; keep existing SSE error payloads for genuine server faults. (edited src/app/api/answer/stream/route.ts)
  • Add a ReadableStream.cancel handler that aborts the same controller, and tidy the request-abort listener lifecycle to avoid leaks. (edited src/app/api/answer/stream/route.ts)
  • Add a focused contract test that starts an answer stream, cancels the response body, and asserts the downstream generation signal is aborted and no duplicate runs continue. (edited tests/private-rag-access.test.ts)
  • Record the targeted review outcome in the branch review ledger. (edited docs/branch-review-ledger.md)

Testing

  • git diff --check / workspace static checks passed before and after the change. (OK)
  • Offline RAG fixture/manifest validation ran via npm run eval:rag:offline and passed the golden fixtures (36 cases, 21 suites); the test harness could not proceed to the local Vitest stage because node_modules/vitest is not present in this worktree. (partial pass)
  • Unit / focused Vitest runs and verify:cheap were not executed because the environment lacks local dev dependencies (vitest, tsx), so lint/typecheck/unit/build gates were selected by a verify:pr-local -- --dry-run but not run for real. (blocked by missing node_modules)
  • The change is present on the local branch codex/harden-answer-streaming and the repository ledger was updated to record the review outcome; the environment had no configured remote so pushing/hosted-PR creation was not performed from this session. (local branch contains the edits)

Files changed:

  • src/app/api/answer/stream/route.ts (stream cancellation wiring, safe error suppression, cancel handler)
  • tests/private-rag-access.test.ts (new cancellation contract test + mock runtime hook)
  • docs/branch-review-ledger.md (recorded review entry)

Notes: provider-backed checks (check:production-readiness, live retrieval/answer evals) were not run because they require explicit approval and live keys; no external services were called or mutated during this work.


Codex Task

Summary by CodeRabbit

  • Bug Fixes
    • Improved answer streaming cancellation so client disconnects promptly stop in-progress retrieval and generation.
    • Avoided emitting SSE error events when cancellations happen intentionally.
    • Strengthened cleanup to reduce side effects during early stream termination.
  • Tests
    • Added coverage ensuring cancelling an answer stream aborts in-progress processing.
  • Documentation
    • Updated the branch review ledger with new completed review entries and associated evaluation notes.

@supabase

supabase Bot commented Jul 17, 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 ↗︎.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@Copilot, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 960cb13e-17da-4ecd-bc75-bac55816b42d

📥 Commits

Reviewing files that changed from the base of the PR and between 1b8bd4d and 02f9c59.

📒 Files selected for processing (2)
  • src/app/api/answer/stream/route.ts
  • tests/private-rag-access.test.ts
📝 Walkthrough

Walkthrough

Answer streaming now propagates both request and reader cancellation through a unified abort signal, suppresses cancellation SSE errors, and cleans up listeners. Tests inject answer behavior to verify abort propagation, while the review ledger records completed reviews.

Changes

Answer stream cancellation

Layer / File(s) Summary
Unified stream abort handling
src/app/api/answer/stream/route.ts
Request aborts and readable-stream cancellation share an abort signal passed to summarization and answer generation; cancellation suppresses SSE errors and removes listeners during cleanup.
Cancellation test coverage and ledger
tests/private-rag-access.test.ts, docs/branch-review-ledger.md
The test runtime supports injectable answer behavior, and a new test verifies reader cancellation aborts generation and invokes the answer function once. The review ledger records completed branch reviews.

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

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ReadableStream
  participant streamAnswer
  participant answerQuestionWithScope
  Client->>ReadableStream: cancel response reader
  ReadableStream->>streamAnswer: cancel(reason)
  streamAnswer->>answerQuestionWithScope: abort unified streamSignal
  answerQuestionWithScope-->>streamAnswer: stop generation
Loading

Possibly related PRs

  • BigSimmo/Database#466: Both changes modify SSE stream cancellation and teardown behavior in streamAnswer.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is mostly complete, but it misses the required Risk and rollout and Clinical Governance Preflight sections from the template. Add the Risk and rollout section and complete the Clinical Governance Preflight checklist, then ensure Summary and Verification match the template headings.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: fixing cancellation handling for answer streams.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/harden-high-value-api-route-family

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

@BigSimmo
BigSimmo marked this pull request as ready for review July 17, 2026 19:37
@BigSimmo
BigSimmo enabled auto-merge July 17, 2026 19:37
@BigSimmo

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request

…ue-api-route-family

# Conflicts:
#	docs/branch-review-ledger.md
auto-merge was automatically disabled July 17, 2026 19:40

Head branch was pushed to by a user without write access

Copilot AI commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved. The only conflict was in docs/branch-review-ledger.md — the branch had added the codex/harden-answer-streaming review entry while main had added PR #635 and PR #718 entries. Both sets of rows are now present in the merged result (merge commit 23b4a73).

@BigSimmo BigSimmo closed this Jul 17, 2026
@BigSimmo
BigSimmo deleted the codex/harden-high-value-api-route-family branch July 18, 2026 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants