d725cafd

Plan 1 of 3 for scenario runner: imgdiff extraction

a73x 2026-04-19 08:40

Commit message

Plan 1 of 3 for scenario runner: imgdiff extraction

Small refactor — move pure RMSE + max-pixel + makeDiffImage out
of the CLI into src/imgdiff.zig so the scenario runner (plans 2
and 3) can call the same code.

Zero behavior change for the existing CLI. test-render continues
to work.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

docs/superpowers/plans/2026-04-19-imgdiff-extraction.md

@@ @@ -0,0 +1,434 @@ @@
+# Imgdiff Extraction Implementation Plan
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+**Goal:** Extract the pure RMSE + max-pixel + diff-heatmap math from `src/tools/imgdiff.zig` into a new reusable module `src/imgdiff.zig`, so the scenario runner (later plans) can call the same code as the existing CLI.
+**Architecture:** Pure-math module with `compare`, `makeDiffImage`, `DiffResult`, and the two default threshold constants (`RMSE_DEFAULT`, `PIXEL_MAX_DEFAULT`). The existing CLI at `src/tools/imgdiff.zig` shrinks to a thin `main` + env-var parsing that imports the new module. Zero behavior change for the CLI.
+**Tech Stack:** Zig 0.15, existing `png` module.
+**Reference spec:** `docs/superpowers/specs/2026-04-19-scenario-runner-design.md` — implementation phasing step 1 of 3.
+---
+## Reference facts (grep once, reuse)
+- Existing CLI + math lives in `src/tools/imgdiff.zig` (152 lines). The pure math is the `compare` function + `DiffResult` struct + `makeDiffImage` helper. The CLI-side logic is `main`, `readFloatEnv`, env-var defaults (`RMSE` default `0.005`, `PIXEL_MAX` default `0.125`), argument parsing, PNG encode/decode, and stderr reporting.
+- Build wiring in `build.zig:368-388` creates `imgdiff_mod` (executable) and `imgdiff_test_mod` (tests). Both use `src/tools/imgdiff.zig` as `root_source_file`.
+- `png.Image` struct is `{ width: u32, height: u32, pixels: []u8 }` (BGRA/RGBA 4 bytes-per-pixel).
+- This plan has no Vulkan, no Wayland, no main-loop touchpoints. Pure refactor.
+- Zig's `zig build test` runs all test modules registered on `test_step`. The existing `imgdiff_tests` contributes two tests; this plan adds one more to the new module.
+---
+## File structure after this plan
+- **`src/imgdiff.zig`** (NEW) — pure module. Public: `DiffResult`, `compare(a, b)`, `makeDiffImage(alloc, a, b)`, `RMSE_DEFAULT`, `PIXEL_MAX_DEFAULT`. Inline unit tests stay with the code.
+- **`src/tools/imgdiff.zig`** (MODIFIED, shrunk) — CLI only. `main`, `readFloatEnv`. Imports `imgdiff` module for the math + constants.
+- **`build.zig`** (MODIFIED) — add a new `imgdiff_mod` module (name collision with the existing variable; we'll rename — see task 2); wire it as an import on both the CLI exe and its test module.
+---
+## Task 1: Create `src/imgdiff.zig` as a pure module
+**Files:**
+- Create: `src/imgdiff.zig`
+- Modify: `build.zig:368-388` — register the new module and its tests
+**Goal of this task:** Stand up the new module alongside the existing CLI. The CLI keeps working unchanged. The new module has its own tests and runs green.
+- [ ] **Step 1: Create `src/imgdiff.zig` with the pure math + constants + three tests.**
+```zig
+//! Pure-math PNG image diff used by both the `imgdiff` CLI
+//! (src/tools/imgdiff.zig) and the scenario runner (to be added later).
+//! Threshold constants live here so both callers agree.
+const std = @import("std");
+const png = @import("png");
+pub const RMSE_DEFAULT: f64 = 0.005;
+pub const PIXEL_MAX_DEFAULT: f64 = 0.125;
+pub const DiffResult = struct {
+    rmse: f64, // [0, 1]
+    max_pixel: f64, // [0, 1]
+    pixel_count: usize,
+};
+pub fn compare(a: png.Image, b: png.Image) !DiffResult {
+    if (a.width != b.width or a.height != b.height) return error.DimensionsDiffer;
+    std.debug.assert(a.pixels.len == b.pixels.len);
+    const px_count = @as(usize, a.width) * a.height;
+    var sum_sq: f64 = 0;
+    var max_d: f64 = 0;
+    var i: usize = 0;
+    while (i < px_count) : (i += 1) {
+        const off = i * 4;
+        const dr = (@as(f64, @floatFromInt(a.pixels[off + 0])) - @as(f64, @floatFromInt(b.pixels[off + 0]))) / 255.0;
+        const dg = (@as(f64, @floatFromInt(a.pixels[off + 1])) - @as(f64, @floatFromInt(b.pixels[off + 1]))) / 255.0;
+        const db = (@as(f64, @floatFromInt(a.pixels[off + 2])) - @as(f64, @floatFromInt(b.pixels[off + 2]))) / 255.0;
+        const d_sq = (dr * dr + dg * dg + db * db) / 3.0;
+        sum_sq += d_sq;
+        const d = @sqrt(d_sq);
+        if (d > max_d) max_d = d;
+    }
+    return .{
+        .rmse = @sqrt(sum_sq / @as(f64, @floatFromInt(px_count))),
+        .max_pixel = max_d,
+        .pixel_count = px_count,
+    };
+}
+pub fn makeDiffImage(alloc: std.mem.Allocator, a: png.Image, b: png.Image) !png.Image {
+    // Side-by-side: [actual | reference | delta-heatmap]
+    const w = a.width * 3;
+    const h = a.height;
+    const pixels = try alloc.alloc(u8, w * h * 4);
+    var y: u32 = 0;
+    while (y < h) : (y += 1) {
+        const row_off = @as(usize, y) * w * 4;
+        const a_off = @as(usize, y) * a.width * 4;
+        @memcpy(pixels[row_off .. row_off + a.width * 4], a.pixels[a_off .. a_off + a.width * 4]);
+        @memcpy(pixels[row_off + a.width * 4 .. row_off + 2 * a.width * 4], b.pixels[a_off .. a_off + a.width * 4]);
+        var x: u32 = 0;
+        while (x < a.width) : (x += 1) {
+            const off = a_off + x * 4;
+            const dr = (@as(f64, @floatFromInt(a.pixels[off + 0])) - @as(f64, @floatFromInt(b.pixels[off + 0]))) / 255.0;
+            const dg = (@as(f64, @floatFromInt(a.pixels[off + 1])) - @as(f64, @floatFromInt(b.pixels[off + 1]))) / 255.0;
+            const db = (@as(f64, @floatFromInt(a.pixels[off + 2])) - @as(f64, @floatFromInt(b.pixels[off + 2]))) / 255.0;
+            const d = @sqrt((dr * dr + dg * dg + db * db) / 3.0);
+            const brightness: u8 = @intFromFloat(@min(255.0, d * 255.0 * 2.0));
+            const dst = row_off + 2 * a.width * 4 + x * 4;
+            pixels[dst + 0] = brightness;
+            pixels[dst + 1] = brightness;
+            pixels[dst + 2] = brightness;
+            pixels[dst + 3] = 255;
+        }
+    }
+    return .{ .width = w, .height = h, .pixels = pixels };
+}
+test "identical images produce zero RMSE" {
+    var pixels_a = [_]u8{ 10, 20, 30, 255, 40, 50, 60, 255 };
+    var pixels_b = [_]u8{ 10, 20, 30, 255, 40, 50, 60, 255 };
+    const a = png.Image{ .width = 2, .height = 1, .pixels = &pixels_a };
+    const b = png.Image{ .width = 2, .height = 1, .pixels = &pixels_b };
+    const r = try compare(a, b);
+    try std.testing.expectEqual(@as(f64, 0.0), r.rmse);
+    try std.testing.expectEqual(@as(f64, 0.0), r.max_pixel);
+}
+test "fully saturated difference produces rmse=1.0 and max=1.0" {
+    var pixels_a = [_]u8{ 0, 0, 0, 255 };
+    var pixels_b = [_]u8{ 255, 255, 255, 255 };
+    const a = png.Image{ .width = 1, .height = 1, .pixels = &pixels_a };
+    const b = png.Image{ .width = 1, .height = 1, .pixels = &pixels_b };
+    const r = try compare(a, b);
+    try std.testing.expectApproxEqAbs(@as(f64, 1.0), r.rmse, 1e-9);
+    try std.testing.expectApproxEqAbs(@as(f64, 1.0), r.max_pixel, 1e-9);
+}
+test "makeDiffImage: width triples, heatmap brighter where delta is larger" {
+    const alloc = std.testing.allocator;
+    // 2x1 image pair where one column differs more than the other.
+    var pixels_a = [_]u8{ 0, 0, 0, 255, 100, 100, 100, 255 };
+    var pixels_b = [_]u8{ 0, 0, 0, 255, 200, 200, 200, 255 };
+    const a = png.Image{ .width = 2, .height = 1, .pixels = &pixels_a };
+    const b = png.Image{ .width = 2, .height = 1, .pixels = &pixels_b };
+    const out = try makeDiffImage(alloc, a, b);
+    defer alloc.free(out.pixels);
+    try std.testing.expectEqual(@as(u32, 6), out.width);
+    try std.testing.expectEqual(@as(u32, 1), out.height);
+    // Heatmap is the last third. Column 0 (no delta) should be black;
+    // column 1 (delta) should be non-zero.
+    const heatmap_offset = 4 * 4; // bytes before heatmap (2 images × 2 pixels × 4 bytes = 16)
+    try std.testing.expectEqual(@as(u8, 0), out.pixels[heatmap_offset + 0]); // R at heatmap col 0
+    try std.testing.expect(out.pixels[heatmap_offset + 4] > 0); // R at heatmap col 1
+}
+```
+- [ ] **Step 2: Wire `src/imgdiff.zig` as a module in `build.zig`.**
+Current state of `build.zig:368-388`:
+```zig
+    // imgdiff — standalone PNG comparison tool
+    const imgdiff_mod = b.createModule(.{
+        .root_source_file = b.path("src/tools/imgdiff.zig"),
+        .target = target,
+        .optimize = optimize,
+    });
+    imgdiff_mod.addImport("png", png_mod);
+    const imgdiff_exe = b.addExecutable(.{
+        .name = "imgdiff",
+        .root_module = imgdiff_mod,
+    });
+    b.installArtifact(imgdiff_exe);
+    const imgdiff_test_mod = b.createModule(.{
+        .root_source_file = b.path("src/tools/imgdiff.zig"),
+        .target = target,
+        .optimize = optimize,
+    });
+    imgdiff_test_mod.addImport("png", png_mod);
+    const imgdiff_tests = b.addTest(.{ .root_module = imgdiff_test_mod });
+    test_step.dependOn(&b.addRunArtifact(imgdiff_tests).step);
+```
+Replace with:
+```zig
+    // imgdiff — pure library used by the CLI + the scenario runner
+    const imgdiff_lib_mod = b.createModule(.{
+        .root_source_file = b.path("src/imgdiff.zig"),
+        .target = target,
+        .optimize = optimize,
+    });
+    imgdiff_lib_mod.addImport("png", png_mod);
+    const imgdiff_lib_test_mod = b.createModule(.{
+        .root_source_file = b.path("src/imgdiff.zig"),
+        .target = target,
+        .optimize = optimize,
+    });
+    imgdiff_lib_test_mod.addImport("png", png_mod);
+    const imgdiff_lib_tests = b.addTest(.{ .root_module = imgdiff_lib_test_mod });
+    test_step.dependOn(&b.addRunArtifact(imgdiff_lib_tests).step);
+    // imgdiff — standalone PNG comparison CLI
+    const imgdiff_mod = b.createModule(.{
+        .root_source_file = b.path("src/tools/imgdiff.zig"),
+        .target = target,
+        .optimize = optimize,
+    });
+    imgdiff_mod.addImport("png", png_mod);
+    imgdiff_mod.addImport("imgdiff", imgdiff_lib_mod);
+    const imgdiff_exe = b.addExecutable(.{
+        .name = "imgdiff",
+        .root_module = imgdiff_mod,
+    });
+    b.installArtifact(imgdiff_exe);
+    const imgdiff_test_mod = b.createModule(.{
+        .root_source_file = b.path("src/tools/imgdiff.zig"),
+        .target = target,
+        .optimize = optimize,
+    });
+    imgdiff_test_mod.addImport("png", png_mod);
+    imgdiff_test_mod.addImport("imgdiff", imgdiff_lib_mod);
+    const imgdiff_tests = b.addTest(.{ .root_module = imgdiff_test_mod });
+    test_step.dependOn(&b.addRunArtifact(imgdiff_tests).step);
+```
+The library module (`imgdiff_lib_mod`) is a separate build unit from the CLI module (`imgdiff_mod`). Both reach `png`; the CLI additionally imports the library. Both have their tests registered.
+At this point the CLI still has its own copy of `compare` / `makeDiffImage` / tests — Task 2 removes them. This task only *adds* the new module. Intermediate state: duplication between `src/imgdiff.zig` and `src/tools/imgdiff.zig`. That's fine for one commit; Task 2 removes the duplication.
+- [ ] **Step 3: Build and run tests to verify the new module compiles and its tests pass.**
+Run:
+```bash
+cd /home/xanderle/code/rad/waystty
+zig build test 2>&1 | tail -30
+```
+Expected:
+- Build succeeds.
+- All existing tests still pass.
+- Three new tests (from `src/imgdiff.zig`) show up in the count.
+If the test runner reports a test-count mismatch relative to before, that's the expected addition of three new tests. If there are *fewer* tests, the old `src/tools/imgdiff.zig` tests didn't get picked up — investigate before committing.
+- [ ] **Step 4: Commit.**
+```bash
+git add src/imgdiff.zig build.zig
+git commit -m "$(cat <<'EOF'
+imgdiff: add src/imgdiff.zig pure-math module
+Copies compare + DiffResult + makeDiffImage into a standalone
+module so it can be reused by the upcoming scenario runner.
+CLI at src/tools/imgdiff.zig is untouched this commit; dedup
+happens in the next commit.
+Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
+EOF
+)"
+```
+---
+## Task 2: Shrink `src/tools/imgdiff.zig` to a thin CLI
+**Files:**
+- Modify: `src/tools/imgdiff.zig` — remove `DiffResult`, `compare`, `makeDiffImage`, inline tests, replace with `@import("imgdiff")` usage.
+- [ ] **Step 1: Replace `src/tools/imgdiff.zig` with a CLI-only version.**
+Full new contents of `src/tools/imgdiff.zig`:
+```zig
+const std = @import("std");
+const png = @import("png");
+const imgdiff = @import("imgdiff");
+pub fn main() !void {
+    var gpa: std.heap.DebugAllocator(.{}) = .init;
+    defer _ = gpa.deinit();
+    const alloc = gpa.allocator();
+    const args = try std.process.argsAlloc(alloc);
+    defer std.process.argsFree(alloc, args);
+    if (args.len < 3) {
+        std.debug.print("usage: imgdiff <actual.png> <reference.png> [diff.png]\n", .{});
+        std.process.exit(2);
+    }
+    const actual_path = args[1];
+    const reference_path = args[2];
+    const diff_path: ?[]const u8 = if (args.len >= 4) args[3] else null;
+    const rmse_max = readFloatEnv("WAYSTTY_TEST_RMSE_MAX", imgdiff.RMSE_DEFAULT);
+    const pixel_max = readFloatEnv("WAYSTTY_TEST_PIXEL_MAX", imgdiff.PIXEL_MAX_DEFAULT);
+    const actual_bytes = try std.fs.cwd().readFileAlloc(alloc, actual_path, 64 * 1024 * 1024);
+    defer alloc.free(actual_bytes);
+    const reference_bytes = try std.fs.cwd().readFileAlloc(alloc, reference_path, 64 * 1024 * 1024);
+    defer alloc.free(reference_bytes);
+    var actual = try png.decode(alloc, actual_bytes);
+    defer actual.deinit(alloc);
+    var reference = try png.decode(alloc, reference_bytes);
+    defer reference.deinit(alloc);
+    if (actual.width != reference.width or actual.height != reference.height) {
+        std.debug.print("FAIL: dimensions differ ({}x{} vs {}x{})\n", .{ actual.width, actual.height, reference.width, reference.height });
+        std.process.exit(3);
+    }
+    const r = try imgdiff.compare(actual, reference);
+    const pass = r.rmse <= rmse_max and r.max_pixel <= pixel_max;
+    if (pass) {
+        std.debug.print("OK:   {s}  RMSE={d:.4}%  worst={d:.4}%\n", .{ reference_path, r.rmse * 100.0, r.max_pixel * 100.0 });
+        std.process.exit(0);
+    }
+    std.debug.print("FAIL: {s}\n  RMSE: {d:.4}% (max {d:.4}%)\n  worst pixel: {d:.4}% (max {d:.4}%)\n", .{ reference_path, r.rmse * 100.0, rmse_max * 100.0, r.max_pixel * 100.0, pixel_max * 100.0 });
+    if (diff_path) |p| {
+        const diff_img = try imgdiff.makeDiffImage(alloc, actual, reference);
+        defer alloc.free(diff_img.pixels);
+        var buf: std.ArrayList(u8) = .empty;
+        defer buf.deinit(alloc);
+        try png.encode(alloc, diff_img, buf.writer(alloc));
+        const out = try std.fs.cwd().createFile(p, .{ .truncate = true });
+        defer out.close();
+        try out.writeAll(buf.items);
+        std.debug.print("  diff: {s}\n", .{p});
+    }
+    std.debug.print("  actual: {s}\n", .{actual_path});
+    std.process.exit(1);
+}
+fn readFloatEnv(name: []const u8, default: f64) f64 {
+    const val = std.posix.getenv(name) orelse return default;
+    return std.fmt.parseFloat(f64, val) catch default;
+}
+```
+What's gone from the old file: `pub const DiffResult`, `pub fn compare`, `fn makeDiffImage`, both `test "identical images ..."` and `test "fully saturated ..."` blocks. What's new: the `imgdiff` import and calls to `imgdiff.compare` / `imgdiff.makeDiffImage` / `imgdiff.RMSE_DEFAULT` / `imgdiff.PIXEL_MAX_DEFAULT`.
+- [ ] **Step 2: Build everything.**
+Run:
+```bash
+cd /home/xanderle/code/rad/waystty
+zig build 2>&1 | tail -20
+```
+Expected: build succeeds, `imgdiff` binary still links.
+- [ ] **Step 3: Run unit tests.**
+Run:
+```bash
+zig build test 2>&1 | tail -30
+```
+Expected: all tests pass. Note: the `imgdiff_tests` test module (the CLI's test module) now has zero tests inside it because we moved them all — that's fine; it just means the test runner reports zero tests for that module while the new `imgdiff_lib_tests` reports three. The total count should be **unchanged minus 2 plus 3 = net +1** relative to before Task 1.
+- [ ] **Step 4: Smoke test the CLI against an existing golden.**
+Run:
+```bash
+cd /home/xanderle/code/rad/waystty
+zig build
+./zig-out/bin/imgdiff tests/golden/reference/basic_ascii.png tests/golden/reference/basic_ascii.png
+```
+Expected: `OK:  tests/golden/reference/basic_ascii.png  RMSE=0.0000%  worst=0.0000%`, exit code 0. Identical image to itself → zero diff.
+Then a failing-diff smoke:
+```bash
+./zig-out/bin/imgdiff tests/golden/reference/basic_ascii.png tests/golden/reference/bold_colors.png || echo "exit=$?"
+```
+Expected: `FAIL: ...` output, exit code 1 (or 3 if dimensions differ — `basic_ascii.png` and `bold_colors.png` are both 80×24 grids so dimensions match; exit 1 is the normal-diff fail).
+If both these smoke cases behave the same as before the refactor, the CLI has not regressed.
+- [ ] **Step 5: Run the full render-test suite (which uses imgdiff CLI).**
+```bash
+zig build test-render 2>&1 | tail -20
+```
+Expected: all golden scripts pass (`basic_ascii`, `bold_colors`, `box_drawing` at least, as of spec-writing time). If this step fails, the CLI refactor broke the existing regression gate — investigate before committing.
+- [ ] **Step 6: Commit.**
+```bash
+git add src/tools/imgdiff.zig
+git commit -m "$(cat <<'EOF'
+imgdiff: shrink CLI to thin wrapper over src/imgdiff.zig
+Removes duplicated pure-math (compare, makeDiffImage, DiffResult,
+inline tests) from the CLI. CLI now imports the library module
+for both the comparison and the default threshold constants.
+No behavior change. test-render still passes against existing
+golden PNGs.
+Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
+EOF
+)"
+```
+---
+## Post-task verification
+- [ ] `zig build` passes with no warnings.
+- [ ] `zig build test` passes (three `imgdiff_lib_tests` + zero `imgdiff_tests` + everything else unchanged).
+- [ ] `zig build test-render` still passes against existing goldens.
+- [ ] `./zig-out/bin/imgdiff` CLI output format is byte-identical to pre-refactor (re-run the two smoke cases in Task 2 Step 4 if unsure).
+- [ ] `src/imgdiff.zig` is importable by future code as `@import("imgdiff")` on any module that declares it — this is what plan 2 (scenario parser) will rely on.
+---
+## Self-review coverage check
+Spec "Implementation phasing" step 1 says: move pure math into `src/imgdiff.zig`, keep CLI as thin wrapper. Task 1 adds the module and its tests; Task 2 shrinks the CLI. Post-task verification confirms no CLI behavior change. ✅ covered.
+Spec "Files touched" lists `src/imgdiff.zig` as new and `src/tools/imgdiff.zig` as the thin wrapper. ✅ matches.
+Spec doesn't require any changes outside imgdiff for plan 1 — plan 2 and plan 3 will consume `src/imgdiff.zig`.