forked from bots-garden/ori
| 🎉 Begin a project. | 1 | --- |
| 2 | name: quality | |
| 3 | 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. | |
| 4 | --- | |
| 5 | ||
| 6 | # Quality | |
| 7 | ||
| 8 | Measure this project's code quality with [qlty](https://docs.qlty.sh), report it, and | |
| 9 | refactor until the gate passes. | |
| 10 | ||
| 11 | The measurement is a script, not a judgement call — run it, then act on what it says. | |
| 12 | It is read-only with respect to the source tree (`qlty check` runs with `--no-fix`), so | |
| 13 | measuring never quietly rewrites the code you are about to reason about. | |
| 14 | ||
| 15 | ## 1. Set up (only if needed) | |
| 16 | ||
| 17 | The setup is self-healing; just run the script. It verifies `qlty` is on `PATH`, that | |
| 18 | the workspace is a git repository, and runs `qlty init --yes` when there is no | |
| 19 | `.qlty/qlty.toml` yet. | |
| 20 | ||
| 21 | If `qlty` is missing entirely, the sandbox was not created with the `dev-toolkit` | |
| 22 | kit. Say so and stop — do not install qlty by hand. | |
| 23 | ||
| 24 | **If the project vendors this kit** — a `kits/` directory holding the sources that get | |
| 25 | installed into `~/.claude/` — add it to `exclude_patterns` in `.qlty/qlty.toml` on the | |
| 26 | first run: | |
| 27 | ||
| 28 | ```toml | |
| 29 | exclude_patterns = [ | |
| 30 | # … the defaults qlty init wrote … | |
| 31 | "kits/**", # the kit's own sources, including this skill's quality_report.py | |
| 32 | ] | |
| 33 | ``` | |
| 34 | ||
| 35 | This is **not** an exception to "do not game the gate" below. The reason is narrower and | |
| 36 | holds regardless of what the numbers say: `quality_report.py` is the script that | |
| 37 | *produces* the measurement, so leaving it in scope means measuring the instrument with | |
| 38 | itself. Two of its findings are structurally unfixable — a script whose job is to run | |
| 39 | `qlty` must import and call `subprocess`, which every Python security linter flags. | |
| 40 | ||
| 41 | Two conditions make this legitimate rather than convenient, and both must hold: | |
| 42 | ||
| 43 | - **It is the project's decision, not yours.** Say what the exclusion hides and why, and | |
| 44 | let the user choose. Adding it unilaterally is the forbidden move. | |
| 45 | - **Write down what it hides.** Put the actual findings in a comment next to the pattern. | |
| 46 | Some of them are usually real defects in the script — over-complex functions, unused | |
| 47 | imports — and they should be fixed at the kit's source rather than forgotten. An | |
| 48 | exclusion that records what it silences stays honest; one that just makes a number | |
| 49 | drop is the start of the drift. | |
| 50 | ||
| 51 | ## 2. Measure | |
| 52 | ||
| 53 | From the project root: | |
| 54 | ||
| 55 | ```bash | |
| 56 | python3 ~/.claude/skills/quality/scripts/quality_report.py --workspace . | |
| 57 | ``` | |
| 58 | ||
| 59 | On a cold cache the first run takes several minutes: qlty downloads its plugin | |
| 60 | definitions and the linter binaries. Subsequent runs are fast (the cache lives on a | |
| 61 | persistent volume at `~/.qlty`). | |
| 62 | ||
| 63 | The script: | |
| 64 | ||
| 65 | - runs `qlty check --all`, `qlty smells --all`, and `qlty metrics --all` | |
| 66 | - writes `.quality/report-<timestamp>.md` and `.quality/report-latest.md` | |
| 67 | - appends one JSON line to `.quality/history.jsonl` | |
| 68 | - prints the report to stdout | |
| 69 | - **exits `0` if the gate passed, `1` if it failed, `2` on a tooling error** | |
| 70 | ||
| 71 | Use the exit code as the loop condition. Do not re-derive the verdict yourself. | |
| 72 | ||
| 73 | ## 3. Read the report | |
| 74 | ||
| 75 | The report contains: | |
| 76 | ||
| 77 | | Section | What to do with it | | |
| 78 | | --- | --- | | |
| 79 | | **Gate violations** | The work list. Each line is a threshold that was breached. | | |
| 80 | | **Lint issues** by level, top rules, most affected files | Where to start — but read the count correctly first, see below. | | |
| 81 | | **Code smells** | Duplication and over-complex structure — these need real refactoring, not a formatter. | | |
| 82 | | **Metrics** | `complex` / `cyclo` per file; the "Most complex files" table names refactoring targets. | | |
| 83 | | **Trend** | Last 10 runs. This is how you show progression. | | |
| 84 | | **Tooling notes** | Present only when a qlty invocation failed — investigate before trusting the numbers. | | |
| 85 | ||
| 86 | ### Read the count before you trust it | |
| 87 | ||
| 88 | "Fix the highest-count rule first" is the right instinct and the wrong first step. A | |
| 89 | count is `occurrences`, not `distinct problems`, and the two diverge badly in two common | |
| 90 | cases: | |
| 91 | ||
| 92 | - **One finding, repeated per manifest.** `osv-scanner` reports a dependency CVE once per | |
| 93 | `go.mod` / `package.json` it appears in. A monorepo with a dozen manifests turns one | |
| 94 | vulnerable version into a hundred-plus findings, and adding a three-line example | |
| 95 | manifest multiplies the total without degrading anything. | |
| 96 | - **One finding, repeated per file.** A linter that fails to parse the language emits one | |
| 97 | finding per source file. That looks like the largest cluster in the report and is worth | |
| 98 | exactly nothing. | |
| 99 | ||
| 100 | And before treating a *delta* as a regression: **re-run.** A count that moves while the | |
| 101 | tree is untouched is measuring the tool, not the code. The usual cause is a linter | |
| 102 | answering from its own partially-populated cache — qlty invokes several with | |
| 103 | `--allow-parallel-runners`, and an invocation returning in a fraction of a second did | |
| 104 | not re-analyse anything. That failure mode **under-reports**: the low number is the | |
| 105 | wrong one, so it looks like an improvement. | |
| 106 | ||
| 107 | When you suspect it, clear that linter's cache and run its binary directly, several | |
| 108 | times, until the result is stable; `.qlty/out/invoke-*.yaml` records the exact command, | |
| 109 | environment and duration qlty used. Work from that figure, not from the report, and say | |
| 110 | in your report which number you trust and why. Remember too that **a gate threshold | |
| 111 | pinned to an unstable measurement will trip on its own** — give it margin, and write | |
| 112 | down that the margin is the tool's fault, so it gets removed rather than inherited. | |
| 113 | ||
| 114 | So before picking a target, check **how many distinct rules and how many distinct files** | |
| 115 | a count spans: | |
| 116 | ||
| 117 | ```bash | |
| 118 | # qlty check has no --json; SARIF is the machine-readable form | |
| 119 | qlty check --all --no-fix --no-upgrade-check --sarif > /tmp/qlty.sarif | |
| 120 | ``` | |
| 121 | ||
| 122 | Then group the results by `ruleId` and by file URI. A rule spanning many files with one | |
| 123 | occurrence each, or many rules spanning the same short file list, means the count is | |
| 124 | telling you about the *shape of the scan*, not the state of the code. Say so in your | |
| 125 | report — a total nobody can interpret gets ignored, and then a real regression hides in | |
| 126 | it. | |
| 127 | ||
| 128 | ## 4. Refactor until the gate passes | |
| 129 | ||
| 130 | If the gate failed, refactor. Loop: | |
| 131 | ||
| 132 | 1. Pick the largest cluster of violations from the report (one rule, or one file). | |
| 133 | 2. Fix formatting mechanically first — `qlty fmt` handles it, and it clears a lot of | |
| 134 | noise cheaply: | |
| 135 | ```bash | |
| 136 | qlty fmt --all --no-upgrade-check | |
| 137 | ``` | |
| 138 | 3. Fix the remaining issues by editing the code. For smells, prefer extracting a | |
| 139 | function or collapsing a duplicated block over suppressing the finding. | |
| 140 | 4. Verify nothing broke, using **this project's own** build and test commands. The | |
| 141 | repository's tooling is the authority on what those are — check for a `Makefile`, | |
| 142 | `Taskfile.yml`, `package.json` scripts, `pyproject.toml`, or the CI workflow before | |
| 143 | assuming. Common cases: | |
| 144 | ||
| 145 | | Stack | Verification | | |
| 146 | | --- | --- | | |
| 147 | | Go | `go build ./... && go test ./...` | | |
| 148 | | Go targeting WebAssembly | also `tinygo build -target=wasip1 ./...` — TinyGo rejects some constructs the standard toolchain accepts, so a passing `go build` is not sufficient | | |
| 149 | | Node / TypeScript | `npm test` (or the script the project defines), plus `tsc --noEmit` if the project is typed | | |
| 150 | | Python | `pytest` | | |
| 151 | | Rust | `cargo build && cargo test && cargo clippy -- -D warnings` | | |
| 152 | | Rust targeting WebAssembly | also `cargo build --target wasm32-wasip2` — and note the asymmetry below | | |
| 153 | | VS Code extension | the compile script, then `vsce package` — packaging validates the manifest, which `tsc` does not | | |
| 154 | | Zed extension | `cargo build --release --target wasm32-wasip2` | | |
| 155 | ||
| 156 | If the project has no tests, say so in the report rather than letting a green build | |
| 157 | stand in as proof the refactoring was safe. | |
| 158 | ||
| 159 | **Two traps specific to WebAssembly targets.** Both let a refactoring look verified | |
| 160 | when it is not: | |
| 161 | ||
| 162 | - A **wasm build passing does not mean the tests ran.** `cargo build --target | |
| 163 | wasm32-wasip2` links with rustc's own bundled `rust-lld`, so it succeeds even | |
| 164 | with no C toolchain present, while `cargo test` — which builds for the *host* — | |
| 165 | fails at link time with ``error: linker `cc` not found``. If you see that error, | |
| 166 | the tests did not run; do not report the wasm build as verification. | |
| 167 | - An **extension cannot be exercised here.** No editor runs in this sandbox, so | |
| 168 | neither VS Code's Extension Development Host nor Zed's *Install Dev Extension* | |
| 169 | is available, and `@vscode/test-electron` needs an X server this kit does not | |
| 170 | install. Compile, unit-test and package — then say plainly that the integration | |
| 171 | path is untested rather than implying it passed. | |
| 172 | 5. Re-run the measurement from step 2 and compare against the previous run. | |
| 173 | ||
| 174 | Stop when any of these is true: | |
| 175 | ||
| 176 | - **The gate passes.** Report the before/after numbers from the Trend table. | |
| 177 | - **Five iterations have run.** Stop and report honestly: what improved, what remains, | |
| 178 | and why the rest is hard. | |
| 179 | - **Two consecutive runs show no improvement.** You are stuck — stop and explain what | |
| 180 | the blocker is rather than churning the code. | |
| 181 | ||
| 182 | Commit as you go, one focused commit per cluster of fixes, so the progression is | |
| 183 | visible in git as well as in `.quality/history.jsonl`. | |
| 184 | ||
| 185 | ## Do not game the gate | |
| 186 | ||
| 187 | The gate is only worth something if it is measuring the code. All of the following make | |
| 188 | the number go down while making the project worse — never do them, even if the gate is | |
| 189 | the only thing standing between you and "done": | |
| 190 | ||
| 191 | - Adding `exclude_patterns`, disabling a plugin, setting a plugin to `mode = "monitor"`, | |
| 192 | or raising a threshold in `.qlty/qlty.toml`. | |
| 193 | - Loosening `.quality/gate.json`. | |
| 194 | - Deleting, skipping, or emptying tests. | |
| 195 | - Adding blanket lint suppressions (`//nolint`, `// qlty-ignore`) to silence a finding | |
| 196 | you could fix. | |
| 197 | ||
| 198 | A narrowly-scoped suppression with a comment explaining why the finding is a false | |
| 199 | positive is legitimate. If you believe a threshold is genuinely wrong for this project, | |
| 200 | say so and let the user decide — do not change it yourself. | |
| 201 | ||
| 202 | ### The one plugin you may remove: a broken one | |
| 203 | ||
| 204 | "Disabling a plugin" above means a plugin that *finds things you would rather not fix*. A | |
| 205 | plugin that **analyses nothing** is a different object, and keeping it is not rigour: its | |
| 206 | findings crowd out the ones that matter and make the total unreadable. | |
| 207 | ||
| 208 | The two are indistinguishable from the report alone, so removal requires a **reproducible | |
| 209 | diagnosis**, never an impression. Establish all four of these before touching the config: | |
| 210 | ||
| 211 | 1. **It is not the language version or a specific construct.** Put a trivially small, | |
| 212 | unambiguously valid file of that language in a *fresh* git repository, enable only | |
| 213 | that plugin, and run it. If a five-line hello-world fails, no source construct is to | |
| 214 | blame. | |
| 215 | 2. **It is not this project.** Same point — the probe repository shares nothing with the | |
| 216 | codebase under measurement. | |
| 217 | 3. **It is not the environment qlty gives plugins.** qlty runs plugins with a stripped | |
| 218 | environment (empty `PATH` and `HOME`). Run the plugin's own binary directly, with your | |
| 219 | full environment, on the same probe file. Read the invocation qlty used from | |
| 220 | `.qlty/out/invoke-*.yaml`, which records the exact `script`, `env` and `stderr`. | |
| 221 | 4. **It is not the plugin family.** If sibling plugins sharing the same runtime produce | |
| 222 | real findings, the failure is specific rather than environmental. | |
| 223 | ||
| 224 | If all four hold, remove the plugin — and **write the diagnosis as a comment where you | |
| 225 | removed it**, so the next session finds the evidence instead of re-adding it on instinct. | |
| 226 | Note in your report which other tooling still covers that ground; if nothing does, the | |
| 227 | answer is to replace the plugin, not merely to drop it. | |
| 228 | ||
| 229 | `mode = "monitor"` is *not* the remedy here. It does not stop a plugin's findings from | |
| 230 | counting; it only changes how they are surfaced. A broken plugin left in monitor mode | |
| 231 | keeps polluting the total, which is how one ends up unnoticed for months. | |
| 232 | ||
| 233 | **Do not encode "plugin X is broken" into this skill.** Such a failure belongs to one | |
| 234 | version, one platform and one moment; a list of known-bad plugins ages badly and, worse, | |
| 235 | tempts the next agent to skip the diagnosis. The method above is what belongs here — its | |
| 236 | results belong in the project's own config and notes. | |
| 237 | ||
| 238 | ## Tuning the gate | |
| 239 | ||
| 240 | Defaults are strict: zero error-level issues, zero warning-level issues, zero smells. | |
| 241 | Override any subset by writing `.quality/gate.json`: | |
| 242 | ||
| 243 | ```json | |
| 244 | { | |
| 245 | "max_error": 0, | |
| 246 | "max_warning": 0, | |
| 247 | "max_note": null, | |
| 248 | "max_smells": 0, | |
| 249 | "max_file_complexity": 60, | |
| 250 | "max_total_complexity": null | |
| 251 | } | |
| 252 | ``` | |
| 253 | ||
| 254 | `null` disables a check. `max_file_complexity` flags individual files whose `complex` | |
| 255 | metric exceeds the limit, which is the most useful knob for a codebase with existing | |
| 256 | debt: it lets you hold the line on the worst files without demanding a perfect score | |
| 257 | everywhere at once. | |
| 258 | ||
| 259 | ## Reference | |
| 260 | ||
| 261 | - `qlty check --help`, `qlty smells --help`, `qlty metrics --help` for the full flag set. | |
| 262 | - Note `qlty check` has **no** `--json`; machine-readable output is `--sarif`. | |
| 263 | - Pass `--no-upgrade-check` to every qlty command — the sandbox blocks its update host. | |
| 264 | - Reports and history under `.quality/` are meant to be committed so progression | |
| 265 | survives sandbox recreation. Add `.quality/` to `.gitignore` if the user prefers them | |
| 266 | to stay local. |