P0 learning quality: mechanical emitters + sheal consolidate (T13–T15, T17)

[luisalima]

Supersedes #46, which was auto-closed when its stacked base (#45) merged and the branch was deleted — the pr-tutor explainer and qa-personas report on #46 still apply to this diff (plus the two fixes below they asked for).

T14 (Compile mechanical learnings into hooks and checkers) — emitter v0

• sheal check --strict — warnings exit non-zero (compiles LEARN-007).
• sheal guard pr — pre-PR gate: working branch, ahead of base, uncommitted-changes warning (compiles LEARN-038/042). Built for hooks/CI.
• Compiled learnings marked status: superseded in the store with pointers to their artifacts.

T13 (sheal consolidate: emit a reviewable change set)

• Mechanical stage from learn lint primitives → dated proposal in .sheal/consolidation/, never mutates the store; --format json; --prompt emits the LLM judgment-stage prompt.
• Superseded/retired learnings excluded from live-rule lint checks (dogfood fix, red-first tests).

T15 + T17 (fixes from the #46 tutor pass)

• T15: lint findings carry a numeric similarity field; the change set consumes it instead of regex-scraping a message (test pins it end-to-end).
• T17: one shared resultHasWarnings predicate drives both summary.warnings and the --strict exit — no more exit 1 with warnings: 0.

Review trail

• pr-tutor explainer: P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 (comment)
• qa-personas: 20/20 deterministic probes passed, no breach; advisory findings in its P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 comment (empty-commit bypass, --skip neutralizing strict, --prompt injection surface) tracked for follow-up.
• T16 (wire gates into hooks/CI) filed as todo — depends on disposition execution.

Testing

13 new tests across the stack, red-first per task; full suite 333/333, lint 0 errors. Verified on the real store (12 collisions / 3 merge candidates / 2 dispositions, store untouched).

Task trail: docs/tasks/t13…t17

🤖 Generated with Claude Code

[luisalima]

🧑‍🏫 PR Tutor — human-review pass (delta: #46 → #47)

0. What this comment is — and what it verified

This is a delta pass, not a from-scratch one. The full tutor explainer lives on the superseded PR: #46 explainer — read that first if you're cold; everything in it still applies to this diff, because:

  refs/pull/46/head  (f0f562d3)  tree 86beb1a0 ─┐
                                                 ├─ identical trees
  refs/pull/47/head  (c5715c12)  tree 86beb1a0 ─┘
  …and their parents' trees are identical too (bd92f9f2)

So #47 is byte-for-byte the final #46 content, rebased onto main after the stacked base (#45, learn lint) merged. The #46 explainer was posted at 15:04Z, before the final commit was authored (15:23Z) — so the one commit that no tutor pass has covered is:

c5715c12 — T15 + T17: tutor-pass fixes — structured similarity, unified warning predicate

That commit is exactly what this comment explains. It answers the #46 explainer's loose ends 1 and 2 and files the three follow-up task stubs (T15/T17 done, T16 todo) as real task files.

1. TL;DR (delta only)

Two plumbing fixes, both "stop deriving facts from prose": the near-duplicate similarity score now travels from learn lint to the change set as a structured numeric field instead of being regex-scraped out of a human-readable message, and one shared predicate now defines "this checker carries warnings" for both the JSON report summary and check --strict's exit code — so strict can no longer exit 1 while the JSON summary says warnings: 0.

2. Picture it

Before → After — similarity (T15, loose end 1):

  BEFORE                                     AFTER
  lint finding:                              lint finding:
    message: "…(similarity 0.52) — merge…"     message: "…(similarity 0.52)…"
         │                                     similarity: 0.52          [NEW]
         │ regex /similarity (\d+\.\d+)/            │ read the field
         ▼                                          ▼
  change set: similarity: "0.52" (string)    change set: similarity: 0.52 (number)
  reword the message → "n/a", silently,      reword the message → still 0.52;
  no test notices                            test pins `typeof === "number"`

The score is data with a contract now; the prose message is just presentation.

Before → After — the warning predicate (T17, loose end 2):

              resultHasWarnings(r)                    [NEW: src/output/json.ts]
              = r.severity === "warn"  OR  any detail.severity === "warn"
                     │                                │
                     ▼                                ▼
         JSON summary.warnings / .passed      --strict exit decision
         (buildSummary, json.ts)              (runCheck, check.ts)

  BEFORE: two hand-rolled, disagreeing definitions
          → CI sees exit 1 next to a report saying "warnings": 0
  AFTER:  one predicate → JSON summary and exit code always agree
  ⚠ but: pretty (default terminal) output still counts its own way
         → see delta loose end 1 below

One predicate, two consumers — but only two of the three output surfaces were unified.

3. Toy example — one checker result through the new summary

  checker result:  { name: "environment", severity: "pass",
                     details: [ {msg: "gh available",        severity: "pass"},
                                {msg: "gh not authenticated", severity: "warn"} ] }

  BEFORE:  summary → { passed: 1, warnings: 0 }   sheal check --strict → exit 1
           (CI red, report green — the #46 finding)
  AFTER:   summary → { passed: 0, warnings: 1 }   sheal check --strict → exit 1
           (report and exit code tell the same story)

Note the deliberate semantic change hiding in there: summary.passed now excludes pass-severity checkers that carry warn details. And the counts are no longer a partition of total — a fail-severity checker with a warn detail is counted in both failed and warnings. Any downstream consumer that assumed passed + warnings + failed + skipped === total is now wrong (delta loose end 2).

4. Walkthrough (delta files only)

The two fixes:

• src/learn/lint.ts:15 — optional numeric similarity field on LintFinding; set at lint.ts:130 for near-duplicates (Number(similarity.toFixed(2)), so it matches the message rendering).
• src/consolidate/change-set.ts:20,34 — ChangeSet.mergeCandidates[].similarity retyped string → number; the regex scrape is deleted, replaced by f.similarity ?? 0.
• src/output/json.ts — resultHasWarnings (the shared predicate) and buildSummary extracted and exported; passed/warnings redefined through the predicate.
• src/commands/check.ts:52 — the strict exit decision now calls resultHasWarnings instead of its own inline copy.

Tests (3 new, per the red-first claim):

• test/check-summary.test.ts — new: a pass-severity checker with a warn detail counts as a warning, not a pass, and the predicate agrees with itself across both call sites.
• test/consolidate.test.ts:99-102 — pins similarity as a number in [0.4, 1] end-to-end through the real CLI.

Docs:

• docs/tasks/t15-…, t16-…, t17-… + TASK_INDEX.md — the three stubs the P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 pass drafted, filed as task files. T15/T17 done, T16 todo.

5. Does it meet "Done when"? (T15, T17)

• T15 — "change set carries a numeric similarity that survives message rewording, proven by a test that failed before" — ✅ in substance: the field is structured and the consolidate test asserts typeof "number" through the CLI. Two honest caveats: ⚠ the test doesn't literally reword the message (but the scraping code is gone, so rewording has nothing left to break), and ⚠ "failed first" isn't provable from a single squashed commit — it rests on the PR body's claim, same as the P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 pass noted for T13/T14.
• T17 — "summary.warnings and --strict agree on the same input, proven by a test that failed before" — ✅ for the JSON summary (shared predicate + check-summary.test.ts); ⚠ same red-first caveat; ⚠ and the pretty summary was left out — see below.

6. Loose ends & red flags (new in this delta)

1. 🟡 Pretty output still uses the old warning definition — src/output/pretty.ts:36-37 counts by top-level r.severity only. check --strict in a terminal (pretty is the default format) can still exit 1 while the printed summary line shows warn-detail checkers under "N passed" with no warnings count. T17's done-when says "summary" and the JSON summary is fixed, so this isn't a broken promise — but it's the same confusion, one output surface over, and CI logs often capture pretty output too.
2. 🟡 summary semantics changed without a note — passed shrinks (warn-detail checkers leave it) and warnings can now overlap failed/skipped, so the counts no longer sum to total. Correct per T17's intent, but it's a small output-contract break with no README/CHANGELOG mention; anything parsing summary.passed sees numbers move on upgrade.
3. 🟡 PR body vs. repo: qa-personas advisory findings are not actually tracked — the body says the P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 qa findings (empty-commit bypass of guard pr, --skip/committed-config neutralizing --strict, --prompt injection surface) are "tracked for follow-up", but the only task files are t13–t17 and none covers them (t16 is about wiring the gates, not hardening them). Surfacing the conflict rather than guessing: either a task exists somewhere I can't see, or "tracked" currently means "mentioned in a closed PR's comment".
4. 🟢 Provenance points at the closed PR — t15/t17 frontmatter links name PR P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46, not P0 learning quality: mechanical emitters + sheal consolidate (T13–T15, T17) #47. Harmless (P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 links here), but the trail takes one extra hop. Same for TASK_INDEX flipping T15/T17 done pre-merge — established repo convention per the P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 pass.
5. 🟢 f.similarity ?? 0 — a missing field coerces to 0. Can't happen today (only near-duplicate findings reach that map and they always set it), so just know that a literal 0 similarity would mean an upstream bug, not "0% similar".

7. Still open from the #46 pass — do not let these die with the closed PR

These #46 findings are not addressed by this delta and now live only in a closed PR's comment (plus, for the first one, a task file):

• Opt-in enforcement (P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 loose end 3) — nothing in the repo invokes guard pr or check --strict automatically. Now tracked as docs/tasks/t16-wire-guards-into-hooks-and-ci.md (todo) — the tracking landed in this PR; the enforcement did not.
• guard pr defaults --base main (part of P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 loose end 3) — a hook running it verbatim misfires on stacked branches (this PR's own history is the example). Called out inside T16's body; still true in the code.
• Supersede provenance lives only in the gitignored store (P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 loose end 4) — LEARN-007/038/042 are marked superseded only on machines that sync the store; the repo itself carries no trace. No task file exists for this one — it's the P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 finding with zero tracking artifact (stub below).
• Minor, unchanged: consolidate --global output path and guard pr on detached HEAD remain untested (P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 loose end 7).

8. Suggested follow-up tasks (drafts — numbers provisional, max existing id across branches is T17)

📋 T18 — Unify pretty output's warning count with resultHasWarnings
   why:        pretty (default) summary still counts top-level severity only — strict can exit 1 while the terminal shows no warnings (delta loose end 1)
   done when:  pretty and JSON summaries report the same warning count for a pass-with-warn-details checker, with a test; the passed/warnings semantic change is noted in README or CHANGELOG (delta loose end 2)
   deliverable: P0 · depends on: #47 (T17 surface)

📋 T19 — Decide and implement git-visible supersede provenance
   why:        compiled learnings are marked superseded only in the gitignored store; other machines still carry them as active prose and the repo has no record (#46 loose end 4 — currently untracked)
   done when:  a decision (ADR or question resolution) exists, and if adopted, a supersede event leaves a committed trace (e.g. a ledger in docs/) that survives without store sync
   deliverable: P0 · depends on: #47 (T14 convention)

📋 T20 — Harden the T14 gates against the qa-personas bypasses
   why:        PR body says the #46 qa advisory findings are tracked, but no task covers them: --allow-empty commits satisfy guard pr; a committed .self-heal.json skip list silently neutralizes a bare `check --strict` in CI; consolidate --prompt interpolates learning bodies unfenced (delta loose end 3)
   done when:  each finding has an explicit disposition (fix with a red-first test, or a documented accepted-risk note); the committed-skip-defeats-strict case in particular is decided
   deliverable: P0 · depends on: #47 (T14/T13 surfaces)

Say "file these" (or "file task 2") and the main session will create them via the opentasks skill — I don't create task files myself.

9. Verify by hand (delta-focused; the #46 checklist still applies)

• git fetch origin refs/pull/47/head && git checkout FETCH_HEAD && npx tsc && npx vitest run — confirm the 333/333 claim yourself (I verified trees and read the diff; I did not execute the suite).
• sheal consolidate --format json | jq '.mergeCandidates[0].similarity | type' on the real store → should print "number".
• Trip loose end 1 live: in a repo where a checker passes with a warn detail (e.g. gh installed but unauthenticated), run sheal check --strict (pretty) vs sheal check --strict --format json — compare what each says against the shared exit code.
• If anything of yours parses summary.passed or assumes the counts partition total (dashboards, scripts, hooks) — eyeball it before merging.
• Judgment call on delta loose end 3: is "advisory findings in a closed PR comment" enough tracking for the --skip-defeats-strict case, given strict is aimed at CI?

10. Check your understanding (optional — skip freely, or say "quiz me")

1. After this delta, sheal check --strict exits 1 but the terminal shows no warnings anywhere. How is that still possible?
2. Why can passed + warnings + failed + skipped now exceed total in the JSON summary — and why is that arguably correct?
3. T15's test never rewords the lint message. Why does it still protect against the failure mode the P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 pass flagged?

---

1. The default output format is pretty, and src/output/pretty.ts still counts warnings by top-level checker severity only — the shared predicate was wired into the JSON summary and the exit code, not the pretty summary (delta loose end 1).
2. warnings is now "carries a warning anywhere" (checker- or detail-level), which can overlap failed/skipped; the categories answer different questions (outcome vs. attention-needed), so overlap is coherent — but it silently breaks the old partition contract (delta loose end 2).
3. The failure mode was the regex scrape degrading to "n/a" on wording drift; that code path was deleted entirely, so there is nothing left for a reworded message to break — the test instead pins that a numeric value flows end-to-end, which fails if anyone reintroduces string plumbing.

---

Tutoring pass, not an approval or a safety claim. It surfaces what changed and what to check by hand; the human reviewer decides.

[luisalima]

🧪 qa-personas — black-box QA, delta run (advisory, non-blocking)

A full qa-personas pass already ran against PR #46 (this branch minus the T15 + T17 commit): 20/20 deterministic oracle probes passed, no breach — full report: #46 (comment). This run is scoped to the delta commit c5715c1 ("T15 + T17: structured similarity, unified warning predicate") plus regression of the previously-passed invariants adjacent to the changed files (src/learn/lint.ts, src/consolidate/change-set.ts, src/output/json.ts, src/commands/check.ts).

The CLI was built (npx tsc) from the PR head in an isolated git worktree and driven black-box (node dist/index.js …) from mktemp sandboxes with HOME redirected. The real repo's .sheal store and git state were never used as a target.

⚠️ Method notes (read first)

• Cross-vendor requirement not met (same as the P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 run): no opposite-vendor agent CLI (codex/amp/gemini/…) is available in this environment. Per the skill, I ran the deterministic oracle probes myself — they are mechanical and fully reproducible below, so the hard signal is unaffected.
• Personas were not re-run. This is a delta run; the P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 persona transcripts cover the unchanged surface. The delta is contract-shaped (JSON field type, summary/exit agreement) and fully covered by the oracle.
• The orchestrator read the diff to aim the oracle (permitted by the skill); there is no persona in this run to keep cold.
• .claude/skills/qa-personas/USER.md (the repo-local probe catalog/bindings the skill references) does not exist — the probe catalog below was derived from the delta scope and the P0 learning quality: mechanical emitters + sheal consolidate (T13, T14) #46 report's probe set.
• The repo's own suite passes on the PR head: npx vitest run → 333/333 (includes the new T15 end-to-end similarity test and T17 detail-level-warn summary test, which pin the one path a black-box fixture can't isolate: a pass-severity checker carrying warn-level details).

---

Deterministic oracle — hard signal (all reproducible by hand)

16/16 passed. No deterministic breach. Canary token generated per run (QA-CANARY-1783355195-2917).

Delta probes (new behavior in c5715c1):

#	Probe	Result
D1	consolidate -f json on a store with a near-duplicate pair: mergeCandidates[0].similarity is typeof number and in [0, 1] (observed 0.43)	✅ pass
D2	learn lint -f json: near-duplicate finding carries numeric similarity equal to the value in its own human-readable message and equal to consolidate's mergeCandidates[].similarity (0.43 end-to-end)	✅ pass
D3	Clean repo: check -f json summary.warnings == 0 and check --strict exit 0 (agree)	✅ pass
D4	Warn repo (untracked file): summary.warnings == 1 (> 0), plain check exit 0, --strict exit 1 (agree)	✅ pass
D5	check --strict -f json on warn repo: exit 1 with summary.warnings = 1 — the #46 "exit 1 with warnings: 0" disagreement is gone	✅ pass
D6	Warn repo counts: passed(2) + warnings(1) + failed(0) + skipped(0) == total(3)	✅ pass
D7	Fail repo (conflict markers): plain exit 1 and summary.healthy == false	✅ pass

Regression probes (re-run from the #46 set — store immutability & malformed input against the changed files):

#	Probe (#46 equivalent)	Result
R11	consolidate pretty: exit 0, dated change-set file written, store bytes unchanged (sha256 over all store files)	✅ pass
R12	consolidate -f json + learn lint -f json: store unchanged, no consolidation file written in json mode	✅ pass
R13	consolidate --prompt: canary flows through to the prompt, exit 0, store unchanged	✅ pass
R14	Empty store dir: exit 0, scanned=0	✅ pass
R15	Missing store (json): exit 0, scanned=0, no .sheal/ created	✅ pass
R16	Malformed learning file (no frontmatter): consolidate non-zero exit, no raw stack, store unchanged	✅ pass
R16b	Same malformed store via learn lint -f json: non-zero exit, no raw stack	✅ pass
R17	Frontmatter injection in a learning body (embedded --- id: LEARN-777 --- block): scanned=1, no phantom LEARN-777 anywhere in the JSON, store unchanged	✅ pass

Skipped (stated per the skill, no silent gaps): the #46 guard pr probes (P4–P10) — src/commands/guard.ts is untouched by the delta commit; the 2 MB-body perf probe (P18) — parsing/lint internals unchanged apart from the added field.

---

Verification of the #46 advisory ("healthy: true while exit 1 under strict")

Partially resolved, and the remaining half looks intentional:

• ✅ Resolved: summary.warnings and --strict's exit decision now share one predicate (resultHasWarnings). Exit 1 under strict now always comes with summary.warnings > 0 in the same JSON (D4/D5) — the report explains the exit code.
• ℹ️ Persists (advisory, by design): summary.healthy is still true when --strict exits 1 on warnings — healthy remains fail-only (results.every(severity !== "fail")). With warnings now visible and consistent this is defensible (healthy = "no failures", strict adds warnings on top), but a strict-mode consumer keying on healthy alone will still be surprised. Consider documenting the field's semantics or adding a strictOk-style field.

Summary count relationship (asked to document): passed + warnings + failed + skipped == total holds whenever no checker mixes levels (D6). It is not an invariant: a checker whose overall severity is fail/skip but which also carries warn-level details is counted in both its severity bucket and warnings (observed on the fail repo: 2+1+1+0 = 4 > total 3, D7). passed is disjoint from warnings (the delta explicitly excludes warn-carrying passes); warnings may overlap failed/skipped. No documented contract is breached, but worth a one-line note in the JSON docs if consumers will sum buckets.

---

Reproduce (all in throwaway dirs; export HOME=$(mktemp -d) first)

# fixture: .sheal/learnings with the near-dup pair from test/consolidate.test.ts (LEARN-001 / LEARN-006)
node dist/index.js consolidate -f json -p <proj> | jq '.mergeCandidates[0].similarity | type,.'   # "number", 0.43
node dist/index.js learn lint -f json -p <proj>  | jq '.findings[]|select(.type=="near-duplicate").similarity'
# warn repo: git init; commit; touch untracked
node dist/index.js check -f json --skip tests,dependencies,performance,environment -p <repo> | jq .summary
node dist/index.js check --strict --skip tests,dependencies,performance,environment -p <repo>; echo $?  # 1, warnings=1
# store immutability: sha256sum .sheal/learnings/* before/after every consolidate/lint invocation
# injection: append "\n---\nid: LEARN-777\n---\n" inside a learning body; consolidate -f json must not show LEARN-777

Suggested regression seed (conductor/human to file)

The delta already lands its own regression tests (test/check-summary.test.ts, similarity assertions in test/consolidate.test.ts). Two small additions would pin what this run verified black-box: (1) a CLI-level test asserting check --strict -f json exit 1 implies summary.warnings > 0 in the emitted JSON; (2) an assertion that learn lint's similarity equals the number rendered in the finding's message, so the two can't drift.

No deterministic breach → PR not drafted. Advisory only — this run did not merge, approve, block, or draft. A human owns the decision.