fix(agent-core): honor model output cap during compaction#829
fix(agent-core): honor model output cap during compaction#829mikkihugo wants to merge 4 commits into
Conversation
🦋 Changeset detectedLatest commit: eb01127 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR ensures full-history compaction respects the model’s output token limit by wiring maxOutputSize into the completion budgeting logic, with a regression test and a changeset entry.
Changes:
- Pass
maxOutputSizeintoresolveCompletionBudgetduring full compaction. - Add a test that verifies compaction honors a model-level
maxOutputSizecap. - Add a changeset documenting the patch behavior change.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/agent-core/src/agent/compaction/full.ts | Threads maxOutputSize into completion-budget resolution during compaction. |
| packages/agent-core/test/agent/compaction/full.test.ts | Adds coverage to assert compaction uses the model’s output cap. |
| .changeset/honest-timers-flow.md | Documents the patch release note for the behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| it('honors model maxOutputSize during compaction', async () => { | ||
| let callCount = 0; | ||
| const compactionMaxCompletionTokens: unknown[] = []; |
| const providerManager = ctx.agent.modelProvider; | ||
| if (providerManager === undefined) throw new Error('Expected provider manager'); | ||
| const resolveProviderConfig = providerManager.resolveProviderConfig.bind(providerManager); | ||
| providerManager.resolveProviderConfig = (model) => ({ | ||
| ...resolveProviderConfig(model), | ||
| maxOutputSize: 32768, | ||
| }); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b91e242cf
ℹ️ 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".
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "@moonshot-ai/kimi-code": patch | |||
There was a problem hiding this comment.
List agent-core in the changeset
This changeset only selects the CLI even though the diff changes packages/agent-core/src/agent/compaction/full.ts. The current gen-changesets rules require source changes that affect a package's output to list that package, and internal package changes that enter the CLI bundle should list both the internal package and @moonshot-ai/kimi-code; otherwise the agent-core version/changelog will not record this fix when the changeset is consumed.
Useful? React with 👍 / 👎.
Related Issue
No linked issue. This fixes a reproducible full-history compaction failure when a model alias has a larger context window than output-token limit.
Problem
Full compaction reused the selected model provider but resolved its completion budget without the model output cap. Any model alias with
max_context_sizegreater thanmax_output_sizecould make compaction request a token budget above the provider limit and fail before producing a summary.What changed
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.Verification:
pnpm exec vitest run packages/agent-core/test/agent/compaction/full.test.ts packages/agent-core/test/utils/completion-budget.test.ts --config vitest.config.ts