test(mcp): benchmark every MCP tool against a committed baseline - #23
Merged
Conversation
Covers all four advertised tools across 20 scenarios, driven through handle_tools_call — the same dispatch the stdio server uses — so argument parsing and error paths are exercised rather than bypassed. `every_advertised_tool_is_benchmarked` reads tools/list and fails if a tool has no scenario, and fails in reverse if a scenario names a tool that no longer exists, so "full coverage" is checked rather than claimed. Separate from efficiency.rs on purpose. That suite measures this repository, which is right for "what does this save on real code" and wrong for showing progress: its numbers move when the corpus moves, and its break-even count went 17 -> 16 within an hour of publication because a commit added a file. This suite measures frozen generated inputs, so a delta can only mean the code changed. Two things had to be fixed before the baseline meant anything. Reproducibility. Every tool embeds the path it was given in its reply header, so the path's token cost lands in returned_tokens. With a random TempDir, three runs with no code change produced 351, 351 and 357 tokens for one scenario — larger than most real improvements, and enough to fail CI at random. An absolute path is no better: /Users/... and /home/runner/... tokenize differently, so a baseline blessed locally could never match in CI. Fixtures now live at a short relative path, and five consecutive runs are bit-identical. Comparability. Measurements alone would let an edited fixture pass as an optimizer improvement — a smaller number for a smaller input. The baseline records an FNV-1a hash of every input beside the measurements, and a changed generator fails with "the deltas are not comparable" rather than reporting a win. FNV rather than DefaultHasher, whose stability across Rust releases is unspecified. Verified in both directions: LUMEN_LINE_NUMBERS=0 registers as -12.2%/-12.4%/-11.1% on the gutter-heavy scenarios, and re-widening the gutter fails the suite naming the three affected scenarios and their exact token deltas. Token counts are asserted; wall-clock is reported and never asserted, because a shared runner cannot support that claim and a flaky perf gate teaches people to re-run until it passes. Not wired into CI yet — that is the next step once the output has been reviewed.
…ld never match Same defect as the random tempdir, one level down. Every tool embeds the path it was given in its reply header, so anything that varies in the path varies in returned_tokens — and PathBuf::join yields target/bench-fixtures\\big.rs on Windows, which tokenizes differently from the Unix form. The Windows CI job failed on nine scenarios by +1 to +3 tokens with no behavioural difference whatsoever. A benchmark that reports a platform's path convention as an optimizer regression is worse than no benchmark. The path is now built as a string with forward slashes, which Windows accepts, so the header is byte-identical everywhere. macOS numbers are unchanged, so the committed baseline still stands.
HackPoint
force-pushed
the
feat/optimizer-benchmarks-2
branch
from
August 1, 2026 19:30
3be5c52 to
c652719
Compare
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.
Token-efficiency benchmarks for all four MCP tools, compared against a committed baseline so each
fix's effect is visible as a delta.
Not wired into CI — that is the follow-up once you have looked at the output.
Coverage
20 scenarios across
smart_read,recall_file,compress_logsandlumen_ping, driven throughhandle_tools_call(the dispatch the stdio server uses) so argument parsing and error paths areexercised rather than bypassed.
Coverage is checked, not claimed:
every_advertised_tool_is_benchmarkedreadstools/listandfails if any advertised tool has no scenario — so a fifth tool fails this suite until someone
measures it. It fails in reverse too, if a scenario names a tool that no longer exists.
Scenarios deliberately include the cases where the tools should not win: an outline larger than its
file, a tiny file, an unparseable language, a range covering everything, an incompressible log.
Measuring only the good cases would make the table meaningless.
Current output
The negative rows are the honest ones:
many_tiny_items,unsupported_language,full_modeandrange_whole_fileall cost slightly more than the file, which is the inflation guard returning thefile plus one explanatory line.
incompressibleis exactly break-even. Those numbers should staysmall and negative; if one grows, something regressed.
Why this is separate from
efficiency.rsefficiency.rsmeasures this repository. That is right for "what does the optimizer save on realcode" and wrong for showing progress — its numbers move when the corpus moves. Its break-even count
went 17 → 16 within an hour of being published, because a commit added a file. A benchmark whose
numbers drift for unrelated reasons cannot attribute a change to a fix.
This one measures frozen generated inputs, so a delta can only mean the code changed.
Two things that had to be fixed before the baseline meant anything
It was not reproducible. Every tool embeds the path it was given in its header, so the path's own
token cost lands in
returned_tokens. With a randomTempDir, three runs with no code change gave351, 351 and 357 tokens for one scenario — noise larger than most real improvements, and enough to
fail CI at random. An absolute path is no better:
/Users/...and/home/runner/...tokenizedifferently, so a baseline blessed locally could never match in CI. Fixtures now live at a short
relative path; five consecutive runs are bit-identical.
Measurements alone are not comparable. An edited fixture would pass as an optimizer improvement —
a smaller number for a smaller input. The baseline records an FNV-1a hash of every input beside the
measurements, and a changed generator fails with "the deltas are not comparable" instead of reporting
a win. FNV rather than
DefaultHasher, whose cross-release stability is unspecified.Verified in both directions
Improvement —
LUMEN_LINE_NUMBERS=0drops the per-line gutter:Regression — re-widening the gutter to
{lineno:>5}::How to use it
Commit the regenerated baseline with the fix that moved it. The git history of one JSON file then
becomes the record of the optimizer's progress, fix by fix — which is the thing you asked for.
Judgement calls worth your attention
flaky perf gate teaches people to re-run until it passes. The µs column is there to spot something
pathological, not to gate.
of fixture text would not be, and nothing can reformat it. The tradeoff is that they are synthetic
rather than real-world shapes —
efficiency.rscovers real code, so the two are complementary.hide small regressions. If an increase is intended, re-bless in the same commit.
history, which is the one thing this file exists to provide.
Once you are happy with the numbers, the CI step is small — add it to the existing test job and
require it, the same way the other checks are required.
Correction to the table above. It originally quoted figures taken before the relative-path fix
(
outline_few_big_itemsat 351 rather than 278). Those were measured against a long random tempdirpath whose token cost was inside the numbers — exactly the noise this PR removes. Replaced with the
committed baseline.
Windows. The first push failed
Test (windows-latest): nine scenarios off by +1 to +3 tokens.PathBuf::joinproducestarget/bench-fixtures\big.rsthere, and the separator is in the header andtherefore in the measurement. Same defect as the tempdir, one level down. Fixed by building the path
as a forward-slash string, which Windows accepts. macOS numbers were unchanged, so the baseline still
stands. All 11 checks green.