diff --git a/core/cli.py b/core/cli.py index 38e7781c..a5a76600 100644 --- a/core/cli.py +++ b/core/cli.py @@ -2038,14 +2038,10 @@ def cmd_teaching_coverage(args: argparse.Namespace) -> int: return 1 report_path = lane_dir / "report.json" - use_reader = bool(args.use_reader) - if args.run or not report_path.exists(): import subprocess runner_module = f"evals.{lane}.{split}.{version}.runner" runner_args = [sys.executable, "-m", runner_module] - if use_reader: - runner_args.append("--use-reader") try: subprocess.run( runner_args, @@ -2069,14 +2065,13 @@ def cmd_teaching_coverage(args: argparse.Namespace) -> int: lane=lane, split=split, version=version, - use_reader=use_reader, baseline_path=baseline_path, ) if args.json: print(json.dumps(report.as_dict(), indent=2, sort_keys=True)) else: - print(f"Lane: {report.lane}/{report.split}/{report.version} (use_reader={report.use_reader})") + print(f"Lane: {report.lane}/{report.split}/{report.version}") if report.delta: print( f"Counts: correct={report.counts.correct} " @@ -4631,10 +4626,6 @@ def build_parser() -> argparse.ArgumentParser: teaching_coverage.add_argument( "--version", default="v1", help="lane version (default: v1)", ) - teaching_coverage.add_argument( - "--use-reader", action="store_true", - help="pass --use-reader to the runner (matches train_sample default)", - ) teaching_coverage.add_argument( "--run", action="store_true", help="re-run the lane's runner even if report.json exists", diff --git a/core/config.py b/core/config.py index b8113849..48f6160e 100644 --- a/core/config.py +++ b/core/config.py @@ -276,17 +276,6 @@ class RuntimeConfig: # ADR-0151 — generate TeachingChainProposals from enriched candidates on load. auto_proposal_enabled: bool = False - # ADR-0164 Phase 1 — incremental comprehension reader for question sentences. - # When True, the candidate-graph path uses the comprehension reader - # (generate/comprehension/lifecycle.py) to parse question sentences BEFORE - # consulting the regex question patterns (Pattern A/B/C in - # generate/math_candidate_parser.py). On reader refusal, falls through to - # the existing regex parser — the reader is purely additive at Phase 1. - # Default False: flag-OFF behaviour is byte-identical to today. - # Phase 3 (per ADR-0164 §Phasing) removes the regex question parser entirely; - # that work is deferred — this PR is the Phase 1 stopgap. - comprehension_reader_questions: bool = False - DEFAULT_IDENTITY_PACK: str = "default_general_v1" DEFAULT_ETHICS_PACK: str = "default_general_ethics_v1" diff --git a/docs/decisions/ADR-0174-held-hypothesis-comprehension.md b/docs/decisions/ADR-0174-held-hypothesis-comprehension.md index 88a5270e..737c7a6c 100644 --- a/docs/decisions/ADR-0174-held-hypothesis-comprehension.md +++ b/docs/decisions/ADR-0174-held-hypothesis-comprehension.md @@ -246,6 +246,14 @@ Wire `contemplate` into `finalize()`. The function consults vault + packs + audi ### Phase 5 — Remove parallel parsers +> **SUPERSEDED by the §"Phase 5 — Scope (amended 2026-05-28)" section +> below.** The text in this subsection rests on three premises that a +> pre-scope investigation proved false against the shipped code: +> `math_parser.py` is already out of the runtime/scoring path, +> `lifecycle.py` admits 0/50 (it is the inert parallel parser, not the +> reader to promote), and `correct ≥ 25` is a *semantic* gate that +> structural collapse cannot meet. Read the amended scope, not this. + Delete `generate/math_parser.py`'s runtime invocation paths. Remove the per-category injector dispatch table; injectors become inlined hypothesis emitters. Collapse the duplicate per-sentence-choices scaffolding in `math_candidate_graph.py`. **Acceptance:** @@ -419,3 +427,218 @@ direct predicate-name assertions in tests, and all 4 manually). Backfill landed in the same Phase 3a PR (10 new tests). --- + +## Phase 5 — Scope (amended 2026-05-28) + +A pre-implementation investigation (per CLAUDE.md §Lookback Review +Discipline, triggered "before starting the next phase") established the +verified ground truth below. The original §Phase 5 subsection is +**superseded** — it inverted the promote/retire direction and attached +the lift gate to the wrong work. + +### Verified ground truth + +| Original Phase 5 premise | Verified reality (2026-05-28) | +|---|---| +| `math_parser.py` is the legacy parser to remove from runtime | Already out of the chat runtime **and** the candidate-graph (`_score_one_candidate_graph` → `parse_and_solve`) train_sample scoring path. Live only in `_score_one`, `evals/gsm8k_math/verify.py`, and the perturbation / OOD / bounded-grammar obligation lanes (`core/capability/perturbation_b3.py`, `generate/perturbation_suite.py`, `evals/gsm8k_parser_dev/*`, `evals/math_bounded_grammar`, `evals/obligation_2_ood_ratio`). CLEANUP-C2 keeps it as the `--legacy-parser` baseline. **Nothing to remove from the live path.** | +| `lifecycle.py` is the reader to promote to primary | `_try_comprehension_reader` (lifecycle apply_word/finalize) **admits 0/50** on train_sample — inert *in GSM8K scoring*. **But `lifecycle.py` is NOT dead:** `generate/comprehension/audit.py` (`audit_problem`/`AuditRow`) imports its reader surface, and `audit_problem` is load-bearing for the ADR-0172 math-contemplation teaching corridor (`teaching/math_evidence.py`, `math_contemplation.py`, `math_inference_proposal.py`, `math_claim_signature.py`, `math_contemplation_proposal.py`, `core/cli.py`, `evals/flywheel_demo`). The reader's *refusals* become teaching evidence. So the file stays; only its **GSM8K-scoring dispatch** is inert and retirable. | +| Integration target is per-token `apply_word` | All Phase 2/3a/3b/4 defenses (`eliminate_violating`, `reevaluate`, `contemplate`) are wired into the recognizer/candidate-graph path, which produces all 3 correct cases. `lifecycle.py` carries none of them. | +| `correct ≥ 25` is the Phase 5 gate | Structural collapse is a refactor → **~0 lift**. The lift lives in removing the 3–5 narrowness layers refusing simultaneously per case — *semantic* work the original phase never separated. | + +**Decision (Invert + split):** the recognizer/candidate-graph path +(`generate/math_candidate_graph.parse_and_solve` + `math_candidate_parser` ++ `recognizer_match` + `recognizer_anchor_inject`, over the `state.py` +hypothesis primitives) is **the canonical reader**. `lifecycle.py` is +retired. Phase 5 splits into a safe structural phase (5a) and a semantic +lift phase (5b). + +### Retirement-safe vs load-bearing + +- **Retire (5a) — GSM8K-scoring-only inert dispatch (~580 LOC):** + `generate/comprehension/lifecycle_runtime_adapter.py` (402 LOC, imported + *only* by the question-reader dispatch); **both** flag-gated reader + dispatches in `math_candidate_graph.py` — `_try_comprehension_reader` + (whole-problem, admits 0/50), `_try_reader_for_question` (question-stage, + via the adapter), and the `_tokenize_sentence` helper they use; the + `comprehension_reader_questions` config flag they share and the + `--use-reader` plumbing in the train_sample runner; + `tests/test_reader_coexistence.py` (its flag-ON/OFF byte-identity premise + dissolves once there is one path). +- **KEEP — load-bearing, corrected from the first draft:** + - `generate/comprehension/lifecycle.py` (**stays** — the + audit→teaching corridor uses its reader surface; only its scoring + dispatch is inert). `tests/test_reader_phase2.py` and + `test_reader_question_frame.py` import it directly and stay. + - `generate/comprehension/audit.py` (`audit_problem`/`AuditRow`) — the + ADR-0172 contemplation/evidence entry point. + - `state.py` — including `ProblemReadingState` (`contemplate()`'s + parameter type, constructed in the Phase 4 recognizer wiring at + `math_candidate_graph.py:928`), `Hypothesis`, `UnknownHeld`, + `HYPOTHESIS_CAP`; `constraint_propagation.py`, `lookback.py`, + `contemplate.py`, and the `recognizer_anchor_inject.py` injector table. + + > **Correction (2026-05-28, during 5a pre-flight):** the first draft of + > this scope said "retire lifecycle.py (~1,872 LOC)". A pre-deletion + > trace found `audit.py` imports lifecycle's reader surface and feeds + > the live teaching corridor, so lifecycle.py is **dual-use** and must + > stay. 5a's real payoff is ~580 LOC (the scoring dispatch + adapter + + > flag), not the projected ~1,872. The deeper LOC reduction the parent + > ADR projected does not materialize while the contemplation corridor + > keeps the reader alive. + +### Phase 5a — Retire the inert parallel parser (structural) + +Scope (corrected): +1. Delete both flag-gated reader **dispatch functions** + (`_try_comprehension_reader`, `_try_reader_for_question`) and their + call sites in `math_candidate_graph.py`, plus the `_tokenize_sentence` + helper; drop the `comprehension_reader_questions` config flag and the + `--use-reader` runner plumbing (the recognizer path runs + unconditionally — it is no longer "opt-in reader vs regex," it is the + only scoring path). +2. Delete `generate/comprehension/lifecycle_runtime_adapter.py` (used only + by the question-reader dispatch). **Do NOT delete `lifecycle.py` or + `audit.py`** — both feed the live ADR-0172 contemplation corridor. +3. Remove `tests/test_reader_coexistence.py` (flag-ON/OFF premise gone); + keep `test_reader_phase2.py` / `test_reader_question_frame.py` (they + test `lifecycle.py`, which stays). +4. Leave `state.py` intact. `ProblemReadingState`/`ReaderRefusal` are + load-bearing (contemplate + audit corridor); `EntityRef`/`SentenceState` + remain referenced by the surviving `lifecycle.py`. No state.py trim in + 5a. +5. Collapsing the duplicate per-sentence-choice scaffolding is deferred — + with the question-reader branch gone the dispatch is already single-path; + any further structural collapse is its own follow-up, not load-bearing + for 5a. + +Acceptance (5a): +- train_sample **3/47/0 unchanged** — byte-identical verdicts; this is a + refactor, not a behaviour change. (Honest: ~0 lift is the *expected* + and *correct* outcome of 5a.) +- Net **−1,038 LOC** of code + tests (as shipped: adapter 402 + + `test_reader_coexistence.py` 302 + train_sample delta-report/`use_reader` + plumbing ~96 + the two `math_candidate_graph` dispatch fns/tokenizer + + coverage-CLI `use_reader` field + stale delta artifact, against ~53 lines + of replacement docstrings/comments). Larger than the first ~580 estimate + because the coexistence test and delta-report harness were bigger than + scoped. The parent ADR's ~1,872-line figure still does **not** apply: + `lifecycle.py` stays for the teaching corridor. +- Capability-axis lanes G1–G5, S1 remain 100% `wrong = 0`. +- Determinism / `trace_hash` invariant holds; pinned lane SHAs pass. +- `math_parser.py` baseline lanes untouched (out of scope — keep). + +### Phase 5b — Operation-capability buildout (semantic, the real lift) + +This is where `correct` climbs toward 25. It is **not** a refactor and +carries the live `wrong = 0` risk; it lands as its own sub-phases +(candidate sub-ADR: **ADR-0174.1**) with per-sub-phase wrong=0 obligations, +not as a big bang. + +#### Verified ground truth (2026-05-28 measurement) + +The 47 train_sample refusals were profiled against **GSM8K's own +`<>` calculator annotations** (ground-truth operations, not +brute-forced — brute-force number-matching was tried and rejected for +producing spurious coincidental hits, e.g. matching gold=4 to a stray +`20/5`): + +| Profile | Count | Share | +|---|---|---| +| Uses multiplication (`*`) | **37/47** | 79% | +| Uses mul **or** div | 43/47 | 91% | +| Pure add/subtract | 4/47 | 9% | +| **Single-step solvable** | **0/47** | **0%** | + +Step-count histogram: `2 steps: 13 · 3: 12 · 4: 8 · 5: 8 · 6: 3 · 7: 3`. + +Two load-bearing consequences: + +1. **Multiplication is the foundational capability, not a niche.** At 79% + it is maximally general — building genuine multiplicative + quantity-extraction is "getting generally smarter," the opposite of + overfitting. (Per Shay's framing: a change that flips a large *general* + chunk is a real capability; a per-phrasing patch that flips one case is + overfitting. Breadth-of-impact is the test, not where the code lives.) + +2. **No case flips from an operation matcher alone.** Zero single-step + cases. Multiplication is *necessary but not sufficient* for all 37 — the + unit that flips a case is **operation + composition** (extract quantities + across sentences/question → apply op → combine). A "multiplication + sentence matcher" in isolation flips ~2 cases (only the single-sentence + multiplicative aggregate, 0021-class) and would be overfitting-adjacent. + +The solver is **already capable**: `VALID_OPERATION_KINDS` = +`{add, subtract, transfer, multiply, divide, apply_rate, compare_additive, +compare_multiplicative}` with pack lemmas for each. The gap is entirely the +**reader → injector → `Operation`** front-end: the recognizer matches a +shape category but extracts **zero anchors** on real corpus sentences, and +the injectors only handle narrow Wave-A stub shapes. Phase 5b builds the +front-end down to the operations the solver is already waiting for. + +#### What to build (and what NOT to) + +- **Build:** general quantity-extraction (operands from a clause, across + sentences, and from the question sentence) + injectors that emit + first-class `multiply` / `divide` / `compare_multiplicative` operations + + the multi-step composition spine that chains them. The held-hypothesis / + `eliminate_violating` / `reevaluate` substrate (Phases 1–4) is the + wrong=0 scaffold for chains longer than 2 steps. +- **Do NOT:** widen the `discrete_count_statement` injector to absorb these. + The recognizer *mis-matches* multiplicative/comparative problems as + discrete-count (number + noun present); the injector *correctly* refuses. + Forcing those into a discrete-count frame is overfitting and a wrong=0 + hazard. Route them to the correct operation injector instead. + +#### Sub-phase sequence (biggest-chunk-first, measure-the-flip-gated) + +Each sub-phase is a complete vertical slice (extraction → injector → +existing solver op) whose success is judged by **how many cases flip**, not +by whether a layer was touched: + +- **5b.1 — Single-sentence multiplicative aggregate** (0021-class: + "15 pounds for 10 reps and does 3 sets"). One clause, N operands, one + `multiply` chain — no cross-sentence binding. The cleanest possible + proof-of-capability. Small expected flip (~2–4) but it stands up the + extraction→multiply→solve path end-to-end with wrong=0. +- **5b.2 — Shallow 2–3 step composition** (the real chunk: **25/47** cases + at ≤3 steps). Cross-sentence + question-sentence operand binding (e.g. + 0003: 48 boxes × 24/box × $0.75-in-question), op + one combine. This is + where the bulk of the lift lives. +- **5b.3 — Deep multi-step (4–7 steps, 22/47)** under the held-hypothesis + elimination machinery. Highest wrong=0 surface (long chains = more ways to + admit a wrong answer); last, and only after 5b.1–5b.2 lock the + extraction/composition contracts. + +#### Dependencies / out-of-scope-but-required + +- **Solver gaps (separate solver ADR):** same-actor multi-quantity + aggregation and cross-unit superordinate sums — the reader can parse some + shapes the solver still refuses to compute. Sequence the solver ADR before + the 5b sub-phases that need those shapes. +- **Comparatives** reuse the existing ADR-0131 G2 axis and + `compare_multiplicative` / `compare_additive` solver ops — extend, don't + reinvent. + +#### Acceptance (5b) + +- Per-sub-phase `correct` delta reported with `wrong = 0` held at every + step; case 0050 canary stays refused. +- **Generality guard:** every flipped case must also hold under the + ADR-0114a perturbation / OOD axes. A capability that flips N train_sample + cases but collapses under reworded inputs was overfitting to the sample + and does not count toward the chunk. +- `correct ≥ 25` is the *cumulative* Round-2 target, reached by composition + of sub-phases — never asserted of any single sub-phase. +- Capability-axis lanes G1–G5, S1 remain 100% `wrong = 0`. + +### Sequencing + +- **5a — shipped** (PR #430): single parse path, net −1,038 LOC, 3/47/0 + byte-identical, wrong=0 held. Behaviour-preserving refactor; ~0 lift was + the expected, correct outcome. +- **5b — next**, as its own sub-ADR (ADR-0174.1): 5b.1 → 5b.2 → 5b.3, + biggest-chunk-first, each gated by measured flip-count + the perturbation + generality guard + wrong=0. The solver ADR (multi-quantity / cross-unit + sums) sequences before the 5b sub-phases that depend on it. + +--- diff --git a/evals/gsm8k_math/runner.py b/evals/gsm8k_math/runner.py index 33dc50bc..cd3f5dbd 100644 --- a/evals/gsm8k_math/runner.py +++ b/evals/gsm8k_math/runner.py @@ -31,12 +31,9 @@ from __future__ import annotations import hashlib import json from dataclasses import dataclass, field -from typing import TYPE_CHECKING, Any +from typing import Any from generate.math_candidate_graph import parse_and_solve - -if TYPE_CHECKING: - from core.config import RuntimeConfig from generate.math_parser import ParseError, parse_problem from generate.math_problem_graph import MathProblemGraph from generate.math_realizer import RealizerError, realize @@ -240,7 +237,6 @@ def _score_one(case: dict[str, Any]) -> CaseOutcome: # the canonical run.honest_runner.json artifact can be trusted for cross-phase comparison. def _score_one_candidate_graph( case: dict[str, Any], - config: "RuntimeConfig | None" = None, ) -> CaseOutcome: """ADR-0126 P4 — score one case via the candidate-graph pipeline. @@ -263,16 +259,13 @@ def _score_one_candidate_graph( Args: case: Case record with keys ``id``, ``problem``, ``expected_answer``, ``expected_unit``. - config: Optional :class:`core.config.RuntimeConfig`. Passed through - to :func:`generate.math_candidate_graph.parse_and_solve`. When - None, flag-OFF (default) behaviour is preserved. """ case_id = case["id"] expected_answer = case["expected_answer"] expected_unit = case["expected_unit"] # Stage 1 — candidate-graph parse + internal solve + decision rule. - cg_result = parse_and_solve(case["problem"], config=config) + cg_result = parse_and_solve(case["problem"]) if not cg_result.is_admitted: return CaseOutcome( case_id=case_id, diff --git a/evals/gsm8k_math/train_sample/v1/reader_phase1_delta.json b/evals/gsm8k_math/train_sample/v1/reader_phase1_delta.json deleted file mode 100644 index d5c54a69..00000000 --- a/evals/gsm8k_math/train_sample/v1/reader_phase1_delta.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "adr": "0164", - "baseline_off": { - "correct": 3, - "refused": 47, - "wrong": 0 - }, - "changed_cases": [], - "delta": { - "correct": 0, - "refused": 0, - "wrong": 0 - }, - "phase": 1, - "reader_on": { - "correct": 3, - "refused": 47, - "wrong": 0 - }, - "schema_version": 1 -} diff --git a/evals/gsm8k_math/train_sample/v1/report.json b/evals/gsm8k_math/train_sample/v1/report.json index bb4dc415..5a39a29f 100644 --- a/evals/gsm8k_math/train_sample/v1/report.json +++ b/evals/gsm8k_math/train_sample/v1/report.json @@ -143,7 +143,7 @@ }, { "case_id": "gsm8k-train-sample-v1-0027", - "reason": "candidate_graph: recognizer matched but produced no injection for statement: 'Malcolm has 240 followers on Instagram and 500 followers on Facebook.' (category=discrete_count_statement)", + "reason": "candidate_graph: recognizer matched but produced no injection for statement: 'The number of followers he has on Twitter is half the number of followers he has on Instagram and Facebook combined.' (category=descriptive_setup_no_quantity)", "verdict": "refused" }, { @@ -264,6 +264,5 @@ ], "sample_count": 50, "sample_path": "evals/gsm8k_math/train_sample/v1/cases.jsonl", - "schema_version": 1, - "use_reader": true + "schema_version": 1 } diff --git a/evals/gsm8k_math/train_sample/v1/runner.py b/evals/gsm8k_math/train_sample/v1/runner.py index 3d5e0dae..87195dfc 100644 --- a/evals/gsm8k_math/train_sample/v1/runner.py +++ b/evals/gsm8k_math/train_sample/v1/runner.py @@ -17,11 +17,9 @@ unit annotation, so ``expected_unit`` is normalized to ``""`` which :func:`evals.gsm8k_math.runner._score_one` already treats as "skip unit comparison". -CLI flags: - - --use-reader Activate the ADR-0164 Phase-1 comprehension reader for - question sentences (RuntimeConfig.comprehension_reader_questions - = True). Default: False (flag-OFF, byte-identical to today). +ADR-0174 Phase 5a: the ``--use-reader`` flag (and the flag-ON/OFF delta +report) were removed — the recognizer/candidate-graph path is now the +single scoring path. """ from __future__ import annotations @@ -36,7 +34,6 @@ from evals.gsm8k_math.runner import _score_one_candidate_graph _HERE = Path(__file__).resolve().parent _CASES_PATH = _HERE / "cases.jsonl" _REPORT_PATH = _HERE / "report.json" -_DELTA_PATH = _HERE / "reader_phase1_delta.json" _SAMPLE_REL = "evals/gsm8k_math/train_sample/v1/cases.jsonl" _EXPECTED_COUNT = 50 _CORRECT_MIN = 10 @@ -74,27 +71,21 @@ def _adapt(case: dict[str, Any]) -> dict[str, Any]: } -def build_report( - cases: list[dict[str, Any]], - use_reader: bool = False, -) -> dict[str, Any]: +def build_report(cases: list[dict[str, Any]]) -> dict[str, Any]: """Build the measurement report for the train-sample cases. Args: cases: Loaded case records from cases.jsonl. - use_reader: When True, activates the ADR-0164 Phase-1 comprehension - reader for question sentences via RuntimeConfig.comprehension_reader_questions. - Default False preserves byte-identical behaviour with today. - """ - config = None - if use_reader: - from core.config import RuntimeConfig - config = RuntimeConfig(comprehension_reader_questions=True) + ADR-0174 Phase 5a: the flag-gated incremental reader was retired; the + recognizer/candidate-graph path is the single scoring path, so the + former ``use_reader`` A/B switch (and its delta report) no longer + exist — there is only one path to measure. + """ per_case: list[dict[str, Any]] = [] counts = {"correct": 0, "wrong": 0, "refused": 0} for raw in cases: - outcome = _score_one_candidate_graph(_adapt(raw), config=config) + outcome = _score_one_candidate_graph(_adapt(raw)) counts[outcome.outcome] += 1 per_case.append( { @@ -109,7 +100,6 @@ def build_report( "adr": "0126", "sample_path": _SAMPLE_REL, "sample_count": len(cases), - "use_reader": use_reader, "counts": counts, "exit_criterion": { "correct_min": _CORRECT_MIN, @@ -127,73 +117,15 @@ def write_report(report: dict[str, Any], path: Path = _REPORT_PATH) -> None: ) -def build_delta_report( - baseline: dict[str, Any], - reader_on: dict[str, Any], -) -> dict[str, Any]: - """Compute per-case delta between flag-OFF and flag-ON reports. - - Returns a JSON-serialisable dict with: - - summary counts - - per-case attribution for every case that changed verdict - """ - base_map = {p["case_id"]: p for p in baseline["per_case"]} - on_map = {p["case_id"]: p for p in reader_on["per_case"]} - changed: list[dict[str, Any]] = [] - for cid, on_case in on_map.items(): - base_case = base_map.get(cid, {}) - if base_case.get("verdict") != on_case["verdict"]: - changed.append( - { - "case_id": cid, - "prior_verdict": base_case.get("verdict"), - "prior_refusal_reason": base_case.get("reason"), - "new_verdict": on_case["verdict"], - "new_reason": on_case.get("reason"), - } - ) - bc = baseline["counts"] - rc = reader_on["counts"] - return { - "schema_version": 1, - "adr": "0164", - "phase": 1, - "baseline_off": {"correct": bc["correct"], "refused": bc["refused"], "wrong": bc["wrong"]}, - "reader_on": {"correct": rc["correct"], "refused": rc["refused"], "wrong": rc["wrong"]}, - "delta": { - "correct": rc["correct"] - bc["correct"], - "refused": rc["refused"] - bc["refused"], - "wrong": rc["wrong"] - bc["wrong"], - }, - "changed_cases": changed, - } - - def main() -> int: """Run the train-sample measurement. - When ``--use-reader`` is passed, also generates a delta report at - ``reader_phase1_delta.json`` comparing flag-OFF vs flag-ON results. + ADR-0174 Phase 5a: the incremental reader was retired, so there is a + single scoring path and no flag-ON/OFF delta to compute. """ - use_reader = "--use-reader" in sys.argv - cases = _load_cases(_CASES_PATH) - - if use_reader: - # Run baseline (flag OFF) first, then reader-on. - baseline_report = build_report(cases, use_reader=False) - reader_report = build_report(cases, use_reader=True) - write_report(reader_report) - delta = build_delta_report(baseline_report, reader_report) - _DELTA_PATH.write_text( - json.dumps(delta, indent=2, sort_keys=True) + "\n", - encoding="utf-8", - ) - report = reader_report - else: - report = build_report(cases, use_reader=False) - write_report(report) - + report = build_report(cases) + write_report(report) return 0 if report["exit_criterion"]["passed"] else 1 diff --git a/generate/comprehension/lifecycle_runtime_adapter.py b/generate/comprehension/lifecycle_runtime_adapter.py deleted file mode 100644 index 3dc842bf..00000000 --- a/generate/comprehension/lifecycle_runtime_adapter.py +++ /dev/null @@ -1,402 +0,0 @@ -"""ADR-0164 Phase 1 — bridge from regex-parser candidates to reader state. - -Converts CandidateInitial / SentenceChoice tuples produced by the existing -regex parser into a ProblemReadingState that the comprehension reader can -consume for question-sentence processing. - -This module is the only place where the Phase 1 coexistence wiring knows -about both worlds simultaneously. It is intentionally a stopgap: Phase 3 -(per ADR-0164 §Phasing Phase 3) removes the regex question parser entirely, -at which point this adapter either shrinks to a pure statement-candidate -helper or is deleted. - -All public functions are pure and deterministic (same inputs → same outputs, -no I/O, no global state mutation). -""" - -from __future__ import annotations - -import json -import re -from typing import TYPE_CHECKING, Final, Union - -from generate.comprehension.lifecycle import ( - _classify, - _get_lexicon, - apply_word, - begin_sentence, - end_sentence, -) -from generate.comprehension.state import ( - EntityRef, - PartialInitialPossession, - PartialOperation, - ProblemReadingState, - QuestionTargetSlot, - QuantityRef, - ReaderRefusal, - SentenceReadingState, -) - -if TYPE_CHECKING: - from generate.math_candidate_parser import CandidateInitial - from generate.math_roundtrip import CandidateOperation - -# Union type for statement-sentence choices (mirrors math_candidate_graph). -SentenceChoice = Union["CandidateInitial", "CandidateOperation"] - -# --------------------------------------------------------------------------- -# Gender inference via lexicon -# --------------------------------------------------------------------------- - -_FEMALE_CATEGORIES: Final[frozenset[str]] = frozenset({"proper_noun_entity_female"}) -_MALE_CATEGORIES: Final[frozenset[str]] = frozenset({"proper_noun_entity_male"}) - -_UNIT_CLASS_CATEGORIES: Final[dict[str, str]] = { - "count_unit_noun": "count", - "currency_unit_noun": "currency", - "time_unit_noun": "time", -} - - -def _infer_gender(entity_name: str) -> str: - """Return 'female', 'male', or 'unknown' for a proper-noun entity. - - Consults the en_core_math_v1 lexicon (via the lifecycle's cached loader) - per ADR-0164.2 gender-inference policy. Defaults to 'unknown' when the - name is absent from the lexicon. - """ - lex = _get_lexicon() - key = entity_name.lower() - entry = lex.get(key) - if entry is None: - return "unknown" - if entry.category in _FEMALE_CATEGORIES: - return "female" - if entry.category in _MALE_CATEGORIES: - return "male" - return "unknown" - - -# --------------------------------------------------------------------------- -# Build ProblemReadingState from regex-parser output -# --------------------------------------------------------------------------- - -def build_problem_state_from_candidates( - statement_choices: list[SentenceChoice], - statement_sentence_count: int, -) -> ProblemReadingState: - """Convert regex-parser output into a ProblemReadingState for reader consumption. - - Args: - statement_choices: Admissible CandidateInitial / CandidateOperation - tuples produced by the existing regex parser, in source-text order. - statement_sentence_count: Number of statement sentences already - processed (sets ``ProblemReadingState.sentence_index``). - - Returns: - A ProblemReadingState with entity_registry, accumulated_initial_state, - and accumulated_operations populated from the candidates. - unknown_target_slot is None (the question hasn't been processed yet). - - This function is the glue layer for Phase 1 coexistence. It does NOT - attempt to reproduce the reader's full incremental behaviour for statement - sentences — that is Phase 2 scope. It produces only what the reader's - pronoun-resolution step needs: an ordered entity registry. - """ - from generate.math_candidate_parser import CandidateInitial as _CI - from generate.math_roundtrip import CandidateOperation as _CO - - entity_registry: list[EntityRef] = [] - seen_names: set[str] = set() - accumulated_initials: list[PartialInitialPossession] = [] - accumulated_ops: list[PartialOperation] = [] - - char_offset = 0 - for choice in statement_choices: - if isinstance(choice, _CI): - entity_name = choice.initial.entity - if entity_name not in seen_names: - gender = _infer_gender(entity_name) - entity_registry.append( - EntityRef( - canonical_name=entity_name, - gender=gender, - first_mention_position=len(seen_names), - ) - ) - seen_names.add(entity_name) - # Convert InitialPossession to PartialInitialPossession - from decimal import Decimal - qty_val = choice.initial.quantity.value - qty = QuantityRef( - value=Decimal(str(qty_val)), - unit=choice.initial.quantity.unit, - unit_class=None, - owner_entity=entity_name, - mention_position=len(accumulated_initials), - ) - accumulated_initials.append( - PartialInitialPossession(entity=entity_name, quantity=qty) - ) - elif isinstance(choice, _CO): - actor = choice.op.actor - if actor not in seen_names: - gender = _infer_gender(actor) - entity_registry.append( - EntityRef( - canonical_name=actor, - gender=gender, - first_mention_position=len(seen_names), - ) - ) - seen_names.add(actor) - if choice.op.target is not None and choice.op.target not in seen_names: - tgt = choice.op.target - gender_t = _infer_gender(tgt) - entity_registry.append( - EntityRef( - canonical_name=tgt, - gender=gender_t, - first_mention_position=len(seen_names), - ) - ) - seen_names.add(tgt) - # Operand — may be Quantity or Comparison; only carry scalar Quantity - from generate.math_problem_graph import Quantity - operand_ref: QuantityRef | None = None - if hasattr(choice.op, "operand") and isinstance(choice.op.operand, Quantity): - from decimal import Decimal - operand_ref = QuantityRef( - value=Decimal(str(choice.op.operand.value)), - unit=choice.op.operand.unit, - unit_class=None, - owner_entity=actor, - mention_position=len(accumulated_ops), - ) - accumulated_ops.append( - PartialOperation( - actor=actor, - kind=choice.op.kind, - operand=operand_ref, - target=choice.op.target, - ) - ) - - return ProblemReadingState( - entity_registry=tuple(entity_registry), - accumulated_initial_state=tuple(accumulated_initials), - accumulated_operations=tuple(accumulated_ops), - unknown_target_slot=None, - pronoun_resolution_history=(), - sentence_index=statement_sentence_count, - source_text_offset=char_offset, - ) - - -# --------------------------------------------------------------------------- -# Tokenisation (matches the reader's apply_word loop convention) -# --------------------------------------------------------------------------- - -_TOKEN_SPLIT_RE: Final[re.Pattern[str]] = re.compile(r"\s+") -_PUNCT_STRIP_RE: Final[re.Pattern[str]] = re.compile(r"^[\"'()\[\]{}<>]+|[\"'()\[\]{}<>]+$") - - -def _tokenise_sentence(sentence: str) -> list[str]: - """Split a sentence into tokens, emitting punctuation as separate tokens. - - Trailing ``?`` and ``.`` become their own token (matched by primitive scanner - as ``question_terminator`` / ``statement_terminator``). Leading/trailing - matched-pair punctuation is stripped per word. Empty strings are dropped. - """ - tokens: list[str] = [] - for raw in _TOKEN_SPLIT_RE.split(sentence.strip()): - if not raw: - continue - # Separate a trailing '?' or '.' from the word body. - if len(raw) > 1 and raw[-1] in "?.!": - body = raw[:-1] - tail = raw[-1] - else: - body = raw - tail = None - body = _PUNCT_STRIP_RE.sub("", body) - if body: - tokens.append(body) - if tail: - tokens.append(tail) - return tokens - - -# --------------------------------------------------------------------------- -# Unit extraction from question sentence -# --------------------------------------------------------------------------- - -def _extract_unit_from_question(question_sentence: str, unit_class: str) -> str | None: - """Scan question tokens for a unit-noun surface word matching ``unit_class``. - - After the reader produces a QuestionTargetSlot with unit_class set, this - helper re-tokenises the question to find the specific unit word. This lets - the projected Unknown carry the actual unit string (e.g. 'apples') rather - than the abstract class ('count'), maximising match probability against - statement candidates' unit strings. - - Returns the canonicalised unit string, or None when no unit noun is found - with the expected class. - """ - from generate.math_candidate_parser import _canonicalize_unit # type: ignore[attr-defined] - target_categories = { - "count": frozenset({"count_unit_noun"}), - "currency": frozenset({"currency_unit_noun"}), - "time": frozenset({"time_unit_noun"}), - }.get(unit_class, frozenset()) - if not target_categories: - return None - for tok in _tokenise_sentence(question_sentence): - cat, _surface = _classify(tok) - if cat in target_categories: - return _canonicalize_unit(tok) - return None - - -# --------------------------------------------------------------------------- -# Run the reader over a question sentence -# --------------------------------------------------------------------------- - -def invoke_reader_for_question( - question_sentence: str, - problem_state: ProblemReadingState, -) -> tuple[QuestionTargetSlot, str] | ReaderRefusal: - """Run the Phase-1 reader over one question sentence. - - Returns: - On success: ``(QuestionTargetSlot, canonical_unit)`` where - ``canonical_unit`` is the actual unit string extracted from the - question tokens (may differ from ``slot.unit_class``). - On refusal: ``ReaderRefusal``. - - The caller is responsible for wrapping the result in a CandidateUnknown - and for emitting the trace event. - """ - tokens = _tokenise_sentence(question_sentence) - sentence_state: SentenceReadingState = begin_sentence( - problem_state, source_text_offset=problem_state.source_text_offset - ) - for tok in tokens: - result = apply_word(sentence_state, problem_state, tok) - if isinstance(result, ReaderRefusal): - return result - sentence_state = result - - end_result = end_sentence(sentence_state, problem_state) - if isinstance(end_result, ReaderRefusal): - return end_result - - # end_sentence succeeded — extract QuestionTargetSlot from the new - # problem_state (it was just committed as unknown_target_slot). - slot = end_result.unknown_target_slot - if slot is None: - return ReaderRefusal( - reason="no_question_target", - detail="end_sentence succeeded but no unknown_target_slot set", - sentence_index=problem_state.sentence_index, - token_index=len(tokens), - token_text="", - ) - - # Extract the canonical unit string from the question surface. - unit_class = slot.unit_class or "unknown" - canonical_unit = _extract_unit_from_question(question_sentence, unit_class) - if canonical_unit is None: - # Fall back to unit_class as the unit string per ADR-0164 Brief-9 spec. - canonical_unit = unit_class - - return slot, canonical_unit - - -# --------------------------------------------------------------------------- -# Project QuestionTargetSlot → CandidateUnknown -# --------------------------------------------------------------------------- - -def project_to_candidate_unknown( - slot: QuestionTargetSlot, - canonical_unit: str, - question_sentence: str, - problem_state: ProblemReadingState, -) -> "CandidateUnknown | None": # type: ignore[name-defined] - """Convert a QuestionTargetSlot into a CandidateUnknown for the candidate graph. - - Returns None if the projection would produce an invalid Unknown (e.g., the - entity is set but not in the problem_state entity registry, which would - cause _build_graph to reject it). - - Modifier flags (aggregate, comparative, residual) from the reader's - lookback are not threaded into Unknown (Unknown has only entity + unit - fields per ADR-0115). Deferral documented here; a follow-up ADR will - extend BoundUnknown resolution to consume these flags via side-channel. - """ - from generate.math_candidate_parser import CandidateUnknown, _canonicalize_unit - from generate.math_problem_graph import Unknown - - entity: str | None = slot.entity - # Validate entity against the registry when set. - if entity is not None: - known = {e.canonical_name for e in problem_state.entity_registry} - if entity not in known: - return None - - matched_unit_token = canonical_unit - matched_entity_token = entity - - try: - unknown = Unknown(entity=entity, unit=canonical_unit) - except Exception: - return None - - try: - return CandidateUnknown( - unknown=unknown, - source_span=question_sentence, - matched_unit_token=matched_unit_token, - matched_entity_token=matched_entity_token, - ) - except Exception: - return None - - -# --------------------------------------------------------------------------- -# Trace-event construction -# --------------------------------------------------------------------------- - -def make_admit_trace_event( - slot: QuestionTargetSlot, - canonical_unit: str, -) -> str: - """Build a JSON-encoded admit trace event for the reader.""" - return json.dumps( - { - "layer": "comprehension_reader", - "phase": 1, - "outcome": "admit", - "entity": slot.entity, - "unit": canonical_unit, - "question_form": slot.kind, - }, - sort_keys=True, - separators=(",", ":"), - ) - - -def make_fallthrough_trace_event(refusal: ReaderRefusal) -> str: - """Build a JSON-encoded fallthrough trace event for the reader.""" - return json.dumps( - { - "layer": "comprehension_reader", - "phase": 1, - "outcome": "fallthrough_to_regex", - "refusal_reason": refusal.reason, - "refusal_token": refusal.token_text, - }, - sort_keys=True, - separators=(",", ":"), - ) diff --git a/generate/math_candidate_graph.py b/generate/math_candidate_graph.py index 7b9f9da0..397685b8 100644 --- a/generate/math_candidate_graph.py +++ b/generate/math_candidate_graph.py @@ -38,10 +38,7 @@ import re from collections.abc import Mapping from dataclasses import dataclass from itertools import product -from typing import TYPE_CHECKING, Final, Union - -if TYPE_CHECKING: - from core.config import RuntimeConfig +from typing import Final, Union from generate.comprehension.state import Hypothesis from generate.math_candidate_parser import ( @@ -121,11 +118,10 @@ class CandidateGraphResult: # Diagnostics for inner-loop signal in P6 runner. branches_enumerated: int branches_admissible: int - # ADR-0164 Phase 1 — reader trace events (JSON-encoded strings). - # Each element is a JSON object carrying {"layer", "phase", "outcome", ...}. - # Empty tuple when comprehension_reader_questions flag is False (default). - # Deferred: full integration with chat/telemetry.py JSONL sink is a - # follow-up; these events are available for tests and delta-report analysis. + # Reader trace events (JSON-encoded strings). Each element is a JSON + # object carrying {"layer", "phase", "outcome", ...}. Carries the + # ADR-0174 Phase-2 constraint-elimination events from the recognizer + # path. Deferred: full integration with chat/telemetry.py JSONL sink. reader_trace: tuple[str, ...] = () @property @@ -358,69 +354,6 @@ def _collapse_per_sentence_ties( return [c for c in choices if _slot_count(c) == max_slots] -# --------------------------------------------------------------------------- -# ADR-0164 Phase 1 — comprehension reader dispatch helper -# --------------------------------------------------------------------------- - -def _try_reader_for_question( - question_sentence: str, - per_sentence_choices: list[list[SentenceChoice]], - statement_sentence_count: int, - trace_out: list[str], -) -> list[CandidateUnknown] | None: - """Invoke the Phase-1 comprehension reader for the question sentence. - - Returns a list with one CandidateUnknown on reader admission, or None - when the reader refuses (caller falls through to the regex parser). - - Appends a JSON-encoded trace event to ``trace_out`` on every invocation - (admit or fallthrough_to_regex). - - This function is the hybrid-dispatch core for ADR-0164 Phase 1. The - fallthrough path (reader refusal → regex) is intentional and must never - raise: the reader is purely additive at Phase 1. - """ - try: - from generate.comprehension.lifecycle_runtime_adapter import ( - build_problem_state_from_candidates, - invoke_reader_for_question, - make_admit_trace_event, - make_fallthrough_trace_event, - project_to_candidate_unknown, - ) - except ImportError: - return None # adapter not available — fall through silently - - # Flatten per_sentence_choices to a single list for state construction. - # Take the first choice per sentence (deterministic: tiebreaker already ran). - flat: list[SentenceChoice] = [choices[0] for choices in per_sentence_choices if choices] - try: - problem_state = build_problem_state_from_candidates(flat, statement_sentence_count) - except Exception: - return None # construction failure → fall through - - result = invoke_reader_for_question(question_sentence, problem_state) - if isinstance(result, tuple): - slot, canonical_unit = result - trace_out.append(make_admit_trace_event(slot, canonical_unit)) - candidate = project_to_candidate_unknown( - slot, canonical_unit, question_sentence, problem_state - ) - if candidate is not None and _question_admissible(candidate): - return [candidate] - # Reader admitted but projection failed or failed admissibility. - # Do NOT fall through to regex (the reader's admission is authoritative - # on what it could parse; if projection fails it's a structural gap, - # not a reason to let the regex guess differently). - return None - else: - # ReaderRefusal — fall through to regex. - from generate.comprehension.state import ReaderRefusal - if isinstance(result, ReaderRefusal): - trace_out.append(make_fallthrough_trace_event(result)) - return None - - # --------------------------------------------------------------------------- # Graph construction from one branch # --------------------------------------------------------------------------- @@ -473,127 +406,16 @@ def _build_graph( return None -# --------------------------------------------------------------------------- -# Comprehension reader integration (ADR-0164 Phase 2) -# --------------------------------------------------------------------------- - - -def _tokenize_sentence(sentence: str) -> list[str]: - """Split a sentence string into tokens for the reader. - - Handles punctuation attachment: "." and "?" are emitted as separate - tokens if attached to a word (e.g. "dollars." → ["dollars", "."]). - Commas are split off too. - """ - import re as _re - raw = _re.split(r"\s+", sentence.strip()) - out: list[str] = [] - for tok in raw: - if not tok: - continue - # Detach trailing punctuation. - if len(tok) > 1 and tok[-1] in (".", "?", "!"): - out.append(tok[:-1]) - out.append(tok[-1]) - elif len(tok) > 1 and tok[-1] == ",": - out.append(tok[:-1]) - out.append(",") - else: - out.append(tok) - return [t for t in out if t] - - -def _try_comprehension_reader(text: str) -> CandidateGraphResult | None: - """Attempt to parse and solve *text* via the incremental reader. - - Returns a :class:`CandidateGraphResult` on success or reader refusal, - or ``None`` if the reader itself errors unexpectedly (caller falls - through to regex path). - - All-or-nothing: if the reader refuses on ANY sentence, returns None - so the existing regex path can try. This guarantees wrong == 0 — - the reader either produces a verified graph or steps aside. - """ - try: - from generate.comprehension.lifecycle import ( - apply_word, - begin_sentence, - end_sentence, - finalize, - ) - from generate.comprehension.state import ( - EntityRef, - ProblemReadingState, - ReaderRefusal, - ) - from generate.math_solver import SolveError, solve - except Exception: - return None - - sentences = _split_sentences(text) - if not sentences: - return None - - ps = ProblemReadingState( - entity_registry=(), - accumulated_initial_state=(), - accumulated_operations=(), - unknown_target_slot=None, - pronoun_resolution_history=(), - sentence_index=0, - source_text_offset=0, - ) - - for sentence in sentences: - tokens = _tokenize_sentence(sentence) - ss = begin_sentence(ps, ps.source_text_offset) - for tok in tokens: - result = apply_word(ss, ps, tok) - if isinstance(result, ReaderRefusal): - return None # fall through to regex - ss = result - end = end_sentence(ss, ps) - if isinstance(end, ReaderRefusal): - return None # fall through to regex - ps = end - - graph_or_refusal = finalize(ps) - if isinstance(graph_or_refusal, ReaderRefusal): - return None # fall through to regex - - graph = graph_or_refusal - try: - answer = solve(graph) - except (SolveError, Exception): - return None # fall through to regex - - return CandidateGraphResult( - answer=answer, - selected_graph=graph, - refusal_reason=None, - branches_enumerated=1, - branches_admissible=1, - ) - - # --------------------------------------------------------------------------- # Orchestrator # --------------------------------------------------------------------------- -def parse_and_solve( - text: str, - config: "RuntimeConfig | None" = None, -) -> CandidateGraphResult: +def parse_and_solve(text: str) -> CandidateGraphResult: """End-to-end: parse text via candidate-graph topology, solve each admissible branch, apply decision rule. Args: text: The problem text to parse. - config: Optional :class:`core.config.RuntimeConfig`. When None, - defaults to flag-OFF behaviour (byte-identical to today). - Pass ``RuntimeConfig(comprehension_reader_questions=True)`` to - activate the ADR-0164 Phase-1 comprehension reader for question - sentences. Returns :class:`CandidateGraphResult` with either an admitted ``answer`` + ``selected_graph`` or a ``refusal_reason`` string @@ -607,11 +429,14 @@ def parse_and_solve( filter at the per-sentence level (already applied during filtering). - Branches that disagree on the final answer trigger refusal. - - When the comprehension reader is active (flag ON), a reader refusal - falls through to the existing regex parser — the reader is purely - additive. A reader admission that produces wrong > 0 cannot occur - because the same admissibility gate, solver, and verifier run - downstream of the reader as they run today. + + ADR-0174 Phase 5a: the recognizer/candidate-graph path is the single + canonical reader. The flag-gated incremental-reader dispatch + (``_try_comprehension_reader`` / ``_try_reader_for_question``) was + retired — it admitted 0/50 on train_sample and only added a dead + fall-through. ``lifecycle.py`` itself survives because the ADR-0172 + contemplation corridor (``comprehension/audit.py`` → + ``teaching/math_*``) still uses its reader surface. """ if not isinstance(text, str) or not text.strip(): return CandidateGraphResult( @@ -620,17 +445,6 @@ def parse_and_solve( branches_enumerated=0, branches_admissible=0, ) - # ADR-0164 Phase 2 — comprehension reader path (flag-gated, off by default). - # All-or-nothing: reader either solves the whole problem or falls through. - # Reuses the same RuntimeConfig.comprehension_reader_questions flag that - # Phase 1 introduced (#331). Whole-problem reader takes priority over the - # Phase 1 question-only hybrid path because finalize() emits a complete - # MathProblemGraph that the existing solver consumes unchanged. - if config is not None and config.comprehension_reader_questions: - reader_result = _try_comprehension_reader(text) - if reader_result is not None: - return reader_result - sentences = _split_sentences(text) if not sentences: return CandidateGraphResult( @@ -1156,35 +970,13 @@ def parse_and_solve( if _head is not None: _prior_subject = _head - # ADR-0164 Phase 1 — comprehension reader hybrid dispatch. - # When comprehension_reader_questions is True, try the reader FIRST. - # On reader admission, use the reader's CandidateUnknown; on refusal, - # fall through to the existing regex question parser (Pattern A/B/C). - # The reader is purely additive: a refusal MUST NOT prevent admission - # by the regex parser. # ADR-0174 Phase 2 — seed reader_trace with statement-stage - # constraint-propagation events so consumers see Phase-1 (ADR-0164) - # reader events and Phase-2 (ADR-0174) elimination events in one - # ordered stream. + # constraint-propagation events so consumers still see the Phase-2 + # elimination events in one ordered stream. (ADR-0174 Phase 5a: the + # flag-gated question-reader dispatch was retired; the recognizer + # question parser is the single path.) reader_trace: list[str] = list(_statement_trace) - reader_question_choices: list[CandidateUnknown] | None = None - _use_reader = ( - config is not None and config.comprehension_reader_questions - ) - if _use_reader: - reader_question_choices = _try_reader_for_question( - question_sentences[0], - per_sentence_choices, - len(statement_sentences), - reader_trace, - ) - - # Fall through to the regex parser when the flag is off OR the reader - # refused on the question sentence. - if reader_question_choices is not None: - question_choices = reader_question_choices - else: - question_choices = _filtered_question_choices(question_sentences[0], text) + question_choices = _filtered_question_choices(question_sentences[0], text) if not question_choices: return CandidateGraphResult( diff --git a/teaching/coverage.py b/teaching/coverage.py index 9c48cbd5..ab627087 100644 --- a/teaching/coverage.py +++ b/teaching/coverage.py @@ -38,7 +38,6 @@ class CoverageReport: counts: CoverageCounts refusal_taxonomy: Mapping[str, int] case_0050_verdict: str | None - use_reader: bool delta: Mapping[str, int] = field(default_factory=dict) def as_dict(self) -> dict[str, object]: @@ -54,7 +53,6 @@ class CoverageReport: }, "refusal_taxonomy": dict(self.refusal_taxonomy), "case_0050_verdict": self.case_0050_verdict, - "use_reader": self.use_reader, "delta": dict(self.delta), } @@ -98,7 +96,6 @@ def build_coverage_report( lane: str, split: str, version: str, - use_reader: bool, baseline_path: Path | None = None, ) -> CoverageReport: """Build a :class:`CoverageReport` from a runner-emitted report.json. @@ -153,7 +150,6 @@ def build_coverage_report( counts=counts, refusal_taxonomy=sorted_taxonomy, case_0050_verdict=case_0050_verdict, - use_reader=use_reader, delta=delta, ) diff --git a/tests/test_adr_0174_phase3_lookback.py b/tests/test_adr_0174_phase3_lookback.py index 7859667c..eb2a4329 100644 --- a/tests/test_adr_0174_phase3_lookback.py +++ b/tests/test_adr_0174_phase3_lookback.py @@ -461,7 +461,7 @@ class TestWrongZeroPreservation: for line in Path(_CASES_PATH).open(encoding="utf-8") if line.strip() ] - report = build_report(cases, use_reader=True) + report = build_report(cases) counts = report["counts"] assert counts["wrong"] == 0, ( f"wrong=0 invariant violated: {counts}" diff --git a/tests/test_adr_0174_phase3b_compound_clause.py b/tests/test_adr_0174_phase3b_compound_clause.py index a31be1bc..04f1935f 100644 --- a/tests/test_adr_0174_phase3b_compound_clause.py +++ b/tests/test_adr_0174_phase3b_compound_clause.py @@ -228,7 +228,7 @@ class TestWrongZeroPreservation: cases = [ json.loads(l) for l in Path(_CASES_PATH).open() if l.strip() ] - report = build_report(cases, use_reader=True) + report = build_report(cases) assert report["counts"]["wrong"] == 0 def test_case_0050_remains_refused(self) -> None: diff --git a/tests/test_adr_0174_phase4_contemplate.py b/tests/test_adr_0174_phase4_contemplate.py index 4ba7380c..fb8e5d50 100644 --- a/tests/test_adr_0174_phase4_contemplate.py +++ b/tests/test_adr_0174_phase4_contemplate.py @@ -373,7 +373,7 @@ class TestWrongZeroPreservation: cases = [ json.loads(line) for line in Path(_CASES_PATH).open() if line.strip() ] - report = build_report(cases, use_reader=True) + report = build_report(cases) assert report["counts"]["wrong"] == 0 def test_case_0050_remains_refused(self) -> None: diff --git a/tests/test_rat1_end_to_end_admission.py b/tests/test_rat1_end_to_end_admission.py index 4b1b3de4..8002e4b9 100644 --- a/tests/test_rat1_end_to_end_admission.py +++ b/tests/test_rat1_end_to_end_admission.py @@ -120,8 +120,7 @@ def test_wrong_zero_preserved_on_train_sample(): import sys result = subprocess.run( - [sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner", - "--use-reader"], + [sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner"], cwd=_repo_root(), capture_output=True, text=True, @@ -141,8 +140,7 @@ def test_case_0050_remains_refused_after_rat1(): import sys subprocess.run( - [sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner", - "--use-reader"], + [sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner"], cwd=_repo_root(), capture_output=True, ) diff --git a/tests/test_reader_coexistence.py b/tests/test_reader_coexistence.py deleted file mode 100644 index 9b9afedd..00000000 --- a/tests/test_reader_coexistence.py +++ /dev/null @@ -1,302 +0,0 @@ -"""ADR-0164 Phase 1 — reader/regex coexistence integration tests. - -Verifies: - 1. Flag-OFF byte-identity: parse_and_solve without config == parse_and_solve - with comprehension_reader_questions=False on the 3 currently-correct cases. - 2. Flag-ON determinism: identical input + flag ON → identical reader_trace, - answer, and graph canonical bytes. - 3. wrong=0 invariant: flag ON never produces a wrong outcome on the - 50-case train_sample. - 4. Trace shape: every reader_trace element is valid JSON with the expected - layer/phase/outcome keys. - 5. Brief-8 targets: reader is consulted (non-empty reader_trace) for the - 5 GSM8K question sentences referenced in ADR-0164.3 §Worked example. - 6. Fallthrough preserved: flag ON on an unknown-word question produces a - fallthrough trace event and the same answer as flag OFF. -""" - -from __future__ import annotations - -import json -from pathlib import Path -from typing import Any - -import pytest - -from core.config import RuntimeConfig -from generate.math_candidate_graph import parse_and_solve - -_CASES_PATH = Path(__file__).resolve().parents[1] / "evals/gsm8k_math/train_sample/v1/cases.jsonl" - - -def _load_cases() -> list[dict[str, Any]]: - return [json.loads(l) for l in _CASES_PATH.open(encoding="utf-8") if l.strip()] - - -def _adapt(case: dict[str, Any]) -> dict[str, Any]: - return { - "id": case["case_id"], - "problem": case["question"], - "expected_answer": case["answer_numeric"], - "expected_unit": "", - } - - -# --------------------------------------------------------------------------- -# Fixtures -# --------------------------------------------------------------------------- - -_CONFIG_OFF = RuntimeConfig(comprehension_reader_questions=False) -_CONFIG_ON = RuntimeConfig(comprehension_reader_questions=True) - -# 3 currently-correct cases (fast-path, reader does not affect them) -_CORRECT_IDS = frozenset({ - "gsm8k-train-sample-v1-0014", - "gsm8k-train-sample-v1-0018", - "gsm8k-train-sample-v1-0042", -}) - -# 5 Brief-8 target question sentences (ADR-0164.3 §Worked example follow-on) -_BRIEF8_IDS = frozenset({ - "gsm8k-train-sample-v1-0007", # How many more boxes... - "gsm8k-train-sample-v1-0017", # How much will it cost him? - "gsm8k-train-sample-v1-0027", # How many followers does Malcolm have... - "gsm8k-train-sample-v1-0036", # How much time did she spend studying... - "gsm8k-train-sample-v1-0043", # How much money will she be left with... -}) - - -@pytest.fixture(scope="module") -def all_cases() -> list[dict[str, Any]]: - return _load_cases() - - -@pytest.fixture(scope="module") -def correct_cases(all_cases: list[dict[str, Any]]) -> list[dict[str, Any]]: - return [c for c in all_cases if c["case_id"] in _CORRECT_IDS] - - -@pytest.fixture(scope="module") -def brief8_cases(all_cases: list[dict[str, Any]]) -> list[dict[str, Any]]: - return [c for c in all_cases if c["case_id"] in _BRIEF8_IDS] - - -# --------------------------------------------------------------------------- -# 1. Flag-OFF byte-identity -# --------------------------------------------------------------------------- - - -class TestFlagOffByteIdentity: - """parse_and_solve(config=None) and parse_and_solve(config=CONFIG_OFF) - must produce the same answer on the 3 currently-correct cases.""" - - def test_correct_cases_no_config_vs_off(self, correct_cases: list[dict[str, Any]]) -> None: - assert correct_cases, "fixture must contain at least one correct case" - for case in correct_cases: - text = case["question"] - r_none = parse_and_solve(text, config=None) - r_off = parse_and_solve(text, config=_CONFIG_OFF) - assert r_none.answer == r_off.answer, ( - f"{case['case_id']}: answer differs between config=None and flag-OFF" - ) - assert r_none.is_admitted == r_off.is_admitted, ( - f"{case['case_id']}: is_admitted differs" - ) - # Flag OFF must produce empty reader_trace (reader never consulted). - assert r_off.reader_trace == (), ( - f"{case['case_id']}: reader_trace must be empty with flag OFF" - ) - - def test_correct_cases_off_vs_on_same_answer(self, correct_cases: list[dict[str, Any]]) -> None: - """Fast-path cases are unaffected by the reader — answer must be identical.""" - for case in correct_cases: - text = case["question"] - r_off = parse_and_solve(text, config=_CONFIG_OFF) - r_on = parse_and_solve(text, config=_CONFIG_ON) - assert r_off.answer == r_on.answer, ( - f"{case['case_id']}: reader flag changed a fast-path answer" - ) - assert r_off.is_admitted == r_on.is_admitted, ( - f"{case['case_id']}: reader flag changed admission status" - ) - - -# --------------------------------------------------------------------------- -# 2. Determinism (flag ON) -# --------------------------------------------------------------------------- - - -class TestDeterminism: - def test_flag_on_reader_trace_deterministic(self, all_cases: list[dict[str, Any]]) -> None: - """Two calls with the same input and flag ON must produce identical reader_trace.""" - sample = all_cases[:10] - for case in sample: - text = case["question"] - r1 = parse_and_solve(text, config=_CONFIG_ON) - r2 = parse_and_solve(text, config=_CONFIG_ON) - assert r1.reader_trace == r2.reader_trace, ( - f"{case['case_id']}: reader_trace not deterministic" - ) - assert r1.answer == r2.answer, ( - f"{case['case_id']}: answer not deterministic" - ) - - def test_flag_off_reader_trace_empty(self, all_cases: list[dict[str, Any]]) -> None: - """Flag OFF must never populate reader_trace.""" - for case in all_cases: - r = parse_and_solve(case["question"], config=_CONFIG_OFF) - assert r.reader_trace == (), ( - f"{case['case_id']}: reader_trace must be empty with flag OFF" - ) - - -# --------------------------------------------------------------------------- -# 3. wrong=0 invariant -# --------------------------------------------------------------------------- - - -class TestWrongIsZero: - def test_flag_on_wrong_is_zero(self, all_cases: list[dict[str, Any]]) -> None: - """Flag ON must never produce wrong > 0 on the 50-case train sample. - - Wrong outcome requires: admitted=True AND answer != expected. Since - the reader is additive (refusal falls through to regex), and the - underlying regex path is already wrong=0, this invariant must hold. - """ - wrong_cases: list[str] = [] - for raw in all_cases: - result = parse_and_solve(raw["question"], config=_CONFIG_ON) - if result.is_admitted and result.answer != raw["answer_numeric"]: - wrong_cases.append( - f"{raw['case_id']}: got {result.answer}, expected {raw['answer_numeric']}" - ) - assert not wrong_cases, f"wrong > 0 with flag ON:\n" + "\n".join(wrong_cases) - - -# --------------------------------------------------------------------------- -# 4. Trace event shape -# --------------------------------------------------------------------------- - -_REQUIRED_TRACE_KEYS = frozenset({"layer", "phase", "outcome"}) -_VALID_OUTCOMES = frozenset({"admit", "fallthrough_to_regex"}) - - -class TestTraceShape: - def test_trace_events_are_valid_json(self, all_cases: list[dict[str, Any]]) -> None: - for case in all_cases: - r = parse_and_solve(case["question"], config=_CONFIG_ON) - for event_str in r.reader_trace: - try: - event = json.loads(event_str) - except json.JSONDecodeError: - pytest.fail(f"{case['case_id']}: reader_trace event is not valid JSON: {event_str!r}") - missing = _REQUIRED_TRACE_KEYS - set(event.keys()) - assert not missing, ( - f"{case['case_id']}: trace event missing keys {missing}: {event}" - ) - assert event["layer"] == "comprehension_reader", ( - f"{case['case_id']}: unexpected layer: {event}" - ) - assert event["phase"] == 1, ( - f"{case['case_id']}: unexpected phase: {event}" - ) - assert event["outcome"] in _VALID_OUTCOMES, ( - f"{case['case_id']}: unexpected outcome: {event}" - ) - - def test_admit_event_has_entity_and_unit(self, all_cases: list[dict[str, Any]]) -> None: - """Every 'admit' trace event must carry entity and unit keys.""" - for case in all_cases: - r = parse_and_solve(case["question"], config=_CONFIG_ON) - for event_str in r.reader_trace: - event = json.loads(event_str) - if event["outcome"] == "admit": - assert "entity" in event, ( - f"{case['case_id']}: admit event missing 'entity': {event}" - ) - assert "unit" in event, ( - f"{case['case_id']}: admit event missing 'unit': {event}" - ) - - def test_fallthrough_event_has_refusal_reason(self, all_cases: list[dict[str, Any]]) -> None: - """Every 'fallthrough_to_regex' trace event must carry refusal_reason.""" - for case in all_cases: - r = parse_and_solve(case["question"], config=_CONFIG_ON) - for event_str in r.reader_trace: - event = json.loads(event_str) - if event["outcome"] == "fallthrough_to_regex": - assert "refusal_reason" in event, ( - f"{case['case_id']}: fallthrough event missing 'refusal_reason': {event}" - ) - - -# --------------------------------------------------------------------------- -# 5. Brief-8 targets: reader is consulted -# --------------------------------------------------------------------------- - - -class TestBrief8Targets: - def test_reader_consulted_for_brief8_cases(self, brief8_cases: list[dict[str, Any]]) -> None: - """When flag ON, the reader is consulted for each of the 5 Brief-8 target - question sentences — reader_trace is non-empty.""" - assert len(brief8_cases) == 5, ( - f"Expected 5 Brief-8 cases, found {len(brief8_cases)}" - ) - for case in brief8_cases: - r = parse_and_solve(case["question"], config=_CONFIG_ON) - assert r.reader_trace, ( - f"{case['case_id']}: reader_trace is empty — reader was not consulted" - ) - - def test_brief8_cases_wrong_stays_zero(self, brief8_cases: list[dict[str, Any]]) -> None: - """Brief-8 cases must not produce wrong outcomes with flag ON.""" - for case in brief8_cases: - r = parse_and_solve(case["question"], config=_CONFIG_ON) - if r.is_admitted: - assert r.answer == case["answer_numeric"], ( - f"{case['case_id']}: wrong answer with flag ON: " - f"got {r.answer}, expected {case['answer_numeric']}" - ) - - def test_case_0027_malcolm_admits(self, brief8_cases: list[dict[str, Any]]) -> None: - """Case 0027 (Malcolm/followers) has no pronoun ambiguity — reader admits it.""" - case = next(c for c in brief8_cases if "0027" in c["case_id"]) - r = parse_and_solve(case["question"], config=_CONFIG_ON) - assert r.reader_trace, "reader must produce a trace for case 0027" - events = [json.loads(e) for e in r.reader_trace] - admit_events = [e for e in events if e["outcome"] == "admit"] - assert admit_events, ( - f"case 0027 must produce at least one admit event; got: {events}" - ) - admit = admit_events[0] - assert admit["entity"] == "malcolm" - assert admit["unit"] == "followers" - - -# --------------------------------------------------------------------------- -# 6. Fallthrough preserved for unknown words -# --------------------------------------------------------------------------- - - -class TestFallthroughPreserved: - def test_unknown_unit_falls_through_to_regex(self) -> None: - """A question with an unknown unit noun falls through to regex — result is correct.""" - problem = "Martha has 5 apples. How many apples does Martha have?" - r_off = parse_and_solve(problem, config=_CONFIG_OFF) - r_on = parse_and_solve(problem, config=_CONFIG_ON) - - # answer must be identical between flag OFF and flag ON - assert r_off.answer == r_on.answer - assert r_off.is_admitted == r_on.is_admitted - - # flag ON must record a fallthrough trace event - assert r_on.reader_trace, "fallthrough case must produce a trace event" - event = json.loads(r_on.reader_trace[0]) - assert event["outcome"] == "fallthrough_to_regex" - assert event["refusal_reason"] == "unknown_word" - - def test_flag_off_no_trace_for_fallthrough_case(self) -> None: - """Flag OFF must never produce any trace events, even for fallthrough-prone inputs.""" - problem = "Martha has 5 apples. How many apples does Martha have?" - r = parse_and_solve(problem, config=_CONFIG_OFF) - assert r.reader_trace == () diff --git a/tests/test_teaching_coverage_cli.py b/tests/test_teaching_coverage_cli.py index f2a716a5..1e33acfd 100644 --- a/tests/test_teaching_coverage_cli.py +++ b/tests/test_teaching_coverage_cli.py @@ -62,7 +62,7 @@ def test_build_coverage_report_basic(tmp_path: Path): ], ) r = build_coverage_report( - report_path, lane="t", split="s", version="v1", use_reader=True, + report_path, lane="t", split="s", version="v1", ) assert r.counts == CoverageCounts(correct=2, refused=3, wrong=0) assert r.counts.total() == 5 @@ -85,7 +85,7 @@ def test_case_0050_verdict_captured(tmp_path: Path): ], ) r = build_coverage_report( - report_path, lane="gsm8k_math", split="train_sample", version="v1", use_reader=True, + report_path, lane="gsm8k_math", split="train_sample", version="v1", ) assert r.case_0050_verdict == "refused" @@ -97,7 +97,7 @@ def test_delta_computation(tmp_path: Path): _write_report(baseline_path, correct=3, refused=47, wrong=0, per_case=[]) r = build_coverage_report( report_path, - lane="t", split="s", version="v1", use_reader=True, + lane="t", split="s", version="v1", baseline_path=baseline_path, ) assert r.delta == {"correct": 1, "refused": -1, "wrong": 0} @@ -108,7 +108,7 @@ def test_no_baseline_means_empty_delta(tmp_path: Path): _write_report(report_path, correct=3, refused=47, wrong=0, per_case=[]) r = build_coverage_report( report_path, - lane="t", split="s", version="v1", use_reader=False, + lane="t", split="s", version="v1", baseline_path=None, ) assert r.delta == {} @@ -118,7 +118,7 @@ def test_missing_report_raises(tmp_path: Path): with pytest.raises(FileNotFoundError): build_coverage_report( tmp_path / "nope.json", - lane="t", split="s", version="v1", use_reader=False, + lane="t", split="s", version="v1", ) @@ -128,11 +128,10 @@ def test_as_dict_round_trip(tmp_path: Path): {"case_id": "x-0001", "verdict": "correct"}, {"case_id": "x-0002", "verdict": "refused", "reason": "candidate_graph: recognizer matched but produced no injection for statement: 'X.' (category=discrete_count_statement)"}, ]) - r = build_coverage_report(report_path, lane="t", split="s", version="v1", use_reader=True) + r = build_coverage_report(report_path, lane="t", split="s", version="v1") d = r.as_dict() assert d["counts"]["total"] == 2 assert "recognizer_empty_injection(discrete_count_statement)" in d["refusal_taxonomy"] - assert d["use_reader"] is True def test_taxonomy_sorted_by_count_desc(tmp_path: Path): @@ -141,7 +140,7 @@ def test_taxonomy_sorted_by_count_desc(tmp_path: Path): {"case_id": f"x-{i:04d}", "verdict": "refused", "reason": "candidate_graph: no admissible candidate for question: '?'" if i < 1 else "candidate_graph: recognizer matched but produced no injection for statement: 'X.' (category=multiplicative_aggregation)"} for i in range(4) ]) - r = build_coverage_report(report_path, lane="t", split="s", version="v1", use_reader=False) + r = build_coverage_report(report_path, lane="t", split="s", version="v1") keys = list(r.refusal_taxonomy.keys()) assert keys[0] == "recognizer_empty_injection(multiplicative_aggregation)" # 3 > 1 assert keys[1] == "no_admissible_question" @@ -150,7 +149,7 @@ def test_taxonomy_sorted_by_count_desc(tmp_path: Path): def test_wrong_zero_invariant_visible_via_as_dict(tmp_path: Path): report_path = tmp_path / "report.json" _write_report(report_path, correct=0, refused=0, wrong=2, per_case=[]) - r = build_coverage_report(report_path, lane="t", split="s", version="v1", use_reader=False) + r = build_coverage_report(report_path, lane="t", split="s", version="v1") assert r.as_dict()["counts"]["wrong"] == 2 diff --git a/tests/test_wave_a_multiplicative_aggregation_injector.py b/tests/test_wave_a_multiplicative_aggregation_injector.py index b749efff..b1eb8232 100644 --- a/tests/test_wave_a_multiplicative_aggregation_injector.py +++ b/tests/test_wave_a_multiplicative_aggregation_injector.py @@ -180,8 +180,7 @@ def test_wrong_zero_preserved(): while here.parent != here and not (here / "pyproject.toml").exists(): here = here.parent subprocess.run( - [sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner", - "--use-reader"], + [sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner"], cwd=here, capture_output=True, ) @@ -200,8 +199,7 @@ def test_case_0050_remains_refused(): while here.parent != here and not (here / "pyproject.toml").exists(): here = here.parent subprocess.run( - [sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner", - "--use-reader"], + [sys.executable, "-m", "evals.gsm8k_math.train_sample.v1.runner"], cwd=here, capture_output=True, )