Part three of a series on chops-search, a hybrid search engine for static sites. This one is less about search than about code review, and it stands alone. Part two is here.
Part two ended with a persistence batch: three calibrated scoring values (min_gap, rrf_alpha, an optional min_cos override) becoming config keys, riding into index.bin next to the field weights under one version bump, and getting read out by the engine at construction. The point was to close an honesty gap, since CI had been certifying a configuration that existed only as CLI flags while every visitor's browser ran the defaults. After the batch: the config file states the scoring, the build bakes it, the browser runs it, and a bare eval measures it. One configuration in four places, none of them shell history.
The batch came with a boundary rule worth restating because everything below leans on it: persist what you calibrated, default what you did not. Only the three knobs that earned calibrated values got config keys. rrf_k stays a compiled constant because its sweep was flat at every value. strong_cos stays off because no fixture case has ever needed the hatch. Every knob now has one of three provenances, corpus-calibrated in the artifact, compiled constant, or per-run flag override, and the scoring: line that eval prints marks flag overrides with an asterisk, because a sweep transcript that cannot distinguish "the index shipped 0.08" from "a flag injected 0.08" is the exact debt the batch exists to close, in a new denomination.
The batch landed. Tests green, artifacts building, search working. This post is about the review that happened next, because the review found seven problems, and the pattern across all seven is the thing worth writing down: a rewrite deletes what it does not understand, and deletions are invisible in the green path. Tests pass, everything works, and the file is quietly more fragile than the one it replaced. The review question that paid was not "is the new code right?" It was "what did the old code do that the new code no longer does?"
First, the one genuinely interesting design decision
Before the review findings, the wire-format decision the batch forced, because two of the tests it produced show up later.
min_cos is an override, not a default, and "absent" and "0.0" are different sentences. Absent means "derive the floor from embedding dimensionality at construction," which is right for almost every corpus and tracks a dims change automatically. An explicit 0.0 means "floor off," per the house convention that floors disable at zero. Conflate them and one of two legitimate engines becomes unspellable.
So the wire keeps them distinct with a fixed-width presence flag: one byte, then a four-byte f32 value field, written as zero when absent and ignored on read. Five bytes where four would have blurred the distinction. Fixed width rather than conditional, so the layout stays trivially seekable and the absent case has exactly one byte representation, which matters here because the artifact filenames are content hashes, and a nondeterministic byte forces every visitor on the planet to re-download the index.
The distinction had to be pinned at every layer that could lose it, because every layer had a habit that would merge them: a defaulted TOML key, a bare float field in the struct, an unwrap_or in the constructor. Three tests: the wire test asserts None and Some(0.0) produce different bytes and both round-trip, the config test asserts min_cos = 0.0 parses to an armed override while an absent key stays None, and the engine test asserts all three states produce the three intended engines. Keeping one distinction alive across three serialization boundaries takes a presence flag, three tests, and the willingness to spend the extra byte on it.
The review: seven absences
The batch landed as large rewrites of format.rs and config.rs rather than surgical diffs, which is what made the review question necessary. Here is what the old code did that the new code no longer did, ordered by how much it would have cost.
The dropped bounds checks. The old Index::read rejected chunk entries and posting doc ids pointing past the document count. The rewrite dropped the checks, and Engine::new indexes per-doc arrays with those values unguarded. In native code that is a caught panic with a stack trace. In wasm it is an aborted module. A truncated or corrupted artifact went from "loud FormatError with a rebuild instruction" to "the search box dies silently on every page of the site." The same rewrite dropped the NaN rejection on field weights, whose failure mode is quieter still: NaN does not crash anything, it just NaNs every score it touches, and search returns garbage with complete confidence.
The thematically perfect one. The config parser's unknown-key rejection disappeared in the same batch that made config keys load-bearing for scoring. The old code had the check, and a comment defending it: "a misspelled chunk_size that silently does nothing is worse than a failed build." Without it, a typo'd min_gp = 0.08 parses cleanly and ships the gate disarmed. Which is the honesty gap this entire batch was built to close, reintroduced through a spelling error. The check went back with the three new keys in its list, and the regression test's fixture is literally the string min_gp. A typo should cost a build, not a calibration, and the stricter the meaning of a config file, the stricter its parser has to be about words it does not know.
The parser's answer to the typo. min_gap sits right there in the known-keys list, which is the point.
Two defaults changed silently inside the refactor. The compiled dims default moved from None (native size) to Some(128), making "native" unspellable and contradicting the field's own doc comment three lines above it. The model path changed too. Both reverted, with the 128 claim moved to where it belongs: the two deployed sites' config files, where the size-versus-recall decision is visible in a diff. Here is the uncomfortable part: 128 was probably even the right value, and that is exactly what made it dangerous, because nothing looked wrong. Defaults changed inside a refactor are claims smuggled past review. A behavior change carries its own commit, its own doc update, and its own measurement, or it carries a future confusion.
The unreachable error message. The batch renamed the artifact magic strings in the same commit that bumped the format version. That combination made the carefully worded version-mismatch error unreachable by its only intended audience: a developer with a stale pre-batch out/ directory now fails the magic check before the version check ever runs, and gets "not an index.bin," which is actively confusing because it is one, just old. The fix recognizes the legacy magics specifically and emits the rebuild message from the magic branch. Writing a good error is half the work. The other half is checking who can actually arrive at it, because an error message is only as good as the path that reaches it.
A pre-batch out/ meeting the new binary. The meta file trips the check first, and the message names the fix instead of denying the file.
And the housekeeping tier, listed because absences come in all sizes: value rails lost (dims = 0 and chunk_chars = 10 were accepted without complaint), a doc comment on with_overrides stating the exact opposite of the function below it, and bytes.rs orphaned because the format rewrite grew its own private byte-cursor, leaving two implementations for the next format change to drift between. Reunified rather than deleted.
Every one of the seven was an absence: a bounds check, a key list, a rail, a reachable error path. The green path cannot show you a deleted guard, because guards only exist for inputs the happy tests never send. The question that finds them is mechanical and worth ritualizing: diff the old file against the new one and account for every rejection the old code could produce.
Rails, mirrored
The hardening that came out of the review has one organizing idea: the artifact must not be able to carry a value the config parser would have refused.
The same range checks, cosine-space quantities finite in 0..=1, alpha finite in 0..=100, now run in three places with one shape: on the TOML keys, on the build flags (through a shared validator, so a flag cannot bake what a key could not), and in the binary reader. The reader check is the one that earns its keep in the dark. A NaN min_gap read from a corrupted artifact would never gate anything, because gap < NaN is false, and the engine would run ungated forever while every test stayed green, every build succeeded, and every visitor got the junk results the gate exists to suppress. Nobody would ever know.
The payoff of checking at the door is what it licenses inside. Engine::new gets to index unguarded and the gate gets to compare unchecked precisely because read refused everything else. Validation is not paranoia layered on trust. Validation is the license for trust.
The scaffold teaches without deciding
chops-search init generates a starter config, and the batch posed a small design question with a sharp edge: the generated file is where a user learns what keys exist, and the three most consequential keys were invisible in it. But scaffolding min_gap = 0.08 would ship one corpus's calibration as if it were a universal default, and even an innocent-looking min_cos = 0.0 would arm the override on a site that should be deriving its floor.
The resolution: commented-out examples, with the calibration loop spelled out beside them. Sweep with eval, verify the mechanism with explain, pin the winner in config, rebuild. The scaffold teaches the keys exist and how to earn values for them, without asserting values it has no basis for.
Then the part that makes it durable: the template went under test. One test parses the generated file and asserts every scoring knob comes out inert, every live key is known to the parser, and no value is smuggled inside a comment block. A second strips the comment markers off the example lines and parses that, so the exact text a user will uncomment is guaranteed to be valid keys with in-range values, and a future key rename cannot leave the scaffold documenting a key the parser rejects. The template is now under test the same way the engine is, because it is an interface the same way the engine is.
Help text is a claim about defaults
A side effect of index-carried defaults that was easy to miss: four flag doc comments became lies the moment the batch landed.
"--min-gap, default 0 (disabled)" was true when the engine could only ever get 0. On a calibrated corpus, the default is now whatever index.bin shipped, and a user reading the help and omitting the flag would believe they were running ungated while the artifact gated at 0.08. Same for --rrf-alpha. Same for --min-cos, whose stated 0.20 was doubly wrong, being the 256-dim constant on top of the stale assumption. Same for build's --dims after the native-default restoration. Each now says "default: whatever index.bin was built with" or names the derivation instead of asserting a number.
The post-sweep "lock it in" hint got the same treatment. It used to prescribe carrying the winning flags around in your shell. Now it prescribes a config key and a rebuild, because locking in is precisely the thing the batch redefined. Help text is documentation that executes in the user's head at the worst possible moment, mid-debugging, and it deserves the same review as the code it describes.
Definition of done
The captures in this post are from 534e1dd, one batch after the one described here, by which point chunk_penalty had earned a key of its own through the same sweep-and-pin loop and the min_cos override had been dropped in favor of the derived floor. min_gap and rrf_alpha are as this batch shipped them.
The acceptance sequence for the batch, in order: workspace tests green. Two consecutive builds byte-identical on the real corpus, because the unit test asserts byte-stability on a toy index and only the real corpus proves it at scale. A stale pre-batch out/ producing the rebuild message rather than a parse failure. And then the line the whole arc was for: bare chops-search eval, no flags, scoring header reading the calibrated values with no asterisks on them.
Bare eval, no flags. Every value came out of index.bin; the 0.28 floor is derived from dims = 128, not set anywhere. 37 cases, 86% recall@1.
The four negative controls from the same run. Three return nothing: rows are warm in eval, so [kw] means the gate suppressed the semantic list and keyword matching had no evidence to offer. The fourth gets through in hybrid mode: no keyword evidence, but a top-median gap of 0.161 against min_gap 0.08, so the gate never fired. That is a real failure at the shipped configuration, and it belongs in the picture.
The last verification happens after deploy, and it is the one this whole series has been pointed at: type "toddler bedtime routine" into the live search box and watch it return nothing.
One open item surfaced while planning the next round of measurement, and it makes a fitting place to stop. No fixture case exercises the strong_cos hatch shape: keyword-empty, flat field, genuinely relevant top document. Which means a sweep of that knob is blind by construction until the case is written. The gap was found not by a failure but by asking what a sweep would even see, and the principle generalizes past search engines: a knob you cannot measure is a knob you cannot calibrate, and the time to notice is before the sweep, not after it returns a column of identical numbers you cannot interpret.
The engine, the config parser with its min_gp fixture, the scaffold tests, and the format reader are all in the repo: github.com/gitbadger-clan/chops-search. The series starts with a launch post about range-fetching an embedding model and runs through the eval that scored 65% and the knobs running out.
Top comments (0)