Port Go search snapshot tests into C++ - #129
Merged
Merged
Conversation
Lift before/after search gates out of rag-service so ctest can assert stable snapshot, partial interleave, retry-fresh, rebuild bound, and hybrid identity without Mongo.
There was a problem hiding this comment.
🟡 Changes recommended
The new snapshot gate currently omits response field consistency checks (e.g., generation/projection_version parity with Go), weakening the intended fail-closed invariant being ported.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Ports the Go searchConsistently snapshot-gating behavior into the C++ cpp_memory_store so rag-service can fail closed when corpus/health changes mid-search, and so the gate can be asserted offline via a self-test.
Changes:
- Adds
SearchHealthSnap+search_before_gate/search_after_gate/same_search_snapshothelpers and an offline snapshot self-test. - Refactors
rag-servicesearch to reuse the shared snapshot gate helpers (including bounded retry attempts). - Updates the C++ memory store self-test runner and README “cut” notes to include the new snapshot self-test.
File summaries
| File | Description |
|---|---|
| godbrain_core/cpp_memory_store/tests/tests.cpp | Runs the new snapshot self-test alongside the existing protocol self-test. |
| godbrain_core/cpp_memory_store/src/rag_snapshot.cpp | Implements snapshot gating helpers and the offline snapshot self-test scenarios. |
| godbrain_core/cpp_memory_store/src/rag_engine.cpp | Switches /v1/search gating to shared snapshot helpers and uses the shared retry bound. |
| godbrain_core/cpp_memory_store/include/godbrain/memory_store/snapshot.hpp | Declares the snapshot gate API and shared retry constant. |
| godbrain_core/cpp_memory_store/CMakeLists.txt | Adds the new snapshot implementation file to the core library build. |
| godbrain_core/cpp_memory_store/README.md | Documents “Cut 13” and the new offline snapshot self-test coverage. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+123
to
+136
| return before.ready && after.ready && before.mongo == "ok" && after.mongo == "ok" && | ||
| !before.active_generation.empty() && before.active_generation == after.active_generation && | ||
| before.building_generation == after.building_generation && !before.projection_version.empty() && | ||
| before.projection_version == after.projection_version && | ||
| before.projection_schema == after.projection_schema && before.indexer_version == after.indexer_version && | ||
| before.retrieval_mode == after.retrieval_mode && same_semantic(before, after) && same_counts(before, after) && | ||
| same_optional_millis( | ||
| before.latest_committed, before.committed_at, after.latest_committed, after.committed_at) && | ||
| same_optional_millis( | ||
| before.latest_projected, before.projected_at, after.latest_projected, after.projected_at) && | ||
| same_optional_millis( | ||
| before.latest_embedded, before.embedded_at, after.latest_embedded, after.embedded_at) && | ||
| valid_response_capability(before, retrieval, hybrid, degradation, response_embedding); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Port the Go
http_test.gosearchConsistentlycases into C++ so the fail-closed snapshot gate is asserted offline (the leftover from #128).What
SearchHealthSnap+search_before_gate/search_after_gate/same_search_snapshotincpp_memory_storecore (no Mongo).rag-servicesearch uses those gates. Hybrid responses bind embedding identity the same way GovalidResponseCapabilitydoes.run_snapshot_self_testcovers: stable snapshot, partial interleave (no result leak, no second search), retry-fresh, rebuild bound (2 attempts), unready first look, hybrid semantic-unavailable, hybrid identity match/mismatch.Go
memory_store/stays rollback.Verify
ctest --test-dir build/cpp_memory_store -C Release: protocol (now includes snapshot) + rag-eval self/offline pass:8084after bounce: health ready, search 8 hitscitation_status=available