refactor: unify naming and style across the library

.clang-format

@@ -25,6 +25,8 @@ DerivePointerAlignment: false
 FixNamespaceComments: true
 IncludeBlocks: Regroup
 IndentCaseLabels: true
+# Brace every control statement body; this was the dominant style before it was enforced.
+InsertBraces: true
 KeepEmptyLinesAtTheStartOfBlocks: false
 PointerAlignment: Left
 ReferenceAlignment: Left

.clang-tidy

@@ -31,13 +31,13 @@ Checks: >
 
 # LLVM 23's new wildcard matches must not change enum ABI, replace the project's
 # pragma-once convention, or reject the CPU detector's required inline assembly.
-# Keep the established checks blocking. Naming starts as warnings while existing
-# internal/parameter names are migrated; do not bulk-fix public interfaces.
-WarningsAsErrors: "*,-readability-identifier-naming"
+# Every enabled check is blocking, including naming. Retained public spellings are
+# listed in the IgnoredRegexp options below; do not bulk-fix public interfaces.
+WarningsAsErrors: "*"
 
 CheckOptions:
-  # Check locals, parameters and private fields. Public member/function names,
-  # types, enums and macros retain their current compatibility contracts.
+  # Public names that keep their compatibility contracts are listed in the
+  # IgnoredRegexp options below instead of being renamed.
   readability-identifier-naming.LocalVariableCase: lower_case
   readability-identifier-naming.ParameterCase: lower_case
   # M is the existing HNSW graph-degree parameter, including its public constructor.
@@ -50,4 +50,35 @@ CheckOptions:
   readability-identifier-naming.PrivateMemberSuffix: '_'
   # Preserve these existing graph parameters and formula coefficient names.
   # Keep exceptions explicit rather than exempting all capitalized identifiers.
-  readability-identifier-naming.PrivateMemberIgnoredRegexp: '^(M|maxM|maxM0|G_add|G_error|G_k1xSumq|G_kbaseSumq|G_kbxSumq)_$'
+  readability-identifier-naming.PrivateMemberIgnoredRegexp: '^(M|G_add|G_error|G_k1xSumq|G_kbaseSumq|G_kbxSumq)_$'
+  readability-identifier-naming.ProtectedMemberCase: lower_case
+  readability-identifier-naming.ProtectedMemberSuffix: '_'
+  readability-identifier-naming.PublicMemberCase: lower_case
+  # Functions, types, namespaces, constants and macros follow AGENTS.md. The
+  # ignored names are retained public API or names the standard library requires.
+  readability-identifier-naming.FunctionCase: lower_case
+  readability-identifier-naming.MethodCase: lower_case
+  readability-identifier-naming.MethodIgnoredRegexp: '^M$'
+  readability-identifier-naming.NamespaceCase: lower_case
+  readability-identifier-naming.ClassCase: CamelCase
+  readability-identifier-naming.StructCase: CamelCase
+  readability-identifier-naming.StructIgnoredRegexp: '^rebind$'
+  readability-identifier-naming.EnumCase: CamelCase
+  readability-identifier-naming.EnumConstantCase: CamelCase
+  readability-identifier-naming.EnumConstantIgnoredRegexp: '^(METRIC_L2|METRIC_IP|RECONSTRUCTION|UNBIASED_ESTIMATION|PLAIN)$'
+  readability-identifier-naming.TypeAliasCase: CamelCase
+  readability-identifier-naming.TypeAliasIgnoredRegexp: '^(value_type|pointer|reference|difference_type|iterator_category|is_always_equal|other|maxheap|minheap|ex_ipfunc)$'
+  readability-identifier-naming.TypedefCase: CamelCase
+  readability-identifier-naming.TemplateParameterCase: CamelCase
+  readability-identifier-naming.GlobalConstantCase: CamelCase
+  readability-identifier-naming.GlobalConstantPrefix: k
+  readability-identifier-naming.StaticConstantCase: CamelCase
+  readability-identifier-naming.StaticConstantPrefix: k
+  readability-identifier-naming.ClassConstantCase: CamelCase
+  readability-identifier-naming.ClassConstantPrefix: k
+  readability-identifier-naming.MacroDefinitionCase: UPPER_CASE
+  # matrix_kernels.hpp temporarily renames the Eigen namespace for each ISA build.
+  readability-identifier-naming.MacroDefinitionIgnoredRegexp: '^Eigen$'
+  # Named parameters must match between declarations and definitions, without the
+  # default prefix/suffix leniency. Unnamed declaration parameters remain allowed.
+  readability-inconsistent-declaration-parameter-name.Strict: true

.github/workflows/test.yaml

@@ -360,10 +360,10 @@ jobs:
         env:
           OMP_NUM_THREADS: "2"
         run: |
-          simd_filter='CpuFeatures.*:FloatDistance.*:MatrixDispatchTest.*:FastScan*.*:BatchEstimatorTest.*'
-          simd_filter+=':FhtDispatchTest.*:FlipSignTest.*:KacsWalkTest.*:ExcodeIp.*:MaskIpX0Q.*'
-          simd_filter+=':PortableKernels.*:QueryPreparation.*:WarmupIpX0Q.*:PackBinary.*'
-          simd_filter+=':ExplicitSimd/RabitqRescaleBackendTest.*:QGEstimatorTest.*'
+          simd_filter='CpuFeaturesTest.*:FloatDistanceTest.*:MatrixDispatchTest.*:FastScan*.*'
+          simd_filter+=':FhtDispatchTest.*:FlipSignTest.*:KacsWalkTest.*:ExcodeIpTest.*:MaskIpX0QTest.*'
+          simd_filter+=':PortableKernelsTest.*:QueryPreparationTest.*:WarmupIpX0QTest.*:PackBinaryTest.*'
+          simd_filter+=':ExplicitSimd/RabitqRescaleBackendTest.*:QgEstimatorTest.*'
           simd_filter+=':QGKMeansTest.TrainsAndReturnsAssignmentsForFinalCentroids'
           simd_filter+=':RaBitQKMeansTest.FinalModesKeepFlatLloydAssignmentsAcrossClusterCounts'
           simd_filter+=':IvfPersistenceTest.ReloadsRawAndQuantizedStorage'

benchmarks/core/main.cpp

@@ -68,13 +68,13 @@ int run(int argc, char** argv) {
     const size_t count = argc == 1 ? 4096 : number(argv[3]);
     const size_t queries = argc == 1 ? 64 : number(argv[4]);
     const size_t repeats = argc == 1 ? 5 : number(argv[5]);
-    constexpr size_t k = 10;
+    constexpr size_t kTopK = 10;
     if (dim < 64 || dim % 32 != 0) {
         throw std::invalid_argument(
             "This FHT/Kac benchmark requires dim >= 64 and divisible by 32"
         );
     }
-    if (count < k) {
+    if (count < kTopK) {
         throw std::invalid_argument("At least ten vectors are needed for recall@10");
     }
     const api::Parameters parameters{dim, bits, rabitqlib::METRIC_L2};
@@ -147,10 +147,10 @@ int run(int argc, char** argv) {
             squared_error += error * error;
             max_error = std::max(max_error, std::abs(error));
         }
-        std::partial_sort(exact.begin(), exact.begin() + k, exact.end());
-        std::partial_sort(estimated.begin(), estimated.begin() + k, estimated.end());
-        for (size_t i = 0; i < k; ++i) {
-            for (size_t j = 0; j < k; ++j) {
+        std::partial_sort(exact.begin(), exact.begin() + kTopK, exact.end());
+        std::partial_sort(estimated.begin(), estimated.begin() + kTopK, estimated.end());
+        for (size_t i = 0; i < kTopK; ++i) {
+            for (size_t j = 0; j < kTopK; ++j) {
                 hits += estimated[i].second == exact[j].second ? 1 : 0;
             }
         }
@@ -167,7 +167,7 @@ int run(int argc, char** argv) {
         << '\n'
         << "# rmse=" << std::sqrt(squared_error / static_cast<double>(comparisons))
         << " max_abs_error=" << max_error << " flat_recall_at_10="
-        << static_cast<double>(hits) / static_cast<double>(product(queries, k)) << '\n'
+        << static_cast<double>(hits) / static_cast<double>(product(queries, kTopK)) << '\n'
         << "operation,repeat,operations,seconds,ops_per_second,ns_per_operation,checksum\n";
     measure("encode_packed", count, repeats, [&] {
         double checksum = 0;

benchmarks/hotpaths/core.cpp

@@ -116,8 +116,9 @@ void run(int argc, char** argv) {
         bench::product(rows, iterations),
         reps,
         [&] {
-            for (size_t i = 0; i < iterations; ++i)
+            for (size_t i = 0; i < iterations; ++i) {
                 encode(threads > 1);
+            }
         },
         checksum
     );

benchmarks/hotpaths/fastscan.cpp

@@ -104,8 +104,9 @@ void run(int argc, char** argv) {
         bench::product(batches, kRows),
         reps,
         [&] {
-            for (size_t batch = 0; batch < batches; ++batch)
+            for (size_t batch = 0; batch < batches; ++batch) {
                 score();
+            }
         },
         [&] {
             uint64_t h = bench::hash(distance.data(), sizeof(distance));

benchmarks/hotpaths/load.cpp

@@ -28,8 +28,9 @@ void run(int argc, char** argv) {
     if (mode != "prepare" && mode != "load") {
         throw std::invalid_argument("Mode must be prepare or load");
     }
-    if (count < 8)
+    if (count < 8) {
         throw std::invalid_argument("At least eight centroids are required");
+    }
     const auto data = bench::values(bench::product(count, dim));
     if (mode == "prepare") {
         // Repeated smoke tests reuse their fixture; never replace an existing file.
@@ -65,11 +66,12 @@ void run(int argc, char** argv) {
             h = bench::hash(&candidate.id, sizeof(candidate.id), h);
             h = bench::hash(&candidate.distance, sizeof(candidate.distance), h);
         }
-        if (rep == 0)
+        if (rep == 0) {
             expected = h;
-        else {
-            if (h != expected)
+        } else {
+            if (h != expected) {
                 throw std::runtime_error("Loaded search results changed");
+            }
             bench::row("sidecar_load", rep - 1, 1, seconds, h);
         }
     }

docs/development.md

@@ -602,7 +602,8 @@ preprocessing objects, `visited` for visited state.
 
 **Search windows**
 
-`ef_search` for a per-call graph window, `kDefaultEfSearch` for HNSW's fixed fallback, and
+`ef_search` for a per-call graph window, `default_ef_search_` for QG's stored default,
+`kDefaultEfSearch` for HNSW's fixed fallback, and
 `ef_construction` for graph-building search breadth. IVF's `nprobe` counts clusters and
 remains distinct.
 
@@ -640,7 +641,8 @@ of 32. These settings are not interchangeable.
 Public names and Python keywords are compatibility contracts. For example,
 `Cluster::num()`, the protected `Initializer::num_cluster_`, QG's
 `degree_bound()`/`set_ef()`, Python's `ef`, and `QGKMeansParameters::ef_build`
-retain their existing spellings. C++ parameter names may be clarified without
+retain their existing spellings, as do the Python vocabulary `random_seed`/`seed`,
+`train`/`build`, `obj`, `nsplit`, `final_obj` and `niter`. C++ parameter names may be clarified without
 changing types, order or defaults. Keep declarations, definitions and comments
 in agreement; do not rename methods or literals by textual substitution.
 
@@ -666,6 +668,8 @@ public and protected names, Python keywords, field order and numerical operation
 - Clustering's private state uses `dim_`, `num_points_`, `num_clusters_`,
   `num_workers_` and `data_`. Keep public `d`, `k`, `num_threads` and graph parameter
   names compatible. Retain the existing private `niter_` graph iteration name.
+  Per-point centroid IDs are `assignments` (arrays) and `cluster_id` (scalars);
+  `label` stays reserved for HNSW's caller-visible labels.
 - FHT backend arguments use `rotated_data`, `padded_dim`, `trunc_dim` and
   `scale_factor` consistently across declarations, dispatch and implementations.
   A lower-level kernel's `dim` still denotes that kernel's own input shape.
@@ -674,6 +678,37 @@ Keep short mathematical symbols, loop indices and SIMD register names when their
 meaning is local and clear. Naming cleanup must not alter formulas, arithmetic
 order, packed layouts, ISA selection or runtime behavior.
 
+### Library-wide code conventions
+
+These rules apply to new and changed code; the listed exceptions predate them.
+
+- **Identifiers.** `.clang-tidy` checks the case of every identifier kind and lists
+  the public spellings kept for compatibility; extend those lists only for retained
+  public API. Naming findings fail the clang-tidy check like any other. Template
+  parameters are PascalCase; `T` is the floating-point element type and `TP` the
+  code or ID element type.
+- **Linkage.** Follow [Implementation linkage](#implementation-linkage) for code
+  compiled under several ISA flags, including public headers that ISA-flagged sources
+  include. Other header functions are plain `inline`, and helpers used by one `.cpp`
+  file go in an anonymous namespace.
+- **Namespaces.** Internal code uses `<module>::detail` or an anonymous namespace.
+  Existing public `*_impl` namespaces are compatibility names, not a pattern.
+- **Errors.** New messages read `"<Component> <text>"` with the component `HNSW`,
+  `IVF`, `QuantizedGraph`, `PiPNN`, `FastScan` or `FastScan high-accuracy`, no colon
+  after it and no trailing period. Core keeps identifier-first messages. Python binding
+  argument errors start with the lowercase parameter name and state errors with the
+  wrapper class name. Some older index-file messages (for example `Invalid padded
+  dimension in IVF index file`) are asserted by tests and keep their wording; changing
+  a Python-visible message is a behavior change. Allocation failure throws
+  `std::bad_alloc`.
+- **Comments.** Use `//`; keep `/** */` for public API documentation blocks and state
+  preconditions as sentences (`// Requires ...`).
+- **Tests.** Name new suites `<Module><Area>Test` (`Hnsw`, `Ivf`, `Qg`, `FastScan`, ...)
+  with PascalCase test names and no underscores. Older suites such as `ThreadCount`
+  keep their names because CI filters select them. Test constants use `kDim`, `kCount`,
+  `kNumQueries` and `kTopK`. Renaming a suite requires updating the GoogleTest filters
+  in `.github/workflows/test.yaml` and `tests/CMakeLists.txt`.
+
 ## Technical contracts
 
 ### Module build boundaries
@@ -807,7 +842,7 @@ nothing in an old file is reinterpreted. A file that has removals is rejected by
 through 0.5.1, whose `load` range-checks every cluster ID; that rejection is the compatibility path.
 This mark is unrelated to the `SearchBuffer` ID marker. Keep these rules when touching HNSW-RaBitQ code:
 
-- Read cluster IDs only through `get_clusterid_by_internalid`, which masks the bit. Estimators,
+- Read cluster IDs only through `cluster_id_of`, which masks the bit. Estimators,
   routing, and the `add` reconstruction index centroids with it.
 - A removed point stays in the graph: base-layer search still inserts it into `search_pool`, and
   `add` may link to it. Only the result buffer skips it, including when it is the entry point.
@@ -887,6 +922,8 @@ a short-vector improvement does not justify applying it to wider vectors.
 
 Put calculations and scratch-storage helpers outside the dispatcher. Shared implementation
 headers use internal linkage so independently compiled backends retain their own bodies.
+This includes public headers reached from ISA-flagged sources, such as the per-ISA HNSW
+search files: keep their non-template helpers `static inline` rather than plain `inline`.
 The AVX FHT intrinsics in `src/simd/fht_kernels.hpp` use the private kernel namespace
 for the same reason. Rotation uses this implementation on GCC, Clang, and MSVC;
 no assembler is required. Keep the FFHT attribution and MIT license in the