Fix DMG VBlank STAT timing and require ROM report deltas (#13)

* docs: require ROM report before-after comparisons

* ppu: fix dmg vblank stat timing

* test: stabilize sameboy tester missing-image case

Author: pakitovic

AI/TESTING.md

@@ -94,6 +94,10 @@ This checklist should move only when one of the following becomes true:
 ## External ROM harness policy
 
 - Test ROM execution must be automatable; manual inspection of the LCD is an auxiliary debugging aid, not the primary acceptance path.
+- When working on already-known external ROM failures, timing regressions, or exploratory PPU/MMIO fixes, always preserve a baseline snapshot of `/.roms/test/test-report.md` before making the validation run, preserve the final report again after the run, and compare the two before deciding whether the iteration is worth keeping.
+- That baseline/final comparison is mandatory for go/no-go decisions on exploratory ROM-driven work. Save the raw markdown report first; also save a rendered image of the report when practical so visual status drift is easy to review later.
+- If the current tree is not already a clean baseline, compare against a clean reference such as `main` in a separate branch or worktree rather than hand-waving the previous state from memory.
+- Do not summarize a ROM-driven iteration as "no regressions" unless the before/after report comparison has been done explicitly and the changed rows have been named.
 - The harness should support at least framebuffer capture and serial / link-port capture when the ROM exposes machine-readable output there.
 - When an external suite prints self-validating text through a documented screen-console protocol, the harness should also support a typed text-extraction path for that protocol rather than falling back to a circular framebuffer fixture generated by this project.
 - Prefer serial / link-port capture for suites such as Blargg `cpu_instrs` when that path is available, because it avoids treating a scrolling framebuffer as the primary machine-readable result channel.

AI/hardware/PPU.md

@@ -107,6 +107,7 @@ For this project, the PPU should be modeled dot-by-dot, where `1 dot = 1 T-cycle
   - `stat_mode1_enable && mode == 1`
   - `stat_mode2_enable && mode == 2`
   - `stat_lyc_enable && ly == lyc`
+- In the current DMG-family baseline, the Mode `2` STAT enable should also request LCD STAT at the exact VBlank-entry transition on line `144`, but it should not be treated as a continuously active source for the rest of VBlank.
 - LCD STAT interrupt requests should be emitted only on a rising edge of that internal line, not merely because one contributing condition is true.
 - STAT blocking should be preserved: if one enabled source keeps the internal line high while another source becomes true, no new LCD STAT interrupt should be requested until the line first drops low and rises again.
 - Mode `3` must not be treated as a direct STAT interrupt source.

AI/index.md

@@ -2,6 +2,10 @@
 
 Read the matching file directly; this index is a routing guide plus a summary of document authority boundaries, but detailed behavioral rules still live in the owning file.
 
+## Mandatory external-ROM regression workflow
+
+When working on already-known external ROM failures or rerunning curated ROM suites to evaluate a timing-sensitive change, always capture a baseline copy of `/.roms/test/test-report.md` before the work, capture the final report again after the run, and compare the two before deciding whether the change is worth keeping. Treat that before/after report-delta check as mandatory, not optional; see `TESTING.md` for the authoritative workflow details.
+
 ## Global docs
 
 - `ARCHITECTURE.md`: project goals, crate layout, subsystem boundaries, and portability rules.

crates/gb-core/src/ppu.rs

@@ -1361,6 +1361,11 @@ impl Ppu {
             != 0
             && self.ly + 1 < VISIBLE_SCANLINES
             && self.line_dot + 4 >= DOTS_PER_SCANLINE;
+        let dmg_mode2_vblank_entry_source = self.console_model.is_dmg_family()
+            && self.stat_interrupt_enable & STAT_MODE2_INTERRUPT_ENABLE_BIT != 0
+            && self.current_access_mode() == PpuAccessMode::VBlank
+            && self.ly == VISIBLE_SCANLINES
+            && self.line_dot == 0;
         let mode_source = match self.current_access_mode() {
             PpuAccessMode::HBlank => {
                 self.stat_interrupt_enable & STAT_MODE0_INTERRUPT_ENABLE_BIT != 0
@@ -1374,7 +1379,10 @@ impl Ppu {
             PpuAccessMode::Drawing => false,
         };
 
-        coincidence_source || mode_source || mode2_pretrigger_source
+        coincidence_source
+            || mode_source
+            || mode2_pretrigger_source
+            || dmg_mode2_vblank_entry_source
     }
 
     fn compute_stat_irq_line(&self, quirk_active: bool) -> bool {
@@ -2367,6 +2375,68 @@ mod tests {
         assert!(drain_ppu_interrupts(&mut ppu).is_empty());
     }
 
+    #[test]
+    fn dmg_mode2_enable_requests_lcd_stat_at_vblank_entry_only() {
+        let mut ppu = Ppu::new(ConsoleModel::Dmg);
+        let oam_bytes = [0; 160];
+
+        ppu.apply_startup_state(PpuStartupState {
+            lcdc: 0x80,
+            stat: STAT_MODE2_INTERRUPT_ENABLE_BIT,
+            scy: 0x00,
+            scx: 0x00,
+            ly: 143,
+            lyc: 0x00,
+            bgp: 0x00,
+            wy: 0x00,
+            wx: 0x00,
+            obj_palette_read_policy: DmgObjPaletteReadPolicy::ReadAsFfUntilWritten,
+        });
+        ppu.line_dot = DOTS_PER_SCANLINE - 1;
+        ppu.refresh_stat_irq_line(false);
+        assert!(!ppu.snapshot().stat_irq_line);
+        assert!(drain_ppu_interrupts(&mut ppu).is_empty());
+
+        tick_ppu(&mut ppu, 0, &oam_bytes);
+
+        assert_eq!(ppu.snapshot().ly, 144);
+        assert_eq!(ppu.snapshot().mode, PpuAccessMode::VBlank);
+        assert!(ppu.snapshot().stat_irq_line);
+        assert_eq!(
+            drain_ppu_interrupts(&mut ppu),
+            vec![InterruptSource::VBlank, InterruptSource::LcdStat]
+        );
+
+        tick_ppu(&mut ppu, 1, &oam_bytes);
+
+        assert_eq!(ppu.snapshot().ly, 144);
+        assert_eq!(ppu.snapshot().line_dot, 1);
+        assert!(!ppu.snapshot().stat_irq_line);
+        assert!(drain_ppu_interrupts(&mut ppu).is_empty());
+    }
+
+    #[test]
+    fn mode2_enable_alone_does_not_hold_stat_high_past_vblank_entry() {
+        let mut ppu = Ppu::new(ConsoleModel::Dmg);
+
+        ppu.apply_startup_state(PpuStartupState {
+            lcdc: 0x80,
+            stat: STAT_MODE2_INTERRUPT_ENABLE_BIT,
+            scy: 0x00,
+            scx: 0x00,
+            ly: 144,
+            lyc: 0x00,
+            bgp: 0x00,
+            wy: 0x00,
+            wx: 0x00,
+            obj_palette_read_policy: DmgObjPaletteReadPolicy::ReadAsFfUntilWritten,
+        });
+        ppu.line_dot = 8;
+        ppu.refresh_stat_irq_line(false);
+        assert!(!ppu.snapshot().stat_irq_line);
+        assert!(drain_ppu_interrupts(&mut ppu).is_empty());
+    }
+
     #[test]
     fn stat_write_quirk_requests_in_mode2_and_coincidence_but_not_plain_mode3() {
         let mut mode2 = Ppu::new(ConsoleModel::Dmg);

crates/gb-test-runner/src/sameboy_tester.rs

@@ -452,6 +452,19 @@ mod tests {
         fs::set_permissions(path, permissions).expect("script should be executable");
     }
 
+    fn success_binary_path(temp_dir: &Path) -> PathBuf {
+        for candidate in ["/usr/bin/true", "/bin/true"] {
+            let path = Path::new(candidate);
+            if path.is_file() {
+                return path.to_path_buf();
+            }
+        }
+
+        let tester_binary = temp_dir.join("fake-success");
+        write_executable(&tester_binary, "#!/bin/sh\nexit 0\n");
+        tester_binary
+    }
+
     fn sample_framebuffer_case() -> RomTestCase {
         RomTestCase::new(
             "acid2",
@@ -625,17 +638,19 @@ mod tests {
             b"rom"
         );
 
-        let tester_binary = temp_dir.join("fake-tester");
-        write_executable(&tester_binary, "#!/bin/sh\nexit 0\n");
+        let tester_binary = success_binary_path(&temp_dir);
         let error = runner
             .clone()
             .with_tester_binary(&tester_binary)
             .run_case(&case, &tester_binary)
             .expect_err("missing image output should fail");
-        assert!(matches!(
-            error,
-            SameBoyTesterExecutionError::MissingImageArtifact { .. }
-        ));
+        assert!(
+            matches!(
+                error,
+                SameBoyTesterExecutionError::MissingImageArtifact { .. }
+            ),
+            "unexpected run_case error: {error:?}"
+        );
 
         let non_strict = case.clone().with_execution_mode(ExecutionMode::Permissive);
         let error = runner