Repository navigation
fix(coderd/x/chatd): stream compaction summaries - #28886
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae69401255
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Require a finish part before accepting a streamed compaction summary and route the summary stream through the stream-silence guard so a silent provider fails fast with a classified retryable error.
bcfe45d to
f6496b1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6496b12f8
ℹ️ 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".
…ary cap Wrap compaction summary stream errors with the same provider stream classification GenerateAssistant uses, so remote stream resets surfaced as bare context.Canceled retry instead of terminally erroring the chat. Double the summary call's output cap so a low configured chat cap does not truncate the summary that replaces pruned history.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30ae3fc134
ℹ️ 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".
|
Looks like this resolves an issue I hit as well with local models. The 600s fixed timeout in |
…t window Sum-enforcing providers reject requests whose input plus max_tokens exceeds the context window, so the doubled summary cap could make compaction itself fail on large windows at default thresholds. Clamp the cap to the remaining window at trigger time; degenerate cases (unknown limit, usage at or over the limit) leave the cap unchanged.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d52f350a1a
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 726961fce4
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…paction-summary # Conflicts: # coderd/x/chatd/chatd_test.go # coderd/x/chatd/chatloop/compaction.go # coderd/x/chatd/chatloop/compaction_internal_test.go
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Chats could get permanently stuck once their context crossed the compaction threshold on adaptive-thinking Claude models: every compaction attempt failed with
compaction produced an empty summary, and both automatic and manual compaction take the same path, so the chat could never recover.Problem
compactionSummaryCalldeliberately setMaxOutputTokens = nilbecause the summary request was non-streaming and the Anthropic SDK rejects non-streaming requests whosemax_tokensimplies a completion longer than ten minutes. But a nil cap makes fantasy's Anthropic provider silently default to 4096 output tokens, and adaptive-thinking Claude models can spend that entire budget on a thinking block, leaving zero summary text.Fix
compaction summary was truncated at the output token capinstead of the generic empty-summary error, so the next occurrence is attributable at a glance.Unit tests cover text-block joining and the truncation classification; the compaction e2e fixtures now serve streamed summaries and assert the summary request streams with
max_tokens: 32000. Dogfood UAT on a deployment built from this branch verified manual compaction with Claude Opus 5 (adaptive thinking, twice, with reload persistence) and an OpenAI regression chat.