From 97b0ee0e13294457265497cac00e860bd8de772b Mon Sep 17 00:00:00 2001 From: Shay Date: Wed, 27 May 2026 07:42:54 -0700 Subject: [PATCH] fix(wrong=0): refuse on recognized-but-uninjectable statements + audit taxonomy + 2 surfaced regressions (#359) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Two test failures on origin/main both trace to PR #315 (ADR-0163.D.2 — discrete_count_statement recognizer + admissibility-intent chain). Earlier runs treated them as "pre-existing unrelated" — they are not unrelated. The first is a real wrong>0 hazard. ## Failure 1: silent admission via recognized-but-uninjectable statement The ratified `discrete_count_statement` recognizer over-matches: ANY sentence containing a number + noun resolves it, irrespective of the verb. When `inject_from_match` returns `()` (the round-2 default for v1 categories without an injector), the old code path used `continue` to silently drop the statement — and the solver then answered from whatever initial state remained. Reproduction: parse_and_solve("Sam has 5 apples. Sam contemplates 3 apples. " "How many apples does Sam have?") → is_admitted=True, answer=5.0 (silent admission of partial graph) This is exactly the case-0050-class hazard wearing a different hat (silently admitting an incomplete graph at the problem level). ADR-0167 / Brief 11 §"correct-count greed" established the principle on the reader path; this commit extends it to the recognizer path. Fix: when a recognizer matches but produces no injection, REFUSE. generate/math_candidate_graph.py: - Replaced the skip-only `continue` with a CandidateGraphResult refusal carrying the recognizer category in the reason. tests/test_math_candidate_graph.py: - test_unparseable_statement now accepts either the legacy "no admissible candidate" reason or the new "recognizer matched but produced no injection" reason. Both legitimately refuse; what matters is is_admitted=False. tests/test_recognizer_skip_wrong_zero.py (NEW): - 5 regression tests pinning the wrong=0 invariant: * 3 parametrized verbs unknown to both regex parser and reader (contemplates / ponders / memorises) — must all refuse * Nonsense token — must refuse * Anti-regression: known initial + known operation still admits ## Failure 2: cognition audit drop-reason taxonomy The audit test hardcoded `dropped.reason.startswith("superseded_by:")` as the only valid drop-reason prefix. Commit da70919 (ADR-0163.D.2) ratified an admissibility-intent chain that the audit categorizes with reason `unsupported_intent:admissibility`, which fails this assertion. Fix: tests/test_teaching_audit.py — expand the allowed-prefix set to include `unsupported_intent:` with a written rationale. Future drop classes extend the allowlist deliberately rather than silently broadening the assertion to any non-empty reason. ## Surfaced regression: partition-test allowlist (ADR-0167 FOLLOWUPS §2) This PR modifies three test files that the test_existing_cognition_tests_untouched assertion would reject under its named-allowlist scheme. Added the three test paths to the allowlist as the tactical fix; the architectural fix (retire / move to CI / move to CODEOWNERS) is queued in docs/handoff/ADR-0167-FOLLOWUPS.md §2. ## Test plan uv run pytest tests/test_recognizer_skip_wrong_zero.py \ tests/test_math_candidate_graph.py \ tests/test_teaching_audit.py \ tests/test_candidate_domain_partition.py \ tests/test_math_evidence_e2e.py \ tests/test_math_evidence_schema.py \ tests/test_math_contemplation_adapter.py \ tests/test_math_claim_signature.py \ tests/test_math_lexical_ratification.py \ tests/test_brief_11b_audit_artifact.py \ tests/test_brief_11b_step2_lexicon.py \ tests/test_brief_11_audit.py → 152 passed ## Hard invariants - wrong == 0 — restored on the recognizer path (was silently violated on main) - ADR-0166 — no new eval lanes - No teaching-store mutation, no pack mutation - The reader path was already correct (it refused these cases); this fix brings the regex/recognizer path back in line --- generate/math_candidate_graph.py | 32 ++++++++-- tests/test_candidate_domain_partition.py | 7 ++ tests/test_math_candidate_graph.py | 13 +++- tests/test_recognizer_skip_wrong_zero.py | 81 ++++++++++++++++++++++++ tests/test_teaching_audit.py | 17 ++++- 5 files changed, 142 insertions(+), 8 deletions(-) create mode 100644 tests/test_recognizer_skip_wrong_zero.py diff --git a/generate/math_candidate_graph.py b/generate/math_candidate_graph.py index 37f616e9..2879acef 100644 --- a/generate/math_candidate_graph.py +++ b/generate/math_candidate_graph.py @@ -735,11 +735,33 @@ def parse_and_solve( _collapse_per_sentence_ties(admitted) ) continue - # Recognized but no injection — skip the sentence, do - # not refuse. Identical to the round-2 skip-only - # wiring; preserves wrong=0 because zero math state - # is contributed. - continue + # Recognized but no injection — REFUSE. + # + # The earlier "skip-only" reasoning ("zero math state + # contributed → wrong=0 preserved by construction") is + # wrong in the same way the case 0050 hazard was wrong: + # silently dropping a recognized math statement is + # equivalent to admitting an incomplete graph at the + # problem level — the solver answers from whatever + # remains, which is not the right answer to the input + # problem. ADR-0167 / Brief 11 §"correct-count greed" + # established this principle on the reader path; this + # commit extends it to the recognizer path. + # + # If the recognizer matches but the injector cannot + # produce typed solver state, the right answer is + # "I don't know" — i.e. refuse. When an injector is + # added that handles this shape, this branch becomes + # dead and can be retired. + return CandidateGraphResult( + answer=None, selected_graph=None, + refusal_reason=( + "recognizer matched but produced no injection " + f"for statement: {s!r} " + f"(category={recognizer_match.category.value})" + ), + branches_enumerated=0, branches_admissible=0, + ) return CandidateGraphResult( answer=None, selected_graph=None, refusal_reason=f"no admissible candidate for statement: {s!r}", diff --git a/tests/test_candidate_domain_partition.py b/tests/test_candidate_domain_partition.py index f73c85a9..b661ea14 100644 --- a/tests/test_candidate_domain_partition.py +++ b/tests/test_candidate_domain_partition.py @@ -89,6 +89,13 @@ def test_existing_cognition_tests_untouched(): allowed = { "test_candidate_domain_partition.py", # W2-C "test_math_evidence_e2e.py", # W3-A + # Fix PR — wrong=0 hazard regression (recognizer skip-only fallback). + # Modifies test_math_candidate_graph.py and test_teaching_audit.py; + # adds test_recognizer_skip_wrong_zero.py. See ADR-0167-FOLLOWUPS §2 + # for the architectural fix that would retire this allowlist. + "test_math_candidate_graph.py", + "test_teaching_audit.py", + "test_recognizer_skip_wrong_zero.py", } for line in lines: path = line.split()[-1] diff --git a/tests/test_math_candidate_graph.py b/tests/test_math_candidate_graph.py index 1fd9590f..a33b9828 100644 --- a/tests/test_math_candidate_graph.py +++ b/tests/test_math_candidate_graph.py @@ -184,13 +184,22 @@ class TestRefusals: assert "question" in (result.refusal_reason or "").lower() def test_unparseable_statement(self) -> None: - # Verb not in any permissive table. + # Verb not in any permissive table. Either the regex parser refuses + # directly ("no admissible candidate") or a ratified recognizer + # matches but cannot inject typed solver state ("recognizer matched + # but produced no injection") — both paths preserve wrong=0 by + # refusing. See the fix that retired the recognizer skip-only + # fallback (silent-drop was a wrong>0 hazard analogous to case 0050). result = parse_and_solve( "Sam has 5 apples. Sam contemplates 3 apples. " "How many apples does Sam have?" ) assert not result.is_admitted - assert "no admissible candidate" in (result.refusal_reason or "") + reason = result.refusal_reason or "" + assert ( + "no admissible candidate" in reason + or "recognizer matched but produced no injection" in reason + ), f"unexpected refusal reason: {reason!r}" def test_question_references_unknown_entity(self) -> None: result = parse_and_solve( diff --git a/tests/test_recognizer_skip_wrong_zero.py b/tests/test_recognizer_skip_wrong_zero.py new file mode 100644 index 00000000..56489ecb --- /dev/null +++ b/tests/test_recognizer_skip_wrong_zero.py @@ -0,0 +1,81 @@ +"""Regression: ratified-recognizer skip-only fallback was a wrong>0 hazard. + +A ratified ``discrete_count_statement`` recognizer (introduced 2026-05-26 +by ADR-0163.D.2 / PR #315) over-matches any sentence containing a +number + noun, irrespective of the verb. When the recognizer matched +but ``inject_from_match`` returned ``()``, the old code path silently +*dropped* the sentence and let the solver answer from whatever initial +state remained — exactly the case-0050-class hazard (silently admitting +a partial graph at the problem level). + +This regression pins the corrected behaviour: recognized-but-uninjected +statements REFUSE. When an injector lands that handles this shape, the +test continues to hold because the injector path returns choices and +the refusal branch is never reached. + +Reference: +* ``feedback-wrong-zero-hazard-case-0050`` (memory) +* ADR-0167 §"correct-count greed" / Brief 11 +* Original drop site: ``generate.math_candidate_graph`` recognizer branch + at the end of ``per_sentence_choices`` enumeration +""" + +from __future__ import annotations + +import pytest + +from generate.math_candidate_graph import parse_and_solve + + +@pytest.mark.parametrize( + "verb", + [ + # Real-English verbs the regex parser has no template for, but the + # sentences are number+noun-shaped — the historical silent-drop + # trigger. Vetted: each verb falls through the parser's templates + # AND is outside the comprehension reader's transactional verb + # categories. "donates" was considered and rejected for this list + # because it resolves to a depletion verb on the existing regex + # path. + "contemplates", + "ponders", + "memorises", + ], +) +def test_unparseable_verb_with_number_noun_refuses(verb: str) -> None: + """When the regex parser cannot match a number+noun statement and the + ratified recognizer matches but cannot inject typed solver state, + parse_and_solve MUST refuse — not silently drop the statement and + answer from whatever initial-state remains.""" + text = ( + f"Sam has 5 apples. Sam {verb} 3 apples. " + f"How many apples does Sam have?" + ) + result = parse_and_solve(text) + assert not result.is_admitted, ( + f"silent admission with verb={verb!r}: answer={result.answer!r}, " + f"graph={result.selected_graph!r} — this is the wrong=0 hazard " + f"the skip-only fallback re-introduced and this fix retired." + ) + + +def test_unparseable_nonsense_verb_refuses() -> None: + """Defence-in-depth: a token that is neither in the regex parser nor in + any natural-language lexicon must still refuse.""" + text = ( + "Sam has 5 apples. Sam ipsum-doloriks 3 apples. " + "How many apples does Sam have?" + ) + result = parse_and_solve(text) + assert not result.is_admitted + assert result.answer is None + + +def test_known_initial_then_known_operation_still_admits() -> None: + """Anti-regression: the fix must not break legitimate admissions where + both the initial-state sentence and the operation sentence have + verbs the parser knows.""" + text = "Sam has 5 apples. Sam gets 3 more apples. How many apples does Sam have?" + result = parse_and_solve(text) + assert result.is_admitted, f"legitimate problem refused: {result.refusal_reason!r}" + assert result.answer == 8 diff --git a/tests/test_teaching_audit.py b/tests/test_teaching_audit.py index c18a1822..0ff5ae1c 100644 --- a/tests/test_teaching_audit.py +++ b/tests/test_teaching_audit.py @@ -103,8 +103,23 @@ def test_audit_real_corpus_runs_clean() -> None: assert report.corpus_id == "cognition_chains_v1" # Every dropped line must carry a typed reason so an operator can # audit why it was retired without re-deriving the supersession. + # Allowed reason prefixes (extend deliberately when a new drop class + # is introduced — do not silently broaden): + # + # - ``superseded_by:`` — ratified curriculum supersession + # - ``unsupported_intent:`` — chain authored with an intent + # the cognition corpus does not yet support (e.g. admissibility + # chains from ADR-0163.D.2 discovery promotion); the chain remains + # on disk for replay/audit but is not loaded into the active + # corpus. + allowed_prefixes = ("superseded_by:", "unsupported_intent:") for dropped in report.dropped: - assert dropped.reason.startswith("superseded_by:") + assert dropped.reason.startswith(allowed_prefixes), ( + f"unrecognised drop reason {dropped.reason!r} for chain " + f"{dropped.chain_id!r}; either add a new prefix to the " + f"allowlist with a written rationale, or fix the drop site " + f"to use an existing one." + ) def test_audit_loaded_entries_have_typed_provenance() -> None: