fix(frontend): omit the empty latency map, and backfill the CHANGELOG

CHANGELOG.md

@@ -14,6 +14,102 @@ cycle-accurate core later replaced.
 
 ## [Unreleased]
 
+Post-v2.3.7 work, landed but not yet cut.
+
+### Added
+
+- **The Divergence Lens — which pixels differ, not just which frame, and why.**
+  (#407, the whole of v2.3.8 "Parallax".) Surfaced as a panel under **Tools →
+  Analysis**, over a headless `rustynes_probe::divergence` core that is tested
+  independently of it. `Probe` could already say whether two
+  configurations of the same ROM diverge and at which frame, because a trial
+  reduces each frame to one `u64`. That reduction is the right shape for
+  *detecting* a difference and the wrong shape for *explaining* one: a hash says
+  frame 412 differs and cannot say which pixel, so it has nothing to hand to
+  Pixel Provenance, which is where an answer actually lives.
+
+  `divergence::localise` re-runs both configurations to the detected frame,
+  keeps the full output instead of its hash, and reports the *shape* of the
+  difference — population count, first pixel in raster order, and the inclusive
+  bounding box. Count and box separate kinds of bug from each other: one pixel is
+  a sprite or a palette entry, 256 in a row is a scanline, tens of thousands is a
+  scroll or a mode change.
+
+  It localises on the **index** framebuffer — 256x240 `u16`s of
+  `(emphasis << 6) | colour`, the PPU's own per-pixel output before the palette
+  lookup — which is half the bytes and at least as sensitive, since the RGBA
+  buffer is a pure function of it given the same palette.
+
+  Three answers, and the third is the point: `Identical`, `Differs`, and
+  **`Inconclusive`** for an exhausted budget or two trials that cannot be
+  compared. The Latency Oracle's precedent applies directly: "I stopped looking"
+  must not arrive wearing the same shape as "they agree". The budget is checked
+  up front for all four trials, so spending two on detection and then finding the
+  localisation pair unaffordable cannot consume the budget that would have
+  answered the question.
+
+  Beyond locating a difference, the Lens **explains** it. Trial-scoped
+  provenance capture lets a located pixel be handed to the machinery that already
+  answers "what wrote this, and from which instruction", so the answer is a cause
+  rather than a coordinate — and it closes v2.3.8 item B without bisection. An
+  **audio** lens resolves a divergence to the CPU cycle, the cadence at which the
+  mix is genuinely computed.
+
+  One defect was found and fixed inside the same work: the Lens left the emulator
+  **thirty frames ahead** of where it started. A trial restores the anchor on the
+  way in and not on the way out, which is deliberate — it is what lets the Lens
+  read the trial's final frame off `nes` directly — but the outermost caller has
+  to put the timeline back, and did not.
+
+- **A Latency Oracle measurement is remembered per game.** (#410.) Reopening a
+  game shows what was measured last time instead of an empty panel, keyed on the
+  ROM SHA-256 in `[input]` — the same key shape `graphics.hd_packs` already uses.
+  `#[serde(default)]` so an older config loads unchanged, plus
+  `skip_serializing_if` so the key stays out of the file until there is something
+  to store: a user who never opens the panel carries nothing and their config is
+  not rewritten.
+
+  **Remembering is not applying.** Nothing here touches `run_ahead`, and
+  restoring never queues a pending apply, so a depth measured in an earlier
+  session is still one explicit click from being applied. An **inconclusive**
+  result is not remembered at all: a stored "I could not tell" is
+  indistinguishable from a stored answer once it has lost the context that
+  produced it.
+
+- **The two-acquisition lock race is measured rather than reasoned about.**
+  (#409.) The `needs_nes` render arm — taken exactly when a debugger or tool
+  panel is open — acquires the emulator lock twice per redraw, and drops it in
+  between so composite work does not hold the emulator. If the emulation thread
+  takes the lock in that gap, the screen shows frame N while a panel describes
+  N+1.
+
+  `Nes::cycle()` is read at both acquisitions and compared: it is cumulative and
+  monotonic, and `produce_one_frame` holds the lock across a **whole** frame, so
+  any difference at all means at least one complete frame landed in the gap.
+  Both a hit count and a denominator are kept — "the race did not fire" and
+  "nothing was observed" both read as zero hits, and only the denominator
+  separates them.
+
+### Changed
+
+- **The accuracy battery runs at review time, scoped by path.** (#408.) `setup`
+  computed one `full` flag and `test-roms` ran only when it was true, so a
+  regular feature PR never ran the battery and an accuracy regression could not
+  be caught on the PR that caused it — #403 is the worked example. A second
+  `paths-filter` output covers the chip crates, core, `rustynes-gamedb`, the test
+  harness and `tests/`, and `test-roms` now runs when it *or* the existing full
+  flag is true. `rustynes-gamedb` is included for a non-obvious reason: it
+  rewrites the iNES header on load, so it changes what the emulator *is* before a
+  cycle runs.
+
+- **Provisioning steps are bounded, not just the jobs.** (#408, #409.) The
+  cross-compile gate's `apt-get update && apt-get install` were network fetches
+  with no timeout of their own, so a stalled mirror hung until the job timeout
+  fired and the run was reported as cancelled rather than as what it was — four
+  times during the v2.3.7 cut. Now one bounded, thrice-retried helper, with
+  elevation outside `timeout` so a killed fetch cannot orphan `apt-get` holding
+  the dpkg lock.
+
 ## [2.3.7] - 2026-08-19 - "Overtone" (the instruction behind every mixed cycle)
 
 An *overtone* is the structure inside a sound that a single pitch reading throws

crates/rustynes-frontend/src/config.rs

@@ -150,15 +150,25 @@ pub struct InputConfig {
     /// run-ahead depth, which is the field directly above; a measurement and the
     /// setting it recommends belong together.
     ///
-    /// `#[serde(default)]` = empty, so a pre-v2.3.9 config round-trips
-    /// byte-identically and a user who never opens the panel carries nothing.
+    /// `#[serde(default)]` = empty, so a pre-v2.3.9 config LOADS unchanged, and
+    /// `skip_serializing_if` keeps the key out of the file entirely until there
+    /// is something to store — so a user who never opens the panel carries
+    /// nothing and their config round-trips byte-identically.
+    ///
+    /// The two halves are separate guarantees and only one of them is
+    /// `default`'s. `#[serde(default)]` alone covers LOADING a file that lacks
+    /// the key; on SAVE the TOML serializer still emits an empty
+    /// `[input.latency_reports]` table, so the first save after upgrading would
+    /// rewrite the file. Review on #414 caught the original claim here, and it
+    /// was settled by serializing a default `Config` and grepping the output
+    /// rather than by re-reading the derive.
     ///
     /// **Remembering a measurement is not applying it.** Nothing here changes
     /// `run_ahead`; the panel still requires an explicit Apply, which is the
     /// separation v2.3.6 built deliberately ("measured" and "applied" are two
     /// auditable steps). Persisting the recommendation would quietly convert one
     /// into the other across a restart.
-    #[serde(default)]
+    #[serde(default, skip_serializing_if = "std::collections::BTreeMap::is_empty")]
     pub latency_reports: std::collections::BTreeMap<String, RememberedLatency>,
     /// v1.1.0 beta.1 (T-110-B2) — turbo/autofire on the A button: while held, A
     /// rapid-fires. Off by default (`false`) = byte-identical input. Applied
@@ -851,16 +861,24 @@ pub struct GraphicsConfig {
     #[serde(default)]
     pub shader_stack: crate::shader_pass::ShaderStackConfig,
     /// v1.2.0 C2 — saved named shader-stack presets (the CRT preset bank +
-    /// user-saved stacks). `#[serde(default)]` = empty, so a pre-C2 config is
-    /// byte-identical. Persisted under `[graphics.shader_presets]`.
+    /// user-saved stacks). `#[serde(default)]` = empty, so a pre-C2 config LOADS
+    /// unchanged. Persisted under `[graphics.shader_presets]`. Same correction as
+    /// `hd_packs` below: `serde(default)` says nothing about what SAVE writes.
     #[serde(default)]
     pub shader_presets: crate::shader_pass::ShaderPresetBank,
     /// v1.2.0 beta.2 (Workstream C3) — per-game HD-pack paths, keyed on the
     /// ROM SHA-256 (hex). When the loaded ROM's hash has an entry here AND the
     /// `hd-pack` feature is built in, the frontend loads the referenced pack
     /// (folder or `.zip`) and substitutes hi-res tiles at blit time. Empty by
-    /// default and `#[serde(default)]`, so a pre-C3 config is byte-identical
-    /// and the default presentation is unchanged. Presentation-only.
+    /// default and `#[serde(default)]`, so a pre-C3 config LOADS unchanged and
+    /// the default presentation is unchanged. Presentation-only.
+    ///
+    /// Deliberately says "loads", not "is byte-identical": `serde(default)` is a
+    /// LOAD guarantee only, and on save the TOML serializer emits an empty
+    /// `[graphics.hd_packs]` table. Left as-is rather than given a
+    /// `skip_serializing_if` like `input.latency_reports`, because that would
+    /// change the file this shipped feature writes; the wrong half here was the
+    /// claim, not the behaviour.
     #[serde(default)]
     pub hd_packs: std::collections::BTreeMap<String, std::path::PathBuf>,
     /// v1.5.0 "Lens" Workstream D1 — per-side overscan crop, in NES pixels. The
@@ -2130,9 +2148,65 @@ fn canonicalize_pad(pad: &PadBindings) -> PadBindings {
 
 #[cfg(test)]
 mod tests {
+
     use super::*;
     use tempfile::TempDir;
 
+    /// `#[serde(default)]` covers LOADING a config that lacks the key. It says
+    /// nothing about SAVING, and the TOML serializer emits an empty table for an
+    /// empty map — so a user who never opened the Latency Oracle would have had
+    /// their config rewritten with a bare `[input.latency_reports]` on the first
+    /// save after upgrading. Review on #414 caught the claim; this pins the fix.
+    ///
+    /// Both directions are asserted, because a `skip_serializing_if` that is too
+    /// eager would silently discard real measurements.
+    #[test]
+    fn an_empty_latency_map_is_not_written_but_a_populated_one_is() {
+        let empty = toml::to_string_pretty(&Config::default()).expect("serialize default");
+        assert!(
+            !empty.contains("latency_reports"),
+            "an empty map was written to the config, so an untouched file does \
+             not round-trip:\n{empty}"
+        );
+
+        let mut c = Config::default();
+        c.input.latency_reports.insert(
+            "deadbeef".to_owned(),
+            RememberedLatency {
+                frames: 3,
+                unanimous: true,
+                frame_micros: 16_639,
+            },
+        );
+        let filled = toml::to_string_pretty(&c).expect("serialize populated");
+        assert!(
+            filled.contains("latency_reports"),
+            "a real measurement was dropped on save:\n{filled}"
+        );
+        assert!(filled.contains("deadbeef"), "the ROM key was not written");
+
+        // Round-trip both, because the string checks alone verify the KEY and
+        // say nothing about the VALUE. `skip_serializing_if` on a field whose
+        // `Deserialize` had drifted would still produce the right text and load
+        // back as something else, and it is the load side the documentation
+        // promises. (Review on #414.)
+        let empty_back: Config = toml::from_str(&empty).expect("empty config re-parses");
+        assert!(
+            empty_back.input.latency_reports.is_empty(),
+            "the omitted key did not come back as an empty map"
+        );
+        let filled_back: Config = toml::from_str(&filled).expect("populated config re-parses");
+        assert_eq!(
+            filled_back.input.latency_reports.get("deadbeef").copied(),
+            Some(RememberedLatency {
+                frames: 3,
+                unanimous: true,
+                frame_micros: 16_639,
+            }),
+            "the measurement did not survive a save/load round trip"
+        );
+    }
+
     #[test]
     fn parse_pal_reads_64_colours_and_rejects_short() {
         // A 192-byte file → 64 RGB triples, in order.

to-dos/plans/v2.3.9-crucible-plan.md

@@ -431,6 +431,63 @@ Scoped here rather than left implicit, and explicitly lower priority than A5/B:
 
 Both are additive and neither touches the deterministic core.
 
+## Item D — the CHANGELOG gate, measured and REJECTED as specified
+
+Not planned; found while working. `[Unreleased]` was **empty** while three merged
+PRs carried user-visible change, including a whole new tool panel:
+
+| PR | landed | CHANGELOG entry |
+| --- | --- | :---: |
+| #407 | the Divergence Lens (v2.3.8 "Parallax" marquee) | none |
+| #409 | the two-acquisition lock-race measurement | none |
+| #410 | per-game Latency Oracle persistence | none |
+
+So the file that is supposed to be the single source of truth for user-visible
+change did not mention a new tool panel. Module 40 says to add the entry in the
+same PR and to enforce it with a quality gate, and there is no such gate here.
+
+### The obvious gate, checked against history rather than argued
+
+**Proposal:** a PR touching `crates/*/src/**` must also touch `CHANGELOG.md`.
+
+**Measured** over the last 50 first-parent merges on `main`: it would have gone
+red on **8**. Of those, **3** are the genuine misses above. The other **5** are
+this repo's normal workflow:
+
+| merge | why the gate is wrong about it |
+| --- | --- |
+| `feat(v2.3.6)`: the probe engine + Latency Oracle | entry written in the v2.3.6 cut PR |
+| `feat(frontend)`: the Latency Oracle + menu regroup | same |
+| `feat`: the RAM Atlas | same |
+| `perf(frontend)`: throttle-oscillation instrument | same |
+| `docs`: specs + user guide for the v2.3.6 tools | touched only `//!` preambles |
+
+**A 62% false-positive rate against the project's own history.** Features land
+bare and the release-cut PR composes the whole section at once. That is a
+deliberate workflow, not sloppiness, and a gate that fires on it would be
+suppressed within a week — which is worse than no gate, because a suppressed gate
+still reads as coverage.
+
+### What the measurement actually shows
+
+The failure mode is not "a feature landed without an entry". It is **"a release
+was never cut"**: v2.3.8 "Parallax" landed its marquee and was never tagged, so
+the cut PR that would have written the entry never ran, and v2.3.9 opened on top
+of an empty `[Unreleased]`. No per-PR gate addresses that, because at the time
+each PR merged nothing was wrong.
+
+**Rejected as specified, and recorded with its numbers** per the convention
+`docs/performance.md` sets for rejected optimizations. What was done instead is
+the thing the situation actually needed: the section is **backfilled now**, while
+the reasons are still recoverable from the commit bodies rather than reconstructed
+from diffs at cut time.
+
+**What would justify revisiting it:** a gate keyed on the *cut*, not the PR — for
+example, refusing to tag when `[Unreleased]` is empty while the workspace version
+is behind the newest plan doc. That is a release-ceremony check (module 70), it
+has no false-positive surface on ordinary PRs, and it targets the mechanism that
+actually failed here. Not built in this release; named so it reads as a decision.
+
 ## Item E — the ROM-transition sweep, and the one it found
 
 The v2.3.6 hook `DebuggerOverlay::clear_rom_bound_analysis` exists because panel