fix(probe): carry provenance across the three restores that were clearing it

CHANGELOG.md

@@ -81,6 +81,44 @@ cycle-accurate core later replaced.
 
 ### Fixed
 
+- **The Latency Oracle and the RAM Atlas no longer empty the provenance
+  panels.** Both drive the emulator and then put the timeline back, and
+  `Nes::restore_inner` clears the pixel- and audio-provenance stores — correctly
+  for a genuine timeline change, wrongly for a restore of the state the user is
+  still looking at. `rustynes-probe` had three such restores and none of them
+  used the `take_provenance` / `put_provenance` stash that v2.3.6 added for
+  exactly this: `Probe::run_uncounted` (once per trial, and a latency
+  measurement runs up to 21 of them), `latency::measure_in_place` (the final
+  restore, which sits outside every per-trial guard), and the RAM Atlas panel's
+  `TimelineGuard`. Both stores are **cumulative** — "which instruction last
+  wrote this" can point thousands of frames back, to a palette byte from level
+  load or a `$4008` reload from init — so the records were not rebuilt by the
+  next frame; they were gone for the session.
+
+  This is the defect class v2.3.6 was written about, found in three more places.
+  The v2.3.6 fix was correct at the call site the bug report named and stopped
+  there, and `docs/pixel-provenance.md` then described run-ahead as "the one
+  caller that needs the exception" — a correct rule with an incomplete
+  enumeration under it. Closed by moving the stash into
+  `rustynes_probe::TrialGuard`, the guard that already carried rewind capture
+  across a trial for the same underlying reason: state that lives outside the
+  save state is not carried by a snapshot round trip.
+
+  Pinned by the **full 2x2 matrix** — each of the two stores against each of the
+  two probe restores — under four independent mutations, so a fix that put back
+  only one store, or guarded only one of the two restores, fails. The fourth cell
+  (`measure_in_place` against the *pixel* store) was **missing until review
+  caught it**, and it was not a rounding error: the `measure_in_place` mutation
+  fails only the audio test, so a final restore that put back the audio stash and
+  dropped the pixel one would have passed everything. The claim of "four tests"
+  was written before the fourth existed. The existing
+  `measure_in_place_restores_the_live_timeline` could not have caught it: it
+  compares `nes.snapshot()` before and after, and provenance is deliberately not
+  in the snapshot, so it asserted something strictly weaker than the contract it
+  is named for. `rustynes-probe` gains a `debug-hooks` passthrough feature,
+  without which the guard would have compiled out in precisely the builds that
+  need it.
+
 - **VRC7 save states now carry the FM synthesizer, so rewind no longer garbles
   the music.** `Vrc7::save_state` wrote the *shadow* OPLL register bytes and
   never the live synthesizer — not `opll`, not `opll_clock_counter`, not

crates/rustynes-frontend/Cargo.toml

@@ -147,7 +147,7 @@ browser-cheevos = []
 # alias just lets `cargo clippy -p rustynes-frontend --features debug-hooks`
 # resolve to the same build; it adds nothing (the dependency below is the real
 # enable). The output-only `debug-hooks` core code is determinism-neutral.
-debug-hooks = ["rustynes-core/debug-hooks"]
+debug-hooks = ["rustynes-core/debug-hooks", "rustynes-probe/debug-hooks"]
 
 # v1.6.0 "Studio" Workstream G — A/V recording (default OFF, native-only).
 #

crates/rustynes-frontend/src/debugger/atlas_panel.rs

@@ -585,15 +585,33 @@ struct TimelineGuard<'a> {
     nes: &'a mut Nes,
     snapshot: Vec<u8>,
     restored: bool,
+    /// v2.3.7 — the pixel- and audio-provenance stores, held across the restore.
+    ///
+    /// The same reasoning as `rustynes_probe::TrialGuard`, and it has to be
+    /// repeated here because this guard owns its own restore: both stores live
+    /// OUTSIDE the save state, and `Nes::restore_quiet` clears them, so putting
+    /// the timeline back also emptied whatever the Pixel Provenance and Audio
+    /// Provenance panels had accumulated. Cumulative attribution is not
+    /// rebuilt by the next frame — a palette byte or a `$4008` write from level
+    /// load does not happen again — so the loss was permanent for the session.
+    #[cfg(feature = "debug-hooks")]
+    provenance: (
+        rustynes_core::rustynes_ppu::ProvenanceStash,
+        rustynes_core::rustynes_apu::provenance::AudioProvenanceStash,
+    ),
 }
 
 impl<'a> TimelineGuard<'a> {
     fn capture(nes: &'a mut Nes) -> Self {
         let snapshot = nes.snapshot();
+        #[cfg(feature = "debug-hooks")]
+        let provenance = (nes.take_provenance(), nes.take_audio_provenance());
         Self {
             nes,
             snapshot,
             restored: false,
+            #[cfg(feature = "debug-hooks")]
+            provenance,
         }
     }
 
@@ -613,6 +631,17 @@ impl Drop for TimelineGuard<'_> {
             // a panic during unwinding aborts.
             let _ = self.nes.restore_quiet(&self.snapshot);
         }
+        // AFTER any restore, on both paths, so the stores are put back into a
+        // timeline that has already stopped being rewritten.
+        #[cfg(feature = "debug-hooks")]
+        {
+            // `mem::take` because `Drop` has only `&mut self`. The guard is
+            // being destroyed, so leaving default stashes behind is invisible —
+            // the same pattern, for the same reason, as `TrialGuard::drop`.
+            let (pixel, audio) = core::mem::take(&mut self.provenance);
+            self.nes.put_provenance(pixel);
+            self.nes.put_audio_provenance(audio);
+        }
     }
 }
 

crates/rustynes-probe/Cargo.toml

@@ -19,6 +19,18 @@ authors.workspace = true
 repository.workspace = true
 description = "Deterministic re-simulation probe: anchor, replay under variation, locate the first divergence"
 
+[features]
+# Off by default, forwarded from `rustynes-frontend/debug-hooks`.
+#
+# The probe crate needs to SEE this feature, not merely inherit its effects.
+# Cargo's feature unification means a frontend build with `debug-hooks` already
+# switches on `rustynes-core/debug-hooks`, so `Nes::restore_quiet` starts
+# clearing the provenance stores — while any `#[cfg(feature = "debug-hooks")]`
+# code in THIS crate stays compiled out, because a crate can only `cfg` on
+# features it declares. Without this passthrough the guard that preserves those
+# stores would be dead code in exactly the build that needs it.
+debug-hooks = ["rustynes-core/debug-hooks"]
+
 [dependencies]
 rustynes-core = { path = "../rustynes-core" }
 

crates/rustynes-probe/src/atlas.rs

@@ -171,7 +171,9 @@ where
     // far larger than cache.
     let mut frame_major: Vec<u8> = Vec::with_capacity(WRAM_LEN * frames as usize);
     // Rewind capture is suppressed for the whole window, by the same guard a
-    // trial uses. These frames advance the live emulator and the caller rolls
+    // trial uses — which also moves the provenance stores out, so an
+    // observation's 180 rolled-back frames cannot contribute attributions to a
+    // timeline they never happened on (v2.3.7). These frames advance the live emulator and the caller rolls
     // them back, so they never happened on the user's timeline — letting them
     // into the ring would allow rewinding *into an observation*, which is exactly
     // the defect this crate had just fixed for `verify_liveness` and then
@@ -180,7 +182,7 @@ where
     // The guard also makes this panic-safe: an observation is 180 frames, and a
     // panic part-way through must not leave capture disabled on a `Nes` the
     // caller keeps using.
-    let guard = crate::CaptureGuard::suppress(nes);
+    let guard = crate::TrialGuard::enter(nes);
     for f in 0..frames {
         let (p1, p2) = input(f);
         guard.nes.set_buttons(0, p1);
@@ -737,7 +739,7 @@ mod tests {
     /// fixed for `verify_liveness` and then reintroduced one function away, caught
     /// in review on PR #392.
     ///
-    /// Mutation-checked: removing the `CaptureGuard` from `observe` fails this
+    /// Mutation-checked: removing the `TrialGuard` from `observe` fails this
     /// with the ring grown by the window length.
     #[test]
     fn observing_does_not_pollute_the_callers_rewind_ring() {

crates/rustynes-probe/src/latency.rs

@@ -189,7 +189,13 @@ pub fn measure(nes: &mut Nes, anchor: &Nes, cfg: LatencyConfig) -> LatencyReport
 pub fn measure_in_place(nes: &mut Nes, cfg: LatencyConfig) -> LatencyReport {
     let restore_point = nes.snapshot();
     let mut probe = Probe::anchor(&*nes, budget_for(cfg));
-    let report = run_measurement(&mut probe, nes, cfg);
+    // v2.3.7 — the guard spans the measurement AND the final restore below,
+    // which is the whole point of putting it here rather than relying on the
+    // per-trial guards inside `run_measurement`. Those end with their trial, so
+    // the final restore on the way out sat outside all of them and cleared the
+    // provenance stores after every one of them had carefully preserved it.
+    let guard = crate::TrialGuard::enter(nes);
+    let report = run_measurement(&mut probe, &mut *guard.nes, cfg);
     // Put the user's timeline back exactly. A measurement that leaves the game
     // 400 frames further on would be a worse bug than the one it measures.
     //
@@ -205,8 +211,13 @@ pub fn measure_in_place(nes: &mut Nes, cfg: LatencyConfig) -> LatencyReport {
     // the snapshot format cannot round-trip itself; returning normally would
     // hand the user a report while leaving their game several hundred frames
     // ahead, which is precisely the outcome this line exists to prevent.
-    nes.restore_quiet(&restore_point)
+    guard
+        .nes
+        .restore_quiet(&restore_point)
         .expect("a snapshot taken from this instance restores to it");
+    // Explicit, so the order is stated rather than inferred from scope: the
+    // stores go back AFTER the restore that would otherwise have cleared them.
+    drop(guard);
     report
 }