feat(manifest): add strict v1 compatibility foundation

[Fieldnote-Echo]

Superseded by #31. During the branch-history replacement, GitHub marked this PR merged because its former feature-branch base came to contain this corrected head. It did not merge into main; review and merge #31 instead.

What changed

• Add ordinaldb-manifest 0.2.0 as OrdinalDB's sole direct ordvec-manifest policy boundary.
• Require ordvec and ordvec-manifest 0.6.0 and pin both to merged ordvec SHA 51ee981c87e9e4dad414106f8946f0464c721907 in all five independently locked Cargo roots.
• Route the documented ordinaldb::manifest load and verification APIs through one compatibility facade.
• Read each manifest once with the upstream bounded reader, then strictly dispatch those exact bytes to current v2 parsing or a frozen v1 DTO.
• Preserve all shared typed fields during v1 conversion; validate and discard only the legacy top-level manifest_id and created_at.
• Keep v1 read-only. Explicit migration writes a separate v2 manifest using atomic no-replace publication and never rewrites the source.
• Add deterministic fresh-process bundle fixtures, one cross-platform framed digest map, dependency/source policy, and package-order coverage.

Integration and release boundary

• Upstream prerequisite: feat: add plan-verified streaming decode ordvec#303
• Current ordvec package version: unpublished 0.6.0
• The exact Git revision is an integration/CI pin, not the publishable release graph.
• Before an OrdinalDB release, both ordvec packages must exist in the target registry, every ordvec Git patch must be removed, and all five locks must be regenerated registry-only.

[qodo-code-review]

PR Summary by Qodo

Bump ordvec to 0.7 (schema v2) and add reproducible bundle test

⚙️ Configuration changes 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Bump ordvec/ordvec-manifest to 0.7.0 (schema v2) across workspace dependencies and lockfiles.
• Add a regression test asserting identical inputs produce byte-identical .odb bundles.
• Clarify Cargo patch comment for using ordvec main until crates.io publish.

Diagram

graph TD
  T["manifest.rs tests"] --> W["IdMapIndex::write"] --> O["ordvec 0.7 schema v2"] --> BA[("bundle A")]
  O --> BB[("bundle B")]
  BA --> R["read + sort files"] --> P["byte-identical asserts"]
  BB --> R

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Pin ordvec patch by git rev (instead of branch=main)

• ➕ Avoids accidental upgrades when regenerating lockfiles
• ➕ Makes workspace resolution fully deterministic even without Cargo.lock
• ➖ Requires manual bump to pick up upstream fixes
• ➖ Less convenient during active upstream development

2. Wait for ordvec crates.io publish and remove [patch.crates-io]

• ➕ Simplifies dependency management and auditability
• ➕ Eliminates git dependency risks (availability, integrity policies)
• ➖ Blocks adopting schema v2 determinism until publish happens
• ➖ May slow iteration if ordvec is still changing quickly

3. Rely on manifest-level determinism only (no bundle-level byte equality test)

• ➕ Less brittle if non-semantic file ordering/formatting changes occur
• ➕ Shorter test runtime
• ➖ Weaker guardrail: could miss non-manifest nondeterminism
• ➖ Doesn’t lock in the end-to-end reproducible bundle guarantee

Recommendation: The PR’s approach (upgrade to schema v2 plus an end-to-end byte-identical bundle test) is the strongest way to enforce reproducibility. Consider pinning the [patch.crates-io] entries by git rev instead of branch=main if you want determinism even when lockfiles are regenerated or absent; otherwise the current setup is acceptable because Cargo.lock already pins the exact commit.

Files changed (9) +96 / -21

Tests (1) +71 / -0

manifest.rsAdd deterministic bundle reproducibility regression test +71/-0

Add deterministic bundle reproducibility regression test

• Adds a test helper to read bundle directory trees into a sorted map and a new test that builds two bundles from identical inputs and asserts manifest.json and all bundle files are byte-identical.

ordinaldb/src/manifest.rs

Other (8) +25 / -21

Cargo.lockLock ordvec/ordvec-manifest to 0.7.0 git rev +4/-4

Lock ordvec/ordvec-manifest to 0.7.0 git rev

• Updates the workspace lockfile entries for ordvec and ordvec-manifest from 0.6.0 to 0.7.0, moving the git source revision to the schema v2 commit.

Cargo.lock

Cargo.tomlUpdate patch.crates-io comment for ordvec 0.7 usage +3/-3

Update patch.crates-io comment for ordvec 0.7 usage

• Rewords the workspace comment explaining why ordvec is patched from git (not yet on crates.io) and notes the current mainline version is 0.7.0.

Cargo.toml

Cargo.lockBump benchmark lockfile to ordvec 0.7.0 +6/-4

Bump benchmark lockfile to ordvec 0.7.0

• Updates the standalone benchmark lockfile to track ordvec/ordvec-manifest 0.7.0 at the new git revision and reflects dependency graph adjustments for ordinaldb in that context.

benchmarks/arxiv-precomputed/Cargo.lock

Cargo.lockBump downstream example lockfile to ordvec 0.7.0 +6/-4

Bump downstream example lockfile to ordvec 0.7.0

• Updates the standalone downstream-smoke lockfile to ordvec/ordvec-manifest 0.7.0 at the new git revision and reflects dependency graph adjustments for ordinaldb in that context.

examples/downstream-smoke/Cargo.lock

Cargo.tomlBump ordvec-manifest dependency to 0.7.0 +1/-1

Bump ordvec-manifest dependency to 0.7.0

• Raises the ordvec-manifest dependency requirement from 0.6.0 to 0.7.0 while preserving default-features=false.

ordinaldb-adapter-store/Cargo.toml

Cargo.tomlBump ordvec-manifest dependency to 0.7.0 +1/-1

Bump ordvec-manifest dependency to 0.7.0

• Updates the CLI crate’s ordvec-manifest dependency requirement to 0.7.0 with default features disabled.

ordinaldb-cli/Cargo.toml

Cargo.tomlBump hybrid crate ordvec dependencies to 0.7.0 +2/-2

Bump hybrid crate ordvec dependencies to 0.7.0

• Updates ordvec-manifest to 0.7.0 and aligns the dev-dependency on ordvec to 0.7.0 for tests/examples in this crate.

ordinaldb-hybrid/Cargo.toml

Cargo.tomlBump core ordinaldb ordvec dependencies to 0.7.0 +2/-2

Bump core ordinaldb ordvec dependencies to 0.7.0

• Updates ordinaldb’s direct dependencies on ordvec and ordvec-manifest to 0.7.0 (manifest still built without default features).

ordinaldb/Cargo.toml

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

1. Non-exclusive temp bundle dirs 🐞 Bug ☼ Reliability

Description

The new reproducibility test uses a predictable path under the shared OS temp directory and
unconditionally calls remove_dir_all on it; this can lead to rare test flakiness (stale paths / PID
reuse / external pre-creation) and makes cleanup unreliable when the test fails mid-assertion. Using
an exclusive TempDir-style API would make the test isolated and self-cleaning.

Code

ordinaldb/src/manifest.rs[R257-297]

+    fn identical_inputs_produce_byte_identical_bundle() {
+        let bundle_a = temp_bundle("repro-a");
+        let bundle_b = temp_bundle("repro-b");
+        let _ = std::fs::remove_dir_all(&bundle_a);
+        let _ = std::fs::remove_dir_all(&bundle_b);
+
+        // Two independent builds from the same inputs.
+        write_idmap_bundle(&bundle_a);
+        write_idmap_bundle(&bundle_b);
+
+        let files_a = read_bundle_files(&bundle_a);
+        let files_b = read_bundle_files(&bundle_b);
+
+        // The manifest must be byte-identical: schema v2 emits no per-build
+        // UUIDs or timestamps.
+        let manifest_a = files_a
+            .get(std::path::Path::new(crate::artifacts::MANIFEST_FILE))
+            .expect("bundle A has a manifest");
+        let manifest_b = files_b
+            .get(std::path::Path::new(crate::artifacts::MANIFEST_FILE))
+            .expect("bundle B has a manifest");
+        assert_eq!(
+            manifest_a, manifest_b,
+            "manifest.json must be byte-identical across builds from identical inputs"
+        );
+
+        // The whole bundle must be byte-identical: same set of files, each
+        // with identical bytes.
+        assert_eq!(
+            files_a.keys().collect::<Vec<_>>(),
+            files_b.keys().collect::<Vec<_>>(),
+            "both bundles must contain the same set of files"
+        );
+        assert_eq!(
+            files_a, files_b,
+            "the entire .odb bundle must be byte-identical across builds from identical inputs"
+        );
+
+        let _ = std::fs::remove_dir_all(&bundle_a);
+        let _ = std::fs::remove_dir_all(&bundle_b);
+    }

Relevance

⭐⭐⭐ High

Team previously accepted switching from predictable temp paths + remove_dir_all to unique
TempDir-style dirs to avoid collisions.

PR-#31

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The repository’s test helper constructs temp paths by formatting a name under
std::env::temp_dir(), and the newly added test uses it and deletes the resulting directories with
remove_dir_all before and after assertions. This demonstrates non-exclusive temp path creation and
non-RAII cleanup in the new test.

ordinaldb/src/manifest.rs[204-218]
ordinaldb/src/manifest.rs[256-297]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test `identical_inputs_produce_byte_identical_bundle` creates bundle directories by joining a formatted string onto `std::env::temp_dir()` and then recursively deletes those paths with `remove_dir_all`. This is not an exclusive/atomic temp directory creation pattern, so in unusual environments (stale leftovers, PID reuse, or external pre-creation) it can cause test flakiness and/or delete content the test didn’t create; additionally, cleanup won’t run if the test panics before the final `remove_dir_all` calls.

## Issue Context
This is test-only code, but it runs in CI and locally; failures or panics can leave behind temp directories and make later runs behave unexpectedly.

## Fix Focus Areas
- ordinaldb/src/manifest.rs[212-218]
- ordinaldb/src/manifest.rs[257-297]

### Suggested approach
- Replace `temp_bundle(...)` usage in this test with `tempfile::TempDir` (or similar) so directories are created uniquely and cleaned up via RAII even on panic.
- Optionally refactor `temp_bundle` (used by other tests) to return a `TempDir`/guard object instead of a plain `PathBuf`, or create a test-local helper for this specific test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Misleading patch removal comment 🐞 Bug ⚙ Maintainability ⭐ New

Description

The comment says to remove the [patch.crates-io] override once “ordvec” is published, but the patch
section also overrides ordvec-manifest; if the crates are not published to crates.io in lockstep,
this wording can lead to premature patch removal and build breakage. This is a documentation hazard
in a release-critical area (dependency source selection).

Code

Cargo.toml[R25-27]

+# ordvec is not yet published to crates.io, so the workspace builds against
+# ordvec main (currently 0.7.0 — deterministic manifest schema v2). Remove this
+# patch once ordvec is published, restoring crates.io version resolution.

Relevance

⭐⭐⭐ High

Team often accepts clarifying misleading/stale wording in release-critical docs/comments to prevent
incorrect assumptions.

PR-#35
PR-#13
PR-#36

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The comment instructs removing the patch once “ordvec” is published, but immediately below it the
[patch.crates-io] section applies to both ordvec and ordvec-manifest, so the removal condition
is underspecified.

Cargo.toml[25-30]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The comment above `[patch.crates-io]` refers only to `ordvec` being published, but the patch section overrides both `ordvec` and `ordvec-manifest`. This can mislead future maintainers into removing the patch when only one of the two crates is available on crates.io.

### Issue Context
The workspace uses git patches for both crates:
- `ordvec = { git = ..., branch = "main" }`
- `ordvec-manifest = { git = ..., branch = "main" }`

### Fix Focus Areas
- Cargo.toml[25-27]

### Suggested change
Update the comment to explicitly mention **both** `ordvec` and `ordvec-manifest` (and ideally the expectation that they are published together), e.g. “Remove these patches once both `ordvec` and `ordvec-manifest` are published to crates.io …”.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Previous review results

Review updated until commit 19ab391 ⚖️ Balanced

Results up to commit 38c3dab ⚖️ Balanced

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Non-exclusive temp bundle dirs 🐞 Bug ☼ Reliability

Description

The new reproducibility test uses a predictable path under the shared OS temp directory and
unconditionally calls remove_dir_all on it; this can lead to rare test flakiness (stale paths / PID
reuse / external pre-creation) and makes cleanup unreliable when the test fails mid-assertion. Using
an exclusive TempDir-style API would make the test isolated and self-cleaning.

Code

ordinaldb/src/manifest.rs[R257-297]

+    fn identical_inputs_produce_byte_identical_bundle() {
+        let bundle_a = temp_bundle("repro-a");
+        let bundle_b = temp_bundle("repro-b");
+        let _ = std::fs::remove_dir_all(&bundle_a);
+        let _ = std::fs::remove_dir_all(&bundle_b);
+
+        // Two independent builds from the same inputs.
+        write_idmap_bundle(&bundle_a);
+        write_idmap_bundle(&bundle_b);
+
+        let files_a = read_bundle_files(&bundle_a);
+        let files_b = read_bundle_files(&bundle_b);
+
+        // The manifest must be byte-identical: schema v2 emits no per-build
+        // UUIDs or timestamps.
+        let manifest_a = files_a
+            .get(std::path::Path::new(crate::artifacts::MANIFEST_FILE))
+            .expect("bundle A has a manifest");
+        let manifest_b = files_b
+            .get(std::path::Path::new(crate::artifacts::MANIFEST_FILE))
+            .expect("bundle B has a manifest");
+        assert_eq!(
+            manifest_a, manifest_b,
+            "manifest.json must be byte-identical across builds from identical inputs"
+        );
+
+        // The whole bundle must be byte-identical: same set of files, each
+        // with identical bytes.
+        assert_eq!(
+            files_a.keys().collect::<Vec<_>>(),
+            files_b.keys().collect::<Vec<_>>(),
+            "both bundles must contain the same set of files"
+        );
+        assert_eq!(
+            files_a, files_b,
+            "the entire .odb bundle must be byte-identical across builds from identical inputs"
+        );
+
+        let _ = std::fs::remove_dir_all(&bundle_a);
+        let _ = std::fs::remove_dir_all(&bundle_b);
+    }

Relevance

⭐⭐⭐ High

Team previously accepted switching from predictable temp paths + remove_dir_all to unique
TempDir-style dirs to avoid collisions.

PR-#31

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The repository’s test helper constructs temp paths by formatting a name under
std::env::temp_dir(), and the newly added test uses it and deletes the resulting directories with
remove_dir_all before and after assertions. This demonstrates non-exclusive temp path creation and
non-RAII cleanup in the new test.

ordinaldb/src/manifest.rs[204-218]
ordinaldb/src/manifest.rs[256-297]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test `identical_inputs_produce_byte_identical_bundle` creates bundle directories by joining a formatted string onto `std::env::temp_dir()` and then recursively deletes those paths with `remove_dir_all`. This is not an exclusive/atomic temp directory creation pattern, so in unusual environments (stale leftovers, PID reuse, or external pre-creation) it can cause test flakiness and/or delete content the test didn’t create; additionally, cleanup won’t run if the test panics before the final `remove_dir_all` calls.

## Issue Context
This is test-only code, but it runs in CI and locally; failures or panics can leave behind temp directories and make later runs behave unexpectedly.

## Fix Focus Areas
- ordinaldb/src/manifest.rs[212-218]
- ordinaldb/src/manifest.rs[257-297]

### Suggested approach
- Replace `temp_bundle(...)` usage in this test with `tempfile::TempDir` (or similar) so directories are created uniquely and cleaned up via RAII even on panic.
- Optionally refactor `temp_bundle` (used by other tests) to return a `TempDir`/guard object instead of a plain `PathBuf`, or create a test-local helper for this specific test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

1. Non-exclusive temp bundle dirs 🐞 Bug ☼ Reliability

The new reproducibility test uses a predictable path under the shared OS temp directory and
unconditionally calls remove_dir_all on it; this can lead to rare test flakiness (stale paths / PID
reuse / external pre-creation) and makes cleanup unreliable when the test fails mid-assertion. Using
an exclusive TempDir-style API would make the test isolated and self-cleaning.

Agent Prompt

## Issue description
The test `identical_inputs_produce_byte_identical_bundle` creates bundle directories by joining a formatted string onto `std::env::temp_dir()` and then recursively deletes those paths with `remove_dir_all`. This is not an exclusive/atomic temp directory creation pattern, so in unusual environments (stale leftovers, PID reuse, or external pre-creation) it can cause test flakiness and/or delete content the test didn’t create; additionally, cleanup won’t run if the test panics before the final `remove_dir_all` calls.

## Issue Context
This is test-only code, but it runs in CI and locally; failures or panics can leave behind temp directories and make later runs behave unexpectedly.

## Fix Focus Areas
- ordinaldb/src/manifest.rs[212-218]
- ordinaldb/src/manifest.rs[257-297]

### Suggested approach
- Replace `temp_bundle(...)` usage in this test with `tempfile::TempDir` (or similar) so directories are created uniquely and cleaned up via RAII even on panic.
- Optionally refactor `temp_bundle` (used by other tests) to return a `TempDir`/guard object instead of a plain `PathBuf`, or create a test-local helper for this specific test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 38c3dab490

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

[chatgpt-codex-connector]

Preserve schema-v1 bundle loading

Keep a compatibility path for schema-v1 manifests before switching the verifier to ordvec-manifest 0.7: existing bundles now reach the v2-only, unknown-field-denying parser through OrdinalIndex::load, so manifests containing schema_version: "ordvec.index_manifest.v1", manifest_id, created_at, and build.invocation_id are rejected. This breaks the explicitly supported migration exercised by test_committed_previous_layout_fixture_loads, whose committed fixture still has exactly that v1 manifest; users likewise cannot reopen or migrate bundles written by the previous release.

Useful? React with 👍 / 👎.

[qodo-code-review]

1. Misleading patch removal comment 🐞 Bug ⚙ Maintainability

The comment says to remove the [patch.crates-io] override once “ordvec” is published, but the patch
section also overrides ordvec-manifest; if the crates are not published to crates.io in lockstep,
this wording can lead to premature patch removal and build breakage. This is a documentation hazard
in a release-critical area (dependency source selection).

Agent Prompt

### Issue description
The comment above `[patch.crates-io]` refers only to `ordvec` being published, but the patch section overrides both `ordvec` and `ordvec-manifest`. This can mislead future maintainers into removing the patch when only one of the two crates is available on crates.io.

### Issue Context
The workspace uses git patches for both crates:
- `ordvec = { git = ..., branch = "main" }`
- `ordvec-manifest = { git = ..., branch = "main" }`

### Fix Focus Areas
- Cargo.toml[25-27]

### Suggested change
Update the comment to explicitly mention **both** `ordvec` and `ordvec-manifest` (and ideally the expectation that they are published together), e.g. “Remove these patches once both `ordvec` and `ordvec-manifest` are published to crates.io …”.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-code-review]

Code review by qodo was updated up to the latest commit 19ab391