Rectify sanitized statement party equivalence - #332
Merged
Merged
Conversation
These arrived on #306 after it was merged. Two are corrections to text I wrote, and one is the same failure in two places: a qualification stated in prose while the thing beside it does the unqualified thing anyway. **The case clause was symmetric while the note admitted it was not.** `ascii_lower(candidate) == ascii_lower(tally_name)` also accepts an UPPERCASE candidate against a lowercase master, a direction never sent — so the symmetric form asserts a second experiment, exactly as a canonical form does for the separator. The note below it said so and the predicate kept the clause. Now `candidate == ascii_lower(tally_name)`, which is what the capture sent: a lowercase candidate against a master carrying uppercase. All four clauses are directional and each is one measured result. **`PROMPT_PLAYBOOK.md` still mandated NFC normalization for name keys**, and it is the active Phase 2 implementation authority — so the reference could carry the MEASURED row saying normalising is wrong while the document people build from told them to normalise. Withdrawn with a dated deviation citing the measurement: an NFD spelling of a UI-created NFC ledger was rejected with `EXCEPTIONS=1` while the NFC spelling created it, and an NFD create read back with identical codepoints. Case folding and the non-Latin fixtures stay; normalisation goes. **`IMPLEMENTATION_GUIDE.md` §3.4 kept the fingerprint alive as duplicate prevention.** My sentence — "it is still true everywhere §3.3a does not reach" — preserved the withdrawn mandate in every case §3.3a excludes, while §3.4a of the same patch says the tuple cannot distinguish a retry from a legitimate identical payment and so prevents no duplicate anywhere. Two paragraphs of one patch contradicting each other is how a withdrawn mandate comes back. What is true outside §3.3a's reach is that there is no proven mechanism there at all, which is narrower and less comfortable. **`IMPROVEMENT_PLAN_2026H2.md` cited both sections wrongly.** The signal table is `IMPLEMENTATION_GUIDE.md` §3.4a, not §6.2 (Unicode round-trip), and the reference's `REMOTEID` rule is §9.3 — `§3.3a` is a section of the guide, not of the reference, so the citation pointed at a section that does not exist. A supersession note that lands readers away from the decision it claims to identify makes the old mandate harder to disambiguate, not easier. Reference resealed: 1 changed file, pins unchanged at 211. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five review findings, docs-only. Defect A (2 findings, P1): PROMPT_PLAYBOOK.md Phase 2 item 5 prescribed a symmetric case-insensitive collation for name keys. §9.4b's accepts(candidate, tally_name) predicate only measured a lowercase candidate against an uppercase master; the reverse was never sent. A symmetric fold accepts that unverified direction and can bind a voucher to the wrong master. Restated as the directional predicate. Defect B (2 findings, P1): the NFC/NFD withdrawal did not reach three other sites that all instructed matching or reading masters back by 'normalized name' without excluding NFC, or flagged NFC normalization only when applied asymmetrically: - PROMPT_PLAYBOOK.md Phase 2 item 5 (paired with the case fix above) - PROMPT_PLAYBOOK.md Phase 4 step 4 (readback verification) - PROMPT_PLAYBOOK.md Phase 2 adversarial review item 4 - IMPROVEMENT_PLAN_2026H2.md §8.6 Each now carries a dated deviation note pointing at TALLY_PROTOCOL_REFERENCE.md §9.4b, in the style already used for the existing 2026-09-11 fingerprint deviation. Over-correction (1 finding, P2): IMPLEMENTATION_GUIDE.md §3.4a claimed no proven duplicate-prevention mechanism exists outside §3.3a's REMOTEID path. TALLY_PROTOCOL_REFERENCE.md §9.8 is VERIFIED that Manual numbering with PREVENTDUPLICATES=Yes cleanly rejects a duplicate (CREATED=0, ALTERED=0, EXCEPTIONS=1) where Automatic numbering silently creates one — a second proven mechanism, already noted in §3.3 of the same file. Narrowed the claim to what's actually unproven (an automatic dedupe decision from the fingerprint tuple) while keeping the correct part: the fingerprint must never auto-suppress. No code changes. TALLY_PROTOCOL_REFERENCE.md (pinned) is untouched; compatibility surface pin count unchanged at 211. Section-number gate scripts pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… dedupe Three findings left open on #314, each checked against the tree before being accepted. **The name fold is prescribed outside the SKU it was measured on.** Phase 4's readback step told implementers to match masters with §9.4b's `accepts()` predicate. §9.4b's case-folding and hyphen-for-space rows carry no licensed-SKU qualification, so they inherit §0's Edit Log 7.0 Educational baseline; the only row measured on a licensed 7.1 instance is NFC/NFD, and it points the other way — exact codepoints. Phase 4 runs against licensed TallyPrime. Applying an Educational-baseline fold there can bind a write to an account Tally keeps distinct. Phase 4 now matches on exact codepoints and widens only where a licensed capture or the connected instance's compatibility result qualifies the predicate. **The fingerprint correction stopped at the plan.** IMPROVEMENT_PLAN was repointed to §3.4a but Phase 4 item 6 still sent implementers to IMPLEMENTATION_GUIDE §6.2 — the Unicode round-trip result, which says nothing about fingerprints — and therefore away from the signal table and its automatic-suppression constraints. **Withdrawing the tuple withdrew a mechanism it had no claim over.** §3.4's "there is no proven duplicate-prevention mechanism there at all" is absolute, and false: §9.8 is VERIFIED that Manual numbering with `PREVENTDUPLICATES=Yes` preserves the supplied number and rejects a duplicate cleanly (`CREATED=0, ALTERED=0, EXCEPTIONS=1`). The claim is now scoped to what lies outside both §3.3a's REMOTEID path and §9.8's, with a sentence saying explicitly that §9.8 supplies a different mechanism rather than rehabilitating the fingerprint — the tuple stays withdrawn everywhere. Surface resealed (rehash → seal → repoint) after the rebase onto #289; gate passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… one The scope gate added in `40dee282` was inserted *between* two directives that still prescribed the fold unconditionally, so Phase 4 step 4 stated its name-matching rule three times: unqualified before the gate, gated, and unqualified again in the closing deviation. An implementer reading to the end of the step finds the fold reinstated. Codex flagged the third; the first was the same defect and is fixed with it. Step 4 now names the gate as the only matching rule in the step, and the NFC deviation says explicitly that it *removes* normalization without widening what remains — on an unqualified licensed SKU, exact codepoints and nothing else. Phase 2 item 5 carried the same unqualified prescription and was not flagged. Fixed anyway: Phase 2 reads a licensed instance too, and a read is the worse place to be wrong, because the mirror rows built there are what later binding decisions resolve against — a fold that merges two masters on read merges them everywhere downstream. The lesson is the doc-level form of "a fix that adds a branch must assert on the new branch": a qualifier inserted between two unqualified statements qualifies neither. Add the gate, then audit every sibling directive in the same step for whether it inherits it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit withdrew an over-broad claim and replaced it with another one. It said Manual numbering with `PREVENTDUPLICATES=Yes` is "a proven duplicate-prevention mechanism". §9.8 measured one thing: how a **failed `Alter`** behaves under that setting. Its own rule says *"Do not apply the failed-`Alter` observation to a different request identity mechanism"*, and a crash retry — the case §3.4 was discussing — sends a `Create`. It overreaches on SKU as well. §9.8 carries no licensed qualification for the numbering path; the scope clarification beneath it covers a licensed *Journal* `ACTION="Create"` repeat carrying `REMOTEID` and says in terms that it does not establish voucher-number identity, the configured numbering method, or other request shapes. Both sites now state the result as what it is — for a failed `Alter` on §9.8's baseline, Manual numbering converts a silent duplicate into a clean rejection — with request shape, SKU and voucher type named as the three limits. The tuple stays withdrawn everywhere; §9.8 reports how one failure mode behaves under one setting, which is a different kind of thing from a mechanism. A paragraph is added saying why this needed two corrections: **a claim is not made safe by being a correction.** Withdrawing one over-broad statement is the moment you are most likely to write another, because the writing feels like narrowing while the sentence is widening something else. `IMPROVEMENT_PLAN_2026H2.md` §8.6 carried the same defect twice over: it prescribed the directional fold with no scope gate, and its NFC deviation then reinstated the fold unconditionally — the identical shape fixed in the playbook last commit, in a file the playbook's own fix did not reach. §8.6 now defers to the playbook's SCOPE GATE and its deviation says explicitly that removing normalization does not decide whether any fold is permitted on top. Audited rather than assumed: all three `accepts()` prescription sites across the tally docs now carry a gate or an exact-codepoint instruction within eight lines. Surface resealed; gate passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
**The NFC/NFD capture is not licensed evidence.** The scope gate added two commits ago said "only the NFC/NFD row was measured on a licensed 7.1 instance". It was not. `TALLY_PROTOCOL_REFERENCE.md` records that capture as "2026-08-19, TallyPrime 7.1, port 9001" and says nothing about licence mode; `tests/fixtures/encoding/PROVENANCE.md` records that day's instance as **EDU**. I supplied "licensed" from a stored note about what port 9001 usually runs, not from the document, and then used it as the premise of an argument about which evidence is qualified. Correcting it makes the gate **stricter**, not weaker: with that row withdrawn, **no** row of §9.4b is qualified on a licensed SKU, so there is no licensed evidence to widen towards at all. The gate's instruction is unchanged — exact codepoints — and now rests on something the tree actually says. **Two identity authorities still said "normalised name".** The withdrawal was carried to four sites and I reported it complete; `IMPLEMENTATION_GUIDE.md` §3.5 and `TALLY_PROTOCOL_REFERENCE.md`'s `LASTMID` entry were not among them, and both are exactly where an implementer looks for the readback rule — so an implementer following the identity guidance rather than the deviation could still resolve an NFD create onto a distinct pre-existing NFC master. Both now say normalised never means NFC/NFD-normalised and defer to the scope gate. Audited rather than counted this time: no `by normalised name` or `by normalized name` directive remains anywhere in the tally docs. The lesson is one already written down and not applied here: a recalled fact is not a measured one. A note saying what an instance usually is cannot qualify a specific capture — only that capture's provenance can. Surface resealed; gate passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ne for creating The scope gate said "match on exact codepoints" and stopped there, as though strictness were automatically safe. It is not, and the failure is in the opposite direction from the one the gate was written to prevent. If the connected SKU **does** share the Educational case behaviour, an existing `FOO` reads as ABSENT for a requested `foo` under exact matching. A step that creates whatever it finds missing then writes a SECOND master differing only by case — a duplicate in a client's book, produced by the conservative rule. The loose rule misbinds; the strict rule duplicates. Neither is safe, and the gate offered no third option. It has three outcomes now: **bind** on an exact match; **create** only when no master differs from the requested name by case or separator alone; otherwise **refuse and raise it for a human**. A near-collision on an unqualified SKU is exactly where neither automatic answer is defensible. The near-collision set is computed with §9.4b's fold used as a **detector**, never as a binder — and that distinction is what makes it legitimate on an unqualified SKU. Folding to conclude "something similar exists, stop" needs no licensed qualification, because the conclusion is a refusal rather than a write. Unverified evidence can justify caution; it cannot justify action. Separately, `TALLY_PROTOCOL_REFERENCE.md`'s crash-retry paragraph still called Manual + `PREVENTDUPLICATES=Yes` a qualified duplicate-rejection mechanism for a repeated number, which is the generalisation corrected in the guide two commits ago — the correction had not reached the behavioural authority, which is the document an implementer trusts most. It now names the failed-`Alter` scope, the UNVERIFIED `Create` retry and the UNVERIFIED licensed SKU, and says plainly not to read it as promising a crash retry is safe. Surface resealed; gate passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e reversed The near-collision guard added last commit reused §9.4b's `accepts()` as its detector, and `accepts()` is **directional**. For a requested `FOO` against an existing `foo`, `accepts(FOO, foo)` is false — so the guard misses precisely the collision it exists to catch, and Phase 4 goes on to create the duplicate master. The hazard survived inside its own fix. The detector now folds symmetrically and deliberately over-wide: case-insensitive both ways, hyphen and space interchangeable both ways, leading and trailing whitespace ignored, internal whitespace runs collapsed. Several of those rows are UNVERIFIED as *matching* behaviour, and that is the point rather than an oversight. **An unverified equivalence cannot justify a write, but it is ample reason to stop and ask.** The asymmetry in consequences sets the direction: a detector that misses a collision creates a duplicate in a client's book, while a detector that over-fires costs one question to a human. Fail toward the question. That generalises past this guard — wherever the same fold serves two purposes, the safe width differs by purpose, and reusing one predicate for both silently adopts the wrong width for one of them. Also: the licence correction cited `tests/fixtures/encoding/PROVENANCE.md`, which exists at no such path. The evidence is at `src-tauri/crates/bridge-tally-protocol/tests/fixtures/encoding/PROVENANCE.md`, verified present. I took the short path from the review comment that raised the finding instead of resolving it — in a commit whose whole subject was citing provenance accurately. Surface resealed; gate passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five findings, three P1, and the first one voids the guard entirely. **The master-creation guard sat inside step 4, which begins "after counters accept".** It ran after dispatch. By then `ACTION="Create"` against a name Tally considers matching has already returned `CREATED=0, ALTERED=1` and overwritten the existing master with the retry payload (guide §3.6, plan §8.5). A refusal raised during readback protects nothing — the client's master is already gone. It is now step **3a**, on the intent, before any import is sent. **And the harm was described wrongly, in the safer direction.** The guard said a near-collision "makes a SECOND master differing only by case". The measured behaviour is an overwrite in place: the existing master's group, opening balance and GST registration are replaced by whatever the new payload carried. A duplicate is visible in a ledger list. An overwrite is not. **The detector omitted canonical equivalence.** §9.4b's exact-codepoint result came from an EDU instance, so a licensed SKU that folds NFC/NFD is not excluded — and an NFD request beside an existing NFC master would pass exact lookup *and* the detector, and overwrite it. Canonical equivalents now collide. This does not reintroduce NFC normalisation into matching: the binder still compares exact codepoints. One folds to refuse, the other folds to write, and only the second needs evidence. **A read receipt cannot qualify a write predicate.** The gate offered "where the compatibility result for the connected instance says it holds" as a widening route. `compatibility/README` defines that evidence as a live-**read** receipt and says it "never establishes ... any write behavior". Withdrawn here, in Phase 2 item 5, and in the plan's §8.6, all of which carried it. **What does qualify, now that #288 has landed: §9.4d.** It re-ran §9.4b's method on licensed 7.1 silver with `education_mode=false`, importing vouchers naming folded spellings and reading the **day book** back to see which master each posted against. That is observed write behaviour on the SKU this project writes to — for **ledgers**. So ledgers on licensed 7.1 match under §9.4d's rows, and every other master type still matches on exact codepoints, because §9.4d measured nothing about them. **§9.5 was carrying implementation policy under a `VERIFIED` marker.** The measurement is `LASTMID=0`; "which name rule applies" is a choice. A reader collecting VERIFIED claims would have collected one that was never measured. The prescription is now a separate block that says so — the evidence marker governs the paragraph it opens, and a prescription placed under it inherits a confidence it has not earned. Surface resealed after the rebase onto #288; gate passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two of the three findings left open when #303 was merged too early. The third is recorded with its reproduction rather than rushed. **A bare `X` or `XX` was copied into the fixture verbatim (P1).** The all-`X` branch treated any token of nothing but `X` as a masking convention and returned it unchanged — which also bypassed `reserve_source_tokens`, the one check that exists to stop source text reaching a fixture. `bank_statement_import` requires `[Xx]{4,}\d*` before it will call something a masked account, so below four the run is a customer value: an initial, for instance. Now gated on that same threshold, named `MASK_MIN_XS` beside the alphabets so the two files cannot drift apart. `XXXXXXXX1234` still keeps its run and fabricates the digits, which is the case the run exists for. **The capture was decoded with the host locale (P2).** `pdftotext` emits UTF-8; `read_text()` without an encoding uses the locale, so on a Windows Python whose locale is not UTF-8 a raw `Café` becomes mojibake with extra code points and Indic bytes raise `UnicodeDecodeError` before sanitisation runs at all. Neither existing check reaches that boundary — CI is ubuntu-only, and the Unicode cases call `_scrub_plain` with strings that are already decoded. Pinned to UTF-8 on the read; the write already was. **Not fixed: the cross-token key collision (P2), recorded instead.** `_taken` keeps fabricated *tokens* distinct, but the reader concatenates tokens and strips whitespace, so: source 'ACD' -> 'ZZZ' key 'ZZZ' source 'A CC' -> 'Z' + 'ZZ' key 'ZZZ' one mapping row It is systematic, not rare — the counter is per shape, so the first token of every shape starts at the alphabet's first letter. Fixing it properly means the fabricated set must be uniquely decodable after whitespace removal, which is a change to the fabricator rather than a guard bolted onto it. The consequence is a fixture that could merge two parties and so fail to catch a mapping-identity regression for that pair — worth fixing, not worth rushing into a data-safety tool. The reachable case is in the test file with its reproduction so it cannot quietly get worse. Reverting the mask threshold makes the suite red on the first assertion. Fixture bytes unchanged; neither file is pinned, so no reseal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`MASK_MIN_XS` decided "is this X part of the masking convention?" one
character at a time. A token mixing X with anything else therefore made the
free-position list nonempty, skipped the all-X branch entirely, and carried
its own X straight through: `XAVIER` sanitised to `XZZZZZ`, `ABXXCD` to
`ZZXXZZ`. Those X characters are letters a customer typed, and they reached a
fixture built for a public repository — the precise leak the branch below
them was added to stop, through a narrower door.
The classification is now made once, for the whole token, against the pattern
the parsers actually use: `bank_statement_import` calls something a masked
account only when it matches `[Xx]{4,}\d*`. Inside that shape an X is
structure and survives; everywhere else an X is data and is fabricated like
any other letter.
This also fixes a second defect in the same expression. The old test was
`character != "X"`, uppercase only, so a lowercase mask was not recognised at
all and `xxxx5678` was destroyed to `zzzz1111` — losing the shape the fixture
exists to preserve. The pattern is `[Xx]` because the parser's is.
Three tests asserted the old per-character behaviour, one of them requiring
that the trailing X be preserved. They encoded the defect, so they are
rewritten rather than patched:
- the trailing-X assertion now asserts the opposite, with `XAVIER`,
`ABXXCD`, `MAX` and `X-RAY` named directly;
- the reservation test floods with tokens that merely contain X and asserts a
real mask keeps its run while none of the flood carries an X through;
- the exhaustion guard moves from `?XX` to single letters. This matters: with
X positions now free, `?XX` has 20**3 replacements and can never run out,
so leaving that shape would have turned a live guard into a test that
cannot fail.
Mutation-checked: six assertions fail against the pre-fix module, reproducing
`XAVIER -> XZZZZZ` and `ABXXCD -> ZZXXZZ` exactly.
**Known gap, stated rather than glossed.** `sbi-bbox-capture.xml` still holds
one `XX111` token generated under the old rule; the corrected rule would
fabricate it. It is a staleness gap, not a leak — `XX` runs appear twenty
times across the real statements as the *bank's* redaction marks, not
customer letters, and the digits beside them were already fabricated. Every
distinct long digit run in both fixtures was cross-checked against four real
statements spanning three banks: zero matches. Regenerating the fixture needs
the source PDF and is not done here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Classifying per token rather than per character was right, but it took the
wrong threshold. `bank_statement_import` recognises **two** account-mask
shapes: `[Xx]{4,}\d*` when deciding whether a standalone narration field is an
account, and `[Xx]+\d+` inside a UPI/IMPS reference, where a run as short as
`XX1234` is the bank's masking rather than a customer's letters. Requiring four
Xs everywhere fabricated `XX1234` into `ZZ1111`, destroying a shape the fixture
exists to preserve.
`_is_mask` is now the union, and still a whole-token test, which is what keeps
`XAVIER`, `ABXXCD` and `MAX` out — an X is structure only when the token is
nothing but a mask, and the short form additionally requires the digits that
make it an account reference rather than an initial. Twelve shapes checked; all
twelve land correctly.
This also settles the staleness gap named in the previous commit: the `XX111`
already in `sbi-bbox-capture.xml` is a mask under the parser's own rule, so it
was never stale.
**The end-to-end check was contradicting the unit cases.** `identifying_tokens`
drops tokens shorter than `IDENTIFYING_LENGTH`, and the survivor set subtracted
every pure-X token as deliberate — so a surviving `X`, `XX` or `XXX`, which the
unit cases define as a leak, was invisible to the artifact check and excused if
seen. The length exclusion exists because a one-digit token has nine possible
replacements and reserving them all starves the allocator; that reasoning is
about digits, not about X. Short tokens containing an X are now included, and
only tokens the parsers would actually call a mask are subtracted.
**The first version of this fix had no test.** Reverting `_is_mask` to the
four-X rule left the whole suite green — the new branch was asserted nowhere,
which is the failure this repository keeps writing down and I reproduced while
fixing someone else's instance of it. Three rows now cover the short form, and
the assertion itself was wrong on first writing: it checked for a fixed-length
run of leading Xs, which is false for `X99`. It now requires every X position to
survive verbatim and every digit position to be fabricated.
Mutation-checked in both directions: green with the union predicate, three
named failures without it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…odex/rectify-pr316-20260912
…odex/rectify-pr316-20260912
These arrived on #306 after it was merged. Two are corrections to text I wrote, and one is the same failure in two places: a qualification stated in prose while the thing beside it does the unqualified thing anyway. **The case clause was symmetric while the note admitted it was not.** `ascii_lower(candidate) == ascii_lower(tally_name)` also accepts an UPPERCASE candidate against a lowercase master, a direction never sent — so the symmetric form asserts a second experiment, exactly as a canonical form does for the separator. The note below it said so and the predicate kept the clause. Now `candidate == ascii_lower(tally_name)`, which is what the capture sent: a lowercase candidate against a master carrying uppercase. All four clauses are directional and each is one measured result. **`PROMPT_PLAYBOOK.md` still mandated NFC normalization for name keys**, and it is the active Phase 2 implementation authority — so the reference could carry the MEASURED row saying normalising is wrong while the document people build from told them to normalise. Withdrawn with a dated deviation citing the measurement: an NFD spelling of a UI-created NFC ledger was rejected with `EXCEPTIONS=1` while the NFC spelling created it, and an NFD create read back with identical codepoints. Case folding and the non-Latin fixtures stay; normalisation goes. **`IMPLEMENTATION_GUIDE.md` §3.4 kept the fingerprint alive as duplicate prevention.** My sentence — "it is still true everywhere §3.3a does not reach" — preserved the withdrawn mandate in every case §3.3a excludes, while §3.4a of the same patch says the tuple cannot distinguish a retry from a legitimate identical payment and so prevents no duplicate anywhere. Two paragraphs of one patch contradicting each other is how a withdrawn mandate comes back. What is true outside §3.3a's reach is that there is no proven mechanism there at all, which is narrower and less comfortable. **`IMPROVEMENT_PLAN_2026H2.md` cited both sections wrongly.** The signal table is `IMPLEMENTATION_GUIDE.md` §3.4a, not §6.2 (Unicode round-trip), and the reference's `REMOTEID` rule is §9.3 — `§3.3a` is a section of the guide, not of the reference, so the citation pointed at a section that does not exist. A supersession note that lands readers away from the decision it claims to identify makes the old mandate harder to disambiguate, not easier. Reference resealed: 1 changed file, pins unchanged at 211. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five review findings, docs-only. Defect A (2 findings, P1): PROMPT_PLAYBOOK.md Phase 2 item 5 prescribed a symmetric case-insensitive collation for name keys. §9.4b's accepts(candidate, tally_name) predicate only measured a lowercase candidate against an uppercase master; the reverse was never sent. A symmetric fold accepts that unverified direction and can bind a voucher to the wrong master. Restated as the directional predicate. Defect B (2 findings, P1): the NFC/NFD withdrawal did not reach three other sites that all instructed matching or reading masters back by 'normalized name' without excluding NFC, or flagged NFC normalization only when applied asymmetrically: - PROMPT_PLAYBOOK.md Phase 2 item 5 (paired with the case fix above) - PROMPT_PLAYBOOK.md Phase 4 step 4 (readback verification) - PROMPT_PLAYBOOK.md Phase 2 adversarial review item 4 - IMPROVEMENT_PLAN_2026H2.md §8.6 Each now carries a dated deviation note pointing at TALLY_PROTOCOL_REFERENCE.md §9.4b, in the style already used for the existing 2026-09-11 fingerprint deviation. Over-correction (1 finding, P2): IMPLEMENTATION_GUIDE.md §3.4a claimed no proven duplicate-prevention mechanism exists outside §3.3a's REMOTEID path. TALLY_PROTOCOL_REFERENCE.md §9.8 is VERIFIED that Manual numbering with PREVENTDUPLICATES=Yes cleanly rejects a duplicate (CREATED=0, ALTERED=0, EXCEPTIONS=1) where Automatic numbering silently creates one — a second proven mechanism, already noted in §3.3 of the same file. Narrowed the claim to what's actually unproven (an automatic dedupe decision from the fingerprint tuple) while keeping the correct part: the fingerprint must never auto-suppress. No code changes. TALLY_PROTOCOL_REFERENCE.md (pinned) is untouched; compatibility surface pin count unchanged at 211. Section-number gate scripts pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… dedupe Three findings left open on #314, each checked against the tree before being accepted. **The name fold is prescribed outside the SKU it was measured on.** Phase 4's readback step told implementers to match masters with §9.4b's `accepts()` predicate. §9.4b's case-folding and hyphen-for-space rows carry no licensed-SKU qualification, so they inherit §0's Edit Log 7.0 Educational baseline; the only row measured on a licensed 7.1 instance is NFC/NFD, and it points the other way — exact codepoints. Phase 4 runs against licensed TallyPrime. Applying an Educational-baseline fold there can bind a write to an account Tally keeps distinct. Phase 4 now matches on exact codepoints and widens only where a licensed capture or the connected instance's compatibility result qualifies the predicate. **The fingerprint correction stopped at the plan.** IMPROVEMENT_PLAN was repointed to §3.4a but Phase 4 item 6 still sent implementers to IMPLEMENTATION_GUIDE §6.2 — the Unicode round-trip result, which says nothing about fingerprints — and therefore away from the signal table and its automatic-suppression constraints. **Withdrawing the tuple withdrew a mechanism it had no claim over.** §3.4's "there is no proven duplicate-prevention mechanism there at all" is absolute, and false: §9.8 is VERIFIED that Manual numbering with `PREVENTDUPLICATES=Yes` preserves the supplied number and rejects a duplicate cleanly (`CREATED=0, ALTERED=0, EXCEPTIONS=1`). The claim is now scoped to what lies outside both §3.3a's REMOTEID path and §9.8's, with a sentence saying explicitly that §9.8 supplies a different mechanism rather than rehabilitating the fingerprint — the tuple stays withdrawn everywhere. Surface resealed (rehash → seal → repoint) after the rebase onto #289; gate passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… one The scope gate added in `40dee282` was inserted *between* two directives that still prescribed the fold unconditionally, so Phase 4 step 4 stated its name-matching rule three times: unqualified before the gate, gated, and unqualified again in the closing deviation. An implementer reading to the end of the step finds the fold reinstated. Codex flagged the third; the first was the same defect and is fixed with it. Step 4 now names the gate as the only matching rule in the step, and the NFC deviation says explicitly that it *removes* normalization without widening what remains — on an unqualified licensed SKU, exact codepoints and nothing else. Phase 2 item 5 carried the same unqualified prescription and was not flagged. Fixed anyway: Phase 2 reads a licensed instance too, and a read is the worse place to be wrong, because the mirror rows built there are what later binding decisions resolve against — a fold that merges two masters on read merges them everywhere downstream. The lesson is the doc-level form of "a fix that adds a branch must assert on the new branch": a qualifier inserted between two unqualified statements qualifies neither. Add the gate, then audit every sibling directive in the same step for whether it inherits it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit withdrew an over-broad claim and replaced it with another one. It said Manual numbering with `PREVENTDUPLICATES=Yes` is "a proven duplicate-prevention mechanism". §9.8 measured one thing: how a **failed `Alter`** behaves under that setting. Its own rule says *"Do not apply the failed-`Alter` observation to a different request identity mechanism"*, and a crash retry — the case §3.4 was discussing — sends a `Create`. It overreaches on SKU as well. §9.8 carries no licensed qualification for the numbering path; the scope clarification beneath it covers a licensed *Journal* `ACTION="Create"` repeat carrying `REMOTEID` and says in terms that it does not establish voucher-number identity, the configured numbering method, or other request shapes. Both sites now state the result as what it is — for a failed `Alter` on §9.8's baseline, Manual numbering converts a silent duplicate into a clean rejection — with request shape, SKU and voucher type named as the three limits. The tuple stays withdrawn everywhere; §9.8 reports how one failure mode behaves under one setting, which is a different kind of thing from a mechanism. A paragraph is added saying why this needed two corrections: **a claim is not made safe by being a correction.** Withdrawing one over-broad statement is the moment you are most likely to write another, because the writing feels like narrowing while the sentence is widening something else. `IMPROVEMENT_PLAN_2026H2.md` §8.6 carried the same defect twice over: it prescribed the directional fold with no scope gate, and its NFC deviation then reinstated the fold unconditionally — the identical shape fixed in the playbook last commit, in a file the playbook's own fix did not reach. §8.6 now defers to the playbook's SCOPE GATE and its deviation says explicitly that removing normalization does not decide whether any fold is permitted on top. Audited rather than assumed: all three `accepts()` prescription sites across the tally docs now carry a gate or an exact-codepoint instruction within eight lines. Surface resealed; gate passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
**The NFC/NFD capture is not licensed evidence.** The scope gate added two commits ago said "only the NFC/NFD row was measured on a licensed 7.1 instance". It was not. `TALLY_PROTOCOL_REFERENCE.md` records that capture as "2026-08-19, TallyPrime 7.1, port 9001" and says nothing about licence mode; `tests/fixtures/encoding/PROVENANCE.md` records that day's instance as **EDU**. I supplied "licensed" from a stored note about what port 9001 usually runs, not from the document, and then used it as the premise of an argument about which evidence is qualified. Correcting it makes the gate **stricter**, not weaker: with that row withdrawn, **no** row of §9.4b is qualified on a licensed SKU, so there is no licensed evidence to widen towards at all. The gate's instruction is unchanged — exact codepoints — and now rests on something the tree actually says. **Two identity authorities still said "normalised name".** The withdrawal was carried to four sites and I reported it complete; `IMPLEMENTATION_GUIDE.md` §3.5 and `TALLY_PROTOCOL_REFERENCE.md`'s `LASTMID` entry were not among them, and both are exactly where an implementer looks for the readback rule — so an implementer following the identity guidance rather than the deviation could still resolve an NFD create onto a distinct pre-existing NFC master. Both now say normalised never means NFC/NFD-normalised and defer to the scope gate. Audited rather than counted this time: no `by normalised name` or `by normalized name` directive remains anywhere in the tally docs. The lesson is one already written down and not applied here: a recalled fact is not a measured one. A note saying what an instance usually is cannot qualify a specific capture — only that capture's provenance can. Surface resealed; gate passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…odex/rectify-pr316-20260912
§9.4 still ended "Pre-read before creating" — which reads as sufficient and is not. A foreign writer can create the master between the read and the dispatch, and this section's own overwrite then lands on a master Bridge never observed: no pre-image, prior content unrecoverable. The behavioural authority was the last site still saying it. The guide and the playbook had both been corrected; an implementer reading the protocol reference, which is the document they trust over either, would have followed the stale rule. Two things this states that the earlier correction did not: **Narrowing the gap does not close it, and neither does reading again.** My own earlier wording — "re-read immediately before dispatch, not once per batch" — invited an implementer to treat a tighter read as the mitigation. It is not. Only a qualified mutation-time condition or a proven exclusive-write window covers that interval; another ordinary pre-read is the same check, sooner. **So the assertion moves to the response.** A create MUST assert `CREATED=1`; `ALTERED=1` is an overwrite alarm and a manual halt, never a success under a different counter. That is checkable at the only moment that is not a window. Surface resealed; gate passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lamemustafa
added
area:tally
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: 1a173e2249
ℹ️ 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 6 commits
September 13, 2026 01:04
…odex/rectify-issue330-20260912
…odex/rectify-issue330-20260912
1 task
Conflicts were the two compatibility manifests, TALLY_PROTOCOL_REFERENCE.md, and the bank-statement sanitiser plus its test, which #316 also touched. - sanitise-bbox-capture.py takes this branch's side: it is a strict superset, carrying the same encoding="utf-8" read that master added plus this branch's keep-list materialisation and _load_parser call. - sanitise-bbox-capture.test.py takes this branch's side. _load_parser does not exist on master, so master's block is the pre-#332 shape; this branch's version adds the EvidenceRefusal handling that matches its own code, plus boundary controls for cropped rows, wrong-profile parsing and row-count mismatch. - TALLY_PROTOCOL_REFERENCE.md takes master's wording, consistent with how the same three paragraphs were resolved when #314 and #316 landed. Verified rather than assumed: scripts/sanitise-bbox-capture.test.py runs green on the merged tree ("all sanitiser contracts hold"). Compatibility surface resealed; a confirming rehash reports zero changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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 #330. Addresses the downstream party-equivalence gap from #303, tracked in #317.
Functional summary
The bbox sanitizer previously guaranteed uniqueness only for individual fabricated tokens. The bank importer groups complete party names with a whitespace-insensitive key, so different token sequences could still become the same party, or one party could split into multiple output groups.
Generation now requires an explicit
hdfcorsbiparser profile. It parses source and generated text using the same retained geometry, requires nonempty aligned rows with a populated balance and exactly one debit/credit side, and rejects changed side/reference shapes. Amounts must pass the importer’s actual unsigned money and signed balance grammars. Date equivalence uses parsed dates, so equivalent date spellings cannot become distinct output dates; a split refuses generation. A bounded two-way class mapping preserves both equality and inequality of parsed parties. Missing or sentinel party evidence refuses generation. Stable refusal categories include parser and zero-based row context when known, without printing party values.The destination is written only after the parser evidence passes. A retained region that cuts away required accounting evidence is refused. Existing captured bytes stay unchanged, and their token-privacy checks still run even when the new parser gate correctly refuses publication.
Stack: depends on #316 and targets that branch for review of the sanitizer delta. After #316 lands, retarget to current master and verify the integrated tree; this feature-base state is not merge acceptance.
Test or reproduction command
Candidate
ab5dc087d72ce1a921dcbc10bd69c40ded51be87:python3 -B scripts/sanitise-bbox-capture.test.py: 117 sanitizer contracts passed on the final sanitizer code atda2c024e937900db705f40dfd51c62c87a5fd406. Subsequent normal merges carry only reviewed directional-scope documentation from Rectify ambiguous mask preservation and recovery guidance #316; sanitizer source/test bytes are unchanged.python3 -B scripts/bank_statement_import.test.py: the unchanged importer suite passed during this fix; no importer implementation is changed by this PR.python3 -m py_compile scripts/sanitise-bbox-capture.py scripts/sanitise-bbox-capture.test.pyandgit diff --check: passed.These checks use already sanitized captures and synthetic control inputs. They do not establish the uniqueness of original raw customer data or live import behavior. Any future real source must pass this same gate; local partial accounting regions or unresolved parties remain explicit refusals.
Reuse/deletion: uses the existing bank profiles, parser, party key and token allocator. Replaces a free-form banner argument with closed parser selection and deletes the unconditional emission path. No new dependency, global allocator search, silent retry or live financial operation is added. Without this check, a generated fixture can misrepresent downstream party identity despite unique fabricated tokens.
Migration compatibility: CLI callers must use
hdfcorsbias BANK; documentation is updated. Existing captures are not rewritten, and no persisted schema changes. Rollback removes the generation gate but restores the known equivalence gap; retain equivalent admission checks in any replacement.Security impact: the gate refuses ambiguous or incomplete evidence before file emission, while existing sanitization still governs retained content. No credentials, raw client capture, Tally request or financial mutation was introduced. Tests ran locally on macOS; native Windows filesystem behavior is not claimed.
The latest #316 directional-plan correction is integrated by normal merge. Only that unpinned plan sentence changed after the 117-control sanitizer validation; sanitizer source/tests are byte-identical. Final rehash has zero drift and all 212 actual-byte pins and the compatibility gate pass.