Skip to content

feat(manifest): add v1 compatibility facade and pin ordvec 0.6.0 - #31

Open
Nelson Spence (Fieldnote-Echo) wants to merge 2 commits into
mainfrom
codex/manifest-compat-foundation
Open

feat(manifest): add v1 compatibility facade and pin ordvec 0.6.0#31
Nelson Spence (Fieldnote-Echo) wants to merge 2 commits into
mainfrom
codex/manifest-compat-foundation

Conversation

@Fieldnote-Echo

@Fieldnote-Echo Fieldnote-Echo commented Jul 25, 2026

Copy link
Copy Markdown
Member

Summary

This PR adds ordinaldb-manifest as the compatibility layer used by OrdinalDB's public manifest APIs and updates the repository to the unpublished ordvec 0.6.0 code.

It:

  • pins ordvec and ordvec-manifest 0.6.0 to exact ordvec commit 51ee981c87e9e4dad414106f8946f0464c721907 in all five independently locked Cargo roots;
  • dispatches one bounded manifest read to current v2 parsing or a frozen v1 representation;
  • keeps v1 loading read-only and provides explicit migration to a separate v2 destination;
  • routes core, hybrid, LTR, CLI, and adapter-store manifest use through the facade;
  • adds compatibility, migration, deterministic-bundle, dependency-policy, and package-smoke coverage.

Actual diff

  • Base: main at a8cd6620892cb7b8c8045f6e5d4d0d24f203c46d
  • Head: 1bd4c19d3eb56d2291a5002256da9e56d4082ef8
  • 2 commits
  • 57 files changed
  • 5,842 additions / 1,691 deletions

The size includes the new facade crate and tests, changes across manifest consumers, dependency-policy/package-smoke tests, and regenerated lockfiles for the five Cargo roots.

Compatibility behavior

  • V2 continues through the upstream current-schema parser.
  • V1 conversion preserves shared typed fields, validates the legacy top-level manifest_id and created_at, and does not represent those two legacy-only fields in v2.
  • Migration never rewrites the source and refuses an existing or aliased destination.

Verified at this head

GitHub reports 27 passing checks and no failures, including Rust/Python platform jobs, dependency policy, package smoke, actionlint, and zizmor.

Limitations and release boundary

  • Ordvec 0.6.0 is still unpublished; the exact Git revision is an integration pin, not the final release graph.
  • Before an OrdinalDB release, publish both ordvec packages, remove every ordvec Git patch, regenerate all five locks registry-only, and rerun package smoke.
  • Loader, sparse-mapping, and load-memory changes are not in this PR; they are in feat(load): stream verified artifacts and enforce load limits #26.

Copy link
Copy Markdown
Member Author

Codex (@codex) review

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

feat(manifest): add ordinaldb-manifest strict v1/v2 compatibility facade

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Introduce new ordinaldb-manifest 0.2.0 crate as OrdinalDB's sole direct policy boundary over
 ordvec-manifest.
• Read manifests once with the upstream bounded reader, strictly dispatch to v2 parsing or a frozen,
 read-only v1 DTO, and validate/discard only legacy manifest_id/created_at.
• Add explicit, atomic no-replace migrate_v1_manifest_file (Rust API and new CLI
 migrate-manifest command) that never rewrites the v1 source.
• Route every OrdinalDB crate (ordinaldb, ordinaldb-cli, ordinaldb-hybrid, ordinaldb-ltr,
 ordinaldb-adapter-store) through the new facade instead of ordvec_manifest directly.
• Pin ordvec/ordvec-manifest 0.6.0 to one merged SHA across all five independently locked Cargo
 roots and add tests/dependency_policy.py to enforce the pin and dependency-boundary policy in CI.
• Add deterministic fresh-process bundle fixtures, v1/v2 compatibility tests, migration rollback
 tests, and packaged-crate smoke coverage.
Diagram

graph TD
  A["ordinaldb / ordinaldb-cli / ordinaldb-hybrid / ordinaldb-ltr / ordinaldb-adapter-store"] --> B["ordinaldb-manifest facade"]
  B --> C{"schema_version dispatch"}
  C -->|"v2"| D["ordvec-manifest parser"]
  C -->|"legacy v1"| E["Frozen v1 DTO converter"]
  E --> D
  B --> F["migrate_v1_manifest_file atomic no-replace"]
  F --> G[("manifest.json v2")]
  D --> H["ordvec-manifest verifier"]
  subgraph Legend
    direction LR
    _svc(["Service / Crate"]) ~~~ _dec{"Decision"} ~~~ _db[("File artifact")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Push v1 compatibility upstream into ordvec-manifest
  • ➕ Single implementation, no duplicate DTO
  • ➕ No extra crate/publish step
  • ➖ Upstream explicitly wants ordvec-manifest v2-only
  • ➖ Couples OrdinalDB's legacy support lifecycle to ordvec's release cadence
2. Inline v1 fallback parsing in each dependent crate
  • ➕ No new workspace member/crate to publish
  • ➖ Duplicates frozen v1 DTO and dispatch logic across ordinaldb, ordinaldb-cli, ordinaldb-hybrid, etc.
  • ➖ No single choke point for the dependency-policy audit script to enforce
3. Silent in-place v1->v2 rewrite on load
  • ➕ Simpler API: callers never see v1 explicitly
  • ➖ Violates PR's explicit non-goal of mutating legacy files implicitly
  • ➖ Risky for read-only/backup workflows and concurrent access

Recommendation: Centralizing v1/v2 compatibility in a single new crate (ordinaldb-manifest) rather than scattering schema-dispatch logic across each dependent crate is the right call: it gives one enforceable policy boundary (verified by tests/dependency_policy.py), keeps ordvec-manifest v2-only as upstream intends, and makes the read-only/migrate-only-explicit invariant auditable in one place. The atomic no-replace migration (rename/renameat2/renamex_np/MoveFileExW) is the correct low-level primitive for 'never silently overwrite' semantics versus a check-then-write approach that would race.

Files changed (47) +4289 / -1562

Enhancement (3) +846 / -15
lib.rsNew ordinaldb-manifest compatibility facade crate +569/-0

New ordinaldb-manifest compatibility facade crate

• Adds the new crate that re-exports ordvec-manifest's v2 API, adds a frozen v1 DTO and strict schema-dispatch parser, read-only v1 loaders, and a migrate_v1_manifest_file function with per-platform (Linux/Apple/Windows) atomic no-replace rename implementations.

ordinaldb-manifest/src/lib.rs

main.rsAdd migrate-manifest CLI command and switch to ordinaldb-manifest +107/-3

Add migrate-manifest CLI command and switch to ordinaldb-manifest

• Introduces a new MigrateManifest CLI subcommand wired to migrate_v1_manifest_file, replaces ordvec_manifest imports with ordinaldb_manifest, and adds v1-manifest test coverage for verify/inspect routes.

ordinaldb-cli/src/main.rs

lib.rsEnforce current-only manifest mutation and add v1 compatibility tests +170/-12

Enforce current-only manifest mutation and add v1 compatibility tests

• Adds load_current_bundle_manifest_for_mutation to reject v1 manifests before feature-cache mutation, refactors attach_feature_cache_auxiliaries to use it, and adds tests rewriting bundles as v1 to confirm read-only enforcement.

ordinaldb-ltr/src/lib.rs

Refactor (8) +39 / -28
manifest.rsRe-route ordinaldb::manifest through ordinaldb-manifest facade +17/-13

Re-route ordinaldb::manifest through ordinaldb-manifest facade

• Replaces direct ordvec_manifest re-exports with ordinaldb_manifest ones, adding migration, parsing, and current-only load functions to the public facade.

ordinaldb/src/manifest.rs

error.rsUpdate error type references to ordinaldb_manifest +6/-6

Update error type references to ordinaldb_manifest

• Renames ordvec_manifest references to ordinaldb_manifest in error type definitions.

ordinaldb-hybrid/src/error.rs

lib.rsUpdate manifest crate references in hybrid lib +3/-3

Update manifest crate references in hybrid lib

• Swaps ordvec_manifest imports/usages for ordinaldb_manifest across the hybrid crate.

ordinaldb-hybrid/src/lib.rs

ltr_manifest.rsUpdate LTR manifest module to use ordinaldb_manifest +1/-1

Update LTR manifest module to use ordinaldb_manifest

• Updates import to route through the new facade crate.

ordinaldb-hybrid/src/ltr_manifest.rs

sparse.rsUpdate sparse module manifest references +3/-3

Update sparse module manifest references

• Switches sparse index manifest handling to use ordinaldb_manifest types.

ordinaldb-hybrid/src/sparse.rs

lib.rsUpdate adapter-store manifest crate reference +1/-1

Update adapter-store manifest crate reference

• Swaps ordvec_manifest import for ordinaldb_manifest in the adapter store crate.

ordinaldb-adapter-store/src/lib.rs

lib.rsMinor update to paste-shim shim implementation +7/-0

Minor update to paste-shim shim implementation

• Small implementation adjustment in the paste-shim compatibility shim.

cookbook/supportsearch/paste-shim/src/lib.rs

embedder.rsMinor embedder adjustment for dependency alignment +1/-1

Minor embedder adjustment for dependency alignment

• Small one-line change in the embedder module related to the dependency update.

cookbook/supportsearch/src/embedder.rs

Tests (13) +1732 / -11
compatibility.rsCompatibility test suite for v1/v2 manifest parsing and migration +262/-0

Compatibility test suite for v1/v2 manifest parsing and migration

• Adds tests covering v1-to-v2 field preservation, current-only load rejection of v1, legacy field validation, unknown-field rejection, manifest size limits, symlink rejection, and migration atomicity/aliasing behavior.

ordinaldb-manifest/tests/compatibility.rs

full-v1.jsonAdd full v1 manifest fixture +166/-0

Add full v1 manifest fixture

• New binary-protected JSON fixture representing a complete legacy v1 manifest used by compatibility tests.

ordinaldb-manifest/tests/fixtures/full-v1.json

manifest_facade.rsNew external facade compilation and behavior tests +265/-0

New external facade compilation and behavior tests

• Adds a new test file verifying every documented ordinaldb::manifest type/function compiles and behaves correctly for both v1 and v2 bundles across public loaders.

ordinaldb/tests/manifest_facade.rs

deterministic_bundles.rsFresh-process deterministic bundle generation tests +369/-0

Fresh-process deterministic bundle generation tests

• Adds cross-platform deterministic bundle generation tests that spawn independent child processes and compare produced bundle tree digests against a committed golden map.

ordinaldb/tests/deterministic_bundles.rs

bundle-tree-digests.sha256Add golden bundle-tree digest map fixture +5/-0

Add golden bundle-tree digest map fixture

• New cross-platform golden digest map used to validate deterministic bundle generation.

ordinaldb/tests/fixtures/determinism/bundle-tree-digests.sha256

legacy-v1-manifest.jsonAdd packaged legacy v1 manifest fixture +31/-0

Add packaged legacy v1 manifest fixture

• New binary-protected v1 manifest fixture used by ordinaldb crate tests.

ordinaldb/tests/fixtures/legacy-v1-manifest.json

boundary_api.rsExtend boundary API tests with v1 compatibility coverage +227/-10

Extend boundary API tests with v1 compatibility coverage

• Adds tests exercising hybrid/sparse and LTR sidecar public routes against real v1-rewritten manifests, verifying preserved typed build info.

ordinaldb/tests/boundary_api.rs

manifest_compat.rsNew CLI end-to-end v1/v2 verify and migrate test +93/-0

New CLI end-to-end v1/v2 verify and migrate test

• Adds an integration test invoking the CLI binary to verify, inspect, and migrate a v1 bundle without overwriting the source, and checks re-migration fails safely.

ordinaldb-cli/tests/manifest_compat.rs

legacy-v1-manifest.jsonAdd packaged legacy v1 manifest fixture for CLI tests +31/-0

Add packaged legacy v1 manifest fixture for CLI tests

• New binary-protected v1 manifest fixture used by ordinaldb-cli tests.

ordinaldb-cli/tests/fixtures/legacy-v1-manifest.json

redb_store.rsAdd v1/v2 active-generation manifest reopen tests +110/-1

Add v1/v2 active-generation manifest reopen tests

• Adds a test rewriting active generation manifests as v1 and confirms redb store reopen/verify routes handle both schemas while preserving typed build info.

ordinaldb-adapter-store/tests/redb_store.rs

test_bindings.pyAdd Python binding tests for legacy v1 manifest loads +24/-0

Add Python binding tests for legacy v1 manifest loads

• Adds a helper to rewrite manifests as v1 and new assertions that OrdinalIndex/IdMapIndex load and search correctly against rewritten v1 bundles.

ordinaldb-python/tests/test_bindings.py

test_adapters_common.pyAdd adapter-store v1/v2 read-only load test +55/-0

Add adapter-store v1/v2 read-only load test

• Adds a test verifying AdapterStore loads and verifies both v1 and v2 active-generation manifests without mutating the v1 source.

ordinaldb-python/tests/test_adapters_common.py

test_adapters_frameworks.pyExtend framework adapter tests for v1 compatibility +94/-0

Extend framework adapter tests for v1 compatibility

• Adds framework-adapter test coverage exercising v1 manifest compatibility paths.

ordinaldb-python/tests/test_adapters_frameworks.py

Documentation (2) +33 / -11
RELEASING.mdDocument five-root pin removal and manifest crate publish order +29/-8

Document five-root pin removal and manifest crate publish order

• Updates release checklist to describe the integration-only exact-revision patch, the ephemeral-registry smoke procedure, and publishing ordinaldb-manifest before ordinaldb-hybrid.

RELEASING.md

persistence.mdDocument the ordinaldb-manifest policy boundary +4/-3

Document the ordinaldb-manifest policy boundary

• Updates persistence docs to describe ordinaldb-manifest as the sole direct dependency on ordvec-manifest and its role as the policy facade.

docs/persistence.md

Other (21) +1639 / -1497
Cargo.tomlNew crate manifest for ordinaldb-manifest 0.2.0 +30/-0

New crate manifest for ordinaldb-manifest 0.2.0

• Declares the new policy crate, its ordvec-manifest 0.6.0 dependency, platform-specific libc/windows-sys deps, and dev-dependencies for tests.

ordinaldb-manifest/Cargo.toml

Cargo.tomlSwap ordvec-manifest dependency for ordinaldb-manifest +11/-1

Swap ordvec-manifest dependency for ordinaldb-manifest

• Replaces the direct ordvec-manifest dependency with the new ordinaldb-manifest path dependency and adds test-only dependencies (serde_json, sha2, tempfile); excludes a large fixture test from packaging.

ordinaldb/Cargo.toml

Cargo.tomlDepend on ordinaldb-manifest instead of ordvec-manifest +1/-1

Depend on ordinaldb-manifest instead of ordvec-manifest

• Updates the hybrid crate's dependency to route through the new compatibility facade crate.

ordinaldb-hybrid/Cargo.toml

Cargo.tomlDepend on ordinaldb-manifest instead of ordvec-manifest +1/-1

Depend on ordinaldb-manifest instead of ordvec-manifest

• Updates the adapter-store crate's dependency to the new facade crate.

ordinaldb-adapter-store/Cargo.toml

Cargo.tomlAdd ordinaldb-manifest workspace member and pin ordvec to exact SHA +6/-5

Add ordinaldb-manifest workspace member and pin ordvec to exact SHA

• Registers the new crate as a workspace member, adds cookbook/supportsearch to excludes, and changes the ordvec git patch from branch main to an exact pinned revision.

Cargo.toml

Cargo.lockRegenerate root lockfile for pinned ordvec revision and new crate +131/-79

Regenerate root lockfile for pinned ordvec revision and new crate

• Updates the root lockfile to reflect the new ordinaldb-manifest crate and the exact-SHA ordvec/ordvec-manifest pin.

Cargo.lock

deny.tomlExpand license allowlist and git allow-list documentation +23/-8

Expand license allowlist and git allow-list documentation

• Adds several permissive SPDX licenses to the allow list and clarifies allow-git policy across the five locked Cargo roots.

deny.toml

.gitattributesProtect new binary fixture files from text normalization +6/-1

Protect new binary fixture files from text normalization

• Marks new v1 manifest and digest-map fixtures as -text to prevent line-ending mutation of golden byte-for-byte fixtures.

.gitattributes

ci.ymlAdd dependency-policy check, per-surface audits, and package-integration job +55/-1

Add dependency-policy check, per-surface audits, and package-integration job

• Adds a dependency_policy.py CI step, splits cargo-deny into per-Cargo-root audit steps, and adds a new package-integration job running the packaged-crate smoke against an exact-revision ephemeral registry.

.github/workflows/ci.yml

audit.ymlSplit cargo-deny audit across all five locked Cargo roots +32/-1

Split cargo-deny audit across all five locked Cargo roots

• Adds explicit named cargo-deny steps for the downstream-smoke, arxiv, beir, and supportsearch surfaces alongside the root workspace.

.github/workflows/audit.yml

release_crate_package_smoke.shAdd integration-mode git-source vendoring and packaged-crate testing +144/-11

Add integration-mode git-source vendoring and packaged-crate testing

• Extends the release smoke script with an INTEGRATION_GIT_PACKAGE_SMOKE mode that vendors the exact-revision ordvec packages into an ephemeral registry with checksum patching, adds packaged-crate test execution, and includes ordinaldb-manifest in the packaging/publish order.

scripts/release_crate_package_smoke.sh

dependency_policy.pyNew fail-closed dependency and manifest policy checker +241/-0

New fail-closed dependency and manifest policy checker

• Adds a new Python script validating that all five Cargo roots resolve ordvec/ordvec-manifest to one exact pinned revision, that no local crate directly depends on ordvec-manifest except the facade, that serde_json features are policy-compliant, and that binary golden fixtures stay byte-identical and protected.

tests/dependency_policy.py

Cargo.tomlUpdate arXiv benchmark dependency pin and metadata +7/-10

Update arXiv benchmark dependency pin and metadata

• Adjusts the arxiv-precomputed benchmark Cargo.toml to align with the ordvec exact-revision pin and workspace policy.

benchmarks/arxiv-precomputed/Cargo.toml

Cargo.lockRegenerate arXiv benchmark lockfile +155/-54

Regenerate arXiv benchmark lockfile

• Updates the lockfile to the pinned ordvec SHA and dependency graph changes.

benchmarks/arxiv-precomputed/Cargo.lock

Cargo.tomlAdd license metadata and pin ordvec to exact SHA for BEIR benchmark +9/-1

Add license metadata and pin ordvec to exact SHA for BEIR benchmark

• Adds license/repository metadata, versions the local ordinaldb path dependency, and adds a patch.crates-io block pinning ordvec/ordvec-manifest to the shared SHA.

benchmarks/beir-rust/Cargo.toml

Cargo.lockRegenerate BEIR benchmark lockfile +185/-366

Regenerate BEIR benchmark lockfile

• Updates the lockfile for the exact-revision ordvec pin and dependency changes.

benchmarks/beir-rust/Cargo.lock

Cargo.tomlAdd SupportSearch cookbook to policy-pinned Cargo roots +20/-2

Add SupportSearch cookbook to policy-pinned Cargo roots

• Updates the SupportSearch cookbook manifest to align with the shared ordvec pin and dependency policy.

cookbook/supportsearch/Cargo.toml

Cargo.lockRegenerate SupportSearch lockfile +395/-892

Regenerate SupportSearch lockfile

• Regenerates the SupportSearch lockfile against the pinned ordvec revision.

cookbook/supportsearch/Cargo.lock

Cargo.tomlUpdate paste-shim crate metadata +11/-0

Update paste-shim crate metadata

• Minor metadata/version adjustments to the internal paste-shim helper crate.

cookbook/supportsearch/paste-shim/Cargo.toml

Cargo.tomlPin downstream-smoke example to exact ordvec SHA +7/-4

Pin downstream-smoke example to exact ordvec SHA

• Updates the downstream-smoke example manifest to the shared exact-revision ordvec patch.

examples/downstream-smoke/Cargo.toml

Cargo.lockRegenerate downstream-smoke lockfile +169/-59

Regenerate downstream-smoke lockfile

• Regenerates the lockfile against the pinned ordvec revision.

examples/downstream-smoke/Cargo.lock

@qodo-code-review

qodo-code-review Bot commented Jul 25, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Epoch-skew temp dir failure 🐞 Bug ☼ Reliability ⭐ New
Description
create_demo_root() propagates SystemTime::now().duration_since(UNIX_EPOCH) errors, so the
example can fail immediately on machines where the clock is set before 1970 (or otherwise skewed).
This breaks the demo even though a safe fallback would still produce a unique temp directory name.
Code

ordinaldb/examples/rust_persist_demo.rs[40]

+    let now = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos();
Evidence
The demo currently bubbles up a duration_since error at temp-dir creation time; other in-repo
temp-path helpers explicitly avoid failing in this case by defaulting the duration to 0.

ordinaldb/examples/rust_persist_demo.rs[39-41]
ordinaldb-hybrid/src/sparse.rs[1321-1325]

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

### Issue description
`create_demo_root()` uses `SystemTime::now().duration_since(UNIX_EPOCH)?`, which returns an error if the system clock is before the Unix epoch. Because the error is propagated with `?`, the example aborts before any persistence verification.

### Issue Context
This is a runnable example; it should be resilient to non-critical environment quirks. Elsewhere in the repo, similar temp-path stamp logic avoids failing on `duration_since` by using a fallback.

### Fix Focus Areas
- ordinaldb/examples/rust_persist_demo.rs[39-41]

### Suggested change
Replace the `?` propagation with a fallback, e.g.:
- `let now = SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default().as_nanos();`

Optionally, combine with an incrementing counter (or reuse the existing `attempt` loop) to ensure uniqueness even if `now` is 0.

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


2. Predictable temp dir deletion ✓ Resolved 🐞 Bug ⛨ Security
Description
The example constructs a deterministic bundle path in the system temp directory using only the
process id and then unconditionally calls remove_dir_all on it, which can delete unrelated
pre-existing directories that happen to match the name. This is avoidable data loss behavior in a
runnable demo and can also make the example fail in confusing ways if removal is blocked
(permissions/locks) because the error is ignored.
Code

ordinaldb/examples/rust_persist_demo.rs[R11-20]

+    let demo_path = std::env::temp_dir().join(format!(
+        "ordinaldb_rust_persist_demo_{}.odb",
+        std::process::id()
+    ));
+    let demo_ids_path = std::env::temp_dir().join(format!(
+        "ordinaldb_rust_persist_demo_ids_{}.odb",
+        std::process::id()
+    ));
+    let _ = std::fs::remove_dir_all(&demo_path);
+    let _ = std::fs::remove_dir_all(&demo_ids_path);
Evidence
The demo builds temp paths from only PID and deletes them unconditionally, so any pre-existing
directory at that exact path will be removed regardless of who created it.

ordinaldb/examples/rust_persist_demo.rs[11-21]

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 persistence demo chooses a predictable path under `std::env::temp_dir()` (based on PID) and unconditionally deletes it via `remove_dir_all`. If that directory already exists for any reason, the demo will delete it.

## Issue Context
This is example code, but it is directly runnable (`cargo run ... --example rust_persist_demo`) and should avoid destructive behavior in shared temp directories.

## Fix Focus Areas
- ordinaldb/examples/rust_persist_demo.rs[11-21]

## Implementation notes
- Prefer creating a unique per-run temporary directory (e.g., `tempfile::TempDir`) and place the two `.odb` bundles inside it.
- If you want to avoid adding a dependency, incorporate a high-entropy component (e.g., `SystemTime::now().as_nanos()` plus PID) and use `create_dir`/retry to ensure the directory does not already exist, then remove only that per-run directory at the end.

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



Informational

3. Cleanup failure aborts demo 🐞 Bug ☼ Reliability
Description
rust_persist_demo propagates errors from std::fs::remove_dir_all(&demo_root)?, so a harmless
teardown failure can make the example exit with an error after persistence has already been
verified. This is inconsistent with other repo examples/tests that treat temp cleanup as best-effort
to avoid noisy failures.
Code

ordinaldb/examples/rust_persist_demo.rs[35]

+    std::fs::remove_dir_all(&demo_root)?;
Evidence
The demo currently propagates cleanup errors via ?, while other repository examples/tests
intentionally ignore temp cleanup failures using let _ = remove_dir_all(...) (or equivalent in
Drop). This demonstrates the current behavior is an outlier and can create avoidable example
failures.

ordinaldb/examples/rust_persist_demo.rs[33-36]
ordinaldb/examples/rust_basic.rs[17-38]
ordinaldb-cli/src/main.rs[870-875]

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 example currently uses `?` on `std::fs::remove_dir_all(&demo_root)` which causes the whole demo to fail if cleanup fails (e.g., transient file locks, permissions).

### Issue Context
This is a runnable example; users expect it to succeed once verification steps complete. Other examples/tests in this repo ignore cleanup errors to avoid spurious failures.

### Fix Focus Areas
- ordinaldb/examples/rust_persist_demo.rs[33-36]

### Suggested change
Replace `std::fs::remove_dir_all(&demo_root)?;` with one of:
- `let _ = std::fs::remove_dir_all(&demo_root);`
- or log and continue:
 ```rust
 if let Err(e) = std::fs::remove_dir_all(&demo_root) {
     eprintln!("warning: failed to clean up {demo_root:?}: {e}");
 }
 ```

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


Grey Divider

Previous review results

Review updated until commit 1bd4c19

Results up to commit c76e99a


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


Remediation recommended
1. Predictable temp dir deletion ✓ Resolved 🐞 Bug ⛨ Security
Description
The example constructs a deterministic bundle path in the system temp directory using only the
process id and then unconditionally calls remove_dir_all on it, which can delete unrelated
pre-existing directories that happen to match the name. This is avoidable data loss behavior in a
runnable demo and can also make the example fail in confusing ways if removal is blocked
(permissions/locks) because the error is ignored.
Code

ordinaldb/examples/rust_persist_demo.rs[R11-20]

+    let demo_path = std::env::temp_dir().join(format!(
+        "ordinaldb_rust_persist_demo_{}.odb",
+        std::process::id()
+    ));
+    let demo_ids_path = std::env::temp_dir().join(format!(
+        "ordinaldb_rust_persist_demo_ids_{}.odb",
+        std::process::id()
+    ));
+    let _ = std::fs::remove_dir_all(&demo_path);
+    let _ = std::fs::remove_dir_all(&demo_ids_path);
Evidence
The demo builds temp paths from only PID and deletes them unconditionally, so any pre-existing
directory at that exact path will be removed regardless of who created it.

ordinaldb/examples/rust_persist_demo.rs[11-21]

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 persistence demo chooses a predictable path under `std::env::temp_dir()` (based on PID) and unconditionally deletes it via `remove_dir_all`. If that directory already exists for any reason, the demo will delete it.

## Issue Context
This is example code, but it is directly runnable (`cargo run ... --example rust_persist_demo`) and should avoid destructive behavior in shared temp directories.

## Fix Focus Areas
- ordinaldb/examples/rust_persist_demo.rs[11-21]

## Implementation notes
- Prefer creating a unique per-run temporary directory (e.g., `tempfile::TempDir`) and place the two `.odb` bundles inside it.
- If you want to avoid adding a dependency, incorporate a high-entropy component (e.g., `SystemTime::now().as_nanos()` plus PID) and use `create_dir`/retry to ensure the directory does not already exist, then remove only that per-run directory at the end.

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


Results up to commit 5abd0ca


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


Informational
1. Cleanup failure aborts demo 🐞 Bug ☼ Reliability
Description
rust_persist_demo propagates errors from std::fs::remove_dir_all(&demo_root)?, so a harmless
teardown failure can make the example exit with an error after persistence has already been
verified. This is inconsistent with other repo examples/tests that treat temp cleanup as best-effort
to avoid noisy failures.
Code

ordinaldb/examples/rust_persist_demo.rs[35]

+    std::fs::remove_dir_all(&demo_root)?;
Evidence
The demo currently propagates cleanup errors via ?, while other repository examples/tests
intentionally ignore temp cleanup failures using let _ = remove_dir_all(...) (or equivalent in
Drop). This demonstrates the current behavior is an outlier and can create avoidable example
failures.

ordinaldb/examples/rust_persist_demo.rs[33-36]
ordinaldb/examples/rust_basic.rs[17-38]
ordinaldb-cli/src/main.rs[870-875]

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 example currently uses `?` on `std::fs::remove_dir_all(&demo_root)` which causes the whole demo to fail if cleanup fails (e.g., transient file locks, permissions).

### Issue Context
This is a runnable example; users expect it to succeed once verification steps complete. Other examples/tests in this repo ignore cleanup errors to avoid spurious failures.

### Fix Focus Areas
- ordinaldb/examples/rust_persist_demo.rs[33-36]

### Suggested change
Replace `std::fs::remove_dir_all(&demo_root)?;` with one of:
- `let _ = std::fs::remove_dir_all(&demo_root);`
- or log and continue:
 ```rust
 if let Err(e) = std::fs::remove_dir_all(&demo_root) {
     eprintln!("warning: failed to clean up {demo_root:?}: {e}");
 }
 ```

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


Qodo Logo

Signed-off-by: Nelson Spence <nelson@projectnavi.ai>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72a92e7032

ℹ️ 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".

Comment thread ordinaldb-manifest/src/lib.rs Outdated
Comment thread tests/dependency_policy.py Outdated
Comment thread ordinaldb-manifest/src/lib.rs Outdated
Comment thread scripts/release_crate_package_smoke.sh
@Fieldnote-Echo
Nelson Spence (Fieldnote-Echo) force-pushed the codex/manifest-compat-foundation branch from 72a92e7 to a0cc14b Compare July 25, 2026 18:49

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a0cc14bd99

ℹ️ 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".

Comment thread ordinaldb-manifest/src/lib.rs Outdated
Comment thread ordinaldb-ltr/src/lib.rs
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
@Fieldnote-Echo
Nelson Spence (Fieldnote-Echo) force-pushed the codex/manifest-compat-foundation branch from ea3ffed to 1bd4c19 Compare July 25, 2026 20:02
@Fieldnote-Echo

Copy link
Copy Markdown
Member Author

Corrected foundation pushed at 1bd4c19d3eb56d2291a5002256da9e56d4082ef8.

  • Both ordvec packages remain version 0.6.0 and resolve in all five locked roots from exact merged ordvec SHA 51ee981c87e9e4dad414106f8946f0464c721907.
  • No active ordvec/ordvec-manifest 0.7.0 claim remains.
  • Integration package smoke passed through the exact-revision staged registry.
  • Release mode now fails closed on every Git manifest declaration or lock source, including the optional BEIR comparator; registry publication remains a later gate after ordvec 0.6.0 is actually published.
  • Manifest migration, source-policy, formatter, workspace, and package proofs are green locally.

@Fieldnote-Echo Nelson Spence (Fieldnote-Echo) changed the title feat(manifest): add strict v1 compatibility foundation feat(manifest): add v1 compatibility facade and pin ordvec 0.6.0 Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant