feat(agent-core-v2): add tokenCounting service with strategy config and measured anchors - #2563
Conversation
🦋 Changeset detectedLatest commit: 588e093 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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: 4f1cbd1a7d
ℹ️ 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".
| // rebases the measured model first, so the max only wins in that window. | ||
| const measured = wire.getModel(ContextSizeModel); | ||
| const contextTokens = Math.max(contextSize.get().size, measured.tokens); | ||
| const contextTokens = Math.max(tokenCounting.get().size, tokenCounting.latestMeasured()); |
There was a problem hiding this comment.
When a user sets [token_counting].strategy = "estimated" to ignore unreliable provider usage, tokenCounting.get().size returns the heuristic estimate but this Math.max immediately reintroduces the last provider-reported anchor. If that provider usage is bogus and higher than the estimate, legacy REST/WS status still reports the provider number for contextTokens (the same calculation is mirrored in packages/node-sdk/src/v2/session-wiring.ts:282), so the context-size display contradicts the new strategy. Gate latestMeasured() by strategy here or have the service expose the correct status count.
Useful? React with 👍 / 👎.
| historyForModel = shrinkCompactionHistoryAfterOverflow( | ||
| messagesToCompact, | ||
| overflowShrinkCount, | ||
| (message) => this.tokenCounting.estimateMessage(message), |
There was a problem hiding this comment.
When [token_counting].strategy = "measured", IAgentTokenCountingService.estimateMessage() returns 0 by design. Passing that gated estimator into shrinkCompactionHistoryAfterOverflow makes totalTokens and tokenBudget both 0, and takeRecentMessagesWithinTokenBudget keeps every message because zero-sized messages never exceed the zero budget. Any compaction request that gets a 413 is therefore retried with the same messagesToCompact until the attempt limit, so measured-only users cannot recover from overlarge compaction prompts; this shrink path needs the raw heuristic estimator or another count-based fallback.
Useful? React with 👍 / 👎.
…nd measured anchors - add IAgentTokenCountingService as the single owner of token counts: context size, full-request size, and estimate primitives, replacing the scattered contextSize/tokenEstimate/fullCompaction paths - add [token_counting] config section with strategy = measured+estimated (default) / measured / estimated, plus the KIMI_TOKEN_COUNTING_STRATEGY env override; measured zeroes all estimates, estimated ignores anchors - keep a live measured-anchor ledger in TokenCountingModel: each LLM exchange writes a real anchor, undo truncates the ledger so the surviving prefix restores its REAL measured size instead of a re-estimate, and compaction rebases to a single anchor that blends the compaction exchange's measured summary output tokens - skip writing an anchor when the stream reports no usage event instead of anchoring emptyUsage() zeros, which zeroed the context size and silenced compaction for providers without usage reporting - return the strategy-resolved size (not measured) from rpc getContext so the tokenCount contract stays correct under the estimated strategy - migrate all consumers (contextMemory, fullCompaction, llmRequester, rpc, mirrorAgentRun, sessionLegacy, kap-server legacyStatus, node-sdk, kimi-inspect) to the new service; edge bridges no longer read the wire model directly - document [token_counting] and KIMI_TOKEN_COUNTING_STRATEGY in the bilingual config reference
- readLegacyStatus falls back to the default model's context limit when no model is bound, and omits maxContextTokens entirely when the limit is unknown (0 is the engine's UNKNOWN_CAPABILITY marker, not a real limit) - profileService no longer emits maxContextTokens in agent.status.updated when the bound model alias does not resolve
…ting edge - keep measured anchors and heuristic estimates both recorded and feeding internal logic (compaction triggers, budgets, overflow backoff) regardless of the configured strategy - add IAgentTokenCountingService.statusSize() as the single strategy-resolved outward reading and route the WS/REST/RPC status surfaces through it - fix the context-size display falling back to provider-reported usage under the estimated strategy - fix compaction overflow backoff retrying identical messages until failure under the measured strategy (the strategy-gated estimator read as 0)
8d61a3d to
588e093
Compare
1 participant
Related Issue
None — the problem is explained in the next section.
Problem
Token counting in the v2 engine had three issues:
emptyUsage()zeros, zeroing the context size and silently disabling compaction).contextSize, compaction trigger/budget helpers, and a private request-size estimate in full compaction, each interpreting sizes its own way.{length, tokens}pair: undoing past it rebased the surviving (previously measured) prefix to a fresh estimate, and compaction adopted an estimatedtokensAfteras the new "measured" value.What changed
IAgentTokenCountingService(agent/tokenCounting) as the single owner of every token count — context size, full-request size (system prompt + tools + messages), and estimate primitives — replacing the splitIAgentContextSizeService/IAgentTokenEstimateService/ full-compaction-local paths. All consumers (contextMemory, fullCompaction, llmRequester, rpc, mirrorAgentRun, sessionLegacy, kap-serverlegacyStatus, node-sdk session wiring, kimi-inspect panels) now go through it; the v1 edge bridges no longer read the wire model directly.[token_counting]config section withstrategy = "measured+estimated" | "measured" | "estimated"(defaultmeasured+estimated) and theKIMI_TOKEN_COUNTING_STRATEGYenv override.measuredreads all estimates as 0 (decisions rely on real usage, compaction splits degrade to count-based);estimatedignores measured anchors and estimates everything — the escape hatch for providers without usage reporting.TokenCountingModel(live-only,persist: false— the wire format is unchanged): every LLM exchange writes a real anchor,undotruncates the ledger so the surviving prefix restores its REAL measured size instead of a re-estimate, and compaction rebases to a single anchor whose summary component is the compaction exchange's measured output tokens (persisted assummaryOutputTokenson the compaction record for replay).rpc getContextreturns the strategy-resolvedsize(measured + estimated tail) instead ofmeasured, keeping the v1context.tokenCountsemantics correct under theestimatedstrategy (wheremeasuredalone reads 0).Verified end-to-end with the klient memory transport against a real provider: under
measured+estimatedandmeasuredthe reported tokenCount exactly equals provider-reported usage after each turn; underestimatedit exactly equals the character heuristic and ignores provider usage. Known limits: provider usage is exchange-granular (real values exist at exchange boundaries only), and anchors are live-only (resume estimates until the next exchange, as before).Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.