A week ago I published a measurement: I had generated descriptions for 1,245 database objects to make retrieval better, and retrieval got worse. The post did well by my standards. Thirty-seven comments, several from people who had seen the same thing in their own systems.
The measurement was real. The conclusion I drew from it was wrong, and this week I found out why. It was my code.
What the numbers said
Spider 2.0-lite is a text-to-SQL benchmark built from real BigQuery and Snowflake databases, and unlike the older benchmarks it ships the warehouses' own documentation. That makes it the one place I could test the claim on somebody else's prose rather than my generator's.
So I deleted the descriptions and re-ran. Removing them won at every cut, on both embedders, five of six cells significant, and the widest cell was thirteen questions fixed against one broken.
Read on its own, that table says delete your descriptions. Which should have been the tell. A design whose whole premise is that prose helps should not lose to deleting the prose. The question was not whether to believe the table. It was what the table was pointing at.
What it was pointing at
The same column comment was being indexed three times.
It went into _prose_text, which feeds the prose channel. It also went into embed_text() — which feeds the body channel and the vectors. Three of the four ranking signals, all carrying the same words.
A Spider 2.0 table carries a description per column. So any table with many columns matched on three of four channels for any question that shared a single word with any one of its columns. The widest tables became magnets. They crowded out the narrow, correct table on question after question.
Deleting the descriptions "helped" because it removed two thirds of a triple-count. It was never evidence about prose. It was evidence about my scoring.
Decomposing it
The useful thing about a triple-count is that you can take it apart one channel at a time. Same 212 questions, one source removed per run:
- remove column comments from the prose channel alone → recovers 4 questions at k=10
- remove them from
embed_text()alone → recovers 11 questions at k=10
Eleven of the fifteen were in the embedding path. That is where the damage was, and it is the one I would not have guessed — the prose channel was the obvious suspect because it is the one named after prose.
The fix, and the constraint that shaped it
Take column comments out of the body channel and the prose channel. Leave embed_text() alone.
That last part is not laziness. The vectors are a published, pinned guarantee: change what goes into them and every stored index built by anyone using the library silently becomes wrong. A retrieval fix that invalidates your users' indexes is not a fix, it is a migration you didn't announce. So the damage gets removed from the two channels that can change freely, and the embedding path — where most of the effect lived — is left byte-identical.
It works anyway. Re-run paired, same 212 questions, with the fix in:
k with prose without prose b c p
5 143/212 67.5% 143/212 67.5% 6 6 1.0000
10 177/212 83.5% 177/212 83.5% 2 2 1.0000
20 188/212 88.7% 187/212 88.2% 2 3 1.0000
Thirteen-to-one became two-to-two. The penalty is gone. Descriptions now neither help nor hurt on this benchmark — which is the floor a fielded design was supposed to guarantee and mine did not.
A second bug, found on the way
While sampling the raw files I found that description in Spider 2.0's schema JSON is not a table description at all. It is a per-column list, aligned by index to nested_column_names when a table has nested fields and to column_names otherwise, entries sometimes null and never a string — 150 of 150 sampled. My loader had been joining that list into a paragraph and indexing it as a table description.
The same loader built columns from column_names alone, so a nested table — gnomAD's v3_genomes__chr7, 61 top-level columns and 181 flattened — arrived missing the very columns its gold SQL reads.
Fixing that moved the resolvable comparison set from 203 questions to 212, and k=20 from 67.2% to 71.3% over the 247-question denominator. It also moved k=10 down, 77.8% to 77.4%, because the nine questions that joined are harder than the average of the 203. Both directions recorded, because quoting only the cut that rose is how you end up publishing the last thing I published.
The format question is filed upstream as a documentation request rather than a data defect, because the files are consistent — the field name just doesn't mean what it looks like: Spider2 issue #222
What I'd take from this
A measurement can be completely true and still not mean what you think. Mine was reproducible, it replicated across embedders, and the direction was stable. All of that was real. None of it made the conclusion right.
If a result argues against your design's core premise, suspect the implementation before the premise. Not because premises are always right, but because you can check the implementation in an afternoon and the premise takes a research programme.
Watch for one field reaching more than one ranking signal. This is the generic version and it is easy to do accidentally: a field that goes into a lexical channel and also into the text you embed is counted twice, and if a third channel derives from the same text, three times. Nothing errors. Your tests pass. Retrieval just quietly prefers whatever documents have the most of that field.
The fix should not invalidate your users' indexes. If it has to, that is a version bump and a note, not a patch release.
Shipped in 0.1.57. The full McNemar grid, the channel decomposition and the re-measured numbers are in BENCHMARKS.md in the repo, along with the withdrawn claim, which stays visible rather than deleted.
Top comments (22)
"A design whose whole premise is that prose helps should not lose to deleting the prose" was the right instinct to trust, and it generalises: a result that is suspiciously clean in the direction of the simplest possible action is usually measuring your harness rather than your system.
The publish-then-correct sequence is the genuinely useful part here. Most people who find the bug after the numbers went around just quietly stop mentioning the topic.
One thing worth adding to the postmortem: duplicate counting is cheap to catch as a hard assertion inside the scorer, not something you eyeball. Check that the retrieved id set contains no repeats and that its size equals k. It costs nothing and it fails loudly the first time a join or a chunk expansion fans out.
"A result that is suspiciously clean in the direction of the simplest possible action is usually measuring your harness rather than your system" is the sentence I want to steal. That is exactly the shape it had: deleting every description won at every cut, on both embedders, five of six cells significant. Nothing in a real system is that tidy.
On the duplicate assertion — it is there, and as a property test rather than an eyeball, which I should have said in the post.
tests/test_any_schema.py, Hypothesis generating random catalogues:plus a separate one over the foreign-key expansion path, which is the fan-out you are describing.
But your framing still lands, because that is the harness asserting it over generated catalogues, not the scorer asserting it on a live run. Those are different guarantees, and the one I have is the weaker of the two. Moving it down into the scorer costs nothing and covers the case where a join or a chunk expansion fans out in production rather than in CI.
Worth adding that the duplicate-id check would not have caught this particular bug, which is the uncomfortable part. The ids were unique. One field was feeding three scoring channels, so the same table won on three arms at once without ever appearing twice in the result. Another reader on the sibling post suggested channel-disjoint field accounting — log which source fields feed which arm — and that is the assertion that would have failed. Both are cheap; they catch different things.
On publishing the correction: the numbers had already gone out. Leaving them standing was the only option that was definitely wrong.
Thanks for distinguishing the unique-result property from the scoring-channel bug. You are right that the duplicate-id assertion would not have detected the same field contributing through three arms, so I should not present it as coverage for this failure.
The source-field accounting sounds like the more direct invariant. A regression fixture with unique table IDs but intentionally overlapping channel inputs would make that distinction visible: result uniqueness should pass, while the channel-disjointness check should fail. Keeping both outcomes in the correction would show why adding a reasonable assertion is insufficient unless it targets the mechanism that actually inflated the score.
Agreed. That's the right split. The source-field accounting invariant exists now: test_body_channel.py builds the index and checks exactly what text each channel holds, so a field showing up in a second channel fails even when every result id is unique. Your fixture idea, unique ids with deliberately overlapping channel inputs, is the clean way to show why the uniqueness check alone was not enough.
The accounting invariant catches a different failure from duplicate result IDs, so test_body_channel.py is the right place to make that distinction executable. I would add a positive control for intentionally repeated literal text across two different source fields.
That keeps the assertion about provenance assignment rather than text equality: identical wording can legitimately occur in a title and a description, while one source field appearing in two scoring channels is the bug. The deliberately overlapping-input fixture should fail for the second reason, even when both fixtures return unique IDs.
Agreed, and that's a sharper statement of what the test is for. The bug was never "the same words appear twice"; it was one source field feeding two scoring channels. A fixture where a title and a description legitimately share a sentence should still score each field once, and the overlapping-channel fixture should fail for the channel reason, not the text reason. I'll add that positive control next to the existing check in
test_body_channel.py.A Hypothesis property test generating random catalogues is stronger than the assertion I suggested, since it exercises shapes nobody would think to write by hand. Worth putting in the post, because "we check for duplicates" and "we check for duplicates under randomly generated catalogues" are read very differently by anyone deciding how much to trust the numbers.
The bound I would keep an eye on as this grows is len(sel) <= max(top_k, 0). An upper bound catches over-retrieval, but under-retrieval passes it happily: the scorer silently returning fewer than k because an earlier stage dropped candidates looks like a hard query rather than a bug, and it biases every metric computed at k.
You're right, and it's the more dangerous half. An upper bound fails loudly -- you get more than you asked for and somebody notices. Under-retrieval is silent, and it does not just lose a result, it corrupts the denominator: recall@k computed over a set that quietly held k-3 items reads as a hard question rather than a broken stage. Same shape as both measurement errors I had to correct in BENCHMARKS.md -- each moved a number in the flattering direction and neither raised anything.
So I measured before asserting, because I was not sure the floor was even true. On 0.1.58, generated catalogues of 1 to 80 objects -- reserved words, quotes, emoji, Cyrillic, 128-character names, empty questions, questions sharing no vocabulary with the corpus,
top_kfrom 0 to 100 -- 12,000 checks, zero cases where the selection came back short. So it holds, and the assertion is one line next to the existing ceiling:One thing to be careful about if anyone copies this: the floor is the eligible count, not the catalogue size. Identity filtering legitimately shrinks supply, so a restricted caller returning fewer objects is the feature working rather than a bug. The test above passes no principal, which is why
len(cat._docs)is right there and would be wrong in the scorer.The 12,000 checks are useful evidence for that implementation, and the eligible-count qualification fixes an omission in my earlier suggestion. A floor based on the whole catalogue would incorrectly flag a correctly restricted caller.
The next property I would add is principal variation on the same catalogue: vary the permitted subset, including zero eligible objects, and check both selection membership and the floor against that subset. If the scorer later introduces a relevance threshold, its rejection policy also needs to be represented explicitly in the expected supply.
Agreed. Same catalogue, different callers, is the case that matters, and the caller who can see nothing at all is the edge I haven't tested yet. That goes in as the next test, checking both what gets selected and the floor against what that caller is allowed to see.
That zero-visible-records case is a useful boundary because the algorithm should have no candidate from which to satisfy a positive floor. I would check both the empty selected set and how the floor is reported, so an unreachable target does not become an invitation to include an ineligible record.
Pairing that caller with one who can see a small subset of the same catalogue would also show that the stopping calculation changes with visibility. The assertions can then be expressed entirely against each caller's eligible set, without needing a universal catalogue-wide threshold.
Added, as you described: github.com/ashishsinha1602/schemag...
A caller with nothing eligible gets an empty selection. No object names and no business-term lines reach the prompt, and top_k isn't filled from outside their set. A caller who can see two tables gets at most those two, even with top_k=10. A third test runs the same question for a one-table caller and a full-access caller, so the stopping point is checked against each caller's own eligible set, as you suggested. With visibility switched off on purpose, all of them fail.
The deliberately disabled visibility check makes those empty and underfilled cases convincing regression controls. I would also assert that the empty eligible set makes no embedding or reranking calls when those services receive catalog text.
An empty final selection proves the prompt stayed clean, but an earlier remote scoring call could still disclose object names before the filter returns nothing. A recording fake for each scoring dependency would let the zero-visible-records fixture cover that intermediate boundary as well as the final prompt.
Good boundary to name. Here is what happens today when nothing is visible to the caller: the question itself is still embedded (that call carries the question text only), the visibility filter runs before anything is ranked, and the reranker is only ever handed allowed candidates, so with an empty eligible set it has nothing to send. No catalog text leaves through either service.
That is a claim about code paths, though, not a test. A recording fake for the embedder and the reranker, asserting no catalog names or comments ever reach them for a zero-visible caller, would turn it into one. Adding it.
All three are in now: github.com/ashishsinha1602/schemag...
test_body_channel.py(same sentence as a description and a column comment; kept by field, not by text)With the visibility filter forced off, all three timing tests fail, so they test something real. Thanks for the suggestions.
The recording fakes close the gap between an empty final selection and an empty outbound candidate payload. Including an allowed-candidate case that proves the reranker actually ran also protects against a test passing because the integration was accidentally disabled.
For the reused-catalog checks, I would keep the role-list and database-GRANT paths named separately in the report. They have different refresh guarantees, so the same revoke fixture should make that boundary visible rather than implying both paths enforce changes immediately.
Agreed. The test I added covers only the role-list path, and its name should say so, so nobody reads it as "revokes take effect immediately" in general. The GRANT-snapshot path gets its own test with its own claim, which is the one in your other comment: a bounded window, not the next call.
Done: the test is now
test_role_list_revoke_on_a_reused_catalog_takes_effect_on_the_next_call, and its module says it covers the role-list path only and points to the GRANT-snapshot tests, whose claim is the max-age window. So neither one reads as covering the other.That test name now states the guarantee precisely, and the pointer to the GRANT-snapshot tests prevents the next-call behavior from being generalized to a different source of permissions. I would preserve that distinction in the test report labels too, since readers may never open the module.
A future change that introduces caching into the role-list path should then break this named contract directly. That makes a performance optimization visibly require a security-contract decision rather than silently weakening what the current test promises.
The name is what the report shows: pytest prints it on every failure and in -v output, so a reader who never opens the module still sees the role-list claim and not a general one.
And yes, that's the intent with caching. Anyone who adds a cache to the role-list path turns this test red under a name that says which promise they broke, so it becomes a decision about the security contract rather than a quiet speed-up.
The publish-then-correct honesty is rare — and the generic failure mode (“one field feeding three ranking signals”) is easy to ship with all tests green.
Two cheap asserts I’d put in front of any hybrid bake-off before arguing BM25 vs dense vs fusion: (1) unique result ids in the top-k (size == k, no dupes), (2) channel-disjoint field accounting — log which source fields feed lexical vs dense vs “prose” arms so the same comment can’t silently inflate three scores. Otherwise Recall@k compares different machines and “delete the prose” can look like a product insight when it’s a harness bug.
Hybrid stays a workload claim; channel integrity is what makes that claim measurable.
Both of these are good, and one of them I can tell you the status of because a reader found the exact failure yesterday.
(1) unique result ids, size <= k. This one exists, and as a property test rather than a fixture —
tests/test_any_schema.py, Hypothesis generating random catalogues:with a second one covering the FK-expansion path, which is where a fan-out would actually show up. Your framing is still the part worth taking though: that is the harness asserting it over generated catalogues, not the scorer asserting it on a live run. Cheap to move down a level.
(2) channel-disjoint field accounting. This does not exist, and you have described the bug precisely. The whole finding in the post was one column comment reaching three of four channels. The fix was to take comments out of the body channel — but the way it was implemented,
_body_textis a hand-copy of the vector builderembed_textwith one line removed, and nothing asserts the relationship. Another reader (howcani, in the other post's thread) noticed yesterday that_body_texthas no test on it at all whileembed_textis asserted on in four files. So the next field added to the vector builder silently misses the body channel, and the symptom is a recall number moving in whatever experiment is running.Which is your point, arriving from the other direction. Accounting that says this field feeds these arms would make it structurally impossible rather than a matter of two functions agreeing. That is going in.
"Recall@k compares different machines" is the sentence I should have had in the postmortem.