diff --git a/docs/decisions/ADR-0184-distinct-unit-product-rule.md b/docs/decisions/ADR-0184-distinct-unit-product-rule.md index 3aaf6f0c..0cca35eb 100644 --- a/docs/decisions/ADR-0184-distinct-unit-product-rule.md +++ b/docs/decisions/ADR-0184-distinct-unit-product-rule.md @@ -1,6 +1,10 @@ # ADR-0184 — Distinct-unit product rule: cut the product-of-all over-commit (the first lever measured against real GSM8K) -**Status:** Proposed (spec only — no code). Follow-on to +**Status:** Accepted / Implemented. Implementation refined the mechanism: the rule +is a **commit-ineligibility downgrade in `classify_derivation`** (§3), *not* the +shared unit-clause failure §3 originally proposed — putting it in the shared gate +regressed the disagreement refusals ADR-0182 relies on (see §3.1, the finding). +Follow-on to [ADR-0175](./ADR-0175-calibrated-attempt-and-eliminate-learning.md) (the self-verification gate), [ADR-0182](./ADR-0182-cross-composer-disagreement-pooling.md) (pooling), and [ADR-0177](./ADR-0177-cue-precision-learning.md) (cue precision — the @@ -62,23 +66,34 @@ loss**. "Span >1 clause" cuts more but destroys `0003` (a *correct* 3-sentence rate-chain `48 boxes × 24 erasers/box × $0.75`); "drop all products" loses both correct answers. Coverage is never bought at the cost of a correct answer. -## 3. The rule +## 3. The rule (as implemented) -Refine the gate's **unit-consistency clause** (`verify.py` clause 3). Today: +The dimensional predicate (`_is_repeated_unit_product`): a **pure multiplicative +chain** (every step `multiply`) whose operands — including the start — repeat a +**non-empty** unit forms `unit²` (`apples × apples`, `cards × cards`), which is +almost never the answer to a word problem asking for that unit. Empty-unit operands +are exempt (an unknown unit cannot be shown to collide — and the *correct* `0003` +multiplies a blank-unit `0.75`). Divide is exempt — same-unit division (`feet / +feet`) is a legitimate dimensionless count. Dimensional, not lexical (ADR-0165-safe). -- `add`/`subtract` require a shared unit; -- `multiply`/`divide` may compose across **any** units. +Such a product is **classified `exempt` (commit-ineligible) in +`classify_derivation`**, *not* failed in the shared `_base_reasons` gate. It stays in +the pool — it may force a disagreement refusal — but can never resolve as the answer. +A unique repeated-unit product is therefore `exempt`-only → refuse (the `0042`-class +cut); a distinct-unit product stays `complete` → commit (`0003`/`0021`). -New: +### 3.1 Finding — why a shared unit-clause failure was wrong -- `multiply`/`divide` may compose across **distinct** units, but a `multiply` step - whose operand carries a **non-empty unit already present** among the product's - prior operands is a **unit-incoherence** failure (it forms `unit²`, which is - almost never the answer to a word problem asking for that unit). - -Empty-unit operands are exempt (an unknown unit cannot be shown to collide — and the -*correct* `0003` multiplies a blank-unit `0.75`). The rule is dimensional, not -lexical: it names a property of the composed quantities, not a cue (ADR-0165-safe). +The first implementation put the predicate in the shared unit-consistency clause, so +a repeated-unit product **failed** (was dropped from the pool entirely). That cut the +5 `train_sample` wrongs **but regressed the confuser probe `1 → 3`**: the +disguised-polarity cases `0001`/`0003` refuse only because the `coins × coins` product +*disagrees* with the `coins + coins` accumulation reading. Dropping the product left +the (wrong) additive reading unique → it committed (`0001 → 80`, `0003 → 30`). The fix +is the **downgrade**: keep the product as a commit-ineligible `exempt` candidate so it +still forces the disagreement, but can never commit. (This is the same commit- +ineligibility concept ADR-0182 introduced.) Pinned by +`test_downgrade_not_removal_preserves_disagreement_refusal`. ### Why this keeps the correct products @@ -98,8 +113,10 @@ mis-read as `4×20×6×25`) — never a real quantity. 1. **`train_sample` (real, the metric):** sealed-reader `wrong 13 → 8`, `correct` held at 2 (0003/0021). Failing-under-violation: a test that the 5 named cases (0001/0017/0042/0045/0048) refuse, and 0003/0021 still commit. -2. **Confuser probe:** `wrong` stays 0, positives still solve (the rule must not make - any curated product-positive refuse). Re-run the probe. +2. **Confuser probe:** `wrong` must not regress and positives still solve. The + §3.1 finding makes this load-bearing: the naive shared-clause version regressed + it `1 → 3`; the downgrade holds it at baseline (`1` on main pre-#481; `0` once + anchor-skip lands). `0001`/`0003` must stay **refused**, not commit `80`/`30`. 3. **Practice search lane:** `search_runner` commits products; confirm `wrong=0` and `correct` not reduced (the rule may only *add* refusals there). 4. **Serving frozen:** `verify` is not on the serving path (`chat/` → @@ -110,11 +127,14 @@ mis-read as `4×20×6×25`) — never a real quantity. ## 5. Where it lives -`generate/derivation/verify.py`, inside the unit-consistency clause shared by -`self_verifies` and `classify_derivation` (so the commit gate and the pool's -classifier cannot drift). Sealed lane only. A focused -`tests/test_adr_0184_distinct_unit_product.py` pins the dimensional rule and the -5-cut / 2-kept obligation. +`generate/derivation/verify.py`: `_is_repeated_unit_product` (the dimensional +predicate) consulted by `classify_derivation`, which downgrades an otherwise- +`complete` repeated-unit product to `exempt`. `self_verifies` / `_base_reasons` are +**unchanged** — so the search lanes (practice) and serving stay byte-identical, and +the change is confined to the pool's commit-eligibility (where the over-commit lives). +Sealed lane only. `tests/test_adr_0184_distinct_unit_product.py` pins the predicate, +the downgrade, the `0042`-class refusal, and the §3.1 downgrade-not-removal +regression guard. ## 6. What this does NOT fix — the honest hand-off to cue precision diff --git a/generate/derivation/verify.py b/generate/derivation/verify.py index 97f51dbf..006b2226 100644 --- a/generate/derivation/verify.py +++ b/generate/derivation/verify.py @@ -116,6 +116,24 @@ def self_verifies(derivation: GroundedDerivation, problem_text: str) -> SelfVeri return SelfVerification(verified=not reasons, reasons=tuple(reasons)) +def _is_repeated_unit_product(derivation: GroundedDerivation) -> bool: + """ADR-0184 — a *pure* multiplicative product that revisits a non-empty + dimension (``apples × apples``, ``cards × cards``), forming ``unit²``. + + A genuine rate-chain composes **distinct** dimensions (``boxes × erasers/box × + $/eraser``); a product that repeats a dimension is multiplying independent groups + (``4 bags×20 + 6 bags×25`` mis-read as ``4×20×6×25``) — never a real quantity. + Empty units are exempt (an unknown dimension cannot be shown to collide, and a + correct rate-chain may carry a blank-unit scalar like ``$0.75``). Divide is + exempt — same-unit division (``feet / feet``) is a legitimate dimensionless count. + Dimensional, not lexical (ADR-0165-safe).""" + if not derivation.steps or not all(step.op == "multiply" for step in derivation.steps): + return False + units = [derivation.start.unit, *(step.operand.unit for step in derivation.steps)] + non_empty = [unit for unit in units if unit] + return len(non_empty) != len(set(non_empty)) + + def classify_derivation(derivation: GroundedDerivation, problem_text: str) -> str | None: """ADR-0182 — the commit-eligibility class of a derivation, for pooling. @@ -123,14 +141,17 @@ def classify_derivation(derivation: GroundedDerivation, problem_text: str) -> st * ``"complete"`` — passes every clause *including* full completeness; **commit-eligible** (may resolve as an answer). - * ``"exempt"`` — passes every base clause and the only unused quantities are - **isolated-foreign** (unit non-empty ∧ equal to no *used* operand's unit, i.e. - a candidate distractor standing alone in a dimension the reading never - touches); **commit-INELIGIBLE**. It exists only to enter the pool and force a - disagreement → refusal, never to be committed alone (see :mod:`generate. - derivation.pool`). This keeps the commit-path completeness guarantee - (ADR-0175's multi-step-incomplete defence) intact — the exemption widens only - what can *refuse*. + * ``"exempt"`` — **commit-INELIGIBLE**: it may enter the pool and force a + disagreement → refusal, but never resolve as the answer alone. Two ways to + earn it: (ADR-0182) the only unused quantities are **isolated-foreign** + (a candidate distractor standing alone in a dimension the reading never + touches); or (ADR-0184) the derivation is a **repeated-unit product** + (``unit²`` — dimensionally impossible as the answer, but still a real reading + that should *disagree* with an additive rival, e.g. ``coins × coins`` vs + ``coins + coins`` on a disguised-polarity confuser). Keeping it commit- + ineligible — rather than dropping it — preserves the disagreement refusals + ADR-0182 relies on; dropping it would unmask the additive reading as a unique + (wrong) commit. * ``None`` — fails a base clause, or an unused quantity is not isolated-foreign (empty unit, or a unit shared with a used operand → real signal the reading dropped). @@ -138,9 +159,12 @@ def classify_derivation(derivation: GroundedDerivation, problem_text: str) -> st tokens = _tokens(problem_text) if _base_reasons(derivation, tokens): return None + repeated_unit_product = _is_repeated_unit_product(derivation) unused = _unused_quantities(derivation, problem_text) if not unused: - return "complete" + # ADR-0184: a dimensionally-impossible product is commit-ineligible (exempt), + # not commit-eligible — but it stays in the pool to force disagreement. + return "exempt" if repeated_unit_product else "complete" used_units = {derivation.start.unit, *(step.operand.unit for step in derivation.steps)} units_by_token: dict[str, set[str]] = {} diff --git a/tests/test_adr_0184_distinct_unit_product.py b/tests/test_adr_0184_distinct_unit_product.py new file mode 100644 index 00000000..48a37a52 --- /dev/null +++ b/tests/test_adr_0184_distinct_unit_product.py @@ -0,0 +1,121 @@ +"""ADR-0184 — distinct-unit product rule. + +A pure multiplicative product that revisits a non-empty dimension (``unit²``: +``apples × apples``, ``cards × cards``) is the over-eager product-of-all multiplying +independent groups — never a real quantity. It is classified **commit-ineligible** +(``exempt``), *not removed*: it still enters the pool to force a disagreement refusal +(the disguised-polarity confusers depend on the ``coins × coins`` product disagreeing +with the ``coins + coins`` accumulation reading). Dropping it instead of downgrading +would unmask the additive reading as a unique wrong commit — that regression is pinned +below. + +Sealed lane: ``chat/`` does not import these; serving ``3/47/0`` cannot move. +""" + +from __future__ import annotations + +from generate.derivation.model import GroundedDerivation, Quantity, Step +from generate.derivation.pool import pooled_candidates, resolve_pooled +from generate.derivation.verify import _is_repeated_unit_product, classify_derivation + + +def _q(v: float, unit: str, tok: str) -> Quantity: + return Quantity(value=v, unit=unit, source_token=tok) + + +class TestIsRepeatedUnitProduct: + def test_same_unit_multiply_is_repeated(self) -> None: + d = GroundedDerivation( + start=_q(4, "apples", "4"), + steps=(Step(op="multiply", operand=_q(5, "apples", "5"), cue="each"),), + ) + assert _is_repeated_unit_product(d) is True + + def test_distinct_units_multiply_is_not_repeated(self) -> None: + # boxes × erasers — a genuine rate-chain, distinct dimensions. + d = GroundedDerivation( + start=_q(48, "boxes", "48"), + steps=(Step(op="multiply", operand=_q(24, "erasers", "24"), cue="each"),), + ) + assert _is_repeated_unit_product(d) is False + + def test_empty_unit_repeat_is_exempt(self) -> None: + # blank units cannot be shown to collide (a correct rate-chain may carry a + # blank-unit scalar like $0.75). + d = GroundedDerivation( + start=_q(48, "boxes", "48"), + steps=( + Step(op="multiply", operand=_q(24, "erasers", "24"), cue="each"), + Step(op="multiply", operand=_q(0.75, "", "0.75"), cue="each"), + ), + ) + assert _is_repeated_unit_product(d) is False + + def test_same_unit_divide_is_exempt(self) -> None: + # feet / feet is a legitimate dimensionless count, not unit². + d = GroundedDerivation( + start=_q(1000, "feet", "1000"), + steps=(Step(op="divide", operand=_q(25, "feet", "25"), cue="per"),), + ) + assert _is_repeated_unit_product(d) is False + + def test_additive_chain_is_not_a_product(self) -> None: + d = GroundedDerivation( + start=_q(20, "apples", "20"), + steps=(Step(op="add", operand=_q(20, "apples", "5"), cue="more"),), + ) + assert _is_repeated_unit_product(d) is False + + +class TestClassifyDowngrade: + def test_repeated_unit_product_is_exempt_not_complete(self) -> None: + text = "There are 4 bags with 20 apples each and 6 bags with 25 apples each." + d = GroundedDerivation( + start=_q(4, "bags", "4"), + steps=( + Step(op="multiply", operand=_q(20, "apples", "20"), cue="each"), + Step(op="multiply", operand=_q(6, "bags", "6"), cue="each"), + Step(op="multiply", operand=_q(25, "apples", "25"), cue="each"), + ), + ) + # complete (uses all 4 quantities) but dimensionally impossible -> exempt. + assert classify_derivation(d, text) == "exempt" + + def test_distinct_unit_product_stays_complete(self) -> None: + text = "There are 48 boxes with 24 erasers each." + d = GroundedDerivation( + start=_q(48, "boxes", "48"), + steps=(Step(op="multiply", operand=_q(24, "erasers", "24"), cue="each"),), + ) + assert classify_derivation(d, text) == "complete" + + +class TestPoolBehaviour: + _PRODUCT_OF_ALL_0042 = ( + "Ella has 4 bags with 20 apples in each bag and six bags with 25 apples in " + "each bag. If Ella sells 200 apples, how many apples does Ella have left?" + ) + # confuser 0001 — the regression guard pair. + _DISGUISED = "Dan has 50 coins. He buys a toy for 30 coins. How many coins does Dan have left?" + + def test_unopposed_repeated_unit_product_refuses(self) -> None: + # 0042 used to commit 2,400,000 (4×20×6×25); now the product is exempt and + # (being commit-ineligible and otherwise unopposed) the pool refuses. + assert resolve_pooled(self._PRODUCT_OF_ALL_0042) is None + + def test_downgrade_not_removal_preserves_disagreement_refusal(self) -> None: + # THE regression guard. The disguised-polarity case refuses because the + # coins×coins product (exempt) DISAGREES with the coins+coins accumulation + # reading (complete). If the rule *dropped* the repeated-unit product instead + # of downgrading it, the additive reading would be unique and commit (wrong). + cands = [ + (classify_derivation(d, self._DISGUISED), round(d.answer, 6)) + for d in pooled_candidates(self._DISGUISED) + ] + # both a commit-eligible additive AND a commit-ineligible product are present: + assert ("complete", 80.0) in cands, f"expected additive 80 (complete): {cands}" + assert any(k == "exempt" for k, a in cands if a == 1500.0), ( + f"expected the coins×coins product (1500) to be exempt, not dropped: {cands}" + ) + # and therefore the case refuses (disagreement), never commits 80: + assert resolve_pooled(self._DISGUISED) is None