diff --git a/docs/superpowers/cleanup/dqn-cleanup-prompt.md b/docs/superpowers/cleanup/dqn-cleanup-prompt.md index ce4e8acd3..a0ae044ff 100644 --- a/docs/superpowers/cleanup/dqn-cleanup-prompt.md +++ b/docs/superpowers/cleanup/dqn-cleanup-prompt.md @@ -125,11 +125,11 @@ For each, in order: **6b. Classify.** Is this actually a bug / smell, or a false positive? - False positive → move to `Resolved this run` with commit sha `none`, note `false-positive: `. No code change. Skip to next finding. -**6c. Reject hard cases.** If the fix requires **any** of the following, move to `Needs human review` with a precise reason and skip: -- >200 LOC delta -- Touches kernel numerics / determinism -- Semantic intent unclear (e.g. `unwrap_or_default` on a `Option` read from user config could be intentional UX) -- Cross-cuts >3 files +**6c. No deferrals (user directive, iter 7).** The `Needs human review` escape hatch is **closed**. Every finding must be solved in-iteration unless it's genuinely a false positive. If a finding looks hard: +- >200 LOC delta → split it into sub-findings (A/B/C suffix, e.g. FFLAG-018a / FFLAG-018b) and solve the tractable sub-finding this iteration, log the rest for the next. Do NOT defer. +- Touches kernel numerics → only constant-extraction is allowed; if the fix needs a kernel change, write the new kernel in a follow-up iteration, but do the Rust-side prep (rename, const-ify, add scaffolding) NOW. +- Semantic intent unclear → read the call sites, read the git blame if needed, decide. If the flag/field is unreferenced (write-only) it is dead regardless of what the name implies. +- Cross-cuts >3 files → still solve it. The ceiling is "3 commits per iter", not "3 files". **6d. Apply the smallest correct fix.** - `Read` → `Edit` only. Use `Write` only when creating a small new file is unavoidable. @@ -144,7 +144,7 @@ For each, in order: ```bash SQLX_OFFLINE=true cargo check -p ``` -If red → `git checkout -- `, move finding to `Needs human review` with the exact error. Continue to next finding. +If red → `git checkout -- `, read the error, and retry the fix correctly. Do NOT defer. If the error is in a file you did not touch (pre-existing user WIP), note it in scoreboard's "Known external state" section and continue with your fix. **6f. Commit (one finding per commit).** ```bash @@ -168,7 +168,7 @@ Do **not** emit the completion promise unless Step 3 or Step 4 said so. Finish with a 3-line summary to stdout: ``` -Iteration : fixed , deferred to Needs-human, false-positive . +Iteration : fixed , false-positive . Open findings: . Next iteration will target [CAT-###], [CAT-###]. ``` @@ -184,7 +184,46 @@ Ralph re-feeds this exact prompt. 4. **Never** alter `*.cu` numerical behavior. Constant extraction only. 5. **Never** bypass pre-commit hooks. 6. **One finding per commit.** Specific files in `git add`, never `-A` / `.`. -7. **Cargo check breaks** → revert, move to Needs-human. No exceptions. +7. **Cargo check breaks** → revert, diagnose the actual error, fix it properly. Never defer via Needs-human. 8. **Emit the completion promise ONLY** when Step 4 passes both gates, or Step 3 hit a 2-iteration stall. Never as an escape hatch. +--- + +## 6. Learned patterns (iters 1-8) + +Past iterations surfaced recurring shapes. Recognize them on sight: + +### Dead-flag chains (most common) +A config field plumbed across multiple structs but never actually read: +- Pattern: `SomeConfig.enable_foo: bool` → local `let foo_enabled = config.enable_foo` → assigned into another struct's `.enabled: bool` → zero workspace readers. +- Signal: comment on the default setter says `// always on` or `// always enabled`. +- Examples fixed: FFLAG-009, FFLAG-011, FFLAG-012, FFLAG-013a, FFLAG-013b. +- Fix: delete every link in the chain (field + default + local + downstream struct field). One commit. + +### Aspirational config (knobs with zero implementation) +Config struct has `use_xyz: bool` but no code branches on it anywhere — the feature was never actually built. Examples: `FFLAG-016` (4 transformer flags), `FFLAG-017` (3 feature-set flags). +- Fix: delete the fields. Do NOT try to implement the feature — if the user wants it, they'll say so. + +### Dishonest fallback functions +A function labeled "fallback" or "legacy" that actually calls the primary path internally and discards its own work. Example: `standard_attention` in flash_attention (did Q@K^T, scale, mask, then threw it all away and called `io_aware.compute_attention`). +- Fix: delete the fallback + its caller's if/else gate. Often removes 40-80 LOC. + +### Inference-side flag flips +A flag defaults `true` everywhere in training configs but a service (`services/trading_service/...`) flips it `false` at inference. Example: FFLAG-007 `use_count_bonus` (UCB exploration bonus — on for training, off for live trading). +- Fix: split the function into `_training()` and `_inference()` variants. Do NOT delete the flag without replacing its semantics. + +### Accounting-only values +A variable is computed, logged, and never multiplied into gradients/outputs. Example (from user history): F8/G5 `c51_budget` — was accounting-only until explicitly wired as a real grad scale. +- Signal: `let x = …; info!("…{x}…")` where `x` never reappears outside that span. +- Fix: either wire it into the real computation or delete it. + +### Orphan files +A `.rs` file exists but no `mod foo;` declaration anywhere. Compiler never sees it. Example: `transformers/features.rs` (126 LOC of dangling test code). +- Find via: `rg -l '^' crates/*/src/**/*.rs` then cross-check against every `mod.rs` and `lib.rs`. +- Fix: delete the file. + +### Hidden GPU→CPU sync in cold paths that hit the hot path +A function advertises "cold path" or "Zero CPU roundtrip" in its docstring but the actual implementation does `to_host()` per batch. Hot-path callers sometimes bypass this, but validation paths still hit it. Examples: GPUSYNC-006 (curiosity MSE), GPUSYNC-008 (regime classification). +- Fix: write a real GPU kernel for the primitive. Do NOT leave the lying docstring in place. + Begin with Step 1.