Skip to content

Treat interrupted AI streams as incomplete - #1844

Merged
laboef1900 merged 6 commits into
devfrom
feature/1801-incomplete-ai-streams
Nov 5, 2026
Merged

laboef1900 merged 6 commits into
devfrom
feature/1801-incomplete-ai-streams

Conversation

@laboef1900

Copy link
Copy Markdown
Contributor

Fixes #1801

Part of #1827.

Change

AI stream consumers require done: true before success or application, retain incomplete/error partial text, and continue reading post-completion metadata. Cached done without a separate final frame remains valid. The editor preview prevents replacement or insertion of incomplete output; the embedding consumer already requires its distinct type: complete event.

Acceptance and verification

  • Managed browser against the changed worktree and isolated loopback stack: Generate EOF retains partial and reports interruption without Save; explicit error retains partial; cached done without final enables Save; finalContent after done replaces the prior text. Enter submits. Desktop Paper and mobile Graphite were exercised with no horizontal overflow.
  • Cancellation used the existing inspector Stop response control, real backend and a loopback provider sending a partial chunk and holding the stream: partial remained visible and the editor remained empty. The full-page Generate surface does not have a Stop control; none is claimed or added.
  • Focused frontend consumer regressions: 275 passed; focused diagram/Generate fixture corrections: 137 passed. Full frontend suite: 5,805 passed; root E2E configuration unit suite: 30 passed. Final lint and typecheck passed. Node v24.22.1.
  • Build, contract/backend tests, and production dependency audit passed in the preceding global gate on base 5edff9e08dad25e9db1c5668ff211c794fc9c2fd plus the then-current patch. They are reused: the subsequent changes were a Mermaid label and a test fixture adding the required terminal frame, not production source. Final global-gate source manifest SHA-256: 328a90c2963838b3109b722f5a29399ec7aac66ef9e2de7e67b9a4821007c132.
  • Earlier full-suite run failed three frontend tests: a Mermaid semicolon and a fixture missing its terminal event; corrected without weakening assertions. Initial browser cached-response fixture contained invalid JSON; the corrected fixture passed. A sanitized focused invocation initially lacked npx in PATH; the direct Node retry passed. Raw evidence is retained outside the worktree under epic-1827-evidence/1801/ (including global-r1, global-r2, and browser captures).
  • Initial fixture backend startup inherited an environment-configured non-loopback provider URL. It was stopped promptly, the disposable fixture provider configuration corrected, and restarted with env -i; zero pages and no provider request were observed during that startup, but absence of all external traffic was not independently established. Later provider traffic used loopback only.

Published revision: fc7547d1e9c6f00bf82b882290ade68fd3daf1eb.

@laboef1900 laboef1900 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Independent Astra-high review of published fc7547d: correct, no actionable findings. EOF without done retains partial/error and bypasses success, promotion and final-application callbacks; explicit errors retain partial, cancellation remains separate, neither consumer stops at the first done marker. Cached completion and later metadata remain supported; embeddings retain their distinct complete event. Inspected actual-worktree browser EOF/error/cached/post-terminal evidence and real backend/loopback cancellation with retained partial and empty editor, desktop/mobile light/dark captures, focused275+corrected137, final frontend5805/rootunit30, lint/type and preceding backend/build/contracts/audit provenance. Initial fixture/diagram failures are retained and corrected; browser-proofed production bytes unchanged. Read-only reviewer did not rerun checks or touch services. No GitHub approval claimed; final-head CI tracked separately.

@laboef1900 laboef1900 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Independent review: PR #1844

Issue: #1801 (bug; no NO AI label). Target: dev.

Head: fc7547d1e9c6f00bf82b882290ade68fd3daf1eb
Base: 5edff9e08dad25e9db1c5668ff211c794fc9c2fd

Verdict

Correct; no confirmed patch-introduced actionable findings. Confidence: 0.96. Review disposition: COMMENT (read-only source/evidence assessment, not newly executed verification or GitHub approval).

Scope and boundary review

Full twelve-file diff and changed regression contexts; SSE decoder, all three SSE consumers, backend cached/stream frame producers, Generate Save and dock callback/application paths. Embedding WorkersTab retains its existing separate type=complete requirement. Retained Paper desktop and Graphite mobile captures inspected; no new browser session.

Acceptance criteria

  • Require route done before success/final application — FULLY ADDRESSED. AiContext.tsx:1447-1553 gates completeExchange/onComplete on done; use-improve-stream.ts:91-128 gates done status; ImprovePanel.tsx:148-164 requires done for Replace/Insert. Generate Save and dock Apply results are populated by onComplete only.

  • Incomplete output distinct and partial text recoverable; explicit errors/cancel preserved — FULLY ADDRESSED. AiContext failLastMessage retains partial with isError; editor retains output plus error and disables application. AbortError keeps prior cancellation semantics without success callbacks. Regressions cover EOF/error/nonpromotion and editor controls.

  • Cached done without final and later metadata still processed — FULLY ADDRESSED. Neither consumer breaks on done. Conversation metadata deferred until successful completion; finalContent still replaces accumulated output. Hook/thread tests and retained cached/post-terminal browser captures cover these cases.

Findings

None.

Evidence inspected (reused, not executed here)

  • pr-1844-metadata.json: exact-head Typecheck & Lint, backend/frontend Tests, Playwright E2E and Semgrep SUCCESS
  • epic-1827-evidence/1801/browser-final-light-eof.json and visual-final-{light-desktop,dark-mobile}.png: retained partial, interruption and no Save
  • epic-1827-evidence/1801/browser-cached-done.txt and browser-post-terminal.txt: valid cached Save and cleaned replacement
  • epic-1827-evidence/1801/browser-real-cancel.json: aborted real request with empty editor
  • epic-1827-evidence/1801/published-source-attestation.json: final source manifest linkage

Evidence limits

Static and retained-evidence review only; no new browser/build/test/formatter/DB service. Context launcher/detector not executed under read-only no-check assignment. Does not claim arbitrary network interruption or upstream protocol coverage beyond terminal frames arriving at these frontend consumers. Initial malformed cached fixture, obsolete missing-done fixture, Mermaid correction and inherited nonloopback startup caveats remain disclosed in PR.

Suggested targeted verification

Parent only: npx vitest run src/features/ai/AiContext.threads.test.tsx src/shared/components/article/use-improve-stream.test.ts src/shared/components/article/EditorBubbleMenu.test.tsx src/features/ai/modes/GenerateMode.test.tsx from frontend; isolated browser EOF/error/cached-done/post-done metadata and Stop-response scenarios if fresh smoke is required.

Cross-PR interaction

No conflicting behavior identified among this assigned group by source inspection.

No source edits, branch changes, services, GitHub reviews, pushes or merges performed.

Parent validation and publication

No additional actionable findings from parent triage.

Current final-head GitHub CI is passing. This is an independent source/evidence review, not a newly rerun full Docker/browser matrix or a GitHub approval. Source-only findings are identified as such. No code modifications or merges performed.

Reviewed by PR Reviewer Agent (eight independent review contexts; parent-validated publication).

@laboef1900 laboef1900 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Automated PR Review

PR: #1844 -- Treat interrupted AI streams as incomplete
Branch: feature/1801-incomplete-ai-streams -> dev (reviewed at fc7547d1)
Diff size: 399 lines (+141/-9)
Reviewed at: 2026-11-01 10:43 UTC


Static Checks

Check Status Details
TypeScript PASS npm run typecheck
Lint PASS npm run lint
Backend Tests PASS* 1 failed
Frontend Tests PASS 5805 passed (5805)
Production Build PASS npm run build
npm audit FAIL pre-existing: 11 advisories (10 moderate, 1 high systeminformation) in the shared lockfile; package-lock.json not changed by this PR
Branch Target PASS feature/* → dev

Docker App Test

Built from source (docker-compose.yml + docker-compose.build.yml, native arch) on an isolated project pr-review-1844; API reached through the nginx proxy (the backend publishes no host port by design).

Check Status
Containers Healthy (postgres, redis, backend, frontend) PASS
Migrations (fresh DB → Migrations complete, through 129) PASS
Backend /api/health + /api/health/ready PASS
Fresh-instance setup (POST /api/setup/admin → 201) + Login PASS
API /api/pages, /api/search?q=…, /api/analytics/trending, /api/settings PASS (200)
API /api/admin/settings, /api/admin/llm-providers, /api/notifications/count PASS (200)
Frontend loads (GET / → 200) PASS
No error/fatal logs (level ≥ 50) PASS

E2E Browser Tests

CI recipe (E2E_CI=1 npx playwright test: built backend + vite preview, fresh kb_e2e database, provisioning via e2e/helpers/ci-setup.ts).

Project Status
e2e helper unit tests (vitest --config e2e/vitest.config.ts) PASS
chromium + collab PASS (68 passed (3.3m))

Code Review Findings

# Severity File:Line Finding
1 WARNING frontend/src/features/ai/AiContext.tsx:1439 failLastMessage now folds the whole partial answer into the error message string ("${text}\n\nPartial output:\n${accumulated}"). Both renderers paint isError messages as plain text in a destructive <p> with no whitespace-pre-wrap (AiAssistantPage.tsx:111, dock/DockPanel.tsx:701-706), so a multi-paragraph Markdown/code partial collapses into one red run with its line breaks and structure gone — the "recoverable partial text" is retained but not usefully readable or copyable. The same string is also what the primed role="alert" announcers speak (AiAssistantPage.tsx:323-326, DockPanel.tsx:262-264), so an interruption after a long generation pushes thousands of characters into an assertive live region. Keep content: accumulated on the message and carry the interruption separately (e.g. an isIncomplete/errorText field) so the partial renders through the normal Markdown path with a distinct notice and only the notice is announced.
2 INFO frontend/src/features/ai/AiContext.tsx:1525-1528, frontend/src/shared/components/article/use-improve-stream.ts:121 The new "loop ended normally while the signal is already aborted" branches (controller.signal.aborted && !completed → commit; if (controller.signal.aborted) return;) have no test. Every abort test (AiContext.threads.test.tsx:868-894, bubble-menu/hook abort tests) throws AbortError from the generator and exercises the catch path only, so the branch that keeps a cancel from being reported as "interrupted before completion" is unverified.
3 INFO CLAUDE.md:14-20 The new SSE-consumer contract paragraph sits in the project intro between "Source-of-truth docs" and "Mandatory Rules", away from the LLM/SSE guidance it belongs with (LLM Provider Model section, or beside the shared SSE audit-lifecycle note near line 1446). Readers scanning the AI sections will not find it there.
4 INFO frontend/src/shared/components/article/ImprovePanel.tsx:225,240 {!hasResult && <div className="flex gap-1"> … </div>} is written inline on the opening/closing lines, unlike every other conditional block in this file ({cond && ( on its own line, )} closing). Cosmetic; worth matching the file's style.

Acceptance Criteria (Issue #1801)

AC Criterion Status Evidence
AC-1 Require the route's terminal event (done: true) before success/final application PASS AiContext.tsx:1449,1455 sets completed only on chunk.done === true; :1529-1533 fails the turn and returns before commitToMessages/completeExchange/onComplete when !completed. use-improve-stream.ts:91,111,121-126 same gate before setStatus('done'). ImprovePanel.tsx:153,162 disable Replace/Insert unless stream.status === 'done'; the only callers of onReplace/onInsertBelow are those buttons (EditorBubbleMenu.tsx:560-561, EditorBlockMenu.tsx:377-378). Backend contract confirmed: provider streamChat always ends with {content:'', done:true} (openai-compatible-client.ts:602,646), sendCachedSSE emits done:true on the content frame (_helpers.ts:248), so every healthy route (incl. diagram/quality with no extras frame) still completes. All runStream consumers apply via onComplete only (GenerateMode.tsx:419, DiagramMode.tsx:185, ImproveMode.tsx:225, use-dock-actions.ts:157,252,278,337); no onContent consumer exists. Tests: AiContext.threads.test.tsx:791-802, use-improve-stream.test.ts:98-105, EditorBubbleMenu.test.tsx:724-734.
AC-2 Surface incomplete output distinctly while retaining recoverable partial text PASS (see Finding 1) AiContext.tsx:1437-1441 keeps accumulated in the isError message; :1530 distinct "interrupted before completion" text. use-improve-stream.ts:122-123 keeps output, sets error/status: 'error'; ImprovePanel.tsx:222-224 shows role="alert" while the preview with the partial stays mounted. Tests assert partial + message: AiContext.threads.test.tsx:798-799, use-improve-stream.test.ts:102-104, EditorBubbleMenu.test.tsx:730-733. Rendering of the partial inside the error bubble is degraded (Finding 1).
AC-3 Preserve legitimate cached responses whose terminal done has no separate final field PASS completed is read off any frame with done: true, including the cached content frame (AiContext.tsx:1455, use-improve-stream.ts:111); matches sendCachedSSE (_helpers.ts:248-253). Tests: AiContext.threads.test.tsx:816-825 ({content, done:true} then final frame), use-improve-stream.test.ts:36,59 ({content, done:true} with no final).
AC-4 Preserve explicit cancel/error behaviour PASS Cancel: AiContext.tsx:1525-1528 (aborted && !completed → commit partial, no error) plus unchanged AbortError catch at :1557-1561; use-improve-stream.ts:121 returns silently on abort before the incomplete check; Stop control still calls abortRef.current?.abort() (DockPanel.tsx:527). Error: in-band {error} still breaks first (AiContext.tsx:1451-1454, use-improve-stream.ts:106-110) so completed is never set by {error, done:true}. Tests: AiContext.threads.test.tsx:868-894 (stopped first answer not promoted), :804-814 (explicit error keeps partial), EditorBubbleMenu.test.tsx:737+. Gap: abort branch where the loop ends normally is untested (Finding 2).
AC-5 Do not discard valid metadata frames that follow a completion marker PASS Loop does not break on done (AiContext.tsx:1455-1511): finalContent, conversationId, sources, refused, historyTruncated, originalMarkdown, layoutTokensLost after done are all still read; conversationId write deferred to the completed path (:1534-1536) so an incomplete stream never adopts an id. use-improve-stream.ts:113-115 still applies finalContent after done. Tests: AiContext.threads.test.tsx:816-825 (navigates to /ai/c/c-cached from metadata after done), use-improve-stream.test.ts:87-96 (finalContent after done).

Story Validation (Issue #1801)

Issue intent: A truncated SSE stream (EOF before the route's terminal done event) must not be presented or applied as a completed AI answer; partial text should stay visible as an interrupted result while cached, cancelled and errored streams keep their existing behaviour and post-done metadata is still honoured.
Story completion: FULLY ADDRESSED
User journey: Before: streamSSE ended silently at EOF and runStream/useImproveStream ran their success path, so a cut-off answer was committed as a normal message, promoted to a conversation, offered for Save/Replace/Insert, and onComplete consumers (Generate save panel, Diagram insert, Improve diff) applied it -> After: EOF without done: true leaves the partial in an error-styled turn with "interrupted before completion", no promotion/onComplete, and the editor preview keeps the partial visible but with Replace/Insert disabled; cached {content, done:true} frames, explicit cancel, in-band errors and post-done final/finalContent frames behave as before. The embedding consumer already required type: 'complete' (WorkersTab.tsx:295-305) and is unchanged.
Gaps: sse.ts (named in the issue) is deliberately untouched — classification lives in the consumers, which is a valid design; Finding 1 (partial text readability/a11y inside the error bubble) is the one quality gap; abort-ends-normally branch lacks a test.
Evidence: frontend/src/features/ai/AiContext.tsx:1437-1441,1449-1455,1520-1536; frontend/src/shared/components/article/use-improve-stream.ts:91,111,121-126; frontend/src/shared/components/article/ImprovePanel.tsx:153,162,222-241; tests AiContext.threads.test.tsx:791-825, use-improve-stream.test.ts:98-105, EditorBubbleMenu.test.tsx:724-734; fixture updates adding the terminal frame in AiAssistantPage.test.tsx, GenerateMode.test.tsx, AiContext.threads.test.tsx:545; docs CHANGELOG.md, CLAUDE.md:14-20, docs/architecture/04-frontend-structure.md:69-91, docs/architecture/09-flow-rag-chat.md:130-134.

Suggested Actions

  1. [SHOULD FIX] frontend/src/features/ai/AiContext.tsx:1439 — keep the partial answer as the message content (rendered through the normal Markdown path) and carry the interruption as a separate field/notice instead of string-concatenating it into the plain-text, role="alert"-announced error body; add renderer support in AiAssistantPage.tsx:108-111 and dock/DockPanel.tsx:701-706.
  2. [CONSIDER] Add tests for the abort-then-normal-EOF branch in AiContext.tsx:1525-1528 and use-improve-stream.ts:121 (generator that yields a chunk, then returns after the signal is aborted without throwing) to lock in that a cancel is never reported as "interrupted".
  3. [OPTIONAL] Move the CLAUDE.md:14-20 paragraph next to the existing LLM/SSE guidance; align the inline {!hasResult && <div …> / </div>} at ImprovePanel.tsx:225,240 with the file's conditional-block style.

Verdict: COMMENT

Reason: All static checks, the Docker smoke run and the Playwright suite pass and issue #1801 is fully addressed (EOF before done: true is now an incomplete stream in both runStream and useImproveStream; cancel, in-band error, cached and post-done metadata frames are preserved). One WARNING: the partial answer is string-concatenated into the plain-text isError message, so a long Markdown/code partial renders as one collapsed red run and is pushed whole into the role="alert" announcer — keep the partial as message content and carry the interruption separately.

Posted as a comment: GitHub rejects APPROVE/REQUEST_CHANGES from the PR author's own account, so the verdict above is advisory.


Reviewed by PR Reviewer Agent | 2026-11-01

…t notice (#1801)

failLastMessage folded the retained partial into the error string, so both
renderers painted a multi-paragraph/code partial as one red plain-text run
and the role="alert" announcers spoke all of it. The partial now stays the
message content and the failure rides as a separate `interruption` field;
renderers show it as a short destructive notice beneath the Markdown body.
@laboef1900

Copy link
Copy Markdown
Contributor Author

Addressed the review WARNING in 50fe7b5d: the partial answer stays as content (regular Markdown path) and the failure rides along as a new transient Message.interruption; AiAssistantPage and DockPanel render a short destructive notice (data-testid="message-error") beneath the partial, and the role="alert" announcers speak only that notice. Tests assert Markdown structure survives and the announcer excludes the partial; 04-frontend-structure.md updated. Full frontend suite: 5809 passed, typecheck/lint/build clean.

@laboef1900
laboef1900 merged commit ebbb2ee into dev Nov 5, 2026
8 checks passed
@laboef1900
laboef1900 deleted the feature/1801-incomplete-ai-streams branch November 5, 2026 18:52
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.

[P2] Treat EOF before the AI terminal event as an incomplete stream

1 participant