diff --git a/docs/ticket/0yb2pgw-rustqual-baselines-go-stale-when-main-moves.md b/docs/ticket/0yb2pgw-rustqual-baselines-go-stale-when-main-moves.md index fc99fa615..4c61773b5 100644 --- a/docs/ticket/0yb2pgw-rustqual-baselines-go-stale-when-main-moves.md +++ b/docs/ticket/0yb2pgw-rustqual-baselines-go-stale-when-main-moves.md @@ -114,3 +114,16 @@ set to `github.event.pull_request.head.sha` for `qual` only, leaving `test` and the other tasks on the merge ref where testing the merge result is the point. Needs one thing verified first — whether `actions/checkout` treats an empty `ref:` as unset, which is what the other matrix entries would pass. + +----- + +- **From**: jp +- **Date**: 2026-10-06T21:46:15Z + +Decided: option 3, with no committed baseline. +Findings are compared one by one by `(rule, file, symbol)` rather than +per-category counts, and the comparison lives in a new `qual` crate (T-0yh5frr). +CI compares the merge ref against `HEAD^1` in the same job (T-0yh5npc), so the +measured tree and the base always match and the interim `checkout_ref` +workaround isn't needed. +This ticket closes when T-0yh5npc lands. diff --git a/docs/ticket/0yh5frr-add-a-qual-crate-that-diffs-rustqual-findings-by-identity.md b/docs/ticket/0yh5frr-add-a-qual-crate-that-diffs-rustqual-findings-by-identity.md new file mode 100644 index 000000000..925bb2c2e --- /dev/null +++ b/docs/ticket/0yh5frr-add-a-qual-crate-that-diffs-rustqual-findings-by-identity.md @@ -0,0 +1,76 @@ +# Add a qual crate that diffs rustqual findings by identity + +- **Status**: Todo +- **Kind**: Feature +- **Authors**: jp +- **Date**: 2026-10-06 +- **Label**: domain=tooling +- **Label**: type=task + +A new internal crate, `crates/internal/qual`, that turns rustqual reports into +individual findings, compares two reports, and renders the result. +CI, `cargo_check` and the burn-down loop harness all call it, so the gate and +its messages behave the same in every place it runs. + +## Why + +The gate today compares per-category counts in `.config/rustqual/baseline.json` +and `baseline-dry.json`. +Those files record a location only for IOSP violations (`violation_details`). +Every other finding (about 2,000 of the ~3,200 distinct ones) is stored as a +count. +That is why CI can say `srp_module_warnings: 207 -> 208` but not which file. +A committed baseline also goes stale as soon as `main` moves (see T-0yb2pgw). + +## Scope + +- **Parse** both passes (`config.toml` and `dry.toml`) into findings keyed by + `(rule, file, symbol)`. + Line numbers are not part of the key, so editing code above a function causes + no churn. + Duplicate keys count as a multiset. + IOSP appears in both passes, so de-duplicate it. + This ticket picks the input format (`json`, `sarif` or `ai-json`): whichever + gives a stable rule ID and symbol most cleanly. +- **Diff** a base report against a head report and return the new findings. + Before comparing, map renamed files using `git diff -M` between the two + revisions. +- **Base analysis:** run rustqual on `git worktree add --detach `. + Cache the report, keyed by base sha, rustqual version and config hash. + A run takes a few seconds, so the cache is a convenience, not a requirement. +- **Policy**, read from `.config/rustqual/` (exact file decided here): + - `exempt`: rules that never gate. + IOSP goes here, because rustqual has no way to turn it off. + - `enforced`: rules that have reached zero on `main`. + Any finding of an enforced rule fails, whether or not it appears in the + diff. + Rules are only ever added to this list; it never shrinks. +- **Render** the same findings as plain text, GitHub annotations (`::error + file=…,line=…::`), a Markdown table for `$GITHUB_STEP_SUMMARY`, and a + compact AI-oriented format. + Each finding carries rustqual's message plus the guidance from + `.config/rustqual/guidance/.md` when that file exists. +- **CLI:** + - `qual diff --base [--format …]` exits non-zero on new or enforced + findings. + - `qual report` lists every current finding (the burn-down picker needs this). +- **Library API:** jp-tools depends on the crate the same way it depends on + `ticket`. + +## Fail closed + +A report that can't be parsed, or a rustqual run that fails, is an error, never +a pass. +This replaces the canary in `qual-ci`: unit tests prove that a new finding fails +the gate and that a malformed report is an error. + +## Tests + +Keep the core pure: parsing, rename mapping, diff, policy and rendering are +tested against fixed fixture reports. +Only the thin shell around them spawns rustqual and git. + +## Out of scope + +Wiring into CI, `cargo_check` or the harness. +Those are separate tickets that depend on this one. diff --git a/docs/ticket/0yh5hqj-extract-a-reusable-loop-harness-from-bug-hunt.md b/docs/ticket/0yh5hqj-extract-a-reusable-loop-harness-from-bug-hunt.md new file mode 100644 index 000000000..eaf6378c4 --- /dev/null +++ b/docs/ticket/0yh5hqj-extract-a-reusable-loop-harness-from-bug-hunt.md @@ -0,0 +1,61 @@ +# Extract a reusable loop harness from bug-hunt + +- **Status**: Todo +- **Kind**: Chore +- **Authors**: jp +- **Date**: 2026-10-06 +- **Label**: domain=tooling +- **Label**: type=task + +Split the unattended bug-hunt script into a generic loop harness plus a +`bug-hunt` mission, with no change in behaviour. +The rustqual burn-down becomes a second mission on the same harness. + +The script currently lives untracked at `bug-hunt/run.sh` in the `rustqual-next` +worktree. +Its header still refers to `.bug-hunt/`. + +## Harness (shared) + +Everything that already works and isn't specific to bugs: + +- the loop, the wall-clock deadline (`--hours` / `--until`) and the iteration + limit; +- conversation rotation by commits, events or consecutive failures; +- rate-limit detection, capacity probes and waiting; +- network-stall detection and backoff; +- per-turn timeouts and interrupt handling; +- the commit probe and the refusal to run on `main` or with a staged index; +- facts measured from git after each turn (commits made, dirty files, gate + result); +- `RUNLOG.md`, with per-run log and snapshot directories. + +## Mission (per directory) + +- **Settings:** model, rotation limits, `--cfg` persona and skill flags, + compaction spec. +- **Prompt templates:** mission, continue, handover, repair. +- **`next-target` executable**, optional. + When a mission has one, the harness runs it before each turn and inserts its + output into the prompt; empty output means no targets remain and ends the run. + When a mission has none, the model picks its own target, as bug-hunt does + today. +- **`gate` executable:** runs after each turn and reports pass or fail with + output. + For bug-hunt this is today's `cargo check --workspace --all-targets`. +- **Discard on gate failure**, opt-in per mission: the harness resets its own + branch to the iteration's starting commit. bug-hunt keeps this off and keeps + today's "next turn repairs" behaviour. + +## Constraints + +- The first commit adds `run.sh` unchanged, so the extraction reads as a diff + against the original. +- Stays in bash. + Rewriting it in Rust at the same stage invites the second-system effect. +- Commit only the harness and mission definitions. + Ledgers, run logs, logs and snapshots stay gitignored. +- No behaviour change for bug-hunt. + Verify with a `--iterations 1` smoke run before and after, and compare the run + logs and prompts. +- Pick a location in the tree (for example under `.config/`) and record why. diff --git a/docs/ticket/0yh5j3r-file-upstream-rustqual-issues-found-during-adoption.md b/docs/ticket/0yh5j3r-file-upstream-rustqual-issues-found-during-adoption.md new file mode 100644 index 000000000..15556cea3 --- /dev/null +++ b/docs/ticket/0yh5j3r-file-upstream-rustqual-issues-found-during-adoption.md @@ -0,0 +1,42 @@ +# File upstream rustqual issues found during adoption + +- **Status**: Todo +- **Kind**: Chore +- **Authors**: jp +- **Date**: 2026-10-06 +- **Label**: domain=tooling +- **Label**: type=task + +rustqual problems found while adopting it, to report at +https://github.com/SaschaOnTour/rustqual/issues (pinned at v1.8.2 in the +`justfile`). +Reproduce each one on the current version before filing, and drop any that a +newer release has fixed. + +1. **`SRP-002` stops counting at the first `#[cfg(test)]` anywhere in a file.** + `count_production_lines` in `src/adapters/analyzers/srp/module.rs` breaks on + the first line starting with `#[cfg(test)]`, so a test-only helper partway + down a file hides all the production code below it. +2. **`--compare --fail-on-regression` misses new findings.** It treats a + regression as `quality_score` dropping. + Adding compliant code alongside a new finding dilutes the score enough to + hold it level. + A baseline that can't be parsed reports "not regressed" and exits 0 under + `--no-fail`. + Details are in the `qual-ci` recipe comment in the `justfile`. +3. **Dimensions that can't be disabled.** IOSP has no `enabled` key, and + `MAGIC_NUMBER` still fires under `[complexity] enabled = false`. + Disabling a dimension then reports its matching `qual:allow` markers as + orphaned (`ORPHAN_SUPPRESSION`), even though the findings keep firing. + See `.config/rustqual/dry.toml`. +4. **`DRY` is all-or-nothing.** Turning off `[duplicates]` also turns off + dead-code (`DRY-002`) and dead-type detection. + A per-rule switch would remove the need for a second pass. +5. **`ai-json` lacks the stable rule ID and the threshold/actual values** (for + example `length=72 > 60`). + Agents need both to fix a finding without a second lookup. +6. **Possible IOSP false positive.** `exponential_backoff` + (`crates/jp_llm/src/retry.rs`) appears as a violation although it seems to + call only std methods and do arithmetic. + Confirm with `--verbose` that a method name matched a project function before + filing. diff --git a/docs/ticket/0yh5m6r-record-the-rustqual-rule-policy-in-config.md b/docs/ticket/0yh5m6r-record-the-rustqual-rule-policy-in-config.md new file mode 100644 index 000000000..ac93cd1c7 --- /dev/null +++ b/docs/ticket/0yh5m6r-record-the-rustqual-rule-policy-in-config.md @@ -0,0 +1,62 @@ +# Record the rustqual rule policy in config + +- **Status**: Todo +- **Kind**: Chore +- **Authors**: jp +- **Date**: 2026-10-06 +- **Label**: domain=tooling +- **Label**: type=task + +Decide, for each rustqual rule, whether JP enforces it, tunes it, or exempts it. +Record each decision and its reason as a comment in `.config/rustqual/`, in the +style the existing configs already use. +The decisions belong to the maintainers. + +**For an assistant picking this up:** apply the items under *Decided* as +written. +For each item under *To decide*, don't choose. +Draft the entry instead: the current finding count, three or four real examples +from the workspace, and the options with what each one costs. +Then stop and ask the user to decide before writing it into the config. +The PR is where the decisions get reviewed. + +Depends on T-0yh5frr for the `exempt` policy list. + +This has to land before the burn-down mission runs, so the loop doesn't refactor +code to satisfy rules JP never adopted. + +## Decided + +- **IOSP: exempt (option A).** rustqual can't disable it, so it goes in the + `qual` crate's `exempt` list rather than behind suppressions (862 violations, + 6.2% of functions, which is over the 5% `max_suppression_ratio`). + Revisit if, after the tier 2 burn-down (complexity, length, nesting), reviews + still turn up functions within those limits that mix orchestration and logic + badly. + The two alternatives: + - **B:** gate IOSP on new code only; + - **C:** burn it down fully, with `allow_recursion = true`. +- **`[srp] file_length = 800`.** rustqual counts only code lines: blank lines, + `//`, `///` and block comments don't count, and counting stops at the first + `#[cfg(test)]`. + The default of 300 is rustqual's own house style. + Note that `srp_module_warnings` also covers the independent-cluster cohesion + check, so it won't drop to zero from the length change alone. + +## To decide + +- **`[tests]` thresholds.** `*_tests.rs` files have no `#[cfg(test)]`, so every + code line counts and they inherit 800 and the 60-line function limit. + JP's tests are deliberately self-contained, which makes them long. +- **`allow_expect`.** `EventId::random` uses `.expect()` with a documented `# + Panics` section, a deliberate pattern. + Decide whether `.expect()` is allowed while `.unwrap()` keeps firing (194 + error-handling findings in total). +- **`unsafe`** (74 findings). + Suppress audited FFI and syscall sites with `qual:allow(unsafe)`, which + doesn't count toward the suppression ratio, or exempt the rule. +- **`[boilerplate] accepted_display_idioms`.** Declare JP's house style for + trivial `Display` impls, so `BP-002` enforces that style instead of firing on + it. +- **Any rule** whose findings the maintainers consider noise after looking at + samples. diff --git a/docs/ticket/0yh5npc-gate-ci-on-new-rustqual-findings-against-the-base-commit.md b/docs/ticket/0yh5npc-gate-ci-on-new-rustqual-findings-against-the-base-commit.md new file mode 100644 index 000000000..423baf457 --- /dev/null +++ b/docs/ticket/0yh5npc-gate-ci-on-new-rustqual-findings-against-the-base-commit.md @@ -0,0 +1,44 @@ +# Gate CI on new rustqual findings against the base commit + +- **Status**: Todo +- **Kind**: Feature +- **Authors**: jp +- **Date**: 2026-10-06 +- **Label**: domain=tooling +- **Label**: type=task + +Replace the committed-baseline gate in `just qual-ci` with `qual diff` from +T-0yh5frr. +CI analyses the tree it builds and its base in the same job, and reports only +the findings that are new. + +Closes T-0yb2pgw: a baseline produced in the same run can't go stale, and it +always shares a base with the measured tree. + +## Base revision + +- `pull_request`: the checkout is `refs/pull/N/merge`, so the base is `HEAD^1`, + the same parent the `changes` job already diffs against. +- `push` to `main`: the base is `github.event.before`. + On a new branch, where `before` is all zeros, there's nothing to compare: + analyse only, and fail only on enforced rules. +- The `qual` task's checkout needs enough history to reach the base. + `fetch-depth: 2` covers pull requests; on push, fetch the `before` sha + explicitly. + +## Output + +- `::error` annotations for new findings only, so GitHub's display cap no longer + hides the regression behind existing findings. +- The full new-findings table, with guidance, in `$GITHUB_STEP_SUMMARY`. +- On success, one line: the head and base counts per pass. + +## Remove + +- `.config/rustqual/baseline.json`, `baseline-dry.json` and `regressions.jq`. +- The `qual-baseline` recipe. +- The canary in `qual-ci`. + The fail-closed behaviour is covered by unit tests in the `qual` crate. +- Check the `justfile`, the configs and `docs/` for any other references to the + baselines. + Keep `just qual` as the exploratory entry point. diff --git a/docs/ticket/0yh5pja-report-new-rustqual-findings-from-cargo_check.md b/docs/ticket/0yh5pja-report-new-rustqual-findings-from-cargo_check.md new file mode 100644 index 000000000..a82cf2563 --- /dev/null +++ b/docs/ticket/0yh5pja-report-new-rustqual-findings-from-cargo_check.md @@ -0,0 +1,37 @@ +# Report new rustqual findings from cargo_check + +- **Status**: Todo +- **Kind**: Feature +- **Authors**: jp +- **Date**: 2026-10-06 +- **Label**: domain=tooling +- **Label**: type=task + +Make the `cargo_check` tool (`.config/jp/tools/src/cargo/check.rs`) report the +rustqual findings that CI would fail on, so an assistant can find out before +pushing. +It uses the `qual` crate from T-0yh5frr as a library, so it gives the same +verdict and the same messages as CI. + +## Behaviour + +- Add a `QualCheck` step next to `ComfortCheck`, with the same `Clean` / + `Findings(note)` / `Failed(stderr)` shape. +- Run it only after clippy succeeds: findings on code that doesn't compile are + noise. +- The head is the working tree, including uncommitted changes. + The base is `merge-base(HEAD, origin/main)`, falling back to `main` when + there's no remote, and its report is cached by `qual`. +- The `package` parameter filters which findings are *reported*, not what is + analysed. + Dead-code and duplicate detection need the whole workspace. +- Render in the AI-oriented format with per-rule guidance, and list the allowed + escape hatches (see the escape-hatch ticket). +- Update the tool summary in `.jp/mcp/tools/cargo/check.toml`, and the + `rust-development` skill description if it lists what `cargo_check` covers. + +## Tests + +Follow `check_tests.rs`: cover clean, findings and failure with +`MockProcessRunner`, plus the case where clippy fails and the qual step is +skipped. diff --git a/docs/ticket/0yh5qjg-surface-rustqual-suppressions-and-config-edits-in-the-gate.md b/docs/ticket/0yh5qjg-surface-rustqual-suppressions-and-config-edits-in-the-gate.md new file mode 100644 index 000000000..93d01f3d3 --- /dev/null +++ b/docs/ticket/0yh5qjg-surface-rustqual-suppressions-and-config-edits-in-the-gate.md @@ -0,0 +1,33 @@ +# Surface rustqual suppressions and config edits in the gate + +- **Status**: Todo +- **Kind**: Feature +- **Authors**: jp +- **Date**: 2026-10-06 +- **Label**: domain=tooling +- **Label**: type=task + +With no committed baseline, the gate can only be silenced in two ways: a +`qual:allow` marker in the code, or an edit under `.config/rustqual/`. +Make both visible wherever the gate reports. +Builds on T-0yh5frr. + +## Gate behaviour (`qual diff`) + +- List `qual:allow`, `qual:api` and `qual:test_helper` markers added since the + base, in their own section, with file, line and reason. +- List changed files under `.config/rustqual/` in their own section. +- Fail on any added `qual:allow` that has no `reason:`. +- Neither section fails the gate otherwise. + They exist so a reviewer sees them, and so CI and `cargo_check` show them to + the author. + +## Assistant instructions + +Update the `dev` and `rfd-implementor` personas, and the `coding` skill if it +covers verification: + +- Fix the finding. + Don't add a suppression, change a threshold, or edit `.config/rustqual/` + unless the user asked for it in this conversation. +- If a finding looks wrong, say so and stop; don't route around it. diff --git a/docs/ticket/0yh5sy9-add-a-rustqual-burn-down-mission-to-the-loop-harness.md b/docs/ticket/0yh5sy9-add-a-rustqual-burn-down-mission-to-the-loop-harness.md new file mode 100644 index 000000000..fa388e280 --- /dev/null +++ b/docs/ticket/0yh5sy9-add-a-rustqual-burn-down-mission-to-the-loop-harness.md @@ -0,0 +1,75 @@ +# Add a rustqual burn-down mission to the loop harness + +- **Status**: Todo +- **Kind**: Feature +- **Authors**: jp +- **Date**: 2026-10-06 +- **Label**: domain=tooling +- **Label**: type=task + +A second mission for the loop harness from T-0yh5hqj, which fixes rustqual +findings unattended, one at a time. +It depends on: + +- T-0yh5frr, for `qual report` and `qual diff`; +- T-0yh5m6r, the rule policy, which must have landed so the loop never works on + a rule JP exempts; +- T-0yh8q2s, the per-rule guidance files the prompt includes. + +## One run, one rule + +A run targets a single rule, passed to the mission as `--rule `, and works +on its own branch named after that rule (for example `qual/cx-004`). +Running tier by tier means running the harness once per rule, in the tier order +below. +This keeps every branch uniform: one rule, reviewed once. + +## `next-target` + +Read `qual report` and print the next finding of the run's rule, one file at a +time. +Print nothing when none is left, which ends the run. +Skip findings already recorded as attempted or rejected. +Insert the rendered finding and its guidance file into the prompt, so the model +fixes exactly that finding and doesn't wander. + +## `gate` + +The harness measures all of this; it doesn't take the model's word for it: + +- the target finding is gone; +- `qual diff --base ` is empty; +- no change under `.config/rustqual/` and no new `qual:allow` marker; +- `cargo check` passes, and tests pass for the affected crate; +- one commit, with a subject that follows the repository's convention. + +On failure the harness discards the iteration (mission setting on) and records +the finding as attempted. + +## Rejections + +The model may append `{key, reason}` to `rejected.jsonl` instead of fixing a +finding, when it judges the finding shouldn't be fixed. +A maintainer reviews these and turns each into a reasoned suppression or a +policy change. +The loop never acts on them itself. + +## Tier order + +| Tier | Rules | Approx. count | +| ---- | ----------------------------------------------------------------------------------------------------------- | ------------- | +| 1 | dead code and types, wildcard imports, magic numbers, remaining boilerplate | ~800 | +| 2 | function length, cognitive and cyclomatic complexity, nesting, duplicates and fragments, untested functions | ~800 | +| 2 | error handling (`unwrap` → `?`): reviewed as behaviour changes, because the failure mode changes | 194 | + +Out of scope: SRP struct and module splits, coupling, and anything tier 3. +Those are human-led and get their own tickets, one per file, once the burn-down +shows what remains. + +## Output and ratchet + +- Each run's branch holds commits for one rule. + Open PRs from it grouped by crate, so a single review covers one rule in one + crate. +- Once a rule reaches zero on `main`, add it to the `enforced` list in a small + follow-up PR. diff --git a/docs/ticket/0yh5sya-write-a-process-rfd-for-unattended-agent-runs.md b/docs/ticket/0yh5sya-write-a-process-rfd-for-unattended-agent-runs.md new file mode 100644 index 000000000..26839418e --- /dev/null +++ b/docs/ticket/0yh5sya-write-a-process-rfd-for-unattended-agent-runs.md @@ -0,0 +1,23 @@ +# Write a process RFD for unattended agent runs + +- **Status**: Todo +- **Kind**: Chore +- **Authors**: jp +- **Date**: 2026-10-06 +- **Label**: domain=tooling +- **Label**: type=task + +Once the loop harness (T-0yh5hqj) and the rustqual mission (T-0yh5sy9) have run +for real, write a Process RFD describing how JP runs unattended agent work. +Write it from what actually happened, not ahead of time. + +Cover: + +- what a mission is and how to add one; +- when the harness may discard an iteration by resetting its own branch, and why + the assistant itself never rewrites history; +- memory across conversations: the ledger, the attempted and rejected lists, and + rotation; +- how rejected findings and overnight output get reviewed and merged, including + the expected PR granularity; +- what an unattended run must never touch. diff --git a/docs/ticket/0yh8q2s-write-per-rule-rustqual-guidance-for-tier-1-and-2-rules.md b/docs/ticket/0yh8q2s-write-per-rule-rustqual-guidance-for-tier-1-and-2-rules.md new file mode 100644 index 000000000..db5e6717f --- /dev/null +++ b/docs/ticket/0yh8q2s-write-per-rule-rustqual-guidance-for-tier-1-and-2-rules.md @@ -0,0 +1,58 @@ +# Write per-rule rustqual guidance for tier 1 and 2 rules + +- **Status**: Todo +- **Kind**: Chore +- **Authors**: jp +- **Date**: 2026-10-06 +- **Label**: domain=tooling +- **Label**: type=task + +Write `.config/rustqual/guidance/.md` for each rule that gates, so a +finding tells the reader how to fix it the JP way, not just what rustqual +detected. +The `qual` crate (T-0yh5frr) appends the matching file to every rendered finding +in CI, `cargo_check` and the burn-down loop. +Depends on T-0yh5frr for the file layout and rule IDs. + +## Rules to cover + +Tier 1 and tier 2, the rules the burn-down mission (T-0yh5sy9) works on: + +- dead code and dead types; +- wildcard imports; +- magic numbers; +- boilerplate (`BP-*`); +- function length, cognitive and cyclomatic complexity, nesting; +- duplicates and fragments; +- untested functions; +- error handling (`unwrap`, `expect`, `panic!`). + +Skip rules that T-0yh5m6r exempts, and write the guidance against the thresholds +that ticket sets. + +## Each file + +Keep each one short, roughly 10 to 30 lines. +Every file covers: + +- **What the rule means in JP terms**, in a sentence or two. +- **The preferred fixes, in order**, tied to project conventions. + Some examples of the kind of thing to write: + - complexity and length: return early, extract a helper only when it has a + name worth reading, and remember that `*_tests.rs` files sit outside + production thresholds; + - magic numbers: a named `const` next to its use, not a constants module; + - error handling: propagate with `?` into the crate's typed error, and use + `expect` only with a documented `# Panics` section. +- **What not to do.** Don't hide logic in a closure, don't split one function + into a chain of single-use helpers, and don't touch unrelated code. +- **The escape hatch, stated once:** a `qual:allow` with a `reason:`, and only + when the user has asked for it. + +## Verify + +Render a sample of real findings for each rule through `qual diff --format ai` +and check that each message gives enough to act on without looking anything else +up. +Where rustqual's own message falls short (no threshold, no actual value), add +the case to T-0yh5j3r.