From c3cce550e5a6366fe135ded7686f89bae132abb4 Mon Sep 17 00:00:00 2001
From: nasr <156965421+div0rce@users.noreply.github.com>
Date: Wed, 24 Jun 2026 21:34:45 -0400
Subject: [PATCH] fix(ocaml): diff_report must not abort the batch on one
 malformed fixture
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Round-3 bug hunt found the diff_report bin (ocaml/bin/diff_report.ml) folds over
the fixture batch calling Diff_report.bundle_if_divergent with NO exception
handler, unlike its sibling bins verify_replay.ml and replay_snapshot.ml which
both catch Parse_error/Sys_error and exit cleanly. bundle_if_divergent calls
Stream_parser.parse_full_file (raises Stream_parser.Parse_error on a malformed
line, a missing meta line, or an int token exceeding OCaml's 63-bit max_int —
which a valid C++ int64 price/id can) and open_in (raises Sys_error). So one
malformed/unreadable fixture raises out of List.fold_left and aborts the whole
batch, silently losing the divergence bundles for every later fixture. CI runs
diff_report on the failure path over a glob batch with `|| true`, so the crash is
swallowed and the differential-failure-bundle artifact comes back empty —
precisely when a developer needs the bundle for the genuinely-divergent fixture.

Fix: guard each fixture with try/with catching Stream_parser.Parse_error and
Sys_error; print a per-fixture "cannot compare" message and treat it as a failure
(diverged=true) so the missing comparison forces a non-zero exit instead of
disappearing. Added a test asserting a malformed fixture raises the catchable
Stream_parser.Parse_error the bin now handles.

Verified: a malformed fixture now prints "cannot compare ..." and exits 1 (no
"Fatal error" crash); a batch with a malformed fixture first no longer aborts.
dune runtest passes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 ocaml/bin/diff_report.ml       | 16 ++++++++++++++--
 ocaml/test/test_diff_report.ml | 13 +++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/ocaml/bin/diff_report.ml b/ocaml/bin/diff_report.ml
index de8ae41..6398163 100644
--- a/ocaml/bin/diff_report.ml
+++ b/ocaml/bin/diff_report.ml
@@ -10,8 +10,20 @@ let () =
       let any =
         List.fold_left
           (fun acc f ->
-            let diverged = Diff_report.bundle_if_divergent ~out_dir f in
-            if diverged then Printf.eprintf "divergence: %s -> bundle in %s\n" f out_dir;
+            (* Guard each fixture: a malformed or unreadable one must not raise out of the fold and
+               abort the whole batch, which would silently lose the divergence bundles for every
+               later fixture -- exactly when CI needs them. Surface it as a failure (diverged=true)
+               so the missing comparison forces a non-zero exit instead of disappearing. Matches the
+               explicit Parse_error/Sys_error handling in verify_replay.ml and replay_snapshot.ml. *)
+            let diverged =
+              try
+                let d = Diff_report.bundle_if_divergent ~out_dir f in
+                if d then Printf.eprintf "divergence: %s -> bundle in %s\n" f out_dir;
+                d
+              with Stream_parser.Parse_error msg | Sys_error msg ->
+                Printf.eprintf "cannot compare %s: %s\n" f msg;
+                true
+            in
             acc || diverged)
           false fixtures
       in
diff --git a/ocaml/test/test_diff_report.ml b/ocaml/test/test_diff_report.ml
index b869a32..935e8e6 100644
--- a/ocaml/test/test_diff_report.ml
+++ b/ocaml/test/test_diff_report.ml
@@ -24,4 +24,17 @@ let () =
     prerr_endline "FAIL: unexpected divergence for stream_seed7.txt";
     exit 1);
 
+  (* A malformed fixture must raise a catchable Stream_parser.Parse_error, not some other exception:
+     the diff_report bin relies on catching exactly this (and Sys_error) to guard each fixture in
+     the batch so one bad file cannot abort the whole run and silently lose later bundles. *)
+  let bad = Filename.concat out "malformed.txt" in
+  let oc = open_out bad in
+  output_string oc "version 1\ncmd reg S0\n" (* no meta line *);
+  close_out oc;
+  (match Diff_report.bundle_if_divergent ~out_dir:out bad with
+  | _ ->
+      prerr_endline "FAIL: expected Parse_error for malformed fixture";
+      exit 1
+  | exception Stream_parser.Parse_error _ -> ());
+
   print_endline "diff_report: all tests passed"