diff --git a/wizard-work/sol/classify.py b/wizard-work/sol/classify.py index 9c99470..60a8e41 100644 --- a/wizard-work/sol/classify.py +++ b/wizard-work/sol/classify.py @@ -5,6 +5,11 @@ Classifies a seed of inbox pending items into (category, decision) pairs and emits a JSON artifact with per-item decisions plus aggregate counts. Categories: archive_fix | app_pr | general +Fallback: unclassified -- an item that matches no category rule is reported + under this label, never folded into a real category. The label is + counted in `categories` beside the three real ones, so the totals + still sum to the number of items (Q119: a silent default is not a + classification). Decisions: approve | table | reject Rules are keyword/phrase regexes, scored per category; highest score wins and @@ -21,6 +26,8 @@ DEFAULT_SEED = HERE / "seed-2026-10-29.json" DEFAULT_ART = HERE / "Triage-2026-10-29.json" CATS = ["archive_fix", "app_pr", "general"] +UNCLASSIFIED = "unclassified" +COUNTED = CATS + [UNCLASSIFIED] # categories emitted in the aggregate PRIORITY = {"archive_fix": 3, "app_pr": 2, "general": 1} CATEGORY_RULES = { @@ -86,7 +93,11 @@ def classify(text): scores, matched = _score(text, CATEGORY_RULES) best = max(scores.values()) if best == 0: - return "general", 0, "no category rule matched; defaulted to general" + return ( + UNCLASSIFIED, + 0, + "no category rule matched; unclassified (no default category)", + ) winners = [c for c in CATS if scores[c] == best] cat = max(winners, key=lambda c: PRIORITY[c]) parts = [f"{c}:{scores[c]}" for c in winners] @@ -129,7 +140,7 @@ def main(argv=None): "reason": f"{reason}; {dec_reason}", }) - cats = {c: 0 for c in CATS} + cats = {c: 0 for c in COUNTED} dec_counts = {"approve": 0, "table": 0, "reject": 0} for it in items: cats[it["category"]] += 1 diff --git a/wizard-work/sol/test_classify_fallback.py b/wizard-work/sol/test_classify_fallback.py new file mode 100644 index 0000000..bebb92a --- /dev/null +++ b/wizard-work/sol/test_classify_fallback.py @@ -0,0 +1,159 @@ +#!/usr/bin/env python3 +"""Cells for the sol classifier's fallback contract (Q119). + +The defect these cells pin: an item that matched no category rule used to be +returned as `general` -- a real category, at score 0 -- so a demand the rule set +did not understand was indistinguishable from one it had understood and scored. +The contract now is `unclassified` at score 0, counted beside the three real +categories so the aggregate still sums to the item count. + +The invariant is one-directional on purpose: a *matched* item may be any real +category (the taxonomy is not this file's business), but a zero-score item may +never be. Four of Q119's five repro strings are zero-score under the current +rules; the fifth, "Triage the inbound queue before standup.", is a `general` +rule hit on `standup` at score 1 -- measured, and pinned as such in +`test_repro_with_a_rule_hit_is_classified_by_rule` rather than being forced +into the fallback to make a table look tidy. +""" +import json +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +HERE = Path(__file__).resolve().parent +sys.path.insert(0, str(HERE)) + +import classify # noqa: E402 (the module under test, imported after sys.path) + +# Q119's five hand-written imperatives, verbatim. +REPRO = [ + "Ship the manifest to ops by Friday.", + "Move the nightly cron to the cheaper provider tonight.", + "We need a call on the provider tonight - reply by 17:00.", + "Triage the inbound queue before standup.", + "Roll back the ingress change.", +] +ZERO_SCORE_REPRO = [t for t in REPRO if "standup" not in t] + + +class FallbackContract(unittest.TestCase): + def test_zero_score_always_reads_unclassified(self): + for text in ZERO_SCORE_REPRO: + with self.subTest(text=text): + cat, score, reason = classify.classify(text) + self.assertEqual(score, 0) + self.assertEqual( + cat, + classify.UNCLASSIFIED, + "%r landed in %r, want %r" % ( + text, cat, classify.UNCLASSIFIED), + ) + self.assertIn("unclassified", reason) + + def test_zero_score_never_lands_in_a_real_category(self): + for text in ZERO_SCORE_REPRO: + with self.subTest(text=text): + cat, score, _ = classify.classify(text) + if score == 0: + self.assertNotIn(cat, classify.CATS) + + def test_every_repro_is_labelled_explicitly(self): + # No string may be ambiguous: either it matched (score >= 1 and a + # `matched` reason) or it is the explicit fallback. A real category at + # score 0 is the silent default Q119 filed. + for text in REPRO: + with self.subTest(text=text): + cat, score, reason = classify.classify(text) + if score == 0: + self.assertEqual(cat, classify.UNCLASSIFIED) + else: + self.assertIn(cat, classify.CATS) + self.assertIn("matched", reason) + + def test_repro_with_a_rule_hit_is_classified_by_rule(self): + cat, score, reason = classify.classify( + "Triage the inbound queue before standup." + ) + self.assertEqual(cat, "general") + self.assertEqual(score, 1) + self.assertIn("standup", reason) + + def test_unclassified_is_counted_beside_real_categories(self): + self.assertNotIn(classify.UNCLASSIFIED, classify.CATS) + self.assertIn(classify.UNCLASSIFIED, classify.COUNTED) + self.assertEqual( + sorted(classify.COUNTED), + sorted(classify.CATS + [classify.UNCLASSIFIED]), + ) + + def test_matched_item_still_classifies(self): + # Control: a phrase that does match a rule is not swept into the + # fallback by the repair. "still renders" is an archive_fix rule. + cat, score, reason = classify.classify( + "The page still renders a dead clone URL." + ) + self.assertEqual(cat, "archive_fix") + self.assertGreaterEqual(score, 1) + self.assertIn("matched", reason) + + def test_unclassified_reason_names_no_category(self): + _, _, reason = classify.classify(ZERO_SCORE_REPRO[0]) + for cat in classify.CATS: + self.assertNotIn( + cat, + reason, + "fallback reason claims category %r: %r" % (cat, reason), + ) + + def test_aggregate_counts_unclassified_and_sums_to_items(self): + seed = { + "pending": [ + {"id": 1, "evidence": "README still renders a dead URL", + "proposed_action": "publish release notes"}, + {"id": 2, "evidence": "Ship the manifest to ops by Friday.", + "proposed_action": ""}, + {"id": 3, "evidence": "Roll back the ingress change.", + "proposed_action": ""}, + ] + } + with tempfile.TemporaryDirectory() as td: + seed_path = Path(td) / "seed.json" + out_path = Path(td) / "out.json" + seed_path.write_text(json.dumps(seed), encoding="utf-8") + proc = subprocess.run( + [sys.executable, str(HERE / "classify.py"), + "--seed", str(seed_path), "--output", str(out_path)], + capture_output=True, text=True, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + art = json.loads(out_path.read_text(encoding="utf-8")) + self.assertEqual(art["categories"].get("unclassified"), 2) + self.assertEqual(sum(art["categories"].values()), len(art["items"])) + by_id = {it["id"]: it for it in art["items"]} + self.assertEqual(by_id[2]["category"], classify.UNCLASSIFIED) + self.assertEqual(by_id[3]["category"], classify.UNCLASSIFIED) + self.assertIn(by_id[1]["category"], classify.CATS) + + def test_cli_refuses_empty_seed(self): + with tempfile.TemporaryDirectory() as td: + seed_path = Path(td) / "seed.json" + out_path = Path(td) / "out.json" + seed_path.write_text(json.dumps({"pending": []}), encoding="utf-8") + proc = subprocess.run( + [sys.executable, str(HERE / "classify.py"), + "--seed", str(seed_path), "--output", str(out_path)], + capture_output=True, text=True, + ) + self.assertEqual(proc.returncode, 1) + self.assertIn("no pending items", proc.stderr) + + def test_source_has_no_general_default_string(self): + src = (HERE / "classify.py").read_text(encoding="utf-8") + self.assertNotIn("defaulted to general", src) + self.assertIn("unclassified", src) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/wizard-work/sol/verify.sh b/wizard-work/sol/verify.sh index 54c5ba0..1a0526f 100755 --- a/wizard-work/sol/verify.sh +++ b/wizard-work/sol/verify.sh @@ -1,24 +1,105 @@ #!/usr/bin/env bash -# Verify the triage classifier artifact end-to-end. +# Verify the sol-lane triage classifier end to end. +# +# Q119: this script used to regenerate the committed `actual.csv` in place and +# diff it against `expected.csv`, so every run dirtied the tree it was grading +# and the "known-good" file was produced by the same rules it was checking. +# It now writes only into a throwaway directory and grades the JSON contract +# plus the committed fallback cells. Exit codes follow the repo contract: +# 0 clean, 1 a graded fault, 2 could not grade. set -u -cd "$(dirname "$0")" || exit 1 +cd "$(dirname "$(readlink -f "$0")")" || exit 2 -# 1) regenerate the artifact -python3 classify.py --input demands.json --output actual.csv --pretty -test $? -eq 0 || { echo "FAIL: classification run"; exit 1; } +if ! python3 --version >/dev/null 2>&1; then + echo "verify: python3 not available -- cannot grade" >&2 + exit 2 +fi -# 2) JSON summary assertion (no -c one-liner) -python3 - <<'PY' +SCRATCH="$(mktemp -d "${TMPDIR:-/tmp}/sol-classify.XXXXXX")" || { + echo "verify: cannot create scratch dir -- cannot grade" >&2 + exit 2 +} +cleanup() { rm -rf "$SCRATCH"; } +trap cleanup EXIT + +BEFORE_GIT="$(git status --porcelain -uno 2>/dev/null)" +IN_WORKTREE=0 +git rev-parse --is-inside-work-tree >/dev/null 2>&1 && IN_WORKTREE=1 +# Checksums of every artifact this directory ships, so the cleanliness leg is +# not vacuous outside a git worktree (a copy in /tmp is still graded). +snapshot() { + md5sum classify.py demands.json expected.csv actual.csv actual.json \ + Triage-2026-10-29.json seed-2026-10-29.json 2>/dev/null +} +BEFORE_MD5="$(snapshot)" + +# 1) the classifier runs on the committed seed and writes outside the tree +python3 classify.py --seed seed-2026-10-29.json --output "$SCRATCH/out.json" +rc=$? +if [ "$rc" -ne 0 ]; then + echo "FAIL: classification run exited $rc" + exit 1 +fi +if [ ! -s "$SCRATCH/out.json" ]; then + echo "FAIL: classifier wrote no artifact" + exit 1 +fi + +# 2) JSON contract: every item is labelled, the aggregate sums to the item count +python3 - "$SCRATCH/out.json" <<'PY' import json -d = json.load(open("actual.json")) -want = {"total": 8, "by_category": {"decision": 2, "info": 2, "action": 3, "done": 1}} -if d.get("summary") != want: - raise SystemExit("summary mismatch: %r" % (d.get("summary"),)) -PY -test $? -eq 0 || { echo "FAIL: summary assertion"; exit 1; } +import sys -# 3) CSV diff against known-good output -diff -u expected.csv actual.csv -test $? -eq 0 || { echo "FAIL: CSV diff"; exit 1; } +art = json.load(open(sys.argv[1], encoding="utf-8")) +cats = art["categories"] +items = art["items"] +real = {"archive_fix", "app_pr", "general"} +allowed = real | {"unclassified"} +problems = [] +if not items: + problems.append("artifact has no items") +if sum(cats.values()) != len(items): + problems.append("categories sum %d != %d items" % (sum(cats.values()), len(items))) +missing = allowed - set(cats) +if missing: + problems.append("categories missing %s" % sorted(missing)) +for it in items: + if it["category"] not in allowed: + problems.append("item %r has category %r" % (it.get("id"), it["category"])) + if it["decision"] not in {"approve", "table", "reject"}: + problems.append("item %r has decision %r" % (it.get("id"), it["decision"])) +ids = [it.get("id") for it in items] +if len(set(ids)) != len(ids): + problems.append("duplicate item ids") +if problems: + raise SystemExit("FAIL: " + "; ".join(problems)) +print("contract ok: %d items, categories=%s" % (len(items), json.dumps(cats, sort_keys=True))) +PY +if [ $? -ne 0 ]; then + echo "FAIL: JSON contract" + exit 1 +fi + +# 3) the committed cells (fallback contract, Q119) +python3 test_classify_fallback.py || { + echo "FAIL: fallback cells" + exit 1 +} + +# 4) the run must not have touched any artifact it grades +AFTER_MD5="$(snapshot)" +if [ "$BEFORE_MD5" != "$AFTER_MD5" ]; then + echo "FAIL: verify.sh changed the tree it grades" >&2 + printf 'before:\n%s\nafter:\n%s\n' "$BEFORE_MD5" "$AFTER_MD5" >&2 + exit 1 +fi +if [ "$IN_WORKTREE" -eq 1 ]; then + AFTER_GIT="$(git status --porcelain -uno 2>/dev/null)" + if [ "$BEFORE_GIT" != "$AFTER_GIT" ]; then + echo "FAIL: verify.sh changed the worktree it grades" >&2 + printf '%s\n' "$AFTER_GIT" >&2 + exit 1 + fi +fi echo "CHECK OK"