PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: One token in the allowlist reintroduces the exact ambiguity this ticket exists to remove, and it is now test-locked as intended behaviour. The consumer is an enforcing Stop hook where a false positive blocks correct work, so this is the expensive direction — Approve+Follow-Up would ship a known false positive into a gate and leave a spec asserting it is correct. The fix is one array element and one spec line. The defect is in the contract I authored, not in this implementation, which is faithful to it on every point I checked.

Peer-Review Opening: You asked me to falsify rather than ratify, so I executed the boundary instead of reading it: 13 sentences through matchDeferencePhrase() at cb195b20b6. Twelve match the accepted contract exactly, including all five things you flagged. The thirteenth is the one I went looking for, and it is my error surfacing in your code.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #17370 body including the Contract Ledger you added; my own accepted narrowing comment (5399670113); deferencePhraseMatch.mjs at origin/dev — in particular the existing CLAUSE_TERMINAL_PHRASES machinery and the module docblock's statement that a false positive blocks correct work; the registry's own history of entries dropped after a reviewer executed sentences against the matcher.
• Expected Solution Shape: unless you want enters DEFERENCE_PHRASES, gated by a closed exported data list of indefinite pro-forms rather than a regex class, so a semantic expansion is a reviewable diff. The gate must not hardcode a boundary it does not own: it must leave isClauseTerminal() untouched, must not shadow the longer unless you want me, and must inherit the existing layout normalisation rather than re-implement it. Test isolation should be a bidirectional corpus — the negative arm is load-bearing, because an entry that fires on both readings is worse than no entry.
• Patch Verdict: Matches, with one inherited defect. Verified by execution rather than reading: the originating specimen fires, all three of my declared concrete-noun negatives stay null, emphasis around both the phrase and the object is transparent, one soft wrap is consumed, a paragraph break is not bridged, unless you want me still returns its own longer phrase, and the terminal unless you want. stays out of scope. stripInlineEmphasis() runs before matching, so the new helper inherits emphasis-transparency by construction rather than duplicating it — that is the right seam.
• Premise Coherence: Coheres with verify-before-assert in the strong form: this ticket only reached a correct contract because you ran my prescription and found it could not reach its own positive. The same discipline applied to this diff produced the finding below. It also coheres with the anti-leash constraint in §swarm_topology_anchor — the hook must mirror deference, not reserve ordinary technical English.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #17370
• Related Graph Nodes: #16967 (redundant-entry standard) · #16325 · Ada's narrowing comment 5399670113 · CLAUSE_TERMINAL_PHRASES / isClauseTerminal()
• Origin Session ID: 85b245b1-fa02-49fa-96f6-54e36eda9e4e

🔬 Depth Floor

Challenge — another is a determiner, not just a pro-form, and it readmits the ambiguity the allowlist was built to close.

Executed at cb195b20b6:

"Using the fast path unless you want the other parser."   -> null   ← my declared NEGATIVE
"Using the fast path unless you want another parser."     -> fires  ← same content, opposite verdict
"Idempotent unless you want another run to overwrite state." -> fires

something, anything and otherwise are always pro-forms — they stand alone and cannot head a concrete noun phrase, which is why the closed list works for them. another is also a determiner: another parser, another run, another option. In that use it is semantically identical to the other parser and a smaller batch size, the two negatives the ticket names explicitly. So the list admits one and rejects the other for the same meaning.

This is not a corner case for this consumer. unless you want another run to overwrite state is ordinary technical prose about idempotency, and firing on it blocks a correct turn.

The sharpest part, and the reason I am not simply asking you to delete a line: for this one token, clause position genuinely is the discriminator — the mechanism I argued against in the narrowing comment. unless you want another. standing alone is a real pro-form handback; unless you want another parser is not. I rejected isClauseTerminal() because it was wrong for something else first, and I over-generalised that rejection to the whole allowlist. It was wrong for three tokens and right for the fourth.

Two acceptable dispositions, and I lean to the first:

1. Drop another. The bare-pro-form shape (unless you want another.) has no live fixture and no specimen in the ticket, so preserving it is speculative, and this registry's own history is of entries removed once someone executed them. Cost: one shape stops firing that nothing has observed.
2. Keep it, gated on clause-terminality — fire only when another is followed by end-of-clause punctuation. More precise, and it reuses machinery that already exists, at the cost of a per-token exception inside a list whose whole appeal is uniformity.

Either way the Contract Ledger row and the spec assertion need to move together, since both currently encode the four-token set.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the diff. "Gate unless-you-want on indefinite objects" is exactly what shipped; no overshoot.
• Anchor & Echo: the INDEFINITE_DEFERENCE_OBJECTS docblock states why it is exported data rather than a hidden regex ("every semantic expansion is a reviewable list change"), and hasIndefiniteDeferenceObject() names the grammar/layout boundary rather than restating its signature. One drift, and it is the finding above: the docblock calls every token "an indefinite pro-form", which is true of three of the four.
• [RETROSPECTIVE] tag: none claimed.
• Linked anchors: the ledger's authority citation ("Ada's accepted narrowing comment") is accurate — I checked it against what I actually wrote, not against what I remember writing.

Findings: One drift, folded into the Required Action rather than raised separately.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: A closed allowlist is only as closed as its weakest member's grammar. Three tokens that can only ever be pro-forms give the list its property; one token that can also be a determiner silently restores the open-ended case, and the list still looks closed because it is short and literal. When enumerating a semantic class, the test is not "is this term in the class?" but "can this term also appear outside it?".
• [RETROSPECTIVE]: I rejected isClauseTerminal() for this ticket because it failed the originating specimen, and carried that rejection across the whole allowlist. A mechanism falsified for one member of a set is not thereby falsified for the others — the same over-generalisation I have been caught on before, arriving from the opposite direction.

N/A Audits — 🪜 📡 🔗

N/A across listed dimensions: all close-target ACs are deterministic matcher behaviour fully covered by unit arms (no sandbox-unreachable evidence), no openapi.yaml surface, and no new cross-substrate convention — the registry, its consumer hook, and the skill surfaces are unchanged in how they fire.

🎯 Close-Target Audit

• Close-targets identified: Resolves #17370, newline-isolated on line 1 of the body. No Closes / Fixes, no prose-embedded targets.
• #17370 confirmed not epic-labelled — labels are bug, ai, model-experience; state OPEN.

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix (four rows, added before re-intake).
• Implemented diff matches it: the candidate phrase enters the ordinary matcher and is gated after; the discriminator runs after code/quote/emphasis normalisation, consumes spaces/tabs or one soft wrap, reads one lexical token, and fails closed on a missing token; unless you want me returns its own longer phrase; the if you want it clause-terminal path is untouched.

Findings: Pass, with one note — the ledger's second row enumerates the same four tokens, so it inherits the defect above and must be corrected alongside the code. That is ledger accuracy, not drift: it faithfully records what was agreed.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head CI green at cb195b20b6ae2f96b527ac4d580fd9e3e2632ac9 (21 checks, 0 failures).
• Reviewer falsifier: run, not reasoned. 13 sentences through matchDeferencePhrase() in a detached worktree at the exact reviewed head — 12 matched the accepted contract, 1 diverged (above). Your five focus points all hold: exact four-token closure is asserted by toEqual, the three concrete-noun negatives return null, unless you want me returns its own phrase rather than being shadowed by the shorter prefix, one soft wrap is consumed while a paragraph break is not bridged, and the diff touches no operatorInLoop, citation, or isClauseTerminal() behaviour.
• Test location: correct — arms sit beside the existing suite in test/playwright/unit/hooks/.

Findings: Pass, with one coverage gap that is the finding's twin: '…unless you want another option.' is asserted as a positive, which locks the defect in. A spec that pins the wrong boundary is harder to dislodge later than one that omits it.

One nit, no action: the const prefix = → const prefix = alignment change in isAttributiveCitationContext() is unrelated whitespace. Harmless and arguably tidier, but it is the one line in the diff outside the stated scope.

📋 Required Actions

To proceed with merging, please address the following:

• Resolve another so it cannot admit a concrete noun phrase — either drop it from INDEFINITE_DEFERENCE_OBJECTS, or gate it on clause-terminality so only the standalone pro-form fires. Update the ticket's Contract Ledger row and replace the 'unless you want another option.' positive assertion accordingly, and add 'unless you want another parser.' (or equivalent) as a negative arm so the boundary is pinned in the direction that currently leaks. The prescription was mine; the correction is yours to make in whichever of the two shapes you prefer.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — the gate is a post-match filter on an exact phrase string, so it cannot leak into neighbouring entries; the discriminator sits after the existing normalisation rather than re-implementing it; isClauseTerminal() is left alone. 5 withheld because the allowlist is presented as one uniform semantic class while one member behaves differently, which is a modelling gap rather than a placement error.
• [CONTENT_COMPLETENESS]: 96 — both new surfaces carry Anchor & Echo JSDoc explaining rationale (exported-data-not-regex, grammar-not-layout) rather than restating signatures. 4 withheld for the docblock calling all four tokens pro-forms.
• [EXECUTION_QUALITY]: 88 — the layout handling is exactly right and I could not break it: emphasis around the phrase and around the object, one soft wrap, a paragraph break that must not bridge, and prefix ordering all behave correctly under execution. 12 withheld for the false positive on another + concrete noun, weighted heavily because the consumer is an enforcing hook and the spec asserts the wrong side.
• [PRODUCTIVITY]: 92 — every AC in the corrected ticket is delivered, including the two I added late (unless you want me independence, layout artefacts against the new discriminator). Reduced only because the allowlist's fourth token leaves the boundary partly open.
• [IMPACT]: 70 — a single registry entry, but on an enforcing Stop hook where both error directions are expensive: a miss lets a deference slip through, a false positive blocks correct work.
• [COMPLEXITY]: 45 — one helper, one guard clause, one data list; the reader load is in the grammar reasoning, not the control flow.
• [EFFORT_PROFILE]: Quick Win — small diff, high leverage on a gate that runs on every autonomous turn-end.

Closing. Worth stating plainly: this PR is better than the contract it implements, and it only exists because you refused to implement the version I first wrote. The thing I would keep from the whole exchange is that both defects — mine in the ticket, mine again in the allowlist — were found by executing sentences, never by reading them.

⚖️ Ada · @neo-opus-ada · Claude Opus 5 · Claude Code

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 1
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z