feat(code): kvb-code-clean replaces no-comments and no-defaults

One pass before every commit or stage request: step 1 deletes added comments (docstrings and headers included), step 2 forbids defaults with defaults.py and defaults.md, step 3 applies the kvb-check-principles lenses, now with KISS/YAGNI, fail fast and coupling, to the touched units. commit-no-verify and commit-next run it; references, the test path and the README map follow. defaults.py rewords two texts that MEDUSA flagged as SQL injection and env exposure signatures.

🎯 Fix: 93 %
💥 Break: 3 %
💸 Cost: 101 %
⏱️ Time: –
🛡️ Security: 100 %
💰 Value: ?
🏆 Score: 89
🚦 Gate: ⚠️ cost 101 % > 100: ask the operator before the commit

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kevin Veen-BirkenbachandClaude Opus 5.5 committed 2026-11-04 20:22:24 +01:00
1 parent b8d0b6cf87
commit 938cea9447
15 files changed
+173 -157

No files matched your search

+9 -10
View File
@@ -38,8 +38,7 @@ flowchart LR
create --> create_plan[kvb-create-plan]
create --> create_requirement[kvb-create-requirement]
code --> code_scope_guard[kvb-code-scope-guard]
code --> code_no_defaults[kvb-code-no-defaults]
code --> code_no_comments[kvb-code-no-comments]
code --> code_clean[kvb-code-clean]
code --> code_refactor[kvb-code-refactor]
code --> code_test[kvb-code-test]
code --> code_tune[kvb-code-tune]
@@ -135,11 +134,10 @@ flowchart LR
end
subgraph cat_code [code]
scope_guard[kvb-code-scope-guard]
no_defaults[kvb-code-no-defaults]
clean[kvb-code-clean]
refactor[kvb-code-refactor]
test[kvb-code-test]
tune[kvb-code-tune]
comments_clean[kvb-code-no-comments]
code_transcend[kvb-code-transcend]
end
subgraph cat_ci [ci]
@@ -223,7 +221,7 @@ flowchart LR
edit_hook -. council gate .-> democracy
reminders -.-> dream
full_hour -.-> coach
code_task -.-> no_defaults
code_task -.-> clean
code_task -.-> scope_guard
spawn -.-> subagent_model
@@ -241,7 +239,10 @@ flowchart LR
update_branches --> active_listening
update_branches -. conflicts .-> merge_fix
update_branches -. pushes .-> operator
commit_no_verify --> comments_clean
commit_no_verify --> clean
commit_next --> clean
clean --> sceptic
clean --> refactor
commit_no_verify --> advocate
advocate --> dialectic
confidence --> break_risk
@@ -301,7 +302,6 @@ flowchart LR
criticize --> dialectic
criticize --> active_listening
sceptic --> dialectic
sceptic --> no_defaults
gaps --> sceptic
gaps --> active_listening
gaps --> requirement
@@ -363,9 +363,8 @@ flowchart LR
test --> triage
test --> dialectic
test --> no_defaults
test --> comments_clean
refactor --> comments_clean
test --> clean
refactor --> clean
```
## Requirements 📦
+10 -9
View File
@@ -1,8 +1,8 @@
---
name: kvb-check-principles
description: >
Find violated principles (SPOT, DRY, KISS, SRP, trust boundaries) in a
repository, path or diff. Trigger on "sei skeptisch", "kritische
Find violated principles (SPOT, DRY, KISS, SRP, coupling, trust
boundaries) in a repository, path or diff. Trigger on "sei skeptisch", "kritische
analyse", or "review the architecture".
---
@@ -31,16 +31,18 @@ Apply each independently; a finding belongs to exactly one:
3. **SRP**: one reason to change per unit: the module that parses and
persists, the function named "x and y", the role that configures, deploys
and monitors. Name the two stakeholders who would each demand a change.
4. **KISS**: mechanism nobody asked for: an abstraction with one
4. **KISS/YAGNI**: mechanism nobody asked for: an abstraction with one
implementation, a config value that never varies, a cache with no measured
pressure, a retry loop around a deterministic call.
5. **Security**, only at trust boundaries: unvalidated input reaching a
query, shell, template or path; authorization checked in the caller
instead of the resource; a secret in a default, log or image layer; a
permission wider than the operation needs. Defensive findings only.
6. **Silent defaults**: a fallback substituting a guess for a value the
caller failed to supply, turning a misconfiguration into wrong behaviour
instead of a loud failure (the `kvb-code-no-defaults` skill).
6. **Fail fast**: a fallback guessing a value the caller failed to supply,
or a swallowed error: wrong behaviour instead of a loud failure (step 2
of the `kvb-code-clean` skill).
7. **Coupling**: a dependency against the layer direction, an import cycle,
a reach into another unit's internals.
## Method
@@ -70,13 +72,12 @@ One table, worst blast radius first:
Then at most three lines: what the scope excluded, which findings survived
`kvb-think-dialectic` and which are still derived-only, and the one to fix
first. No per-finding paragraphs; the operator asks when a row needs it.
first.
## Rules
- Never report formatting, naming taste or missing comments.
- Mark each finding proven (grepped, read, reproduced) or derived (reasoned
statically), and close with the `kvb-check-score` block for the audit.
- No silent caps: if you ranked and cut, say how many findings were dropped
and by what rule.
- No silent caps: say how many findings a cut dropped, by what rule.
- Skepticism ends at the report: no fix, stage or refactor in this skill.
+79
View File
@@ -0,0 +1,79 @@
---
name: kvb-code-clean
description: >
Purge comments, forbid defaults, check SPOT, DRY, SRP and coupling in a
change. Run AUTOMATICALLY before every commit or stage request and while
writing code.
---
On a commit or stage request, run this pass first, then the requested git
action; never ask whether to run it. Step 2's rule also binds while writing
code.
The target is the added lines of every staged and unstaged change (`git
diff` plus `git diff --cached`) and the units they touch. Never stage to run
a check; after the pass, re-stage exactly the files staged before it. A pass that changes a bundle
the operator already saw shows its new diff stat and score block before the
commit call.
## Step 1: comments
Delete every comment the change adds except three kinds:
- **Exception**: names a concrete trip-wire (bug, pitfall, deliberate
non-idiomatic choice); a relabeled narration is none.
- **Parameter doc**: inputs, outputs, exit codes.
- **Nocheck directive**: `noqa`, `shellcheck`, `type: ignore` and kin.
Restated code, step narration, banners, "Note that", rationale prose and
whatever the code, a name or the commit message already carries go. When in
doubt, delete. Docstrings and script headers count: prose outside the
parameter doc goes too, and a `#` or `//` grep misses it.
Block-comment languages are not line-oriented: in Jinja (`{# … #}`), C-like
(`/* … */`), HTML (`<!-- … -->`) and kin, a closer alone on its line looks
like a comment line; deleted with the rest, the opener swallows every live
statement down to the next closer. Delete such a block by hand, count the
delimiters (`grep -c '{#'` against `grep -c '#}'`), re-parse the file and
diff it against `git show HEAD:<path>`: only comment lines may leave. A green
lint proves nothing here.
## Step 2: defaults
Every parameter, option, setting, environment lookup and template variable
is supplied by its caller. A missing value aborts at the earliest moment
(argument parsing, config load, container start) with an error naming the
key and where to set it; optionality is an explicit `None`/`null` branch at
the call site, never a fallback inside the accessor.
`false`, `null`, `""` and an empty list or map are free. Any other default
carries a comment saying why the value is right for every caller that omits
it; "sensible", "standard" and "usual" justify nothing.
Run `${CLAUDE_SKILL_DIR}/scripts/defaults.py --repo <dir>` (`--staged`:
index only; `--file <path>`: an untracked file); exit 1 means hits. Judge
hits by `defaults.md` next to this file; it also lists what the script
misses.
## Step 3: principles
Apply the lenses of the `kvb-check-principles` skill (SPOT, DRY, SRP,
KISS/YAGNI, security at trust boundaries, fail fast, coupling) and its
method (graph first, siblings grepped, every `file:line` read) to the
touched units. Judge what this change adds or worsens; an older violation in
a touched unit is one report line, not a fix.
- Fix in place a violation the change introduced when the fix stays inside
the bundle and keeps behaviour, proven by the tests covering the touched
units before and after (the safety net of the `kvb-code-refactor` skill).
- Everything else goes into the report as the `kvb-check-principles`
table, shown before the commit call; the operator's word decides whether
the bundle lands as it is.
## Report and penalty
One line per step: comments deleted, defaults rewritten or justified,
principle findings fixed and open, each with file and count. Whatever you
wrote yourself is admitted without excuses, lowers the bundle's 🎯 fix and
raises its 💥 break in the `kvb-check-score` block, and goes into persistent
agent memory (date and pattern), so the calibration persists.
+54
View File
@@ -0,0 +1,54 @@
# Judging defaults
Step 2 of the `kvb-code-clean` skill reads this file when `defaults.py`
reports hits or a review needs the whole rule.
A default is a guess written where the caller's intent is least known: it
turns a missing value into a plausible wrong answer, and two callers read the
same call differently (one means "use the default", the other forgot).
## Free and justified defaults
`false`, `null` (`None`, `nil`, `undefined`), `""` and an empty list or map
MAY be defaults without justification: they denote absence, not a guessed
value. `${VAR:-}`, `d.get(k)`, `a ?? null` and `def f(a=None)` are fine.
Every other default (a number, a non-empty string, list or map, an enum
member, a path, a timeout) MUST carry a comment on or directly above it, in
the project's exception-comment form if it has one, saying why that value is
right for every caller that omits it; no comment, no default. The comment
names the assumption so a reviewer can disagree.
"Sensible", "standard", "usual", "conventional", "every other project does
it", "obvious" and "keeps the signature short" justify nothing: they assert
someone else chose. A valid justification names why this value is right for
a caller who says nothing. The operator asking for a specific default, in
this conversation, for this value, settles it without a comment.
## Judge each hit
- **Real default**: rewrite it as the printed replacement, at the value's
true owner.
- **Justified**: a `justified?` hit stays only if its comment meets the bar
above.
- **False positive**: a `candidate` (`??`, `||`, Python `or`) that is logic,
or no default at all; report why.
## What the script cannot see
Go fields left to zero values (a constructor validating every field),
defaults in a Helm `values.yaml` (none; fail on the missing input), Docker
`ENV` placeholders for config (`${VAR:?VAR is required}`), Java overloads
filling an optional parameter (required constructor arguments), and any other
value-resolution fallback (`?:`, `getOrElse`, `fallback`, a dataclass field).
## How to apply
- Before writing any parameter, ask what happens if the caller omits it;
anything but "it fails and says so" gets rewritten.
- Pushing a default one layer up is no fix: a caller passing an invented
constant only moved the guess. Require the value at its true owner
(operator, inventory, deployment config).
- Removing an existing default is a behaviour change, never a silent
drive-by: name it and let the operator decide whether it lands in this
change or its own.
@@ -550,7 +550,7 @@ def call_default(node: ast.Call):
elif "getenv" in (attribute, name) or attribute == "get":
fallback = node.args[1] if len(node.args) == 2 else keywords.get("default")
owner = ast.unparse(func.value) if attribute == "get" else "os.environ"
instead = "os.environ[key]" if owner.endswith("environ") else f"{owner}[key]"
instead = "os.environ[name]" if owner.endswith("environ") else f"{owner}[name]"
else:
return []
if fallback is None or absent(fallback):
@@ -580,7 +580,7 @@ def scan_python(path: str, text: str, wanted: set[int] | None) -> list[Hit]:
tree = ast.parse(text, filename=path)
tokens = list(tokenize.generate_tokens(io.StringIO(text).readline))
except (SyntaxError, ValueError, tokenize.TokenError) as error:
raise ScanError(f"{path}: not parseable as Python: {error}") from error
raise ScanError(f"{path}: unparseable Python: {error}") from error
commented = {token.start[0] for token in tokens if token.type == tokenize.COMMENT}
comment_only = {line for line, row in enumerate(text.split("\n"), 1) if row.lstrip().startswith("#")}
hits = []
-38
View File
@@ -1,38 +0,0 @@
---
name: kvb-code-no-comments
description: >
Purge explanatory comments from staged and unstaged changes. Run
AUTOMATICALLY before every commit or stage request (commit, commit
no-verify, stage next, "commite ..."); also on comment complaints.
---
On a commit or stage request, run this audit first, then the requested git
action; never ask whether to run it.
Audit the added lines of every staged and unstaged change (`git diff` plus
`git diff --cached`) and delete every explanatory comment:
1. Delete restated code, step narration, section banners, "Note that",
rationale prose, apology comments, and any comment whose information the
code, task name or commit message already carries. When in doubt, delete.
2. Keep only parameter and exit-code docs, nocheck/noqa/shellcheck
directives, and project-sanctioned exception markers naming a concrete
trip-wire. A relabeled narration is not an exception; delete it.
3. Block-comment languages are not line-oriented. In Jinja (`{# … #}`),
C-like (`/* … */`), HTML (`<!-- … -->`) and their kin, a closing delimiter
alone on its line looks like a comment line; deleted with the rest, it
lets the opening delimiter swallow every live statement down to the next
closer. Never strip these with a line filter: delete the whole block by
hand, count the delimiters (`grep -c '{#'` against `grep -c '#}'`) and
re-parse the file.
4. Re-run the project's comment lint if one exists. Green does NOT prove the
deletion safe: a swallowed block stays syntactically valid, so lint and
parser pass over code that no longer executes. Diff against the pre-edit
revision (`git show HEAD:<path>`) and confirm only comment lines left.
5. Do not stage to run a check (the `kvb-git-commit-next` skill's step 4 says
why). After the audit, re-stage exactly the files staged before it.
6. Penalty: anything you wrote yourself goes into the report (file and
count, no excuses), lowers your confidence numbers for the affected
bundle, and is written to persistent agent memory (extend the existing
minimal-comments feedback entry with the date and the pattern you slipped
on) so the calibration survives the session.
-80
View File
@@ -1,80 +0,0 @@
---
name: kvb-code-no-defaults
description: >
Forbid defaults for parameters, settings and environment lookups;
demand explicit values. Apply AUTOMATICALLY whenever code, config,
templates or infrastructure are written, edited or reviewed.
---
A default is a guess written where the caller's intent is least known: it
turns a missing value into a plausible wrong answer, and two callers read the
same call differently (one means "use the default", the other forgot). Never
write a substantive default: require the value; when it is absent, fail at
once and loudly, naming the missing thing.
## The rule
1. Every parameter, option, setting, environment lookup, column and template
variable MUST be supplied explicitly by its caller.
2. A missing value MUST abort at the earliest moment (argument parsing,
config load, container start), never at the point of use and never by
proceeding with a substitute.
3. The error MUST name the missing key and where to set it.
4. Genuine optionality MUST be an explicit named case the call site handles:
`None`/`null` in a visible branch is fine, a fallback baked into the
accessor is not.
## Free and justified defaults
`false`, `null` (`None`, `nil`, `undefined`), `""` and an empty list or map
MAY be defaults without justification: they denote absence, not a guessed
value. `${VAR:-}`, `d.get(k)`, `a ?? null` and `def f(a=None)` are fine.
Every other default (a number, a non-empty string, list or map, an enum
member, a path, a timeout) MUST carry a comment on or directly above it, in
the project's exception-comment form if it has one, saying why that value is
right for every caller that omits it; no comment, no default. The comment
names the assumption so a reviewer can disagree.
"Sensible", "standard", "usual", "conventional", "every other project does
it", "obvious" and "keeps the signature short" justify nothing: they assert
someone else chose. A valid justification names why this value is right for
a caller who says nothing. The operator asking for a specific default, in
this conversation, for this value, settles it without a comment.
## Detect
Before presenting a diff or a review, run
`${CLAUDE_SKILL_DIR}/scripts/defaults.py --repo <dir>` (`--staged`: index
only; `--file <path>`: a whole untracked file). Each hit prints `file:line`,
language, construct and replacement; exit 1 means hits. Judge each:
- **Real default**: rewrite it as the replacement, at the value's true owner.
- **Justified**: a `justified?` hit stays only if its comment meets the bar
above.
- **False positive**: a `candidate` (`??`, `||`, Python `or`) that is logic,
or no default at all; report why.
Check by hand what the script cannot see: Go fields left to zero values (a
constructor validating every field), defaults in a Helm `values.yaml` (none;
fail on the missing input), Docker `ENV` placeholders for config
(`${VAR:?VAR is required}`), Java overloads filling an optional parameter
(required constructor arguments), and any other value-resolution fallback
(`?:`, `getOrElse`, `fallback`, a dataclass field).
## How to apply
- Before writing any parameter, ask what happens if the caller omits it;
anything but "it fails and says so" gets rewritten.
- Pushing a default one layer up is no fix: a caller passing an invented
constant only moved the guess. Require the value at its true owner
(operator, inventory, deployment config).
- Removing an existing default is a behaviour change, never a silent
drive-by: name it and let the operator decide whether it lands in this
change or its own.
## Penalty
An unjustified default you wrote: report it with file and count, no excuses;
lower the bundle's confidence numbers; write the date and the construct to
persistent agent memory so the calibration survives the session.
+1 -1
View File
@@ -57,7 +57,7 @@ skill sets it up), grep and read after.
nesting; magic values become a named constant at the use site.
- **Errors**: never widen or swallow a caught exception, never remove a
validation or security check to shorten code.
- **Comments**: invoke the `kvb-code-no-comments` skill.
- **Comments, defaults**: the `kvb-code-clean` skill.
Never reformat a region without a substantive fix: cosmetic churn buries the
real change. An architectural problem the change set cannot fix is reported,
+2 -2
View File
@@ -23,8 +23,8 @@ after three simultaneous changes proves nothing about which one mattered.
unbuilt artefact. Fix the environment or report it; do not patch code
around it.
- **Fix** with the smallest change that addresses the root cause, in the
project's idioms and under the `kvb-code-no-defaults` and
`kvb-code-no-comments` rules. Forbidden without explicit operator approval:
project's idioms and under the `kvb-code-clean` rules. Forbidden without
explicit operator approval:
deleting, skipping, `xfail`-ing or commenting out a failing test; loosening
an assertion to what the code produces; swallowing the exception the test
surfaces; retries, sleeps or reruns over a flaky failure; disabling a linter
+6 -5
View File
@@ -18,7 +18,7 @@ description: >
Wait for the operator's decision; do not commit.
3. Nothing similar found, or the operator picked the staged version: run the
kvb-git-commit-no-verify skill on the currently staged changes
(kvb-code-no-comments audit and kvb-check-legal check first, then the
(kvb-code-clean pass and kvb-check-legal check first, then the
full staged diff and the kvb-check-score commit table of 📁 worktree,
🌿 branch, 📊 diff stat, scores with 🛡️ security and gate, then
`git commit --no-verify`, exact staging index only, its message carrying
@@ -40,10 +40,11 @@ description: >
operator's own edits and local config. A file that carries two bundles is
staged by line: `${CLAUDE_SKILL_DIR}/scripts/stage_hunks.py --repo
<worktree> --file <path> --match <regex>`, first with `--dry-run`.
5. For the newly staged bundle run the kvb-think-explain skill (what the
changes do and how the pieces interact, file:line-anchored), then the
kvb-check-confidence skill (its kvb-check-score block with 🛡️ security
from `security.py` of kvb-check-security, evidence, residuals).
5. For the newly staged bundle run the kvb-code-clean pass, then the
kvb-think-explain skill (what the changes do and how the pieces
interact, file:line-anchored), then the kvb-check-confidence skill (its
kvb-check-score block with 🛡️ security from `security.py` of
kvb-check-security, evidence, residuals).
6. Close with the staged bundle's commit table (`score.py ...
--commit-table <worktree>`: 📁 worktree, 🌿 branch, 📊 diff stat, scores,
gate), what remains unstaged, and wait for the operator's commit
+3 -3
View File
@@ -6,9 +6,9 @@ description: >
carried over to later commits.
---
1. Run the kvb-code-no-comments audit over the staged and unstaged changes first,
then the kvb-check-legal check over the staging index. A legal blocker stops
here.
1. Run the kvb-code-clean pass (comments, defaults, principles) over the
staged and unstaged changes first, then the kvb-check-legal check over
the staging index. A legal blocker stops here.
2. Show in the message right before the `git commit` call, never only after
it, the full staged diff (`git diff --cached`), then the commit table of
the kvb-check-score skill (`score.py ... --commit-table <worktree>`):
+2 -2
View File
@@ -16,8 +16,8 @@ Produce a one-shot overview of every skill the session can invoke:
compressing the description instead of pasting it.
3. When the operator names a specific skill or topic, explain only the
matching skills in more depth: what they do, when they fire on their
own, and how they compose (e.g. kvb-git-commit-next → kvb-git-commit-no-verify →
kvb-code-no-comments).
own, and how they compose (e.g. kvb-git-commit-next →
kvb-git-commit-no-verify → kvb-code-clean).
4. End with the activation hint: project installers or `make install` in
the skills repository refresh the live copies; a restart loads renamed
or new skills.
+1 -1
View File
@@ -40,7 +40,7 @@ Compile what runs often or went wrong; a one-off stays prose. Per candidate:
library in the skills repository); no new language, framework or dependency
without the operator.
- **Interface**: every host path, name and value an explicit parameter (the
`kvb-code-no-defaults` skill), documented exit codes, `--json` when another
`kvb-code-clean` skill), documented exit codes, `--json` when another
script or skill consumes the output.
- **Location**: `skills/<skill>/scripts/<verb>.py` with a test under `tests/`
in the skills repository; elsewhere the owning repository's convention,
+3 -3
View File
@@ -33,7 +33,7 @@ new skill, a sharper step in an existing skill, a memory entry instead, or
drop. A recurring deterministic move without a script goes to the
`kvb-skill-compile` skill, any other recurring move to the `kvb-skill-tune`
skill as evidence. A script is portable only once its host paths, names and
values are explicit parameters (the `kvb-code-no-defaults` skill); one that
values are explicit parameters (the `kvb-code-clean` skill); one that
needs a secret, a credential or a single host is dropped or stays a memory
entry.
@@ -49,8 +49,8 @@ or diff.
- Write where the `kvb-skill-tune` skill's **Persistence** section says: the
skills repository as source of truth, then the live mirrors.
- Read every script whole before carrying it; strip comments the way the
`kvb-code-no-comments` skill does.
- Read every script whole before carrying it; clean it the way the
`kvb-code-clean` skill does.
- A carried script gets a unit test in the repository's `tests/` with a
negative control, run once `watch.py admit --kind test-nostack` (the
`kvb-org-organize` skill) prints `grant`; `make test` runs through
+1 -1
View File
@@ -10,7 +10,7 @@ import unittest
from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parents[1]
SCRIPT = REPO_ROOT / "skills" / "kvb-code-no-defaults" / "scripts" / "defaults.py"
SCRIPT = REPO_ROOT / "skills" / "kvb-code-clean" / "scripts" / "defaults.py"
BASE = {
"app.py": "import os\n\n\ndef old(a=1):\n return a\n",
"run.sh": 'echo "${OLD:-1}"\n',