From 5d1f1001f42f27548689953149eea74de3683008 Mon Sep 17 00:00:00 2001 From: Shay Date: Thu, 28 May 2026 08:51:29 -0700 Subject: [PATCH 1/2] feat(adr-0174-phase3a): lookback re-evaluation operator + pronoun resolution substrate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0174 Phase 3a — substrate for held-hypothesis lookback. Score unchanged at 3/47/0 (this PR is correctly-engineered infrastructure; eval impact gated on ADR-0163.x recognizer expansion documented in the follow-up brief). Adds generate/comprehension/lookback.py: - VALID_REFINEMENT_KINDS, VALID_UNRESOLVED_SLOTS — closed sets contracted with reader_trace consumer - PronounResolution refinement dataclass (pronoun + resolved_to + evidence_source, all validated) - Refinement Union (Phase 3b will widen with CompoundClauseExpansion) - ReevaluateResult dataclass with admit/eliminate consistency - reevaluate(hypothesis, refinement) operator — applies refinement, re-runs check_constraints, returns refined Hypothesis or None. - _rebuild_candidate_with_resolved_actor — rebuilds CandidateOperation / CandidateInitial replacing the semantic actor field (op.actor / initial.entity) while preserving matched_actor_token / matched_entity_token as the pronoun (so grounding still passes against the held statement's source span). Modifies generate/recognizer_match.py: - _try_extract_discrete_count_anchor: pronoun-subject statements now emit anchors with subject_role= + requires_pronoun_resolution marker, rather than refusing at the _REFUSED_SUBJECT_TOKENS check. The other narrowness layers (clause split, verb whitelist) still refuse; only the pronoun layer changes. Modifies generate/math_candidate_graph.py: - After inject_from_match, when any parsed_anchor carries requires_pronoun_resolution, the candidates are held as Hypothesis objects with unresolved=('actor_pronoun',). The lookback path then resolves via the existing _discourse_prior_subjects map and runs PronounResolution refinements through reevaluate. Resolved hypotheses flow into per_sentence_choices as if the regex parser had produced them; unresolved hypotheses drop cleanly (refusal- preferring). Emits 'lookback' JSON trace events with outcome ∈ {admitted, eliminated, no_antecedent}. Tests: - tests/test_adr_0174_phase3_lookback.py — 17 acceptance tests covering operator semantics on Operation/Initial, dataclass invariants, closed-set constants, end-to-end wiring on synthetic problems, and wrong=0 preservation on train_sample. Phase 3.1 follow-up brief: - docs/handoff/PHASE-3.1-FOLLOWUP-RECOGNIZER-EXPANSION.md documents the empirical finding that the train_sample bottleneck is verb-coverage (recognizer scope, ADR-0163.x) not lookback (ADR-0174 scope). 11 verbs identified for HITL contemplation pass. Recommends sequencing: Phase 3a now (substrate), ADR-0163.x verb expansion next, Phase 3b after coverage matures. Acceptance verified: - 17/17 Phase 3a tests pass - 95/95 existing tests pass (Phase 1 + Phase 2 + brief_11 + reader_phase2) - Smoke 67/67, packs 141/141, lanes 8/8 - wrong=0 preserved, score unchanged 3/47/0 (intentional per brief) Stacks on Phase 2 (PR #420). Rebases onto main after #416 + #420 land. --- ...PHASE-3.1-FOLLOWUP-RECOGNIZER-EXPANSION.md | 179 ++++++++ generate/comprehension/lookback.py | 387 +++++++++++++++++ generate/math_candidate_graph.py | 101 +++++ generate/recognizer_match.py | 19 +- tests/test_adr_0174_phase3_lookback.py | 410 ++++++++++++++++++ 5 files changed, 1093 insertions(+), 3 deletions(-) create mode 100644 docs/handoff/PHASE-3.1-FOLLOWUP-RECOGNIZER-EXPANSION.md create mode 100644 generate/comprehension/lookback.py create mode 100644 tests/test_adr_0174_phase3_lookback.py diff --git a/docs/handoff/PHASE-3.1-FOLLOWUP-RECOGNIZER-EXPANSION.md b/docs/handoff/PHASE-3.1-FOLLOWUP-RECOGNIZER-EXPANSION.md new file mode 100644 index 00000000..e8f5fe28 --- /dev/null +++ b/docs/handoff/PHASE-3.1-FOLLOWUP-RECOGNIZER-EXPANSION.md @@ -0,0 +1,179 @@ +# Phase 3.1 Follow-up — Verb-coverage bottleneck on train_sample/v1 + +**Status:** Open recommendation +**Date:** 2026-05-28 +**Author:** Shay (analysis surfaced during ADR-0174 Phase 3a) +**Parent:** [ADR-0174 — Held-Hypothesis Comprehension](../decisions/ADR-0174-held-hypothesis-comprehension.md) +**Related ADRs:** ADR-0163 (path to GSM8K mastery), ADR-0167 (audit-as-teaching-evidence), ADR-0150/0152/0155/0161 (HITL corridor) + +--- + +## Context + +ADR-0174 Phase 3 specified a `correct ≥ 8` lift target on +`evals/gsm8k_math/train_sample/v1` (≥ 5 of the 21 currently-empty +`discrete_count_statement` anchors admitted via lookback). Empirical +analysis during Phase 3a implementation found this target is +**not achievable through lookback alone** on this corpus. The +substrate is built correctly; the bottleneck is elsewhere. + +## What Phase 3a shipped + +- `generate/comprehension/lookback.py` — the `reevaluate` operator, + `PronounResolution` refinement type, `ReevaluateResult` dataclass. +- Held-anchor emission in `recognizer_match._try_extract_discrete_count_anchor` + (pronoun-subject statements carry `requires_pronoun_resolution=True` + rather than refusing). +- Lookback wiring at `math_candidate_graph.parse_and_solve`'s + recognizer-injection branch — applies `PronounResolution` against + the existing `_discourse_prior_subjects` map; emits `lookback` JSON + trace events with `outcome ∈ {admitted, eliminated, no_antecedent}`. +- 17 acceptance tests proving the wiring works on synthetic problems + (`tests/test_adr_0174_phase3_lookback.py`). +- `wrong = 0` invariant preserved; score unchanged at 3/47/0. + +## Why Phase 3a did not lift the score + +The 21 empty-anchor `discrete_count_statement` refusals on +train_sample/v1 break down as: + +| Structural cause | Cases | +|---|---| +| Pronoun-only (no compound clause) | 2 — 0002, 0034 | +| Compound-only | 8 | +| Pronoun + compound | 5 | +| Other narrowness fail (verb/structure) | 6 | + +For Phase 3a to lift any case, **three conditions** must all hold: + +1. The matcher's recognizer registry recognises the statement. +2. The extractor passes every narrowness layer **before** the pronoun + check. Specifically the verb must be in `_POSSESSION_VERBS` (`has`, + `have`, `had`) or `_ACQUISITION_VERBS` (`collected`, `collects`, + `collect`, `received`, `receives`, `receive`, `bought`, `buys`, + `buy`, `got`, `gets`, `get`). +3. The candidate-graph's regex path (`_filtered_statement_choices`) + must return empty for the same statement — otherwise the regex + path commits the candidate (with the pronoun still as actor) and + the recognizer-injection branch never runs. + +Verb checks against the 13 cases with compound/pronoun structure: + +| Case | Statement (excerpt) | Verb | In whitelist? | +|---|---|---|---| +| 0002 | She **splits** it up... | splits | No | +| 0034 | He can **run** 40 yards... | run | No | +| 0020 | Two puppies, two kittens... **were for sale**... | were | No | +| 0021 | He **bench presses** 15 pounds... | presses | No | +| 0027 | Malcolm **has** 240 followers... | has | **Yes** | +| 0033 | Rachel **is** 12 years old... | is | No | +| 0040 | He now **has** 2 horses... | has | **Yes** | +| 0041 | Troy **bakes** 2 pans... | bakes | No | +| 0044 | John **invests** in a bank... | invests | No | +| 0045 | On Monday he **finished** 3 surveys... | finished | No | +| 0047 | John **bakes** 12 coconut macaroons... | bakes | No | +| ... | | | | + +Only **two** cases (0027, 0040) cross the verb whitelist. Both also +fail at the compound-clause narrowness layer (which comes earlier +than the pronoun check), so even adding compound-clause held +hypotheses (Phase 3b) would have to fire first. + +**Conclusion:** the empirical bottleneck on train_sample/v1 is +**verb-set coverage**, not lookback or held hypotheses. ADR-0174 is +the wrong tool for moving this score. + +## Recommended path forward + +ADR-0163 is the correct scope for verb-coverage expansion via the +HITL corridor. The path: + +1. **Run `core eval math-contemplation` on the 11 failing verbs** — + `splits`, `run`, `bench presses`, `is`, `bakes`, `invests`, + `finished`, `donated`, `wants`, `gained`, `eat`. These surface as + `MathReaderRefusalEvidence` audit rows that the contemplation lane + already consumes (ADR-0167). +2. **Operator review in workbench** — categorise each verb: + - Acquisition-class (engine should treat as `add`): `received`, + `bought`, etc. — verbs that grammatically gain quantity to actor. + Candidates from list: `gained`, `won`, `earned`, `saved`, + `accumulated`, `acquired`. + - Depletion-class (engine should treat as `subtract`): `gives`, + `loses`, `spends`. Candidates: `donated`, `gave`, `eats`, + `consumed`, `lost`, `spent`. + - Non-arithmetic verbs (engine should refuse and ask): `is`, + `wants`, `bench presses`, `splits`, `run`, `bakes`, `invests`. + These do not carry possession/acquisition semantics; the right + answer is a different intent (rate / capacity / descriptive), + not a wider `add`/`subtract` whitelist. + + The first two classes ratify into the registry via the existing + ADR-0150/0152 corridor (proposal → review → packed). The third + class becomes refusal-typed evidence that informs whether a + separate recognizer category is needed (e.g. a `capacity_statement` + recognizer for "He can run 40 yards in 5 seconds" rather than + forcing it into `discrete_count_statement`). + +3. **After verb widening lands** — re-run Phase 3a's lookback wiring + on the corpus. The cases that were previously verb-blocked now + reach the pronoun-check layer, and the held-hypothesis path admits + them. Expected lift from this combination: roughly the 13 cases + with pronoun/compound structure that have an arithmetic-class verb + under the widened whitelist. + +## What this means for ADR-0174 + +The held-hypothesis substrate (Phase 1 + 2 + 3a) is correct +architecture and load-bearing for Phase 4 (in-loop contemplation) and +Phase 5 (legacy-parser removal). Its **eval impact** depends on +upstream recognizer coverage maturing through the ADR-0163.x +corridor. These two efforts are complementary, not competing — the +substrate makes lookback possible, the recognizer expansion gives +lookback something to fire on. + +The cleanest sequencing is: + +1. **ADR-0174 Phase 3a (this PR)** — substrate landed. +2. **ADR-0163.x verb expansion** (this brief's recommendation) — + widens the corpus surface that the substrate can act on. +3. **ADR-0174 Phase 3b** — compound-clause held hypotheses. Once the + verb-coverage bottleneck is gone, compound-clause expansion + surfaces real cases. Currently it would surface zero on + train_sample for the same reason Phase 3a does: most compound + cases also fail the verb check before reaching the clause-split + narrowness layer. +4. **ADR-0174 Phase 4** — in-loop contemplation. Builds on Phase 3 + substrate. +5. **ADR-0174 Phase 5** — legacy parser removal. + +## Decision needed (from operator) + +- **Authorise the ADR-0163.x verb-expansion contemplation pass?** + Concretely: run `core eval math-contemplation` against the 11 + failing verbs above; review the proposals in workbench; ratify + acquisition/depletion entries that are unambiguous. + +- **Re-scope ADR-0174 Phase 3b** to "post-recognizer-expansion + re-measurement" rather than "compound-clause held hypotheses"? + Phase 3b should land only after verb expansion exposes cases that + exercise its compound-clause logic. + +No timelines are proposed; this is a sequencing recommendation. The +substrate work in Phase 3a is already merged on its own merits +(correctness and Phase 4/5 prerequisite); Phase 3b waits on +recognizer coverage. + +## Cross-references + +- ADR-0174 §Phase 3 acceptance — the criteria this brief documents + as unmet (with structural-cause analysis). +- `tests/test_adr_0174_phase3_lookback.py` — proves the substrate + works on synthetic problems even though no train_sample case + exercises it. +- `feedback-wrong-zero-hazard-case-0050` memory — verb expansion + must preserve the case-0050 canary; the recommended depletion-class + additions should be reviewed against this hazard before ratification. +- `thesis-decoding-not-generating` — the verb-class + contemplation/HITL path is the right "teach the engine to find + better" mechanism; widening the static whitelist directly would be + "storing another found thing." diff --git a/generate/comprehension/lookback.py b/generate/comprehension/lookback.py new file mode 100644 index 00000000..4dffe3d4 --- /dev/null +++ b/generate/comprehension/lookback.py @@ -0,0 +1,387 @@ +"""ADR-0174 Phase 3 — lookback re-evaluation operator. + +When a hypothesis carries unresolved slots (entries in +``Hypothesis.unresolved``), a later token can refine those slots — +binding a pronoun to its proper-noun antecedent, attaching a unit to a +dangling quantity, narrowing an ambiguous verb category. This module +provides the ``reevaluate`` operator that applies a refinement to a +held hypothesis and re-runs the existing admissibility check. + +Phase 3a (this module): the **substrate** for lookback. Concrete +refinement types: + + - :class:`PronounResolution` — bind a pronoun actor to a resolved + proper-noun referent. The ``matched_actor_token`` stays as the + pronoun (which IS in source, so grounding still passes); only the + semantic actor field on the underlying Operation / + InitialPossession is rewritten to the resolved name. + +Phase 3b (follow-up): :class:`CompoundClauseExpansion` and other +refinement types covering the remaining compound-clause and verb-set +narrowness layers. The reevaluate operator handles any refinement +type implementing :class:`Refinement`; adding new types does not +require ADR amendments to this module (only to the closed list of +``VALID_REFINEMENT_KINDS`` so the trace consumer can branch +deterministically). + +Trust boundary: this module never weakens an admissibility predicate. +Every refined hypothesis is re-run through +:func:`generate.comprehension.constraint_propagation.check_constraints` +after refinement; a hypothesis that would fail constraints in its +refined form is eliminated, returning ``None`` from ``reevaluate``. +The ``wrong = 0`` invariant is preserved by construction — refinement +can only convert a refused hypothesis into an admitted one if every +admissibility sub-check passes on the refined candidate. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Final, Literal, Union + +from generate.comprehension.constraint_propagation import ( + ConstraintResult, + check_constraints, +) +from generate.comprehension.state import ( + ComprehensionStateError, + Hypothesis, +) + + +# --------------------------------------------------------------------------- +# Closed set of refinement kinds (trace contract) +# --------------------------------------------------------------------------- + +# Each kind name identifies one concrete refinement subclass below. +# Adding a new kind requires adding a subclass AND extending this set +# (so the reader_trace consumer can branch deterministically without +# pattern-matching on Python types). +VALID_REFINEMENT_KINDS: Final[frozenset[str]] = frozenset( + { + "pronoun_resolution", + # Phase 3b will add: "compound_clause_expansion", etc. + } +) + + +# Closed slot names that may appear in Hypothesis.unresolved tuples. +# Refinements bind to one of these slots; the reevaluate operator +# matches refinement→slot via this closed set. +VALID_UNRESOLVED_SLOTS: Final[frozenset[str]] = frozenset( + { + "actor_pronoun", + # Phase 3b will add: "clause_separator", etc. + } +) + + +# --------------------------------------------------------------------------- +# Refinement types — sealed union via Refinement Union alias +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True, slots=True) +class PronounResolution: + """Refinement: bind an unresolved-actor pronoun to a proper-noun antecedent. + + Phase 3a — applied to a held :class:`Hypothesis` carrying + ``"actor_pronoun"`` in its ``unresolved`` tuple. The refinement + leaves the underlying candidate's ``matched_actor_token`` intact + (the pronoun, which grounds in the held statement's source span), + and rewrites the candidate's semantic actor field + (``Operation.actor`` for :class:`CandidateOperation`, + ``InitialPossession.entity`` for :class:`CandidateInitial`) to the + resolved name. + + The ``pronoun`` field carries the surface form for trace fidelity + so the reader_trace event can record which pronoun was resolved at + which token position. + + Fields: + kind: Literal "pronoun_resolution" (matches + VALID_REFINEMENT_KINDS membership; structural + discriminator for the Union below). + pronoun: Surface pronoun in the held statement + (e.g. ``"She"``, ``"He"``, ``"it"``). + resolved_to: Proper-noun referent + (e.g. ``"Jan"``, ``"Georgie"``). + evidence_source: Where the resolution came from — a closed set + of values so the trace consumer can attribute + deterministically. + """ + + pronoun: str + resolved_to: str + evidence_source: Literal["discourse_prior_subjects", "running_subject"] + kind: Literal["pronoun_resolution"] = "pronoun_resolution" + + def __post_init__(self) -> None: + if not isinstance(self.pronoun, str) or not self.pronoun: + raise ComprehensionStateError( + "PronounResolution.pronoun must be non-empty str" + ) + if not isinstance(self.resolved_to, str) or not self.resolved_to: + raise ComprehensionStateError( + "PronounResolution.resolved_to must be non-empty str" + ) + if self.evidence_source not in ( + "discourse_prior_subjects", + "running_subject", + ): + raise ComprehensionStateError( + "PronounResolution.evidence_source must be in " + "{'discourse_prior_subjects', 'running_subject'}; " + f"got {self.evidence_source!r}" + ) + if self.kind != "pronoun_resolution": + raise ComprehensionStateError( + "PronounResolution.kind must be 'pronoun_resolution'" + ) + + +# Sealed union of all Phase-3 refinement types. Phase 3b will extend +# this with CompoundClauseExpansion etc. +Refinement = Union[PronounResolution] + + +# --------------------------------------------------------------------------- +# Internal: rebuild candidate with resolved actor +# --------------------------------------------------------------------------- + + +def _rebuild_candidate_with_resolved_actor( + candidate: object, resolved_to: str +) -> object | None: + """Rebuild a candidate replacing the semantic-actor field. + + For :class:`CandidateOperation`: rebuilds with a new + :class:`Operation` carrying ``actor=resolved_to`` and keeps every + other slot (operand, matched_verb, matched_value_token, etc.) + intact. ``matched_actor_token`` deliberately stays as the pronoun + so the grounding check (``_token_in(matched_actor_token, haystack)``) + continues to pass against the held statement's source span. + + For :class:`CandidateInitial`: rebuilds with a new + :class:`InitialPossession` carrying ``entity=resolved_to`` and + preserves ``matched_entity_token`` as the pronoun. + + Returns ``None`` when the candidate is not a known type — the + caller treats this as a refinement-no-op (the hypothesis remains + held, lookback did not find anything to do here). + """ + # Lazy imports to avoid circular dependency on candidate-graph layer. + from generate.math_candidate_parser import CandidateInitial + from generate.math_problem_graph import ( + InitialPossession, + Operation, + ) + from generate.math_roundtrip import CandidateOperation + + if isinstance(candidate, CandidateOperation): + old_op = candidate.op + new_op = Operation( + actor=resolved_to, + kind=old_op.kind, + operand=old_op.operand, + target=old_op.target, + ) + return CandidateOperation( + op=new_op, + source_span=candidate.source_span, + matched_verb=candidate.matched_verb, + matched_value_token=candidate.matched_value_token, + matched_unit_token=candidate.matched_unit_token, + matched_actor_token=candidate.matched_actor_token, + matched_target_token=candidate.matched_target_token, + matched_reference_actor_token=candidate.matched_reference_actor_token, + ) + if isinstance(candidate, CandidateInitial): + old_initial = candidate.initial + new_initial = InitialPossession( + entity=resolved_to, + quantity=old_initial.quantity, + ) + return CandidateInitial( + initial=new_initial, + source_span=candidate.source_span, + matched_anchor=candidate.matched_anchor, + matched_value_token=candidate.matched_value_token, + matched_unit_token=candidate.matched_unit_token, + matched_entity_token=candidate.matched_entity_token, + composition_evidence=candidate.composition_evidence, + ) + return None + + +# --------------------------------------------------------------------------- +# reevaluate operator +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True, slots=True) +class ReevaluateResult: + """Outcome of a single reevaluate call. + + Fields: + refined: The refined Hypothesis if admitted, else None. + previous: The original hypothesis before refinement + (carried so trace events can record what changed). + refinement_kind: The refinement kind that was attempted + (matches Refinement.kind on the input). + constraint_result: The result of re-running check_constraints + on the refined candidate (or None if refinement + could not be applied because the candidate + type was unknown). + elimination_reason: Non-None iff refined is None. + """ + + refined: Hypothesis | None + previous: Hypothesis + refinement_kind: str + constraint_result: ConstraintResult | None + elimination_reason: str | None + + def __post_init__(self) -> None: + if self.refinement_kind not in VALID_REFINEMENT_KINDS: + raise ComprehensionStateError( + "ReevaluateResult.refinement_kind must be in " + f"VALID_REFINEMENT_KINDS; got {self.refinement_kind!r}" + ) + if self.refined is None and self.elimination_reason is None: + raise ComprehensionStateError( + "ReevaluateResult.refined=None requires a non-None " + "elimination_reason" + ) + if self.refined is not None and self.elimination_reason is not None: + raise ComprehensionStateError( + "ReevaluateResult.refined is not None but " + f"elimination_reason={self.elimination_reason!r} is set; " + "these are inconsistent" + ) + + +def reevaluate( + hypothesis: Hypothesis, refinement: Refinement +) -> ReevaluateResult: + """Apply ``refinement`` to ``hypothesis``, re-run constraint check. + + Per ADR-0174 §Decision §Lookback: lookback walks open hypotheses + and recomputes prior assignments when a later token resolves an + earlier ambiguity. This operator is the per-hypothesis primitive + that pass invokes. + + Semantics: + + - If the refinement targets a slot that isn't in + ``hypothesis.unresolved``, the refinement is a no-op: the + function returns ``ReevaluateResult(refined=hypothesis, …)`` so + the caller can keep the hypothesis unchanged. This matches + the ADR's "uncontested tokens contribute no recomputation + work" bound. + - If the refinement applies but the rebuilt candidate fails the + re-run constraint check, ``refined`` is ``None`` and + ``elimination_reason`` carries the first failing predicate's + reason. + - If the refinement applies and constraints pass, ``refined`` is + a new :class:`Hypothesis` with the resolved slot removed from + ``unresolved`` and a ``category_assignments`` entry recording + the refinement event. + + The returned :class:`ReevaluateResult` always includes the previous + hypothesis so trace serialisation can record the before/after pair. + """ + # Dispatch on refinement kind. Phase 3a knows pronoun_resolution. + if isinstance(refinement, PronounResolution): + return _apply_pronoun_resolution(hypothesis, refinement) + raise ComprehensionStateError( + f"reevaluate: unsupported refinement type {type(refinement).__name__}" + ) + + +def _apply_pronoun_resolution( + hypothesis: Hypothesis, refinement: PronounResolution +) -> ReevaluateResult: + """Inner: rebuild candidate with resolved actor, re-run constraints.""" + if "actor_pronoun" not in hypothesis.unresolved: + # No-op: this hypothesis doesn't carry the slot the refinement + # targets. Return unchanged. + return ReevaluateResult( + refined=hypothesis, + previous=hypothesis, + refinement_kind=refinement.kind, + constraint_result=None, + elimination_reason=None, + ) + + rebuilt = _rebuild_candidate_with_resolved_actor( + hypothesis.candidate, refinement.resolved_to + ) + if rebuilt is None: + # Candidate type unknown — refinement cannot apply. Return the + # hypothesis unchanged; caller can decide whether to eliminate + # on its own terms. + return ReevaluateResult( + refined=hypothesis, + previous=hypothesis, + refinement_kind=refinement.kind, + constraint_result=None, + elimination_reason=None, + ) + + # Build the refined hypothesis: same rank, same confidence_rank, + # category_assignments extended with a refinement trace entry, + # unresolved minus the now-resolved slot. + # + # The trace entry uses token_index=0 because Phase 3a applies + # refinements at the problem level (after all sentences have been + # processed), not at a specific token position. Phase 3b/4 may + # specialise this when refinements fire mid-token-stream. + refined_assignments = hypothesis.category_assignments + ( + (0, "pronoun_resolved", refinement.pronoun), + ) + refined_unresolved = tuple( + slot for slot in hypothesis.unresolved if slot != "actor_pronoun" + ) + + refined_hyp = Hypothesis( + candidate=rebuilt, + category_assignments=refined_assignments, + constraint_state=hypothesis.constraint_state, + confidence_rank=hypothesis.confidence_rank, + unresolved=refined_unresolved, + ) + + # Re-run constraints. Refinement preserves wrong=0 by gating on the + # same admissibility predicates that admit candidates today; a + # refinement that produces a candidate failing any sub-check is + # eliminated. + result = check_constraints(refined_hyp) + if result.admitted: + return ReevaluateResult( + refined=refined_hyp, + previous=hypothesis, + refinement_kind=refinement.kind, + constraint_result=result, + elimination_reason=None, + ) + return ReevaluateResult( + refined=None, + previous=hypothesis, + refinement_kind=refinement.kind, + constraint_result=result, + elimination_reason=( + f"pronoun_resolution refined candidate failed re-check: " + f"{result.elimination_reason}" + ), + ) + + +__all__ = [ + "PronounResolution", + "Refinement", + "ReevaluateResult", + "VALID_REFINEMENT_KINDS", + "VALID_UNRESOLVED_SLOTS", + "reevaluate", +] diff --git a/generate/math_candidate_graph.py b/generate/math_candidate_graph.py index 9448c0d6..f06c2760 100644 --- a/generate/math_candidate_graph.py +++ b/generate/math_candidate_graph.py @@ -35,6 +35,7 @@ from __future__ import annotations import json import re +from collections.abc import Mapping from dataclasses import dataclass from itertools import product from typing import TYPE_CHECKING, Final, Union @@ -833,6 +834,106 @@ def parse_and_solve( inject_from_match, ) injected = inject_from_match(recognizer_match, s) + # ADR-0174 Phase 3 — lookback pronoun resolution. + # When the matcher tagged any anchor with + # ``requires_pronoun_resolution``, the injected + # candidates carry the pronoun as actor/entity and + # are held until lookback either binds them to a + # discourse antecedent or drops them. The + # discourse map (_discourse_prior_subjects / + # _prior_subject) is consulted in the same + # precedence as ME-2 cross-sentence binding so + # behaviour is consistent across recognizer + # categories. When no antecedent is available, + # we drop the candidates (refusal-preferring; + # preserves wrong=0). + if injected and any( + isinstance(a, Mapping) + and a.get("requires_pronoun_resolution") + for a in recognizer_match.parsed_anchors + ): + from generate.comprehension.lookback import ( + PronounResolution, + reevaluate, + ) + from generate.comprehension.constraint_propagation import ( + hypothesis_from_initial as _hyp_from_initial, + hypothesis_from_operation as _hyp_from_operation, + ) + + # Extract the held pronoun (the matcher + # guarantees a single subject_role across the + # anchor set for discrete_count_statement v1). + _held_pronoun: str | None = None + for a in recognizer_match.parsed_anchors: + if ( + isinstance(a, Mapping) + and a.get("requires_pronoun_resolution") + ): + _sr = a.get("subject_role") + if isinstance(_sr, str): + _held_pronoun = _sr + break + + _antecedent = _effective_prior + if _held_pronoun is None or not _antecedent: + # No resolution path available — drop the + # held candidates and log the lookback + # event so the trace records why. + _statement_trace.append(json.dumps({ + "layer": "lookback", + "phase": 3, + "outcome": "no_antecedent", + "pronoun": _held_pronoun or "", + "sentence_index": s_idx, + }, sort_keys=True)) + injected = () + else: + _refinement = PronounResolution( + pronoun=_held_pronoun, + resolved_to=_antecedent, + evidence_source=( + "discourse_prior_subjects" + if s in _discourse_prior_subjects + else "running_subject" + ), + ) + _resolved: list[object] = [] + _all_resolved = True + for _rank, _c in enumerate(injected): + if isinstance(_c, CandidateInitial): + _base = _hyp_from_initial(_c, _rank) + elif isinstance(_c, CandidateOperation): + _base = _hyp_from_operation(_c, _rank) + else: + _all_resolved = False + break + _held = Hypothesis( + candidate=_base.candidate, + category_assignments=_base.category_assignments, + constraint_state=_base.constraint_state, + confidence_rank=_base.confidence_rank, + unresolved=("actor_pronoun",), + ) + _result = reevaluate(_held, _refinement) + _statement_trace.append(json.dumps({ + "layer": "lookback", + "phase": 3, + "outcome": "admitted" if _result.refined else "eliminated", + "pronoun": _held_pronoun, + "resolved_to": _antecedent, + "confidence_rank": _rank, + "evidence_source": _refinement.evidence_source, + "sentence_index": s_idx, + }, sort_keys=True)) + if _result.refined is None: + _all_resolved = False + break + _resolved.append(_result.refined.candidate) # type: ignore[arg-type] + if _all_resolved and _resolved: + injected = tuple(_resolved) + else: + injected = () if injected: # ADR-0174 Phase 2 — hypothesis-based admission # with structured elimination tracing. Each diff --git a/generate/recognizer_match.py b/generate/recognizer_match.py index 3c63e98d..75209bb4 100644 --- a/generate/recognizer_match.py +++ b/generate/recognizer_match.py @@ -944,8 +944,15 @@ def _try_extract_discrete_count_anchor( return None subject = m.group("subject") - if subject.lower() in _REFUSED_SUBJECT_TOKENS: - return None + # ADR-0174 Phase 3 — pronoun-subject statements are no longer + # rejected outright. Instead they emit a HELD anchor (with the + # pronoun in subject_role and ``requires_pronoun_resolution=True``) + # so the downstream candidate-graph layer can stash them in + # ``ProblemReadingState.open_hypotheses`` and run the lookback + # reevaluate pass against the discourse subject map. When no + # antecedent resolves, the held hypothesis is dropped (refusal- + # preferring discipline preserves wrong=0). + requires_pronoun_resolution = subject.lower() in _REFUSED_SUBJECT_TOKENS verb = m.group("verb").lower() if verb in _POSSESSION_VERBS: @@ -988,7 +995,7 @@ def _try_extract_discrete_count_anchor( canon = observed_n break - return { + anchor: dict[str, Any] = { "kind": "discrete_count", "subject_role": subject, "count_token": count_token, @@ -1000,6 +1007,12 @@ def _try_extract_discrete_count_anchor( "anchor_kind": anchor_kind, "verb_token": verb, } + if requires_pronoun_resolution: + # ADR-0174 Phase 3 marker — the downstream injector reads this + # and emits a held CandidateOperation/CandidateInitial whose + # Hypothesis carries unresolved=("actor_pronoun",). + anchor["requires_pronoun_resolution"] = True + return anchor def _match_multiplicative_aggregation( diff --git a/tests/test_adr_0174_phase3_lookback.py b/tests/test_adr_0174_phase3_lookback.py new file mode 100644 index 00000000..8494d05a --- /dev/null +++ b/tests/test_adr_0174_phase3_lookback.py @@ -0,0 +1,410 @@ +"""ADR-0174 Phase 3a — lookback re-evaluation operator + pronoun resolution. + +Acceptance tests: + + 1. ``reevaluate`` operator semantics: no-op when refinement doesn't + apply, refined hypothesis when constraints pass, None when + constraints fail post-refinement. + + 2. ``PronounResolution`` dataclass invariants — validates pronoun / + resolved_to / evidence_source / kind shape. + + 3. End-to-end wiring: a synthetic problem with pronoun-subject + statement that the regex parser refuses fires the lookback path + and emits an "admitted" trace event when the discourse antecedent + resolves. + + 4. Refusal-preferring discipline: a held statement with no discourse + antecedent emits a "no_antecedent" trace event and drops cleanly. + + 5. wrong=0 preserved on train_sample/v1 (score unchanged at 3/47/0). + +Phase 3a substrate scope: this PR builds the reevaluate operator and +wires pronoun resolution into the recognizer-injection branch of +math_candidate_graph.parse_and_solve. The wiring is correct but does +not fire on any of the 50 train_sample cases because the cases the +ADR identified (21 empty-anchor discrete_count failures) refuse for +verb-set narrowness reasons (recognizer scope, ADR-0163.x) BEFORE +reaching the pronoun layer. This file therefore exercises the wiring +via synthetic problems that target the path directly. + +See docs/handoff/PHASE-3.1-FOLLOWUP-RECOGNIZER-EXPANSION.md for the +follow-up brief documenting which recognizer expansions would surface +real cases that exercise this path on the train_sample corpus. +""" + +from __future__ import annotations + +import json + +import pytest + +from generate.comprehension.constraint_propagation import ( + hypothesis_from_initial, + hypothesis_from_operation, +) +from generate.comprehension.lookback import ( + PronounResolution, + ReevaluateResult, + VALID_REFINEMENT_KINDS, + VALID_UNRESOLVED_SLOTS, + reevaluate, +) +from generate.comprehension.state import ( + ComprehensionStateError, + Hypothesis, +) +from generate.math_candidate_parser import CandidateInitial +from generate.math_problem_graph import ( + InitialPossession, + Operation, + Quantity, +) +from generate.math_roundtrip import CandidateOperation + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _operation_with_pronoun_actor( + pronoun: str = "He", + source_span: str = "He buys 3 apples.", +) -> CandidateOperation: + return CandidateOperation( + op=Operation( + actor=pronoun, kind="add", + operand=Quantity(value=3, unit="apples"), + ), + source_span=source_span, + matched_verb="buys", + matched_value_token="3", + matched_unit_token="apples", + matched_actor_token=pronoun, + ) + + +def _initial_with_pronoun_actor( + pronoun: str = "She", + source_span: str = "She has 5 books.", +) -> CandidateInitial: + return CandidateInitial( + initial=InitialPossession( + entity=pronoun, + quantity=Quantity(value=5, unit="books"), + ), + source_span=source_span, + matched_anchor="has", + matched_value_token="5", + matched_unit_token="books", + matched_entity_token=pronoun, + ) + + +def _held_hypothesis(candidate: object, rank: int = 0) -> Hypothesis: + """Construct a Hypothesis with unresolved=('actor_pronoun',) as the + Phase 3 wiring would produce.""" + if isinstance(candidate, CandidateOperation): + base = hypothesis_from_operation(candidate, rank) + elif isinstance(candidate, CandidateInitial): + base = hypothesis_from_initial(candidate, rank) + else: + raise ValueError(f"unknown candidate type {type(candidate).__name__}") + return Hypothesis( + candidate=base.candidate, + category_assignments=base.category_assignments, + constraint_state=base.constraint_state, + confidence_rank=base.confidence_rank, + unresolved=("actor_pronoun",), + ) + + +# --------------------------------------------------------------------------- +# 1. reevaluate operator semantics +# --------------------------------------------------------------------------- + + +class TestReevaluateOnOperation: + def test_pronoun_resolution_succeeds_when_constraints_pass(self) -> None: + cand = _operation_with_pronoun_actor(pronoun="He") + hyp = _held_hypothesis(cand, rank=0) + ref = PronounResolution( + pronoun="He", resolved_to="Bob", + evidence_source="discourse_prior_subjects", + ) + result = reevaluate(hyp, ref) + assert result.refined is not None + assert result.elimination_reason is None + # The Operation.actor is rewritten to the resolved name. + assert result.refined.candidate.op.actor == "Bob" # type: ignore[attr-defined] + # matched_actor_token STAYS as the pronoun so grounding still + # passes against the held statement's source span. + assert result.refined.candidate.matched_actor_token == "He" # type: ignore[attr-defined] + # The 'actor_pronoun' slot is no longer unresolved. + assert "actor_pronoun" not in result.refined.unresolved + # category_assignments records the refinement event for trace. + assert any( + tup[1] == "pronoun_resolved" + for tup in result.refined.category_assignments + ) + + def test_noop_when_hypothesis_has_no_unresolved_pronoun(self) -> None: + cand = _operation_with_pronoun_actor() + hyp = hypothesis_from_operation(cand, 0) # unresolved=() by default + ref = PronounResolution( + pronoun="He", resolved_to="Bob", + evidence_source="discourse_prior_subjects", + ) + result = reevaluate(hyp, ref) + # No-op: refined == original hypothesis, no elimination. + assert result.refined is hyp + assert result.elimination_reason is None + assert result.constraint_result is None + + def test_eliminated_when_refined_candidate_fails_constraints(self) -> None: + # Construct a candidate whose source_span doesn't contain the + # pronoun — the rebuilt candidate will fail matched_actor_token + # grounding because the pronoun isn't in haystack. This is a + # degenerate case but proves the constraint re-check actually + # fires. + cand = CandidateOperation( + op=Operation( + actor="He", kind="add", + operand=Quantity(value=3, unit="apples"), + ), + source_span="Sam buys 3 apples.", # 'He' not in source + matched_verb="buys", + matched_value_token="3", + matched_unit_token="apples", + matched_actor_token="He", # would fail _token_in even before refinement + ) + hyp = _held_hypothesis(cand, 0) + ref = PronounResolution( + pronoun="He", resolved_to="Bob", + evidence_source="discourse_prior_subjects", + ) + result = reevaluate(hyp, ref) + assert result.refined is None + assert result.elimination_reason is not None + # The first failing predicate is operation.actor_grounds — + # observable via constraint_result.predicates_run. + assert result.constraint_result is not None + first_fail = next( + (p for p, o in result.constraint_result.predicates_run if o == "fail"), + None, + ) + assert first_fail == "operation.actor_grounds" + + +class TestReevaluateOnInitial: + def test_pronoun_resolution_rewrites_initial_entity(self) -> None: + cand = _initial_with_pronoun_actor(pronoun="She") + hyp = _held_hypothesis(cand, rank=0) + ref = PronounResolution( + pronoun="She", resolved_to="Jan", + evidence_source="discourse_prior_subjects", + ) + result = reevaluate(hyp, ref) + assert result.refined is not None + assert result.refined.candidate.initial.entity == "Jan" # type: ignore[attr-defined] + assert result.refined.candidate.matched_entity_token == "She" # type: ignore[attr-defined] + + +class TestReevaluateResultDataclass: + def test_invalid_refinement_kind_refused(self) -> None: + cand = _operation_with_pronoun_actor() + hyp = _held_hypothesis(cand, 0) + with pytest.raises( + ComprehensionStateError, match="refinement_kind must be in" + ): + ReevaluateResult( + refined=hyp, previous=hyp, + refinement_kind="not_a_kind", + constraint_result=None, + elimination_reason=None, + ) + + def test_inconsistent_refined_and_elimination_refused(self) -> None: + cand = _operation_with_pronoun_actor() + hyp = _held_hypothesis(cand, 0) + with pytest.raises( + ComprehensionStateError, match="inconsistent" + ): + ReevaluateResult( + refined=hyp, previous=hyp, + refinement_kind="pronoun_resolution", + constraint_result=None, + elimination_reason="impossible combo", + ) + + def test_none_refined_requires_elimination_reason(self) -> None: + cand = _operation_with_pronoun_actor() + hyp = _held_hypothesis(cand, 0) + with pytest.raises( + ComprehensionStateError, match="requires a non-None" + ): + ReevaluateResult( + refined=None, previous=hyp, + refinement_kind="pronoun_resolution", + constraint_result=None, + elimination_reason=None, + ) + + +# --------------------------------------------------------------------------- +# 2. PronounResolution dataclass invariants +# --------------------------------------------------------------------------- + + +class TestPronounResolutionConstruction: + def test_minimal_valid(self) -> None: + r = PronounResolution( + pronoun="He", resolved_to="Bob", + evidence_source="discourse_prior_subjects", + ) + assert r.kind == "pronoun_resolution" + assert r.pronoun == "He" + + def test_empty_pronoun_refused(self) -> None: + with pytest.raises(ComprehensionStateError, match="pronoun"): + PronounResolution( + pronoun="", resolved_to="Bob", + evidence_source="discourse_prior_subjects", + ) + + def test_empty_resolved_to_refused(self) -> None: + with pytest.raises(ComprehensionStateError, match="resolved_to"): + PronounResolution( + pronoun="He", resolved_to="", + evidence_source="discourse_prior_subjects", + ) + + def test_invalid_evidence_source_refused(self) -> None: + with pytest.raises(ComprehensionStateError, match="evidence_source"): + PronounResolution( + pronoun="He", resolved_to="Bob", + evidence_source="grok_intuition", # type: ignore[arg-type] + ) + + def test_kind_must_be_pronoun_resolution(self) -> None: + with pytest.raises(ComprehensionStateError, match="kind"): + PronounResolution( + pronoun="He", resolved_to="Bob", + evidence_source="discourse_prior_subjects", + kind="wrong_kind", # type: ignore[arg-type] + ) + + +# --------------------------------------------------------------------------- +# 3. Closed-set constants +# --------------------------------------------------------------------------- + + +class TestADR0174Phase3Constants: + def test_pronoun_resolution_in_valid_kinds(self) -> None: + assert "pronoun_resolution" in VALID_REFINEMENT_KINDS + + def test_actor_pronoun_in_valid_unresolved_slots(self) -> None: + assert "actor_pronoun" in VALID_UNRESOLVED_SLOTS + + +# --------------------------------------------------------------------------- +# 4. End-to-end integration via parse_and_solve +# --------------------------------------------------------------------------- + + +class TestPhase3WiringEndToEnd: + """Synthetic problems that exercise the Phase 3 wiring in + math_candidate_graph. These do NOT correspond to any train_sample + case (the train_sample cases refuse for other narrowness reasons + before reaching the pronoun-resolution path — see PHASE-3.1 + follow-up brief).""" + + def test_resolved_pronoun_emits_admitted_trace_event(self) -> None: + from generate.math_candidate_graph import parse_and_solve + # 'He collected N Pokemon cards' — regex returns 0 choices + # (multi-word unit + acquisition verb), recognizer matches + # discrete_count_statement and the pronoun marker fires. + # 'Bob' is the discourse antecedent. + text = ( + "Bob has 10 Pokemon cards. " + "He collected 5 Pokemon cards. " + "How many Pokemon cards does Bob have?" + ) + r = parse_and_solve(text) + # The downstream question regex doesn't admit this exact + # phrasing — but our wiring should fire and produce a lookback + # admitted event on the second sentence. + lookback_events = [ + json.loads(ev) for ev in r.reader_trace + if json.loads(ev).get("layer") == "lookback" + ] + assert any( + ev.get("outcome") == "admitted" + and ev.get("pronoun") == "He" + and ev.get("resolved_to") == "Bob" + for ev in lookback_events + ), f"expected lookback admitted event; trace={lookback_events}" + + def test_no_antecedent_emits_no_antecedent_trace_event(self) -> None: + from generate.math_candidate_graph import parse_and_solve + # No proper-noun antecedent before the held pronoun sentence. + text = ( + "He collected 5 Pokemon cards. " + "How many Pokemon cards does he have?" + ) + r = parse_and_solve(text) + lookback_events = [ + json.loads(ev) for ev in r.reader_trace + if json.loads(ev).get("layer") == "lookback" + ] + # Either we emitted a no_antecedent event, OR the sentence + # refused before reaching the lookback path. Both preserve + # wrong=0; the assertion captures the intent. + no_antecedent = any( + ev.get("outcome") == "no_antecedent" + for ev in lookback_events + ) + # The problem should refuse cleanly regardless. + assert r.refusal_reason is not None or r.answer is None + # If lookback fired at all, it must have been no_antecedent. + for ev in lookback_events: + assert ev.get("outcome") in ("no_antecedent", "eliminated"), ( + f"unexpected lookback outcome on no-antecedent input: {ev}" + ) + + +# --------------------------------------------------------------------------- +# 5. wrong=0 preservation on train_sample/v1 +# --------------------------------------------------------------------------- + + +class TestWrongZeroPreservation: + def test_train_sample_score_unchanged(self) -> None: + """Phase 3 substrate must not move the train_sample score from + 3/47/0. Any change would indicate the lookback path is firing + on cases it shouldn't (or breaking cases it shouldn't).""" + import json + from pathlib import Path + from evals.gsm8k_math.train_sample.v1.runner import ( + build_report, _CASES_PATH, + ) + cases = [ + json.loads(line) + for line in Path(_CASES_PATH).open(encoding="utf-8") + if line.strip() + ] + report = build_report(cases, use_reader=True) + counts = report["counts"] + assert counts["wrong"] == 0, ( + f"wrong=0 invariant violated: {counts}" + ) + assert counts["correct"] == 3, ( + f"correct count moved from 3 to {counts['correct']}; " + "Phase 3a substrate should not lift score on this corpus " + "(see PHASE-3.1 follow-up brief for what would lift it)" + ) + assert counts["refused"] == 47, ( + f"refused count moved from 47 to {counts['refused']}" + ) From 619cd622276658d604794cdf8df717836e424132 Mon Sep 17 00:00:00 2001 From: Shay Date: Thu, 28 May 2026 09:18:29 -0700 Subject: [PATCH 2/2] fix(adr-0174-phase3a): multi-actor pronoun hazard defense + test backfills + ADR amendment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All findings from the 2026-05-28 Phase 1-3a lookback review addressed in one commit on the Phase 3a branch: Wrong=0 hazard defense (the load-bearing fix): - generate/math_candidate_graph.py: Phase 3a wiring now collects the set of distinct proper-noun subjects seen in prior context. When more than one exists, refuses with no_antecedent_ambiguous trace event rather than guessing the most-recent (which was gender-blind single-binding — wrong attribution in multi-actor problems). - Refusals from the statement loop now preserve _statement_trace via reader_trace in CandidateGraphResult (pre-existing latent issue: Phase 2/3 trace events were dropped on early statement refusal). - New tests assert: ambiguous case refuses with correct trace; single- actor case still resolves normally. Test coverage backfills (closes the 13 untested predicate-name gaps): - TestCheckConstraintsInitialPredicateNames — 3 tests asserting the exact predicate name on initial.value_grounds / initial.unit_grounds / initial.entity_grounds failure paths. - TestCheckConstraintsOperationPredicateNames — 3 tests asserting operation.verb_grounds / operation.value_grounds / operation.unit_grounds failure-predicate-name parity. - TestCheckConstraintsComposedInitialPath — 4 tests for the RAT-1 composed_initial path which was entirely untested in Phase 2 (parity manually verified during lookback review; now automated). ADR amendment (honest doc vs impl drift): - docs/decisions/ADR-0174-held-hypothesis-comprehension.md: appended 'Implementation Notes' section documenting: - reevaluate signature differs from spec text (shipped is more composable; treat as amended) - Phase 2 wires per-candidate, not per-token (per-token is Phase 5) - Lookback recompute is candidate-level, not token-level - Hypothesis.constraint_state is never populated by Phase 2 - Multi-actor pronoun hazard defense rationale - Honest LOC accounting: Phases 1-3a net +1,500 lines (Phase 5 delivers the projected net removal) - Test coverage backfill summary Cosmetic: - lookback.py:297 unreachable raise — added # type: ignore[unreachable] with comment explaining defensive future-proofing for Phase 3b. Acceptance verified: - 124/124 Phase 1+2+3a + reader tests pass (was 95/95 before backfills) - Smoke 67/67, packs 141/141 - train_sample 3/47/0 preserved (wrong=0 invariant held) - Multi-actor hazard live-tested: parse_and_solve refuses the Alice/Bob/She case with no_antecedent_ambiguous trace event See CLAUDE.md §Lookback Review Discipline and memory feedback-lookback-review-discipline for the doctrine that surfaced all of these issues at the right time. --- .../ADR-0174-held-hypothesis-comprehension.md | 127 ++++++++++++ generate/comprehension/lookback.py | 6 +- generate/math_candidate_graph.py | 44 +++++ ..._adr_0174_phase2_constraint_propagation.py | 182 ++++++++++++++++++ tests/test_adr_0174_phase3_lookback.py | 83 ++++++-- 5 files changed, 429 insertions(+), 13 deletions(-) diff --git a/docs/decisions/ADR-0174-held-hypothesis-comprehension.md b/docs/decisions/ADR-0174-held-hypothesis-comprehension.md index 763a235c..88a5270e 100644 --- a/docs/decisions/ADR-0174-held-hypothesis-comprehension.md +++ b/docs/decisions/ADR-0174-held-hypothesis-comprehension.md @@ -292,3 +292,130 @@ This ADR moves to **Accepted** when: - **Thesis anchor**: [[thesis-decoding-not-generating]] — every change in this ADR must pass the "teach the engine to find, not store another found thing" gate. - **HITL corridor preserved**: ADR-0150 (contemplation), ADR-0152 (proposal), ADR-0155 (review), ADR-0161 (HITL queue), ADR-0172 (math-corpus decomposition). - **Anti-overfitting obligations**: ADR-0114a — held-hypothesis reads are evaluated against the same obligations as the existing pipeline; perturbation, OOD ratio, depth curve, and adversarial axes all apply. + +--- + +## Implementation Notes (added 2026-05-28 after Phase 1-3a lookback review) + +The 2026-05-28 lookback review (per CLAUDE.md §Lookback Review Discipline) +surfaced drift between this ADR's spec text and what Phases 1-3a +actually shipped. Documenting honestly here so Phase 4-5 implementers +work from accurate ground truth. + +### `reevaluate` signature — implementation differs from spec + +**ADR text (§Decision §2):** +```text +reevaluate(hypotheses, new_token, position) -> (refined_hypotheses, eliminations) +``` + +**As shipped (Phase 3a, PR #423):** +```python +reevaluate(hypothesis: Hypothesis, refinement: Refinement) -> ReevaluateResult +``` + +Differences and rationale: + +- Single hypothesis + refinement object, not hypothesis set + token+position. + More composable: refinement objects (`PronounResolution` etc.) are + reusable; the caller decides which hypotheses to apply to which + refinements. +- Returns a single `ReevaluateResult` (refined-or-None plus + bookkeeping), not a (refined_hypotheses, eliminations) tuple. + Bulk-eliminate semantics belong on a higher-level orchestrator (not + yet built — Phase 4 work). + +The shipped design is preferred and the ADR text should be considered +amended. Phase 4 implementers should follow the shipped signature. + +### Phase 2 is per-candidate, not per-token + +**ADR text (§Decision §3):** +> Move them inside the reader so they fire per-token: After every EMIT, +> run the in-flight constraint check against the partial hypothesis. + +**As shipped (Phase 2, PR #420):** +Constraint propagation runs at the `math_candidate_graph` recognizer- +injection site (per-candidate, after `inject_from_match` returns), not +inside `lifecycle.apply_word` (per-token, during reading). + +This is a real substrate-vs-active-wiring gap. The check_constraints +primitive is available; the per-token integration is Phase 5 work +(legacy parser removal + apply_word refactor). Phase 2 is more +honestly described as "constraint propagation substrate ready for +per-token wiring in Phase 5." + +### Lookback recompute scope is candidate-level, not token-level + +**ADR text (§Decision §2):** +> For each hypothesis, recomputes the category assignment of any *prior* +> token whose role depended on the now-resolved ambiguity. + +**As shipped (Phase 3a, PR #423):** +`PronounResolution` appends one `(0, "pronoun_resolved", pronoun)` +entry to `Hypothesis.category_assignments` and rewrites the candidate's +semantic actor field. It does not walk back through per-token +assignments because Phase 1-3a's `category_assignments` is +candidate-level, not token-level. Per-token category assignment +becomes meaningful in Phase 5 (apply_word refactor). + +Phase 3b will widen this when compound-clause refinement enters +(multiple per-clause assignments do need recompute walks). + +### Hypothesis.constraint_state is never populated by Phase 2 + +The Phase 1 substrate carries `Hypothesis.constraint_state: tuple[tuple[str, str], ...]` +for recording predicate outcomes. Phase 2's `check_constraints` +populates `ConstraintResult.predicates_run` but does NOT copy that +into `Hypothesis.constraint_state` on the survivors. Survivors carry +forward their original (empty) constraint_state. + +Phase 4 (in-loop contemplation) may want to consult +`constraint_state` to decide which evidence to seek. If so, Phase 4 +must wire the population step explicitly. Not a Phase 2 defect — it's +a Phase 2 scope limit. + +### Multi-actor pronoun wrong=0 hazard defense (Phase 3a follow-up) + +The 2026-05-28 review surfaced a real wrong=0 hazard in Phase 3a's +`PronounResolution` wiring: the `_discourse_prior_subjects` lookup is +gender-blind and stores only most-recent-prior subject. In multi-actor +problems ("Alice has 5. Bob has 3. She buys 2."), this could resolve +"She" to "Bob" and produce wrong attribution. + +Fix landed in the same Phase 3a PR (PR #423): defensive +`no_antecedent_ambiguous` refusal when more than one distinct +proper-noun subject appears in prior context. Refusal-preferring +discipline preserves `wrong = 0`. + +This is the prototype for refinement-quality gating that Phase 4 (in- +loop contemplation) inherits: ambiguity that resolution cannot +disambiguate is a refusal, not a guess. + +### LOC accounting + +The ADR §"What this collapses" projects "Net ~1,900 lines removed" +once Phase 5 retires the legacy parser. Honest current state: + +- Phase 1 added ~243 lines (`state.py`) +- Phase 2 added ~726 lines (`constraint_propagation.py` new module) + + ~50 lines `math_candidate_graph.py` wiring +- Phase 3a added ~387 lines (`lookback.py` new module) + ~125 lines + `math_candidate_graph.py` wiring + ~15 lines `recognizer_match.py` + +**Phases 1-3a net: +1,500 lines added.** Phase 5 will remove +math_parser.py (~1,100 lines) + per-category dispatch (~400 lines) + +duplicate per-sentence-choice scaffolding (~300 lines) to reach the +projected net removal. + +The substrate is correctly load-bearing; the line-count payoff is in +Phase 5. + +### Test coverage backfill + +The lookback review found 13 of 17 `VALID_PREDICATE_NAMES` lacked +direct predicate-name assertions in tests, and all 4 +`_check_composed_initial` sub-checks were untested (parity verified +manually). Backfill landed in the same Phase 3a PR (10 new tests). + +--- diff --git a/generate/comprehension/lookback.py b/generate/comprehension/lookback.py index 4dffe3d4..87a0cbc6 100644 --- a/generate/comprehension/lookback.py +++ b/generate/comprehension/lookback.py @@ -292,9 +292,13 @@ def reevaluate( hypothesis so trace serialisation can record the before/after pair. """ # Dispatch on refinement kind. Phase 3a knows pronoun_resolution. + # The defensive raise below is unreachable today (the Refinement + # Union has one member), but it is correct future-proofing for + # Phase 3b's CompoundClauseExpansion and other refinement types — + # if a caller passes a non-Union type by accident, fail loudly. if isinstance(refinement, PronounResolution): return _apply_pronoun_resolution(hypothesis, refinement) - raise ComprehensionStateError( + raise ComprehensionStateError( # type: ignore[unreachable] f"reevaluate: unsupported refinement type {type(refinement).__name__}" ) diff --git a/generate/math_candidate_graph.py b/generate/math_candidate_graph.py index f06c2760..02ba546d 100644 --- a/generate/math_candidate_graph.py +++ b/generate/math_candidate_graph.py @@ -876,6 +876,29 @@ def parse_and_solve( break _antecedent = _effective_prior + # ADR-0174 Phase 3a — multi-actor pronoun + # ambiguity defense. When the problem has + # more than one distinct proper-noun subject + # in prior context, picking the most-recent + # is a guess (e.g. "Alice has 5. Bob has 3. + # She buys 2." — "She" should bind to Alice + # by gender but the discourse map returns + # Bob). No safety net downstream would catch + # a wrong attribution (single-binding + # emission → no multi-branch disagreement; + # verifier re-derives the same wrong graph). + # Refuse rather than guess. Surfaced by + # 2026-05-28 Phase 1-3a lookback review; + # see project-adr-0174-multi-actor-pronoun-hazard + # memory and CLAUDE.md §Lookback Review + # Discipline. + _distinct_priors = { + v for v in _discourse_prior_subjects.values() + if v is not None + } + if _prior_subject is not None: + _distinct_priors.add(_prior_subject) + _multi_actor_ambiguous = len(_distinct_priors) > 1 if _held_pronoun is None or not _antecedent: # No resolution path available — drop the # held candidates and log the lookback @@ -888,6 +911,21 @@ def parse_and_solve( "sentence_index": s_idx, }, sort_keys=True)) injected = () + elif _multi_actor_ambiguous: + # Refusal-preferring discipline: multiple + # distinct proper-noun subjects in prior + # context means the resolver would be + # guessing. Drop the held candidates, + # log the ambiguous-antecedent event. + _statement_trace.append(json.dumps({ + "layer": "lookback", + "phase": 3, + "outcome": "no_antecedent_ambiguous", + "pronoun": _held_pronoun, + "candidate_antecedents": sorted(_distinct_priors), + "sentence_index": s_idx, + }, sort_keys=True)) + injected = () else: _refinement = PronounResolution( pronoun=_held_pronoun, @@ -1013,11 +1051,17 @@ def parse_and_solve( f"(category={recognizer_match.category.value})" ), branches_enumerated=0, branches_admissible=0, + # ADR-0174 Phase 3a — preserve statement-stage + # trace events on early refusal so consumers see + # WHY admission failed (lookback no_antecedent, + # constraint_propagation eliminations, etc.). + reader_trace=tuple(_statement_trace), ) return CandidateGraphResult( answer=None, selected_graph=None, refusal_reason=f"no admissible candidate for statement: {s!r}", branches_enumerated=0, branches_admissible=0, + reader_trace=tuple(_statement_trace), ) per_sentence_choices.append(_collapse_per_sentence_ties(choices)) # ME-2 — update prior_subject AFTER this sentence is processed. diff --git a/tests/test_adr_0174_phase2_constraint_propagation.py b/tests/test_adr_0174_phase2_constraint_propagation.py index b091c383..8cddad4d 100644 --- a/tests/test_adr_0174_phase2_constraint_propagation.py +++ b/tests/test_adr_0174_phase2_constraint_propagation.py @@ -248,6 +248,188 @@ class TestCheckConstraintsOperationParity: assert roundtrip_admissible(op) is False +class TestCheckConstraintsInitialPredicateNames: + """Predicate-name assertions for every initial.* failure path. + Surfaced by 2026-05-28 lookback review — 13 of 17 predicates in + VALID_PREDICATE_NAMES lacked direct elimination-reason assertions.""" + + def test_initial_value_grounds_predicate_name(self) -> None: + ic = _initial(matched_value_token="99") # source has "3" + result = check_constraints(hypothesis_from_initial(ic, 0)) + assert result.admitted is False + first_fail = next( + (p for p, o in result.predicates_run if o == "fail"), None + ) + assert first_fail == "initial.value_grounds" + + def test_initial_unit_grounds_predicate_name(self) -> None: + ic = _initial(matched_unit_token="oranges") # source has "apples" + result = check_constraints(hypothesis_from_initial(ic, 0)) + assert result.admitted is False + first_fail = next( + (p for p, o in result.predicates_run if o == "fail"), None + ) + assert first_fail == "initial.unit_grounds" + + def test_initial_entity_grounds_predicate_name(self) -> None: + ic = _initial(matched_entity_token="Tom") # source has "Sam" + result = check_constraints(hypothesis_from_initial(ic, 0)) + assert result.admitted is False + first_fail = next( + (p for p, o in result.predicates_run if o == "fail"), None + ) + assert first_fail == "initial.entity_grounds" + + +class TestCheckConstraintsOperationPredicateNames: + """Predicate-name assertions for every operation.* failure path.""" + + def test_operation_verb_grounds_predicate_name(self) -> None: + # Verb registered for kind but not in source span — distinct + # from verb_registered failure. + op = CandidateOperation( + op=Operation(actor="Sam", kind="add", + operand=Quantity(value=5, unit="apples")), + source_span="Sam now owns 5 apples.", # 'buys' not in source + matched_verb="buys", + matched_value_token="5", + matched_unit_token="apples", + matched_actor_token="Sam", + ) + result = check_constraints(hypothesis_from_operation(op, 0)) + assert result.admitted is False + first_fail = next( + (p for p, o in result.predicates_run if o == "fail"), None + ) + assert first_fail == "operation.verb_grounds" + + def test_operation_value_grounds_predicate_name(self) -> None: + op = _operation_add( + matched_value_token="99", # source has "5" + source_span="Sam buys 5 apples.", + ) + result = check_constraints(hypothesis_from_operation(op, 0)) + assert result.admitted is False + first_fail = next( + (p for p, o in result.predicates_run if o == "fail"), None + ) + assert first_fail == "operation.value_grounds" + + def test_operation_unit_grounds_predicate_name(self) -> None: + op = _operation_add( + matched_unit_token="oranges", # source has "apples" + source_span="Sam buys 5 apples.", + ) + result = check_constraints(hypothesis_from_operation(op, 0)) + assert result.admitted is False + first_fail = next( + (p for p, o in result.predicates_run if o == "fail"), None + ) + assert first_fail == "operation.unit_grounds" + + +class TestCheckConstraintsComposedInitialPath: + """The RAT-1 composed_initial path was completely untested before + 2026-05-28 lookback review — parity was manually verified but no + automated test asserted the 4 sub-checks fire correctly.""" + + def _composed(self, **ev_overrides: str) -> CandidateInitial: + from generate.math_problem_graph import InitialPossession, Quantity + ev: dict[str, str] = { + "composition_shape": "bound(count) × bound(unit_cost)", + "input_tokens": "3|400", + "entity_source": "prior_sentence", + "currency_symbol": "$", + } + ev.update(ev_overrides) + return CandidateInitial( + initial=InitialPossession( + entity="John", quantity=Quantity(value=1200, unit="dollars"), + ), + source_span="3 vet appointments at $400 each", + matched_anchor="has", + matched_value_token="1200", + matched_unit_token="dollars", + matched_entity_token="John", + composition_evidence=ev, + ) + + def test_well_formed_composed_initial_admits(self) -> None: + ic = self._composed() + result = check_constraints(hypothesis_from_initial(ic, 0)) + assert result.admitted is True + # All 4 sub-checks run (some may skip). + predicate_names = {p for p, _ in result.predicates_run} + assert "composed_initial.evidence_complete" in predicate_names + assert "composed_initial.input_tokens_ground" in predicate_names + assert "composed_initial.entity_token_present" in predicate_names + + def test_composed_initial_missing_evidence_key_eliminated(self) -> None: + # Construct via dict mutation since the field is a Mapping + ev_partial = { + "composition_shape": "shape", + "input_tokens": "3|400", + # entity_source missing + } + ic = self._composed() + # Replace composition_evidence via dataclasses.replace would + # break frozen; build a new candidate directly. + from generate.math_problem_graph import InitialPossession, Quantity + ic2 = CandidateInitial( + initial=InitialPossession( + entity="John", quantity=Quantity(value=1200, unit="dollars"), + ), + source_span=ic.source_span, + matched_anchor=ic.matched_anchor, + matched_value_token=ic.matched_value_token, + matched_unit_token=ic.matched_unit_token, + matched_entity_token=ic.matched_entity_token, + composition_evidence=ev_partial, + ) + result = check_constraints(hypothesis_from_initial(ic2, 0)) + assert result.admitted is False + first_fail = next( + (p for p, o in result.predicates_run if o == "fail"), None + ) + assert first_fail == "composed_initial.evidence_complete" + + def test_composed_initial_input_token_missing_eliminated(self) -> None: + ic = self._composed(input_tokens="999|400") # 999 not in source + result = check_constraints(hypothesis_from_initial(ic, 0)) + assert result.admitted is False + first_fail = next( + (p for p, o in result.predicates_run if o == "fail"), None + ) + assert first_fail == "composed_initial.input_tokens_ground" + + def test_composed_initial_currency_symbol_missing_eliminated(self) -> None: + # Build a candidate whose source span has no $ but evidence + # claims currency_symbol="$". + from generate.math_problem_graph import InitialPossession, Quantity + ic = CandidateInitial( + initial=InitialPossession( + entity="John", quantity=Quantity(value=1200, unit="dollars"), + ), + source_span="3 vet appointments at 400 each", # no $ + matched_anchor="has", + matched_value_token="1200", + matched_unit_token="dollars", + matched_entity_token="John", + composition_evidence={ + "composition_shape": "shape", + "input_tokens": "3|400", + "entity_source": "prior_sentence", + "currency_symbol": "$", + }, + ) + result = check_constraints(hypothesis_from_initial(ic, 0)) + assert result.admitted is False + first_fail = next( + (p for p, o in result.predicates_run if o == "fail"), None + ) + assert first_fail == "composed_initial.currency_symbol_present" + + class TestCheckConstraintsResultShape: def test_predicates_run_only_uses_known_predicate_names(self) -> None: ic = _initial() diff --git a/tests/test_adr_0174_phase3_lookback.py b/tests/test_adr_0174_phase3_lookback.py index 8494d05a..108f1d9f 100644 --- a/tests/test_adr_0174_phase3_lookback.py +++ b/tests/test_adr_0174_phase3_lookback.py @@ -347,6 +347,74 @@ class TestPhase3WiringEndToEnd: for ev in lookback_events ), f"expected lookback admitted event; trace={lookback_events}" + def test_multi_actor_ambiguous_refuses_with_no_antecedent_ambiguous(self) -> None: + """ADR-0174 Phase 3a wrong=0 hazard defense — surfaced by + 2026-05-28 lookback review. + + When a problem has more than one distinct proper-noun subject + in prior context, the _discourse_prior_subjects lookup is + gender-blind and would silently pick the most-recent-prior as + the antecedent. In 'Alice has 5. Bob has 3. She buys 2.', + this would resolve 'She' to 'Bob' and attribute Alice's + purchase to Bob — wrong attribution with no downstream safety + net. + + Defense: refuse with no_antecedent_ambiguous trace event when + multiple distinct proper-noun subjects appear in prior + context. Refusal-preferring discipline preserves wrong=0. + """ + from generate.math_candidate_graph import parse_and_solve + text = ( + "Alice has 5 Pokemon cards. " + "Bob has 3 Pokemon cards. " + "She collected 2 Pokemon cards. " + "How many Pokemon cards does Bob have?" + ) + r = parse_and_solve(text) + # MUST refuse — wrong attribution is the hazard. + assert r.answer is None + lookback_events = [ + json.loads(ev) for ev in r.reader_trace + if json.loads(ev).get("layer") == "lookback" + ] + ambig = [ + ev for ev in lookback_events + if ev.get("outcome") == "no_antecedent_ambiguous" + ] + assert ambig, ( + f"expected no_antecedent_ambiguous event; trace={lookback_events}" + ) + ev = ambig[0] + assert "Alice" in ev["candidate_antecedents"] + assert "Bob" in ev["candidate_antecedents"] + assert ev["pronoun"] == "She" + + def test_single_actor_pronoun_still_resolves(self) -> None: + """Counter-test: when there's only ONE distinct prior subject, + the defense MUST NOT fire — pronoun resolution proceeds.""" + from generate.math_candidate_graph import parse_and_solve + text = ( + "Bob has 10 Pokemon cards. " + "He collected 5 Pokemon cards. " + "How many Pokemon cards does Bob have?" + ) + r = parse_and_solve(text) + lookback_events = [ + json.loads(ev) for ev in r.reader_trace + if json.loads(ev).get("layer") == "lookback" + ] + assert not any( + ev.get("outcome") == "no_antecedent_ambiguous" + for ev in lookback_events + ), ( + f"single-actor case must not trigger ambiguity defense; " + f"trace={lookback_events}" + ) + assert any( + ev.get("outcome") == "admitted" and ev.get("resolved_to") == "Bob" + for ev in lookback_events + ) + def test_no_antecedent_emits_no_antecedent_trace_event(self) -> None: from generate.math_candidate_graph import parse_and_solve # No proper-noun antecedent before the held pronoun sentence. @@ -359,20 +427,11 @@ class TestPhase3WiringEndToEnd: json.loads(ev) for ev in r.reader_trace if json.loads(ev).get("layer") == "lookback" ] - # Either we emitted a no_antecedent event, OR the sentence - # refused before reaching the lookback path. Both preserve - # wrong=0; the assertion captures the intent. - no_antecedent = any( - ev.get("outcome") == "no_antecedent" - for ev in lookback_events - ) - # The problem should refuse cleanly regardless. assert r.refusal_reason is not None or r.answer is None - # If lookback fired at all, it must have been no_antecedent. for ev in lookback_events: - assert ev.get("outcome") in ("no_antecedent", "eliminated"), ( - f"unexpected lookback outcome on no-antecedent input: {ev}" - ) + assert ev.get("outcome") in ( + "no_antecedent", "no_antecedent_ambiguous", "eliminated" + ), f"unexpected lookback outcome on no-antecedent input: {ev}" # ---------------------------------------------------------------------------