Merge pull request #430 from AssetOverflow/feat/adr-0174-phase5a-retire-inert-reader
ADR-0174 Phase 5a: retire inert GSM8K scoring-path reader (net -1,038 LOC)
This commit is contained in:
Shay
GitHub
commit
52227920f3
17 changed files with 276 additions and 1091 deletions
11
core/cli.py
11
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",
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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
|
||||
`<<a*b=c>>` 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.
|
||||
|
||||
---
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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=(",", ":"),
|
||||
)
|
||||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -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}"
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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 == ()
|
||||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in a new issue