feat: cap completion tokens to remaining context window for chat-completions - #1131
Conversation
🦋 Changeset detectedLatest commit: c3eff4c The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b75c6451e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| reservedContextSize: this.agent.kimiConfig?.loopControl?.reservedContextSize, | ||
| }), | ||
| capability: this.agent.config.modelCapabilities, | ||
| usedContextTokens: this.agent.context.tokenCount, |
There was a problem hiding this comment.
During auto/overflow compaction, messagesToCompact can be much smaller than the live history, but this passes the live context.tokenCount into the Kimi/OpenAI clamp. When compaction is started after the session has reached or exceeded max_context_size, the provider turns that into max_context_tokens - usedContextTokens (then 1), so the summary request is forced to a one-token completion and repeatedly fails as truncated instead of recovering the context. Compute the used tokens from the actual compaction request after messagesToCompact is chosen, or avoid this clamp for compaction.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c96440a — reverted the usedContextTokens passthrough for compaction so the summary request keeps an uncapped budget and can't be forced to a one-token completion. Compaction now matches its previous behavior.
| capability: this.config.modelCapabilities, | ||
| generate: this.generate, | ||
| completionBudgetConfig, | ||
| usedContextTokens: () => this.context.tokenCount, |
There was a problem hiding this comment.
context.tokenCount is only updated from the last sealed step.end; it does not include the just-appended user prompt, injected reminders, or tool results that buildMessages() sends on this request, and it is 0 on the first turn. For Kimi/OpenAI this means the remaining-window clamp can still set max_tokens too high whenever the current prompt/tool output is large, so the input+max_tokens overflow this change is meant to avoid still occurs. Pass a count for the current request instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is intentional — we use the sealed API usage from the last step rather than an estimate, accepting a looser cap on the first turn or after large pending messages in exchange for no estimation error.
c96440a to
496b298
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 496b298d7a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| capability: this.config.modelCapabilities, | ||
| generate: this.generate, | ||
| completionBudgetConfig, | ||
| usedContextTokens: () => this.context.tokenCount, |
There was a problem hiding this comment.
Here usedContextTokens reads ContextMemory.tokenCount, but the current user prompt, hook injections, and tool results are appended before runStepLoop and are only reflected in tokenCountWithPending until step.end records provider usage. On a large new prompt/tool result after the last API call (and on the first turn, where tokenCount is still 0), Kimi/OpenAI providers still size max_completion_tokens to nearly the full window, so input + max_tokens can exceed max_context_size and the overflow this change is meant to prevent still happens.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3eff4cb1c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| capability: this.config.modelCapabilities, | ||
| generate: this.generate, | ||
| completionBudgetConfig, | ||
| usedContextTokens: () => this.context.tokenCount, |
There was a problem hiding this comment.
When a new user prompt, steer message, injection, or tool result has been appended since the last sealed step, context.tokenCount still only reflects the API-reported usage from the previous completed step; those pending messages are nevertheless included in the next params.messages. For near-window inputs this undercounts the actual request size, so withMaxCompletionTokens can still send max_tokens/max_completion_tokens larger than the remaining context window and the chat-completions API rejects the request. Use tokenCountWithPending or an equivalent estimate for the in-flight prompt when sizing this cap.
Useful? React with 👍 / 👎.
51913c8 to
c3eff4c
Compare
…letions (MoonshotAI#1131) * feat: cap completion tokens to remaining context window for chat-completions * test: cover dynamic completion budget for kimi and openai-legacy * fix: leave compaction budget uncapped to avoid one-token summaries * fix: add hookCount to plugins-selector test mocks (cherry picked from commit 76c643b)
1 participant
Related Issue
N/A — no prior issue; the problem is described below.
Problem
Chat-completions providers (Kimi and OpenAI-compatible third parties) reject requests where
input + max_tokensexceeds the model context window, or wheremax_tokensexceeds a fixed ceiling (e.g. third-party endpoints that enforce[1, 131072]). The completion budget previously defaulted to the full context-window size, which triggered these errors once the context grew large.What changed
withMaxCompletionTokensnow accepts an optional{ usedContextTokens, maxContextTokens }, so each provider can tighten the caller-supplied cap to its own transport constraints.max_context_size - usedContextTokens; OpenAI Legacy additionally clamps to 128k.max_tokensis not subject to the input+output window constraint.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.