fix(adr-0174-phase3a): multi-actor pronoun hazard defense + test backfills + ADR amendment

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.
This commit is contained in:
Shay 2026-05-28 09:18:29 -07:00
parent 5d1f1001f4
commit 619cd62227
5 changed files with 429 additions and 13 deletions

View file

@ -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).
---

View file

@ -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__}"
)

View file

@ -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.

View file

@ -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()

View file

@ -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}"
# ---------------------------------------------------------------------------