Decide which proposed vouchers are already in the book - #294
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
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>
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. |
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>
lamemustafa
force-pushed
the
claude/voucher-presence
branch
from
September 10, 2026 19:14
8ded397 to
7bc66ad
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ded39786b
ℹ️ 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".
lamemustafa
force-pushed
the
claude/voucher-presence
branch
from
September 10, 2026 19:20
7bc66ad to
6294652
Compare
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>
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c56a03765
ℹ️ 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".
lamemustafa
force-pushed
the
claude/voucher-presence
branch
2 times, most recently
from
September 10, 2026 20:06
e4004bd to
bff99de
Compare
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>
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a192c875b
ℹ️ 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".
…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>
lamemustafa
force-pushed
the
claude/voucher-presence
branch
from
September 10, 2026 20:27
3a192c8 to
f5bd780
Compare
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbe2934151
ℹ️ 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".
…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>
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf421524d1
ℹ️ 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".
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>
left a comment
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39d6da2de5
ℹ️ 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".
…397) `BookWindow::observed` refused a `WindowRead::Partial` read outright (`PresenceError::WindowIncomplete`). That placement was right about the hazard and wrong about its blast radius: only `Absent` ever needed a complete window, but a construction-time refusal withheld `Present` and `PossiblyPresent` too. Since the adapter has no source-side control total for a nonempty window, every nonempty window is `Partial` — so the tool answered nothing at all over any range that held vouchers. Move the gate to where `Absent` is produced. `BookWindow` now retains its `WindowRead` and exposes `read()`; both states construct. `decide` checks it at the sole `PresenceStatus::Absent` site, last among the withholdings, and degrades a `Partial` window's would-be `Absent` to `PossiblyPresent(UndecidedReason::WindowNotProvenComplete)`. `Present` and `PossiblyPresent` are produced from a `Partial` window exactly as before. `agent_presence.rs` continues past a nonempty window instead of refusing early, and reports the window's real `read` state rather than a hardcoded "complete". `agent_catalog.rs`'s `voucher_presence` description states the new contract. That description names the reason a caller will actually read, which is not the one `safe_reason_code` returns. `UndecidedReason` serialises through `rename_all = "snake_case"`, so an item's `reason` field carries `window_not_proven_complete`; `presence_window_not_proven_complete` is the error-path spelling and appears nowhere in a successful response. The description advertises the serde form, and the schema test now asserts the anchored string and rejects the prefixed one, so the two cannot drift apart again without failing. Tests pin the invariant the type system used to buy: a `Partial` window withholds `absent` when nothing resembles a proposal, and the identical contents read `Complete` still issue it — the pair, so the first cannot pass because `Absent` broke generally. ADR 0017 is updated to match: the completeness gate is documented at `Absent`'s production rather than at construction, and the three Consequences passages asserting construction-time refusal now describe verdict-time degradation. Its measured analysis is unchanged — the source-side control total is still the only thing that would close nonempty qualification, and the widened re-read stays rejected. The ADR is not a pinned compatibility-surface file; the three touched Rust files are, and the surface and matrix are resealed here. Co-authored-by: t <dev@example.invalid> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
commented
Sep 15, 2026
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
master gained sixteen merges while this branch was held, so the merge is large but the only conflicts were the two compatibility manifests; agent_catalog.rs auto-merged despite both sides touching it. The manifests needed care rather than a side-pick. This branch's pin set is 216 entries against master's 212: it adds book_presence.rs, agent_catalog.rs, agent_presence.rs and agent_presence_tests.rs for the files it introduces. Taking master's manifest would have silently dropped all four and left the new code unpinned while the gate reported success -- the failure the brain records as a gate only seeing what it is pointed at. This branch's manifest is kept instead, and the reseal used the documented --pins-changed inversion because the pin list itself differs. rehash reported 33 changed hashes, all accounted for by what landed on master today: the CI workflows, both lockfiles and the crate manifests, the binding and read-path sources, the two files whose tests were extracted, and the sources the DSC removal touched. Pin count is unchanged at 216 with none added or removed, so the four new pins survived. Verified on the merged tree: cargo fmt --all --check clean, cargo check --workspace --all-targets clean, and cargo clippy --workspace --all-targets -- -D warnings -A clippy::pedantic clean at CI's exact flags. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
commented
Sep 15, 2026
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
replay_the_twenty_invoice_engagement still asserted that a nonempty window fails closed with presence_window_incomplete and evidence state partial. #397 deleted that error variant and made a partial window answer, withholding only Absent, so the assertion named a string the tool can no longer produce. It compiled because the code is a string literal rather than the enum, and it never failed because the test is #[ignore]d for the lab. An ignored test cannot fail, so it rots without telling anyone -- the same shape as the tool description that advertised the prefixed reason code, caught in review on #397, and the second instance of it in the same change. The assertions now mirror the offline equivalent: the window answers, its read is partial, totals.absent is zero, and no item may come back absent, because absence is the only verdict that needs to have seen the whole range. Nothing in the tree references presence_window_incomplete any more. Not run against the lab: the replay needs BRIDGE_TALLY_LIVE_* set, the gateway free, and owner authorization. The assertions are corrected but unexercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
commented
Sep 15, 2026
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
a_party_difference_echoes_the_source_spelling_not_its_catalog_binding proposed the party as "alpha traders" against a catalogue holding "Alpha Traders". That bound through BindingBasis::NormalizedName, which #331 removed from master while this branch was held. Post-#331 the folded spelling binds to nothing, so PartyOutcome is not Bound, and book_presence declines to report a difference at all -- by design: "Only a *bound* party can disagree: an ambiguous one has no single name to" compare against. The test was asserting pre-#331 binding semantics, not a property of difference reporting. The proposal now uses the catalogue's exact name. The test's actual subject is unchanged: a reported difference echoes the source spelling against the observed one, rather than echoing whatever the source bound to. This is the integration cost of a branch held open across a binding-contract change, and it was only visible in the test suite -- fmt, check and clippy all passed on the merged tree. 267 pass in bridge-tally-core. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
commented
Sep 15, 2026
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
replay_the_twenty_invoice_engagement needs sixteen usable vouchers whose types are all manually numbered, because presence by voucher number is only meaningful under Manual -- under Automatic Tally discards the supplied number. But the read passed only company_guid, from and to, then took the first sixteen rows of whatever came back. A real book cannot satisfy that. Measured offline across twelve monthly captures of the reference book: every month holds roughly 150 automatic vouchers against 50 manual ones, so the first sixteen usable rows always include an automatic type and the manual-numbering precondition trips. So this replay could never have run against a real book, which is why the open P1 asking for live evidence was never answered: the mechanism for obtaining that evidence had itself never been exercised. A test that cannot run is indistinguishable from one nobody has tried. BRIDGE_PRESENCE_LIVE_VOUCHER_TYPE now scopes the read when set. Narrowing to a declared manual type is what the test already means rather than a way around its assertion. Running it this way reaches the presence call for the first time, and the call refuses with ledger_export_invalid -- a ledger-catalogue read failure, not a presence-window one. ledger_masters reads the same book's 864 ledgers cleanly on the same binary, so the divergence is in the presence engine's own catalogue path. Handed to the read-path lane; not diagnosed here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
commented
Sep 15, 2026
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The test asserts that a reported difference echoes the source's spelling rather than the master it bound to. Repairing it after "Rectify unqualified folded ledger binding" removed the folded basis, I used the catalogue's exact name -- which makes the two sides the same string and so is the one fixture under which the assertion cannot decide between those behaviours. It also made the test byte-equivalent to a_party_difference_compares_the_observed_party_field_not_every_ledger. This takes the #308 lane's version instead: bind on an identifier embedded in the master name, the remaining basis that decides without byte equality, so the source spells it its own way and still binds. Driven to failure two ways, restored byte-identical after each: - master name without the identifier -> fails at expect("party difference") - `proposed: Some(label(catalog_name))` in book_presence.rs, i.e. echoing the binding -> left "Alpha Traders 9876543210", right "ALPHA 9876543210" The second is the behaviour the name denies, and the previous fixture passed it unchanged. Also reseals the compatibility surface. The branch head failed `scripts/reseal.sh --verify` before this change -- confirmed against a clean 78e2237 with the exit status read directly rather than through a pipe, and not caused by this edit, which touches only the unpinned tests file. Pins 216 before and after, compared as sets rather than by count: none added or dropped. Gate: 1497 passed, 0 failed, 1 ignored; clippy silent on both workspaces; reseal --verify exits 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
commented
Sep 15, 2026
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The window JSON spells an absent string as `""` rather than omitting the key,
so `row["party"].as_str()` yields `Some("")` -- which claims the voucher *has*
a party whose name is blank. Two guards then fired on a book that merely held a
voucher naming nobody: the catalogue check reported all 864 masters incomplete,
and the presence engine refused the whole window with `presence_text_blank`.
`present_text` translates that back at the boundary where it is introduced, so
neither guard needs a special case. Only the **observed** side uses it. A blank
on a caller's proposal is input the schema already refuses, and mapping it to
"absent" here would quietly accept what the schema rejects -- so the two
proposal sites keep reading the field literally.
An empty `PARTYLEDGERNAME` is a real Tally shape rather than something we
invent: 277 of 2,829 party fields across 53 captured windows of the reference
book are genuinely empty, none absent. That is the opposite of an entry's
ledger name, which is populated 7,633 times and blank zero times -- measured by
the read-path session, and the reason this fix reads a blank party as "no
party" instead of teaching the engine to carry blanks generally.
The replay assertion now prints the refusal it failed on. Asserting `isError`
alone reported that the read failed while withholding the one thing that says
why, which cost a full diagnosis round the first time it fired.
With the catalogue half in #400, the live replay
`replay_the_twenty_invoice_engagement` passes against the real book for the
first time: 20 vouchers, presence verdicts, no refusal.
Gate: 1498 passed, 0 failed; clippy silent on both workspaces; reseal --verify
exits 0 with the pin set unchanged at 216.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
commented
Sep 15, 2026
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
# Conflicts: # docs/tally/compatibility/compatibility-matrix.json # docs/tally/compatibility/compatibility-surface.json
Kept this branch's pin superset: master's 212 plus the four presence files it does not carry. Pin sets compared as paths against both parents -- none dropped from either. Regenerated, verified, then staged, in that order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
commented
Sep 15, 2026
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This was referenced Sep 15, 2026
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.
Adds the voucher-presence engine: given a window Bridge read and a set of proposed vouchers, decide for each whether the book already holds it.
This PR was held for weeks on a single reviewer objection — "obtain live evidence before shipping the presence engine" — and that objection was right. Getting the evidence found four real defects that no synthetic fixture could have reached.
Live evidence
replay_the_twenty_invoice_engagementnow runs against a licensed TallyPrime 7.1 book (864 ledgers, a real month of Sales vouchers) and passes.It had never run before. The harness required sixteen manually numbered vouchers but read the window without a type filter, and a real book is overwhelmingly automatic — so no book could satisfy its precondition, and it looked like a test nobody had tried rather than one that could not run. It now honours
BRIDGE_PRESENCE_LIVE_VOUCHER_TYPE, and its assertion prints the refusal it failed on instead of only thatisErrorwas true.What the first live run found
The engine refused rather than answering. Four guards, each failing closed on data Tally legitimately produces, each losing a whole window rather than one row:
, which XML attribute normalization preserves — failed the whole catalogue parse. One master made all 864 unreadable.MasterCatalog::new."party": ""; looking that blank up reported all 864 masters incomplete.#400 fixes the catalogue half and is on master. This PR carries the presence half, which could not go there because
agent_presence.rsdoes not exist on master.present_texttranslates JSON's "absent means empty string" back toNoneat the boundary that introduces it, so neither guard needs a special case. Only the observed side uses it — a blank on a caller's proposal is input the schema already refuses, and mapping it to "absent" would quietly accept what the schema rejects.That an empty party is real rather than invented is measured, not assumed: across 53 captured windows of this book, 277 of 2,829 party fields are genuinely empty and none absent, while an entry's ledger name is populated 7,633 times and blank zero times. Two blanks that look identical in our types, with opposite origins.
Verification
src-tauriandtools— clippy silent on both, exit status read directly rather than through a pipe.reseal.sh --verifyexit 0. Pin superset kept at 216 through the master merge: master's 212 plus the four presence files it does not carry, compared as paths against both parents, none dropped.Scope
One book, one month, one voucher type, one feature set. This is live evidence, not a corpus.
🤖 Generated with Claude Code