cleanup: close Needs-human escape + document learned patterns

Per user directive from iter 7 ("no deferred tasks, solve properly"),
the Needs-human review escape hatch is closed. Updated:
- Step 6c: replaced "move to Needs human review" with split-into-
  sub-findings guidance; every finding must be solved.
- Step 6e: cargo-check failure now requires diagnosis + retry, not
  deferral. Pre-existing external errors go to Known external state.
- Step 8 summary line: dropped the "deferred j" counter.
- Rule 7: similar rephrasing.
- New §6: learned patterns from iters 1-8 (dead flag chains,
  aspirational config, dishonest fallbacks, inference-side flag flips,
  accounting-only values, orphan files, hidden GPU sync).

Ralph re-feeds this file verbatim, so the next iteration picks up the
updated rules automatically.
This commit is contained in:
jgrusewski
2026-04-20 23:51:34 +02:00
parent aa43b6dddb
commit b27a35ca28

View File

@@ -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: <reason>`. 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<String>` 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 <owning-crate>
```
If red → `git checkout -- <touched files>`, move finding to `Needs human review` with the exact error. Continue to next finding.
If red → `git checkout -- <touched files>`, 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 <N>: fixed <k>, deferred <j> to Needs-human, false-positive <f>.
Iteration <N>: fixed <k>, false-positive <f>.
Open findings: <count>. 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.