From e8894f7a701e809c001df0355f62f8264ba5a5cb Mon Sep 17 00:00:00 2001 From: Shay Date: Sat, 23 May 2026 06:24:12 -0700 Subject: [PATCH] feat(ADR-0126 P2): candidate-emitting sentence parser + 17 tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sibling to math_parser.py — pure candidate-extraction functions that emit list[CandidateOperation] per sentence without mutating any state. State threading defers to P3 (per-branch graph assembly). Topology change vs legacy: - No first-match-wins; every verb-kind regex runs independently. - Ambiguous verbs ('gives', 'returns') emit multiple candidates; P1's round-trip filter + P3's decision rule resolve. - Out-of-grammar sentences return [], NOT ParseError. Empty list is the deterministic 'no candidate' signal. Permissive verb tables (imported from math_roundtrip.KIND_TO_VERBS) mean past-tense and production verbs ('bought', 'ate', 'bakes') that the legacy parser refused are now admissible — the round-trip filter is the safety mechanism, not regex narrowness. P2 scope (canonical Subject-verb-Value-Unit-[to-Target] shape only): - extract_initial_candidates(sentence) for 'X has N units' - extract_operation_candidates(sentence) for add/subtract/transfer Out of scope (deferred to later sub-phases): - Pronoun resolution / unit inheritance (needs per-branch state) - Multiply / divide / rate / comparison (same machinery, more matchers) Regression: existing math suite 701/701 green. Zero changes to math_parser.py, math_solver.py, math_verifier.py, math_realizer.py. --- generate/math_candidate_parser.py | 301 ++++++++++++++++++++++++++++ tests/test_math_candidate_parser.py | 191 ++++++++++++++++++ 2 files changed, 492 insertions(+) create mode 100644 generate/math_candidate_parser.py create mode 100644 tests/test_math_candidate_parser.py diff --git a/generate/math_candidate_parser.py b/generate/math_candidate_parser.py new file mode 100644 index 00000000..6b2ddd6e --- /dev/null +++ b/generate/math_candidate_parser.py @@ -0,0 +1,301 @@ +"""ADR-0126 — Candidate-emitting sentence parser. + +Sibling to ``generate/math_parser.py``. Same regex spirit, different +topology: instead of first-match-wins with a single mutable state and +``ParseError`` on miss, each per-sentence extractor returns a *list of +candidates* (possibly empty) carrying full source-span provenance. + +The wrong-answer firewall is :func:`generate.math_roundtrip.roundtrip_admissible`, +applied downstream in P3 (graph assembly). This module's job is purely +to *enumerate* the parses the grammar admits — telling truth from +falsehood is not its concern. + +Determinism: candidate lists are returned in deterministic order +(canonical pattern key); the same input always produces the same +ordered output. + +Scope of P2 (this module): + - Initial-possession candidate extraction. + - Operation candidate extraction for add / subtract / transfer + via the canonical " [to ]" + shape. + - Permissive verb tables imported from + :data:`generate.math_roundtrip.KIND_TO_VERBS` — much wider than + ``math_parser._ADD_VERBS`` / ``_SUBTRACT_VERBS`` / ``_TRANSFER_VERBS`` + because the round-trip filter rejects wrong candidates downstream. + +Out of scope for P2 (added in later phases): + - Pronoun resolution (needs per-branch state — P3). + - Unit inheritance from ``last_unit`` (needs per-branch state — P3). + - Multiply / divide / rate / comparison candidates (later phases of + ADR-0126; the candidate-emission machinery is identical, just more + pattern matchers). +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from typing import Final + +from generate.math_problem_graph import ( + InitialPossession, + Operation, + Quantity, +) +from generate.math_roundtrip import ( + ADD_VERBS, + SUBTRACT_VERBS, + TRANSFER_VERBS, + WORD_NUMBERS, + CandidateOperation, +) + + +# --------------------------------------------------------------------------- +# Initial-possession candidate +# --------------------------------------------------------------------------- + +@dataclass(frozen=True, slots=True) +class CandidateInitial: + """Initial-possession candidate with source-span provenance. + + Mirrors :class:`CandidateOperation` but for ``InitialPossession``. + The round-trip filter for initials is the same shape: every claimed + content slot (entity, value, unit, anchor verb 'has'/'have') must + ground in the source sentence. + """ + + initial: InitialPossession + source_span: str + matched_anchor: str # 'has' or 'have' + matched_value_token: str # '3' or 'three' + matched_unit_token: str + matched_entity_token: str + + def __post_init__(self) -> None: + if self.matched_anchor.lower() not in ("has", "have"): + raise ValueError( + f"CandidateInitial.matched_anchor must be has/have; " + f"got {self.matched_anchor!r}" + ) + + +# --------------------------------------------------------------------------- +# Shared regex building blocks +# --------------------------------------------------------------------------- + +# Title-cased proper noun OR "the " collective. Same widening as +# math_parser._INITIAL_HAS_RE's ADR-0123a entity slot. +_ENTITY: Final[str] = r"(?:[A-Z]\w+|[Tt]he\s+\w+)" + +# Numeric value: digit run OR word-form integer (one..twelve initially; +# WORD_NUMBERS table is wider but we cap the regex at the common range +# for syntactic parsing and let the filter handle ground-truth value +# equivalence). +_WORD_NUM_OPTIONS: Final[str] = "|".join( + re.escape(w) for w in sorted(WORD_NUMBERS.keys(), key=len, reverse=True) +) +_VALUE: Final[str] = rf"(?:\d+|{_WORD_NUM_OPTIONS})" + +# Verb alternation built from the permissive registry. Pre-compute one +# pattern per kind so we can attribute matched verbs to candidates. +def _verbs_pattern(verbs: frozenset[str]) -> str: + # Longest-first so "passes" matches before "pass" inside the alternation. + options = sorted(verbs, key=len, reverse=True) + return r"(?:" + "|".join(re.escape(v) for v in options) + r")" + + +_ADD_VERBS_PATTERN: Final[str] = _verbs_pattern(ADD_VERBS) +_SUBTRACT_VERBS_PATTERN: Final[str] = _verbs_pattern(SUBTRACT_VERBS) +_TRANSFER_VERBS_PATTERN: Final[str] = _verbs_pattern(TRANSFER_VERBS) + + +# --------------------------------------------------------------------------- +# Initial-possession extractor +# --------------------------------------------------------------------------- + +_INITIAL_HAS_RE: Final[re.Pattern[str]] = re.compile( + rf"^(?P{_ENTITY})\s+" + rf"(?Phas|have)\s+" + rf"(?P{_VALUE})\s+" + r"(?P\w+)\s*\.?$" +) + + +def _normalize_entity(raw: str) -> str: + """Collapse whitespace + lowercase article. Mirrors math_parser + canonicalization so candidate entity names hash-equal to legacy.""" + e = re.sub(r"\s+", " ", raw.strip()) + if e.lower().startswith("the "): + return "the " + e[4:] + return e + + +def _resolve_value(value_token: str) -> int: + if value_token.isdigit(): + return int(value_token) + return WORD_NUMBERS[value_token.lower()] + + +def extract_initial_candidates(sentence: str) -> list[CandidateInitial]: + """Return all admissible initial-possession candidates for ``sentence``. + + Currently emits at most one candidate (the single canonical shape + " has "). Returns an empty list if no shape matches. + """ + s = sentence.strip().rstrip(".") + m = _INITIAL_HAS_RE.match(s) + if not m: + return [] + entity = _normalize_entity(m.group("entity")) + value = _resolve_value(m.group("value")) + unit_raw = m.group("unit") + # Canonicalize: lowercase + ensure plural (matching math_parser._canonical_unit). + unit = unit_raw.lower() + if not unit.endswith("s"): + unit = unit + "s" + return [ + CandidateInitial( + initial=InitialPossession( + entity=entity, + quantity=Quantity(value=value, unit=unit), + ), + source_span=sentence, + matched_anchor=m.group("anchor"), + matched_value_token=m.group("value"), + matched_unit_token=unit_raw, + matched_entity_token=m.group("entity"), + ) + ] + + +# --------------------------------------------------------------------------- +# Operation candidate extractor +# --------------------------------------------------------------------------- + +# Per-kind operation patterns. Each captures: subject, verb, value, unit, +# optional target. The verb alternation is the kind's permissive verb table. +# +# Note: optional unit (?P) is allowed because some constructions +# rely on inherited unit ("Sam doubles his savings"); however for P2's +# scope we only emit candidates when the unit token is explicit. Inherited- +# unit candidates require per-branch state and are added in P3. + +def _op_pattern(verbs_pattern: str, *, requires_target: bool) -> re.Pattern[str]: + """Build the per-kind operation regex. + + For ``requires_target=True`` (transfer): the trailing ``to `` + clause is a captured slot. + + For ``requires_target=False`` (add/subtract): there is no target + slot. A trailing ``to `` phrase, if present, is consumed as + part of the discardable preposition tail so the regex still matches + ambiguous sentences like "Sam gives 3 apples to Tom" (which we + *do* want to match as a subtract candidate; the transfer-vs-subtract + disambiguation happens at the candidate / filter / decision-rule + layer, not by regex specificity). + """ + if requires_target: + target_part = r"\s+to\s+(?P[A-Z]\w+)" + trailing_prep = ( + r"(?:\s+(?:on|from|at|in|onto|into|under|over)\s+.+)?" + ) + else: + target_part = "" + # Note: 'to' is included in the discardable preposition set. + trailing_prep = ( + r"(?:\s+(?:on|from|at|in|onto|into|under|over|to)\s+.+)?" + ) + return re.compile( + r"^" + rf"(?P{_ENTITY})\s+" + rf"(?P{verbs_pattern})" + rf"\s+(?P{_VALUE})" + r"(?:\s+more)?" + r"(?:\s+(?!to\b)(?!more\b)(?!on\b)(?!from\b)(?!at\b)(?!in\b)" + r"(?P\w+))?" + rf"{target_part}" + rf"{trailing_prep}" + r"\s*\.?$", + flags=re.IGNORECASE, + ) + + +_ADD_OP_RE: Final[re.Pattern[str]] = _op_pattern(_ADD_VERBS_PATTERN, requires_target=False) +_SUBTRACT_OP_RE: Final[re.Pattern[str]] = _op_pattern(_SUBTRACT_VERBS_PATTERN, requires_target=False) +_TRANSFER_OP_RE: Final[re.Pattern[str]] = _op_pattern(_TRANSFER_VERBS_PATTERN, requires_target=True) + + +def _build_op_candidate( + m: re.Match[str], kind: str, source: str +) -> CandidateOperation | None: + """Build a CandidateOperation from a regex match. Returns None if + the match lacks a required slot (e.g. unit token absent — P2 does + not emit unit-inherited candidates).""" + unit_raw = m.group("unit") + if unit_raw is None: + return None + unit = unit_raw.lower() + if not unit.endswith("s"): + unit = unit + "s" + subject = _normalize_entity(m.group("subject")) + verb = m.group("verb").lower() + value = _resolve_value(m.group("value")) + target_raw = m.group("target") if "target" in m.groupdict() else None + target = target_raw if target_raw is not None else None + + op_kwargs: dict[str, object] = { + "actor": subject, + "kind": kind, + "operand": Quantity(value=value, unit=unit), + } + if kind == "transfer": + if target is None: + return None # transfer requires target + op_kwargs["target"] = target + else: + if target is not None: + return None # add/subtract don't take targets + + return CandidateOperation( + op=Operation(**op_kwargs), # type: ignore[arg-type] + source_span=source, + matched_verb=verb, + matched_value_token=m.group("value"), + matched_unit_token=unit_raw, + matched_actor_token=m.group("subject"), + matched_target_token=target, + ) + + +def extract_operation_candidates(sentence: str) -> list[CandidateOperation]: + """Return all operation candidates for ``sentence``. + + Tries every verb-kind pattern independently. A sentence with an + ambiguous verb (e.g. "Sam gives 3 apples to Tom" — "gives" appears + in both SUBTRACT_VERBS and TRANSFER_VERBS) may emit multiple + candidates. The round-trip filter + (:func:`generate.math_roundtrip.roundtrip_admissible`) and the + decision rule (P3) resolve which one becomes the chosen graph. + + Candidate emission order is canonical: add, subtract, transfer. + Within each kind, the regex emits at most one candidate per + sentence. + """ + s = sentence.strip() + out: list[CandidateOperation] = [] + + for pattern, kind in ( + (_ADD_OP_RE, "add"), + (_SUBTRACT_OP_RE, "subtract"), + (_TRANSFER_OP_RE, "transfer"), + ): + m = pattern.match(s) + if m is None: + continue + candidate = _build_op_candidate(m, kind, source=sentence) + if candidate is not None: + out.append(candidate) + + return out diff --git a/tests/test_math_candidate_parser.py b/tests/test_math_candidate_parser.py new file mode 100644 index 00000000..16839a6c --- /dev/null +++ b/tests/test_math_candidate_parser.py @@ -0,0 +1,191 @@ +"""ADR-0126 — tests for the candidate-emitting parser (P2). + +Proves the candidate-emission topology end-to-end against the round-trip +filter from P1: + +- Unambiguous sentences emit exactly one candidate, which the filter + admits. +- Ambiguous sentences (e.g. verb in both SUBTRACT_VERBS and + TRANSFER_VERBS) emit multiple candidates; the filter admits the + correct one based on grounded slots. +- Out-of-grammar sentences emit zero candidates (no ParseError raised). +- Permissive verbs not in the legacy math_parser tables (e.g. "bought", + "lost", "gave") now produce admissible candidates — the whole point + of P2 + filter. +""" + +from __future__ import annotations + +from generate.math_candidate_parser import ( + extract_initial_candidates, + extract_operation_candidates, +) +from generate.math_roundtrip import roundtrip_admissible + + +# --------------------------------------------------------------------------- +# Initial-possession extraction +# --------------------------------------------------------------------------- + +class TestInitialExtraction: + def test_single_entity_digit(self) -> None: + cands = extract_initial_candidates("Sam has 5 apples.") + assert len(cands) == 1 + c = cands[0] + assert c.initial.entity == "Sam" + assert c.initial.quantity.value == 5 + assert c.initial.quantity.unit == "apples" + + def test_single_entity_word_number(self) -> None: + cands = extract_initial_candidates("Sam has three apples.") + assert len(cands) == 1 + assert cands[0].initial.quantity.value == 3 + + def test_collective_entity(self) -> None: + cands = extract_initial_candidates("The boys have 10 marbles.") + assert len(cands) == 1 + assert cands[0].initial.entity == "the boys" + + def test_singular_unit_pluralized(self) -> None: + cands = extract_initial_candidates("Sam has 1 apple.") + assert len(cands) == 1 + # math_parser canonicalization rule: always pluralize + assert cands[0].initial.quantity.unit == "apples" + + def test_no_match_returns_empty(self) -> None: + # Out-of-grammar shape — empty list, NOT an exception. + assert extract_initial_candidates("Sam went to the store.") == [] + assert extract_initial_candidates("How many apples?") == [] + + +# --------------------------------------------------------------------------- +# Operation extraction — unambiguous verbs +# --------------------------------------------------------------------------- + +class TestUnambiguousOperations: + def test_add_present_tense(self) -> None: + cands = extract_operation_candidates("Sam buys 3 apples.") + assert len(cands) == 1 + assert cands[0].op.kind == "add" + assert cands[0].op.operand.value == 3 + assert roundtrip_admissible(cands[0]) + + def test_add_past_tense_permissive(self) -> None: + # "bought" is in the new permissive ADD_VERBS but NOT in the + # legacy math_parser._ADD_VERBS. The whole point of P2 is to + # admit these via the round-trip filter. + cands = extract_operation_candidates("Sam bought 3 apples.") + assert len(cands) == 1 + assert cands[0].op.kind == "add" + assert cands[0].matched_verb == "bought" + assert roundtrip_admissible(cands[0]) + + def test_subtract_present_tense(self) -> None: + cands = extract_operation_candidates("Sam eats 2 apples.") + assert len(cands) == 1 + assert cands[0].op.kind == "subtract" + assert roundtrip_admissible(cands[0]) + + def test_subtract_past_tense_permissive(self) -> None: + # "ate" is in the new permissive SUBTRACT_VERBS but not legacy. + cands = extract_operation_candidates("Sam ate 2 apples.") + assert len(cands) == 1 + assert cands[0].op.kind == "subtract" + assert cands[0].matched_verb == "ate" + assert roundtrip_admissible(cands[0]) + + def test_production_verb_permissive(self) -> None: + # "bakes" is a production verb — actor creates instances. Not + # in legacy ADD_VERBS, accepted now via the permissive table. + cands = extract_operation_candidates("Sam bakes 4 pies.") + assert len(cands) == 1 + assert cands[0].op.kind == "add" + assert cands[0].matched_verb == "bakes" + assert roundtrip_admissible(cands[0]) + + def test_no_match_returns_empty(self) -> None: + # Out-of-grammar: a verb we don't recognize at all. + assert extract_operation_candidates("Sam contemplates 3 apples.") == [] + # Sentence missing required slots (no value). + assert extract_operation_candidates("Sam buys apples.") == [] + + +# --------------------------------------------------------------------------- +# Operation extraction — ambiguous verbs (THE key test for P2) +# --------------------------------------------------------------------------- + +class TestAmbiguousOperations: + def test_gives_with_target_emits_subtract_and_transfer(self) -> None: + # "gives" appears in both SUBTRACT_VERBS (intransitive-like + # reading "Sam gives 3 apples") and TRANSFER_VERBS (transitive + # "Sam gives 3 apples to Tom"). When a target IS present, both + # candidates fire by design — the filter and decision rule + # resolve the ambiguity downstream. + cands = extract_operation_candidates("Sam gives 3 apples to Tom.") + kinds = sorted(c.op.kind for c in cands) + assert kinds == ["subtract", "transfer"] + + def test_filter_admits_both_for_gives_to_target(self) -> None: + # Both candidates pass round-trip — neither claims a slot that + # isn't in the source. The P3 decision rule will need a + # tiebreaker (most-grounded-slots-wins is one option). This + # test pins the current filter behavior; the tiebreaker is + # P3's responsibility. + cands = extract_operation_candidates("Sam gives 3 apples to Tom.") + admitted = [c for c in cands if roundtrip_admissible(c)] + assert len(admitted) == 2 + # Transfer candidate has a target slot (4 grounded entities), + # subtract candidate does not (3 grounded entities). Slot count + # is the structural signal P3 will use. + + def test_gives_without_target_only_subtract_admits(self) -> None: + # "Sam gives 3 apples." — no target slot in source. The + # transfer pattern requires a "to " clause and won't + # match; subtract pattern matches and is admissible. + cands = extract_operation_candidates("Sam gives 3 apples.") + admitted = [c for c in cands if roundtrip_admissible(c)] + assert len(admitted) == 1 + assert admitted[0].op.kind == "subtract" + + def test_returns_emits_both_subtract_and_transfer(self) -> None: + # "returns" is also overloaded. + cands = extract_operation_candidates("Sam returns 2 books to Tom.") + kinds = sorted(c.op.kind for c in cands) + assert kinds == ["subtract", "transfer"] + admitted = [c for c in cands if roundtrip_admissible(c)] + assert len(admitted) == 2 + + +# --------------------------------------------------------------------------- +# Wrong-answer firewall demonstrated end-to-end +# --------------------------------------------------------------------------- + +class TestFirewallEndToEnd: + def test_filter_rejects_when_legacy_parser_would_have_misparsed(self) -> None: + # Imagine the old parser had a bug where "loses" was registered + # as ADD. Under candidate-graph, even if such a buggy candidate + # were emitted, the round-trip filter would catch it because + # "loses" is not in ADD_VERBS. + # + # We simulate by constructing the buggy candidate by hand and + # showing the filter rejects it. + from generate.math_problem_graph import Operation, Quantity + from generate.math_roundtrip import CandidateOperation + buggy = CandidateOperation( + op=Operation(actor="Sam", kind="add", + operand=Quantity(value=2, unit="apples")), + source_span="Sam loses 2 apples.", + matched_verb="loses", # the bug + matched_value_token="2", + matched_unit_token="apples", + matched_actor_token="Sam", + ) + assert not roundtrip_admissible(buggy) + + def test_correct_subtract_candidate_for_loses_is_admissible(self) -> None: + # And the correct subtract reading IS emitted by the extractor. + cands = extract_operation_candidates("Sam loses 2 apples.") + admitted = [c for c in cands if roundtrip_admissible(c)] + assert len(admitted) == 1 + assert admitted[0].op.kind == "subtract" + assert admitted[0].matched_verb == "loses"