CL-6204: real tool-payload budget estimation + default Ollama adapter - #264
Merged
Conversation
…adapter registration Reproduces the CL-6204 gaps: estimateTurnsChars undercounting tool_call/ tool_result payloads by orders of magnitude (the reviewer's 10-turn, 20,000-char tool_result repro), compaction output exceeding the budget it folded to once the summary turn's own size is counted, and SIDECAR_ADAPTER_MANIFEST defaulting to no adapters so a seeded Ollama model's num_ctx never reaches the request.
…Ollama adapter by default
The context-budget estimator (estimateTurnsChars) measured excerptBlock's
human-readable placeholder ("[tool_result callId]", ~20 chars) instead of
a turn's real payload size, so tool-heavy history silently undercounted
by orders of magnitude, missed the hard limit, and let Ollama truncate
server-side with no error. Compaction now also reserves the summary
turn's own worst-case size out of the budget, so a fold no longer lands
a summary on top of an already-full budget.
SIDECAR_ADAPTER_MANIFEST defaulted to [], so @corbits/ollama-adapter
(shipped in this repo) was only ever wired up by hand-editing an env var
-- a default deployment ran the built-in OpenAI adapter against Ollama
and never sent options.num_ctx. The manifest now defaults to registering
the Ollama adapter for the "ollama" provider key; an operator-set
manifest still fully replaces it rather than merging. The adapter package
is added as a dependency of the sidecar app and hoisted to the workspace
root (matching the other @corbits/* packages already hoisted there) --
without that, @intx/inference's dynamic import of the specifier from
deep inside its own node_modules cannot resolve it at all.
Also removes workbench-director.ts's dead duplicate director factory
(defined/workbenchDirectorFactory/buildWorkbenchDirectorRef): it never
received a contextBudget and had zero non-test call sites, a legacy path
left beside the one createWorkbenchDirectorRegistry actually uses.
…ama adapter Documents that the sidecar registers @corbits/ollama-adapter out of the box and that setting the variable replaces the default wholesale rather than merging with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes CL-6535
Summary
CL-6204 (the context-compaction work) merged as Done, but two of its
core pieces were inert on
main:estimateTurnsCharsmeasured
excerptBlock's human-readable placeholder(
[tool_result callId], ~20 chars) instead of a turn's real payload.10 turns each carrying a 20,000-char
tool_resultmeasured as ~160chars against a 32,000-char hard limit — overflow detection and
compaction never fired, and Ollama silently truncated server-side.
@corbits/ollama-adapternever loaded by default.SIDECAR_ADAPTER_MANIFESTdefaulted to[], so a default deploymentran the built-in OpenAI adapter against Ollama and never sent
options.num_ctx, even though the seed already writes per-modelnumCtxquirks onto seeded Ollama offerings.Changes
estimateTurnsCharsnow measures real payload size:tool_callargument JSON,
tool_resultcontent (including media source sizes),thinking/citation/safety/code-execution blocks — kept separate from
the human-readable excerpt used in the recap summary.
createBudgetedContextCompactorreserves the summary turn's ownworst-case size out of the budget before selecting kept turns, so
folded output can't exceed the budget it folded to (previously the
summary could land on top of an already-full budget).
SIDECAR_ADAPTER_MANIFEST's default now registers@corbits/ollama-adapterfor the"ollama"provider key instead ofan empty manifest. An operator-set value still fully replaces the
default rather than merging. This also required hoisting
@corbits/ollama-adapterto the workspace root —@intx/inference'sdynamic
import()of the specifier, invoked from deep inside its ownnode_modules, cannot otherwise resolve a package only linked intoindividual consumers'
node_modules.workbench-director.ts's dead duplicate director factory(
defined/workbenchDirectorFactory/buildWorkbenchDirectorRef): itnever received a
contextBudgetand had zero non-test call sites.Follow-ups filed, not fixed here
context-budget.ts's hard limit reserves no headroom for the systemprompt or tool definitions, though its header comment claims to.
Needs the actual system-prompt size threaded through at that layer.
Test plan
apps/sidecarpackage test suite: 186 pass, 0 failpackages/ollama-adapterpackage test suite: 14 pass, 0 failapps/sidecarpackage-scopedtsc --noEmit: cleantool_resultturns) and proves the honest-overflow path nowfires
SIDECAR_ADAPTER_MANIFEST)resolves
ollamaand thatquirks.numCtxreachesoptions.num_ctxin the built request