From 131e711054f4cc2d9de549970e53184baec541db Mon Sep 17 00:00:00 2001 From: Shay Date: Wed, 27 May 2026 13:43:16 -0700 Subject: [PATCH] =?UTF-8?q?feat(ADR-0172/tightening):=20three=20follow-ups?= =?UTF-8?q?=20=E2=80=94=20self-contained=20JSONL,=20widened=20dispatch,=20?= =?UTF-8?q?shape=5Fcategory=20gap=20(#386)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundles three post-Tier-1 follow-ups into one PR (no scope change, no new ADR — implementation tightening on the already-shipped corridor). (1) Standalone JSONL self-containment teaching/math_contemplation_proposal.py + to_jsonl_record() — emits proposal_id + full evidence_pointers (nested dicts including audit_row) + full reasoning_trace.steps + from_jsonl_record() — inverse; goes through build_proposal() so all invariants are re-validated; raises on proposal_id mismatch canonical_bytes() UNCHANGED (still the content-hash function; trace_id/proposal_id stability preserved) core/cli.py W3 lane now writes to_jsonl_record() output instead of canonical_bytes() — same compact-JSON encoding (sort_keys=True, ensure_ascii=False, separators=(",", ":")) workbench/readers.py loads via self-contained record fields directly; decompose_audit() re-run removed. read_math_proposal() now reads reasoning_trace.steps and evidence_pointers from the JSONL record. (2) Widened change_kind heuristic dispatch teaching/math_contemplation.py + _CHANGE_KIND_BY_PAIR table on (refusal_reason, missing_operator): (unexpected_category, pre_frame_filler_sentence) → matcher_extension (unexpected_category, multi_subject_sentence) → frame_reclassification (unexpected_category, fraction_percentage_literal) → matcher_extension (unexpected_category, descriptive_frame_question) → frame_reclassification (unresolved_pronoun, pronoun_resolution) → matcher_extension Single-key fallback (lexicon_entry/narrowness_violation/ frame_unrecognized) retained for completeness. hypothesis-step justification text updated to reflect new table. Result on audit_brief_11.json: 3 matcher_extension (was 0) 2 frame_reclassification (was 0) 3 injector_sub_shape (was 8) 0 vocabulary_addition (no unknown_word group ≥2 in train sample) (3) shape_category structural gap MathReaderRefusalEvidence does not carry shape_category, so the proposal cannot derive it. All proposals continue to emit ShapeCategory.UNCATEGORIZED with a structural-gap comment. No invented values — handler dispatch decision (per ADR-0167-FOLLOWUPS §1) drives ratification routing today, not shape_category. Tests + W1: 5 new tests (to_jsonl_record self-containment, round-trip, byte stability, proposal_id mismatch rejection, canonical_bytes unchanged invariant) + W2: 3 new pair-dispatch tests + real-audit change_kind distribution test + shape_category-uncategorized test + W3: 2 new tests (records are self-contained, round-trip via from_jsonl_record); existing byte-comparison test updated to use proposal_id ordering instead of canonical_bytes + W4: existing 6 tests updated to build JSONL via to_jsonl_record; + 1 new decoupling test that drops teaching.math_contemplation from sys.modules and verifies the workbench still loads + serves detail Verification - core eval math-contemplation produces the expected 3/2/3 distribution - core test --suite teaching -q → 33 passed - core test --suite runtime -q → 20 passed - All 57 ADR-0172 W1-W4 tests pass (49 existing + 8 new) Determinism / invariants preserved - canonical_bytes() byte-stable (test pins this) - to_jsonl_record() byte-stable via sort_keys=True + no floats - wrong=0 invariant: proposals stay evidence-only; no auto-apply - ChangeKind Literal unchanged (4 values; no new ones invented) --- core/cli.py | 17 +- teaching/math_contemplation.py | 53 ++++- teaching/math_contemplation_proposal.py | 177 ++++++++++++++- tests/test_adr_0172_w1_shape_proposal.py | 119 ++++++++++ tests/test_adr_0172_w2_decomposer.py | 120 ++++++++++ tests/test_adr_0172_w3_cli_lane.py | 56 ++++- tests/test_adr_0172_w4_workbench_e2e.py | 271 ++++++++++++----------- workbench/readers.py | 75 ++++--- 8 files changed, 700 insertions(+), 188 deletions(-) diff --git a/core/cli.py b/core/cli.py index 93cb9c07..8d7ddf59 100644 --- a/core/cli.py +++ b/core/cli.py @@ -2250,7 +2250,7 @@ def cmd_eval_math_contemplation(args: argparse.Namespace) -> int: ``teaching/math_proposals/`` is written, the audit file is not mutated. """ from teaching.math_contemplation import decompose_audit - from teaching.math_contemplation_proposal import canonical_bytes + from teaching.math_contemplation_proposal import to_jsonl_record audit_raw = getattr(args, "audit", None) output_raw = getattr(args, "output", None) @@ -2271,7 +2271,20 @@ def cmd_eval_math_contemplation(args: argparse.Namespace) -> int: output_path.parent.mkdir(parents=True, exist_ok=True) - lines = [canonical_bytes(p) + b"\n" for p in proposals] + # Self-contained JSONL (ADR-0172 tightening follow-up #1): each line + # carries proposal_id, full evidence_pointers, and full + # reasoning_trace.steps so consumers can load without re-running the + # decomposer. + lines: list[bytes] = [] + for proposal in proposals: + record = to_jsonl_record(proposal) + encoded = json.dumps( + record, + ensure_ascii=False, + sort_keys=True, + separators=(",", ":"), + ).encode("utf-8") + lines.append(encoded + b"\n") output_path.write_bytes(b"".join(lines)) if not getattr(args, "json", False): diff --git a/teaching/math_contemplation.py b/teaching/math_contemplation.py index b27715d5..a4035a14 100644 --- a/teaching/math_contemplation.py +++ b/teaching/math_contemplation.py @@ -125,6 +125,23 @@ def audit_problem_to_evidence( # --------------------------------------------------------------------------- +# Widened dispatch (ADR-0172 tightening follow-up #2): the original +# single-key heuristic (refusal_reason only) collapsed every audit_brief_11 +# group to injector_sub_shape because the reader's actual refusal_reasons +# (unexpected_category, unresolved_pronoun, …) never matched the legacy +# keys. The (refusal_reason × missing_operator) pair carries the +# information needed to route to the queued handlers from +# docs/handoff/ADR-0167-FOLLOWUPS.md §1. +_CHANGE_KIND_BY_PAIR: dict[tuple[str, str], str] = { + ("unexpected_category", "pre_frame_filler_sentence"): "matcher_extension", + ("unexpected_category", "multi_subject_sentence"): "frame_reclassification", + ("unexpected_category", "fraction_percentage_literal"): "matcher_extension", + ("unexpected_category", "descriptive_frame_question"): "frame_reclassification", + ("unresolved_pronoun", "pronoun_resolution"): "matcher_extension", +} + +# Single-key fallback retained for completeness — covers reader refusals +# that share a refusal_reason regardless of missing_operator. _CHANGE_KIND_BY_REFUSAL_REASON: dict[str, str] = { "lexicon_entry": "vocabulary_addition", "narrowness_violation": "matcher_extension", @@ -153,9 +170,19 @@ def _audit_row_from_case(case: dict) -> AuditRow: ) -def _change_kind_for_group(refusal_reason: str) -> str: - """Heuristic per ADR-0172 §"Six open questions" #1.""" +def _change_kind_for_group(refusal_reason: str, missing_operator: str) -> str: + """Dispatch on (refusal_reason, missing_operator) pair, then refusal_reason. + Per ADR-0172 tightening follow-up #2: the pair-based table covers the + GSM8K train-sample audit groups that route to queued handlers + (matcher_extension, frame_reclassification). The single-key fallback + preserves the original ADR-0172 §"Six open questions" #1 mapping for + reader refusals that share a refusal_reason regardless of operator. + """ + + paired = _CHANGE_KIND_BY_PAIR.get((refusal_reason, missing_operator)) + if paired is not None: + return paired return _CHANGE_KIND_BY_REFUSAL_REASON.get( refusal_reason, "injector_sub_shape" ) @@ -233,10 +260,16 @@ def _build_reasoning_trace( f"The structural change kind for this group is {change_kind!r}." ), justification=( - "Dispatched via the refusal_reason heuristic: lexicon_entry → " - "vocabulary_addition; narrowness_violation → matcher_extension; " - "frame_unrecognized → frame_reclassification; else → " - "injector_sub_shape." + "Dispatched via the (refusal_reason, missing_operator) pair table: " + "(unexpected_category, pre_frame_filler_sentence|" + "fraction_percentage_literal) → matcher_extension; " + "(unexpected_category, multi_subject_sentence|" + "descriptive_frame_question) → frame_reclassification; " + "(unresolved_pronoun, pronoun_resolution) → matcher_extension. " + "Refusal-reason fallback: lexicon_entry → vocabulary_addition; " + "narrowness_violation → matcher_extension; " + "frame_unrecognized → frame_reclassification; " + "default → injector_sub_shape." ), output_payload={"proposed_change_kind": change_kind}, ) @@ -270,7 +303,7 @@ def _build_proposal_for_group( ) -> MathReaderRefusalShapeProposal: """Assemble one :class:`MathReaderRefusalShapeProposal` for a group.""" - change_kind = _change_kind_for_group(refusal_reason) + change_kind = _change_kind_for_group(refusal_reason, missing_operator) payload = _modal_anchor_payload( refusal_reason=refusal_reason, missing_operator=missing_operator, @@ -301,6 +334,12 @@ def _build_proposal_for_group( "Proposal is evidence-only; ratification handler is the wrong=0 " "surface, not this proposal." ) + # Structural gap (ADR-0172 tightening follow-up #3): the evidence record + # carries `sub_type` but not `shape_category`, so the proposal cannot + # derive shape_category from the evidence today. All groups emit + # UNCATEGORIZED until a future wave adds shape_category to + # MathReaderRefusalEvidence (or the reader's ShapeCategory inference is + # plumbed through the audit row). Do NOT invent a shape_category here. return build_proposal( shape_category=ShapeCategory.UNCATEGORIZED, structural_commonality=structural_commonality, diff --git a/teaching/math_contemplation_proposal.py b/teaching/math_contemplation_proposal.py index ee127190..46786a62 100644 --- a/teaching/math_contemplation_proposal.py +++ b/teaching/math_contemplation_proposal.py @@ -26,13 +26,12 @@ from __future__ import annotations import hashlib import json from dataclasses import dataclass -from typing import TYPE_CHECKING, Any, Literal +from typing import Any, Literal from evals.refusal_taxonomy.shape_categories import ShapeCategory +from generate.comprehension.audit import AuditRow from teaching.math_evidence import MathReaderRefusalEvidence - -if TYPE_CHECKING: - from teaching.math_reasoning_trace import ReasoningTrace +from teaching.math_reasoning_trace import ReasoningStep, ReasoningTrace, build_trace # --------------------------------------------------------------------------- @@ -296,10 +295,180 @@ def build_proposal( ) +# --------------------------------------------------------------------------- +# Self-contained JSONL persistence serializer (ADR-0172 tightening follow-up #1) +# --------------------------------------------------------------------------- +# +# canonical_bytes() above is the content-hash function: it reduces +# evidence_pointers to evidence_hashes and reasoning_trace to its trace_id. +# That's the right shape for the proposal_id derivation but NOT for +# round-tripping through disk — a reader cannot reconstruct the full +# proposal from canonical bytes without re-running the decomposer. +# +# to_jsonl_record() / from_jsonl_record() are a SEPARATE persistence +# serializer that emits a self-contained record (proposal_id, full +# evidence_pointers, full reasoning_trace.steps) so the workbench can +# read proposals.jsonl without re-running decompose_audit(). +# +# Determinism contract preserved: sort_keys=True, compact separators, +# no floats (rejected by reasoning_trace validators). + + +def _audit_row_to_dict(row: AuditRow) -> dict[str, Any]: + return { + "case_id": row.case_id, + "sentence_index": row.sentence_index, + "token_index": row.token_index, + "token_text": row.token_text, + "recognized_terms": list(row.recognized_terms), + "skipped_frame": row.skipped_frame, + "missing_operator": row.missing_operator, + "refusal_reason": row.refusal_reason, + "refusal_detail": row.refusal_detail, + } + + +def _audit_row_from_dict(data: dict[str, Any]) -> AuditRow: + return AuditRow( + case_id=str(data["case_id"]), + sentence_index=int(data["sentence_index"]), + token_index=int(data["token_index"]), + token_text=str(data["token_text"]), + recognized_terms=tuple(data.get("recognized_terms") or ()), + skipped_frame=data.get("skipped_frame"), + missing_operator=data.get("missing_operator"), + refusal_reason=str(data.get("refusal_reason", "")), + refusal_detail=str(data.get("refusal_detail", "")), + ) + + +def _evidence_to_dict(ev: MathReaderRefusalEvidence) -> dict[str, Any]: + return { + "case_id": ev.case_id, + "sentence_index": ev.sentence_index, + "token_index": ev.token_index, + "refusal_reason": ev.refusal_reason, + "missing_operator": ev.missing_operator, + "claim_signature": ev.claim_signature, + "evidence_hash": ev.evidence_hash, + "sub_type": ev.sub_type, + "audit_row": _audit_row_to_dict(ev.audit_row), + } + + +def _evidence_from_dict(data: dict[str, Any]) -> MathReaderRefusalEvidence: + return MathReaderRefusalEvidence( + case_id=str(data["case_id"]), + sentence_index=int(data["sentence_index"]), + token_index=int(data["token_index"]), + refusal_reason=str(data["refusal_reason"]), + missing_operator=data.get("missing_operator"), + claim_signature=str(data.get("claim_signature", "")), + evidence_hash=str(data["evidence_hash"]), + audit_row=_audit_row_from_dict(data["audit_row"]), + sub_type=data["sub_type"], + ) + + +def _step_to_dict(step: ReasoningStep) -> dict[str, Any]: + return { + "step_index": step.step_index, + "step_kind": step.step_kind, + "input_pointers": list(step.input_pointers), + "claim": step.claim, + "justification": step.justification, + "output_payload": step.output_payload, + } + + +def _step_from_dict(data: dict[str, Any]) -> ReasoningStep: + return ReasoningStep( + step_index=int(data["step_index"]), + step_kind=data["step_kind"], + input_pointers=tuple(str(p) for p in data.get("input_pointers", ())), + claim=str(data.get("claim", "")), + justification=str(data.get("justification", "")), + output_payload=data.get("output_payload"), + ) + + +def to_jsonl_record(proposal: MathReaderRefusalShapeProposal) -> dict[str, Any]: + """Return a self-contained dict representation suitable for JSONL persistence. + + Unlike :func:`canonical_bytes`, this record includes: + - ``proposal_id`` (so consumers don't need to recompute it) + - full ``evidence_pointers`` (nested dicts — not just hashes) + - full ``reasoning_trace.steps`` (inline — not just trace_id) + + The output is JSON-serializable. Encoding to bytes via + ``json.dumps(record, sort_keys=True, separators=(",", ":"), + ensure_ascii=False)`` produces deterministic byte-identical output + across reruns. + """ + trace = proposal.reasoning_trace + return { + "proposal_id": proposal.proposal_id, + "domain": proposal.domain, + "shape_category": proposal.shape_category.value, + "structural_commonality": proposal.structural_commonality, + "evidence_pointers": [ + _evidence_to_dict(ev) for ev in proposal.evidence_pointers + ], + "proposed_change_kind": proposal.proposed_change_kind, + "proposed_change_payload": proposal.proposed_change_payload, + "wrong_zero_assertion": proposal.wrong_zero_assertion, + "replay_equivalence_hash": proposal.replay_equivalence_hash, + "reasoning_trace": { + "trace_id": trace.trace_id, + "steps": [_step_to_dict(s) for s in trace.steps], + }, + } + + +def from_jsonl_record(record: dict[str, Any]) -> MathReaderRefusalShapeProposal: + """Reconstruct a proposal from a :func:`to_jsonl_record` dict. + + Goes through :func:`build_proposal` so all invariants are re-validated + (evidence floor, change_kind allowlist, wrong_zero min-length, trace_id + presence, JSON-serializable payload). The reconstructed ``proposal_id`` + must match the persisted one — mismatch indicates tampering or schema + drift and raises :class:`ValueError`. + """ + evidence_records = tuple( + _evidence_from_dict(d) for d in record.get("evidence_pointers", ()) + ) + steps = tuple(_step_from_dict(d) for d in record["reasoning_trace"]["steps"]) + trace = build_trace(steps) + + shape_category = ShapeCategory(record["shape_category"]) + + proposal = build_proposal( + domain=record.get("domain", "math"), + shape_category=shape_category, + structural_commonality=str(record["structural_commonality"]), + evidence_pointers=evidence_records, + proposed_change_kind=str(record["proposed_change_kind"]), + proposed_change_payload=record.get("proposed_change_payload"), + wrong_zero_assertion=str(record["wrong_zero_assertion"]), + replay_equivalence_hash=str(record["replay_equivalence_hash"]), + reasoning_trace=trace, + ) + + persisted_id = str(record.get("proposal_id", "")) + if persisted_id and persisted_id != proposal.proposal_id: + raise ValueError( + "proposal_id mismatch on JSONL round-trip: persisted " + f"{persisted_id!r} != recomputed {proposal.proposal_id!r}" + ) + return proposal + + __all__ = [ "ChangeKind", "MathReaderRefusalShapeProposal", "build_proposal", "canonical_bytes", "compute_proposal_id", + "from_jsonl_record", + "to_jsonl_record", ] diff --git a/tests/test_adr_0172_w1_shape_proposal.py b/tests/test_adr_0172_w1_shape_proposal.py index 68446356..f07163c4 100644 --- a/tests/test_adr_0172_w1_shape_proposal.py +++ b/tests/test_adr_0172_w1_shape_proposal.py @@ -249,3 +249,122 @@ def test_proposal_id_sensitive_to_shape_category() -> None: p1 = _build(shape_category=ShapeCategory.RATE_WITH_CURRENCY) p2 = _build(shape_category=ShapeCategory.CURRENCY_AMOUNT) assert p1.proposal_id != p2.proposal_id + + +# --------------------------------------------------------------------------- +# ADR-0172 tightening follow-up #1 — self-contained JSONL round-trip +# --------------------------------------------------------------------------- + + +def _build_with_real_trace(**overrides: Any) -> MathReaderRefusalShapeProposal: + """Build a proposal using a real ReasoningTrace (round-trip requires it).""" + from teaching.math_reasoning_trace import ReasoningStep, build_trace + + steps = tuple( + ReasoningStep( + step_index=i, + step_kind=kind, + input_pointers=("ev-001", "ev-002"), + claim=f"step {i} claim", + justification=f"step {i} justification", + output_payload={"i": i}, + ) + for i, kind in enumerate( + ("observation", "grouping", "hypothesis", "conclusion") + ) + ) + trace = build_trace(steps) + kwargs: dict[str, Any] = dict( + shape_category=ShapeCategory.RATE_WITH_CURRENCY, + structural_commonality="subject earns currency per time-unit", + evidence_pointers=_two_evidences(), + proposed_change_kind="injector_sub_shape", + proposed_change_payload={"narrow_form": "$ per "}, + wrong_zero_assertion=_VALID_ASSERTION, + replay_equivalence_hash="b" * 64, + reasoning_trace=trace, + ) + kwargs.update(overrides) + return build_proposal(**kwargs) + + +def test_to_jsonl_record_self_contained() -> None: + """to_jsonl_record emits proposal_id, full evidence_pointers, full trace steps.""" + from teaching.math_contemplation_proposal import to_jsonl_record + + proposal = _build_with_real_trace() + record = to_jsonl_record(proposal) + + # proposal_id is present (canonical_bytes omits it; this is the difference) + assert record["proposal_id"] == proposal.proposal_id + + # evidence_pointers are nested dicts (not just hashes) + assert isinstance(record["evidence_pointers"], list) + assert len(record["evidence_pointers"]) == 2 + assert isinstance(record["evidence_pointers"][0], dict) + assert "evidence_hash" in record["evidence_pointers"][0] + assert "audit_row" in record["evidence_pointers"][0] + + # reasoning_trace carries full steps inline (not just trace_id) + assert isinstance(record["reasoning_trace"], dict) + assert record["reasoning_trace"]["trace_id"] == proposal.reasoning_trace.trace_id + assert len(record["reasoning_trace"]["steps"]) == 4 + + +def test_jsonl_record_round_trip() -> None: + """to_jsonl_record → from_jsonl_record returns an equivalent proposal.""" + from teaching.math_contemplation_proposal import from_jsonl_record, to_jsonl_record + + proposal = _build_with_real_trace() + record = to_jsonl_record(proposal) + restored = from_jsonl_record(record) + + assert restored.proposal_id == proposal.proposal_id + assert restored.domain == proposal.domain + assert restored.proposed_change_kind == proposal.proposed_change_kind + assert restored.shape_category == proposal.shape_category + assert restored.replay_equivalence_hash == proposal.replay_equivalence_hash + assert restored.reasoning_trace.trace_id == proposal.reasoning_trace.trace_id + assert len(restored.evidence_pointers) == len(proposal.evidence_pointers) + + +def test_jsonl_record_byte_stability() -> None: + """Same proposal → byte-identical JSON line across reruns (sort_keys=True).""" + from teaching.math_contemplation_proposal import to_jsonl_record + + p1 = _build_with_real_trace() + p2 = _build_with_real_trace() + line1 = json.dumps( + to_jsonl_record(p1), sort_keys=True, separators=(",", ":"), ensure_ascii=False + ) + line2 = json.dumps( + to_jsonl_record(p2), sort_keys=True, separators=(",", ":"), ensure_ascii=False + ) + assert line1 == line2 + + +def test_jsonl_record_proposal_id_mismatch_rejected() -> None: + """A tampered proposal_id in the persisted record raises ValueError.""" + from teaching.math_contemplation_proposal import from_jsonl_record, to_jsonl_record + + proposal = _build_with_real_trace() + record = to_jsonl_record(proposal) + record["proposal_id"] = "0" * 64 # tamper + + with pytest.raises(ValueError, match="proposal_id mismatch"): + from_jsonl_record(record) + + +def test_canonical_bytes_unchanged_by_tightening() -> None: + """canonical_bytes (the content-hash function) still omits proposal_id. + + Tightening follow-up #1 added to_jsonl_record as a SEPARATE serializer; + canonical_bytes itself must remain stable so trace_id and proposal_id + derivations don't shift. + """ + proposal = _build_with_real_trace() + raw = canonical_bytes(proposal) + decoded = json.loads(raw.decode("utf-8")) + assert "proposal_id" not in decoded + # evidence_pointers in canonical_bytes are still hash strings, not dicts + assert all(isinstance(p, str) for p in decoded["evidence_pointers"]) diff --git a/tests/test_adr_0172_w2_decomposer.py b/tests/test_adr_0172_w2_decomposer.py index 8de5c811..230d0b19 100644 --- a/tests/test_adr_0172_w2_decomposer.py +++ b/tests/test_adr_0172_w2_decomposer.py @@ -311,3 +311,123 @@ def test_decompose_audit_missing_file_raises(tmp_path: Path) -> None: """A non-existent audit path raises (no silent empty-tuple swallow).""" with pytest.raises(FileNotFoundError): decompose_audit(tmp_path / "nope.json") + + +# --------------------------------------------------------------------------- +# ADR-0172 tightening follow-up #2 — pair-based dispatch table +# --------------------------------------------------------------------------- + + +def test_decompose_audit_pair_dispatch_unexpected_category_matcher( + tmp_path: Path, +) -> None: + """(unexpected_category, pre_frame_filler_sentence) → matcher_extension.""" + audit = _write_audit( + tmp_path, + [ + _case( + case_id="pf-001", + refusal_reason="unexpected_category", + missing_operator="pre_frame_filler_sentence", + ), + _case( + case_id="pf-002", + refusal_reason="unexpected_category", + missing_operator="pre_frame_filler_sentence", + ), + ], + ) + proposals = decompose_audit(audit) + assert len(proposals) == 1 + assert proposals[0].proposed_change_kind == "matcher_extension" + + +def test_decompose_audit_pair_dispatch_unexpected_category_frame_reclass( + tmp_path: Path, +) -> None: + """(unexpected_category, multi_subject_sentence) → frame_reclassification.""" + audit = _write_audit( + tmp_path, + [ + _case( + case_id="ms-001", + refusal_reason="unexpected_category", + missing_operator="multi_subject_sentence", + ), + _case( + case_id="ms-002", + refusal_reason="unexpected_category", + missing_operator="multi_subject_sentence", + ), + ], + ) + proposals = decompose_audit(audit) + assert len(proposals) == 1 + assert proposals[0].proposed_change_kind == "frame_reclassification" + + +def test_decompose_audit_pair_dispatch_pronoun(tmp_path: Path) -> None: + """(unresolved_pronoun, pronoun_resolution) → matcher_extension.""" + audit = _write_audit( + tmp_path, + [ + _case( + case_id="pr-001", + refusal_reason="unresolved_pronoun", + missing_operator="pronoun_resolution", + ), + _case( + case_id="pr-002", + refusal_reason="unresolved_pronoun", + missing_operator="pronoun_resolution", + ), + ], + ) + proposals = decompose_audit(audit) + assert len(proposals) == 1 + assert proposals[0].proposed_change_kind == "matcher_extension" + + +def test_decompose_audit_real_audit_change_kind_distribution() -> None: + """Against the real audit_brief_11.json, dispatch yields the expected mix. + + Per ADR-0172 tightening follow-up #2, the widened heuristic must produce + at least 3 matcher_extension and at least 2 frame_reclassification + proposals on the train-sample audit. + """ + proposals = decompose_audit(REAL_AUDIT_PATH) + kinds = [p.proposed_change_kind for p in proposals] + matcher = sum(1 for k in kinds if k == "matcher_extension") + frame = sum(1 for k in kinds if k == "frame_reclassification") + assert matcher >= 3, f"expected ≥3 matcher_extension, got {matcher}: {kinds}" + assert frame >= 2, f"expected ≥2 frame_reclassification, got {frame}: {kinds}" + + +# --------------------------------------------------------------------------- +# ADR-0172 tightening follow-up #3 — shape_category structural gap +# --------------------------------------------------------------------------- + + +def test_decompose_audit_shape_category_is_uncategorized(tmp_path: Path) -> None: + """Until shape_category is added to MathReaderRefusalEvidence, all proposals + emit ShapeCategory.UNCATEGORIZED — do NOT invent values.""" + from evals.refusal_taxonomy.shape_categories import ShapeCategory + + audit = _write_audit( + tmp_path, + [ + _case( + case_id="sc-001", + refusal_reason="unexpected_category", + missing_operator="pre_frame_filler_sentence", + ), + _case( + case_id="sc-002", + refusal_reason="unexpected_category", + missing_operator="pre_frame_filler_sentence", + ), + ], + ) + proposals = decompose_audit(audit) + assert proposals + assert all(p.shape_category == ShapeCategory.UNCATEGORIZED for p in proposals) diff --git a/tests/test_adr_0172_w3_cli_lane.py b/tests/test_adr_0172_w3_cli_lane.py index 2ab67877..706ef724 100644 --- a/tests/test_adr_0172_w3_cli_lane.py +++ b/tests/test_adr_0172_w3_cli_lane.py @@ -104,21 +104,59 @@ def test_cli_lane_writes_jsonl_with_real_audit(tmp_path: Path) -> None: def test_cli_lane_output_sorted_by_proposal_id(tmp_path: Path) -> None: - # canonical_bytes() omits proposal_id by design; verify by comparing the - # written bytes to the decomposer output (which is itself sorted by proposal_id). - from teaching.math_contemplation import decompose_audit - from teaching.math_contemplation_proposal import canonical_bytes + """Output lines are sorted by proposal_id (matches decomposer sort contract).""" + with _patch_proposals_dir(tmp_path) as output: + args = _Args(audit=str(REAL_AUDIT_PATH)) + cmd_eval_math_contemplation(args) - proposals = decompose_audit(REAL_AUDIT_PATH) - expected = b"".join(canonical_bytes(p) + b"\n" for p in proposals) + lines = output.read_bytes().splitlines() + ids = [json.loads(ln)["proposal_id"] for ln in lines] + assert ids == sorted(ids), "output lines must be sorted by proposal_id" + + +def test_cli_lane_jsonl_records_are_self_contained(tmp_path: Path) -> None: + """ADR-0172 tightening follow-up #1: each line is a self-contained record. + + Per to_jsonl_record(), each line must carry proposal_id, full + evidence_pointers (as nested dicts), and full reasoning_trace.steps — + so the workbench can load without re-running decompose_audit(). + """ + with _patch_proposals_dir(tmp_path) as output: + args = _Args(audit=str(REAL_AUDIT_PATH)) + cmd_eval_math_contemplation(args) + + lines = output.read_bytes().splitlines() + assert lines + for ln in lines: + record = json.loads(ln) + # Self-containment invariants: + assert "proposal_id" in record and record["proposal_id"] + assert record["domain"] == "math" + # evidence_pointers are nested dicts, not hash strings + assert isinstance(record["evidence_pointers"], list) + assert all(isinstance(ev, dict) for ev in record["evidence_pointers"]) + assert all("evidence_hash" in ev for ev in record["evidence_pointers"]) + assert all("audit_row" in ev for ev in record["evidence_pointers"]) + # reasoning_trace carries full steps inline + assert isinstance(record["reasoning_trace"], dict) + assert "trace_id" in record["reasoning_trace"] + assert len(record["reasoning_trace"]["steps"]) == 4 + + +def test_cli_lane_records_round_trip_via_from_jsonl_record(tmp_path: Path) -> None: + """Each line round-trips through from_jsonl_record() back to a proposal.""" + from teaching.math_contemplation_proposal import from_jsonl_record with _patch_proposals_dir(tmp_path) as output: args = _Args(audit=str(REAL_AUDIT_PATH)) cmd_eval_math_contemplation(args) - assert output.read_bytes() == expected, ( - "output bytes must match decomposer output in proposal_id order" - ) + for ln in output.read_bytes().splitlines(): + record = json.loads(ln) + proposal = from_jsonl_record(record) + # proposal_id must round-trip identically (validates evidence_hashes + # and trace_id are stable through serialization) + assert proposal.proposal_id == record["proposal_id"] def test_cli_lane_idempotent(tmp_path: Path) -> None: diff --git a/tests/test_adr_0172_w4_workbench_e2e.py b/tests/test_adr_0172_w4_workbench_e2e.py index f364a977..2b475827 100644 --- a/tests/test_adr_0172_w4_workbench_e2e.py +++ b/tests/test_adr_0172_w4_workbench_e2e.py @@ -1,6 +1,6 @@ """ADR-0172 W4 — Workbench math-proposals e2e tests. -Six tests: +Six core tests plus an ADR-0172 tightening decoupling test: 1. Loads from JSONL: GET /math-proposals returns items when proposals.jsonl exists. 2. Renders domain badge: items carry domain='math', distinct from cognition /proposals. @@ -8,92 +8,68 @@ Six tests: 4. ratify-matcher_extension fails loudly (501, not_implemented). 5. All 4 trace steps visible: GET /math-proposals/{id} includes 4-step trace. 6. No cross-contamination: cognition /proposals and math /math-proposals are independent. +7. (Tightening follow-up #1) Workbench reads self-contained JSONL — no decompose_audit() coupling. """ from __future__ import annotations import hashlib import json +import sys from pathlib import Path import pytest from workbench.api import WorkbenchApi -from workbench.readers import ( - MATH_PROPOSALS_JSONL, - _DEFAULT_MATH_AUDIT_PATH, - _load_math_proposals_raw, +from workbench.readers import MATH_PROPOSALS_JSONL + +# Resolve the real audit once (used only to build test JSONL via the +# decomposer); the workbench itself no longer needs the audit file. +REAL_AUDIT_PATH = ( + Path(__file__).resolve().parent.parent + / "evals" / "gsm8k_math" / "train_sample" / "v1" / "audit_brief_11.json" ) + # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- -REAL_AUDIT_PATH = _DEFAULT_MATH_AUDIT_PATH +def _write_jsonl_from_proposals(tmp_path: Path, proposals) -> Path: + """Write proposals.jsonl using the self-contained to_jsonl_record() format.""" + from teaching.math_contemplation_proposal import to_jsonl_record -def _write_jsonl(tmp_path: Path, proposals_jsonl_bytes: bytes) -> Path: - p = tmp_path / "proposals.jsonl" - p.write_bytes(proposals_jsonl_bytes) - return p - - -def _api(jsonl_path: Path | None = None, audit_path: Path | None = None) -> WorkbenchApi: - """Return a WorkbenchApi with its readers patched to use tmp paths.""" - import workbench.readers as _r - - api = WorkbenchApi() - _orig_list = _r.list_math_proposals - _orig_read = _r.read_math_proposal - _orig_ratify = _r.ratify_math_proposal - - if jsonl_path is not None: - import functools - - api._list_math_proposals = functools.partial(_r.list_math_proposals, jsonl_path=jsonl_path) - api._read_math_proposal = functools.partial( - _r.read_math_proposal, jsonl_path=jsonl_path, audit_path=audit_path or REAL_AUDIT_PATH + lines: list[bytes] = [] + for p in proposals: + record = to_jsonl_record(p) + lines.append( + json.dumps( + record, ensure_ascii=False, sort_keys=True, separators=(",", ":") + ).encode("utf-8") + + b"\n" ) - api._ratify_math_proposal = functools.partial( - _r.ratify_math_proposal, jsonl_path=jsonl_path - ) - # Patch the module-level functions temporarily - _r.list_math_proposals = api._list_math_proposals # type: ignore[assignment] - _r.read_math_proposal = api._read_math_proposal # type: ignore[assignment] - _r.ratify_math_proposal = api._ratify_math_proposal # type: ignore[assignment] - - yield api - - # Restore - _r.list_math_proposals = _orig_list - _r.read_math_proposal = _orig_read - _r.ratify_math_proposal = _orig_ratify + path = tmp_path / "proposals.jsonl" + path.write_bytes(b"".join(lines)) + return path -def _get(api: WorkbenchApi, path: str) -> dict: - resp = api.handle("GET", path) - return {"status": resp.status, "payload": resp.payload} - - -def _post(api: WorkbenchApi, path: str, body: bytes = b"{}") -> dict: - resp = api.handle("POST", path, body) - return {"status": resp.status, "payload": resp.payload} - - -def _make_synthetic_proposals_jsonl(tmp_path: Path, change_kinds: list[str]) -> tuple[Path, list[str]]: - """Build a synthetic proposals.jsonl with one proposal per change_kind. - - Returns (jsonl_path, list_of_proposal_ids). - """ - from evals.refusal_taxonomy.shape_categories import ShapeCategory +def _real_audit_proposals(): from teaching.math_contemplation import decompose_audit - from teaching.math_contemplation_proposal import build_proposal, canonical_bytes - from teaching.math_evidence import MathReaderRefusalEvidence, SUB_TYPE_FOR_OPERATOR + return decompose_audit(REAL_AUDIT_PATH) + + +def _make_synthetic_proposals_jsonl( + tmp_path: Path, change_kinds: list[str] +) -> tuple[Path, list[str]]: + """Build a synthetic proposals.jsonl in to_jsonl_record() format.""" + from evals.refusal_taxonomy.shape_categories import ShapeCategory + from teaching.math_contemplation_proposal import build_proposal + from teaching.math_evidence import MathReaderRefusalEvidence, from_audit_row from teaching.math_reasoning_trace import ReasoningStep, build_trace from generate.comprehension.audit import AuditRow def _ev(case_id: str, missing_op: str) -> MathReaderRefusalEvidence: - from teaching.math_evidence import from_audit_row row = AuditRow( case_id=case_id, sentence_index=0, @@ -107,7 +83,7 @@ def _make_synthetic_proposals_jsonl(tmp_path: Path, change_kinds: list[str]) -> ) return from_audit_row(row, "lexical", claim_signature="") - lines: list[bytes] = [] + proposals = [] proposal_ids: list[str] = [] for i, ck in enumerate(change_kinds): @@ -115,42 +91,54 @@ def _make_synthetic_proposals_jsonl(tmp_path: Path, change_kinds: list[str]) -> ev2 = _ev(f"c{i:02d}b", "drain_token") obs = ReasoningStep( - step_index=0, step_kind="observation", input_pointers=(ev1.case_id, ev2.case_id), + step_index=0, step_kind="observation", + input_pointers=(ev1.case_id, ev2.case_id), claim=f"synthetic observation for {ck}", justification="test", output_payload={"evidence_count": 2}, ) grp = ReasoningStep( - step_index=1, step_kind="grouping", input_pointers=(ev1.case_id, ev2.case_id), + step_index=1, step_kind="grouping", + input_pointers=(ev1.case_id, ev2.case_id), claim="group key", justification="test", output_payload={"k": "v"}, ) hyp = ReasoningStep( - step_index=2, step_kind="hypothesis", input_pointers=(ev1.case_id, ev2.case_id), - claim=f"change_kind={ck}", justification="test", output_payload={"ck": ck}, + step_index=2, step_kind="hypothesis", + input_pointers=(ev1.case_id, ev2.case_id), + claim=f"change_kind={ck}", justification="test", + output_payload={"ck": ck}, ) con = ReasoningStep( - step_index=3, step_kind="conclusion", input_pointers=(ev1.case_id, ev2.case_id), + step_index=3, step_kind="conclusion", + input_pointers=(ev1.case_id, ev2.case_id), claim="conclude", justification="test", output_payload={"done": 1}, ) trace = build_trace((obs, grp, hyp, con)) p = build_proposal( shape_category=ShapeCategory.UNCATEGORIZED, - structural_commonality=f"synthetic {ck} test proposal — sufficient length for wrong_zero_assertion", + structural_commonality=( + f"synthetic {ck} test proposal — sufficient length for " + "wrong_zero_assertion gating" + ), evidence_pointers=(ev1, ev2), proposed_change_kind=ck, # type: ignore[arg-type] - proposed_change_payload={"evidence_count": 2, "group_key": {"k": "v"}, "modal_sub_type": "lexical"}, + proposed_change_payload={ + "evidence_count": 2, + "group_key": {"k": "v"}, + "modal_sub_type": "lexical", + }, wrong_zero_assertion=( "synthetic proposal for test; ratification handler enforces wrong=0" ), - replay_equivalence_hash=hashlib.sha256(f"replay-{ck}-{i}".encode()).hexdigest(), + replay_equivalence_hash=hashlib.sha256( + f"replay-{ck}-{i}".encode() + ).hexdigest(), reasoning_trace=trace, ) - cb = canonical_bytes(p) - lines.append(cb + b"\n") + proposals.append(p) proposal_ids.append(p.proposal_id) - jsonl_path = tmp_path / "proposals.jsonl" - jsonl_path.write_bytes(b"".join(lines)) + jsonl_path = _write_jsonl_from_proposals(tmp_path, proposals) return jsonl_path, proposal_ids @@ -161,12 +149,8 @@ def _make_synthetic_proposals_jsonl(tmp_path: Path, change_kinds: list[str]) -> def test_1_loads_from_jsonl(tmp_path: Path) -> None: """GET /math-proposals returns items from proposals.jsonl when it exists.""" - from teaching.math_contemplation import decompose_audit - from teaching.math_contemplation_proposal import canonical_bytes as cb - - proposals = decompose_audit(REAL_AUDIT_PATH) - jsonl_bytes = b"".join(cb(p) + b"\n" for p in proposals) - jsonl = _write_jsonl(tmp_path, jsonl_bytes) + proposals = _real_audit_proposals() + jsonl = _write_jsonl_from_proposals(tmp_path, proposals) import workbench.readers as _r orig = _r.MATH_PROPOSALS_JSONL @@ -178,8 +162,7 @@ def test_1_loads_from_jsonl(tmp_path: Path) -> None: _r.MATH_PROPOSALS_JSONL = orig assert resp.status == 200 - data = resp.payload.get("data", {}) - items = data.get("items", []) + items = resp.payload.get("data", {}).get("items", []) assert len(items) == len(proposals) @@ -190,12 +173,8 @@ def test_1_loads_from_jsonl(tmp_path: Path) -> None: def test_2_renders_domain_math_badge(tmp_path: Path) -> None: """Items from /math-proposals have domain='math'; /proposals returns cognition (no domain field).""" - from teaching.math_contemplation import decompose_audit - from teaching.math_contemplation_proposal import canonical_bytes as cb - - proposals = decompose_audit(REAL_AUDIT_PATH) - jsonl_bytes = b"".join(cb(p) + b"\n" for p in proposals) - jsonl = _write_jsonl(tmp_path, jsonl_bytes) + proposals = _real_audit_proposals() + jsonl = _write_jsonl_from_proposals(tmp_path, proposals) import workbench.readers as _r orig = _r.MATH_PROPOSALS_JSONL @@ -207,17 +186,12 @@ def test_2_renders_domain_math_badge(tmp_path: Path) -> None: _r.MATH_PROPOSALS_JSONL = orig items = resp.payload["data"]["items"] - assert all(item["domain"] == "math" for item in items), ( - "all math proposals must carry domain='math'" - ) + assert all(item["domain"] == "math" for item in items) - # Cognition /proposals carries ProposalSummary — no domain field at all cog_resp = WorkbenchApi().handle("GET", "/proposals") cog_items = cog_resp.payload.get("data", {}).get("items", []) for cog_item in cog_items: - assert cog_item.get("domain") != "math", ( - "cognition proposals must not carry domain='math'" - ) + assert cog_item.get("domain") != "math" # --------------------------------------------------------------------------- @@ -227,7 +201,9 @@ def test_2_renders_domain_math_badge(tmp_path: Path) -> None: def test_3_ratify_vocabulary_addition_routes_to_lexical_claim(tmp_path: Path) -> None: """POST /math-proposals/{id}/ratify routes vocabulary_addition to LexicalClaim (200).""" - jsonl_path, proposal_ids = _make_synthetic_proposals_jsonl(tmp_path, ["vocabulary_addition"]) + jsonl_path, proposal_ids = _make_synthetic_proposals_jsonl( + tmp_path, ["vocabulary_addition"] + ) vocab_pid = proposal_ids[0] import workbench.readers as _r @@ -241,9 +217,7 @@ def test_3_ratify_vocabulary_addition_routes_to_lexical_claim(tmp_path: Path) -> assert resp.status == 200, f"expected 200, got {resp.status}: {resp.payload}" data = resp.payload.get("data", {}) - assert data.get("handler_name") == "LexicalClaim", ( - f"vocabulary_addition must route to LexicalClaim, got: {data.get('handler_name')!r}" - ) + assert data.get("handler_name") == "LexicalClaim" assert data.get("routing_status") == "routed" assert data.get("change_kind") == "vocabulary_addition" @@ -255,7 +229,9 @@ def test_3_ratify_vocabulary_addition_routes_to_lexical_claim(tmp_path: Path) -> def test_4_ratify_matcher_extension_fails_loudly(tmp_path: Path) -> None: """POST /math-proposals/{id}/ratify returns 501 for matcher_extension with clear message.""" - jsonl_path, proposal_ids = _make_synthetic_proposals_jsonl(tmp_path, ["matcher_extension"]) + jsonl_path, proposal_ids = _make_synthetic_proposals_jsonl( + tmp_path, ["matcher_extension"] + ) matcher_pid = proposal_ids[0] import workbench.readers as _r @@ -267,15 +243,11 @@ def test_4_ratify_matcher_extension_fails_loudly(tmp_path: Path) -> None: finally: _r.MATH_PROPOSALS_JSONL = orig - assert resp.status == 501, f"expected 501 for unimplemented handler, got {resp.status}" + assert resp.status == 501 err = resp.payload.get("error", {}) message = err.get("message", "") - assert "handler not yet implemented" in message, ( - f"expected 'handler not yet implemented' in message, got: {message!r}" - ) - assert "matcher_extension" in message, ( - f"expected change_kind in error message, got: {message!r}" - ) + assert "handler not yet implemented" in message + assert "matcher_extension" in message # --------------------------------------------------------------------------- @@ -284,33 +256,30 @@ def test_4_ratify_matcher_extension_fails_loudly(tmp_path: Path) -> None: def test_5_all_4_trace_steps_visible(tmp_path: Path) -> None: - """GET /math-proposals/{id} includes reasoning_trace_steps with all 4 steps.""" - from teaching.math_contemplation import decompose_audit - from teaching.math_contemplation_proposal import canonical_bytes as cb + """GET /math-proposals/{id} includes reasoning_trace_steps with all 4 steps. - proposals = decompose_audit(REAL_AUDIT_PATH) - assert proposals, "real audit must produce at least one proposal" - jsonl_bytes = b"".join(cb(p) + b"\n" for p in proposals) - jsonl = _write_jsonl(tmp_path, jsonl_bytes) + Post-tightening: the workbench reads steps directly from the + self-contained JSONL record; no decompose_audit() re-run. + """ + proposals = _real_audit_proposals() + assert proposals + jsonl = _write_jsonl_from_proposals(tmp_path, proposals) first_id = proposals[0].proposal_id import workbench.readers as _r orig = _r.MATH_PROPOSALS_JSONL - orig_audit = _r._DEFAULT_MATH_AUDIT_PATH _r.MATH_PROPOSALS_JSONL = jsonl - _r._DEFAULT_MATH_AUDIT_PATH = REAL_AUDIT_PATH try: api = WorkbenchApi() resp = api.handle("GET", f"/math-proposals/{first_id}") finally: _r.MATH_PROPOSALS_JSONL = orig - _r._DEFAULT_MATH_AUDIT_PATH = orig_audit assert resp.status == 200, f"expected 200, got {resp.status}: {resp.payload}" detail = resp.payload.get("data", {}) steps = detail.get("reasoning_trace_steps", []) - assert len(steps) == 4, f"expected 4 trace steps, got {len(steps)}" + assert len(steps) == 4 step_kinds = [s["step_kind"] for s in steps] assert "observation" in step_kinds assert "grouping" in step_kinds @@ -325,12 +294,8 @@ def test_5_all_4_trace_steps_visible(tmp_path: Path) -> None: def test_6_no_cross_contamination(tmp_path: Path) -> None: """Math /math-proposals and cognition /proposals are independent queues.""" - from teaching.math_contemplation import decompose_audit - from teaching.math_contemplation_proposal import canonical_bytes as cb - - proposals = decompose_audit(REAL_AUDIT_PATH) - jsonl_bytes = b"".join(cb(p) + b"\n" for p in proposals) - jsonl = _write_jsonl(tmp_path, jsonl_bytes) + proposals = _real_audit_proposals() + jsonl = _write_jsonl_from_proposals(tmp_path, proposals) import workbench.readers as _r orig = _r.MATH_PROPOSALS_JSONL @@ -345,16 +310,60 @@ def test_6_no_cross_contamination(tmp_path: Path) -> None: math_items = math_resp.payload.get("data", {}).get("items", []) cog_items = cog_resp.payload.get("data", {}).get("items", []) - # Math queue carries domain:math; cognition queue has no domain:math entries assert all(item.get("domain") == "math" for item in math_items) for cog_item in cog_items: - assert cog_item.get("domain") != "math", ( - "cognition proposal leaked into math domain" - ) + assert cog_item.get("domain") != "math" - # proposal_ids from the two queues are disjoint math_ids = {item["proposal_id"] for item in math_items} cog_ids = {item["proposal_id"] for item in cog_items} - assert math_ids.isdisjoint(cog_ids), ( - f"proposal IDs overlap between math and cognition queues: {math_ids & cog_ids}" - ) + assert math_ids.isdisjoint(cog_ids) + + +# --------------------------------------------------------------------------- +# Test 7: Tightening follow-up #1 — workbench decoupled from decomposer +# --------------------------------------------------------------------------- + + +def test_7_workbench_decoupled_from_decompose_audit(tmp_path: Path) -> None: + """Workbench reads math proposals without decompose_audit() being importable. + + Mocks teaching.math_contemplation out of sys.modules to prove the + workbench load path does not depend on the decomposer. Prior to the + tightening follow-up, read_math_proposal() re-ran decompose_audit() to + recover full trace steps — that coupling is now removed. + """ + proposals = _real_audit_proposals() + jsonl = _write_jsonl_from_proposals(tmp_path, proposals) + first_id = proposals[0].proposal_id + + import workbench.readers as _r + orig_jsonl = _r.MATH_PROPOSALS_JSONL + _r.MATH_PROPOSALS_JSONL = jsonl + + # Drop teaching.math_contemplation from the module cache to surface any + # residual import dependency. Install a sentinel that raises on access. + class _ExplodingModule: + def __getattr__(self, name: str): + raise ImportError( + f"decoupling violated: workbench tried to access " + f"teaching.math_contemplation.{name}" + ) + + saved_module = sys.modules.pop("teaching.math_contemplation", None) + sys.modules["teaching.math_contemplation"] = _ExplodingModule() # type: ignore[assignment] + + try: + api = WorkbenchApi() + list_resp = api.handle("GET", "/math-proposals") + detail_resp = api.handle("GET", f"/math-proposals/{first_id}") + finally: + _r.MATH_PROPOSALS_JSONL = orig_jsonl + if saved_module is not None: + sys.modules["teaching.math_contemplation"] = saved_module + else: + sys.modules.pop("teaching.math_contemplation", None) + + assert list_resp.status == 200 + assert detail_resp.status == 200 + detail = detail_resp.payload["data"] + assert len(detail["reasoning_trace_steps"]) == 4 diff --git a/workbench/readers.py b/workbench/readers.py index 61f5049b..b8f2c049 100644 --- a/workbench/readers.py +++ b/workbench/readers.py @@ -290,7 +290,13 @@ def read_eval_lane(lane_name: str) -> EvalLaneSummary: def _load_math_proposals_raw(jsonl_path: Path) -> list[dict[str, Any]]: - """Parse proposals.jsonl; derive proposal_id = sha256(canonical_line_bytes).""" + """Parse proposals.jsonl into self-contained record dicts. + + ADR-0172 tightening follow-up #1: each line is a self-contained record + written by :func:`teaching.math_contemplation_proposal.to_jsonl_record` + that carries its own ``proposal_id``, full ``evidence_pointers``, and + full ``reasoning_trace.steps``. No decomposer re-run required. + """ if not jsonl_path.exists(): return [] results: list[dict[str, Any]] = [] @@ -298,16 +304,14 @@ def _load_math_proposals_raw(jsonl_path: Path) -> list[dict[str, Any]]: stripped = raw_line.strip() if not stripped: continue - proposal_id = hashlib.sha256(stripped).hexdigest() data: dict[str, Any] = json.loads(stripped) - data["proposal_id"] = proposal_id results.append(data) return results def _math_proposal_summary(record: dict[str, Any]) -> MathProposalSummary: - payload = record.get("proposed_change_payload") or {} - evidence_count = int(payload.get("evidence_count", 0)) if isinstance(payload, dict) else 0 + evidence_pointers = record.get("evidence_pointers", []) + evidence_count = len(evidence_pointers) if isinstance(evidence_pointers, list) else 0 return MathProposalSummary( proposal_id=str(record.get("proposal_id", "")), domain="math", @@ -325,22 +329,24 @@ def list_math_proposals(*, jsonl_path: Path | None = None) -> list[MathProposalS return [_math_proposal_summary(r) for r in records] -def _math_trace_steps_from_proposal(proposal: Any) -> list[MathReasoningStep]: - """Extract 4 ReasoningStep objects from a MathReaderRefusalShapeProposal.""" - trace = getattr(proposal, "reasoning_trace", None) - if trace is None: +def _math_trace_steps_from_record(record: dict[str, Any]) -> list[MathReasoningStep]: + """Extract 4 reasoning steps from a self-contained JSONL record.""" + trace = record.get("reasoning_trace") or {} + if not isinstance(trace, dict): return [] - steps_raw = getattr(trace, "steps", ()) + steps_raw = trace.get("steps", []) or [] steps: list[MathReasoningStep] = [] for step in steps_raw: + if not isinstance(step, dict): + continue steps.append( MathReasoningStep( - step_index=int(getattr(step, "step_index", 0)), - step_kind=str(getattr(step, "step_kind", "")), - claim=str(getattr(step, "claim", "")), - justification=str(getattr(step, "justification", "")), - input_pointers=list(getattr(step, "input_pointers", ())), - output_payload=getattr(step, "output_payload", {}), + step_index=int(step.get("step_index", 0)), + step_kind=str(step.get("step_kind", "")), + claim=str(step.get("claim", "")), + justification=str(step.get("justification", "")), + input_pointers=list(step.get("input_pointers", [])), + output_payload=step.get("output_payload"), ) ) return steps @@ -350,39 +356,38 @@ def read_math_proposal( proposal_id: str, *, jsonl_path: Path | None = None, - audit_path: Path | None = None, + audit_path: Path | None = None, # retained for backward-compat; unused ) -> MathProposalDetail: - """Return full proposal detail including 4-step reasoning trace. + """Return full proposal detail loaded entirely from the JSONL record. - Re-runs :func:`teaching.math_contemplation.decompose_audit` to recover - the full :class:`MathReaderRefusalShapeProposal` (canonical bytes only - carry the trace_id, not the full steps). Deterministic: same audit → - same proposals. + ADR-0172 tightening follow-up #1: no longer re-runs decompose_audit(). + The self-contained JSONL line carries the full reasoning_trace.steps + and evidence_pointers; the workbench is decoupled from the decomposer. + + The ``audit_path`` keyword is preserved for call-site backward + compatibility but is unused. """ - from teaching.math_contemplation import decompose_audit + del audit_path # decoupled — kept for backward-compat callers - # Verify the proposal_id exists in the JSONL first (fast path). path = jsonl_path or MATH_PROPOSALS_JSONL records = _load_math_proposals_raw(path) record = next((r for r in records if r.get("proposal_id") == proposal_id), None) if record is None: raise FileNotFoundError(proposal_id) - # Re-run decomposer to get the full proposal with trace steps. - apath = audit_path or _DEFAULT_MATH_AUDIT_PATH - full_proposals = decompose_audit(apath) - full = next((p for p in full_proposals if p.proposal_id == proposal_id), None) - if full is None: - raise FileNotFoundError(f"{proposal_id} (not found in decomposer output)") - change_kind = str(record.get("proposed_change_kind", "")) handler_name = _HANDLER_DISPATCH.get(change_kind) - trace_id = str(record.get("reasoning_trace_id", "")) - trace_steps = _math_trace_steps_from_proposal(full) + trace_obj = record.get("reasoning_trace") or {} + trace_id = str(trace_obj.get("trace_id", "")) if isinstance(trace_obj, dict) else "" + trace_steps = _math_trace_steps_from_record(record) - evidence_pointers = record.get("evidence_pointers", []) - evidence_hashes = list(evidence_pointers) if isinstance(evidence_pointers, list) else [] + evidence_pointers_raw = record.get("evidence_pointers", []) or [] + evidence_hashes = [ + str(ev.get("evidence_hash", "")) + for ev in evidence_pointers_raw + if isinstance(ev, dict) + ] suggested_cli: str | None = None if handler_name == "LexicalClaim":