Skip to content

feat(warp-core): add WAL projection fact types#601

Open
flyingrobots wants to merge 1 commit into
mainfrom
cycle/559-wal-projection-fact-types
Open

feat(warp-core): add WAL projection fact types#601
flyingrobots wants to merge 1 commit into
mainfrom
cycle/559-wal-projection-fact-types

Conversation

@flyingrobots

Copy link
Copy Markdown
Owner

Summary

Closes #559.

This adds typed WAL projection fact records for the GP2 WAL evidence projection slice:

  • WalRoot
  • WalWriterEpoch
  • WalSegmentRef
  • WalCommitAnchor
  • RecoveryCertificateRef

WalSegmentRef::identity_digest() now binds writer epoch, logical segment id, LSN range, commit digest chain, segment digest, commit anchors, and seal posture while deliberately excluding storage locator metadata. WalRoot::identity_digest() canonicalizes writer epoch and segment members by identity digest so caller-provided vector order does not create projection nondeterminism.

Validation

  • cargo test -p warp-core wal_projection_fact_identity
  • cargo clippy -p warp-core --lib --test causal_wal_tests -- -D warnings
  • cargo fmt --check
  • npx markdownlint-cli2 CHANGELOG.md docs/BEARING.md
  • git diff --check
  • pre-commit hook: cargo clippy -p warp-core --lib, cargo check -p warp-core, Markdown lint
  • pre-push hook: cargo fmt --all -- --check, cargo check -p warp-core --quiet, cargo test -p warp-core --test causal_wal_tests, Prettier check

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@flyingrobots, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 30 minutes and 36 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8f61b842-c793-4f5c-bd43-05401c28ab75

📥 Commits

Reviewing files that changed from the base of the PR and between 627f236 and e623377.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • crates/warp-core/src/causal_wal.rs
  • crates/warp-core/tests/causal_wal_tests.rs
  • docs/BEARING.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cycle/559-wal-projection-fact-types

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@flyingrobots

Copy link
Copy Markdown
Owner Author

@coderabbitai review please 🥕

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

gh pr view 601 --repo flyingrobots/echo --json title,body,files,additions,deletions,changedFiles

echo "---DIFF---"
gh pr diff 601 --repo flyingrobots/echo

Length of output: 24776


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== len_u64 definition ==="
rg -n 'fn len_u64' crates/warp-core/src/causal_wal.rs -A5

echo "=== Hash type alias ==="
rg -n 'pub type Hash\|type Hash' crates/warp-core/src/ -r ''
rg -n 'type Hash' crates/warp-core/src/

echo "=== WalTransactionId Ord/PartialOrd ==="
rg -n 'WalTransactionId' crates/warp-core/src/causal_wal.rs | head -20

echo "=== WriterEpochId as_hash ==="
ast-grep --pattern 'impl WriterEpochId { $$$ }' 

echo "=== Hash derive/impl Ord ==="
rg -n 'pub.*Hash' crates/warp-core/src/ -A2 | head -40

echo "=== WalSegmentId as_u64 ==="
rg -n 'fn as_u64' crates/warp-core/src/causal_wal.rs | head -10

echo "=== existing non_exhaustive usage in causal_wal.rs ==="
rg -n 'non_exhaustive' crates/warp-core/src/causal_wal.rs | head -20

echo "=== RecoveryTailPosture variants ==="
rg -n 'RecoveryTailPosture' crates/warp-core/src/causal_wal.rs | head -20

Length of output: 1355


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Hash type in lib.rs ==="
fd 'lib.rs' crates/warp-core/src --exec grep -n 'Hash'

echo "=== Hash definition ==="
rg -n 'type Hash\b' crates/warp-core/src/

echo "=== WalTransactionId struct/derive ==="
rg -n -A4 'struct WalTransactionId' crates/warp-core/src/causal_wal.rs

echo "=== WriterEpochId struct/derive ==="
rg -n -A4 'struct WriterEpochId' crates/warp-core/src/causal_wal.rs

echo "=== WalSegmentId struct/derive ==="
rg -n -A4 'struct WalSegmentId' crates/warp-core/src/causal_wal.rs

echo "=== non_exhaustive in causal_wal.rs ==="
rg -n 'non_exhaustive' crates/warp-core/src/causal_wal.rs

echo "=== RecoveryTailPosture enum ==="
rg -n -A15 'enum RecoveryTailPosture' crates/warp-core/src/causal_wal.rs

echo "=== Hash type in warp-core ==="
rg -rn 'pub type Hash' crates/warp-core/src/

echo "=== Hash in top-level lib.rs ==="
cat crates/warp-core/src/lib.rs | head -60

Length of output: 2132


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== WalTransactionId full definition ==="
rg -n -B3 'struct WalTransactionId' crates/warp-core/src/causal_wal.rs

echo "=== WriterEpochId full definition ==="
rg -n -B3 'struct WriterEpochId' crates/warp-core/src/causal_wal.rs

echo "=== RecoveryTailPosture full enum ==="
rg -n -B1 -A20 'enum RecoveryTailPosture' crates/warp-core/src/causal_wal.rs

echo "=== WalSegmentSealPosture full definition ==="
rg -n -B3 -A15 'enum WalSegmentSealPosture' crates/warp-core/src/causal_wal.rs

echo "=== WalSegmentStorageLocator full definition ==="
rg -n -B3 -A15 'enum WalSegmentStorageLocator' crates/warp-core/src/causal_wal.rs

echo "=== WalCommitAnchor full definition ==="
rg -n -B2 'struct WalCommitAnchor' crates/warp-core/src/causal_wal.rs

echo "=== Lsn struct derive ==="
rg -n -B3 'struct Lsn' crates/warp-core/src/causal_wal.rs | head -20

echo "=== WalTransactionId as_hash ==="
rg -n 'as_hash\|as_u64' crates/warp-core/src/causal_wal.rs | head -20

Length of output: 4532



@flyingrobots — solid work. Domain separation, length prefixes, optional-field discriminants, and sort_unstable canonicalization are all correct. Three findings below.


Finding 1 — Unnecessary clone in WalSegmentRef::identity_digest() (medium)

WalRoot::identity_digest() correctly collects digests and sorts those. WalSegmentRef::identity_digest() takes a different path: it clones the entire commit_anchors vec, sorts the structs by a field-tuple key, then computes digests in the loop. That's one full Vec<WalCommitAnchor> allocation per call.

// current — clones all WalCommitAnchor structs
let mut anchors = self.commit_anchors.clone();
anchors.sort_by_key(|anchor| {
    (anchor.first_lsn, anchor.last_lsn, anchor.transaction_id, anchor.commit_digest)
});
h.update(&len_u64(anchors.len()).to_le_bytes());
for anchor in &anchors {
    h.update(&anchor.identity_digest());
}

Consistent with the WalRoot pattern and avoids the allocation:

// preferred — collect digests, sort those
let mut anchor_digests: Vec<Hash> = self
    .commit_anchors
    .iter()
    .map(WalCommitAnchor::identity_digest)
    .collect();
anchor_digests.sort_unstable();
h.update(&len_u64(anchor_digests.len()).to_le_bytes());
for d in &anchor_digests {
    h.update(d);
}

Since WalCommitAnchor::identity_digest() is injective over the fields that matter for identity, sort-by-digest is equivalent to sort-by-field-tuple for determinism purposes.


Finding 2 — Writer-epoch reordering is not exercised by the test (low)

WalRoot::identity_digest() sorts writer_epoch_digests before hashing — that branch is untested. Both root and reordered_root in the test carry a single epoch, so no reordering can occur.

// add a second epoch and verify reordering doesn't change the digest
let root = WalRoot {
    writer_epochs: vec![writer_epoch_a.clone(), writer_epoch_b.clone()],
    ...
};
let reordered_root = WalRoot {
    writer_epochs: vec![writer_epoch_b, writer_epoch_a],
    ...
};
assert_eq!(root.identity_digest(), reordered_root.identity_digest());

Finding 3 — Public enums missing #[non_exhaustive] (low)

WalSegmentSealPosture and WalSegmentStorageLocator are public enums. Any downstream crate with an exhaustive match on them will fail to compile when a new variant is added. Given that Open/Sealed and RelativePath/AbsolutePath are natural extension points (e.g., an S3 locator, a PartiallySealed posture), #[non_exhaustive] is worth adding now before these types stabilise.

#[non_exhaustive]
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum WalSegmentSealPosture { ... }

#[non_exhaustive]
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum WalSegmentStorageLocator { ... }

Everything else checks out: domain constants are versioned and NUL-terminated, all optional fields use a 1-byte discriminant, RecoveryTailPosture is exhaustively matched (compile-time guard against new variants), and len_u64 length-prefixes both sorted collections to prevent extension attacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

[GP2-S1] WAL Projection Fact Types

1 participant