Skip to content

Keep what arrived when a stream dies - #5072

Open
elias-ba wants to merge 1 commit into
timeout-stuck-message-reaperfrom
timeout-keep-partial-response
Open

Keep what arrived when a stream dies#5072
elias-ba wants to merge 1 commit into
timeout-stuck-message-reaperfrom
timeout-keep-partial-response

Conversation

@elias-ba

@elias-ba elias-ba commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Description

A reply the user watched appear was thrown away the moment the stream failed. Every event except the final one returned the accumulator unchanged, so the saved message was built entirely from a complete event that never came: the panel typed out an answer and then replaced it with a red box.

The accumulator now carries the text as it arrives, and the yaml that workflow and global chat send ahead of the text. If the stream ends without completing, whatever did arrive is saved as an assistant message marked incomplete rather than discarded. It is deliberately not presented as a finished answer, so the panel can show it as cut off and offer a retry. The content is clamped to the column's limit, because a reply that ran past it would fail the changeset and lose the lot.

Saving it is not enough on its own. The channel has to send it, and send it before the error, or the client clears its streaming buffer and the user only finds the reply again by reloading. That is a new new_message broadcast on the error path.

Apollo's own words stop reaching the panel and the row. Apollo wraps any unhandled exception as str(e), which carries internal hostnames, upstream URLs and container paths. That it failed is worth recording, because it separates a service giving up from a socket going quiet; its words are not, so they go to the log and the user gets a sentence of ours. Three existing tests changed with it, since they asserted on Apollo's text.

Job chat's suggested code is not recoverable this way and is not attempted. It is assembled from the tool call after the model finishes, so when a stream dies mid-answer the code does not exist yet on either side. There is nothing to save rather than something being dropped.

Chunks are prepended and reversed once at the end rather than appended to a growing binary, since a long reply arrives as thousands of them.

Validation steps

  1. Ask the assistant something that produces a long answer, and watch the text stream in. Kill Apollo part-way through.
  2. The text you had already watched appear should stay on screen, marked as cut off, with a retry — rather than being replaced by a red error. Reload the page and it should still be there.
  3. Check the server log has Apollo's own error text, and that the panel never showed it.

Additional notes for the reviewer

This is where Apollo's own error text stops reaching the panel. Three existing tests changed with it, since they asserted on Apollo's words.

AI Usage

Please disclose whether you've used AI anywhere in this PR (it's cool, we just want to know!):

  • I have used Claude Code
  • I have used another model
  • I have not used AI

You can read more details in our Responsible AI Policy

Pre-submission checklist

  • I have performed an AI review of my code (we recommend using /review with Claude Code)
  • I have implemented and tested all related authorization policies. (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog
  • I have ticked a box in "AI usage" in this PR

@github-project-automation github-project-automation Bot moved this to New Issues in Core Aug 15, 2026
@github-actions

Copy link
Copy Markdown

I have enough context to render the review. The PR is entirely AI-assistant reliability plumbing: partial-response preservation on stream death, a stuck-message reaper, failure-category storage/broadcast, shutdown_grace_period sizing, and detached telemetry handler IDs.

Security Review ✅

  • S0 (project scoping): No new user-facing entrypoints; the reaper is a background Oban worker reading ai_chat_messages and oban_jobs internally, and the new failure_category/failure_message fields flow through the existing ai_assistant_channel.ex:39 authorize_session_access gate before broadcast (ai_assistant_channel.ex:296, message_processor.ex:316).
  • S1 (authorization): N/A — no new handle_in/controller actions or policy surface; the reaper and telemetry :stop handler are non-user-invoked, and channel changes only enrich an existing authorized broadcast payload.
  • S2 (audit trail): N/A — chat messages are conversational state, not project/instance configuration, so failure_category/failure_message writes (ai_assistant.ex:1113, message_processor.ex:194, stuck_message_reaper.ex:75) fall outside the S2 audit-resource list.

@elias-ba
elias-ba force-pushed the timeout-stuck-message-reaper branch from 01753c3 to 55042f3 Compare August 15, 2026 22:50
@elias-ba
elias-ba force-pushed the timeout-keep-partial-response branch from 043e22e to 1bb4437 Compare August 15, 2026 22:50
@elias-ba
elias-ba force-pushed the timeout-stuck-message-reaper branch from 55042f3 to 3abf115 Compare August 15, 2026 23:40
@elias-ba
elias-ba force-pushed the timeout-keep-partial-response branch from 1bb4437 to 46eb44f Compare August 15, 2026 23:40
@elias-ba
elias-ba force-pushed the timeout-stuck-message-reaper branch from 3abf115 to d0b1213 Compare August 17, 2026 02:53
@elias-ba
elias-ba force-pushed the timeout-keep-partial-response branch from 46eb44f to 34430bf Compare August 17, 2026 02:53
@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.30769% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.6%. Comparing base (43d8b43) to head (b990493).

Files with missing lines Patch % Lines
lib/lightning/ai_assistant/ai_assistant.ex 93.5% 2 Missing ⚠️
lib/lightning_web/channels/ai_assistant_channel.ex 85.7% 1 Missing ⚠️
Additional details and impacted files
@@                     Coverage Diff                      @@
##           timeout-stuck-message-reaper   #5072   +/-   ##
============================================================
  Coverage                          90.5%   90.6%           
============================================================
  Files                               422     422           
  Lines                             20044   20066   +22     
============================================================
+ Hits                              18148   18175   +27     
+ Misses                             1896    1891    -5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@elias-ba
elias-ba force-pushed the timeout-stuck-message-reaper branch from d0b1213 to 8d50a20 Compare August 17, 2026 04:10
@elias-ba
elias-ba force-pushed the timeout-keep-partial-response branch from 34430bf to 5bdb4ad Compare August 17, 2026 04:11
@elias-ba
elias-ba force-pushed the timeout-stuck-message-reaper branch from 8d50a20 to abe0cd6 Compare August 17, 2026 09:24
@elias-ba
elias-ba force-pushed the timeout-keep-partial-response branch 2 times, most recently from 9db7ecf to 6874963 Compare August 17, 2026 16:47
A reply the user watched appear was thrown away the moment the stream
failed. Every event except the final one returned the accumulator
unchanged, so the saved message was built entirely from a complete event
that never came - the panel typed out an answer and then replaced it with
a red box.

The accumulator now carries the text as it arrives, and the yaml that
workflow and global chat send ahead of the text. If the stream ends
without completing, whatever did arrive is saved as an assistant message
marked incomplete rather than discarded.

It is deliberately not presented as a finished answer: the message
carries the incomplete_response category, so the panel can show it as
cut off and offer a retry.

Saving it is not enough on its own - the channel has to send it, before
the error, or the client clears its buffer and the user only finds the
reply again by reloading.

Apollo's own words stay out of the panel and the row. Apollo wraps any
unhandled exception as str(e), which carries internal hostnames, upstream
URLs and container paths. That it failed is worth recording, because it
separates a service giving up from a socket going quiet; its words are
not, so they go to the log.

Job chat's suggested code is not recoverable this way and is not
attempted. It is assembled from the tool call after the model finishes,
so when a stream dies mid-answer the code does not exist yet on either
side - there is nothing to save, rather than something being dropped.

Chunks are prepended and reversed once at the end rather than appended to
a growing binary, since a long reply arrives as thousands of them.
@elias-ba
elias-ba force-pushed the timeout-stuck-message-reaper branch from abe0cd6 to 43d8b43 Compare August 18, 2026 07:29
@elias-ba
elias-ba force-pushed the timeout-keep-partial-response branch from 6874963 to b990493 Compare August 18, 2026 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New Issues

Development

Successfully merging this pull request may close these issues.

1 participant