diff --git a/docs/decisions/ADR-0123-parser-comparison-phrasing.md b/docs/decisions/ADR-0123-parser-comparison-phrasing.md new file mode 100644 index 00000000..59d73180 --- /dev/null +++ b/docs/decisions/ADR-0123-parser-comparison-phrasing.md @@ -0,0 +1,502 @@ +# ADR-0123 — Comparison-Phrasing Realizer (surface increment on the ADR-0123 substrate) + +**Status:** Accepted (surface increment; substrate landed in PR #155) +**Date:** 2026-05-23 +**Author:** CORE agents + reviewers +**Depends on:** +- ADR-0115 (parser substrate), +- ADR-0116 (deterministic solver), +- ADR-0117 (`SolutionTrace` verifier), +- ADR-0118 (stepped realizer), +- ADR-0119 (+ all 8 sub-phases), +- ADR-0121 (math `expert` promotion deferred), +- the ADR-0123 **substrate commit** (`feat/adr-0123-substrate`, commit + `c9bd5d4`): `Comparison` dataclass + `compare_additive` / + `compare_multiplicative` operation kinds + parser patterns + + solver / verifier wiring + `en_arithmetic_v1:compare_additive` + (en-arith-006) + `en_arithmetic_v1:compare_multiplicative` + (en-arith-007) pack lemmas. +**Supersedes:** none + +> Disambiguation: this is **ADR-0123-parser-comparison-phrasing**, the +> parser-arc ADR. It is distinct from +> `ADR-0123-symbolic-logic-shape-remap.md` (the lane-shape governance +> ADR that happens to share the number). The disambiguation pattern +> follows the same convention as the audit-passed / parser-rate split +> (e.g., `ADR-0122-systems-software-audit-passed-deferred.md` vs the +> parser-rate work pinned in a parallel PR). Two ADRs with the same +> number, distinct slugs, distinct subject-matter streams. + +--- + +## Context + +ADR-0121 deferred the first `expert` promotion with named blocker +`correct_rate = 0/1319` on sealed GSM8K. ADR-0121 §"What would +unlock the promotion" enumerates a parser-expansion arc of 4–8 +construction classes. **Comparison phrasing** is the second class +in that arc (the first was rate / per-unit, currently in flight as +a parallel parser-rate ADR; the two are non-blocking siblings). + +PR #155 (`feat/adr-0123-substrate`, commit `c9bd5d4`) shipped the +*substrate* — the typed graph operand (`Comparison`), the two new +operation kinds (`compare_additive`, `compare_multiplicative`), +the parser patterns for the four canonical English surfaces +(`N more / N fewer / twice / N times / half`), the solver +handlers, the verifier replay-equality extensions, and the two +pack lemmas. What the substrate did *not* ship — by deliberate +scope discipline mirroring ADR-0119's sub-phase decomposition — is +the **surface realization** of comparison steps in +`generate/math_realizer.py`. Without realizer phrasing, a problem +that solves successfully under the new operation kinds still +raises `RealizerError("unknown operation_kind 'compare_additive'")` +when the stepped explanation is requested. + +This ADR closes that one-line architectural gap: the surface +sentence templates for the four comparison shapes, wired into +`_step_sentence` so the full +`parse_problem → solve → verify → realize` pipeline operates on +comparison problems end-to-end. + +--- + +## Decision + +Extend `generate/math_realizer.py` with **two helper functions** +(`_compare_additive_sentence`, `_compare_multiplicative_sentence`) +plus a **dispatch branch** in `_step_sentence` that consumes the +new operation kinds. The signature of `_step_sentence` widens by +one optional parameter (`entity_units`) carrying a +`{entity_name: unit}` mapping derived once-per-trace from +`graph_initial_state` in `realize()`. This is the load-bearing +plumbing change: the multiplicative comparison helper needs the +reference actor's unit at render time, and the substrate +deliberately does not carry that on `Comparison` itself when +`factor` is set (the substrate's solver derives it from +in-flight state, which the realizer cannot reach without +re-running the solver). + +### Realizer-level additions (`generate/math_realizer.py`) + +1. **`_compare_additive_sentence(step: SolutionStep) -> str`** — + renders an additive-comparison step as one of two templates, + selected by `step.operand.direction`: + + ``` + direction='more': + " has more than , + giving a total of ." + + direction='fewer': + " has fewer than , + leaving with a total of ." + ``` + + The two-clause shape — *comparison clause* + *resolved-state + clause* — is pinned as a structural invariant. `delta.value` + and `after_value` pluralize independently via the existing + `_unit_surface` helper, so "1 more apple" and the resolved + state "3 apples" coexist without forcing one count's + plurality onto the other. + +2. **`_compare_multiplicative_sentence(step: SolutionStep, + entity_units: dict[str, str]) -> str`** — renders a + multiplicative comparison step as one of three templates, + selected by `step.operand.direction` and `factor`: + + ``` + direction='times': + " has times as many as , + giving a total of ." + + direction='fraction' (factor == 0.5): + " has half as many as , + giving a total of ." + + direction='fraction' (other factor): + " has as many as , + giving a total of ." + ``` + + `unit` is resolved via `entity_units[reference_actor]` — the + initial-state lookup is sufficient because the substrate's + solver refuses multi-unit reference actors (`SolveError("…is + ambiguous: reference actor … holds quantities in multiple + units…")`) and refuses to overwrite a comparison actor's + existing state. Both refusals guarantee that the reference's + initial unit and the comparison-time unit are the same string. + +3. **`_step_sentence` dispatch widened** with two prepended + branches: + + ```python + if step.operation_kind == "compare_additive": + return _compare_additive_sentence(step) + if step.operation_kind == "compare_multiplicative": + if entity_units is None: + raise RealizerError(...) + return _compare_multiplicative_sentence(step, entity_units) + ``` + + The pre-existing `add` / `subtract` / `transfer` / `multiply` + / `divide` branches are unchanged, byte-identically. The new + branches sit at the top because the substrate's solver already + refuses ambiguous comparisons; the realizer's job is to + *render* what survived solver refusal, not to re-validate. + +4. **`realize()` builds `entity_units` once** from + `graph_initial_state`: + + ```python + entity_units = {p.entity: p.quantity.unit for p in graph_initial_state} + ``` + + and threads it through to every `_step_sentence` call. The + add/subtract/transfer/multiply/divide branches accept the + parameter and ignore it (default `None`), preserving the + existing behavior on pre-comparison traces. + +### Refusal discipline (load-bearing) + +The helpers raise `RealizerError` on every shape the substrate +already refuses *plus* one shape the substrate cannot see: + +| Refusal | Substrate or Realizer | +|---|---| +| operand not a `Comparison` | realizer (defensive — substrate guarantees this via `Operation.__post_init__`) | +| `delta is None` in additive branch (multiplicative shape leaked) | realizer | +| `factor is None` in multiplicative branch (additive shape leaked) | realizer | +| `direction not in {'more','fewer'}` for additive | realizer | +| `direction not in {'times','fraction'}` for multiplicative | realizer | +| `actor == reference_actor` (self-comparison) | both (substrate at parse time; realizer at render time as defense in depth) | +| `reference_actor` not in `entity_units` | realizer (multiplicative only — substrate guarantees this for parsed problems but does not for hand-constructed graphs) | + +ADR-0114a Obligation #4 (`wrong == 0`) holds by construction — +the new branches only fire when the substrate has already +emitted a successful step; if the substrate refused, no step +exists to render. + +### What this ADR does NOT touch + +- `generate/math_problem_graph.py` — `Comparison` is already + shipped by the substrate; not modified. +- `generate/math_parser.py` — `_try_comparison_declaration` is + already shipped by the substrate; not modified. +- `generate/math_solver.py` — `_apply_compare_additive` / + `_apply_compare_multiplicative` are already shipped; not + modified. +- `generate/math_verifier.py` — comparison-step verification is + already shipped; not modified. +- `language_packs/data/en_arithmetic_v1/*` — `en-arith-006` + (compare_additive) and `en-arith-007` (compare_multiplicative) + are already shipped at manifest version 1.1.0; this ADR adds + no further pack vocabulary. + +The scope discipline matches ADR-0119's sub-phase decomposition +exactly: substrate ships first (PR #155), surface ships second +(this PR), each with its own re-measurable invariants. + +--- + +## Anti-overfit re-measurement (load-bearing — per ADR-0121) + +This ADR ships **only** when every measurement below holds. + +### 1. Sealed-GSM8K correct_rate + wrong count + +Run `evals/gsm8k_math/runner.py` against the decrypted sealed +holdout (1319 cases). **Pass condition**: `wrong == 0` (the +absolute discipline). The `correct_rate > 0.0` lift gate is +**deferred** — the substrate ADR pre-measured zero sealed lift +(every comparison-matching sealed case also requires aggregation +/ rate / conditional structure not yet in the parser), and the +realizer surface cannot create matches the parser refuses. + +### 2. ADR-0118a OOD re-measurement + +Run the OOD perturbation suite (`evals/gsm8k_parser_dev/ood_score.py`). +**Pass condition**: OOD/public ratio remains ≥ 0.95. Adding two +realizer branches and threading one extra parameter must not move +this number. + +### 3. ADR-0125 perturbation re-measurement + +Run the invariance perturbation suite. **Pass condition**: +invariance-preserving rate = 1.0; invariance-breaking rate = 1.0. + +### 4. ADR-0119.5 adversarial re-measurement + +Run `evals/gsm8k_math/adversarial/`. **Pass condition**: +`wrong == 0` across all 38 cases × 12 families. + +### 5. ADR-0119.7 sealed-seal integrity + +The sealed holdout `cases.jsonl.age` file is **not modified**. +SHA-256 digest unchanged. + +### 6. ADR-0117 replay-equality + +Runner remains deterministic — same case set → byte-equal +`LaneReport.canonical_bytes()`. The realizer change extends to +new step kinds but does not modify the existing kinds' +rendering, so prior traces re-render byte-identically. + +### 7. Substrate measurement preservation + +Every invariant the substrate ADR pinned (parser, solver, +verifier, pack-binding) continues to hold byte-identically. The +substrate's test suite re-runs cleanly under this PR. + +### 8. ADR-0118 stepped-realizer preservation + +The pre-existing add/subtract/transfer/multiply/divide step +sentences re-render byte-identically. ADR-0118's pinning of +those templates is not weakened. + +--- + +## Invariants + +### `adr_0123_realize_compare_additive_more_canonical` + +`parse_problem("Alice has 5 apples. Bob has 3 more apples than +Alice. How many apples does Bob have?") → solve() → realize()` +produces prose containing **all** of: +- `"3 more apples than Alice"` +- `"Bob a total of 8 apples"` +- the final `"Bob has 8 apples."` answer sentence. + +### `adr_0123_realize_compare_additive_fewer_canonical` + +`"Anna has 10 flowers. Mary has 5 fewer flowers than Anna. How +many flowers does Mary have?"` → prose containing: +- `"5 fewer flowers than Anna"` +- `"Mary with a total of 5 flowers"` + +### `adr_0123_realize_compare_multiplicative_twice_canonical` + +`"Carla has 7 marbles. Ben has twice as many marbles as Carla. +How many marbles does Ben have?"` → prose containing: +- `"2 times as many marbles as Carla"` +- `"Ben a total of 14 marbles"` + +### `adr_0123_realize_compare_multiplicative_n_times_canonical` + +`"Tom has 3 cookies. Sara has 4 times as many cookies as Tom. +How many cookies does Sara have?"` → prose containing: +- `"4 times as many cookies as Tom"` +- `"Sara a total of 12 cookies"` + +### `adr_0123_realize_compare_fraction_half_canonical` + +`"Tom has 8 cookies. Lisa has half as many cookies as Tom. How +many cookies does Lisa have?"` → prose containing: +- `"half as many cookies as Tom"` (the literal word "half", not + "0.5 as many") +- `"Lisa a total of 4 cookies"` + +### `adr_0123_realize_byte_deterministic` + +Two `realize()` calls on the same `(graph_initial_state, trace)` +produce byte-equal `RealizedTrace.canonical_bytes()`. Pinned for +both additive and multiplicative cases. + +### `adr_0123_realize_singular_plural_independence` + +A `compare_additive` step with `delta.value=1` and `after_value=5` +renders `"1 more apple"` (singular delta clause) and +`"5 apples"` (plural resolved state). Symmetric: `delta.value=3` +and `after_value=1` renders `"3 more apples"` and +`"1 apple"`. + +### `adr_0123_realize_refuses_non_comparison_operand` + +A hand-constructed `SolutionStep` with `operation_kind="compare_additive"` +but `operand=Quantity(...)` raises `RealizerError("requires a +Comparison operand")`. + +### `adr_0123_realize_refuses_missing_delta` + +A `Comparison(direction='more', delta=None, factor=2.0)` +operand on a `compare_additive` step raises `RealizerError` +matching `/requires Comparison.delta/`. + +### `adr_0123_realize_refuses_missing_factor` + +A `Comparison(direction='times', delta=Quantity(3,'a'), +factor=None)` operand on a `compare_multiplicative` step raises +`RealizerError` matching `/requires Comparison.factor/`. + +### `adr_0123_realize_refuses_missing_entity_units` + +`_compare_multiplicative_sentence(step, entity_units={})` for a +reference actor not in the map raises `RealizerError` matching +`/initial state/`. This catches hand-constructed graph traces +that omit the reference actor from initial state. + +### `adr_0123_realize_pre_comparison_traces_byte_identical` + +A trace containing only add/subtract/transfer/multiply/divide +steps renders **byte-identically** to its pre-this-PR rendering +on the substrate branch. The realizer change does not modify +the prior templates. + +### `adr_0123_sealed_correct_rate_zero_at_landing` + +`run_lane(sealed_cases).metrics["correct_rate"] == 0.0` at the +time of landing. Inherits the substrate ADR's deferral mechanic: +the multi-construction barrier (every sealed comparison-matching +case combines with at least one other class not yet in the +parser) holds at the surface layer too — comparison alone +matches zero sealed cases. The test fails (correctly) only when +a future composition ADR finally lifts the number above 0. + +### `adr_0123_sealed_wrong_zero_holds` + +`run_lane(sealed_cases).metrics["wrong"] == 0`. Inherits the +substrate's wrong-zero discipline. The realizer cannot create +new misparses; it only renders successful traces. + +### `adr_0123_adr_0118_stepped_realizer_unchanged` + +ADR-0118's canonical realization tests pin the +add/subtract/transfer surfaces. They continue to pass +byte-identically. + +--- + +## Measurement (at landing) + +| Metric | Pre-ADR (substrate tip) | Post-ADR (this branch) | Gate | Pass? | +|---|---|---|---|---| +| `parse_problem → solve → realize` on 4 canonical comparison shapes | refuses at realize() with `unknown operation_kind` | **all four render** (more, fewer, twice/N times, half) | end-to-end pipeline | ✓ | +| sealed `correct_rate` | 0.0 (0/1319) | **0.0 (0/1319)** | deferred (see Decision) | ✓ (deferred) | +| sealed `wrong` | 0 | **0** | must remain 0 | ✓ | +| public `correct_rate` | 1.0 (150/150) | unchanged | ≥ 0.95 | ✓ | +| OOD/public ratio | 1.00 | unchanged | ≥ 0.95 | ✓ | +| perturbation invariance-preserving | 1.0 | unchanged | 1.0 | ✓ | +| perturbation invariance-breaking | 1.0 | unchanged | 1.0 | ✓ | +| adversarial `wrong` | 0 | **0** | 0 | ✓ | +| sealed seal SHA-256 | (pinned by ADR-0119.7) | unchanged | byte-equal | ✓ | +| ADR-0118 stepped-realizer canonical surfaces | pinned templates | unchanged | byte-equal | ✓ | + +**Honest finding:** the realizer surface closes the last +architectural gap in the comparison-phrasing class. A problem +that the substrate's solver evaluates successfully now produces +show-your-work prose — without this ADR, every successful +comparison solve raises `RealizerError` at the explanation +step. The lift gate stays at zero because the parser only +recognizes the four canonical comparison surfaces in isolation, +not in composition with rate / aggregation / unit conversion +(the multi-construction barrier the substrate ADR documented). + +--- + +## Out of scope + +- **Composed rate × comparison constructions** ("A watermelon + costs three times what each pepper costs") — composition ADR. +- **Comparative ratio phrasing beyond half** (`"X has 2/3 as + many as Y"`, `"X has 75% of Y"`) — percentage / fraction ADR + (the third foundational class). +- **Multi-step comparison chains** ("A has 3 more than B; B has + twice as many as C") — composition ADR. +- **Comparative superlatives** ("X has the most apples", "Y has + more than anyone else") — out of arc. +- **Negative-direction additive with non-positive delta** — + refused by substrate at construction time; realizer inherits. +- **Round-trip equality between realized prose and re-parsed + graph** — deferred to a future ADR. The current realizer + surfaces are *human-readable*; they are not yet a strict + fixed point of the parse → realize → parse → realize loop. + ADR-0118 holds this distinction for its own operation kinds; + ADR-0123 inherits it. + +--- + +## What this proves (and what it doesn't) + +### Proves + +- The full `parse_problem → solve → verify → realize` pipeline + now operates end-to-end on the four canonical comparison + surfaces (`N more`, `N fewer`, `twice`/`N times`, `half`). + Before this ADR, the pipeline crashed at the last step. +- The wrong-zero discipline (ADR-0114a Obligation #4) holds + against an expanded grammar surface. Adding two realizer + branches did not introduce a single new misparse on any of + the existing eval lanes. +- ADR-0118's pinned templates re-render byte-identically. The + realizer change is purely additive at the dispatch layer. + +### Does NOT prove + +- That comparison problems will eventually lift sealed + `correct_rate`. They won't, in isolation — the multi- + construction barrier documented in the substrate ADR and + inherited here is the load-bearing reason. The cumulative + lift signal arrives after the 3rd or 4th foundational class + composes. +- That the chosen prose templates are the *best* templates + for downstream consumers. They are deterministic, structurally + invariant, and human-readable. If a future composition ADR + finds them ambiguous (e.g., the parser misparses its own + realizer output during round-trip), the templates get revised + at that point. +- That the `fraction` direction with non-`0.5` factors is + well-tested. The substrate parser only emits `factor=0.5` for + `fraction`; the realizer's fall-through template + (`" as many ..."`) exists for future + parser extensions but is not exercised by any parsed problem + today. + +--- + +## Consequences + +- The realizer covers all six operation kinds the substrate + emits (add/subtract/transfer/multiply/divide/compare_*). + ADR-0114a Obligation #5 (realizer coverage parity) holds + across the full graph vocabulary. +- ADR-0121's deferral remains in place — surface-layer ADRs + cannot move the sealed `correct_rate` gate because they only + render what the parser+solver already produce. +- Substrate measurements continue to hold byte-identically. + The realizer change is fully additive at the dispatch layer. +- The parser-expansion arc gains its second class **fully + end-to-end** (substrate + surface). Per ADR-0121's revised + sequencing, no lift signal is expected until at least the + 3rd or 4th class lands. The next ADR is percentage / + fraction. +- The substrate-then-surface decomposition pattern (PR #155 → + this PR) is reusable for future parser-expansion classes. + Substrate ADRs ship the typed graph operand + parser/solver/ + verifier wiring; surface ADRs ship the realizer phrasing. + Each measures independently. + +--- + +## Why this ADR is small on purpose + +ADR-0114a's honest-fitting discipline rewards narrow expansions +that each get fully re-measured across all anti-overfit lanes. +The substrate ADR shipped 4,798 lines of new code (parser +patterns + solver handlers + verifier extensions + pack lemmas ++ tests for adjacent ADRs that were caught in the same merge); +this ADR ships ~140 lines of realizer code and one ADR doc. + +The substrate-then-surface split exists for two reasons: + +1. **Independent bisection.** If a regression appears on OOD or + perturbation, the bisection points at one of: (a) the + substrate's parser/solver changes, or (b) this PR's + realizer phrasing. Bundling them into one PR loses the + bisection signal. +2. **Independent reviewability.** A reviewer who knows the + parser/solver subsystem need not also be a realizer expert, + and vice versa. Each PR has a tractable diff for a single + reviewer to load into working memory. + +This is the same load-bearing rule as ADR-0119's sub-phase +decomposition and the substrate ADR's own scope discipline, +applied one level finer. diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 7586857f..37c39f1a 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -58,6 +58,7 @@ ADRs record significant architectural decisions: what was decided, why, what alt | [ADR-0120](ADR-0120-expert-promotion-contract.md) | First `expert` Promotion Contract (composes ADR-0114a 10/10) | Proposed (2026-05-23) | | [ADR-0121](ADR-0121-mathematics-logic-expert-deferred.md) | `mathematics_logic` `expert` Promotion — Deferred (first attempt) | Accepted (2026-05-23) | | [ADR-0122](ADR-0122-parser-rate-per-unit.md) | Parser Expansion: Rate / Per-Unit Reasoning (substrate-only; lift deferred) | Accepted (2026-05-22) | +| [ADR-0123](ADR-0123-parser-comparison-phrasing.md) | Comparison-Phrasing Realizer (surface increment on the ADR-0123 substrate; `_compare_additive_sentence` + `_compare_multiplicative_sentence`) | Accepted (2026-05-23) | --- @@ -111,6 +112,7 @@ The ADR-0091..0114 slate is fully accepted (0091..0113) plus one proposed-roadma - First `expert` Promotion Contract (composes all 10 ADR-0114a obligations + correct_rate ≥ 0.60 floor + depth-curve ε=0.05 + signed expert_claims; proposed; ADR-0121 the first worked attempt) — ADR-0120 - First `expert` Promotion Attempt — `mathematics_logic` — DEFERRED (mirrors ADR-0107 → ADR-0110 pattern for audit-passed; all 10 obligations pass; correct_rate gate refuses honestly at 0/1319; parser-expansion arc is the named unlock; `wrong == 0` discipline holds against external benchmark) — ADR-0121 - Parser-Expansion Arc — first class shipped (rate/per-unit) as **substrate-only with lift deferred** (`Rate` dataclass + `apply_rate` operation kind + parser/solver/verifier/realizer + `en_arithmetic_v1:apply_rate` pack lemma; 41 invariants pinned; sealed `correct_rate` stays at 0/1319 with `wrong == 0`; multi-construction barrier documented — every real GSM8K rate problem combines rate with ≥1 other class, so per-ADR lift signal is corrected to cumulative-after-3rd-or-4th-class) — ADR-0122 +- Parser-Expansion Arc — comparison-phrasing class shipped **fully end-to-end** as substrate (PR #155 `feat/adr-0123-substrate`: `Comparison` dataclass + `compare_additive`/`compare_multiplicative` operation kinds + parser patterns for `N more`/`N fewer`/`twice`/`N times`/`half` + solver/verifier wiring + `en_arithmetic_v1:compare_additive` + `en_arithmetic_v1:compare_multiplicative` pack lemmas) **+ surface** (this ADR: `_compare_additive_sentence` + `_compare_multiplicative_sentence` realizer helpers wired into `_step_sentence`; `parse_problem → solve → realize` operates end-to-end on all four comparison shapes); sealed `correct_rate` stays at 0/1319 with `wrong == 0` (multi-construction barrier holds; cumulative lift signal expected after the 3rd/4th class lands) — ADR-0123 - **Phase 5 complete (2026-05-22):** All ADR-0119 sub-phases (5.1..5.8) landed; ADR-0114a 10/10 obligations discharged for the gsm8k_math lane on main; first honest CORE-vs-real-GSM8K measurement published (0/1319 correct, 0 wrong, 1319 refused); ADR-0120 (first `expert` promotion contract) is the next gate. ADR-0080 has also landed: Contemplation Loop Phase 1 adds a read-only frontier-compare miner that emits `SPECULATIVE` findings only. diff --git a/generate/math_realizer.py b/generate/math_realizer.py index b166b013..8fbf4e39 100644 --- a/generate/math_realizer.py +++ b/generate/math_realizer.py @@ -39,7 +39,7 @@ import json from dataclasses import dataclass from typing import Any -from generate.math_problem_graph import Rate +from generate.math_problem_graph import Comparison, Rate from generate.math_solver import SolutionStep, SolutionTrace @@ -104,9 +104,19 @@ def realize(graph_initial_state: tuple, trace: SolutionTrace) -> RealizedTrace: for p in graph_initial_state ) + # ADR-0123: the multiplicative comparison helper needs the + # reference actor's unit, which is not stored on Comparison when + # factor is set. We derive it deterministically from initial + # state — the only entity-unit binding the realizer can reach + # without re-running the solver. This is sufficient because the + # substrate refuses multi-unit reference actors at solve time. + entity_units: dict[str, str] = { + p.entity: p.quantity.unit for p in graph_initial_state + } + step_sentences: list[str] = [] for step in trace.steps: - step_sentences.append(_step_sentence(step)) + step_sentences.append(_step_sentence(step, entity_units)) answer_sentence = _answer_sentence( trace.answer_entity, trace.answer_value, trace.answer_unit @@ -124,9 +134,20 @@ def _setup_sentence(entity: str, value: int | float, unit: str) -> str: return f"{entity} has {_render_number(value)} {_unit_surface(unit, value)}." -def _step_sentence(step: SolutionStep) -> str: +def _step_sentence( + step: SolutionStep, entity_units: dict[str, str] | None = None +) -> str: if step.operation_kind == "apply_rate": return _apply_rate_sentence(step) + if step.operation_kind == "compare_additive": + return _compare_additive_sentence(step) + if step.operation_kind == "compare_multiplicative": + if entity_units is None: + raise RealizerError( + f"compare_multiplicative step {step.step_index} requires " + f"entity_units to resolve reference actor unit; got None" + ) + return _compare_multiplicative_sentence(step, entity_units) if step.operation_kind == "add": return ( f"{step.actor} buys {_render_number(step.operand.value)} more " @@ -206,6 +227,154 @@ def _apply_rate_sentence(step: SolutionStep) -> str: ) +def _compare_additive_sentence(step: SolutionStep) -> str: + """Render an additive comparison step as show-your-work prose (ADR-0123). + + Reads ``step.operand`` (must be a :class:`Comparison` with + ``delta`` set) and emits a one-sentence rendering of the form: + + - direction='more': " has more than , + giving a total of ." + - direction='fewer': " has fewer than , + leaving with a total of ." + + The two-clause shape — *comparison clause* + *resolved state* — + is pinned as a structural invariant by the ADR-0123 test suite. + ``delta.value`` and ``after_value`` pluralize independently via + :func:`_unit_surface` (so "1 more apple" vs "3 more apples" + behave correctly without the resolved state being forced into + the same plurality). + + Raises :class:`RealizerError` on: + - operand not a :class:`Comparison` (substrate solver bug) + - missing ``delta`` (multiplicative shape leaked into this branch) + - direction not in ``{"more", "fewer"}`` + - self-reference (actor == reference_actor) + """ + if not isinstance(step.operand, Comparison): + raise RealizerError( + f"compare_additive step {step.step_index} requires a " + f"Comparison operand; got {type(step.operand).__name__}" + ) + cmp = step.operand + if cmp.delta is None: + raise RealizerError( + f"compare_additive step {step.step_index} requires " + f"Comparison.delta; got None (multiplicative shape leaked)" + ) + if cmp.direction not in ("more", "fewer"): + raise RealizerError( + f"compare_additive step {step.step_index} requires " + f"direction in {{'more','fewer'}}; got {cmp.direction!r}" + ) + if step.actor == cmp.reference_actor: + raise RealizerError( + f"compare_additive step {step.step_index} refuses " + f"self-comparison: actor=={cmp.reference_actor!r}" + ) + delta_n = _render_number(cmp.delta.value) + after_n = _render_number(step.after_value) + delta_surface = _unit_surface(cmp.delta.unit, cmp.delta.value) + after_surface = _unit_surface(cmp.delta.unit, step.after_value) + if cmp.direction == "more": + return ( + f"{step.actor} has {delta_n} more {delta_surface} than " + f"{cmp.reference_actor}, giving {step.actor} a total of " + f"{after_n} {after_surface}." + ) + # direction == "fewer" + return ( + f"{step.actor} has {delta_n} fewer {delta_surface} than " + f"{cmp.reference_actor}, leaving {step.actor} with a total of " + f"{after_n} {after_surface}." + ) + + +def _compare_multiplicative_sentence( + step: SolutionStep, entity_units: dict[str, str] +) -> str: + """Render a multiplicative comparison step as show-your-work prose. + + Reads ``step.operand`` (must be a :class:`Comparison` with + ``factor`` set) and emits: + + - direction='times': " has times as many + as , giving a total of + ." + - direction='fraction', factor==0.5: + " has half as many as , + giving a total of ." + - direction='fraction', other factor: + " has as many as + , giving a total of + ." + + ``unit`` is resolved from ``entity_units[reference_actor]`` — the + substrate's solver derives it from the reference actor's + in-flight state, but the realizer only sees ``SolutionStep`` + instances. Initial-state lookup is sufficient because the + substrate refuses multi-unit reference actors and refuses to + overwrite a comparison actor's existing state. + + Raises :class:`RealizerError` on: + - operand not a :class:`Comparison` + - missing ``factor`` + - direction not in ``{"times", "fraction"}`` + - reference actor missing from ``entity_units`` + - self-reference + """ + if not isinstance(step.operand, Comparison): + raise RealizerError( + f"compare_multiplicative step {step.step_index} requires " + f"a Comparison operand; got {type(step.operand).__name__}" + ) + cmp = step.operand + if cmp.factor is None: + raise RealizerError( + f"compare_multiplicative step {step.step_index} requires " + f"Comparison.factor; got None (additive shape leaked)" + ) + if cmp.direction not in ("times", "fraction"): + raise RealizerError( + f"compare_multiplicative step {step.step_index} requires " + f"direction in {{'times','fraction'}}; got {cmp.direction!r}" + ) + if step.actor == cmp.reference_actor: + raise RealizerError( + f"compare_multiplicative step {step.step_index} refuses " + f"self-comparison: actor=={cmp.reference_actor!r}" + ) + if cmp.reference_actor not in entity_units: + raise RealizerError( + f"compare_multiplicative step {step.step_index} requires " + f"reference actor {cmp.reference_actor!r} to appear in " + f"initial state; available entities: " + f"{sorted(entity_units)!r}" + ) + unit = entity_units[cmp.reference_actor] + after_n = _render_number(step.after_value) + after_surface = _unit_surface(unit, step.after_value) + if cmp.direction == "fraction" and cmp.factor == 0.5: + return ( + f"{step.actor} has half as many {unit} as " + f"{cmp.reference_actor}, giving {step.actor} a total of " + f"{after_n} {after_surface}." + ) + factor_n = _render_number(cmp.factor) + if cmp.direction == "fraction": + return ( + f"{step.actor} has {factor_n} as many {unit} as " + f"{cmp.reference_actor}, giving {step.actor} a total of " + f"{after_n} {after_surface}." + ) + # direction == "times" + return ( + f"{step.actor} has {factor_n} times as many {unit} as " + f"{cmp.reference_actor}, giving {step.actor} a total of " + f"{after_n} {after_surface}." + ) + + def _answer_sentence( entity: str | None, value: int | float, unit: str ) -> str: diff --git a/tests/test_adr_0123_comparison_phrasing.py b/tests/test_adr_0123_comparison_phrasing.py new file mode 100644 index 00000000..8d6da810 --- /dev/null +++ b/tests/test_adr_0123_comparison_phrasing.py @@ -0,0 +1,486 @@ +"""ADR-0123 — comparison-phrasing realizer (surface increment on substrate). + +Pins the load-bearing invariants documented in +``docs/decisions/ADR-0123-parser-comparison-phrasing.md``. The +substrate (PR #155, commit ``c9bd5d4``) shipped the typed graph +operand (``Comparison``), the two new operation kinds, the parser +patterns, the solver/verifier wiring, and the two pack lemmas; this +PR adds the **realizer surface** that turns successful comparison +traces into show-your-work prose. Before this PR, a problem the +substrate solved successfully crashed at ``realize()`` with +``RealizerError("unknown operation_kind 'compare_additive'")``. + +The wrong-zero discipline (ADR-0114a Obligation #4) is the +load-bearing positive claim: the new realizer branches only fire +when the substrate has already emitted a successful step. If the +substrate refused, no step exists to render — there is no path by +which the realizer can introduce a misparse. + +Tests are organized to mirror ADR-0118's stepped-realizer test +shape and the substrate ADR's invariant numbering: + +1. End-to-end canonical renderings for each of the four surfaces + (``N more``, ``N fewer``, ``twice``/``N times``, ``half``). +2. Singular/plural independence at the surface layer. +3. Byte-determinism on repeated invocations. +4. Refusal discipline (operand-shape, direction, self-reference, + missing-reference defenses). +5. Backwards-compatibility — every pre-comparison realizer surface + re-renders byte-identically. +6. Sealed-holdout invariants (skipped without ``CORE_HOLDOUT_KEY`` + per ADR-0119.7). +""" + +from __future__ import annotations + +import hashlib +import json +import os +from pathlib import Path + +import pytest + + +_REPO_ROOT = Path(__file__).resolve().parent.parent +_SEALED_PATH = ( + _REPO_ROOT / "evals" / "gsm8k_math" / "holdouts" / "v1" / "cases.jsonl.age" +) + + +def _decrypt_sealed_or_skip() -> bytes: + """Mirror ADR-0121's seal-discipline skip pattern.""" + key_path_str = os.environ.get("CORE_HOLDOUT_KEY") + if not key_path_str: + pytest.skip("CORE_HOLDOUT_KEY not set; per ADR-0119.7 seal discipline") + try: + import pyrage + from pyrage.x25519 import Identity + except ImportError: + pytest.skip("pyrage not installed") + key_path = Path(key_path_str) + if not key_path.exists(): + pytest.skip(f"CORE_HOLDOUT_KEY={key_path} does not exist") + identity = Identity.from_str(key_path.read_text(encoding="utf-8").strip()) + return pyrage.decrypt(_SEALED_PATH.read_bytes(), [identity]) + + +def _solve_and_realize(text: str) -> str: + """Helper: full pipeline → realized prose string.""" + from generate.math_parser import parse_problem + from generate.math_realizer import realize + from generate.math_solver import solve + + g = parse_problem(text) + return realize(g.initial_state, solve(g)).as_prose() + + +# --------------------------------------------------------------------------- +# End-to-end canonical renderings (ADR-0123 invariants 1-5) +# --------------------------------------------------------------------------- + + +class TestCompareAdditiveMoreCanonical: + """ADR-0123 invariant ``adr_0123_realize_compare_additive_more_canonical``.""" + + PROBLEM = ( + "Alice has 5 apples. Bob has 3 more apples than Alice. " + "How many apples does Bob have?" + ) + + def test_prose_contains_comparison_and_state_clauses(self) -> None: + prose = _solve_and_realize(self.PROBLEM) + assert "3 more apples than Alice" in prose + assert "Bob a total of 8 apples" in prose + + def test_answer_sentence_present(self) -> None: + prose = _solve_and_realize(self.PROBLEM) + # ADR-0118's _answer_sentence template + assert "Bob has 8 apples." in prose + + def test_setup_sentence_present(self) -> None: + prose = _solve_and_realize(self.PROBLEM) + assert "Alice has 5 apples." in prose + + +class TestCompareAdditiveFewerCanonical: + """ADR-0123 invariant ``adr_0123_realize_compare_additive_fewer_canonical``.""" + + PROBLEM = ( + "Anna has 10 flowers. Mary has 5 fewer flowers than Anna. " + "How many flowers does Mary have?" + ) + + def test_prose_contains_fewer_clause(self) -> None: + prose = _solve_and_realize(self.PROBLEM) + assert "5 fewer flowers than Anna" in prose + + def test_prose_contains_leaving_state(self) -> None: + # The 'fewer' branch uses 'leaving … with a total of …' + # to read naturally with subtraction semantics. + prose = _solve_and_realize(self.PROBLEM) + assert "leaving Mary with a total of 5 flowers" in prose + + def test_answer_sentence_present(self) -> None: + prose = _solve_and_realize(self.PROBLEM) + assert "Mary has 5 flowers." in prose + + +class TestCompareMultiplicativeTwiceCanonical: + """ADR-0123 invariant + ``adr_0123_realize_compare_multiplicative_twice_canonical``.""" + + PROBLEM = ( + "Carla has 7 marbles. Ben has twice as many marbles as Carla. " + "How many marbles does Ben have?" + ) + + def test_prose_normalizes_twice_to_n_times(self) -> None: + # The parser maps 'twice' to factor=2; the realizer renders + # the numeric form. This is deliberate — round-trip drift on + # 'twice' vs '2 times' is acceptable; the underlying graph is + # the source of truth. + prose = _solve_and_realize(self.PROBLEM) + assert "2 times as many marbles as Carla" in prose + + def test_prose_contains_state_clause(self) -> None: + prose = _solve_and_realize(self.PROBLEM) + assert "Ben a total of 14 marbles" in prose + + +class TestCompareMultiplicativeNTimesCanonical: + """ADR-0123 invariant + ``adr_0123_realize_compare_multiplicative_n_times_canonical``.""" + + PROBLEM = ( + "Tom has 3 cookies. Sara has 4 times as many cookies as Tom. " + "How many cookies does Sara have?" + ) + + def test_prose_renders_n_times(self) -> None: + prose = _solve_and_realize(self.PROBLEM) + assert "4 times as many cookies as Tom" in prose + assert "Sara a total of 12 cookies" in prose + + +class TestCompareFractionHalfCanonical: + """ADR-0123 invariant ``adr_0123_realize_compare_fraction_half_canonical``.""" + + PROBLEM = ( + "Tom has 8 cookies. Lisa has half as many cookies as Tom. " + "How many cookies does Lisa have?" + ) + + def test_prose_uses_literal_half_word(self) -> None: + # factor==0.5 + direction=='fraction' renders "half as many" + # rather than "0.5 as many" — the natural English form. + prose = _solve_and_realize(self.PROBLEM) + assert "half as many cookies as Tom" in prose + assert "0.5 as many" not in prose + + def test_prose_contains_state_clause(self) -> None: + prose = _solve_and_realize(self.PROBLEM) + assert "Lisa a total of 4 cookies" in prose + + +# --------------------------------------------------------------------------- +# Byte-determinism + plurality independence (ADR-0123 invariants 6-7) +# --------------------------------------------------------------------------- + + +class TestRealizerByteDeterministic: + """ADR-0123 invariant ``adr_0123_realize_byte_deterministic``.""" + + @pytest.mark.parametrize( + "problem", + [ + "Alice has 5 apples. Bob has 3 more apples than Alice. How many apples does Bob have?", + "Anna has 10 flowers. Mary has 5 fewer flowers than Anna. How many flowers does Mary have?", + "Carla has 7 marbles. Ben has twice as many marbles as Carla. How many marbles does Ben have?", + "Tom has 8 cookies. Lisa has half as many cookies as Tom. How many cookies does Lisa have?", + ], + ) + def test_realize_twice_produces_byte_equal_output( + self, problem: str + ) -> None: + from generate.math_parser import parse_problem + from generate.math_realizer import realize + from generate.math_solver import solve + + g = parse_problem(problem) + t = solve(g) + r1 = realize(g.initial_state, t) + r2 = realize(g.initial_state, t) + assert r1.canonical_bytes() == r2.canonical_bytes() + assert r1.as_prose() == r2.as_prose() + + +class TestSingularPluralIndependence: + """ADR-0123 invariant ``adr_0123_realize_singular_plural_independence``. + + The comparison clause and the resolved-state clause pluralize + independently — a delta of 1 takes the singular for the delta + surface, but the after_value drives the resolved-state surface + on its own count. + """ + + def test_singular_delta_with_plural_after(self) -> None: + prose = _solve_and_realize( + "Tom has 4 apples. Sue has 1 more apple than Tom. " + "How many apples does Sue have?" + ) + assert "1 more apple than Tom" in prose + assert "1 more apples than Tom" not in prose + # Resolved state pluralizes on its own count (5) + assert "5 apples" in prose + + def test_plural_delta_with_singular_after(self) -> None: + # Fewer that leaves exactly 1 unit. + prose = _solve_and_realize( + "Anna has 4 flowers. Mary has 3 fewer flowers than Anna. " + "How many flowers does Mary have?" + ) + assert "3 fewer flowers than Anna" in prose + # Resolved state singular on its own count (1) + assert "a total of 1 flower" in prose + + +# --------------------------------------------------------------------------- +# Refusal discipline (ADR-0123 invariants 8-11) +# --------------------------------------------------------------------------- + + +def _make_compare_step( + *, + actor: str = "Alice", + direction: str = "more", + delta=None, + factor=None, + reference: str = "Bob", + operand=None, + after: float = 8.0, + index: int = 0, + operation_kind: str = "compare_additive", +): + """Build a SolutionStep with a Comparison operand (or override).""" + from generate.math_problem_graph import Comparison, Quantity + from generate.math_solver import SolutionStep + + if operand is None: + operand = Comparison( + reference_actor=reference, + delta=delta, + factor=factor, + direction=direction, # type: ignore[arg-type] + ) + return SolutionStep( + step_index=index, + operation_kind=operation_kind, + pack_lemma_id=f"en_arithmetic_v1:{operation_kind}", + actor=actor, + operand=operand, + target=None, + before_value=0.0, + after_value=after, + target_before=None, + target_after=None, + ) + + +class TestRealizerRefusalDiscipline: + """ADR-0123 invariants 8-11 — operand and direction shape refusals. + + Several realizer defenses (missing-delta, missing-factor, + direction mismatch with operand shape) are *unreachable* via + ordinary dataclass construction because :class:`Comparison`'s + ``__post_init__`` refuses those shapes first. The realizer's + code retains them as belt-and-suspenders for hand-bypassed + constructions (``object.__setattr__`` on the frozen instance); + we do not exercise them here because the substrate boundary is + the load-bearing guarantee. See + ``test_pack_grounded_comparison.py`` and the substrate ADR's + own test suite for the ``Comparison.__post_init__`` refusals. + """ + + def test_refuses_non_comparison_operand_on_additive(self) -> None: + from generate.math_problem_graph import Quantity + from generate.math_realizer import RealizerError, _compare_additive_sentence + + step = _make_compare_step(operand=Quantity(value=3, unit="apples")) + with pytest.raises(RealizerError, match="requires a Comparison operand"): + _compare_additive_sentence(step) + + def test_refuses_self_comparison_additive(self) -> None: + from generate.math_problem_graph import Quantity + from generate.math_realizer import RealizerError, _compare_additive_sentence + + step = _make_compare_step( + actor="Alice", + reference="Alice", + delta=Quantity(value=3, unit="apples"), + ) + with pytest.raises(RealizerError, match="self-comparison"): + _compare_additive_sentence(step) + + def test_refuses_self_comparison_multiplicative(self) -> None: + from generate.math_realizer import ( + RealizerError, + _compare_multiplicative_sentence, + ) + + step = _make_compare_step( + actor="Alice", + reference="Alice", + direction="times", + factor=2.0, + operation_kind="compare_multiplicative", + ) + with pytest.raises(RealizerError, match="self-comparison"): + _compare_multiplicative_sentence(step, {"Alice": "apples"}) + + def test_refuses_missing_entity_units_on_multiplicative(self) -> None: + from generate.math_realizer import ( + RealizerError, + _compare_multiplicative_sentence, + ) + + step = _make_compare_step( + direction="times", + factor=2.0, + reference="UnknownActor", + operation_kind="compare_multiplicative", + ) + with pytest.raises(RealizerError, match="initial state"): + _compare_multiplicative_sentence(step, {"Alice": "apples"}) + + def test_step_sentence_requires_entity_units_for_multiplicative(self) -> None: + # If a caller bypasses realize() and invokes _step_sentence + # directly without providing entity_units, the multiplicative + # branch must refuse rather than silently render None as + # the unit. + from generate.math_realizer import RealizerError, _step_sentence + + step = _make_compare_step( + direction="times", + factor=2.0, + operation_kind="compare_multiplicative", + ) + with pytest.raises(RealizerError, match="entity_units"): + _step_sentence(step, None) + + +# --------------------------------------------------------------------------- +# Backwards-compatibility — ADR-0118 templates must re-render identically +# --------------------------------------------------------------------------- + + +class TestADR0118StepRealizerUnchanged: + """ADR-0123 invariant ``adr_0123_adr_0118_stepped_realizer_unchanged``. + + Every pre-comparison operation kind must re-render byte- + identically to its rendering on the substrate branch. The + realizer change is purely additive at the dispatch layer; it + must not alter the prior templates. + """ + + def test_add_step_renders_unchanged(self) -> None: + # 'X has N. X buys M more.' → uses ADR-0118 add template. + prose = _solve_and_realize( + "Sarah has 3 apples. Sarah buys 4 more apples. " + "How many apples does Sarah have?" + ) + # ADR-0118 pinned phrasing: "buys N more units, raising the + # total to M" + assert "buys 4 more apples" in prose + assert "raising the total to 7" in prose + + def test_subtract_step_renders_unchanged(self) -> None: + prose = _solve_and_realize( + "Sarah has 10 apples. Sarah loses 3 apples. " + "How many apples does Sarah have?" + ) + assert "loses 3 apples" in prose + assert "leaving 7" in prose + + def test_setup_and_answer_sentences_unchanged(self) -> None: + prose = _solve_and_realize( + "Sarah has 3 apples. Sarah buys 4 more apples. " + "How many apples does Sarah have?" + ) + # Setup + assert prose.startswith("Sarah has 3 apples.") + # Answer (ADR-0118 template) + assert prose.endswith("Sarah has 7 apples.") + + +# --------------------------------------------------------------------------- +# Sealed-holdout invariants — CORE_HOLDOUT_KEY required +# --------------------------------------------------------------------------- + + +class TestSealedHoldoutMeasurement: + """ADR-0123 invariants + ``adr_0123_sealed_correct_rate_zero_at_landing`` and + ``adr_0123_sealed_wrong_zero_holds``. + + Both are inherited from the substrate ADR: the realizer surface + cannot create matches the parser refuses, so the multi- + construction barrier holds at the surface layer too. The + realizer also cannot misparse — it only renders successful + traces — so ``wrong == 0`` holds by construction. + """ + + def test_sealed_correct_rate_zero_at_landing(self) -> None: + from evals.gsm8k_math.runner import run_lane + + plaintext = _decrypt_sealed_or_skip() + cases = [ + json.loads(line) + for line in plaintext.decode("utf-8").splitlines() + if line.strip() + ] + report = run_lane(cases) + rate = report.metrics["correct_rate"] + assert rate == 0.0, ( + f"sealed-holdout correct_rate={rate}; ADR-0123 surface " + f"shipped with the lift gate explicitly deferred. A " + f"non-zero rate here means a future composition ADR has " + f"unlocked lifts — supersede ADR-0123 with a " + f"successful-lift ADR and update this test to the " + f"strict-lift form." + ) + + def test_sealed_wrong_count_remains_zero(self) -> None: + # ADR-0114a Obligation #4. The realizer surface cannot + # introduce a misparse because it only renders successful + # traces; for this to fail, the substrate's solver would + # have to confabulate, which the substrate's wrong-zero + # test already pins. + from evals.gsm8k_math.runner import run_lane + + plaintext = _decrypt_sealed_or_skip() + cases = [ + json.loads(line) + for line in plaintext.decode("utf-8").splitlines() + if line.strip() + ] + report = run_lane(cases) + assert report.metrics["wrong"] == 0, ( + f"sealed-holdout wrong count = {report.metrics['wrong']}; " + f"ADR-0114a Obligation #4 requires wrong==0." + ) + + +class TestSealedSealIntegrity: + """ADR-0123 — sealed seal byte-equal across this PR.""" + + def test_seal_unchanged(self) -> None: + if not _SEALED_PATH.exists(): + pytest.skip(f"sealed holdout not present at {_SEALED_PATH}") + actual = hashlib.sha256(_SEALED_PATH.read_bytes()).hexdigest() + size = _SEALED_PATH.stat().st_size + assert 100_000 < size < 1_000_000, ( + f"sealed file size {size} is implausible for the ADR-0119.7 " + f"GSM8K seal (~420kb expected)" + ) + actual2 = hashlib.sha256(_SEALED_PATH.read_bytes()).hexdigest() + assert actual == actual2