Skip to content

feat(core): fit each run into the model's context window - #478

Merged
Sun-sunshine06 merged 3 commits into
OpenCoworkAI:mainfrom
wudilyy999:feat/context-token-budget
Oct 13, 2026
Merged

Sun-sunshine06 merged 3 commits into
OpenCoworkAI:mainfrom
wudilyy999:feat/context-token-budget

Conversation

@wudilyy999

@wudilyy999 wudilyy999 commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Second step of #475. A typical run already sends about 7.5k tokens before any chat history: the system prompt is about 13k characters and the 13 tool definitions about 16k. Local runtimes often default to a 4k–8k window (Ollama's default num_ctx), and an over-long request is truncated by the server rather than rejected, so the model works from a cut-off system prompt.

Core now measures the system prompt, tool definitions, and the prompt (which carries the brief, memory, and attachments as text) in tokens the way pi estimates them, and keeps a quarter of the window, at most the 32,000 tokens a run requests, free for the reply. The request asks for no more output than that reserve, so input plus requested output fits the window: models with a window of 128k tokens or more keep 32,000, a 32k window requests 8,192.

  • If those parts do not fit the window resolved in feat(core): use each model's real context window #476, the run fails before the request with GENERATION_CONTEXT_EXCEEDED. The message names the tokens needed and the window, and suggests shortening the request or its attachments, setting the provider's context window, or using a model with a larger window; the diagnostic offers Settings → Models, where the context window is set.
  • Chat history keeps its newest messages that fit what remains, starting at a user message. The context pack's own history cap is unchanged.
  • The budget (window, reserve, part sizes, history kept and dropped) is logged with each run, for the visibility step that follows.

Type of change

  • Bug fix
  • New feature
  • Refactor (no behavior change)
  • Documentation
  • Build / CI / tooling
  • Breaking change

Linked issue

Refs #475, #394

Checklist

  • I checked the linked issue / relevant context before starting
  • pnpm lint && pnpm typecheck && pnpm test passes locally
  • Added/updated tests for the change
  • Added a changeset (pnpm changeset) if user-visible
  • Updated docs if behavior changed

Principles

  • Compatibility: with today's 200k default and the catalog windows, normal runs fit and keep their history; only runs that cannot fit now stop early.
  • Upgradeability: estimates use pi's estimateTokens.
  • No bloat: no new dependencies; 118 lines outside tests, strings, and the changeset.
  • Elegance: one budget function decides what fits; the agent starts with the history it returns.

Checks

  • pnpm -r typecheck, pnpm lint, CI=1 pnpm test: 10 tasks passed; core 568 tests, shared 327, desktop 180 files / 2,629 tests.
  • context-budget.test.ts: reserve in large and small windows, tool definitions counted by name, description and parameters (a tool alone can push a run past the window), oldest history dropped first, a leading assistant message dropped, prompt images counted, and the error when the fixed parts do not fit.
  • agent.test.ts: a 32,768-token window gives the agent model that window and an 8,192-token output limit; a 2,048-token window fails before the agent starts; with a 32,768-token window the agent starts with the newest history that fits, opening at a user message.
  • diagnostics.test.ts: the new code maps to a Settings → Models fix.
  • Each of the fail-fast check, the budgeted history, and the user-message start fails its tests when removed.

The system prompt, tool definitions and prompt are measured in tokens
the way pi estimates them, and a quarter of the window (at most pi's
compaction reserve) stays free for the reply. If those parts do not fit,
the run fails before the request with GENERATION_CONTEXT_EXCEEDED, which
names the window and points to Settings -> Models, instead of sending a
request the model would truncate. History keeps its newest messages that
fit what remains, starting at a user message. The budget is logged with
each run.
Copilot AI balanced review requested due to automatic review settings October 11, 2026 11:44
@github-actions github-actions Bot added docs Documentation area:core packages/core (generation orchestration) labels Oct 11, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Tool-definition tokens are counted but never tested. Every planContextBudget case in the new test passes tools: [] (packages/core/src/context-budget.test.ts:19), so the toolTokens path (packages/core/src/context-budget.ts:42) only ever runs on an empty array. Because the PR's central motivation is the ~16k characters of tool definitions, a regression in that mapping (name/description/parameters) or in the estimate would still pass CI.
    Suggested fix: add a case with one or two tools and assert budget.toolTokens reflects their serialized size, plus a window that fits system prompt + prompt but not the tools and throws GENERATION_CONTEXT_EXCEEDED.

Questions

  • When the fixed parts overflow because the prompt itself is large (long brief, memory, or a pasted attachment), the diagnostic still says "Check the context window" → Settings → Models (packages/shared/src/diagnostics.ts:356). Raising the window may not resolve an oversized prompt. Should this path also hint at shortening the prompt/attachments, or be worded to cover both causes?

Summary

Review mode: initial.

No blockers or majors found. The change is well scoped: planContextBudget fails before any request when the fixed parts (system prompt + tool definitions + prompt/images) do not fit the window, trims history to the newest messages that fit and opens at a user boundary, and the new GENERATION_CONTEXT_EXCEEDED code flows consistently through ERROR_CODES/ERROR_CODE_DESCRIPTIONS, diagnoseGenerateFailure, and all four locales (en, es, pt-BR, zh-CN). The PR uses Refs #475, #394 rather than a closure claim, which is the correct form for partial work, so there is no issue-closure mismatch to flag.

Residual risk / worth confirming with a maintainer eye:

  • With common local 4k–8k defaults (e.g. Ollama num_ctx), the fixed parts alone already exceed the window, so those runs now hard-fail where they previously sent a request the server would truncate. This aligns with the project's "no silent fallbacks for user-visible failure" constraint, but the user-facing advice ("Check the context window") may need to point local users at raising the model's own context length, since our setting cannot grow a model that only supports 4k.
  • contextWindow: piModel.contextWindow is assumed to be an always-resolved number (from #476). If it can ever be undefined/NaN, Math.min/subtraction produce NaN, available < 0 is never true, and every run silently keeps full history instead of failing. I could not confirm the type contract from the provided diff, so treating this as a question rather than a finding.
  • Public docs were not updated for the new error/behavior (the PR body leaves that checkbox open); fine if the diagnostic copy is considered sufficient, but worth a deliberate decision.

Testing

  • Not run (automation). Author reports pnpm -r typecheck, pnpm lint, and CI=1 pnpm test passing.
  • Suggested additions: the non-empty-tools case above; and a history that ends on a toolResult/assistant boundary to lock in the "history opens at a user message" invariant that the new trimming relies on.

Open-CoDesign Bot

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The agent’s 32,000-token completion limit is not capped to the newly calculated output reserve, so requests can still exceed the model window.

1 open finding
What changed in this PR

Implements #475’s token-budgeting step atop #476’s model-specific context windows.

Changes:

  • Budgets fixed prompt content, output reserve, and recent history.
  • Adds context-overflow diagnostics and translations.
  • Adds focused unit and integration coverage.
File Description
.changeset/​context-token-budget.md Records user-visible behavior.
packages/​core/​src/​context-budget.ts Implements token budgeting.
packages/​core/​src/​context-budget.test.ts Tests budgeting behavior.
packages/​core/​src/​agent.ts Applies budgets before agent creation.
packages/​core/​src/​agent.test.ts Tests fail-fast and history trimming.
packages/​shared/​src/​error-codes.ts Adds the overflow error code.
packages/​shared/​src/​diagnostics.ts Adds remediation diagnostics.
packages/​shared/​src/​diagnostics.test.ts Tests diagnostic routing.
packages/​i18n/​src/​locales/​en.json Adds English strings.
packages/​i18n/​src/​locales/​es.json Adds Spanish strings.
packages/​i18n/​src/​locales/​pt-BR.json Adds Brazilian Portuguese strings.
packages/​i18n/​src/​locales/​zh-CN.json Adds Simplified Chinese strings.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/core/src/agent.ts
Every budget test passed an empty tool list, so the tool definitions,
the largest fixed part of a run, were never counted in a test. A new case
checks that only name, description and parameters are counted and that a
tool alone can push a run past the window. The error now also suggests
shortening the request or its attachments, since a long prompt is one of
the parts that must fit.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

No blockers, majors, or minors in the follow-up commits.

  • [Minor] (from the previous review) Tool-definition tokens are now tested — resolved. packages/core/src/context-budget.test.ts:62 adds a tool with a label field and asserts toolTokens === 1000 from the name/description/parameters-only serialization, and that a 2,400-token window fits the fixed parts without tools but throws GENERATION_CONTEXT_EXCEEDED with them. The serialized length checks out at exactly 4,000 chars ([{"name":"read","description":"…3935…","parameters":{"type":"object"}}]), and base.tools defaults to [] so the empty-array case (ceil(2/4) === 1) matches the 1,101 noted in the neighboring test.

Questions

None required.

Summary

Review mode: follow-up after new commits.

Both points from the prior review are addressed:

  • The tool-token coverage gap is closed by the new counts tool definitions by name, description and parameters case (packages/core/src/context-budget.test.ts:62).
  • The wording question (raise the window vs. shorten the request) is answered in the user-facing copy: err.GENERATION_CONTEXT_EXCEEDED / ERROR_CODE_DESCRIPTIONS.GENERATION_CONTEXT_EXCEEDED now lead with "Shorten the request or its attachments", synchronized across en, es, pt-BR, and zh-CN (packages/shared/src/error-codes.ts:233).

No new behavior was introduced by the follow-up diff — it is a test addition plus message-copy changes, and the messages remain consistent between error-codes.ts and all four locale files.

Residual observations (unchanged from the initial review, not blockers):

  • The diagnostic's suggestedFix.label is still "Check the context window" → Settings → Models (packages/shared/src/diagnostics.ts:356). That remains the correct primary action for the overflow; the shorter-request guidance now lives in the error text, so the earlier question is resolved at the message level. Optional polish if you want the fix action to name both options.
  • piModel.contextWindow is assumed to be an always-resolved number. I could not verify that contract from the provided diff/search context, so this stays a residual risk rather than a finding: if it can ever be undefined/NaN, Math.min and the subtraction yield NaN, available < 0 is never true, and the run would keep full history instead of failing fast. Worth a Number.isFinite guard if the type does not guarantee it.

Testing

  • Not run (automation). Author reports pnpm -r typecheck, pnpm lint, and CI=1 pnpm test passing (core 567, shared 327, desktop 2,629).
  • The prior suggested non-empty-tools test is now present; no further additions are proportional to the remaining risk.

Open-CoDesign Bot

The budget kept a quarter of the window free for the reply, but every
request still asked for 32,000 output tokens, so an 8k model received
about 6k tokens of input plus a 32k completion limit, which providers
that check input plus output against the window reject. The reserve is
now a quarter of the window, at most the requested output, and the
request's maxTokens is set to it. Models with a window of 128k tokens or
more keep 32,000.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Nit] The PR description now contradicts the code and the changeset — the follow-up replaced pi's compaction reserve with the run's requested output (packages/core/src/context-budget.ts:40, outputReserve = Math.min(input.maxOutputTokens, Math.floor(input.contextWindow / 4))), and the durable changeset says "at most the 32,000 tokens a run requests" (.changeset/context-token-budget.md:7), but the PR body still says "at most pi's compaction reserve of 16,384 tokens" and the Principles section still claims estimates use "pi's estimateTokens and its compaction reserve". The merged artifact (changeset) is correct, so this is only description accuracy.
    Suggested fix: update the PR body to describe the reserve as the model's requested output (and drop the DEFAULT_COMPACTION_SETTINGS reference), matching the code and changeset.

Questions

  • None required.

Summary

Review mode: follow-up after new commits.

No blockers or majors in this follow-up. The delta since the prior review is small and directionally sound:

  • Reserve is now the run's requested output instead of pi's DEFAULT_COMPACTION_SETTINGS.reserveTokens, and the agent's maxTokens is set to that reserve so input + output fit the window (packages/core/src/agent.ts:1390). The wiring is covered by the new assertion expect(...model?.maxTokens).toBe(8_192) (packages/core/src/agent.test.ts:662), which is consistent with min(32_000, floor(32_768/4)).
  • context-budget.test.ts and the changeset were updated in lockstep (outputReserve === 32_000 in a large window, 2_048 in an 8,192 window), and the values check out.

Prior findings are resolved: the tool-definition coverage gap is closed, and the user-facing message wording (shorten the request vs. raise the window) is consistent across error-codes.ts and all four locale files.

Residual observations (not blockers, no action required now):

  • In-place mutation of the model object at packages/core/src/agent.ts:1390 is safe only if piModel is a per-run object. The new test passing 8,192 on the first captured call, together with reserve = min(maxTokens, window/4) being idempotent for a fixed window, means a shared/cached object would neither drift nor shrink progressively; the only concern would be a different consumer of that same object. If piModel is ever cached, prefer returning maxTokens from planContextBudget and constructing a fresh model rather than mutating the input.
  • The piModel.contextWindow finite guard raised in the earlier review is unchanged; it remains a residual risk rather than a finding.

Testing

  • Not run (automation). Author reports pnpm -r typecheck, pnpm lint, and CI=1 pnpm test passing (core 568, shared 327, desktop 2,629).
  • New coverage is proportional: context-budget.test.ts:26 for the reserve in large/small windows and agent.test.ts:662 for the maxTokens output limit; no further additions are needed for this slice.

Open-CoDesign Bot

@Sun-sunshine06
Sun-sunshine06 merged commit 029128d into OpenCoworkAI:main Oct 13, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core packages/core (generation orchestration) docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants