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: