fix(wrong=0): refuse on recognized-but-uninjectable statements + audit taxonomy + 2 surfaced regressions (#359)
## 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
This commit is contained in:
parent
a7d1547fbb
commit
97b0ee0e13
5 changed files with 142 additions and 8 deletions
|
|
@ -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}",
|
||||
|
|
|
|||
|
|
@ -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]
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
81
tests/test_recognizer_skip_wrong_zero.py
Normal file
81
tests/test_recognizer_skip_wrong_zero.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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:<chain_id>`` — ratified curriculum supersession
|
||||
# - ``unsupported_intent:<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:
|
||||
|
|
|
|||
Loading…
Reference in a new issue