fix: defects that surfaced while building the distribution funnel (expand-tool gate, count_tokens, C-toolchain claim, preset facts)

[amiddavid]

Pulled out of #141 at review request: none of this is distribution work. Every item is a defect in code or docs that already shipped, found while building the funnel, and it can land whether or not the funnel does. #141 now contains only genuine distribution work and stacks on this.

What is here

#	Defect	Pre-existing?
1	context_guru_expand advertised to pipelines that cannot mint a marker — including off, the A/B control arm	yes
2	POST /v1/messages/count_tokens not served, so clients count context with inference requests	yes
3	Our docs told every evaluator to install a C toolchain; setup.md named bifrost's tokenizer as a cgo dep, which it never was	yes
4	The binary defaults to house; five documented sites said codesmart. Stale pipeline lists in README and the IBM page (#143, #145)	yes

On #1 — the control arm is the part I would look at first. expand.Inject under auto gated on "declares tools" + "store persists", and nothing asked whether the pipeline contained an Offload. So off — the passthrough baseline — forwarded an extra tool declaration, meaning every A/B taken against it compared two arms differing by more than the pipeline. Verified on the wire before fixing: [Read Bash] in, [Read Bash context_guru_expand] out.

It survived because the code contradicted its own comment: Options.InjectExpand claimed a marker condition three lines from expand/inject.go's "No marker condition, deliberately". Both now say what happens. The gate asks Pipeline.HasOffload() by type assertion, not a name list — a name list is a second copy of "which components are lossy" and rots on the next component added.

Ten existing tests changed fixture, and that deserves a look. Every expand-loop test hand-seeds the Store to simulate an offload but built its handler with pipeline: [], which cannot offload. Harmless while injection ignored the pipeline. They now use offloadCapablePipeline ([linecap], inert on their short bodies). No assertion weakened — each fixture just now matches its own premise.

On #3, the claim is asserted rather than restated: a new purego CI job builds with CGO_ENABLED=0 and CC=/nonexistent-c-compiler, checks the binary is statically linked, starts it, probes /healthz. It also runs the component-sensitive packages with CGO off, because build-test runs exclusively with CGO_ENABLED=1 (the race detector needs it) — so TestEveryPresetBuilds, the guard for unknown component "skeleton", had never run in the configuration a user would actually build.

Verification

Five mutations, each proven to have landed in the source before its result counted, each failing the named test and passing when restored — full output in the commit body. The one worth naming is HasOffload always false, which proves the gate did not trade the original defect for its mirror image: an offloader whose output nothing can expand.

Two mistakes of mine that tests caught, not me:

1. I asserted mcp had no offloader. smartcrush implements components.Offload, so that pipeline really does mint markers and really does need the tool. The case now asserts the opposite with the reason attached — and it is the argument for asking the interface rather than maintaining a list.
2. Copying proxy/proxy.go wholesale from the older distribution branch onto current main silently reverted fix(dash): report unpriced components in tokens, state the cache-frozen ceiling, and export the counters that only reached /stats #155's effPreset/notePreset work. TestCompactRowNamesThePresetThatRan, a test I had never read, failed with "the dashboard names a pipeline that did not run". I restored main's file and re-applied my two edits on top; fix(dash): report unpriced components in tokens, state the cache-frozen ceiling, and export the counters that only reached /stats #155 is intact and verified.

One flake, reported not hidden

TestConcurrentCallsDoNotRaceOnTheGateHistogram failed once in a full-suite run — "no single-flight follower ran ... the race was never exercised" — then passed 8/8 in isolation, in two further full suites here, and in a full suite on clean main. Unrelated to this change (nothing here touches extract_llm), filed separately.

Related

• feat(dist): pure-Go releases, a cache preset, --idle-exit, and gateway conformance #141 — the distribution work, stacked on this. Merge this first.
• fix(docs): correct every row of the "What each one runs" preset table, and guard it #142 — the preset tables inside the drift guard's reach. Untouched here.
• docs: README and connect-ibm-service describe pipelines that no longer ship (prose, so #142's guard misses them) #143 / docs: preset facts outside docdrift's scope are stale — wrong default preset in 5 places, stale README pipeline lists #145 — the doc defects fixed in item 4; docs: README and connect-ibm-service describe pipelines that no longer ship (prose, so #142's guard misses them) #143 duplicates docs: preset facts outside docdrift's scope are stale — wrong default preset in 5 places, stale README pipeline lists #145 and should be closed in its favour.

🤖 Generated with Claude Code

[OsherElhadad]

Reviewed, built, and ran locally. The engineering is sound and I couldn't break it: all five listed mutations reproduce by hand (revert-and-confirm-FAIL, restore-and-confirm-PASS), and I extended the check into a full matrix over all 14 presets and all 21 registered components — HasOffload() is correct everywhere, no mirror-image gap. The purego claim reproduces end-to-end: CGO_ENABLED=0 CC=/nonexistent-c-compiler go build ./cmd/context-guru-proxy really does produce a static binary that starts and answers /healthz, and I drove the new count_tokens route against the real Anthropic upstream and got its actual 401 relayed verbatim. Both of the two near-misses this PR self-reports catching are genuinely resolved — smartcrush really does implement components.Offload, and proxy/proxy.go against current main is a clean three-hunk superset with #155's effPreset/notePreset fully intact.

What blocks this from merging as-is is that the PR's prose — which is the whole point of a PR themed on "our docs/comments say things that aren't true" — has the same problem in three more places, one of them in shipped code, not just docs.

Merge-blocking

1. proxy/proxy.go:1164's comment repeats the exact mistake this PR says it caught twice already.

It reads: an offloader-free pipeline (\cache`, `off`, `safe`, `mcp`). mcpis not offloader-free — this PR's own test assertswantAdd: truefor it, and forcingHasOffload()to always-false fails on exactlymcpplus the offloader-capable fixture, proving the gate genuinely depends onsmartcrushbeing anOffload`. The self-caught error was fixed in the test and left standing in the comment a future reader will actually trust.

2. Several sites name a preset that doesn't exist on this base. proxy/proxy.go:94, proxy/proxy.go:1164, proxy/proxy_test.go:43, proxy/counttokens_test.go:93, proxy/expandgate_test.go:15 all reference "the cache preset." There's no cache entry in config/config.go's presets map — only an unrelated Cache CacheConfig yaml field. This looks like a copy-paste artifact from the sibling distribution branch (#141), consistent with the admitted wholesale copy of proxy/proxy.go from there. The underlying defect these comments describe is real (proved it independently on off/safe/cachesplit-only) — just the attribution needs to name a preset that's actually on this branch.

3. proxy/counttokens_test.go carries copy-paste artifacts gofmt/vet won't catch. Lines 93-112 are a 20-line orphaned doc comment for TestCachePresetAdvertisesNoExtraTool, a function that doesn't exist here (moved/renamed into expandgate_test.go); it also cites TestCachePresetIsCachesplitAlone, which exists nowhere in the repo. Lines 61-63 duplicate 65-67 verbatim.

Should land, not necessarily blocking

4. docs/reference/config.md:88 and docs/components.md:82-84 still say auto injection fires on exactly two conditions ("Both conditions are properties of the session") — there are now three. In a PR about false doc claims, this is a new one.

5. Items 3 and 4 are incomplete:

• docs/get-started/quickstart-proxy.md:6 still says a C toolchain is required — and make build (its own step 1) still does, since the Makefile exports CGO_ENABLED=1 and I confirmed it fails without a C compiler. make build-static exists but nothing points a reader at it. README.md:88 and CLAUDE.md:26 are also still unfixed.
• config/config.go:406,445, docs/components.md:34, docs/how-to/choose-a-preset.md:14,46, and docs/how-to/use-with-claude-code.md:84 still assert codesmart is the default. choose-a-preset.md is one of the four docs the orphaned comment in finding 3 names, so it was in view and skipped.

6. docs/setup.md:8-14 overstates its own evidence — claims CI removes the C compiler from PATH (it doesn't; CGO_ENABLED=0 is what does the work, and I confirmed CC is never consulted once cgo is off) and claims cross-compilation across four GOOS/GOARCH targets is asserted, when CI only builds native linux/amd64.

7. ci.yaml:78 names TestCtlGetCampaignAggregatesPredictedAndRealPerTenant as the flake with "see the issue linked below" and no link follows; the PR body separately names a different flake (TestConcurrentCallsDoNotRaceOnTheGateHistogram, #163). One of the two references is wrong — worth a one-line fix.

Not asking for, but noting

HasOffload() itself has no unit test inside ./components, only proxy-level integration coverage — cheap insurance for a method whose whole selling point is not rotting, but not blocking. Also: main has since gained #142, which also touches the preset tables finding 5 needs to fix — rebase before doing that pass so the two don't re-fix or re-break each other's work.

Fail-open holds: the new route forwards verbatim so there's nothing to fail open from, 502-on-unreachable matches every other route, counttokens.go:203-208 deliberately returns a fixed string rather than err.Error() so a *url.Error can't leak the operator's upstream address, and I found no panic path.

Requesting changes for 1-3; happy to see 4-7 as an immediate follow-up if you'd rather land the logic now and the doc pass separately.

[amiddavid]

All seven fixed, plus the HasOffload unit test you said you weren't asking for — it was cheap and you were right that integration-only coverage is thin insurance for a method whose selling point is not rotting.

Rebased onto current main first, as you suggested, so #142's guard and finding 5's preset pass don't undo each other.

Merge-blocking

1. The mcp comment. Fixed, and the lesson generalised rather than the list corrected: I deleted the list. The comment now says which shapes are affected (off, safe, cachesplit-only) and then says explicitly that enumerating presets here is the wrong move because mcp looks offloader-free and isn't — with smartcrush named. A second source of truth that was wrong on its first draft has no business being the thing a future reader trusts.

2. cache doesn't exist on this base. All five sites reworded. You diagnosed the cause correctly — it is copy-paste from the wholesale proxy/proxy.go copy I admitted to, and it travelled into the test files when I split them out of that branch's conformance_test.go.

3. counttokens_test.go artifacts. Both gone: the duplicated 3-line doc comment and the 20-line orphan for a function that lives in expandgate_test.go under a different name. Same origin — my splitter took each test's doc comment by scanning back to the previous blank line, which swallowed the following test's comment as a trailing block. I also found and rewrote a third artifact you didn't list: expandgate_test.go's doc comment still described "the preset's promise" and cited docs/how-to/install-plugin.md and an install skill, all of which belong to #160.

Should land

4. Both sites now say three conditions, and say what the third is for. The cache-stability argument those passages make survives — a pipeline doesn't change turn to turn either — so it gained a member rather than needing a rewrite.

5. Both halves done, and the first one deserves a note because I changed behaviour rather than wording:

• make build now sets CGO_ENABLED=0. You pointed out that the docs could say "no C toolchain" all they liked while step 1 of the quickstart still required one. Pointing readers at build-static would have fixed the sentence; making the default build pure Go makes the claim true of the command we actually tell people to run. CGO_ENABLED=1 stays exported for the test targets, where -race genuinely needs it, and the Makefile comment now says exactly that. build-static is now documented as "same as build, plus -trimpath" — the release build. Verified with CC=/nonexistent make build.
• README, CLAUDE.md and the quickstart updated. All five codesmart-is-default sites fixed, including the two in config/config.go — which is how the claim spread: it sat three lines from the flag that disproves it. The chooser row in choose-a-preset.md now distinguishes "recommended pipeline" from "the binary's default", and I checked it does not disturb fix(docs): correct every row of the "What each one runs" preset table, and guard it #142's guard (that guard matches rows whose first cell is a backticked preset name; the chooser's preset is in the second).

6. Corrected, and this one was the most deserved hit in the review — an evidence claim overstating its evidence, in a PR about exactly that. setup.md now says CI asserts the pure-Go build natively for linux/amd64, and states separately that the other three targets were verified by hand and are asserted at release time, not per PR. I also fixed the same overstatement in the ci.yaml comment: CGO_ENABLED=0 is what does the work, and CC is a tripwire for the day someone flips that variable, not the mechanism.

7. The comment now links #163 (the campaign flake it actually describes). The PR body's #162 is a different test; both are real, the comment was pointing at the wrong one.

The one you weren't asking for

Pipeline.HasOffload now has unit tests in ./components — nil-safe, empty pipeline, reformatters-only, and offloader-in-various-positions — revert-verified in both directions (always-true fails the empty and reformatter cases; always-false fails the offloader cases).

I also wrote a registry-walking test to make it rot-proof, and it skipped, because registrations live in components/all and a test inside components can neither see them nor import the package that does. A skipping test reads as coverage and isn't, so it moved to components/all, where it runs: 21 of 21 registered components checked, 13 implementing Offload. It asserts both counts are non-zero, so it cannot pass against a HasOffload that always answers the same way.

Full suite, vet, gofmt clean, and I re-ran the doc link/anchor checker over every doc touched.