feat(report): require an explanation of why each rule matters

[theCodeDrift]

ReportV1 on tskl.es now carries an optional per-rule explain: plain prose on how a rule improves the codebase. This makes the agent supply one for every rule, so a published report says why each rule matters, not only what it enforces.

What changes

• Vendored ReportV1 regenerated from https://tskl.es/api/v1/schema. Adds explain on rules (1–1000 chars, non-blank) and a non-blank pattern on title.
• explain is required in the facts file on every rules, legacy, and cloud entry. ReportV1 keeps it optional; the CLI is stricter so no report it publishes has a rule without one. Missing, blank, or over-length is REPORT_INVALID. A blank title is now refused too, matching the schema.
• Assembly carries explain onto every published rule and screens it with the other agent-supplied strings, so a secret or contact detail refuses the publish under REPORT_REDACTION, naming the field.
• Report recipe (topic v6; main took v5 for the estimating-violations change) asks for explain on every rule and adds ## Explaining a rule: one or two sentences on what goes wrong without the rule and what holding it buys, grounded in the session, no counts, no names or hosts.
• The terminal summary is unchanged: it still lists title and tier only.

Delivery

Single PR; the OpenSpec change report-rule-explain is archived here. The delta restates three cli-report requirements in full; all 50 prior scenarios survive the archive, plus 3 new ones. report is unreleased (0.12.0 predates it), so this extends the existing report changeset as a patch rather than adding one.

Verification

pnpm typecheck, pnpm lint, and the full CLI suite (2382 tests) pass.

[theCodeDrift]

@claude /review

[github-actions]

Claude finished @theCodeDrift's task in 2m 56s —— View job

---

Review complete

• Read .prior-review.json (no prior threads/reviews — this is the first review)
• Gather PR diff and metadata
• Review schema/type changes
• Review facts validation (report-facts.ts)
• Review assembly/redaction changes (assemble.ts)
• Review report recipe (report.md)
• Review OpenSpec archive + changeset
• Review tests
• Post final review

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained no review threads, review summaries, or comments beyond the trigger comment itself, so this is the first substantive review of the PR; everything below is assessed fresh.

Overall: Solid, well-scoped change. explain is threaded consistently through the facts schema (report-facts.ts), the vendored ReportV1 schema, assemble.ts (merge + redaction screening), types.ts, the recipe (report.md, topic v5), and the test suite across all three tiers (rules/legacy/cloud). Specifics I checked:

• explain is required (non-blank, 1–1000 chars via .min(1).max(1000).regex(/\S/)) in the facts schema but left optional in the vendored ReportV1 schema (report-v1.schema.json, required array at the rule object does not include explain) — matches the stated "ReportV1 keeps it optional; the CLI is stricter" design. schemaVersion correctly stays "1" since adding an optional field is non-breaking.
• screenStrings in assemble.ts correctly adds explain to the fields scanned for secrets/contact details for all three groups (rules, legacy, cloud), and is covered by new tests (legacy[l1].explain, cloud[c1].explain).
• The terminal summary (summary.ts) is untouched and still only renders title/tier, matching the PR's claim that the summary is unchanged — explain never reaches telemetry either, since cli_report_published only ever sends tier counts (commands/report.ts), not rule content.
• The OpenSpec archive restates all three cli-report requirements in full under matching titles (verified by diffing the archived delta against openspec/specs/cli-report/spec.md), so none of the pre-existing scenarios were silently dropped during archiving. Scenario count in the live spec is 53, matching the PR's "50 prior + 3 new" claim.
• The changeset correctly extends the existing unreleased report entry as a patch rather than adding a new changeset or bumping minor, consistent with report being unreleased.

[New] Minor test-coverage gap: the PR description says "a blank title is now refused too, matching the schema," and packages/cli/src/schemas/report-facts.ts:46 does add .regex(/\S/) to title (previously only .min(1).max(200)). However, unlike explain — which gets a dedicated it.each covering missing/empty/blank/over-length (packages/cli/test/report.test.ts) — there's no test exercising a whitespace-only title. The implementation looks correct (same pattern as explain), so this is low severity, but worth a follow-up test to lock in the behavior the PR body explicitly calls out.

No other correctness, security, or clarity issues found in the diff.

[theCodeDrift]

Re: @github-actions — "Claude finished @theCodeDrift's task in 2m 56s … Minor test-coverage gap: … no test exercising a whitespace-only title"…
#525 (comment)

Added in 24ada17: an it.each refusing an empty, blank, or over-200-character title, beside the explain cases. The blank case is the one that exercises the new /\S/, since " " already passes min(1).

— AI Coding Agent