chore(release): cut v1.25.0 "Workbench" - #272
Conversation
Version ceremony for the frontend-parity release: the ten increments T-FP-A through T-FP-G2 (PRs #260, #271, #262-#269) are on `main` and green, so this promotes them from `[Unreleased]` to a named, tagged release. ## Version bump `1.24.0` -> `1.25.0` across the workspace manifest and all 16 member crates (`rustysnes-libretro` inherits via `version.workspace = true`). `Cargo.lock` regenerated; `cargo metadata` and a full `cargo check --workspace` confirm the graph still resolves. MINOR, not MAJOR or PATCH, and the classification is load-bearing rather than habitual: every item is additive and default-off or default-inert (`video.stack` defaults to `Off`, the preset chain is empty, the debugger needs `debug-hooks`, GPU timing needs `gpu-timing`, a unity per-voice gain is a bit-exact bypass in the DSP), so an existing `config.toml` renders and sounds byte-identically. Save-state `FORMAT_VERSION` stays at 10 — no format break, which is what would have forced a MAJOR under `docs/adr/0006`. ## Documents updated - `CHANGELOG.md` — `[Unreleased]` promoted to `## [1.25.0] "Workbench" - 2026-07-30` with a release summary: the five headline capabilities, the additive/default-off posture, and a named section on the recurring dead-config defect class this release exists to remove. A fresh empty `[Unreleased]` is left in place for the next cycle. - `docs/STATUS.md` — the single source of truth. `Current release` moved to `v1.25.0 "Workbench"` with `v1.24.0` demoted into the precedent list, plus a paragraph stating explicitly that this release changes no emulation behaviour, that the accuracy dashboard / per-suite pass counts / coprocessor tier matrix below it are unchanged, and naming the two engine-side fixes that rode along (`Cpu::interrupts_taken`; the `delta::Chain` front-eviction defect, which affected the rewind buffer as much as the new greenzone). - `README.md` — version badge to v1.25.0, and the `Current Release` section rewritten. That section was **stale at v1.8.0** — seventeen releases behind — so this is a drift repair, not just a bump. The Roadmap list gained the three rungs that had accumulated undocumented (`v1.9.0`-`v1.20.0`, `v1.21.0`-`v1.24.0`, `v1.25.0`). - `to-dos/VERSION-PLAN.md` — a full `v1.25.0 "Workbench"` rung in the house style: per-ticket detail for A/B/C1/C2/D/E/F/G1/G2, the dead-config rule that came out of it ("grep a setting's readers, not its definition"), the two engine-side fixes, and the verification matrix. - `.gitignore` — ignores for the artifact types this release can now produce: perf CSV session logs, `.y4m`/`.wav` captures, and dropped-in shader presets and symbol maps. Deliberately **anchored and specifically named** (`/*.wav`, `/rustysnes-*.png`) rather than bare `*.wav`/`*.png` globs, which would also swallow committed test fixtures and `screenshots/`. Verified `screenshots/montage.png` is still tracked. `to-dos/ROADMAP.md` already carried the `T-FP-*` rows as `landed v1.25.0`; no change needed. ## Verification - `cargo test --workspace`: 804 passed, 0 failed, 1 ignored. - `cargo clippy --workspace --all-targets -- -D warnings`: clean. - Per-feature frontend clippy across all eight sets (default, full, emu-thread, debug-hooks, cheats, scripting, hd-pack, gpu-timing). The `full` set excludes `emu-thread`, so default+full alone is not sufficient — this gap has shipped a real bug before. - `no_std` core builds for `thumbv7em-none-eabihf`. - `cargo fmt --check` and `RUSTDOCFLAGS=-D warnings cargo doc --workspace` clean. - `markdownlint` clean on every edited region. The residual MD060 findings are a newer-linter rule the project's own docs note is ungated in CI, and the two MD024 hits are pre-existing at lines 1698/3489 of the CHANGELOG's history. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughThe release preparation updates the project and crate versions to 1.25.0, adds a savestate dependency, documents the Workbench release and roadmap, and ignores frontend capture and shader-related artifacts. ChangesWorkbench 1.25.0 release preparation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Comment |
Antigravity review (Gemini via Ultra)This PR cuts release Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@to-dos/VERSION-PLAN.md`:
- Around line 1504-1509: Update the Workbench increment count from ten to nine
to match the nine documented labels. Apply this correction in
to-dos/VERSION-PLAN.md lines 1504-1509, CHANGELOG.md lines 14-17, docs/STATUS.md
lines 44-46, and README.md lines 670-673; make no other documentation changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1c8ff9e9-b8ec-499c-9b1a-5f6af2346391
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lockand included byCargo.lock
📒 Files selected for processing (22)
.gitignoreCHANGELOG.mdCargo.tomlREADME.mdcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-core/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-savestate/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomldocs/STATUS.mdto-dos/VERSION-PLAN.md
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: lint
- GitHub Check: accuracysnes
- GitHub Check: test-light
- GitHub Check: build
- GitHub Check: build demo + docs
⚠️ CI failures not shown inline (2)
GitHub Actions: Antigravity PR Review / review: chore(release): cut v1.25.0 "Workbench"
Conclusion: failure
##[group]Run chmod +x scripts/agy-review.sh scripts/_agy_print.sh
�[36;1mchmod +x scripts/agy-review.sh scripts/_agy_print.sh�[0m
�[36;1mscripts/agy-review.sh�[0m
shell: /usr/bin/bash -e {0}
env:
GH_***REDACTED_SECRET_ASSIGNMENT***
AGY_EFFORT: high
##[endgroup]
[agy-review] reviewing doublegate/RustySNES#272
[agy-review] diff is 27279 bytes; inlined into the prompt
[agy-review] running agy via unbuffer (allocates a PTY) [attempt 1/3]
##[error]Process completed with exit code 1.
GitHub Actions: Antigravity PR Review / 0_review.txt: chore(release): cut v1.25.0 "Workbench"
Conclusion: failure
##[group]Run chmod +x scripts/agy-review.sh scripts/_agy_print.sh
�[36;1mchmod +x scripts/agy-review.sh scripts/_agy_print.sh�[0m
�[36;1mscripts/agy-review.sh�[0m
shell: /usr/bin/bash -e {0}
env:
GH_***REDACTED_SECRET_ASSIGNMENT***
AGY_EFFORT: high
##[endgroup]
[agy-review] reviewing doublegate/RustySNES#272
[agy-review] diff is 27279 bytes; inlined into the prompt
[agy-review] running agy via unbuffer (allocates a PTY) [attempt 1/3]
##[error]Process completed with exit code 1.
🧰 Additional context used
📓 Path-based instructions (12)
**/Cargo.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Do not configure the
scriptingandscript-wasmbackends as simultaneously enabled features; they are mutually exclusive.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlCargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlcrates/rustysnes-core/Cargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-savestate/Cargo.toml
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Do not commit or vendor the generatedsnesdev_wiki/mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits:<type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevantto-dos/sprint file.
**/*: Preserve the one-directional crate graph: chip crates must not depend on one another;rustysnes-coreties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keepdocs/STATUS.mdas the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNESv2.0orengine-lineageanchors as project releases.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlCargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlcrates/rustysnes-core/Cargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-savestate/Cargo.tomlREADME.mddocs/STATUS.mdCHANGELOG.mdto-dos/VERSION-PLAN.md
crates/rustysnes-*/**/*
📄 CodeRabbit inference engine (Custom checks)
For the full pull request diff against its base branch, any observable behavior change under
crates/rustysnes-<chip>/must be accompanied by an edit to the matchingdocs/<chip>.md; a crate change passes without documentation only when it does not alter observable behavior, with the non-behavioral change stated explicitly.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlcrates/rustysnes-core/Cargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-savestate/Cargo.toml
**/*.{rs,toml}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,toml}: Additive features must be default-off so shipped/native,no_std, and wasm builds remain byte-identical.
Never use or configure--all-features; validate opt-in feature combinations individually as required by the project recipe.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlCargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlcrates/rustysnes-core/Cargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-savestate/Cargo.toml
crates/**
⚙️ CodeRabbit configuration file
crates/**: Emulator core. Hot paths are allocation-free;unsaferequires a// SAFETY:comment
naming the invariant. Any change to save-stated fields needs aFORMAT_VERSIONbump and a
docs/adr/0006bump-log entry. Behavior changes must update the matchingdocs/<chip>.md
in the same change.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlcrates/rustysnes-core/Cargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-savestate/Cargo.toml
crates/*/Cargo.toml
⚙️ CodeRabbit configuration file
crates/*/Cargo.toml: The frontend pins one compatible GUI tier (currently winit 0.30 / wgpu 29 / egui 0.35 and
itsegui-wgpu/egui-winitsiblings). Flag a bump to any one of those that is not
accompanied by matching bumps to the others. Optional features are default-off by
convention so shipped, no_std and wasm builds stay byte-identical; flag a new feature added
to adefault = [...]list.
Files:
crates/rustysnes-gfx-shaders/Cargo.tomlcrates/rustysnes-monetization/Cargo.tomlcrates/rustysnes-script/Cargo.tomlcrates/rustysnes-cheevos/Cargo.tomlcrates/rustysnes-apu/Cargo.tomlcrates/rustysnes-ios/Cargo.tomlcrates/rustysnes-cart/Cargo.tomlcrates/rustysnes-netplay/Cargo.tomlcrates/rustysnes-test-harness/Cargo.tomlcrates/rustysnes-core/Cargo.tomlcrates/rustysnes-mobile/Cargo.tomlcrates/rustysnes-android/Cargo.tomlcrates/rustysnes-cpu/Cargo.tomlcrates/rustysnes-frontend/Cargo.tomlcrates/rustysnes-ppu/Cargo.tomlcrates/rustysnes-savestate/Cargo.toml
Cargo.toml
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Cargo.toml: Keep crate dependencies one-directional; chip crates must not depend on each other.
Additive features must remain default-off so native, no_std, and wasm outputs remain byte-identical in default configurations.
Do not usecargo clippy --all-features; usecargo clippy --workspace --all-targets -- -D warnings, with explicit feature jobs when needed.
Files:
Cargo.toml
**/*.{rs,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Chip-behavior changes must update both the chip implementation and the corresponding
docs/<subsystem>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
README.mddocs/STATUS.mdCHANGELOG.mdto-dos/VERSION-PLAN.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).
Files:
README.mddocs/STATUS.mdCHANGELOG.mdto-dos/VERSION-PLAN.md
docs/**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Before changing a subsystem, consult
docs/architecture.md,docs/STATUS.md,CONTRIBUTING.md, the relevant subsystem documentation, and applicable ADRs.New subsystems must add documentation under
docs/.
Files:
docs/STATUS.md
docs/**
⚙️ CodeRabbit configuration file
docs/**: Docs are the spec, not a history log. Flag claims that contradict the code, counts that
contradict the generateddocs/accuracysnes-coverage.md, and any statement of coverage that
is broader than what the corresponding test actually asserts.
Files:
docs/STATUS.md
CHANGELOG.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
User-visible changes must be recorded under the
[Unreleased]section.For the full pull request diff against its base branch, modify
CHANGELOG.mdwhen user-visible behavior changes, including emulator output, frontend features, CLI flags, public APIs, or AccuracySNES cartridge contents. Do not require it for purely internal changes, tests, comments, or CI configuration.
Files:
CHANGELOG.md
🔇 Additional comments (24)
.gitignore (1)
240-256: LGTM!Cargo.toml (1)
25-25: LGTM!crates/rustysnes-android/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-apu/Cargo.toml (1)
3-3: LGTM!crates/rustysnes-cart/Cargo.toml (1)
3-3: LGTM!crates/rustysnes-savestate/Cargo.toml (1)
3-3: LGTM!Also applies to: 10-10
crates/rustysnes-script/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-test-harness/Cargo.toml (1)
3-3: LGTM!CHANGELOG.md (1)
19-55: LGTM!docs/STATUS.md (2)
6-6: LGTM!
47-61: LGTM!README.md (3)
17-17: LGTM!
626-633: LGTM!
664-669: LGTM!to-dos/VERSION-PLAN.md (2)
1511-1520: LGTM!
1521-1537: LGTM!Also applies to: 1538-1541, 1542-1553, 1554-1557
crates/rustysnes-cheevos/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-cpu/Cargo.toml (1)
3-3: LGTM!crates/rustysnes-frontend/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-gfx-shaders/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-mobile/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-monetization/Cargo.toml (1)
4-4: LGTM!crates/rustysnes-ppu/Cargo.toml (1)
3-3: LGTM!crates/rustysnes-core/Cargo.toml (1)
3-3: 🎯 Functional CorrectnessNo dependency version updates needed. The
version = "1.4.0"path requirements already accept1.25.0, so these package bumps do not require matching dependency bumps.> Likely an incorrect or invalid review comment.
| ### `v1.25.0 "Workbench"` — the frontend catches up with the engine — **RELEASED 2026-07-30** | ||
|
|
||
| Ten reviewed increments (`T-FP-A` … `T-FP-G2`, one PR each) taking the egui shell from "boots games" | ||
| to a tool you can debug, profile, record, retime, and author a TAS with. **No emulation behaviour | ||
| changes**: additive and default-off or default-inert throughout, an existing `config.toml` renders | ||
| and sounds byte-identically, and save-state `FORMAT_VERSION` stays at 10. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use one count for the Workbench increment inventory.
The release text says “ten” while the named increments are T-FP-A, T-FP-B, T-FP-C1, T-FP-C2, T-FP-D, T-FP-E, T-FP-F, T-FP-G1, and T-FP-G2—nine labels. Update every occurrence to nine or add the missing increment.
to-dos/VERSION-PLAN.md#L1504-L1509: correct the introductory count to match the nine detailed labels.CHANGELOG.md#L14-L17: correct the release summary count.docs/STATUS.md#L44-L46: correct the release-policy count.README.md#L670-L673: correct the roadmap count.
As per path instructions, docs are the spec and claims must match the documented implementation.
📍 Affects 4 files
to-dos/VERSION-PLAN.md#L1504-L1509(this comment)CHANGELOG.md#L14-L17docs/STATUS.md#L44-L46README.md#L670-L673
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@to-dos/VERSION-PLAN.md` around lines 1504 - 1509, Update the Workbench
increment count from ten to nine to match the nine documented labels. Apply this
correction in to-dos/VERSION-PLAN.md lines 1504-1509, CHANGELOG.md lines 14-17,
docs/STATUS.md lines 44-46, and README.md lines 670-673; make no other
documentation changes.
Source: Path instructions
…iden Review fix for #272. The root-anchored patterns miss a capture written from a subdirectory, which is the common case for a test runner or a `cd`-ed shell. Each pattern's breadth is now set from what is actually COMMITTED rather than from caution: - Zero tracked `.y4m`, `.wav`, `.slangp`, `.cgp`, `.sym`, or `rustysnes-perf-*.csv` files exist, so all six widen to recursive `**/` safely. - Screenshots cannot. The repo tracks **712** PNGs, 30 of them `rustysnes-*.png` (the branding banners), and `assets/RustySNES_Icon_Pack/transparent/icon/rustysnes-icon-transparent-1024.png` matches even a generic `*-NNNN.png`. So the split is by precision, not by timidity: `screenshot::file_stem`'s default-title form (`rustysnes-` + exactly four digits + `.png`) has zero tracked collisions and goes recursive; the ROM-titled form `<title>-NNNN.png` stays root-anchored, because recursive it would ignore that icon. - `**/captures/` added alongside `/captures/` — pointing the emulator at a captures directory makes none of the above matter, which is the outcome the review's "scope strictly to /captures/" suggestion is really after. Verified both directions rather than asserted: `git ls-files | git check-ignore --stdin -v` reports NOTHING, i.e. no tracked file became ignored; and `captures/x.y4m`, `sub/dir/out.wav`, `rustysnes-perf-3.csv`, `rustysnes-0007.png`, `SUPER_MARIO-0001.png`, `deep/nest/rustysnes-0002.png`, `my.slangp` are all ignored. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review response — 1 fix landed in 3d9a448, 1 verification, 1 notedNo blocking issues raised. Both suggestions were worth acting on; one changed the code, one is answered with evidence. Suggestion 1 — root-anchored ignores miss subdirectory captures: fixedCorrect, and the common case is exactly the one named: a test runner or a The reason the patterns were anchored in the first place was fear of swallowing committed assets — so rather than trade one guess for another, I checked each extension against what is actually tracked:
Screenshots are the one place caution was justified, and the evidence is specific:
Verified in both directions rather than asserted: Suggestion 2 — confirm the constituent PRs are merged before tagging: verifiedRight thing to ask of a docs-only release commit, since the notes describe code that is not in this diff. All ten are merge-commit ancestors of this branch:
Each checked with Nitpick — tag published with the mergeThat is the plan: merge, then immediately |
Antigravity review (Gemini via Ultra)This PR performs release maintenance for cutting Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
Review fix for #272. `**/captures/` already matches the repo root — `**/` expands to zero or more directories — so the separate `/captures/` line ignored nothing the pattern above it did not. Verified in a scratch repo containing only `**/captures/`: both `captures/a.txt` and `sub/captures/b.txt` are ignored. Re-checked here after the removal, plus `git ls-files | git check-ignore --stdin` still reports no tracked file ignored. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review response (round 2) — 1 fix in d1500fc, 1 nitpick not realNo blocking issues. Suggestion —
|
Antigravity review (Gemini via Ultra)This PR bumps workspace crate versions to Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
… --arg Regression I introduced in #270 and did not verify. `--arg`/`--argjson` are **jq** flags; `gh api` has no such options and rejects them. So the delete pipeline errored on every run, `2>/dev/null` swallowed the message, and with `set -o pipefail` the non-zero status propagated and `set -e` killed the script — AFTER the review comment had already been posted. Two visible consequences, both confirmed: - The `review` job has gone red on every run since #270 merged. Every run before it succeeded; every run after posted its comment and then exited 1. - Stale review comments stopped being deleted. PR #272 accumulated **three** bot comments where the design is one, which is the observable proof the delete step had been dead rather than merely noisy. The fix fetches the JSON raw and pipes it into a real `jq`, which does take `--arg`/`--argjson`. (`--paginate` without `--jq` emits one array per page; jq reads that stream fine, applying `.[]` per input.) The ids are captured first and the loop reads from the captured value, so a listing failure degrades to "leave them in place" with a logged warning instead of taking the script down. stderr is no longer discarded on that call. Swallowing it is what made a guaranteed-failing command look like a mysterious exit code, and cost the diagnosis more than the noise would have. The script now ends with an explicit `exit 0`, so a stray non-zero from the delete loop can never again be reported as "the review failed" once the comment is published. Verified: `bash -n` clean; `shellcheck -S warning` shows only the pre-existing SC1007 on line 138; and the exact pipeline shape, run under `set -euo pipefail` against #272's real comment list, exits 0 and selects precisely the two stale comments while correctly excluding the newest. This lands on the release branch because it is my regression currently making a check red on this very PR; holding the release for a separate round-trip would add delay without adding safety. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Correction: the red
|
| review runs | result |
|---|---|
every run before #270 merged (a7cb807, 2c046e7, 6c5395d, 6c814a5, 1f0f670, 17cabc5, f4213e0, …) |
success |
every run after (3d9a448, d1500fc) |
failure — comment posted, then exit 1 |
And the functional damage: this PR accumulated three bot review comments where the design is exactly one. That is the delete step having been dead since #270, not merely noisy.
The fix
Fetch the JSON raw and pipe it into a real jq, which does take those flags. --paginate without --jq emits one array per page and jq reads that stream fine. The ids are captured first, so a listing failure degrades to "leave them in place" with a warning rather than taking the script down. The script now ends with an explicit exit 0 so a stray status from the delete loop can never again be reported as a failed review.
stderr is no longer discarded on that call. Swallowing it is precisely what turned a guaranteed-failing command into a mysterious exit code, and it cost more in diagnosis than the noise ever would have.
Verified: bash -n clean; shellcheck -S warning shows only the pre-existing SC1007 on line 138; and the exact pipeline shape, run under set -euo pipefail against this PR's real comment list, exits 0 and selects precisely the two stale comments (5136854716, 5136902785) while correctly excluding the newest.
Why it is on the release branch
It is my regression, currently making a check red on this very PR. Holding the release for a separate PR round-trip would add delay without adding safety. It is a self-contained commit and described as such.
On merging
review lives in a separate workflow and is not a required check — ci-success is the only one, and it gates on changes, setup, lint, test-light, full-test, no_std, bench, accuracysnes. But I would not have merged past a red check on the strength of that alone without first establishing what it was, which is what the above is.
Antigravity review (Gemini via Ultra)This PR cuts the Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
Correction to my previous comment:
|
…ice (#273) * test(reviewer): guard the comment-selection filter that has broken twice `agy-review.sh`'s filter decides which PR comments the bot DELETES. It has been wrong twice, and neither time was observable from the outside — the review posted fine on both occasions: 1. The just-posted comment was not reliably excluded. `new_comment_id` came from re-querying the comment list, which races GitHub's read replication; on a miss the exclusion degenerated to `select(.id != null)` — true for every id — and the run deleted the review it had just published. 2. jq's `--arg`/`--argjson` were passed to `gh api`, which has no such flags. It exited non-zero, `2>/dev/null` hid the message, and `set -o pipefail` plus `set -e` killed the script AFTER posting. Stale comments silently accumulated (PR #272 reached four) and the job went red with nothing in the log to say why. Both bugs lived in the same ~10 lines, and the second was mine. - The filter moves into a named `SELECT_STALE_JQ` so a test can exercise the real thing rather than a copy that can drift. - `scripts/agy-review-selftest.sh` lifts `MARKER` and that filter out of the script by pattern and runs them against fixtures — offline, no `gh`, no network, no self-hosted runner. Lifting by pattern is deliberate: rename or reshape either declaration and the test fails loudly instead of quietly checking something stale. - Six checks: the just-posted comment is never selected; other users and other bots are ignored (the author filter is a security control — without it anyone could paste the marker into a comment and have the bot delete comments on the next run); bot comments without the marker are ignored; an id of 0 excludes nothing real; the exclusion is genuinely by value; and `--arg`/`--argjson` are not passed to `gh api`. - Wired into the `lint` job, so it runs on every PR rather than only where `agy` is installed. Verified by injecting each historical regression and confirming the suite fails at the matching check and exits 1: dropping the id exclusion fails "excludes whichever id it is given"; moving `--arg` back onto `gh api` fails the flag check. Healthy tree exits 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(reviewer): make the self-test's own extraction fail loudly, not silently Review fixes for #273. All three findings describe silent-failure modes in a test whose entire premise is that silent failure is what went wrong twice — so all three are taken. - The `sed` range that lifts `SELECT_STALE_JQ` ends at the first line closing with a quote, so a filter body that ever did so would be TRUNCATED — and a truncated jq program can still compile and still return ids, i.e. the test would keep passing while checking something that is not the shipped filter. Two independent guards now: the extracted program must compile, and it must end with the `| .id` projection that makes it a complete pipeline rather than a prefix of one. The compile check passes `--arg marker` / `--argjson new_id` itself: the filter references both, and jq rejects an undefined variable at compile time, so omitting them fails a perfectly good program. Caught by running it. - The `gh api ... --arg` scan was line-based, so moving the flag onto a continuation line would have produced a false pass on precisely the mistake the check exists to catch. Continuations are folded before matching. - `tr '\n' ' '` left a trailing space that every expected value had to mirror. `paste -sd' '` instead, and the expectations now read as what they are. Verified by injecting each new failure mode: dropping the final `| .id` trips the truncation guard; putting `--arg` on a continuation line trips the flag check. Both exit 1. Healthy tree passes all six checks and exits 0. shellcheck clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
What
Version ceremony for the frontend-parity release. The ten increments
T-FP-A…T-FP-G2(PRs #260, #271, #262–#269) are already onmainand green; this promotes them from[Unreleased]to a named, tagged release and repairs the documentation drift that had accumulated alongside them.No code changes — manifests, docs, and
.gitignoreonly.Why MINOR
Every item in the release is additive and default-off or default-inert:
video.stackdefaults toOff, the preset chain is empty, the debugger needsdebug-hooks, GPU timing needsgpu-timing, and a unity per-voice gain is a bit-exact bypass in the DSP. An existingconfig.tomlrenders and sounds byte-identically.Save-state
FORMAT_VERSIONstays at 10 — no format break, which is what would have forced a MAJOR underdocs/adr/0006.Changes
Cargo.toml+ 16 member manifests1.24.0→1.25.0(rustysnes-libretroinherits viaversion.workspace); lock regeneratedCHANGELOG.md[Unreleased]→## [1.25.0] "Workbench" - 2026-07-30with a release summary; fresh empty[Unreleased]left for the next cycledocs/STATUS.mdREADME.mdCurrent Releaserewritten; Roadmap gained three undocumented rungsto-dos/VERSION-PLAN.mdv1.25.0 "Workbench"rung in house style, per-ticket.gitignore.y4m/.wavcaptures, dropped-in presets and symbol maps)Two things worth a reviewer's attention
README.md'sCurrent Releasesection was stale atv1.8.0— seventeen releases behind. That is a drift repair rather than a bump, and the Roadmap list had similarly stopped atv1.8.0; three rungs' worth of history (v1.9.0–v1.20.0,v1.21.0–v1.24.0,v1.25.0) were added.The
.gitignoreentries are anchored and specifically named (/*.wav,/rustysnes-*.png,/*.y4m) rather than bare*.wav/*.pngglobs. A bare glob would also swallow committed test fixtures and thescreenshots/assets the README embeds. Verifiedscreenshots/montage.pngis still tracked (git check-ignorereports it unignored).Verification
cargo test --workspace— 804 passed, 0 failed, 1 ignoredcargo clippy --workspace --all-targets -- -D warnings— clean--features fullexcludesemu-thread, so default+full alone is not sufficient — that exact gap has shipped a real bug beforeno_stdcore builds forthumbv7em-none-eabihfcargo fmt --checkandRUSTDOCFLAGS=-D warnings cargo doc --workspace— cleanmarkdownlintclean on every edited region. ResidualMD060findings are a newer-linter rule the project's own docs note is ungated in CI; the twoMD024hits are pre-existing at CHANGELOG lines 1698/3489After merge
Tag
v1.25.0and publish the release with full notes.🤖 Generated with Claude Code
Summary
Cuts the
v1.25.0 “Workbench”release, promoting the existing frontend-parity work from Unreleased without changing emulator-core behavior.FORMAT_VERSIONremains10; new functionality is default-off or inert.no_std, formatting, documentation, and markdown checks.The release claim would be false if any frontend feature changes default emulator behavior, the save-state format or accuracy coverage changes, the dossier denominator or pass assertions differ, or the stated verification matrix fails.