Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ These cross-cutting decisions span multiple files. Reading individual chip docs
- **Never write a commit body with `git commit -m` in this harness.** The shell is zsh, so **backticks are command substitution** and `<word>` is an input redirect. A v2.4.0 message documenting three `gh` invocations lost all three to substitution and emitted `no such file or directory: owner` from a literal `repos/<owner>/<repo>/...` — the commit succeeded with a mangled body reading "recorded, because it is reusable: RETURNS THE PULL REQUEST". This project's house style puts command examples in commit bodies routinely, so `-m` is structurally the wrong tool: write the message to a file and use `git commit -F`. After amending, grep the message for each phrase that was supposed to survive.
- **A test that reimplements its subject is testing itself, and it will agree forever.** v2.4.0 hit this in a test written *for a review finding*: it declared a local `fn strip(t) { t.trim_start_matches([' ', '*']) }` and asserted against that, so deleting the production stripping came back **NOT CAUGHT**. Only the mutation pass could see it — the test passed, read correctly, and covered nothing. The fix is the one this release needed three times over: **extract the decision into a named item both the code and the test call.** The other two were `atomic_write`'s injectable rename predicate (with it hard-wired, the exhaustion branch is unreachable on Unix and a mutation returning `Ok(())` for a save that never happened went uncaught) and `TimelineWatch` (`DebuggerOverlay::new` needs a window and a wgpu device, so nothing living only inside it is unit-testable). In all three the code **read** as testable beforehand.
- **Never byte-slice in a panic or format path.** `&text[at..(at + 24).min(text.len())]` panics when the offset lands inside a multi-byte character, and these documents are full of em-dashes and arrows — so the audit would crash *while formatting the diagnostic*, replacing the message explaining the real failure with a char-boundary error about the reporting code. **A diagnostic that can crash the diagnosis is worse than none**, because the failure it exists to explain becomes harder to read than if the excerpt were omitted. Use `s.chars().take(n).collect::<String>()`.
- **Two separately-verified facts do not verify the claim you assemble from them — and `AGENTS.md` is a notes file, not an oracle.** v2.4.1's published release notes carried a section headed "a claim v2.3.9 made that v2.3.9 did not ship", asserting that v2.3.9's release body *described* `release_anchor_audit.rs` while the tag did not contain it. **False, and it shipped to users.** Both inputs were real and each was checked: the file genuinely is absent from the tag (`git cat-file -e v2.3.9:crates/rustynes-test-harness/tests/release_anchor_audit.rs` fails), and v2.3.9 genuinely is where the anchor drift was found. The conjunction was never checked, and it was **one command away** — `gh release view v2.3.9 --json body`. The body says the eight drifted documents "All now read v2.3.9", a MANUAL re-synchronisation which IS in its tag, and mentions no audit, no gate, no "15 anchors" anywhere. A hand fix followed by a later commit adding the mechanism is ordinary sequencing, not an overclaiming release note. **Two propagation mechanisms matter more than the instance.** First, the sentence was written into `AGENTS.md` during the v2.4.0 work and the v2.4.1 summary was then drafted FROM `AGENTS.md` — so an unverified claim entering this file is laundered into a verified one by the next release that quotes it; treat every factual assertion here as re-checkable, and re-check it before it reaches a user-facing surface. Second, the claim had the shape this project rewards — a recursive irony, "the same failure one level up" — and that shape is exactly what suppresses the check, because it reads as an insight rather than as an assertion needing evidence. **The bar: before writing that a document, release, commit or person SAYS something, open it and grep for the words.** Corrected in #431, and retracted in place rather than deleted, because the claim was an accusation against a shipped release and silently removing it would erase the record that it was made.
- **Verify a reviewer's claim before writing the fix, especially when their other findings were right.** On #427 a reviewer stated `starts_with("[workspace.package]")` also matches `[workspace.package.metadata]`. Plausible, a real class of bug, and the fix plus a commit body describing "the regression I introduced" were written before it was tested. It is **false**: the literal ends with `]` and the sub-table has `.` there, so the match is `false`; injecting such a sub-table and running the audit reads `2.3.9` under both forms. The same reviewer's two other findings that pass were both correct — which is exactly what makes the third easy to wave through. Adopt the change if it is better anyway (it was), but write down what is *true*, not a fix for a bug that never existed.
- **The libretro "let-chains are unstable" review claim is FALSE and has now been raised seven times.** `if let Some(x) = e && cond` is stable in **edition 2024**, which this workspace uses on a pinned stable 1.96.0; the identical construct has been in `libretro_info_audit.rs` on `main` since v2.3.5; and CI's `fmt + clippy + rustdoc` job has compiled it green at five distinct SHAs. Do not "fix" it. Refute with the edition, the existing site, and the green lint job **on the current SHA** rather than by reference to earlier ones.
- **The workspace cannot carry a SemVer pre-release version.** Setting `[workspace.package] version = "2.3.9-rc.1"` fails before any test runs: `failed to select a version for the requirement rustynes-apu = "^2.0.0" / candidate versions found which didn't match: 2.3.9-rc.1`. A caret requirement does not match a pre-release, so every intra-workspace dependency would have to be rewritten first. Relevant when reasoning about version parsing — `release_anchor_audit.rs` guards the case anyway, and its `version_core` is tested directly because the manifest route is unreachable.
Expand Down
Loading