nandi/oripublic Fork 0
db7446cc6b52b6bd9196294c1e66e5eb56754d5e
Commits
Clone
git clone https://git.rickub.com/nandi/ori.git
git clone ssh://git@rickub.com/nandi/ori.git

Host key fingerprint (ed25519): SHA256:iycHnxEyq0Q7uyVpB7JlznP0G7JrTPXLYRcAU5CSLhc — verify it before your first connect.

forked from bots-garden/ori

🎉 Begin a project. 4edda86 · on db7446cc6b52b6bd9196294c1e66e5eb56754d5e · k33g · yesterday
SKILL.md · 266 lines · 13.3 KBmarkdown
Blame HistoryOpen raw

name: quality
description: Audit and improve this project's code quality with qlty. Sets qlty up on first use, measures lint issues / code smells / complexity, writes a timestamped Markdown report under .quality/, tracks progression across runs, and refactors until the quality gate passes. Use when asked to check code quality, run a quality report, review technical debt, see whether quality is improving, or clean up / refactor the codebase.

Quality

Measure this project's code quality with qlty, report it, and
refactor until the gate passes.

The measurement is a script, not a judgement call — run it, then act on what it says.
It is read-only with respect to the source tree (qlty check runs with --no-fix), so
measuring never quietly rewrites the code you are about to reason about.

1. Set up (only if needed)

The setup is self-healing; just run the script. It verifies qlty is on PATH, that
the workspace is a git repository, and runs qlty init --yes when there is no
.qlty/qlty.toml yet.

If qlty is missing entirely, the sandbox was not created with the dev-toolkit
kit. Say so and stop — do not install qlty by hand.

If the project vendors this kit — a kits/ directory holding the sources that get
installed into ~/.claude/ — add it to exclude_patterns in .qlty/qlty.toml on the
first run:

exclude_patterns = [
  # … the defaults qlty init wrote …
  "kits/**",   # the kit's own sources, including this skill's quality_report.py
]

This is not an exception to "do not game the gate" below. The reason is narrower and
holds regardless of what the numbers say: quality_report.py is the script that
produces the measurement, so leaving it in scope means measuring the instrument with
itself. Two of its findings are structurally unfixable — a script whose job is to run
qlty must import and call subprocess, which every Python security linter flags.

Two conditions make this legitimate rather than convenient, and both must hold:

  • It is the project's decision, not yours. Say what the exclusion hides and why, and
    let the user choose. Adding it unilaterally is the forbidden move.
  • Write down what it hides. Put the actual findings in a comment next to the pattern.
    Some of them are usually real defects in the script — over-complex functions, unused
    imports — and they should be fixed at the kit's source rather than forgotten. An
    exclusion that records what it silences stays honest; one that just makes a number
    drop is the start of the drift.

2. Measure

From the project root:

python3 ~/.claude/skills/quality/scripts/quality_report.py --workspace .

On a cold cache the first run takes several minutes: qlty downloads its plugin
definitions and the linter binaries. Subsequent runs are fast (the cache lives on a
persistent volume at ~/.qlty).

The script:

  • runs qlty check --all, qlty smells --all, and qlty metrics --all
  • writes .quality/report-<timestamp>.md and .quality/report-latest.md
  • appends one JSON line to .quality/history.jsonl
  • prints the report to stdout
  • exits 0 if the gate passed, 1 if it failed, 2 on a tooling error

Use the exit code as the loop condition. Do not re-derive the verdict yourself.

3. Read the report

The report contains:

Section What to do with it
Gate violations The work list. Each line is a threshold that was breached.
Lint issues by level, top rules, most affected files Where to start — but read the count correctly first, see below.
Code smells Duplication and over-complex structure — these need real refactoring, not a formatter.
Metrics complex / cyclo per file; the "Most complex files" table names refactoring targets.
Trend Last 10 runs. This is how you show progression.
Tooling notes Present only when a qlty invocation failed — investigate before trusting the numbers.

Read the count before you trust it

"Fix the highest-count rule first" is the right instinct and the wrong first step. A
count is occurrences, not distinct problems, and the two diverge badly in two common
cases:

  • One finding, repeated per manifest. osv-scanner reports a dependency CVE once per
    go.mod / package.json it appears in. A monorepo with a dozen manifests turns one
    vulnerable version into a hundred-plus findings, and adding a three-line example
    manifest multiplies the total without degrading anything.
  • One finding, repeated per file. A linter that fails to parse the language emits one
    finding per source file. That looks like the largest cluster in the report and is worth
    exactly nothing.

And before treating a delta as a regression: re-run. A count that moves while the
tree is untouched is measuring the tool, not the code. The usual cause is a linter
answering from its own partially-populated cache — qlty invokes several with
--allow-parallel-runners, and an invocation returning in a fraction of a second did
not re-analyse anything. That failure mode under-reports: the low number is the
wrong one, so it looks like an improvement.

When you suspect it, clear that linter's cache and run its binary directly, several
times, until the result is stable; .qlty/out/invoke-*.yaml records the exact command,
environment and duration qlty used. Work from that figure, not from the report, and say
in your report which number you trust and why. Remember too that a gate threshold
pinned to an unstable measurement will trip on its own
— give it margin, and write
down that the margin is the tool's fault, so it gets removed rather than inherited.

So before picking a target, check how many distinct rules and how many distinct files
a count spans:

# qlty check has no --json; SARIF is the machine-readable form
qlty check --all --no-fix --no-upgrade-check --sarif > /tmp/qlty.sarif

Then group the results by ruleId and by file URI. A rule spanning many files with one
occurrence each, or many rules spanning the same short file list, means the count is
telling you about the shape of the scan, not the state of the code. Say so in your
report — a total nobody can interpret gets ignored, and then a real regression hides in
it.

4. Refactor until the gate passes

If the gate failed, refactor. Loop:

  1. Pick the largest cluster of violations from the report (one rule, or one file).

  2. Fix formatting mechanically first — qlty fmt handles it, and it clears a lot of
    noise cheaply:

    qlty fmt --all --no-upgrade-check
    
  3. Fix the remaining issues by editing the code. For smells, prefer extracting a
    function or collapsing a duplicated block over suppressing the finding.

  4. Verify nothing broke, using this project's own build and test commands. The
    repository's tooling is the authority on what those are — check for a Makefile,
    Taskfile.yml, package.json scripts, pyproject.toml, or the CI workflow before
    assuming. Common cases:

    Stack Verification
    Go go build ./... && go test ./...
    Go targeting WebAssembly also tinygo build -target=wasip1 ./... — TinyGo rejects some constructs the standard toolchain accepts, so a passing go build is not sufficient
    Node / TypeScript npm test (or the script the project defines), plus tsc --noEmit if the project is typed
    Python pytest
    Rust cargo build && cargo test && cargo clippy -- -D warnings
    Rust targeting WebAssembly also cargo build --target wasm32-wasip2 — and note the asymmetry below
    VS Code extension the compile script, then vsce package — packaging validates the manifest, which tsc does not
    Zed extension cargo build --release --target wasm32-wasip2

    If the project has no tests, say so in the report rather than letting a green build
    stand in as proof the refactoring was safe.

    Two traps specific to WebAssembly targets. Both let a refactoring look verified
    when it is not:

    • A wasm build passing does not mean the tests ran. cargo build --target wasm32-wasip2 links with rustc's own bundled rust-lld, so it succeeds even
      with no C toolchain present, while cargo test — which builds for the host
      fails at link time with error: linker `cc` not found. If you see that error,
      the tests did not run; do not report the wasm build as verification.
    • An extension cannot be exercised here. No editor runs in this sandbox, so
      neither VS Code's Extension Development Host nor Zed's Install Dev Extension
      is available, and @vscode/test-electron needs an X server this kit does not
      install. Compile, unit-test and package — then say plainly that the integration
      path is untested rather than implying it passed.
  5. Re-run the measurement from step 2 and compare against the previous run.

Stop when any of these is true:

  • The gate passes. Report the before/after numbers from the Trend table.
  • Five iterations have run. Stop and report honestly: what improved, what remains,
    and why the rest is hard.
  • Two consecutive runs show no improvement. You are stuck — stop and explain what
    the blocker is rather than churning the code.

Commit as you go, one focused commit per cluster of fixes, so the progression is
visible in git as well as in .quality/history.jsonl.

Do not game the gate

The gate is only worth something if it is measuring the code. All of the following make
the number go down while making the project worse — never do them, even if the gate is
the only thing standing between you and "done":

  • Adding exclude_patterns, disabling a plugin, setting a plugin to mode = "monitor",
    or raising a threshold in .qlty/qlty.toml.
  • Loosening .quality/gate.json.
  • Deleting, skipping, or emptying tests.
  • Adding blanket lint suppressions (//nolint, // qlty-ignore) to silence a finding
    you could fix.

A narrowly-scoped suppression with a comment explaining why the finding is a false
positive is legitimate. If you believe a threshold is genuinely wrong for this project,
say so and let the user decide — do not change it yourself.

The one plugin you may remove: a broken one

"Disabling a plugin" above means a plugin that finds things you would rather not fix. A
plugin that analyses nothing is a different object, and keeping it is not rigour: its
findings crowd out the ones that matter and make the total unreadable.

The two are indistinguishable from the report alone, so removal requires a reproducible
diagnosis
, never an impression. Establish all four of these before touching the config:

  1. It is not the language version or a specific construct. Put a trivially small,
    unambiguously valid file of that language in a fresh git repository, enable only
    that plugin, and run it. If a five-line hello-world fails, no source construct is to
    blame.
  2. It is not this project. Same point — the probe repository shares nothing with the
    codebase under measurement.
  3. It is not the environment qlty gives plugins. qlty runs plugins with a stripped
    environment (empty PATH and HOME). Run the plugin's own binary directly, with your
    full environment, on the same probe file. Read the invocation qlty used from
    .qlty/out/invoke-*.yaml, which records the exact script, env and stderr.
  4. It is not the plugin family. If sibling plugins sharing the same runtime produce
    real findings, the failure is specific rather than environmental.

If all four hold, remove the plugin — and write the diagnosis as a comment where you
removed it
, so the next session finds the evidence instead of re-adding it on instinct.
Note in your report which other tooling still covers that ground; if nothing does, the
answer is to replace the plugin, not merely to drop it.

mode = "monitor" is not the remedy here. It does not stop a plugin's findings from
counting; it only changes how they are surfaced. A broken plugin left in monitor mode
keeps polluting the total, which is how one ends up unnoticed for months.

Do not encode "plugin X is broken" into this skill. Such a failure belongs to one
version, one platform and one moment; a list of known-bad plugins ages badly and, worse,
tempts the next agent to skip the diagnosis. The method above is what belongs here — its
results belong in the project's own config and notes.

Tuning the gate

Defaults are strict: zero error-level issues, zero warning-level issues, zero smells.
Override any subset by writing .quality/gate.json:

{
  "max_error": 0,
  "max_warning": 0,
  "max_note": null,
  "max_smells": 0,
  "max_file_complexity": 60,
  "max_total_complexity": null
}

null disables a check. max_file_complexity flags individual files whose complex
metric exceeds the limit, which is the most useful knob for a codebase with existing
debt: it lets you hold the line on the worst files without demanding a perfect score
everywhere at once.

Reference

  • qlty check --help, qlty smells --help, qlty metrics --help for the full flag set.
  • Note qlty check has no --json; machine-readable output is --sarif.
  • Pass --no-upgrade-check to every qlty command — the sandbox blocks its update host.
  • Reports and history under .quality/ are meant to be committed so progression
    survives sandbox recreation. Add .quality/ to .gitignore if the user prefers them
    to stay local.
  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68
 69
 70
 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83
 84
 85
 86
 87
 88
 89
 90
 91
 92
 93
 94
 95
 96
 97
 98
 99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
---
name: quality
description: Audit and improve this project's code quality with qlty. Sets qlty up on first use, measures lint issues / code smells / complexity, writes a timestamped Markdown report under .quality/, tracks progression across runs, and refactors until the quality gate passes. Use when asked to check code quality, run a quality report, review technical debt, see whether quality is improving, or clean up / refactor the codebase.
---

# Quality

Measure this project's code quality with [qlty](https://docs.qlty.sh), report it, and
refactor until the gate passes.

The measurement is a script, not a judgement call — run it, then act on what it says.
It is read-only with respect to the source tree (`qlty check` runs with `--no-fix`), so
measuring never quietly rewrites the code you are about to reason about.

## 1. Set up (only if needed)

The setup is self-healing; just run the script. It verifies `qlty` is on `PATH`, that
the workspace is a git repository, and runs `qlty init --yes` when there is no
`.qlty/qlty.toml` yet.

If `qlty` is missing entirely, the sandbox was not created with the `dev-toolkit`
kit. Say so and stop — do not install qlty by hand.

**If the project vendors this kit** — a `kits/` directory holding the sources that get
installed into `~/.claude/` — add it to `exclude_patterns` in `.qlty/qlty.toml` on the
first run:

```toml
exclude_patterns = [
  # … the defaults qlty init wrote …
  "kits/**",   # the kit's own sources, including this skill's quality_report.py
]
```

This is **not** an exception to "do not game the gate" below. The reason is narrower and
holds regardless of what the numbers say: `quality_report.py` is the script that
*produces* the measurement, so leaving it in scope means measuring the instrument with
itself. Two of its findings are structurally unfixable — a script whose job is to run
`qlty` must import and call `subprocess`, which every Python security linter flags.

Two conditions make this legitimate rather than convenient, and both must hold:

- **It is the project's decision, not yours.** Say what the exclusion hides and why, and
  let the user choose. Adding it unilaterally is the forbidden move.
- **Write down what it hides.** Put the actual findings in a comment next to the pattern.
  Some of them are usually real defects in the script — over-complex functions, unused
  imports — and they should be fixed at the kit's source rather than forgotten. An
  exclusion that records what it silences stays honest; one that just makes a number
  drop is the start of the drift.

## 2. Measure

From the project root:

```bash
python3 ~/.claude/skills/quality/scripts/quality_report.py --workspace .
```

On a cold cache the first run takes several minutes: qlty downloads its plugin
definitions and the linter binaries. Subsequent runs are fast (the cache lives on a
persistent volume at `~/.qlty`).

The script:

- runs `qlty check --all`, `qlty smells --all`, and `qlty metrics --all`
- writes `.quality/report-<timestamp>.md` and `.quality/report-latest.md`
- appends one JSON line to `.quality/history.jsonl`
- prints the report to stdout
- **exits `0` if the gate passed, `1` if it failed, `2` on a tooling error**

Use the exit code as the loop condition. Do not re-derive the verdict yourself.

## 3. Read the report

The report contains:

| Section | What to do with it |
| --- | --- |
| **Gate violations** | The work list. Each line is a threshold that was breached. |
| **Lint issues** by level, top rules, most affected files | Where to start — but read the count correctly first, see below. |
| **Code smells** | Duplication and over-complex structure — these need real refactoring, not a formatter. |
| **Metrics** | `complex` / `cyclo` per file; the "Most complex files" table names refactoring targets. |
| **Trend** | Last 10 runs. This is how you show progression. |
| **Tooling notes** | Present only when a qlty invocation failed — investigate before trusting the numbers. |

### Read the count before you trust it

"Fix the highest-count rule first" is the right instinct and the wrong first step. A
count is `occurrences`, not `distinct problems`, and the two diverge badly in two common
cases:

- **One finding, repeated per manifest.** `osv-scanner` reports a dependency CVE once per
  `go.mod` / `package.json` it appears in. A monorepo with a dozen manifests turns one
  vulnerable version into a hundred-plus findings, and adding a three-line example
  manifest multiplies the total without degrading anything.
- **One finding, repeated per file.** A linter that fails to parse the language emits one
  finding per source file. That looks like the largest cluster in the report and is worth
  exactly nothing.

And before treating a *delta* as a regression: **re-run.** A count that moves while the
tree is untouched is measuring the tool, not the code. The usual cause is a linter
answering from its own partially-populated cache — qlty invokes several with
`--allow-parallel-runners`, and an invocation returning in a fraction of a second did
not re-analyse anything. That failure mode **under-reports**: the low number is the
wrong one, so it looks like an improvement.

When you suspect it, clear that linter's cache and run its binary directly, several
times, until the result is stable; `.qlty/out/invoke-*.yaml` records the exact command,
environment and duration qlty used. Work from that figure, not from the report, and say
in your report which number you trust and why. Remember too that **a gate threshold
pinned to an unstable measurement will trip on its own** — give it margin, and write
down that the margin is the tool's fault, so it gets removed rather than inherited.

So before picking a target, check **how many distinct rules and how many distinct files**
a count spans:

```bash
# qlty check has no --json; SARIF is the machine-readable form
qlty check --all --no-fix --no-upgrade-check --sarif > /tmp/qlty.sarif
```

Then group the results by `ruleId` and by file URI. A rule spanning many files with one
occurrence each, or many rules spanning the same short file list, means the count is
telling you about the *shape of the scan*, not the state of the code. Say so in your
report — a total nobody can interpret gets ignored, and then a real regression hides in
it.

## 4. Refactor until the gate passes

If the gate failed, refactor. Loop:

1. Pick the largest cluster of violations from the report (one rule, or one file).
2. Fix formatting mechanically first — `qlty fmt` handles it, and it clears a lot of
   noise cheaply:
   ```bash
   qlty fmt --all --no-upgrade-check
   ```
3. Fix the remaining issues by editing the code. For smells, prefer extracting a
   function or collapsing a duplicated block over suppressing the finding.
4. Verify nothing broke, using **this project's own** build and test commands. The
   repository's tooling is the authority on what those are — check for a `Makefile`,
   `Taskfile.yml`, `package.json` scripts, `pyproject.toml`, or the CI workflow before
   assuming. Common cases:

   | Stack | Verification |
   | --- | --- |
   | Go | `go build ./... && go test ./...` |
   | Go targeting WebAssembly | also `tinygo build -target=wasip1 ./...` — TinyGo rejects some constructs the standard toolchain accepts, so a passing `go build` is not sufficient |
   | Node / TypeScript | `npm test` (or the script the project defines), plus `tsc --noEmit` if the project is typed |
   | Python | `pytest` |
   | Rust | `cargo build && cargo test && cargo clippy -- -D warnings` |
   | Rust targeting WebAssembly | also `cargo build --target wasm32-wasip2` — and note the asymmetry below |
   | VS Code extension | the compile script, then `vsce package` — packaging validates the manifest, which `tsc` does not |
   | Zed extension | `cargo build --release --target wasm32-wasip2` |

   If the project has no tests, say so in the report rather than letting a green build
   stand in as proof the refactoring was safe.

   **Two traps specific to WebAssembly targets.** Both let a refactoring look verified
   when it is not:

   - A **wasm build passing does not mean the tests ran.** `cargo build --target
     wasm32-wasip2` links with rustc's own bundled `rust-lld`, so it succeeds even
     with no C toolchain present, while `cargo test` — which builds for the *host* —
     fails at link time with ``error: linker `cc` not found``. If you see that error,
     the tests did not run; do not report the wasm build as verification.
   - An **extension cannot be exercised here.** No editor runs in this sandbox, so
     neither VS Code's Extension Development Host nor Zed's *Install Dev Extension*
     is available, and `@vscode/test-electron` needs an X server this kit does not
     install. Compile, unit-test and package — then say plainly that the integration
     path is untested rather than implying it passed.
5. Re-run the measurement from step 2 and compare against the previous run.

Stop when any of these is true:

- **The gate passes.** Report the before/after numbers from the Trend table.
- **Five iterations have run.** Stop and report honestly: what improved, what remains,
  and why the rest is hard.
- **Two consecutive runs show no improvement.** You are stuck — stop and explain what
  the blocker is rather than churning the code.

Commit as you go, one focused commit per cluster of fixes, so the progression is
visible in git as well as in `.quality/history.jsonl`.

## Do not game the gate

The gate is only worth something if it is measuring the code. All of the following make
the number go down while making the project worse — never do them, even if the gate is
the only thing standing between you and "done":

- Adding `exclude_patterns`, disabling a plugin, setting a plugin to `mode = "monitor"`,
  or raising a threshold in `.qlty/qlty.toml`.
- Loosening `.quality/gate.json`.
- Deleting, skipping, or emptying tests.
- Adding blanket lint suppressions (`//nolint`, `// qlty-ignore`) to silence a finding
  you could fix.

A narrowly-scoped suppression with a comment explaining why the finding is a false
positive is legitimate. If you believe a threshold is genuinely wrong for this project,
say so and let the user decide — do not change it yourself.

### The one plugin you may remove: a broken one

"Disabling a plugin" above means a plugin that *finds things you would rather not fix*. A
plugin that **analyses nothing** is a different object, and keeping it is not rigour: its
findings crowd out the ones that matter and make the total unreadable.

The two are indistinguishable from the report alone, so removal requires a **reproducible
diagnosis**, never an impression. Establish all four of these before touching the config:

1. **It is not the language version or a specific construct.** Put a trivially small,
   unambiguously valid file of that language in a *fresh* git repository, enable only
   that plugin, and run it. If a five-line hello-world fails, no source construct is to
   blame.
2. **It is not this project.** Same point — the probe repository shares nothing with the
   codebase under measurement.
3. **It is not the environment qlty gives plugins.** qlty runs plugins with a stripped
   environment (empty `PATH` and `HOME`). Run the plugin's own binary directly, with your
   full environment, on the same probe file. Read the invocation qlty used from
   `.qlty/out/invoke-*.yaml`, which records the exact `script`, `env` and `stderr`.
4. **It is not the plugin family.** If sibling plugins sharing the same runtime produce
   real findings, the failure is specific rather than environmental.

If all four hold, remove the plugin — and **write the diagnosis as a comment where you
removed it**, so the next session finds the evidence instead of re-adding it on instinct.
Note in your report which other tooling still covers that ground; if nothing does, the
answer is to replace the plugin, not merely to drop it.

`mode = "monitor"` is *not* the remedy here. It does not stop a plugin's findings from
counting; it only changes how they are surfaced. A broken plugin left in monitor mode
keeps polluting the total, which is how one ends up unnoticed for months.

**Do not encode "plugin X is broken" into this skill.** Such a failure belongs to one
version, one platform and one moment; a list of known-bad plugins ages badly and, worse,
tempts the next agent to skip the diagnosis. The method above is what belongs here — its
results belong in the project's own config and notes.

## Tuning the gate

Defaults are strict: zero error-level issues, zero warning-level issues, zero smells.
Override any subset by writing `.quality/gate.json`:

```json
{
  "max_error": 0,
  "max_warning": 0,
  "max_note": null,
  "max_smells": 0,
  "max_file_complexity": 60,
  "max_total_complexity": null
}
```

`null` disables a check. `max_file_complexity` flags individual files whose `complex`
metric exceeds the limit, which is the most useful knob for a codebase with existing
debt: it lets you hold the line on the worst files without demanding a perfect score
everywhere at once.

## Reference

- `qlty check --help`, `qlty smells --help`, `qlty metrics --help` for the full flag set.
- Note `qlty check` has **no** `--json`; machine-readable output is `--sarif`.
- Pass `--no-upgrade-check` to every qlty command — the sandbox blocks its update host.
- Reports and history under `.quality/` are meant to be committed so progression
  survives sandbox recreation. Add `.quality/` to `.gitignore` if the user prefers them
  to stay local.