feat(assistant): let a failed turn name what the caller can do - #316
Conversation
The debate record decided the persistence boundary: one store, not two. The deciding argument was the defending side's own concession that no meaningful value remains in Spring AI's stock JDBC repository or dialect, which left a permanently project-owned persistence stack holding a projection derivable from the transcript. Measurement retired the drift claim that opened the question. In 546 conversations the model window equals LEAST(transcript, 20) in 514, every remaining difference but one is a failed or model-free turn, and no row exists in memory that is absent from the transcript. The second store is not diverging; it is a pure function of the first, which is why it earns nothing. The winning position won conditionally, so the design records six binding constraints rather than the shape originally proposed: no no-op add(), explicit turn identity, in-flight USER excluded by construction, clear() not delegated to the domain delete, snap-forward counted as a cost rather than a gain, and the drift claim withdrawn. Composer attachment, Knowledge Base format coverage and the unwired multimodal pipeline go to the backlog. Attachment is a permission boundary question, not a UI change, and the multimodal pipeline cannot run while image upload is refused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe change classifies assistant-stream failures into fixed user-facing messages. ChangesAssistant stream failure messaging
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AssistantStream as UiMessageStream
participant Classifier as AssistantStreamFailures
participant Encoder
participant Browser
AssistantStream->>Classifier: describe(error)
Classifier-->>AssistantStream: fixed error text
AssistantStream->>Encoder: error(errorText)
Encoder->>Browser: SSE error frame
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TegamiThis repository uses Tegami to manage releases. When your changes affect published packages, add a changelog file under Create a changelog → · Changelog format Release preview
Changelogs in this PR
Run Managed by Tegami. |
Every failure ended on the fixed frame "The assistant stream failed.", so an expired gateway credential, a rate limit, a retired model and a broken deployment were indistinguishable to the only person able to act on them. f38a535 made a failure attributable to whoever operates the deployment; this makes it actionable to whoever is sitting in front of it. Ported from Northstar's AssistantStreamFailures, whose javadoc names the same before-state. Two departures. Saturation is read from the bounded failureCode on AssistantUnavailableException rather than a status, because the retrieval scheduler rejects the turn before any gateway is contacted and so it never had one. Administrator-facing advice does not name a Settings screen the ordinary actor may not reach. Every returned sentence stays a fixed string with nothing interpolated from the failure, which is why this reads a status rather than a message: a chatty gateway must not be able to echo a key or a prompt fragment into a browser. An unrecognized failure still ends opaque. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
655cbd2 to
45c5cef
Compare
The change alters what a user reads when a turn fails, so it is product-impacting and the release gate requires an entry. Patch rather than minor: no new capability, an existing message becomes specific. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
What
Two commits, one shippable behavior change and one increment record.
ad581117— a failed turn names its cause. Every failure ended on the fixed frame"The assistant stream failed.", so an expired gateway credential, a rate limit, a retired model and a broken deployment were indistinguishable to the only person able to act on them. Ported from Northstar'sAssistantStreamFailures, whose javadoc names the same before-state.Two deliberate departures from the Northstar original:
failureCodeonAssistantUnavailableExceptionrather than an HTTP status, because the retrieval scheduler rejects the turn before any gateway is contacted and so it never had one.Every returned sentence stays a fixed string with nothing interpolated from the failure — that constraint is why this reads a status rather than a message. An unrecognized failure still ends opaque.
9ecb76f0— opens the conversation memory SSOT increment. Design, plan, and the full architecture-challenge record.Why this shape
f38a5357made a failure attributable to whoever operates the deployment (failure_codetag,WARNline). This makes the same failure actionable to whoever is sitting in front of it. They are two halves of one problem.Increment decision
The persistence boundary was decided by an independent two-architect debate with a no-tools judge, not by the proposer. Record is in
challenge-brief.md/challenge-verdict.md.Outcome: collapse to one store. The deciding argument was the defending side's own concession that no meaningful value remains in Spring AI's stock JDBC repository or dialect, leaving a permanently project-owned persistence stack holding a projection derivable from the transcript.
Measurement retired the drift claim that opened the question — in 546 conversations the model window equals
LEAST(transcript, 20)in 514, every remaining difference but one is a failed or model-free turn, and no row exists in memory that is absent from the transcript. The second store is not diverging; it is a pure function of the first, which is why it earns nothing. The design records this correction explicitly rather than quietly dropping it.The winning position won conditionally, so the design carries six binding constraints instead of the originally proposed shape — including that the proposed no-op
add()is withdrawn and that upstream already provides the snap-forward behavior I had listed as a gain.Verification
:apps:api:testand:core:testgreen, re-run after merging 10 commits frommaincompileJavagreenNot in scope
Composer file attachment, Knowledge Base format coverage, and the built-but-unwired LightRAG multimodal pipeline are recorded in the roadmap Engineering Backlog. Attachment is a permission-boundary question needing its own challenge, and the multimodal pipeline cannot run while image upload is refused.
🤖 Generated with Claude Code
Summary by CodeRabbit