Skip to content

feat(cpp): gaia::VectorIndex — flat vector index with persistence - #2807

Open
kovtcharov wants to merge 1 commit into
mainfrom
cpp/vector-index
Open

feat(cpp): gaia::VectorIndex — flat vector index with persistence#2807
kovtcharov wants to merge 1 commit into
mainfrom
cpp/vector-index

Conversation

@kovtcharov

Copy link
Copy Markdown
Contributor

The C++ framework has no vector search of any kind today, so a native GAIA binary cannot do RAG, code indexing, or memory recall — the tools that make an agent domain-specific rather than general-purpose. This adds an exhaustive flat index over float32 embeddings with save/load persistence, which is all the Python SDK actually needs: every FAISS call site in the repo uses IndexFlatL2 or IndexFlatIP (brute-force scans — no IVF, HNSW, or PQ anywhere), so a C++ agent now gets the same rankings Python gets, with zero new dependencies across the Windows/Linux/macOS builds. Scores match Python's convention exactly — 1/(1+d²) for L2 and the raw dot product for inner product — and mismatches raise instead of quietly returning wrong results: a wrong-sized vector names both dimensions, and loading an index built by a different embedder names both models.

Unblocks P2.2 (RAG) and P2.3 (code index). C++ cannot read Python's index.faiss and does not try to; the two runtimes use separate cache directories and share only metadata.json.

Test plan

  • cmake -S cpp -B cpp/build -DGAIA_BUILD_TESTS=ON && cmake --build cpp/build && ctest --test-dir cpp/build --output-on-failure — 501/501 pass (38 new)
  • Recall parity: VectorIndexTest.MatchesPythonIndexFlatL2TopK / MatchesPythonIndexFlatIPTopK assert the same top-5 ordering and scores that faiss 1.13.2 produces for the same vectors (the regeneration snippet is in the test file)
  • Hand-computed distances for both metrics, top-k ordering, ties, k > index size, k = 0, search on an empty index
  • Save/load round-trip returns bit-identical scores; the documented byte layout is asserted offset by offset
  • Corrupt-file guards: impossible vector count, NaN payload, unknown metric code, empty/duplicate ids, truncated and trailing-junk files — each raises, and a failed load leaves the existing index usable
  • Clean under -fsanitize=address,undefined

Closes #2792

The C++ framework had no vector search of any kind, so a native binary could
not do RAG, code indexing, or memory recall. This adds an exhaustive flat index
over float32 embeddings with save/load, which is all the Python SDK actually
needs: every FAISS call site in gaia uses IndexFlatL2 or IndexFlatIP, so the
rankings match vector-for-vector with no BLAS-linked dependency added to three
platform builds.

Scores follow the Python convention exactly — 1/(1+d²) over the squared L2
distance FAISS reports, and the raw dot product for inner product, which is
cosine similarity when normalize-on-add is enabled. Ties break by insertion
order so rankings are reproducible across runs and platforms.

Mismatches raise instead of returning misleading results: a wrong-sized vector
on add/search names both dimensions, and loading an index built by a different
embedder names both models. The .vec parser validates every field against the
actual file length before allocating, and rejects non-finite payloads, so a
corrupt or hostile cache file cannot trigger a huge allocation, an OOB read, or
NaN-poisoned rankings.

C++ cannot read Python's index.faiss and does not try to; the runtimes use
separate cache directories and share only metadata.json.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Verdict: Approve

This adds gaia::VectorIndex — a flat (brute-force) C++ vector index with binary .vec persistence, built to return the exact same rankings as the Python SDK's faiss.IndexFlatL2 / IndexFlatIP without pulling in a BLAS dependency. It's a self-contained building block (header + impl + 40-odd gtest cases + a C++ API-reference section), not yet wired into a caller, and it lands cleanly.

The bottom line: this is a genuinely well-built change. Every failure path raises an actionable error (what failed, why, how to fix) rather than returning a plausible-but-wrong ranking — right in the spirit of GAIA's fail-loudly rule — and the load path guards against a corrupt header ballooning into a multi-terabyte allocation before it reserves anything. No blocking issues found.

Real-world evidence

N/A — this is an internal C++ library component with no user-facing CLI / REST / Agent UI / MCP surface, so there's no runnable surface for the evidence lane to exercise. Correctness rests on the bundled gtest suite (hand-computed L2/IP scores, tie-stability, save/load round-trips, corrupt-file rejection, and numeric parity against faiss 1.13.2) plus static review. The suite couldn't be compiled/run in this review environment (gh/build tooling unavailable here), so the parity and round-trip claims are verified by reading the tests, not by executing them — worth a local ctest run before merge if not already green in CI.

🔍 Technical details

Correctness review — no issues found. Spot-checks that held up:

  • load() DoS guard is correct (cpp/src/vector_index.cpp:715-720): count is validated against payloadBytes / minRecordBytes before any reserve, and since minRecordBytes >= fileDim*4, the subsequent data.reserve(count * fileDim) is bounded by the actual file size. A corrupt count=2^60 header raises a named error instead of throwing a bare std::length_error or over-allocating.
  • Strong exception guarantee on add() (vector_index.cpp:432-447): data_/ids_/positions_ are committed together with rollback, so a throw can't leave ids_ describing a row data_ lacks — which the comment correctly notes would let the next search() read past the buffer.
  • load() is transactional (vector_index.cpp:771-782): parsing writes into locals and only moves them into members after the trailing-byte check passes; a failed load leaves the index usable, and FailedLoadLeavesTheExistingIndexUsable locks that in.
  • Per-record bounds use 64-bit arithmetic (vector_index.cpp:740) so fileDim * 4 can't wrap on a 32-bit target — a real, easy-to-miss overflow, handled.
  • Score convention matches Python: 1/(1+d²) over squared L2 (vector_index.cpp:528), and L2UsesSquaredDistanceLikeFaiss explicitly pins the squared-vs-euclidean distinction that's the usual source of parity drift.
  • Conventions: #include "gaia/export.h" + GAIA_API and the header/src/test + CMakeLists.txt registration match the existing git_tools / session pattern exactly; the edited docs/cpp/api-reference.mdx is already in docs/docs.json nav.

Strengths:

  • Fail-loudly done right — every throw names the offending value and the remedy (re-embed, rebuild, construct without embeddingModel), e.g. the dimension-mismatch and embedding-model-mismatch messages at vector_index.cpp:354-360 / 697-700.
  • Test suite is unusually thorough for the corrupt-input surface: unknown metric code, dim-0-with-count, empty/duplicate ids, model-length past EOF, non-finite payload, truncated and trailing-junk files — plus numeric parity against a named faiss version with a documented regen recipe.
  • Atomic save (temp-file + rename) with the durability caveats (no fsync, fixed .tmp name → no concurrent saves) documented honestly rather than papered over.

Nits: none worth posting.

@kovtcharov

Copy link
Copy Markdown
Contributor Author

The failing C++ Integration Tests (STX) check is not this PR

Diagnosed during the milestone #63 sweep — all three open Wave 1 PRs (#2807, #2809, #2816) fail this same check, and none for a reason related to their diffs. The build never reaches compilation:

CMake Error: Could not find CMAKE_ROOT !!!
Modules directory not found in
C:/Windows/Temp/cmake/cmake-3.31.4-windows-x86_64/share/cmake-3.31

.github/workflows/build_cpp.yml caches CMake under $env:TEMP on the self-hosted runner and gates re-download on Test-Path "$cmakeCached\cmake.exe" — it validates that bin/cmake.exe exists but never that share/cmake-3.31/Modules/ does, which is what CMAKE_ROOT resolves to. Temp cleanup removed share/ and left bin/, so the guard sees a healthy cache, skips the re-download, and prepends a broken CMake to PATH.

Tracked as #2817, fix in flight. Nothing to do on this PR for it.

kovtcharov-amd pushed a commit to Jonesxq/gaia that referenced this pull request Aug 6, 2026
…xe (amd#2818)

Every `cpp/**` PR has been failing the `C++ Integration Tests (STX)`
check before it compiles anything — amd#2807, amd#2809 and amd#2816 are all red
for a reason unrelated to their diffs. The self-hosted runner cached
CMake under `$env:TEMP` and re-downloaded it only when `bin\cmake.exe`
was missing; Windows Temp cleanup deleted `share\cmake-3.31\Modules` and
left `bin\`, so the job kept trusting a CMake that cannot resolve
`CMAKE_ROOT` and every run died the same way until someone cleared Temp
by hand. Now each candidate toolchain is probed for the thing the build
actually depends on, a failed probe falls through to a clean
re-download, and tools live in the runner tool cache instead of a
directory the OS sweeps.

One measurement drove the design and is worth flagging for review:
**exit codes cannot detect this failure.** A CMake missing its Modules
tree prints `Could not find CMAKE_ROOT` to stderr and still exits 0 —
for `--version` and for `--help-module-list` (measured on 4.4.2). So the
issue's suggested `cmake --version` exit-0 check would not have caught
it on its own; validity requires the Modules tree on disk *and* a probe
that does not report a broken root.

The stale `%TEMP%\cmake` tree on the runner is now inert — nothing reads
it — so no manual cleanup is needed to make this work; deleting it just
reclaims disk.

Closes amd#2817

## Test plan

- [ ] `pwsh -File .github/scripts/tests/CppBuildTools.Tests.ps1` passes
(21/21). It asserts the exact regression: `bin/cmake.exe` present +
`share/` absent reports **invalid**, both present reports **valid**, and
includes negative controls showing the old `Test-Path cmake.exe` check
and an exit-code-only check would both have accepted the broken install.
- [ ] New `C++ toolchain script tests` job is green (parse-checks every
`.github/scripts/*.ps1`, then runs the unit tests).
- [ ] `C++ Integration Tests (STX)` on this PR gets past `Ensure C++
build tools are available` and reaches compilation. The step log should
name which CMake it accepted and, if it rejected one, why.
- [ ] Reproduce the root cause on any machine: copy a `cmake` binary
alone into an empty directory and run `--version` — it prints the
`CMAKE_ROOT` error and exits 0.
- [ ] After merge, re-run CI on amd#2807, amd#2809 and amd#2816 with no changes
to their diffs and confirm the STX check goes green.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cpp documentation Documentation changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cpp): gaia::VectorIndex — flat vector index with persistence

1 participant