From 864e7c689c0342a7fb73f9de9bc5c9152ec3df40 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Sat, 8 Aug 2026 20:54:19 +0000 Subject: [PATCH] docs(lessons): record three patterns from PR #85's review rounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three rounds of review on the Vale prefilter surfaced patterns worth keeping rather than just fixing. The first has now recurred three times in a single PR — a check reporting success because it had silently not run — so it is flagged as a graduation candidate per LESSONS.md's own three-instance rule. - A clean linter result can mean "nothing was checked": the frontmatter scope silently not matching, warning-level rules never affecting an exit code, and globs matching zero files all produced green results that were then cited as evidence of cleanliness. - One signal, two consumers, no named distinction: Vale severities were tuned for the audit report while the commit gate silently inherited the resulting exit code, because CONTEXT.md described both as one mechanism. - Measure a rule's false-positive rate at the severity you will ship it at: VagueQualifier was trialled at warning, where a false positive is free, and shipped at error, where it costs a blocked commit. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MCQ648fLSFXPHGZdQ8gn58 --- LESSONS.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/LESSONS.md b/LESSONS.md index b81c9d8..7925c4e 100644 --- a/LESSONS.md +++ b/LESSONS.md @@ -137,3 +137,15 @@ After a PR merge (with Gitea's default auto-delete-branch behavior), `git branch ## 2026-05-18 — Planning meta-commentary does not belong in deployed artifacts During write-skill refactor, an "open thread" note (about a deferred research step) was written directly into the SKILL.md Process section. The user caught it. The rule it violated: a deployed artifact (SKILL.md, a runtime file loaded by agents) must not contain planning meta-commentary — deferred items, open threads, and implementation notes belong in the issue file, which is the planning artifact. The skill body should contain only content relevant to runtime execution. If a decision is deferred, record it in the issue and leave no trace in the skill. The distinction: issue = planning record; skill = executable instruction. + +## 2026-08-08 — A clean linter result can mean "nothing was checked" + +Three separate times in one PR (#85), a check reported success because it had silently not run. (1) Vale's `text.frontmatter.description` scope stops matching once the value is a multi-line YAML block scalar — the style most skills here use — so a repo-wide sweep returned 0 alerts across 49 files and was read as a clean repo. (2) Five of six rules were `level: warning`, but Vale's exit code keys on `error` alone and pre-commit hides output from passing hooks, so those rules were invisible and blocked nothing for two review rounds while the ADR described them as "enforcing immediately." (3) `.vale.ini`'s globs matched no file outside `plugins/`, so Vale printed "0 files" and exited 0, which both audit skills read as "no findings" and used to skip their own judgment passes. Each time the green result was worse than no check at all, because it was cited as positive evidence of cleanliness. Fix: for any new check, prove it fails before trusting that it passes — run it against a deliberately-bad fixture, confirm the failure, then run the real corpus. Where a check can scan zero inputs, assert on the input count, not just the exit code. **Graduation candidate:** three instances of one pattern. + +## 2026-08-08 — One signal, two consumers, no named distinction + +Vale's output fed two consumers with different contracts: the audit skills read severity *strings* to grade a report (`error`→FAIL, `warning`→SUGGESTION), while the pre-commit hook read the process *exit code* to allow or block a commit. Severities were tuned for the first consumer; the second silently inherited whatever exit code that produced, which was always 0. CONTEXT.md described both as a single mechanism under one heading, which is precisely why the divergence went unnoticed — there was no vocabulary in which "the gate" and "the prefilter" were different things that could disagree. Fix: when one output feeds two consumers, name them separately in the domain language and state each contract explicitly. If they cannot be given independent contracts, collapse them into one — which is what happened here: every rule became `level: error`, so the gate and the audit now share a single verdict with nothing to keep in sync. + +## 2026-08-08 — Measure a rule's false-positive rate at the severity you will ship it at + +`Kyberforge.VagueQualifier` was cherry-picked from `write-good` after being trialled as "low-noise against this repo's corpus" — but the trial ran at `level: warning`, where a false positive costs nothing because nobody ever sees it. Shipped at `error`, the same false positive costs a blocked commit and a permanent suppression comment. Re-measured at the severity it actually shipped at, the rule scored one marginal true positive and one unfixable false positive across 41 files (`caveman/SKILL.md` *quotes* filler words as its subject matter — a mention, not a use), and was deleted. Fix: trial conditions must match shipping conditions. A noise measurement taken where false positives are free does not transfer to a context where they are expensive, and "low-noise" is not a property of a rule alone — it is a property of the rule at a severity.