Commit graph

3 commits

Author SHA1 Message Date
Shay
3fd317290b feat(adr-0174-phase5a): retire inert GSM8K scoring-path reader
The recognizer/candidate-graph path is the single canonical reader.
Retires the flag-gated incremental-reader dispatch that admitted 0/50 on
train_sample and only added a dead fall-through:

- remove _try_comprehension_reader, _try_reader_for_question, _tokenize_sentence
  and both dispatch blocks from generate/math_candidate_graph.py
- delete generate/comprehension/lifecycle_runtime_adapter.py (402 LOC,
  used only by the question-reader dispatch)
- drop the comprehension_reader_questions config flag and the parse_and_solve
  / _score_one_candidate_graph config threading
- remove the --use-reader runner plumbing + flag-ON/OFF delta report from
  the train_sample runner; refresh report.json (drops stale use_reader field
  and a stale refusal-reason; verdicts unchanged at 3/47/0)
- remove the now-dead use_reader field from teaching/coverage.py
  CoverageReport + the core teaching coverage CLI flag
- delete tests/test_reader_coexistence.py (flag-ON/OFF premise dissolved);
  fix 3 ADR-0174 build_report calls and 2 subprocess invocations

lifecycle.py and audit.py are KEPT — they are load-bearing for the ADR-0172
math-contemplation teaching corridor (audit_problem -> teaching/math_*),
which a pre-deletion trace surfaced. The parent ADR's plan to delete
lifecycle.py was wrong; only its GSM8K scoring dispatch was inert.

Net -1,038 LOC (code + tests). Behavior-preserving:
- train_sample 3/47/0, byte-identical verdicts to pre-5a baseline
- determinism holds; smoke/packs/runtime/cognition/teaching lanes green
- contemplation corridor + lifecycle/audit tests pass

Pre-existing (NOT introduced here; reproduce on base with changes stashed):
5 out-of-curated-lane stale committed-artifact / stale-assertion failures
(test_math_evidence_e2e, test_adr_0126_runner_wiring, G3/coverage_probe
report-match, test_refusal_taxonomy_lane rebuild).
2026-05-28 13:38:44 -07:00
Shay
36f3dbfc4e fix(D): address Sourcery review findings on PR #410
Three review fixes:

1. Security: validate lane/split/version against ^[a-z0-9_]+$ before
   building the runner module name. The runner_args list is passed to
   subprocess.run without shell=True (no shell injection possible),
   but defense-in-depth blocks arbitrary token characters from
   reaching Python's -m module loader. Bad input now errors at the
   CLI boundary with a clear message.

2. Bug-risk: _classify_refusal docstring referenced a
   no_admissible_candidate bucket that the implementation never
   emitted. Aligned docstring with actual buckets
   (no_admissible_question / no_admissible_statement). Also made all
   matching consistently case-insensitive (was mixed — some checks
   used raw reason, one used .lower()).

3. Bug-risk: fetch_committed_baseline wrote to
   .git/coverage_baseline_tmp.json. Replaced with tempfile.mkstemp in
   the system temp dir — avoids (a) failures in non-git worktrees
   where .git is a file pointer, (b) concurrent-access collisions
   between simultaneous operators.

Tests (+3 new):
- test_classify_refusal_is_case_insensitive
- test_classify_docstring_matches_implementation_buckets
- test_fetch_committed_baseline_uses_system_temp

All 16 coverage tests green. Verified the validation:
  core teaching coverage --lane 'evil; rm -rf /'
  → ERROR: lane='evil; rm -rf /' must match ^[a-z0-9_]+$
2026-05-27 21:20:01 -07:00
Shay
d91ea3d36e feat(D): core teaching coverage — per-shape admission histogram
Brief D from PR #407. Closes the "flying blind on per-shape coverage"
gap identified in RAT-1's audit (finding 6).

After this PR, every operator can run a single command to see exactly
which refusal modes their work moved (or didn't), without re-eyeballing
report.json by hand.

Modules
-------
- teaching/coverage.py — pure aggregator:
  - _classify_refusal — maps each per-case refusal reason to a
    stable bucket (recognizer_empty_injection(<ShapeCategory>),
    no_admissible_question, no_admissible_statement,
    unexpected_question_count, other)
  - build_coverage_report — reads a lane's report.json + emits a
    CoverageReport with counts, refusal_taxonomy (sorted by count
    desc), case_0050_verdict, optional delta vs baseline
  - fetch_committed_baseline — uses `git show HEAD:<relpath>` to
    pull the baseline report.json for delta computation

- core/cli.py:
  - cmd_teaching_coverage — formats the report for terminal output
  - core teaching coverage [--lane gsm8k_math] [--split train_sample]
    [--version v1] [--use-reader] [--run] [--delta] [--json]

CLI output example
------------------
  Lane: gsm8k_math/train_sample/v1 (use_reader=True)
  Counts: correct=3 refused=47 wrong=0

  Refusal taxonomy:
     21  recognizer_empty_injection(discrete_count_statement)
      6  no_admissible_statement
      5  recognizer_empty_injection(multiplicative_aggregation)
      4  no_admissible_question
      4  recognizer_empty_injection(currency_amount)
      3  recognizer_empty_injection(rate_with_currency)
      2  recognizer_empty_injection(descriptive_setup_no_quantity)
      2  recognizer_empty_injection(temporal_aggregation)

  Wrong=0: ✓
  Case 0050 hazard pin: refused ✓

Tests (13 new)
--------------
tests/test_teaching_coverage_cli.py — classification narrowness,
counts aggregation, case 0050 verdict capture, delta computation,
missing-baseline path, missing-report error, taxonomy sort order,
wrong=0 invariant visibility via as_dict.

Suite results
-------------
core test --suite teaching -q → 106 passed (93 → +13)
core test --suite runtime  -q → 20 passed
core test --suite packs    -q → 127 passed
core eval gsm8k_math --split public → 150/150, wrong=0

Note on Brief E (lexical auto-compile): the audit was WRONG. The
lexicon loader (generate/comprehension/lexicon.py::load_lexicon)
reads from the per-category source files directly; the compiled
lexicon.jsonl is only a manifest-checksum pin, not the source of
truth at runtime. apply_lexical_claim() writes a new entry → next
turn the loader sees it. Brief E is a non-issue; closing without a
code PR.

Verified by direct test: stage a clone of the math pack, write a
synthetic lemma to drain_token.jsonl, clear the lexicon cache, load
again → new entry present. So 3 of the 5 audit gaps closed (A, D,
E-as-correction); B and C remain as the next operator dispatch
targets.

Independent of PR #406 (RAT-1) and PR #408 (WAVE-A). Based on main.
2026-05-27 21:20:00 -07:00