Deferred from #288 review round 9 (P2, no P1 in that round). Not fixed there, under the severity bar agreed for that PR: a wrong bind blocks, a stall or a weak refusal report is filed.
What happens
bind pre-counts which source keys repeat, so the candidate memo holds only results a second row will ask for again. The count is on (entity.key, entity.identifiers) — the source-side determinant — while remembered_candidates keys the memo on (entity.key, identifier_matches), the masters those identifiers actually reached.
Those agree in one direction and not the other. Identical key and identical identifiers always produce the same memo key, so nothing is counted as repeated that is not. The converse fails: many distinct identifier sets can reach the same masters, and the commonest case is the one where they reach none.
So 40,000 entities all named Acme Branch, each carrying a different numeric hint that matches no catalogue identifier, are 40,000 distinct pre-count keys and one single memo key ("acme branch", {}). None is counted as repeated, nothing is ever held, and the candidate search re-runs once per row. Against a large Acme Branch … prefix family that search is the expensive one.
The code already concedes this in a comment above the pre-count ("The converse does not hold — two different identifier sets can reach the same masters — so this counts no pair as repeated that is not, and at worst declines to cache one that is"). The comment is accurate; "at worst" is doing more work than it looks.
Why the pre-count exists at all
It replaced a first-come cap, under which 1,024 distinct cheap misses at the head of a draft filled the memo and the repeated expensive key behind them was never cached — the same stall, reachable by reordering the same rows. Any fix has to keep that property: the memo must not fill with entries nothing asks for twice.
Suggested fix
Admit on the second encounter of the computed key rather than pre-counting a proxy for it. A small set of key hashes seen once; on the second sighting of a hash, insert the result into the memo. That is correct in both directions because it is keyed on the thing the memo is keyed on, and it lets the pre-count and SearchMemo::repeated be deleted rather than corrected — the borrow lifetime on SearchMemo<'a> goes with them.
Hashes rather than owned keys because the set is O(distinct keys) and an owned (String, BTreeSet<usize>) per entry is what the borrowing pre-count was avoiding. A hash collision costs one unnecessary cache insert, which is harmless.
What would close this
A test in master_binding_tests.rs of the shape of hint_variants_of_one_name_do_not_crowd_out_a_key_that_repeats, but with hints that match no catalogue identifier, asserting CANDIDATE_SEARCHES stays at 1 across many rows. Mutation control: the current pre-count fails it.
Source: src-tauri/crates/bridge-tally-core/src/master_binding.rs, bind and remembered_candidates.
Deferred from #288 review round 9 (P2, no P1 in that round). Not fixed there, under the severity bar agreed for that PR: a wrong bind blocks, a stall or a weak refusal report is filed.
What happens
bindpre-counts which source keys repeat, so the candidate memo holds only results a second row will ask for again. The count is on(entity.key, entity.identifiers)— the source-side determinant — whileremembered_candidateskeys the memo on(entity.key, identifier_matches), the masters those identifiers actually reached.Those agree in one direction and not the other. Identical key and identical identifiers always produce the same memo key, so nothing is counted as repeated that is not. The converse fails: many distinct identifier sets can reach the same masters, and the commonest case is the one where they reach none.
So 40,000 entities all named
Acme Branch, each carrying a different numeric hint that matches no catalogue identifier, are 40,000 distinct pre-count keys and one single memo key("acme branch", {}). None is counted as repeated, nothing is ever held, and the candidate search re-runs once per row. Against a largeAcme Branch …prefix family that search is the expensive one.The code already concedes this in a comment above the pre-count ("The converse does not hold — two different identifier sets can reach the same masters — so this counts no pair as repeated that is not, and at worst declines to cache one that is"). The comment is accurate; "at worst" is doing more work than it looks.
Why the pre-count exists at all
It replaced a first-come cap, under which 1,024 distinct cheap misses at the head of a draft filled the memo and the repeated expensive key behind them was never cached — the same stall, reachable by reordering the same rows. Any fix has to keep that property: the memo must not fill with entries nothing asks for twice.
Suggested fix
Admit on the second encounter of the computed key rather than pre-counting a proxy for it. A small set of key hashes seen once; on the second sighting of a hash, insert the result into the memo. That is correct in both directions because it is keyed on the thing the memo is keyed on, and it lets the pre-count and
SearchMemo::repeatedbe deleted rather than corrected — the borrow lifetime onSearchMemo<'a>goes with them.Hashes rather than owned keys because the set is O(distinct keys) and an owned
(String, BTreeSet<usize>)per entry is what the borrowing pre-count was avoiding. A hash collision costs one unnecessary cache insert, which is harmless.What would close this
A test in
master_binding_tests.rsof the shape ofhint_variants_of_one_name_do_not_crowd_out_a_key_that_repeats, but with hints that match no catalogue identifier, assertingCANDIDATE_SEARCHESstays at 1 across many rows. Mutation control: the current pre-count fails it.Source:
src-tauri/crates/bridge-tally-core/src/master_binding.rs,bindandremembered_candidates.