fix: harden IVF file handling, filtered test runs, and formatter scripts from the audit

.github/workflows/test.yaml

@@ -374,6 +374,16 @@ jobs:
           simd_filter+=':HnswSaveTest.SuccessfulSavePreservesSearchAfterLoading'
           simd_filter+=':HnswPaddingTest.TailDimensionsMatchEveryAvailableSearchBackend'
           simd_filter+=':IvfLifecycleTest.BatchTailSurvivesAddRemoveAndReload:HnswRemoveTest.*'
+          # A renamed suite would silently drop out of the filter: GoogleTest exits 0
+          # with zero matches, so require every pattern to select at least one test.
+          IFS=: read -ra patterns <<< "$simd_filter"
+          for pattern in "${patterns[@]}"; do
+            if ! build-install/tests/rabitq_tests --gtest_list_tests \
+                --gtest_filter="$pattern" | grep -q '^  '; then
+              echo "error: no test matches the filter pattern $pattern" >&2
+              exit 1
+            fi
+          done
           for cpu_model in hsw icl; do
             RABITQ_TEST_CPU="$cpu_model" "$RUNNER_TEMP/sde/sde64" "-$cpu_model" -- \
               build-install/tests/rabitq_tests --gtest_filter="$simd_filter"

include/rabitqlib/clustering/detail/kmeans.hpp

@@ -297,7 +297,6 @@ class LloydKMeans : public Parameters {
         assignments.resize(n);
         distances.resize(n);
         iteration_stats.clear();
-        iteration_stats.reserve(niter);
         initialize_centroids(x, n);
         if (spherical) {
             normalize_centroids();

include/rabitqlib/index/ivf/initializer.hpp

@@ -119,13 +119,19 @@ class FlatInitializer : public Initializer {
             reinterpret_cast<const char*>(centroids_.data()),
             static_cast<std::streamsize>(sizeof(float) * dim_ * num_cluster_)
         );
+        if (!output) {
+            throw std::runtime_error("IVF Flat initializer cannot be written");
+        }
     }
 
     void load(std::ifstream& input, const char* /*filename*/) override {
         input.read(
             reinterpret_cast<char*>(centroids_.data()),
             static_cast<std::streamsize>(sizeof(float) * dim_ * num_cluster_)
         );
+        if (!input) {
+            throw std::runtime_error("IVF Flat initializer cannot be read");
+        }
     }
 };
 

scripts/apply-format.sh

@@ -18,4 +18,4 @@ if [[ "$($formatter --version)" != *"version 23."* ]]; then
 fi
 
 "$repo_root/scripts/clang-format-files.sh" "$@" \
-    | xargs -0 --no-run-if-empty "$formatter" -i
+    | xargs -0 -r "$formatter" -i

scripts/check-format.sh

@@ -34,5 +34,5 @@ fi
 
 if ((${#files[@]} > 0)); then
     printf '%s\0' "${files[@]}" \
-        | xargs -0 --no-run-if-empty "$formatter" --dry-run --Werror
+        | xargs -0 -r "$formatter" --dry-run --Werror
 fi

scripts/clang-format-files.sh

@@ -13,7 +13,15 @@ else
         ':(exclude)include/rabitqlib/third/**'
 fi | while IFS= read -r -d '' file; do
     if [[ "$file" == /* ]]; then
-        file="$(realpath --relative-to="$repo_root" "$file")"
+        # Resolve the directory portably (no GNU realpath) and keep only
+        # repository paths; anything outside is skipped below.
+        directory="$(cd "$(dirname "$file")" 2>/dev/null && pwd -P)" || continue
+        file="$directory/$(basename "$file")"
+        if [[ "$file" == "$repo_root"/* ]]; then
+            file="${file#"$repo_root"/}"
+        else
+            continue
+        fi
     else
         file="${file#./}"
     fi

scripts/format-changed.sh

@@ -17,7 +17,14 @@ if [[ "$($formatter --version)" != *"version 23."* ]]; then
     exit 1
 fi
 
-mapfile -d '' absolute_files < <("$repo_root/scripts/clang-format-files.sh")
+# Process substitution would discard the file-list helper's exit status.
+file_list="$(mktemp)"
+trap 'rm -f -- "$file_list"' EXIT
+if ! "$repo_root/scripts/clang-format-files.sh" > "$file_list"; then
+    echo "error: could not list files for formatting" >&2
+    exit 1
+fi
+mapfile -d '' absolute_files < "$file_list"
 files=()
 for file in "${absolute_files[@]}"; do
     files+=("${file#"$repo_root/"}")

src/index/ivf.cpp

@@ -701,6 +701,9 @@ void IVF::save(const char* filename) const {
 
 void IVF::write_file(const char* filename) const {
     std::ofstream output(io_impl::filesystem_path(filename), std::ios::binary);
+    if (!output.is_open()) {
+        throw std::runtime_error("IVF cannot open index file for writing");
+    }
     output.exceptions(std::ios::failbit | std::ios::badbit);
     const uint32_t flags = raw_reranking_ ? 1 : 0;
     const uint64_t stored_padded_dim = padded_dim_;
@@ -950,6 +953,9 @@ void IVF::load(const char* filename) {
             }
         }
     }
+    if (remaining != 0) {
+        throw std::runtime_error("IVF index file has unexpected trailing data");
+    }
     loaded.ready_ = true;
 
     input.close();

src/index/qg_builder.cpp

@@ -57,6 +57,9 @@ QGBuilder::QGBuilder(
     , dim_(qg_.dimension())
     , max_degree_(qg_.degree_bound())
     , seed_(seed) {
+    if (ef_construction == 0) {
+        throw std::invalid_argument("QuantizedGraph ef_construction must be positive");
+    }
     if (cache_vectors) {
         cache_.emplace();
     }

tests/CMakeLists.txt

@@ -102,8 +102,12 @@ if(RABITQ_BUILD_INDEX)
     # Binding is initialized before main; test it in its own process.
     add_test(NAME OpenMPBinding COMMAND rabitq_tests
         --gtest_filter=ThreadCount.BoundParallelForUsesRuntimePlacementAndPropagatesErrors)
+    # GoogleTest exits 0 when a filter matches nothing; require the one expected test
+    # to be selected. It skips itself on runtimes without thread placement (MSVC).
     set_tests_properties(OpenMPBinding PROPERTIES
-        ENVIRONMENT "OMP_PROC_BIND=spread;OMP_PLACES=threads;OMP_NUM_THREADS=4")
+        ENVIRONMENT "OMP_PROC_BIND=spread;OMP_PLACES=threads;OMP_NUM_THREADS=4"
+        PASS_REGULAR_EXPRESSION "\\[  PASSED  \\] 1 test\\.;\\[  SKIPPED \\] 1 test"
+        FAIL_REGULAR_EXPRESSION "\\[  FAILED  \\]")
 
     # Substitute CPU predicates only in this executable to exercise real cached dispatch
     # on a CPU without SIMD. The static library's detector object is not linked here.

tests/common/temp_file.hpp

@@ -7,14 +7,33 @@
 #include <string>
 #include <system_error>
 
+#if defined(_WIN32)
+#include <process.h>
+#else
+#include <unistd.h>
+#endif
+
 namespace rabitq_test {
 
+// Prefix a temporary file name with the process ID so suites running at the same
+// time in one temporary directory cannot delete or overwrite each other's files.
+inline std::string process_unique(const std::string& name) {
+#if defined(_WIN32)
+    const int pid = _getpid();
+#else
+    const int pid = static_cast<int>(getpid());
+#endif
+    return "rabitq-" + std::to_string(pid) + "-" + name;
+}
+
 // A file under the GoogleTest temporary directory that is removed, together with an IVF
 // HNSW sidecar written next to it, when the test leaves its scope, including after a
-// failed assertion. Leftovers from an interrupted earlier run are removed up front.
+// failed assertion. Leftovers from an interrupted earlier run of this process are
+// removed up front.
 class TempFile {
    public:
-    explicit TempFile(const std::string& name) : path_(::testing::TempDir() + name) {
+    explicit TempFile(const std::string& name)
+        : path_(::testing::TempDir() + process_unique(name)) {
         remove();
     }
     ~TempFile() { remove(); }

tests/core_consumer/run.cmake

@@ -99,7 +99,8 @@ foreach(component IN LISTS components)
             "-DRABITQ_SOURCE_DIR=${consumer_source}"
             "-DCMAKE_PREFIX_PATH=${work}/relocated")
         run("${CMAKE_COMMAND}" --build "${build}" --config Release --parallel 2)
-        run("${CMAKE_CTEST_COMMAND}" --test-dir "${build}" -C Release --output-on-failure)
+        run("${CMAKE_CTEST_COMMAND}" --test-dir "${build}" -C Release --output-on-failure
+            --no-tests=error)
     endforeach()
     foreach(build IN ITEMS producer ADD_SUBDIRECTORY FETCHCONTENT)
         if(EXISTS "${work}/${build}/compile_commands.json")
@@ -175,5 +176,5 @@ run("${CMAKE_COMMAND}" -S "${RABITQ_SOURCE_DIR}/benchmarks/core"
     "-DCMAKE_PREFIX_PATH=${WORK_DIR}/core/relocated")
 run("${CMAKE_COMMAND}" --build "${WORK_DIR}/benchmark" --config Release --parallel 2)
 run("${CMAKE_CTEST_COMMAND}" --test-dir "${WORK_DIR}/benchmark" -C Release
-    --output-on-failure)
+    --output-on-failure --no-tests=error)
 message(STATUS "External consumers and core benchmark passed for: ${components}")

tests/dispatch/fastscan_no_simd_test.cpp

@@ -5,6 +5,7 @@
 #include <cstdint>
 #include <stdexcept>
 
+#include "fastscan_backend.hpp"
 #include "rabitqlib/clustering/rabitqkmeans.hpp"
 #include "rabitqlib/fastscan/fastscan.hpp"
 #include "rabitqlib/fastscan/highacc_fastscan.hpp"
@@ -81,6 +82,8 @@ TEST(FastScanDispatchTest, HighAccuracyUsesScalarFallbackWithoutSimd) {
     transfer_lut_hacc(lut.data(), kDim, high_accuracy_lut.data());
     std::array<int32_t, kBatchSize> result{};
     accumulate_hacc(codes.data(), high_accuracy_lut.data(), result.data(), kDim);
+    // The AVX2 kernel produces the same sums for this layout; prove the fallback ran.
+    EXPECT_STREQ(detail::selected_backend().name, "generic");
     for (const auto value : result) {
         EXPECT_EQ(value, 4000);
     }

tests/unit/rabitqlib/index/historical_persistence_test.cpp

@@ -18,6 +18,7 @@
 #include "rabitqlib/index/ivf/ivf.hpp"
 #include "rabitqlib/index/symqg/qg.hpp"
 #include "rabitqlib/index/types.hpp"
+#include "temp_file.hpp"
 
 namespace rabitqlib {
 namespace {
@@ -65,8 +66,9 @@ TEST_P(HistoricalPersistenceTest, LoadsSearchesResavesAndPreservesStateAfterFail
         expected >> reference_ids[i] >> reference_distances[i];
     }
     ASSERT_TRUE(expected.good());
-    const auto output = std::filesystem::temp_directory_path() /
-                        ("rabitq_historical_" + GetParam().directory + "_" + name);
+    const auto output =
+        std::filesystem::temp_directory_path() /
+        rabitq_test::process_unique("historical_" + GetParam().directory + "_" + name);
     const auto compare = [&] {
         EXPECT_EQ(ids, reference_ids);
         for (size_t i = 0; i < ids.size(); ++i) {

tests/unit/rabitqlib/index/initializer_test.cpp

@@ -390,5 +390,35 @@ TEST(ParallelForTest, JoinsWorkersBeforeRethrowingFunctionException) {
     EXPECT_TRUE(result.get());
 }
 
+TEST(IvfInitializerTest, FlatInitializerReportsStreamFailures) {
+    constexpr size_t kDim = 8;
+    constexpr size_t kCount = 3;
+    std::vector<float> centroids(kCount * kDim, 1.5F);
+    FlatInitializer flat(kDim, kCount);
+    flat.add_vectors(centroids.data(), 1);
+    const rabitq_test::TempFile path_file("rabitq_flat_initializer.bin");
+    const std::string& path = path_file.path();
+    {
+        std::ofstream output(path, std::ios::binary);
+        ASSERT_NO_THROW(flat.save(output, path.c_str()));
+    }
+    {
+        // A stream that failed to open cannot be written.
+        std::ofstream unopened(path + ".missing/initializer.bin", std::ios::binary);
+        EXPECT_THROW(flat.save(unopened, path.c_str()), std::runtime_error);
+    }
+    {
+        std::ifstream input(path, std::ios::binary);
+        FlatInitializer restored(kDim, kCount);
+        ASSERT_NO_THROW(restored.load(input, path.c_str()));
+    }
+    std::filesystem::resize_file(
+        std::filesystem::u8path(path), sizeof(float) * kDim * kCount - 1
+    );
+    std::ifstream truncated(path, std::ios::binary);
+    FlatInitializer partial(kDim, kCount);
+    EXPECT_THROW(partial.load(truncated, path.c_str()), std::runtime_error);
+}
+
 }  // namespace
 }  // namespace rabitqlib::ivf

tests/unit/rabitqlib/index/ivf_test.cpp

@@ -1548,6 +1548,45 @@ TEST(IvfInputTest, RejectsNonFiniteAndOversizedCoordinatesBeforeMutation) {
     );
 }
 
+TEST(IvfPersistenceTest, RejectsTrailingBytesAndReportsUnopenableDestination) {
+    constexpr size_t kCount = 33;
+    const auto points = random_rows(kCount, 911);
+    const auto centroids = random_rows(kDynNumClusters, 912);
+    std::vector<PID> assigned(kCount);
+    for (size_t i = 0; i < kCount; ++i) {
+        assigned[i] = static_cast<PID>(i % kDynNumClusters);
+    }
+    IVF index(kCount, kDynDim, kDynNumClusters, 4);
+    index.construct(points.data(), centroids.data(), assigned.data(), false, 1);
+    const rabitq_test::TempFile path_file("rabitq_ivf_trailing.index");
+    const std::string& path = path_file.path();
+    index.save(path.c_str());
+
+    IVF loaded;
+    ASSERT_NO_THROW(loaded.load(path.c_str()));
+    {
+        std::ofstream append(path, std::ios::binary | std::ios::app);
+        const std::array<char, 64> extra{};
+        append.write(extra.data(), static_cast<std::streamsize>(extra.size()));
+    }
+    try {
+        loaded.load(path.c_str());
+        ADD_FAILURE() << "trailing bytes were accepted";
+    } catch (const std::runtime_error& error) {
+        EXPECT_STREQ(error.what(), "IVF index file has unexpected trailing data");
+    }
+    // The failed load leaves the previously loaded index intact.
+    EXPECT_EQ(loaded.num_points(), kCount);
+
+    const std::string missing_directory = path + ".missing/index.bin";
+    try {
+        index.save(missing_directory.c_str());
+        ADD_FAILURE() << "saving into a missing directory succeeded";
+    } catch (const std::runtime_error& error) {
+        EXPECT_STREQ(error.what(), "IVF cannot open index file for writing");
+    }
+}
+
 }  // namespace
 }  // namespace rabitqlib::ivf
 

tests/unit/rabitqlib/index/qg_test.cpp

@@ -2192,3 +2192,15 @@ TEST(QgInputTest, RejectsNonFiniteAndOversizedCoordinatesBeforeMutation) {
     );
 }
 }  // namespace rabitqlib::symqg
+
+namespace rabitqlib::symqg {
+TEST(QgConstructionTest, RejectsZeroEfConstruction) {
+    constexpr size_t kCount = 33, kDim = 64;
+    const std::vector<float> data(kCount * kDim, 0.25F);
+    for (auto init : {QGInitialization::Random, QGInitialization::PiPNN}) {
+        QuantizedGraph<float> graph(kCount, kDim, 32);
+        EXPECT_THROW((QGBuilder(graph, 0, data.data(), 1, init)), std::invalid_argument);
+        EXPECT_EQ(graph.num_points(), 0U);
+    }
+}
+}  // namespace rabitqlib::symqg