Add a format version to the index file header

DEVELOPER_GUIDE.md

@@ -383,6 +383,28 @@ If your change alters what the Python bindings expose or how an index behaves en
 
 If your changes could affect backward compatibility, please include relevant tests along with your PR.
 
+### Index file format
+
+Every serialized index starts with a fixed header (`nsparse::IndexHeader` in `nsparse/io/io.h`). It is written and parsed centrally, by `write_header`/`read_header` in `nsparse/io/index_io.cpp` — an index type never reads its own header, it receives the already-parsed one:
+
+| Field | Type | Notes |
+|---|---|---|
+| id | `uint32` | fourcc of the index type, e.g. `SEIS` |
+| version | `uint32` | layout revision of the payload that follows |
+| dimension | `int32` | |
+
+The payload follows immediately, and its layout is the index type's own business. A type parses it in `read_index` (copying), `mmap_index` (borrowed from a file mapping), or both — `DiskSeismicIndex` is mmap-only and its `read_index` just throws, while `IDMapIndex` has no `mmap_index` at all and instead threads `io_flags` down to its delegate.
+
+Versions are numbered **per index type**, not per file: `IndexIO::format_version()` returns the type's own `kFormatVersion`, so revising one type's payload leaves the others' numbering alone. An `IDMapIndex` writes its own header for the id map and then a second, complete header for the delegate it wraps, each with its own version.
+
+The same central code rejects a version outside `1..format_version()` before dispatching to either parse path — a file from a newer build fails with a clear error instead of consuming whatever its fields happen to align with.
+
+To change a payload layout:
+
+1. Bump that type's `kFormatVersion`.
+2. Branch on `header.version` wherever that type actually parses its payload — its `read_index` and/or its `mmap_index` — keeping the older branch so existing files still load.
+3. Add a round-trip test for the new version and a test that reads the old layout.
+
 ### Outdated or irrelevant code
 
 Do not submit code that is not used or needed, even if it's commented. We rely on GitHub as a version control system; code can be restored if needed.

nsparse/disk_seismic_index.cpp

@@ -312,19 +312,21 @@ void DiskSeismicIndex::write_index(IOWriter* io_writer) {
     forward.serialize(io_writer);
 }
 
-void DiskSeismicIndex::read_index(IOReader* /*io_reader*/, int /*io_flags*/) {
+void DiskSeismicIndex::read_index(IOReader* /*io_reader*/,
+                                  const IndexHeader& /*header*/,
+                                  int /*io_flags*/) {
     // The inline forward index is borrowed from a mapping, never copied onto
     // the heap, so this index has no copying read path.
     throw std::runtime_error(
         "DiskSeismicIndex is mmap-only; load with read_index(file, "
         "IndexIoFlag::kUseMmap)");
 }
 
-DiskSeismicIndex* DiskSeismicIndex::mmap_index(int dimension,
+DiskSeismicIndex* DiskSeismicIndex::mmap_index(const IndexHeader& header,
                                                const char* index_file,
                                                size_t pos) {
     throw_if_null(index_file, "index_file must not be null");
-    auto index = std::make_unique<DiskSeismicIndex>(dimension);
+    auto index = std::make_unique<DiskSeismicIndex>(header.dimension);
 
     MmapFile mmap_file(std::string{index_file});
     MmapCursor cursor(mmap_file.data(), mmap_file.size());

nsparse/disk_seismic_index.h

@@ -11,6 +11,7 @@
 #define DISK_SEISMIC_INDEX_H
 #include <array>
 #include <cstddef>
+#include <cstdint>
 #include <vector>
 
 #include "absl/container/flat_hash_set.h"
@@ -52,6 +53,8 @@ struct DiskSeismicSearchParameters : public SeismicSearchParameters {
 class DiskSeismicIndex : public MmapIndex, public IndexIO {
 public:
     static constexpr std::array<char, 4> name = {'D', 'S', 'E', 'I'};
+    // Bump whenever write_index's payload layout changes.
+    static constexpr uint32_t kFormatVersion = 1;
 
     explicit DiskSeismicIndex(int dim);
     DiskSeismicIndex(int dim, SeismicClusterParameters parameter);
@@ -70,16 +73,20 @@ class DiskSeismicIndex : public MmapIndex, public IndexIO {
 
     // Borrows a serialized index from a file mapping. `pos` is where the
     // payload begins.
-    static DiskSeismicIndex* mmap_index(int dimension, const char* index_file,
-                                        size_t pos);
+    static DiskSeismicIndex* mmap_index(const IndexHeader& header,
+                                        const char* index_file, size_t pos);
 
 protected:
     std::vector<InvertedListClusters> clustered_inverted_lists;
 
 private:
+    [[nodiscard]] uint32_t format_version() const override {
+        return kFormatVersion;
+    }
     void write_index(IOWriter* io_writer) override;
     // Unsupported: the inline forward index is mmap-only. Throws.
-    void read_index(IOReader* io_reader, int io_flags = 0) override;
+    void read_index(IOReader* io_reader, const IndexHeader& header,
+                    int io_flags = 0) override;
 
     auto search(idx_t n, const idx_t* indptr, const term_t* indices,
                 const float* values, int k,

nsparse/id_map_index.cpp

@@ -89,7 +89,8 @@ void IDMapIndex::write_index(IOWriter* io_writer) {
     nsparse::detail::write_index(delegate_.get(), io_writer, true);
 }
 
-void IDMapIndex::read_index(IOReader* io_reader, int io_flags) {
+void IDMapIndex::read_index(IOReader* io_reader, const IndexHeader& header,
+                            int io_flags) {
     // Read internal_to_external_ vector
     size_t map_size = 0;
     io_reader->read(&map_size, sizeof(size_t), 1);

nsparse/id_map_index.h

@@ -11,6 +11,7 @@
 #define ID_MAP_INDEX_H
 #include <algorithm>
 #include <array>
+#include <cstdint>
 #include <memory>
 #include <ranges>
 #include <vector>
@@ -79,6 +80,9 @@ class IDMapIndex : public Index, public IndexIO {
 public:
     IDMapIndex() = default;
     static constexpr std::array<char, 4> name = {'I', 'D', 'M', 'P'};
+    // Covers the id map only; the delegate that follows carries its own header
+    // and versions its payload independently.
+    static constexpr uint32_t kFormatVersion = 1;
     // Takes ownership of the delegate index; it is freed when this IDMapIndex
     // is destroyed.
     explicit IDMapIndex(Index*);
@@ -97,8 +101,12 @@ class IDMapIndex : public Index, public IndexIO {
 
     void add_with_ids(idx_t n, const idx_t* indptr, const term_t* indices,
                       const float* values, const idx_t* ids) override;
+    [[nodiscard]] uint32_t format_version() const override {
+        return kFormatVersion;
+    }
     void write_index(IOWriter* io_writer) override;
-    void read_index(IOReader* io_reader, int io_flags = 0) override;
+    void read_index(IOReader* io_reader, const IndexHeader& header,
+                    int io_flags = 0) override;
 
 private:
     // Owns the wrapped delegate index. Using unique_ptr ensures the delegate is

nsparse/inverted_index.cpp

@@ -427,7 +427,8 @@ void InvertedIndex::write_index(IOWriter* io_writer) {
     io_align::write_padded(io_writer, max_term_scores_.data(), scores_size);
 }
 
-void InvertedIndex::read_index(IOReader* io_reader, int io_flags) {
+void InvertedIndex::read_index(IOReader* io_reader, const IndexHeader& header,
+                               int io_flags) {
     size_t num_vectors = 0;
     io_reader->read(&num_vectors, sizeof(size_t), 1);
 
@@ -450,10 +451,10 @@ void InvertedIndex::read_index(IOReader* io_reader, int io_flags) {
     }
 }
 
-InvertedIndex* InvertedIndex::mmap_index(int dimension, const char* index_file,
-                                         size_t pos) {
+InvertedIndex* InvertedIndex::mmap_index(const IndexHeader& header,
+                                         const char* index_file, size_t pos) {
     throw_if_null(index_file, "index_file must not be null");
-    auto index = std::make_unique<InvertedIndex>(dimension);
+    auto index = std::make_unique<InvertedIndex>(header.dimension);
 
     MmapFile mmap_file(std::string{index_file});
     // `pos` is where write_index's payload begins, past the header read_header

nsparse/inverted_index.h

@@ -11,6 +11,7 @@
 #define INVERTED_INDEX_H
 
 #include <array>
+#include <cstdint>
 #include <memory>
 #include <vector>
 
@@ -35,12 +36,14 @@ class InvertedIndex : public MmapIndex, public IndexIO {
     size_t num_vectors() const override { return num_vectors_; }
     std::array<char, 4> id() const override { return name; }
     static constexpr std::array<char, 4> name = {'I', 'N', 'V', 'T'};
+    // Bump whenever write_index's payload layout changes.
+    static constexpr uint32_t kFormatVersion = 1;
 
     // Reads what write_index wrote, with the posting lists borrowing from a
     // mapping of `index_file` instead of being copied onto the heap. `pos` is
     // where the payload begins, past the header read_header consumed.
-    static InvertedIndex* mmap_index(int dimension, const char* index_file,
-                                     size_t pos);
+    static InvertedIndex* mmap_index(const IndexHeader& header,
+                                     const char* index_file, size_t pos);
 
 protected:
     auto search(idx_t n, const idx_t* indptr, const term_t* indices,
@@ -50,8 +53,12 @@ class InvertedIndex : public MmapIndex, public IndexIO {
 
 private:
     // IndexIO overrides
+    [[nodiscard]] uint32_t format_version() const override {
+        return kFormatVersion;
+    }
     void write_index(IOWriter* io_writer) override;
-    void read_index(IOReader* io_reader, int io_flags) override;
+    void read_index(IOReader* io_reader, const IndexHeader& header,
+                    int io_flags) override;
 
     // `id_selector` may be null; when set, only member docs are returned.
     auto single_query(const term_t* indices, const float* values, int size,

nsparse/io/index_io.cpp

@@ -9,9 +9,11 @@
 
 #include "nsparse/io/index_io.h"
 
+#include <cstddef>
 #include <cstdint>
 #include <memory>
 #include <stdexcept>
+#include <string>
 
 #include "nsparse/brutal_index.h"
 #include "nsparse/disk_seismic_index.h"
@@ -24,6 +26,8 @@
 namespace nsparse {
 
 namespace {
+constexpr uint32_t kBitsPerByte = 8;
+
 constexpr uint32_t BRUT = fourcc(BrutalIndex::name);
 constexpr uint32_t SEIS = fourcc(SeismicIndex::name);
 constexpr uint32_t SESQ = fourcc(SeismicScalarQuantizedIndex::name);
@@ -71,57 +75,92 @@ class StreamCloser {
     T* stream_;
 };
 
-// Reads `id`'s payload by borrowing it from the file rather than copying it, or
-// returns nullptr for an index type without a mapped reader. `pos` is where the
-// payload begins, past the header read_header consumed.
-Index* mmap_index_payload(uint32_t id, int dimension, const char* file_name,
+// Reads `header`'s payload by borrowing it from the file rather than copying
+// it, or returns nullptr for an index type without a mapped reader. `pos` is
+// where the payload begins, past the header read_header consumed.
+Index* mmap_index_payload(const IndexHeader& header, const char* file_name,
                           size_t pos) {
-    switch (id) {
+    switch (header.id) {
         case SEIS:
-            return SeismicIndex::mmap_index(dimension, file_name, pos);
+            return SeismicIndex::mmap_index(header, file_name, pos);
         case SESQ:
-            return SeismicScalarQuantizedIndex::mmap_index(dimension, file_name,
+            return SeismicScalarQuantizedIndex::mmap_index(header, file_name,
                                                            pos);
         case INVT:
-            return InvertedIndex::mmap_index(dimension, file_name, pos);
+            return InvertedIndex::mmap_index(header, file_name, pos);
         case DSEI:
-            return DiskSeismicIndex::mmap_index(dimension, file_name, pos);
+            return DiskSeismicIndex::mmap_index(header, file_name, pos);
         default:
             return nullptr;
     }
 }
 
-void write_header(Index* index, IOWriter* io_writer) {
+// The id as it reads in the file, for error messages: a fourcc is four
+// printable characters, and its numeric value is not what a reader would
+// recognise.
+std::string id_to_string(uint32_t id_val) {
+    std::string chars(4, '\0');
+    for (size_t i = 0; i < chars.size(); ++i) {
+        chars[i] = static_cast<char>((id_val >> (kBitsPerByte * i)) & 0xFFU);
+    }
+    return chars;
+}
+
+void write_header(const IndexHeader& header, IOWriter* io_writer) {
     // write index type
-    auto id_val = fourcc(index->id());
+    uint32_t id_val = header.id;
     io_writer->write(&id_val, sizeof(uint32_t), 1);
+    // write payload layout version
+    uint32_t version = header.version;
+    io_writer->write(&version, sizeof(uint32_t), 1);
     // write dimension
-    auto dimension = index->get_dimension();
+    int dimension = header.dimension;
     io_writer->write(&dimension, sizeof(int), 1);
 }
 
-Index* read_header(IOReader* io_reader) {
-    uint32_t id_val = 0;
-    io_reader->read(&id_val, sizeof(uint32_t), 1);
-    int dimension = 0;
-    io_reader->read(&dimension, sizeof(int), 1);
-    switch (id_val) {
+IndexHeader read_header(IOReader* io_reader) {
+    IndexHeader header;
+    io_reader->read(&header.id, sizeof(uint32_t), 1);
+    io_reader->read(&header.version, sizeof(uint32_t), 1);
+    io_reader->read(&header.dimension, sizeof(int), 1);
+    return header;
+}
+
+// Constructs the index the id names, still empty: the payload is what
+// read_index/mmap_index fill in.
+Index* make_index(const IndexHeader& header) {
+    switch (header.id) {
         case BRUT:
-            return new BrutalIndex(dimension);
+            return new BrutalIndex(header.dimension);
         case SEIS:
-            return new SeismicIndex(dimension);
+            return new SeismicIndex(header.dimension);
         case SESQ:
-            return new SeismicScalarQuantizedIndex(dimension);
+            return new SeismicScalarQuantizedIndex(header.dimension);
         case DSEI:
-            return new DiskSeismicIndex(dimension);
+            return new DiskSeismicIndex(header.dimension);
         case IDMP:
             return new IDMapIndex();
         case INVT:
-            return new InvertedIndex(dimension);
+            return new InvertedIndex(header.dimension);
         default:
             throw std::runtime_error("Unknown index type");
     }
 }
+
+// A version outside 1..supported is one this build cannot lay out: either it
+// postdates this binary, or no writer ever produced it. Reading the payload
+// anyway would consume whatever the fields happen to align with, so the file is
+// rejected here instead.
+void throw_if_version_unsupported(const IndexHeader& header,
+                                  uint32_t supported) {
+    if (header.version == 0 || header.version > supported) {
+        throw std::runtime_error("Unsupported " + id_to_string(header.id) +
+                                 " index format version " +
+                                 std::to_string(header.version) +
+                                 "; this build reads versions 1 through " +
+                                 std::to_string(supported));
+    }
+}
 }  // namespace
 
 namespace detail {
@@ -132,20 +171,27 @@ void write_index(Index* index, IOWriter* io_writer, bool keep_open) {
         throw std::runtime_error("Index does not support serialization");
     }
     // write header
-    write_header(index, io_writer);
+    write_header({.id = fourcc(index->id()),
+                  .version = index_io->format_version(),
+                  .dimension = index->get_dimension()},
+                 io_writer);
     // write index customized payload
     index_io->write_index(io_writer);
     closer.close();
 }
 
 Index* read_index(IOReader* io_reader, bool keep_open, int io_flags) {
     StreamCloser closer(io_reader, keep_open);
+    const IndexHeader header = read_header(io_reader);
     // Held so it does not leak if anything below throws, close() included.
-    std::unique_ptr<Index> index(read_header(io_reader));
+    std::unique_ptr<Index> index(make_index(header));
     auto* index_io = dynamic_cast<IndexIO*>(index.get());
     if (index_io == nullptr) {
         throw std::runtime_error("Index does not support serialization");
     }
+    // Ahead of either read below, so a payload this build cannot lay out is
+    // never parsed.
+    throw_if_version_unsupported(header, index_io->format_version());
 
     // handle mmap
     if ((io_flags & IndexIoFlag::kUseMmap) == IndexIoFlag::kUseMmap) {
@@ -155,8 +201,7 @@ Index* read_index(IOReader* io_reader, bool keep_open, int io_flags) {
             // serialize() padded against. An index type without a mapped reader
             // returns null and falls through to the copying read below.
             std::unique_ptr<Index> mapped(mmap_index_payload(
-                fourcc(index->id()), index->get_dimension(),
-                file_io_reader->file_name().c_str(), io_reader->pos()));
+                header, file_io_reader->file_name().c_str(), io_reader->pos()));
             if (mapped != nullptr) {
                 index.reset();
                 closer.close();
@@ -165,7 +210,7 @@ Index* read_index(IOReader* io_reader, bool keep_open, int io_flags) {
         }
     }
 
-    index_io->read_index(io_reader, io_flags);
+    index_io->read_index(io_reader, header, io_flags);
     closer.close();
     return index.release();
 }