From eb452da9bed702e0dc0278973ced20cfca36356d Mon Sep 17 00:00:00 2001 From: Shay Date: Wed, 27 May 2026 11:23:08 -0700 Subject: [PATCH] =?UTF-8?q?feat(ADR-0170/W1):=20widen=20inject=5Ffrom=5Fma?= =?UTF-8?q?tch=20return=20type=20=E2=80=94=20no=20behavior=20change=20(#37?= =?UTF-8?q?4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First implementation PR of the ADR-0170 wave. Type-level widening only: the recognizer-injector dispatch now returns ``tuple[InjectorEmission, ...]`` where ``InjectorEmission = CandidateInitial | CandidateOperation``. The existing ``inject_discrete_count_statement`` continues to emit only ``CandidateInitial`` — the widening unlocks but does not exercise operation emission. Subsequent W2-W5 PRs ship the per-injector emission shapes: - W2 — DCS-S1 acquisition verbs (CandidateOperation(add)) - W3 — A1 currency_amount (CandidateInitial reimplementation) - W4 — A3 multiplicative_aggregation (CandidateInitial(product)) - W5 — A4 temporal_aggregation (deferred until apply_rate primitive) ## Changes ### `generate/recognizer_anchor_inject.py` - New `InjectorEmission = Union[CandidateInitial, CandidateOperation]` - `inject_from_match` return type widened to `tuple[InjectorEmission, ...]` - `__all__` exports `InjectorEmission` - Documentation comment names ADR-0170 §"Implementation outline" ### `generate/math_candidate_graph.py` (admissibility dispatch) The per-statement admission loop now dispatches admissibility on the concrete candidate type: if isinstance(c, CandidateInitial): if _initial_admissible(c): admitted.append(c) elif isinstance(c, CandidateOperation): if roundtrip_admissible(c): admitted.append(c) No new admission semantics — each type is gated by the predicate it was already gated by elsewhere in the codebase. The dispatch unifies the injector path with the parser path. ### `tests/test_adr_0170_w1_injector_type_widening.py` (new) - Pin: `InjectorEmission` union members are exactly the two candidate types - Pin: `inject_from_match` return type is widened - Pin: `inject_discrete_count_statement` still emits CandidateInitial (W1 is type-level only) - Hazard pin: case 0050 remains refused - Hazard pin: unparseable-verb refusal path (#359) unchanged - Anti-regression: canonical DCS narrow-form extraction still works ## Test plan - tests/test_adr_0170_w1_injector_type_widening.py: 6 passed (new) - tests/test_adr_0163_d2_discrete_count_injection.py: 21 passed (existing D.2 v1 injector regression) - tests/test_brief_11b_audit_artifact.py + step2_lexicon + recognizer_skip_wrong_zero + brief_11_audit: 55 passed - tests/test_candidate_graph_recognizer_wiring.py: 7 passed - tests/test_candidate_domain_partition.py: 5 passed - tests/test_adr_0131_G2_comparatives + G4 + G5 + S1_rate_events: 130 passed - Total: 225 passed - evals/gsm8k_math/train_sample/v1: counts=correct=3 refused=47 wrong=0 (unchanged; verified no behavioral regression) ## Hard invariants - `wrong == 0` preserved (admissibility dispatch is type-aware but semantically identical to the parser path's gating) - ADR-0166: no new eval lanes - No teaching-store / pack mutation - Five-layer wrong=0 safety net (ADR-0163.D.2) intact - Reader path unchanged --- generate/math_candidate_graph.py | 21 ++- generate/recognizer_anchor_inject.py | 24 ++- ...test_adr_0170_w1_injector_type_widening.py | 159 ++++++++++++++++++ 3 files changed, 196 insertions(+), 8 deletions(-) create mode 100644 tests/test_adr_0170_w1_injector_type_widening.py diff --git a/generate/math_candidate_graph.py b/generate/math_candidate_graph.py index 2879acef..0e5b6a7d 100644 --- a/generate/math_candidate_graph.py +++ b/generate/math_candidate_graph.py @@ -727,9 +727,24 @@ def parse_and_solve( ) injected = inject_from_match(recognizer_match, s) if injected: - admitted: list[SentenceChoice] = [ - c for c in injected if _initial_admissible(c) - ] + # ADR-0170 — dispatch admissibility on the + # concrete candidate type. CandidateInitial uses + # the existing _initial_admissible gate; + # CandidateOperation uses the parser's + # roundtrip_admissible gate (same predicate + # operations from the regex path already pass + # through). No new admission semantics — each + # type is gated by the predicate it was always + # gated by; the dispatch just unifies the + # injector path with the parser path. + admitted: list[SentenceChoice] = [] + for c in injected: + if isinstance(c, CandidateInitial): + if _initial_admissible(c): + admitted.append(c) + elif isinstance(c, CandidateOperation): + if roundtrip_admissible(c): + admitted.append(c) if len(admitted) == len(injected) and admitted: per_sentence_choices.append( _collapse_per_sentence_ties(admitted) diff --git a/generate/recognizer_anchor_inject.py b/generate/recognizer_anchor_inject.py index 9c4a5f19..befdcce4 100644 --- a/generate/recognizer_anchor_inject.py +++ b/generate/recognizer_anchor_inject.py @@ -42,13 +42,23 @@ section) is preserved across this module: from __future__ import annotations -from typing import Mapping +from typing import Mapping, Union from evals.refusal_taxonomy.shape_categories import ShapeCategory -from generate.math_candidate_parser import CandidateInitial +from generate.math_candidate_parser import CandidateInitial, CandidateOperation from generate.math_problem_graph import InitialPossession, MathGraphError, Quantity from generate.recognizer_match import RecognizerMatch +# ADR-0170 — the widened injector emission type. Per-category injectors +# may emit a tuple of ``CandidateInitial`` (existing) or +# ``CandidateOperation`` (new, ADR-0170). The downstream +# ``per_sentence_choices`` aggregator dispatches admissibility on the +# concrete type (``_initial_admissible`` vs ``roundtrip_admissible``). +# No new admission paths are introduced by the widening itself; new +# emission shapes ship in subsequent per-injector PRs (ADR-0170 §"impl +# outline" W2/W3/W4/W5). +InjectorEmission = Union[CandidateInitial, CandidateOperation] + # --------------------------------------------------------------------------- # Public surface @@ -58,12 +68,15 @@ from generate.recognizer_match import RecognizerMatch def inject_from_match( match: RecognizerMatch, sentence: str, -) -> tuple[CandidateInitial, ...]: +) -> tuple[InjectorEmission, ...]: """Dispatch a recognizer match to its per-category injector. Returns an empty tuple when the category has no v1 injector or when - the v1 injector refused. Skip-only behavior (the round-2 default) - is the empty-tuple result. + the v1 injector refused. Per ADR-0170, the return type is now + ``tuple[InjectorEmission, ...]`` (``CandidateInitial | CandidateOperation``) + so per-category injectors can emit operations as well as initials. + The v1 ``discrete_count_statement`` injector continues to emit only + ``CandidateInitial`` — the widening is type-level only in this PR. """ injector = _INJECTORS.get(match.category) if injector is None: @@ -260,6 +273,7 @@ _INJECTORS: Mapping[ShapeCategory, "type"] = { __all__ = [ + "InjectorEmission", "inject_from_match", "inject_discrete_count_statement", ] diff --git a/tests/test_adr_0170_w1_injector_type_widening.py b/tests/test_adr_0170_w1_injector_type_widening.py new file mode 100644 index 00000000..09696c17 --- /dev/null +++ b/tests/test_adr_0170_w1_injector_type_widening.py @@ -0,0 +1,159 @@ +"""ADR-0170 W1 — type widening pinning tests. + +These tests pin the no-behavior-change widening of +``inject_from_match``'s return type. The contract becomes +``tuple[CandidateInitial | CandidateOperation, ...]`` so per-category +injectors can emit operations as well as initials. The existing +``inject_discrete_count_statement`` still emits only ``CandidateInitial``; +the widening is type-level only in this PR. + +References: +- docs/decisions/ADR-0170-injector-contract-widening.md §"Implementation + outline" W1 (this PR) +- docs/handoff/DCS-S1-FINDING.md — the investigation that surfaced the + contract gap +- PR #369 (A2) — the schema-refusal that first observed the gap +""" + +from __future__ import annotations + +from typing import get_type_hints, Union, get_args, get_origin + +import pytest + +from generate.math_candidate_parser import CandidateInitial, CandidateOperation +from generate.recognizer_anchor_inject import ( + InjectorEmission, + inject_from_match, + inject_discrete_count_statement, +) + + +# --------------------------------------------------------------------------- +# Type-level contract +# --------------------------------------------------------------------------- + + +def test_injector_emission_union_includes_both_candidate_types(): + """``InjectorEmission`` is the union of ``CandidateInitial`` and + ``CandidateOperation``. Future injector PRs can emit either.""" + args = get_args(InjectorEmission) + assert CandidateInitial in args + assert CandidateOperation in args + # No third type smuggled in — the union is exactly two members. + assert len(args) == 2 + + +def test_inject_from_match_return_type_is_widened(): + """The dispatcher returns a tuple of ``InjectorEmission`` (not just + ``CandidateInitial``). This pins the W1 widening; reverting to a + narrower return type without an explicit ADR amendment fails this + test.""" + hints = get_type_hints(inject_from_match) + return_type = hints["return"] + # tuple[InjectorEmission, ...] + assert get_origin(return_type) is tuple + inner = get_args(return_type) + # tuple[X, ...] reports (X, Ellipsis) + assert inner[-1] is Ellipsis + inner_type = inner[0] + # The element type is either InjectorEmission directly or its + # Union[CandidateInitial, CandidateOperation] expansion. + if get_origin(inner_type) is Union: + members = set(get_args(inner_type)) + assert {CandidateInitial, CandidateOperation}.issubset(members) + else: + # Annotated alias case — resolve once. + assert inner_type is InjectorEmission + + +# --------------------------------------------------------------------------- +# Behavioral pin — existing DCS injector unchanged +# --------------------------------------------------------------------------- + + +def test_discrete_count_injector_still_emits_only_candidate_initial(): + """W1 is type-level only. The existing + ``inject_discrete_count_statement`` returns ``CandidateInitial`` — + not ``CandidateOperation`` — at runtime. This is the byte-identical + behavior guarantee for the W1 PR. + + Mechanically: pre-W1 the function returned + ``tuple[CandidateInitial, ...]``. Post-W1 it still does. Subsequent + PRs (W2 DCS-S1 acquisition, W3 currency, W4 multiplicative) widen + the per-injector emission shapes; W1 ships only the dispatcher + contract.""" + import inspect + sig = inspect.signature(inject_discrete_count_statement) + # With ``from __future__ import annotations`` the return annotation + # is stored as a string. The W1 pin is that the existing DCS + # injector's *narrower* return type is unchanged — only the + # dispatcher (``inject_from_match``) widens. + assert sig.return_annotation == "tuple[CandidateInitial, ...]" + + +# --------------------------------------------------------------------------- +# Behavioral pin — case 0050 hazard +# --------------------------------------------------------------------------- + + +def test_case_0050_remains_refused_post_widening(): + """The widening must not weaken the wrong=0 invariant. Case 0050 + refuses pre-W1 and must continue to refuse post-W1.""" + from generate.math_candidate_graph import parse_and_solve + case_text = ( + "Mark does a gig every other day for 2 weeks. " + "For each gig, he plays 3 songs. " + "2 of the songs are 5 minutes long and the last song is twice that long. " + "How many minutes did he play?" + ) + result = parse_and_solve(case_text) + assert not result.is_admitted, ( + f"case 0050 admitted post-W1 — wrong=0 hazard re-introduced: " + f"answer={result.answer!r} graph={result.selected_graph!r}" + ) + + +def test_unparseable_verb_still_refuses_post_widening(): + """The recognizer-no-injection refusal path (the #359 wrong=0 fix) + is unchanged by the W1 widening. Unparseable verbs still produce + explicit refusals, not silent admissions.""" + from generate.math_candidate_graph import parse_and_solve + result = parse_and_solve( + "Sam has 5 apples. Sam contemplates 3 apples. " + "How many apples does Sam have?" + ) + assert not result.is_admitted + assert result.refusal_reason is not None + + +# --------------------------------------------------------------------------- +# Anti-regression: existing DCS path still admits +# --------------------------------------------------------------------------- + + +def test_existing_dcs_admission_path_unchanged(): + """A canonical narrow-form DCS sentence (proper noun + has + count + + observed counted_noun) still admits via the existing injector. + The widening must not regress the v1 admission path.""" + from generate.recognizer_match import _try_extract_discrete_count_anchor, _padded_lower + from generate.math_candidate_graph import _load_ratified_registry_or_empty + + reg = _load_ratified_registry_or_empty() + dcs_specs = [ + r.canonical_pattern for r in reg + if r.shape_category.value == "discrete_count_statement" + ] + assert dcs_specs, "no ratified DCS recognizer on main" + spec = dcs_specs[0] + + stmt = "Nicole has 400 Pokemon cards." + padded = _padded_lower(stmt) + anchor = _try_extract_discrete_count_anchor(stmt, padded, spec) + assert anchor is not None, ( + "canonical DCS extraction regressed post-W1 — " + f"'Nicole has 400 Pokemon cards.' should extract" + ) + assert anchor["subject_role"] == "Nicole" + assert anchor["count_token"] == "400" + assert anchor["counted_noun"] == "Pokemon cards"