Skip to content

feat: add plan-verified streaming decode#303

Open
Nelson Spence (Fieldnote-Echo) wants to merge 4 commits into
mainfrom
codex/ordvec-plan-verified-decode
Open

feat: add plan-verified streaming decode#303
Nelson Spence (Fieldnote-Echo) wants to merge 4 commits into
mainfrom
codex/ordvec-plan-verified-decode

Conversation

@Fieldnote-Echo

Copy link
Copy Markdown
Member

Summary

  • add forward-only sized decoders for RankQuant and SignBitmap while preserving the existing seek-capable entry points
  • add typed plan-verified primary and auxiliary decoding with explicit artifact-kind checks
  • open the final component without following links, require a regular disk file, bound and hash delivered bytes, drain after early decoder failure, and recheck descriptor state
  • add bounded manifest-byte reading and strict current-v2 parsing, with the existing file loader delegating through those APIs
  • document the observable error precedence and the limits of the freshness guarantee

Why

OrdinalDB needs to load large artifacts directly into their final representations without first allocating an artifact-sized byte buffer. The verified-plan API also needs one consistent contract for access failures, type rejection, stale content, decoder failures, and incomplete consumption.

Contract and limits

Plan-verified means that the delivered bytes and observed descriptor state match the size and digest recorded in a verified plan. It does not authenticate the manifest producer or signer.

The loader rejects a final symlink or reparse point and a nonregular descriptor, but it trusts the caller-selected local parent directory. It detects mutations reflected in delivered bytes or observed descriptor state; it cannot promise detection of a transient mutation that is perfectly restored between observations.

Validation

  • workspace formatting and strict Clippy across targets and feature sets
  • default, experimental, test-utils, no-default, manifest, and FFI test suites
  • Rust 1.89 minimum-version build and tests
  • Windows GNU, WASI, FreeBSD, and NetBSD target checks
  • rustdoc with warnings denied
  • cargo-deny and release-invariant suites
  • package verification for ordvec and ordvec-manifest

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

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.04762% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/rank_io.rs 98.94% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add plan-verified streaming decode for ordvec-manifest artifacts

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

Grey Divider

AI Description

• Add forward-only, exactly-sized decoders for RankQuant and SignBitmap.
• Add plan-verified primary/aux decoding with no-follow open, hashing, and strict error precedence.
• Harden manifest loading with bounded reads, strict v2 parsing, and lossless JSON-number
 validation.
Diagram

graph TD
  A["Caller / App"] --> B["VerifiedLoadPlan decode_*"] --> C["decode_plan_verified"] --> D[("No-follow file")] --> E["DigestingBoundedReader"] --> F["Caller decoder"]
  A --> M["read_manifest_bytes_bounded"] --> N["parse_current_manifest_bytes"] --> O["validate_number_tokens"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Return a VerifiedReader instead of a decode closure
  • ➕ Lets callers compose decoding with existing streaming utilities without closure signatures
  • ➕ Makes it easier to reuse the same verified stream for multi-stage decoders
  • ➖ Harder to enforce 'must consume exactly N bytes' without API footguns
  • ➖ More surface area/lifetime complexity; callers could accidentally leak the reader past the verification boundary
2. Pin file descriptors during verification (verify-and-hold-open)
  • ➕ Stronger freshness guarantee vs path-reopen races; avoids descriptor re-open entirely
  • ➕ Can avoid some metadata re-checking complexity at decode time
  • ➖ Cross-platform complexity (FD ownership, duplication, sandboxing) and harder ergonomics
  • ➖ Not always compatible with current workflow (verification cache, later decoding, process boundaries)
3. Use OS advisory locks / mandatory locking policy
  • ➕ Could reduce mutation risk during decode on cooperative local filesystems
  • ➖ Not portable/reliable across filesystems and OSes; policy-heavy and easy to misconfigure
  • ➖ Doesn't address non-lock-respecting writers; complicates threat model

Recommendation: The PR’s closure-based plan-verified decode is the best fit for the stated contract: it keeps the verification boundary tightly scoped to one open descriptor, enforces exact byte consumption, and provides a clear typed error precedence model. A VerifiedReader-style API is plausible, but would need careful design to preserve the same fail-closed guarantees.

Files changed (14) +1749 / -140

Enhancement (4) +935 / -55
lib.rsImplement plan-verified decoding, safe no-follow open, and strict manifest parsing +803/-32

Implement plan-verified decoding, safe no-follow open, and strict manifest parsing

• Introduces bounded manifest read and strict current-schema parsing with lossless JSON-number token validation, and updates the existing loader to delegate to these APIs. Adds cross-platform final-component no-follow regular-file opening (unix/windows/wasi), a bounded hashing reader, and new plan decode entry points (primary and auxiliary) that enforce kind checks, size/digest validation, drain-on-failure, and documented error precedence.

ordvec-manifest/src/lib.rs

quant.rsExpose forward-only exactly-sized RankQuant decoder +11/-0

Expose forward-only exactly-sized RankQuant decoder

• Adds RankQuant::read_from_sized(Read, encoded_len) as a forward-only decoder for a single exactly-sized record, delegating to the new sized loader in rank_io.

src/quant.rs

rank_io.rsAdd sized streaming loaders and chunked typed reads for RankQuant/SignBitmap +110/-23

Add sized streaming loaders and chunked typed reads for RankQuant/SignBitmap

• Introduces sized loader variants that cap reads to an explicit encoded_len boundary (including header) and fail closed on truncation or trailing bytes without requiring Seek. Refactors typed vector decoding to use fixed 64KiB chunks to avoid per-element reads and adds tests for fallible allocation behavior.

src/rank_io.rs

sign_bitmap.rsExpose forward-only exactly-sized SignBitmap decoder +11/-0

Expose forward-only exactly-sized SignBitmap decoder

• Adds SignBitmap::read_from_sized(Read, encoded_len) as a forward-only decoder for a single exactly-sized record, delegating to the new sized loader in rank_io.

src/sign_bitmap.rs

Tests (3) +705 / -54
deterministic.rsRefactor tests to exercise strict current-manifest parsing on raw bytes +99/-53

Refactor tests to exercise strict current-manifest parsing on raw bytes

• Reworks numeric-token fixtures to generate raw manifest bytes, run parse_current_manifest_bytes before writing, and validate early rejection of lossy number lexemes. Adds regression tests ensuring the number scan ignores numeric-looking strings and escapes.

ordvec-manifest/tests/deterministic.rs

verified_decode.rsAdd end-to-end tests for plan-verified decode semantics and precedence +414/-0

Add end-to-end tests for plan-verified decode semantics and precedence

• Adds a comprehensive suite validating primary/aux decode success, kind mismatch short-circuiting, stale-vs-decoder error precedence, initial/final size change detection, required full consumption, optional-absent typing, and symlink/reparse/fifo rejections (platform-gated). Includes a subprocess test to ensure FIFO rejection cannot hang.

ordvec-manifest/tests/verified_decode.rs

loader_validation.rsAdd forward-only sized decoder validation under fragmentation/interrupts +192/-1

Add forward-only sized decoder validation under fragmentation/interrupts

• Adds custom Read spies to simulate short reads, interruptions, and over-read detection. Adds tests ensuring sized decoders never read past the declared boundary, reject inexact lengths before allocation, and batch underlying reads for large typed payloads.

tests/index/loader_validation.rs

Documentation (4) +79 / -30
CHANGELOG.mdDocument plan-verified streaming decode and strict manifest parsing +32/-6

Document plan-verified streaming decode and strict manifest parsing

• Adds release notes for the new forward-only sized decoders, plan-verified decode APIs, bounded manifest read/parse helpers, and the stable serde_json feature profile and number-token validation behavior. Updates the canonical-addressing fix note and adds a security note describing the closed verify/path-reopen gap and remaining limits.

CHANGELOG.md

THREAT_MODEL.mdClarify plan-verified decode guarantees and remaining TOCTOU limits +8/-2

Clarify plan-verified decode guarantees and remaining TOCTOU limits

• Updates threat model text to describe the new plan-verified decode behavior: no-follow open, regular-file requirements, disk-handle check on Windows, and size/digest revalidation. Explicitly documents what is and is not protected (trusted parent boundary, no producer authentication, transient mutation limits).

THREAT_MODEL.md

INDEX_PROVENANCE.mdUpdate guidance to use plan decode APIs at the load boundary +9/-8

Update guidance to use plan decode APIs at the load boundary

• Replaces older guidance (require_auxiliary + load-by-path) with decode_primary_with / decode_verified_with usage. Clarifies that callers supply the decoder and that the plan-verified methods validate delivered bytes without making later mmaps immutable.

docs/INDEX_PROVENANCE.md

README.mdShow plan-verified decoding examples and freshness limits +30/-14

Show plan-verified decoding examples and freshness limits

• Updates examples to decode the primary artifact and auxiliaries via plan-verified APIs using a forward-only sized decoder. Expands the contract section to explain mutation detection boundaries, trusted parent directory assumptions, and non-goals like producer authentication.

ordvec-manifest/README.md

Other (3) +30 / -1
Cargo.lockLock new platform-specific dependencies for no-follow opens +3/-0

Lock new platform-specific dependencies for no-follow opens

• Adds locked entries for libc, rustix, and windows-sys as transitive/runtime dependencies used by ordvec-manifest for platform-specific final-component-safe opens.

Cargo.lock

Cargo.tomlSwitch serde_json feature profile and add OS-specific open deps +10/-1

Switch serde_json feature profile and add OS-specific open deps

• Replaces serde_json arbitrary_precision with float_roundtrip for production builds. Adds libc (unix), windows-sys (windows), and rustix (wasi) dependencies to support final-component no-follow opens and file-type checks.

ordvec-manifest/Cargo.toml

release_publish_invariants.pyEnforce production serde_json feature policy for ordvec-manifest +17/-0

Enforce production serde_json feature policy for ordvec-manifest

• Extends release invariants to require ordvec-manifest’s production serde_json features to be exactly ['float_roundtrip'], rejecting arbitrary_precision/preserve_order/raw_value/unbounded_depth in release metadata.

tests/release_publish_invariants.py

@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: a47c95fb77

ℹ️ 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 src/rank_io.rs Outdated
r.read_exact(&mut buf)?;
v.push(parse(buf));
let elements_per_chunk = READ_CHUNK_BYTES / W;
let mut scratch = [0u8; READ_CHUNK_BYTES];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Move the 64 KiB read buffer off the stack

On threads configured with small stacks, every nonempty Rank, Bitmap, or SignBitmap load now places a 64 KiB array in the loader's stack frame, replacing the previous 2–8 byte buffer; this can overflow the thread stack and abort the process even for a tiny valid artifact. Allocate this chunk fallibly on the heap, or use a substantially smaller stack buffer, so the loader preserves its recoverable-error behavior.

Useful? React with 👍 / 👎.

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

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@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: 6607c98b23

ℹ️ 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 ordvec-manifest/src/lib.rs Outdated
let decoded = decoder(&mut reader, expected_size);
let decoder_consumed = reader.consumed;

let mut scratch = [0u8; VERIFIED_DECODE_DRAIN_BYTES];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Move the plan decode drain buffer off the stack

On worker threads configured with small stacks, every call to decode_primary_with or decode_verified_with reserves this 64 KiB array in decode_plan_verified's stack frame—even when the decoder consumes the entire artifact and the drain loop never runs—so a valid decode can overflow the thread stack and abort the process. Although the similar rank_io scratch buffer has now been moved to fallible heap storage, this separate plan-decode buffer remains; allocate it fallibly on the heap or use a substantially smaller stack chunk.

Useful? React with 👍 / 👎.

Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
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