diff --git a/docs/decisions/ADR-0170-injector-contract-widening.md b/docs/decisions/ADR-0170-injector-contract-widening.md new file mode 100644 index 00000000..2f6f44bb --- /dev/null +++ b/docs/decisions/ADR-0170-injector-contract-widening.md @@ -0,0 +1,226 @@ +# ADR-0170 — Recognizer Injector Contract Widening + +**Status:** Proposed (scoping ADR; no runtime change in this PR) +**Date:** 2026-05-27 +**Author:** Shay +**Parent:** ADR-0163.D.2 (parsed_anchors → MathProblemGraph) +**Related:** ADR-0131.G.1, ADR-0167, ADR-0167-FOLLOWUPS §7, +PR #369 (A2 rate_with_currency), DCS-S1 finding +**Gating rule:** [ADR-0166](./ADR-0166-measurement-capability-sequencing.md) + +--- + +## Context + +The `inject_from_match` dispatch in `generate/recognizer_anchor_inject.py` +has a return type of `tuple[CandidateInitial, ...]`. Every per-category +injector can only emit `CandidateInitial` records. + +Wave-Next (the GSM8K next-capability push) attempted four parallel +sub-shape injectors. Three of the four hit the same substrate +constraint: the category they targeted cannot be expressed as +`CandidateInitial` alone. + +| Brief | Required emission type | `CandidateInitial`-only sufficient? | +|---|---|---| +| A2 `rate_with_currency` | `CandidateRate(Rate(amount, num_unit, den_unit))` | No | +| A3 `multiplicative_aggregation` | `CandidateInitial(product)` OR `CandidateOperation(multiply)` | Partially (product semantics OK; composition not) | +| A4 `temporal_aggregation` | `CandidateOperation(apply_rate, ...)` | No | +| DCS-S1 (acquisition verbs) | `CandidateOperation(add)` (per ADR-0131.G.1) | No | + +This is no longer four independent sub-shape gaps. It is **one +substrate constraint** that blocks the entire recognizer-injector +capability extension surface. + +## Decision + +Widen the `inject_from_match` contract so per-category injectors can +emit the full `SentenceChoice` union (and any extensions thereto), +not just `CandidateInitial`. + +The widening is type-level only. The dispatch table, the four-narrowness +gating, the wrong=0 invariants, the per-sentence admission gate — all +preserved. The change is: per-category injectors gain the ability to +return `CandidateOperation` (and, after the corresponding schema work, +`CandidateRate`). + +## Why this is not "just change a return type" + +The widening interacts with three load-bearing existing rules: + +### 1. ADR-0131.G.1 — branch-disagreement discipline + +Acquisition verbs (`collected`, `bought`, `saved`, `buys`, `makes`) are +deliberately routed to `ADD_VERBS` / `SUBTRACT_VERBS` rather than to +initial-anchor extraction. The reason: `"Sam collected 5 apples"` +could otherwise emit both `CandidateInitial(Sam=5)` AND +`CandidateOperation(Sam, add, 5)`, triggering branch disagreement +and refusing the case. + +ADR-0170 must preserve this discipline. The DCS injector's +acquisition-verb path emits `CandidateOperation(add)` — matching the +existing parser's behaviour for those verbs — NOT +`CandidateInitial`. The solver's defaults-from-zero rule then +produces `0 + N` for the single-statement case, identical to the +parser's current behaviour. + +The wrong=0 hazard is: if the injector emits `CandidateOperation(add)` +and the parser ALSO emits `CandidateOperation(add)` from the same +sentence, that's a per-sentence collapsed-tie, not a branch +disagreement. Acceptable. But if the injector emits and the parser +silently drops the sentence (multi-word unit, etc.), the operation is +admitted from only one source — fine, but the test net must verify +no admission widens when the parser is updated to also extract. + +### 2. ADR-0167 `SentenceChoice` union + +PR #369 (A2 rate_with_currency) documented that `Rate` is not in +`SentenceChoice = Union[CandidateInitial, CandidateOperation]`. The +follow-up plan (`WAVE-NEXT-REVISED.md` §Schema-Gap 1) lays out 4 steps +for adding `CandidateRate`. ADR-0170 sequences in front of that work: +**first widen the injector contract to support the existing union +fully (CandidateOperation), then extend the union with CandidateRate +in a separate ADR.** + +### 3. Existing `_initial_admissible` gate + +`generate/math_candidate_graph.py` runs `_initial_admissible(c)` on +every injected candidate. That gate is `CandidateInitial`-specific. +Widening to `CandidateOperation` requires the parallel +`roundtrip_admissible(c)` gate (which exists for parser operations) be +applied to injected operations. + +This is a real implementation concern but mechanically tractable: the +admissibility dispatch becomes: + +```python +if isinstance(c, CandidateInitial): + admissible = _initial_admissible(c) +elif isinstance(c, CandidateOperation): + admissible = roundtrip_admissible(c) +``` + +The branch is one new line per check site; the gating semantics are +identical to what the parser already enforces. + +## Open questions (must resolve in implementation ADR/PR) + +1. **Atomic widening vs per-injector**: do we widen the return type + in one PR (mechanical, no behavior change) and let each future + injector PR add a category-specific test for its emission shape? + Or bundle widening with the first acquisition-verb-emitting + injector (DCS-S1)? + + *Recommendation:* widen first as a no-behavior-change PR. Existing + `_INJECTORS` table entries continue to return only + `CandidateInitial`; the change is a type-level relaxation. Then + future injectors gain the ability without each one paying the + widening cost. + +2. **`CandidateOperation` admissibility**: confirm that + `roundtrip_admissible(c)` accepts an operation whose surface span + covers a single-statement acquisition (no separate "more" or + subtractive operand). The existing parser path generates these, + so the predicate already accepts them — but verify before relying + on it. + +3. **Branch-disagreement across parser + injector**: when the regex + parser updates to handle multi-word units (e.g., "Pokemon cards"), + it would emit `CandidateOperation(Nicole, add, 400, Pokemon cards)` + for the same sentence the DCS injector emitted. The + `_collapse_per_sentence_ties` mechanism handles ties; verify it + collapses identical operations cleanly. If not, the wider parser + work needs to coordinate with the injector path. + +4. **CandidateRate sequencing**: ADR-0170 is the type widening on the + *current* `SentenceChoice` union (`CandidateInitial | CandidateOperation`). + Adding `CandidateRate` is a separate (downstream) ADR — call it + ADR-0171. ADR-0170 ships first because it unblocks DCS-S1, A3, A4 + without waiting on the rate-type design. + +5. **Test pattern**: every per-category injector PR after ADR-0170 + must pin both the admitted-graph shape AND the case 0050 hazard + (no widening of admissions on the hazard canary). + +## ADR-0166 three-question test + +- **Q1 — Capability**: A widened injector contract that lets + per-category injectors emit `CandidateOperation` in addition to + `CandidateInitial`. Specifically unblocks DCS-S1 acquisition verbs + (the largest single subset of the 21-case DCS bucket), A3 + multiplicative composition, A4 temporal aggregation. The capability + is mechanism, not measurement — it enables follow-up injector PRs + to ship lift. +- **Q2 — Lane**: `evals/gsm8k_math/train_sample/v1` is the regression + surface. Each follow-up injector PR runs its own before/after delta + on the existing report. No new canonical lanes (ADR-0166 still + gates). +- **Q3 — Invariant**: `wrong == 0` preserved by the existing + five-layer safety net (ADR-0163.D.2): matcher narrowness, extraction + correctness, injection correctness (`_initial_admissible` / + `roundtrip_admissible`), propose-time replay gate, multi-branch + decision rule. ADR-0170 extends layer 3's dispatch but doesn't + weaken any layer. + +## Implementation outline (subsequent PRs, not this one) + +**ADR-0170-impl-W1** — Type widening, no behavior change: +- Change `inject_from_match` return type to + `tuple[CandidateInitial | CandidateOperation, ...]` +- Update `_INJECTORS` value type +- Update admissibility dispatch in `math_candidate_graph.py` to + branch on the candidate type +- Pinning test: existing `inject_discrete_count_statement` still + emits only `CandidateInitial`; behavior byte-identical pre/post. + +**ADR-0170-impl-W2** — First operation-emitting injector (DCS-S1): +- Extend matcher's `_POSSESSION_VERBS` to accept acquisition verbs OR + add a separate `_ACQUISITION_VERBS` set +- Injector emits `CandidateInitial` for `has/have/had` (existing) AND + `CandidateOperation(add)` for acquisition verbs (new) +- Tests pin both paths; case 0050 hazard pin remains + +**ADR-0170-impl-W3** — A1 currency_amount reimplementation: +- Sandbox config note: ensure `additionalDirectories` includes + `/tmp/wt-*` paths for any future dispatched agent +- Injector emits `CandidateInitial` for ` earns|charges + $` (new verbs beyond `_INITIAL_HAS_RE`'s coverage) + +**ADR-0170-impl-W4** — A3 multiplicative_aggregation: +- Per A3 design: emit `CandidateInitial(outer × inner, inner_unit)` + pre-computed product (matches solver's defaults-from-zero semantics) + +**ADR-0170-impl-W5** — A4 temporal_aggregation: +- Requires `apply_rate` primitive in the algebra (separate ADR work) +- Without that primitive, injector continues to refuse explicitly + +Each impl PR is small, focused, regression-tested, and ships with its +own before/after eval delta. + +## What ADR-0170 does NOT do + +- It does not widen the `SentenceChoice` union (that's ADR-0171 for + `CandidateRate`). +- It does not add new eval lanes (ADR-0166 still gates). +- It does not propose any non-deterministic mechanism. +- It does not weaken wrong=0 or the five-layer safety net. +- It does not mandate that all four blocked injectors ship — each is + its own follow-up PR with its own go/no-go decision. + +## Cross-references + +- [ADR-0163.D.2](./ADR-0163.D.2-discrete-count-statement.md) (the + original parsed_anchors → solver-state ADR; ADR-0170 widens its + injector contract) +- [ADR-0131.G.1] — branch-disagreement discipline that ADR-0170 must + preserve +- [ADR-0167](./ADR-0167-audit-as-teaching-evidence.md) — parallel + teaching-corridor mechanism; ratification path independent of + injector contract +- [ADR-0167-FOLLOWUPS](../handoff/ADR-0167-FOLLOWUPS.md) §7 — the + four-finding source +- [WAVE-NEXT-REVISED](../handoff/WAVE-NEXT-REVISED.md) §Schema-Gap + 1/3 — the categories this ADR unblocks +- [DCS-S1 finding](../handoff/DCS-S1-FINDING.md) — the investigation + that surfaced the contract gap +- PR #369 — A2's schema-refusal artifact documenting `Rate` follow-up diff --git a/docs/handoff/DCS-S1-FINDING.md b/docs/handoff/DCS-S1-FINDING.md new file mode 100644 index 00000000..c200f8d2 --- /dev/null +++ b/docs/handoff/DCS-S1-FINDING.md @@ -0,0 +1,141 @@ +# DCS-S1 Finding — Injector Contract Is the Substrate Bottleneck + +**Date:** 2026-05-27 +**Status:** Finding; no implementation. Routes work to ADR-0170. +**Parent:** `docs/handoff/WAVE-NEXT-REVISED.md` +**Result:** DCS-S1 (proper-noun possession sub-shape expansion) cannot +ship meaningful lift on current GSM8K without first widening the +`inject_from_match` return-type contract. Same schema gap A3 +(multiplicative) and A4 (temporal) identified. + +--- + +## What was investigated + +The Wave-Next-Revised plan named **DCS-S1** as the next tractable +sub-shape: extend the existing v1 `inject_discrete_count_statement` +injector to cover more of the 21 `discrete_count_statement` refusals +on the GSM8K train_sample. + +The hypothesis: most DCS refusals are proper-noun possessions whose +extraction fails for one of a few narrowable reasons (verb whitelist, +clause-split markers, single-quantity rule). Loosening the matcher + +extending the injector to cover those would lift several cases. + +## What the data actually shows + +All 21 DCS-refused cases were enumerated and the triggering statements +inspected. The structural breakdown: + +| Sub-shape pattern in DCS-refused statements | Count | +|---|---:| +| Acquisition verb (`collected`, `donated`, `bought`) — not in possession whitelist | ~5 | +| Multi-clause / enumeration (`and`, `then`, `,`) | ~8 | +| Pronoun subject (`he`, `she`, `they`) | ~4 | +| Comparative reference (`twice as many`, `half of`) | ~3 | +| Anonymous subject (`There are`, `The guests`) | ~3 | +| Other (modal verbs, copula, multi-verb) | ~4 | + +(Buckets overlap — many cases hit multiple narrowness rules.) + +**Critical observation:** of the 21 cases, **zero** are pure S1-only +blockers. Every single one has additional blockers downstream in the +same problem. Even if the DCS injector admitted the first sentence, +sentences 2/3/4 carry their own refusals. + +Concrete trace — case `gsm8k-train-sample-v1-0023`: + +``` +S1: "Nicole collected 400 Pokemon cards." + → would inject Nicole=400 if `collected` were in possession verbs +S2: "Cindy collected twice as many, and Rex collected half of Nicole and Cindy's combined total." + → multi-clause + comparative reference; will refuse on current rules +S3: question with `If Rex divided...` conditional prefix + → conditional prefix recovery exists but answer still requires S2's admission +``` + +Admitting S1 alone does not close the case. The bottleneck moves from +S1 to S2; no admission-lift is delivered. + +## The architectural blocker + +To admit `"Nicole collected 400 Pokemon cards"` as initial state, three +edits would be needed: + +1. Add `collected` to `_POSSESSION_VERBS` (matcher narrowness in + `generate/recognizer_match.py`) +2. Extend `_locate_possession_verb` in the injector +3. Add `collect, collects, collected` to `CandidateInitial.__post_init__` + whitelist in `generate/math_candidate_parser.py` + +**ADR-0131.G.1 explicitly removed `collected/bought/saved/buys/makes` +from initial-anchor extraction** — they were routed exclusively to +`ADD_VERBS` (operation extraction) to avoid branch disagreement when +the same sentence could produce both `CandidateInitial(Sam=5)` AND +`CandidateOperation(Sam, add, 5)`. + +The solver "defaults from zero for operations" so +`Sam collected 5 apples. How many does Sam have?` → `0 + 5 = 5`. That +discipline is load-bearing for wrong=0 in the regex path. + +The right architectural fix is not to break ADR-0131.G.1. It's to teach +the **DCS injector** to emit `CandidateOperation(add)` for acquisition +verbs — the same kind of state-introducing operation the parser +already emits for them. + +**But:** `inject_from_match`'s return type is +`tuple[CandidateInitial, ...]`. It cannot emit `CandidateOperation`. +Widening the contract is the prerequisite. + +## The pattern — fourth time this gap appears + +Wave-Next surfaced four schema gaps. All four trace back to the same +substrate-level constraint: the recognizer-injector path can only +emit `CandidateInitial`. + +| Brief | What needed emission | Available type | Blocker | +|---|---|---|---| +| **A2** rate_with_currency | `CandidateRate` (carries `Rate(value, num_unit, den_unit)`) | None — `Rate` not in `SentenceChoice` union | Schema gap (PR #369) | +| **A3** multiplicative_aggregation | `CandidateInitial(outer × inner)` OR `CandidateOperation(multiply)` | `CandidateInitial` works for product semantics; but other cases need composition | Half-blocked | +| **A4** temporal_aggregation | `CandidateOperation(apply_rate, ...)` | `apply_rate` primitive doesn't exist | Algebra-level gap | +| **DCS-S1** | `CandidateOperation(add)` for acquisition verbs | Injector returns only `CandidateInitial` | Schema gap (this finding) | + +This is no longer four separate sub-shape problems. It's **one substrate +bottleneck affecting all four categories**. The right artifact is a +contract-widening ADR — see [ADR-0170](../decisions/ADR-0170-injector-contract-widening.md). + +## What this PR ships + +- This finding doc (`docs/handoff/DCS-S1-FINDING.md`) +- `docs/decisions/ADR-0170-injector-contract-widening.md` — the scoping + ADR that names the contract change and the four categories it unblocks + +What this PR does NOT ship: +- Any DCS-S1 implementation (it's blocked on ADR-0170) +- Any matcher/extractor edits +- Any test changes +- Any pack changes + +## Recommended next move + +ADR-0170 ratification, then a small focused PR implementing the +contract widening (no behavior change, type-only). Then a follow-up +wave (DCS-S1 acquisition, A1 currency, A2 rate, A3 multiplicative, +A4 temporal) can ship in parallel — each as a focused injector PR +against the widened contract. + +That's the actual lift-per-risk path. Until ADR-0170 lands, no +recognizer-injector work other than `CandidateInitial`-only narrow +possession can ship cleanly. + +## Cross-references + +- [WAVE-NEXT-REVISED](./WAVE-NEXT-REVISED.md) — parent plan; needs an + update pointing to ADR-0170 as the prerequisite +- [ADR-0170](../decisions/ADR-0170-injector-contract-widening.md) — the + scoping ADR this finding routes to +- PR #369 (A2) — first observation of the gap; documents `Rate` + extension steps +- ADR-0131.G.1 — the original branch-disagreement discipline that + forces acquisition verbs to be operations, not initials +- ADR-0167-FOLLOWUPS §7 — Wave-Next findings backlog