Rectify catalogue candidate precision and lookup work - #290
Merged
Merged
Conversation
Every failure across four document-import engagements was binding a document's entities to the target book's masters, never reading the document. Bridge was growing two answers to it: an MCP-private prefix matcher that named one near-miss candidate as `exact_live_spelling`, and a desktop screen that correctly ranks nothing but narrows nothing either. Add `bridge_tally_core::master_binding` as the single contract both surfaces consume, per ADR 0016. It matches an identifier embedded in a master name before the name itself, binds only where a rule is unique on both sides, and never resolves a near-miss: it reports candidates with the rule that surfaced each, and no score. An empty catalogue is a typed refusal rather than a report full of "missing". Deletes `master_match` and `master_key` from agent_import and moves unicode-normalization down a layer with them. The write gate is unchanged: build_import_xml and the approved-post recheck still admit byte-exact names only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…scale Self-review found a real defect: a catalog name was trimmed at the boundary, so a bound row reported a spelling the book does not contain. The write gate compares byte-exact against Tally's own name, so that would have refused with no explanation. Observed names are now retained verbatim; only source names are trimmed. Names differing solely in surrounding whitespace are an ambiguity, not a refused catalog. Adds a characterization suite over one fabricated 200-master book with the recorded naming pathologies. The assertion that matters is that no entity binds to a master a human would not have chosen; the counts are pinned underneath so loosening a threshold has to move a number. The mutation sweep was checked against two positive controls rather than trusted for passing: resolving a near-miss to its first candidate trips it, and binding a lone candidate does not. Both results are recorded in the test, so it is read as "no mutation reaches the wrong master" and not as "no rule change can loosen binding". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rzakhani-94af40 # Conflicts: # docs/tally/compatibility/compatibility-matrix.json # docs/tally/compatibility/compatibility-surface.json
The preparation screen listed every ledger in the company for every source entry, so the operator read the whole catalogue once per line. #288 already returns a per-entry binding; nothing drew it. Each entry's list now opens with the ledger binding matched, or with the candidates it could not choose between, followed by the complete catalogue under its own heading. The full list stays reachable in every case: this is a shortcut through it, never a filter on it. Nothing is selected for the operator. A near miss says plainly that nothing was chosen and reports a truncated candidate list truthfully, and a capture carrying no bindings renders exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8ad41d1d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
9 tasks
Ten review findings, read against the tree rather than taken at face value, and all reproduced. The severe one: bind_one selected Identifier before ExactName, so a ledger carrying a number, requested byte-exactly, returned match_state identifier while the write gate admits exact only. Every ledger with a phone or account number in its name was permanently unimportable — the exact population this contract was built for. When the two signals agree it now reports the byte-level fact. Also: neither side trims a name any more, so a trailing space cannot claim byte equality it does not have; digits inside a mixed code are no longer emitted as a standalone numeric; all admitted eight-digit date orders are excluded, not just year-first; more identifiers than the bound is refused rather than truncated, which could hide a conflict; the source-entity bound now covers what the source parser admits and the desktop states whether narrowing ran; desktop candidate bytes are capped in aggregate; a binding refusal keeps the catalogue evidence it already read; and master_binding.rs is sealed into the compatibility surface, which needed a deliberate one-file cap raise since it now decides admission outcomes. The larger finding came from running the binder over 470 real ledger names from all 16 synthetic books. Prefix matching offered a median of 40 candidates, 63% of the catalogue, and omitted the right master a third of the time: a truncated name reaches a whole family, and an alphabetically capped slice of DN Party 001..120 does not contain DN Party 057. A prefix matching more masters than the cap is now counted and deliberately not listed. Re-measured on the same names: where candidates are listed the right master is present in 403 of 403 rows, median list length 2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The remaining review finding, and the one I had triaged but not fixed. FallbackBinding::assign took an EntityBinding detached from its report plus any catalog, so a stock-item binding could be parked against a ledger catalog and the result carried no provenance for anything downstream to detect. Assignment is now a method on BindingReport taking an index into its own entities, so an entity from another report cannot be named at all, the catalog class is checked, and the binding carries its class forward. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A fix is a change and generates its own findings; the re-review of the previous commits raised four, all reproduced. An unusable ledger name in a parsed draft was skipped while the response still claimed a complete narrowing pass, so the rows that vanished were exactly the ones worth looking at; the pass is now reported unavailable. The reported candidate total took the larger of the suppressed family and the retained candidates, which under-reports when they are different masters; it is now their union. An identifier hint reached extraction without the bound applied to every other name. And the operator workflow in docs/agent/README.md still told readers to correct only near_miss rows, which now leaves a bound-but-not-exact row refused at the build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two further findings from the re-review, both reproduced. `Purchases FY25` and `Sales FY25` both yielded the code identifier `FY25`, so identifier-first matching bound the source to whichever existed before it ever compared the names. A fiscal-period label identifies a period, not a party, and is now excluded by shape; the minimum code length also rises from four to six, since a four-character mixed token is weak evidence of identity and the failure mode here is money against the wrong party. Re-measured against the same 470 live ledger names: no change to the distribution, so the tightening costs nothing observed. Candidate collection recounted every prefix from the start of the name, making it quadratic in a field the source parser lets reach 4 KiB. It now carries the character count forward in one pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
No book on the instance carried an embedded identifier: across 470 live ledger names from all 16 loaded companies, zero yielded a numeric identifier and exactly one a code identifier. The rule that separates this from fuzzy matching had no live coverage at all. Ten `MB ` ledgers now exist in BRIDGE CORPUS OPENING, parented to Suspense A/c so no receivable, payable or ageing measurement moves, and documented in TEST_CORPUS.md section 9 with the import method and the company-choice reasoning. BRIDGE PROBE B SANDBOX was rejected as the target despite the manufacturing precedent: it shares a GUID with a second loaded company and Bridge's own reads refuse it as company_identity_ambiguous. Within minutes the pair sharing one identifier exposed a defect no fabricated fixture had produced. A byte-exact request for a ledger whose embedded number is shared with another was refused as IdentifierConflict, making that ledger permanently unimportable, since the write gate admits exact only. Byte equality with an observed master name is now decisive: it names exactly one master, and an ambiguous identifier does not undermine it. Only a decisive identifier pointing elsewhere still outranks an exact name, and that stays a reported conflict. Re-measured over 485 live names, 2,330 cases: identifier binds 3 -> 11, every uppercase mutation now binds, and where candidates are listed the right master is present in 434 of 434 rows at a median list length of 2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two findings from the latest re-review, both reproduced. Enumerating the period shapes that must not become identifiers was a losing game: FY25 was fixed, then APR2025 and 2025Q1 were still binding two unrelated ledgers that merely share a period. The rule is now a shape — every run in the token is a short alphabetic marker or a number reading as a year or small ordinal, at most three runs — and a code identifier additionally needs eight alphanumerics, three digits and two letters. Requiring real length is the part that does not depend on having thought of every label. Measured against 485 live ledger names, exactly one yields a code identifier at all, and it still does. The aggregate candidate budget was applied to the consumer's copy, so the report's own clones were already allocated by then; capping the copy bounded only the copy. The budget now lives in bind() and is spent in entity order, and the desktop's second budget is deleted as redundant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The voucher-presence lane needs the same fold for voucher numbers and voucher-type names that master names use, and is exposing this function crate-wide to get it. That is the right call — a second, subtly different normaliser is the divergence ADR 0016 exists to end, and it would diverge silently, agreeing on every name tested by hand and differing on the punctuation nobody thinks to try. Records that obligation at the function, and its corollary: changing what this folds changes every consumer's notion of sameness at once. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…40' into claude/source-draft-narrow-targets # Conflicts: # docs/tally/compatibility/compatibility-matrix.json # docs/tally/compatibility/compatibility-surface.json
The narrowing copy predated the state the live measurement made necessary. For a source name that reaches a family of ledgers and tells none of them apart, the report now lists no candidates and reports the count — so the screen would have rendered "0 possible ledgers are listed first", a count of nothing, over the case that matters most. That state now says what it means: the line matches N existing ledgers and separates none of them, so none is listed; use a fuller source name or pick from the full list. No misleading "Possible" heading appears over an empty group, and the whole catalogue stays reachable as before. Also surfaces bindings_state: when the narrowing pass could not run, the toolbar says so rather than letting every row look merely unnarrowed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
Rebased onto
|
A client-supplied REMOTEID is not readable back — Tally overwrites the attribute with its own value (IMPLEMENTATION_GUIDE.md §3.3a, fourth property, verified). Nothing here uses it, but the doc on unresolved_identity said only that the identity is retained "for later reallocation" without naming the channel, and the obvious wrong choice fails silently: an amount parked with its identity in a write-only field is unreallocatable, and nothing about the write says so. Says narration, and says why. Surfaced by the voucher-presence lane, which had to correct three claims of its own after reading the reference's §9.3 heading as the general case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…facts candidates can be empty because nothing resembles the name, because a family resembles it and none is separable, or because the list was cut — and those mean opposite things to whoever decides what to do next. The disambiguators are reason, candidate_count and candidates_truncated, and reading the empty vector alone is wrong in two cases out of three. Recorded here rather than left to each consumer because it has already been got wrong twice by different lanes: the preparation screen rendered "0 possible ledgers are listed first" over a family of 120, and the voucher-presence contract needed a paired test to stop its rule collapsing into "no candidates means unknown". Same defect class this ADR was written against — a refusal whose neighbouring value reads as an answer. Also records why this is a doc and not a type. An enum of Listed / Truncated / Withheld / None is the stronger fix and the one P2 asks for, but it is breaking, a stacked consumer already depends on candidates_truncated as a predicate and holds the boundary with tests, and forcing that rework mid-review trades an improvement for a regression risk. Revisit once both have merged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…isting Five items in one round, because review reopens once either way and the marginal cost of the rest once it is reopened is small. Tally's master-name matching is measured, not guessable: IMPLEMENTATION_GUIDE.md §3.3b found it case-insensitive AND separator-insensitive — a hyphen matches a space — and otherwise exact. The binder was stricter, which is not the safe direction it looks like: it refused names Tally accepts, and `X - Y` is a common ledger convention. A separate master_identity_key follows §3.3b and stops where Tally stops; `AND` for `&`, a missing suffix word and a singular for a plural still refuse. It is separate from comparison_key rather than a widening of it, because that one is shared with voucher numbers and voucher-type names and §3.3b says nothing about those. Measured live: 16 of 16 hyphenated masters now bind from the spelling Tally itself accepts, where all 16 were near-misses before. Candidates becomes None | Listed | Truncated | Withheld. An empty vector was three different facts and a consumer reading is_empty() was wrong in two of them, a shape already got wrong twice by different lanes. Taken before merge because the contract has not shipped and this is the cheapest it will ever be; the consumer who pays for it measured thirty lines and reported the change improves its code. The MCP result gains an explicit listing discriminator, since a model is the caller that would read an empty array as "no such ledger exists"; the desktop DTO stays flat, where the screen already distinguishes the cases and is tested. Also: FallbackBinding says reallocate with a Journal and never Alter or Cancel, which §9.7 measured as duplicating with the target untouched while reporting success; the ADR records that identifier coverage is bimodal by client (42%, 0%, 0%, 0%) so the rule is a first-pass check and never a primary key; and BindingStatus says what a Bound does not establish — not that the master still exists, not that the requested name may be written, not that it is right in business terms, and no authority at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three were the same shape recurring: an identifier built from something that identifies a period rather than a party. A token carrying letters now never yields a standalone numeric, whether or not it qualified as a code — `Part A12345678` was reaching an unrelated `Bank 12345678` through the one-letter gap the code test rejects. Period labels are recognised by their numbers rather than their words, which catches `SEPTEMBER2025` and `2025QUARTER1` that no cap on the alphabetic run ever would: a month name can be any length, a year cannot. And a fiscal range is excluded before its digits are fused, since `2025-2026` strips to an eight-digit run no calendar reading rejects. Fallback assignment now checks catalog provenance, not just class: two ledger catalogs are both Ledger, and a fallback drawn from the one the report never saw names a master that was never a candidate. Candidate collection selects by index and clones only what it retains, instead of cloning every match before the cap and the budget discard most of it. ADR 0016 quoted thresholds this module stopped using two rounds ago, and it is the contract two surfaces integrate against. Synced — and a test now reads the ADR and asserts it quotes the live constants, so the next drift fails rather than waiting to be noticed. Verified against a positive control: changing a constant without the document fails it. It also caught a false positive of its own on first run, which was the detector being too strict about `(10%)` rather than the ADR being wrong. Live re-measure over 485 names, 2,330 cases: 434 of 434 listed rows still contain the right master, median listed length 2, no wrong binds. Identifier binds 11 -> 8 with bound and unbound totals unchanged: three mutations that had bound to themselves through a leaked numeric now bind by name instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ializable Both reproduced by execution before fixing, and both re-verified against the same reproduction after. A token shared across a name family IS the family. Where the catalog is large enough that the token stays under the common-token threshold — 30 rows among 330 is 9% — the shared-token pass re-offered exactly the rows the prefix pass had withheld, restoring the arbitrary capped slice that withholding exists to prevent. Measured: the case reported near_miss with 25 of 30 listed and the intended master outside the slice; it now reports no_discriminating_candidate with the family counted and none listed. Weaker rules now skip the withheld set; decisive rules still reach a family member on its own evidence, because the identifier or the whole key is what separates that one from its siblings. Candidates::Listed was a newtype variant carrying a sequence, and under Serde's internally tagged representation a tag cannot be merged into a sequence — so serializing the MOST COMMON unresolved result failed at runtime while the other three variants succeeded. Now a struct variant. No test caught it because none had ever serialized an Unresolved, only a Bound; every variant now round-trips in a test. Live re-measure unchanged: 434 of 434 listed rows contain the right master, median listed length 2, no wrong binds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…40' into claude/source-draft-narrow-targets # Conflicts: # docs/tally/compatibility/compatibility-matrix.json # docs/tally/compatibility/compatibility-surface.json
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17c3384e58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…k a name Both reproduced by execution before fixing, both re-verified after, and both fixes are smaller than what they replace. `FY2025-26` canonicalizes to `FY202526`, whose six-digit run reads as no period at all, so the label became a Code identifier and `Purchases FY2025-26` bound to a sole `Sales FY2025-26`. The period test now runs on the raw token and splits on the separators operators actually write, so `FY2025` and `26` stay legible as what they are. That subsumes is_year_range, which is deleted: one period test where there were two, covering ranges the numeric path caught and the code path did not. Two identifier hints selecting two other masters, with the source name byte-matching a third, bound the name and silently discarded the conflict. A byte-exact name survives an identifier that is merely shared — the ambiguous set still contains the master the name spells — but not identifiers that all point elsewhere. The predicate is now that one sentence rather than three conditions, and the report offers every master the evidence reached, so the operator sees the disagreement rather than one side of it. Live re-measure unchanged: 434 of 434 listed rows contain the right master, median listed length 2, no wrong binds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two wrong binds, both reproduced before fixing and both re-verified after, and both the same class one level down: an identifier built from something that is not identity. The letter guard was ASCII-only, so `पार्टी12345678` read as digits standing alone and bound a party to an unrelated `Bank 12345678`. The observed books carry Devanagari, Tamil and Bengali ledger names, so this was reachable on the corpus this PR already reads. The guard is now Unicode alphabetic. `XXXXX1234X` cleared every length and composition test — ten characters, six letters, four digits, no period — while carrying only a last four that any number of parties share, so two unrelated ledgers with the same mask bound to each other. A token whose letters are a single repeated character is a mask; an identity-bearing code has distinct letters. TEST_CORPUS.md §9 recorded live counts without the confidence marker AGENTS.md requires, so the seeding, the coverage counts, the rule's live behaviour and the general safety claim are now separated into VERIFIED, VERIFIED, PARTIAL and UNVERIFIED with the scope of each. The strongest claim in that section was never the one a reader would have taken from it. Live re-measure unchanged: 434 of 434 listed rows contain the right master, no wrong binds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Third finding on the same predicate, so the fix is the structural one rather than a fourth condition. The predicate asked whether the *union* of matched masters contains the byte-exact one. That answers the shared case correctly — one number on two masters, where the name separates them — and the mixed case wrongly: `ACME 11111111` with a hint reaching `BETA 22222222` has the exact master in the union because its own number is one of the identifiers, while a second identifier plainly disagrees. Flattening identifier-to-master provenance into one set discarded the only fact that separates those two, and they need opposite answers. The match is now kept per identifier, and a byte-exact name is outranked when any single identifier reached somewhere else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…40' into claude/source-draft-narrow-targets # Conflicts: # docs/tally/compatibility/compatibility-matrix.json # docs/tally/compatibility/compatibility-surface.json
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfe3b974cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`is_masked` guards the code branch, where a mask spelled with letters is caught by the letter test anyway. A mask spelled with punctuation never reaches it: in the numeric branch every non-digit is an ordinary delimiter, so `********12345678` split cleanly and offered its visible suffix as though it were the whole account, binding two unrelated ledgers that share it. A value written with mask punctuation is partial by construction, so what it exposes is a suffix and not the number. Ordinary punctuation around a whole number is untouched — `(5550001001)` and `5550001-002` still bind — because a fix that rejected all punctuation would have been quietly worse than the bug. Fourth finding in the same family: an identifier built from something that is not identity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
added 3 commits
September 12, 2026 19:25
…odex/rectify-pr290-325-20260912
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b55defcee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce0fd7564a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…reaching something
The precision predicate compared the search result against zero. `masters_found`
counts everything `collect_candidates` found, and the materialized identifier
matches are among them — they are offered as candidates — so a nonzero result
does not mean the name reached anything. Those matches are *already* unioned
into the withheld count exactly, by testing each against the skipped family.
So one family skipped beside a second identifier reaching one distinct master
reported "at least 31" while `a_withheld_family_counts_the_masters_the_other_identifier_listed_too`
asserted, two lines earlier, that the total is exactly `family + 1`. A test
claiming the number and a predicate denying it, in the same file.
The comparison is now against what has already been accounted for:
skipped_families > 1 || (skipped_families == 1 && searched > materialized)
with the missing positive control added: one skipped family beside masters the
**name** reached by token, where the overlap genuinely is unmeasured. Without
it the clause could have been deleted silently.
Three mutation controls, each caught by a different test — comparing against
zero fails the mixed case, dropping the clause fails the by-name case, and
`skipped_families > 0` fails both.
Third round on this predicate, each narrowing it, and the through-line is the
one this PR keeps finding: every correction made the number better and left the
claim about it wider than the new truth.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfe5a9a332
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…d the listing has three names **Two identifiers on the identical holder set are one family.** A duplicated party carrying both a registration code and a phone number on every one of its ledgers produced two skipped families by count and one by content, so a union that was known got hedged. Skipped families are deduplicated by **content** now, not by the identifier that reached them: two identifiers are the same family when they name the same masters, however they were spelled. Mutation-checked — counting occurrences fails the twinned case. **The ADR named one boundary's field as though it were universal.** The table was headed `candidate_listing`, which is the desktop DTO's name; the MCP result emits the same four words as `listing`, and the core reaches them through `Candidates::listing()`. An MCP consumer following the contract would have gone looking for a field that does not exist. The table is keyed on the state itself now, with the three names given above it — and the irony is recorded because it is the lesson: the paragraph explaining that the state is named differently at each boundary was written directly above a table that used one of those names unqualified. Also corrects a stale claim two sections down that the desktop DTO "stays flat". It was flat until this PR, and the defence offered for flattening — "a projection with a tested consumer" — is now recorded as wrong in an instructive way: the tests were real and tested the wrong thing, because the DTO could not express the distinction they would have had to make. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
The orchestrated rectification lane is integrating the parallel review fixes and retaining both histories through normal merges. The remote head has changed twice during the final local review. Please leave this PR branch stable for reconciliation and current-commit validation; do not force-push or merge it during this pass. The final candidate must retain the binding-key cache isolation, bounded lookup work and explicit count precision together. Publication from this lane uses a fresh head check and a normal fast-forward only. |
…odex/rectify-pr290-325-20260912 # Conflicts: # docs/adr/0016-master-binding-authority.md # docs/tally/compatibility/compatibility-matrix.json # docs/tally/compatibility/compatibility-surface.json # src-tauri/crates/bridge-tally-core/src/master_binding.rs # src-tauri/crates/bridge-tally-core/src/master_binding_tests.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96f5666133
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
added 3 commits
September 12, 2026 19:58
…odex/rectify-pr290-325-20260912 # Conflicts: # docs/tally/compatibility/compatibility-matrix.json # docs/tally/compatibility/compatibility-surface.json # src-tauri/crates/bridge-tally-core/src/master_binding.rs # src-tauri/crates/bridge-tally-core/src/master_binding_tests.rs
…odex/rectify-pr290-325-20260912 # Conflicts: # docs/tally/compatibility/compatibility-matrix.json # docs/tally/compatibility/compatibility-surface.json
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b64495db27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b81057eabc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 task
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0267c45bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b7d916db1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
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
1 participant
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.
Closes #323. Closes #324. Closes #325.
Functional summary
Source-draft choices show deterministic bindings or unresolved candidates first, then the complete catalogue, while retaining explicit operator selection and identifier-conflict explanations. Identifier-conflict guidance requires the complete observed catalogue and confirmation of intended identity even when a truncated list shows an outside candidate while withholding the conflicting family. A fuller name alone cannot resolve an identifier conflict. Count precision is independent of listing state: the UI says “at least N” when the matcher has only established a lower bound, including unlisted or unproven overlapping families.
The matcher uses full named memo keys, including the resolving key and holder set. Exact holder-family identity is computed once during catalogue construction. Per-entity nested-family proof spends at most 256 membership probes; a completed subset proof gives an exact union count, while exhaustion retains the lower-bound flag. Listed candidates outside the largest withheld family are included without double-counting its members. Equal large families do not require repeated vector comparisons. Existing admission, scalar-work and memo-storage limits remain.
Test or reproduction command
Candidate
5dab44e54de8f7066afaea901035299a61be54e9, based on master9da3b7205a79c30eaf901e52618155e0860c55a6, preserves both parallel histories through normal merges.cargo test --locked --manifest-path src-tauri/Cargo.toml -p bridge-tally-core --lib: 147 passed, including nested/exhausted proof, disjoint families, exact union and memo isolation cases.cargo clippy --locked --manifest-path src-tauri/Cargo.toml -p bridge-tally-core --all-targets --all-features -- -D warnings: passed.Reuse/deletion: reuses the existing matcher, catalogue capture and assignment control. Replaces repeated deep comparisons with catalogue family IDs and a bounded proof; removes the tuple-index cache hazard. No new dependency or transport. Without the change, withheld counts can look exact and repeated large families can consume excessive lookup work.
Migration compatibility
Additive response precision metadata; no database or saved-draft migration. Old captures retain the full catalogue fallback.
Rollback notes
Revert to restore previous display/work allocation. Browser-engine coverage is now supplied by merged #327; packaged native accessibility, Windows interactive behavior and new live Tally qualification are not claimed here.
Security impact: candidates remain non-authoritative and the operator chooses explicitly. No credential, endpoint, customer fixture, financial mutation or company admission change. The separate folded-name scope finding is tracked by #314 and is not claimed fixed by this count/candidate PR.
The constructor builds family IDs only for holder sets larger than the candidate cap. Ordinary singleton identifiers remain outside sorting and family-ID allocation. ADR 0016 distinguishes containment proven within the membership budget from unproven overlap.
The candidate-listing discriminator is a four-value TypeScript union. Empty listings are handled exhaustively: withheld families and report truncation retain distinct guidance; inconsistent or unknown runtime states show unavailable detail without claiming budget exhaustion. Final screen suite: 58 passed; frontend production build passed. Rust and desktop source are unchanged by this UI follow-up, so their earlier core/Clippy/desktop receipts remain relevant. The final surface was resealed and all 212 actual-byte pins verified.