reader: refuse a CBF that is not a detector image instead of misreading it

Three ways a miniCBF opened silently wrong.

A byte-offset CBF with no PILATUS header at all was accepted: Count_cutoff then
defaulted to 0, SaturationLimitFromValue(0) is 1, and every pixel at or above one
count was flagged saturated - the integration accept gate drops the whole
reflection, so the run comes out empty for a reason nothing reports. The pixel
size defaulted to 0 with no validation anywhere downstream, which collapses every
resolution, every scattering vector and the beam centre in millimetres. XDS
writes its correction files in exactly this shape, so this is not hypothetical.
A pixel size is now required to claim the file at all, and a missing Count_cutoff
leaves the saturation limit unset - falling back to the container's own overflow,
which can only fail to call a pixel saturated - with a warning saying so.

And the header captures are character classes, not number grammars: "[\d.eE+-]+"
matches a bare "." and "(\d+)" matches a digit string too long for int64.
std::stod and std::stoll answer both with a raw std:: exception, which escaped
the format probe - CanRead catches JFJochException only - so merely LOOKING at a
corrupt file threw out of the viewer's open path. A header field that does not
parse is now reported as a malformed header.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EFEJG6WBQv8th4UJFNe53N

docs/CHANGELOG.md

@@ -9,6 +9,9 @@
 * `rugnux` writes the unmerged MTZ `<prefix>_unmerged.mtz` by default; `--no-export-unmerged` skips it.
 * Every rotation run that determines its own space group also writes `<prefix>_P1.mtz`, the same observations merged in P1, so a wrong space group can be re-merged, re-solved or re-refined without processing the images again; `--no-p1-crosscheck` declines it, and a run given `-S` writes nothing because its space group's centring absences were never integrated.
 * The space-group search names the setting the data support, so a screw or a 2-fold on the a or c axis is reported as such instead of costing the crystal its space group.
+* A space-group candidate whose predicted absences were never measurable no longer outranks one whose absences are genuinely dead - an unmeasured zone was worth about 690 nats per reflection, which inverted the ranking outright.
+* A CBF with no detector header is refused instead of being opened with a pixel size of zero and every pixel called saturated, and a header number that does not parse is reported as a malformed header.
+* A master whose companion `_meta.h5` was not kept still opens: the fields an Eiger master links into that file are treated as absent rather than as a read error, and a run that finds no saturation value anywhere says so and carries on instead of refusing the dataset.
 * The primitive cell of an R- or H-centred lattice is that lattice: the change of basis was applied without the transposition the convention needs, which left the volume right and the cell wrong, and reached the path that re-seats a lattice into a space group fixed by hand.
 * A centred monoclinic lattice that reduces to the mI form keeps its centring: the character naming that reduced form was missing from the table, so such a cell was reported as triclinic.
 * A lattice keeps the symmetry it has whatever orientation it was refined in: the cell reduction now judges a structurally-zero scalar product against the size of the cell rather than against a fixed tolerance eight decades smaller, so a body-centred tetragonal lattice is no longer read as C-centred monoclinic on most rotations of itself.

reader/JFJochCBFReader.cpp

@@ -12,6 +12,7 @@
 #include <optional>
 
 #include "../common/JFJochException.h"
+#include "../common/Logger.h"
 #include "../common/JFJochMath.h"
 
 namespace {
@@ -171,7 +172,11 @@ bool JFJochCBFReader::CanRead(const std::string &path) {
     if (!HasCBFExtension(std::filesystem::path(path)))
         return false;
     try {
-        return minicbf::ReadHeader(path).byte_offset;
+        // A pixel size as well as the compression: a byte-offset CBF with no "# Pixel_size" line has
+        // no geometry, and XDS writes its correction files in exactly that shape. Claiming one here
+        // would open it with a pixel size of zero.
+        const auto h = minicbf::ReadHeader(path);
+        return h.byte_offset && h.pixel_x_m > 0.0 && h.pixel_y_m > 0.0;
     } catch (const JFJochException &) {
         return false;
     }
@@ -187,6 +192,14 @@ void JFJochCBFReader::ReadFiles(const std::string &path) {
     if (!header0_.byte_offset)
         throw JFJochException(JFJochExceptionCategory::InputParameterInvalid,
                               "Unsupported CBF compression (only x-CBF_BYTE_OFFSET)");
+    // Beside the compression, because a pixel size is what makes the file an IMAGE: every resolution,
+    // every scattering vector and the beam centre in millimetres scale by it, and the default of 0
+    // collapses all of them without a word. A byte-offset CBF with no "# Pixel_size" line is not a
+    // detector image from this family - XDS writes its correction files in exactly that shape.
+    if (!(header0_.pixel_x_m > 0.0) || !(header0_.pixel_y_m > 0.0))
+        throw JFJochException(JFJochExceptionCategory::InputParameterInvalid,
+                              files_[0] + " has no pixel size in its header (no '# Pixel_size' line); "
+                              "it carries a CBF binary section but is not a detector image");
 
     dataset_ = std::make_shared<JFJochReaderDataset>();
     dataset_->experiment = default_experiment;
@@ -195,7 +208,18 @@ void JFJochCBFReader::ReadFiles(const std::string &path) {
     detector.PixelSize_um(static_cast<int64_t>(std::lround(header0_.pixel_x_m * 1e6)));
     detector.SensorThickness_um(static_cast<int64_t>(std::lround(header0_.thickness_m * 1e6)));
     detector.SensorMaterial(header0_.material);
-    detector.SaturationLimit(SaturationLimitFromValue(header0_.count_cutoff));
+    // Only when the header states one. Count_cutoff defaults to 0 and SaturationLimitFromValue(0) is
+    // 1, so an absent line marked EVERY pixel at or above one count as saturated - the integration
+    // accept gate then drops the whole reflection and the run comes out empty for a reason nothing
+    // reports. Left unset, DiffractionExperiment::GetSaturationLimit() falls back to the container's
+    // own overflow, which is the safe direction: it can only fail to call a pixel saturated.
+    if (header0_.count_cutoff > 0)
+        detector.SaturationLimit(SaturationLimitFromValue(header0_.count_cutoff));
+    else
+        Logger("CBFReader").Warning("{} states no Count_cutoff, so no pixel will be called "
+                                    "saturated; if this detector overloads, its strongest "
+                                    "reflections will be integrated as if they were valid.",
+                                    files_[0]);
     // Images are handed out as signed 32-bit whatever the file stored, so that is the depth the rest
     // of the code must see; the real overflow is the header's Count_cutoff, set above.
     detector.BitDepthImage(32);

reader/MiniCBF.cpp

@@ -28,14 +28,33 @@ std::optional<std::string> Match(const std::string &text, const char *pattern) {
     return m[1].str();
 }
 
+// The captures are character classes, not number grammars: "[\d.eE+-]+" matches "." and "+-", and
+// "(\d+)" matches a digit string too long for int64. std::stod and std::stoll answer both with a raw
+// std:: exception, which would leave the format probe below - CanRead catches JFJochException only -
+// and reach the caller as an unhandled throw from merely LOOKING at a file. A header field that does
+// not parse is a malformed header, so say that.
 double Num(const std::string &text, const char *pattern, double fallback = 0.0) {
     const auto s = Match(text, pattern);
-    return s.has_value() ? std::stod(*s) : fallback;
+    if (!s.has_value())
+        return fallback;
+    try {
+        return std::stod(*s);
+    } catch (const std::exception &) {
+        throw JFJochException(JFJochExceptionCategory::InputParameterInvalid,
+                              "Malformed number in CBF header: '" + *s + "'");
+    }
 }
 
 int64_t Int(const std::string &text, const char *pattern, int64_t fallback = 0) {
     const auto s = Match(text, pattern);
-    return s.has_value() ? std::stoll(*s) : fallback;
+    if (!s.has_value())
+        return fallback;
+    try {
+        return std::stoll(*s);
+    } catch (const std::exception &) {
+        throw JFJochException(JFJochExceptionCategory::InputParameterInvalid,
+                              "Malformed integer in CBF header: '" + *s + "'");
+    }
 }
 
 // A goniometer angle, or nothing where the head has no such axis. Writers spell that -9999, and
@@ -44,7 +63,13 @@ std::optional<double> Angle(const std::string &text, const char *pattern) {
     const auto s = Match(text, pattern);
     if (!s.has_value())
         return {};
-    const double v = std::stod(*s);
+    double v;
+    try {
+        v = std::stod(*s);
+    } catch (const std::exception &) {
+        throw JFJochException(JFJochExceptionCategory::InputParameterInvalid,
+                              "Malformed angle in CBF header: '" + *s + "'");
+    }
     if (v < -9998.0)
         return {};
     return v;
@@ -349,6 +374,15 @@ Header ReadCommon(const std::string &path, std::vector<uint8_t> &buf, size_t &bi
     if (h.nelem <= 0 || h.nx <= 0 || h.ny <= 0 || h.nelem != h.nx * h.ny)
         throw JFJochException(JFJochExceptionCategory::InputParameterInvalid,
                               "Inconsistent image dimensions in " + path);
+    // Without a pixel size there is no geometry at all - every resolution, every scattering vector
+    // and the beam centre in millimetres all scale by it - and the default of 0 collapses all of
+    // them silently. A byte-offset CBF carrying no "# Pixel_size" line is not a detector image from
+    // this family at all; XDS writes correction files in exactly that shape. Refuse it here rather
+    // than let it through with a geometry of zero.
+    if (!(h.pixel_x_m > 0.0) || !(h.pixel_y_m > 0.0))
+        throw JFJochException(JFJochExceptionCategory::InputParameterInvalid,
+                              path + " has no pixel size in its header (no '# Pixel_size' line); "
+                              "it carries a CBF binary section but is not a detector image");
 
     binary_start = *start;
     return h;

tests/JFJochReaderTest.cpp

@@ -4005,3 +4005,78 @@ TEST_CASE("JFJochCBFReader_AxisTableStatesTheMounting", "[HDF5][Full]") {
         RemoveMiniCBFSweep("cbfaxis_turn");
     }
 }
+
+namespace {
+    // A byte-offset CBF whose header lines the caller chooses, so a header that is MISSING something
+    // can be built. WriteMiniCBFSweep above always writes a complete PILATUS head.
+    void WriteRawMiniCBF(const std::string &name, const std::string &head_lines,
+                         int64_t nx, int64_t ny) {
+        std::ostringstream head;
+        head << "###CBF: VERSION 1.5\n_array_data.header_contents\n" << head_lines
+             << "_array_data.data\n--CIF-BINARY-FORMAT-SECTION--\n"
+             << "Content-Type: application/octet-stream;\n"
+             << "     conversions=\"x-CBF_BYTE_OFFSET\"\n"
+             << "Content-Transfer-Encoding: BINARY\n"
+             << "X-Binary-Size: " << nx * ny << "\n"
+             << "X-Binary-Element-Type: \"signed 32-bit integer\"\n"
+             << "X-Binary-Number-of-Elements: " << nx * ny << "\n"
+             << "X-Binary-Size-Fastest-Dimension: " << nx << "\n"
+             << "X-Binary-Size-Second-Dimension: " << ny << "\n\n";
+        std::ofstream f(name, std::ios::binary);
+        const std::string text = head.str();
+        f.write(text.data(), static_cast<std::streamsize>(text.size()));
+        f.write(reinterpret_cast<const char *>(minicbf::BINARY_SEPARATOR),
+                sizeof(minicbf::BINARY_SEPARATOR));
+        const std::vector<char> zeros(static_cast<size_t>(nx * ny), 0);
+        f.write(zeros.data(), static_cast<std::streamsize>(zeros.size()));
+    }
+}
+
+// Three ways a CBF that is not a detector image, or is one with a hole in its head, used to be
+// opened anyway - each of them silently, which is the failure this project cares most about.
+TEST_CASE("JFJochCBFReader_incomplete_header_is_refused_not_misread", "[HDF5][Full]") {
+    const int64_t nx = 24, ny = 16;
+
+    SECTION("a byte-offset CBF with no PILATUS header is not ours to read") {
+        // XDS writes its correction files in exactly this shape: a real byte-offset binary section
+        // and not one '#' line. Claiming it opened it with a pixel size of zero, which collapses
+        // every resolution and scattering vector the run computes.
+        WriteRawMiniCBF("cbfbare_0001.cbf", "", nx, ny);
+        CHECK_FALSE(JFJochCBFReader::CanRead("cbfbare_0001.cbf"));
+        JFJochCBFReader reader;
+        CHECK_THROWS_AS(reader.ReadFiles("cbfbare_0001.cbf"), JFJochException);
+        remove("cbfbare_0001.cbf");
+    }
+
+    SECTION("a header number that does not parse is a malformed header, not a raw std throw") {
+        // The captures are character classes, not number grammars: "[\\d.eE+-]+" matches a bare ".".
+        // std::stod answers that with std::invalid_argument, which CanRead does not catch - so merely
+        // LOOKING at the file threw out of the format probe.
+        WriteRawMiniCBF("cbfbadnum_0001.cbf",
+                        "# Pixel_size 172e-6 m x 172e-6 m\n# Wavelength . A\n"
+                        "# Detector_distance 0.3 m\n# Count_cutoff 1000 counts\n", nx, ny);
+        CHECK_NOTHROW(JFJochCBFReader::CanRead("cbfbadnum_0001.cbf"));
+        CHECK_FALSE(JFJochCBFReader::CanRead("cbfbadnum_0001.cbf"));
+        remove("cbfbadnum_0001.cbf");
+    }
+
+    SECTION("no Count_cutoff does not mean every pixel is saturated") {
+        // SaturationLimitFromValue(0) is 1, so an absent line marked every pixel at or above one
+        // count as an overload and the integration accept gate then dropped the whole reflection.
+        WriteMiniCBFSweep("cbfnocut", "# Oscillation_axis OMEGA\n", nx, ny);
+        // ...rewrite frame 1 without the Count_cutoff line, keeping everything else.
+        WriteRawMiniCBF("cbfnocut_0001.cbf",
+                        "# Detector: PILATUS3 6M, S/N 60-0119\n"
+                        "# Pixel_size 172e-6 m x 172e-6 m\n"
+                        "# Silicon sensor, thickness 0.000450 m\n"
+                        "# Wavelength 0.96864 A\n# Detector_distance 0.33161 m\n"
+                        "# Beam_xy (12.00, 8.00) pixels\n"
+                        "# Start_angle 0 deg.\n# Angle_increment 0.1000 deg.\n", nx, ny);
+        JFJochCBFReader reader;
+        REQUIRE_NOTHROW(reader.ReadFiles("cbfnocut_0001.cbf"));
+        const auto x = reader.GetDataset()->experiment;
+        CHECK(x.GetSaturationLimit() > 1);
+        reader.Close();
+        RemoveMiniCBFSweep("cbfnocut");
+    }
+}