feat(ADR-0170/W1): widen inject_from_match return type — no behavior change (#374)
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
This commit is contained in:
parent
ecc0072ea1
commit
eb452da9be
3 changed files with 196 additions and 8 deletions
|
|
@ -727,9 +727,24 @@ def parse_and_solve(
|
||||||
)
|
)
|
||||||
injected = inject_from_match(recognizer_match, s)
|
injected = inject_from_match(recognizer_match, s)
|
||||||
if injected:
|
if injected:
|
||||||
admitted: list[SentenceChoice] = [
|
# ADR-0170 — dispatch admissibility on the
|
||||||
c for c in injected if _initial_admissible(c)
|
# 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:
|
if len(admitted) == len(injected) and admitted:
|
||||||
per_sentence_choices.append(
|
per_sentence_choices.append(
|
||||||
_collapse_per_sentence_ties(admitted)
|
_collapse_per_sentence_ties(admitted)
|
||||||
|
|
|
||||||
|
|
@ -42,13 +42,23 @@ section) is preserved across this module:
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from typing import Mapping
|
from typing import Mapping, Union
|
||||||
|
|
||||||
from evals.refusal_taxonomy.shape_categories import ShapeCategory
|
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.math_problem_graph import InitialPossession, MathGraphError, Quantity
|
||||||
from generate.recognizer_match import RecognizerMatch
|
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
|
# Public surface
|
||||||
|
|
@ -58,12 +68,15 @@ from generate.recognizer_match import RecognizerMatch
|
||||||
def inject_from_match(
|
def inject_from_match(
|
||||||
match: RecognizerMatch,
|
match: RecognizerMatch,
|
||||||
sentence: str,
|
sentence: str,
|
||||||
) -> tuple[CandidateInitial, ...]:
|
) -> tuple[InjectorEmission, ...]:
|
||||||
"""Dispatch a recognizer match to its per-category injector.
|
"""Dispatch a recognizer match to its per-category injector.
|
||||||
|
|
||||||
Returns an empty tuple when the category has no v1 injector or when
|
Returns an empty tuple when the category has no v1 injector or when
|
||||||
the v1 injector refused. Skip-only behavior (the round-2 default)
|
the v1 injector refused. Per ADR-0170, the return type is now
|
||||||
is the empty-tuple result.
|
``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)
|
injector = _INJECTORS.get(match.category)
|
||||||
if injector is None:
|
if injector is None:
|
||||||
|
|
@ -260,6 +273,7 @@ _INJECTORS: Mapping[ShapeCategory, "type"] = {
|
||||||
|
|
||||||
|
|
||||||
__all__ = [
|
__all__ = [
|
||||||
|
"InjectorEmission",
|
||||||
"inject_from_match",
|
"inject_from_match",
|
||||||
"inject_discrete_count_statement",
|
"inject_discrete_count_statement",
|
||||||
]
|
]
|
||||||
|
|
|
||||||
159
tests/test_adr_0170_w1_injector_type_widening.py
Normal file
159
tests/test_adr_0170_w1_injector_type_widening.py
Normal file
|
|
@ -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"
|
||||||
Loading…
Reference in a new issue