feat(warp-core): TTD domain logic — provenance storage + causal cone walk#298
feat(warp-core): TTD domain logic — provenance storage + causal cone walk#298flyingrobots merged 10 commits intomainfrom
Conversation
…c digest exports) Port TTD domain logic from origin/ttd-spec into main: - Remove dead_code allows on compute_tick_commit_hash_v2, compute_op_emission_index_digest, OpEmissionEntry, and compute_emissions_digest; export them from warp-core public API. - Wire LocalProvenanceStore::append_with_writes() to actually store atom writes (previously accepted but discarded the parameter). - Add atom_writes(w, tick) and atom_history(w, atom) query methods for TTD "Show Me Why" provenance tracking. - Fix fork() to copy atom_writes alongside other history data. - Add 7 tests covering atom write storage, queries, filtering, and fork.
…II D(v)) Replace the brute-force linear scan in atom_history with a causal cone walk using the declared out_slots (Paper III's Out(μ)) on each tick patch. The algorithm: - Walks backwards from tip, checking each tick's out_slots for the atom's SlotId (attachment α-plane or node skeleton) - Only collects AtomWrites from ticks whose Out(μ) declares the atom - Terminates early when a creation write is found (atom's origin) - No reverse index needed — the slot declarations ARE the index Updated tests to populate out_slots on patches, matching how the engine builds patches from footprints. Added new tests: - atom_history_walks_causal_cone: 3-tick linear write chain - atom_history_skips_ticks_not_in_causal_cone: verifies unrelated ticks are skipped - atom_history_terminates_at_creation: creation at tick 5 with 5 prior unrelated ticks — walk stops at creation, never scans 0-4
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughExposes previously-suppressed snapshot symbols and implements per-tick atom write persistence and causal-cone traversal in the provenance store; appends and forks now carry atom_writes, and Rustdoc/invariants updated to reflect the added field and behavior. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef9f0b0a3d
ℹ️ 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 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 address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/warp-core/src/provenance_store.rs (2)
517-523:⚠️ Potential issue | 🟡 MinorUpdate the public
fork()contract.The implementation now clones
atom_writes, but the rustdoc above still says the fork copies only patches, expected hashes, outputs, and checkpoints. Keep the public contract aligned with the behavior.
As per coding guidelines, "Every public API across crates (warp-core, warp-wasm, etc.) must carry rustdoc comments that explain intent, invariants, and usage."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/warp-core/src/provenance_store.rs` around lines 517 - 523, The rustdoc for the public fork() API is out of date: the implementation now clones atom_writes in addition to patches, expected hashes, outputs, and checkpoints. Update the fork() rustdoc (the public comment on the fork() function that references WorldlineHistory) to explicitly mention that atom_writes are also copied/cloned into the new history, clarifying intent, invariants, and usage for callers and referencing the WorldlineHistory fields (patches, expected, outputs, checkpoints, atom_writes) so the public contract matches the implementation.
292-318:⚠️ Potential issue | 🟠 MajorReject
AtomWriterows that are unreachable fromout_slots.
atom_history()treatspatch.out_slotsas its only index, butappend_with_writes()will currently persist anyAtomWriteSet. That means writes whosetickdisagrees withpatch.global_tick()or whose atom slot is absent fromout_slotsremain visible throughatom_writes()and permanently invisible toatom_history(). The new test at Line 921 already stores exactly that inconsistent shape (test_patch(0)has emptyout_slots). Fail fast here, or at minimum document and enforce the invariant at the API boundary.
As per coding guidelines, "Every public API across crates (warp-core, warp-wasm, etc.) must carry rustdoc comments that explain intent, invariants, and usage."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/warp-core/src/provenance_store.rs` around lines 292 - 318, append_with_writes must validate AtomWriteSet rows against the patch invariants before persisting: reject any AtomWrite whose tick != patch.global_tick() or whose atom slot is not listed in patch.out_slots so they cannot be stored visible to atom_writes() but invisible to atom_history(); modify append_with_writes (and the Worldline history mutation path that pushes history.atom_writes) to iterate atom_writes, verify each AtomWrite.tick == patch.global_tick() and AtomWrite.slot ∈ patch.out_slots, and return a suitable HistoryError (or convert the set to a filtered/empty set) on violation; also add a rustdoc on append_with_writes explaining the invariant that atom_writes must be reachable from patch.out_slots and aligned to patch.global_tick() so callers know the expectation and failure mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/warp-core/src/lib.rs`:
- Around line 193-194: Expose the new digest helpers and types
(compute_commit_hash_v2, compute_emissions_digest,
compute_op_emission_index_digest, compute_state_root_for_warp_store,
compute_tick_commit_hash_v2, OpEmissionEntry, Snapshot) to the DIND end-to-end
harness by adding a deterministic golden-hash test that exercises the full
TTDR/provenance hash chain via the crate-root re-exports; update the DIND test
harness to call these public functions in sequence using the snapshot unit
vectors from snapshot.rs and record/verify the resulting hashes against
checked-in golden values so any future drift in the public digest surface is
detected as a test failure.
In `@crates/warp-core/src/provenance_store.rs`:
- Around line 380-391: The current reverse loop in the provenance lookup
(scanning history.patches and checking out_slots) makes reads O(history length);
change to maintain a deterministic per-slot last-writer index during
append_with_writes() and query that index instead of scanning. Specifically,
update append_with_writes() to record for each attachment_slot/node_slot the
patch index (or a stable writer-chain id) into a deterministic data structure
(e.g., a Vec or BTree keyed by slot) maintained on the History/ProvenanceStore;
then replace the reverse for-loop that inspects history.patches[*].out_slots
with a direct lookup into that per-slot last-writer map when resolving
last-writer/causal-cone queries so lookups are O(1)/O(log n) and avoid
non-deterministic or allocation-heavy hot-path work. Ensure the new index is
updated atomically with patch appends and referenced by the same methods that
currently use out_slots.
- Around line 393-403: The current backward scan over ticks collects AtomWrite
entries by iterating tick_writes forward and reversing per-tick which scrambles
within-tick order and can truncate a later mutation when a create appears
earlier; change the inner loop over history.atom_writes.get(tick_idx) so you
iterate tick_writes in reverse order (e.g., for aw in tick_writes.iter().rev())
while still scanning ticks backward, remove any per-tick reverse so only the
single final writes_rev.reverse() remains, and ensure the is_create() check
still stops the outer walk correctly; add a regression test that inserts two
AtomWrite entries in the same tick in order [create, mutate] and asserts the
returned history preserves both with the mutate after the create.
---
Outside diff comments:
In `@crates/warp-core/src/provenance_store.rs`:
- Around line 517-523: The rustdoc for the public fork() API is out of date: the
implementation now clones atom_writes in addition to patches, expected hashes,
outputs, and checkpoints. Update the fork() rustdoc (the public comment on the
fork() function that references WorldlineHistory) to explicitly mention that
atom_writes are also copied/cloned into the new history, clarifying intent,
invariants, and usage for callers and referencing the WorldlineHistory fields
(patches, expected, outputs, checkpoints, atom_writes) so the public contract
matches the implementation.
- Around line 292-318: append_with_writes must validate AtomWriteSet rows
against the patch invariants before persisting: reject any AtomWrite whose tick
!= patch.global_tick() or whose atom slot is not listed in patch.out_slots so
they cannot be stored visible to atom_writes() but invisible to atom_history();
modify append_with_writes (and the Worldline history mutation path that pushes
history.atom_writes) to iterate atom_writes, verify each AtomWrite.tick ==
patch.global_tick() and AtomWrite.slot ∈ patch.out_slots, and return a suitable
HistoryError (or convert the set to a filtered/empty set) on violation; also add
a rustdoc on append_with_writes explaining the invariant that atom_writes must
be reachable from patch.out_slots and aligned to patch.global_tick() so callers
know the expectation and failure mode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 196dba57-c4aa-4a99-a23a-cfdbedc8c2c9
📒 Files selected for processing (4)
CHANGELOG.mdcrates/warp-core/src/lib.rscrates/warp-core/src/provenance_store.rscrates/warp-core/src/snapshot.rs
The backward tick walk in atom_history() collected per-tick writes in forward order, so the final reverse() flipped within-tick execution sequence. Worse, a [create, mutate] pair in the same tick would lose the mutate because forward iteration hit is_create() first and returned early. Fix: iterate tick_writes.iter().rev() so the global reverse restores original execution order within each tick. Add regression test for same-tick [create, mutate] ordering. Also: - Update fork() rustdoc to mention atom_writes in copied fields - Document append_with_writes() invariant: atom writes must reference atoms declared in patch.out_slots for atom_history() visibility
|
Fixed the within-tick write ordering bug and creation truncation in
On the per-slot reverse index suggestion (CodeRabbit inline at L380-391): On runtime validation in On DIND golden-hash test (CodeRabbit nit at L193-194): Good idea for a follow-up. The snapshot functions already have unit-level golden vectors. A DIND end-to-end test exercising the full hash chain through crate-root re-exports is worth doing but is orthogonal to this PR. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 16-18: Update the CHANGELOG entry for
LocalProvenanceStore::atom_history(w, atom) to remove or soften the inaccurate
"No linear scan" claim and state the correct complexity: note that the
implementation still traverses patches from tip to origin (iterating ticks with
for tick_idx in (0..len).rev()) and thus is O(history_length) for patch
traversal, but uses out_slots/Out(μ) to filter examined ticks and supports early
termination at creation; reword to reflect that behavior and complexity
precisely.
In `@crates/warp-core/src/provenance_store.rs`:
- Around line 275-298: Add a debug-only assertion in the append tick path (the
function that takes atom_writes and patch.out_slots—e.g., append_tick or
whatever method processes AtomWrite) to validate the documented invariant: for
each AtomWrite in atom_writes assert that either
SlotId::Attachment(AttachmentKey::node_alpha(aw.atom)) or SlotId::Node(aw.atom)
is present in patch.out_slots; implement this under #[cfg(debug_assertions)]
using debug_assert! so it costs nothing in release builds and helps catch
third-party/invalid patches that would make atom_history() return incomplete
results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f1ee135-37ca-4980-8820-7f293ef15ff2
📒 Files selected for processing (2)
CHANGELOG.mdcrates/warp-core/src/provenance_store.rs
Add DIND-level golden-hash tests that exercise the full TTD digest public API surface through warp-core crate-root re-exports: - compute_emissions_digest: pinned golden for 2-channel emission set - compute_op_emission_index_digest: pinned golden for 2-op index - compute_tick_commit_hash_v2: full hash chain (emissions → op-index → tick-commit) with pinned golden These tests catch wire format drift and accidental re-export removal that module-internal tests in snapshot.rs would miss.
…into feat/ttd-domain-logic
…slots invariant - Fix CHANGELOG: replace inaccurate "No linear scan" with "O(history) scan" for atom_history() — the implementation traverses all patches from tip to origin, filtering via out_slots. - Add debug_assert in append_with_writes() validating that every AtomWrite references an atom declared in patch.out_slots. Zero cost in release builds, catches invariant violations during development. - Fix test: append_with_writes_stores_atom_writes now uses test_patch_with_atom_slots instead of bare test_patch, satisfying the new debug assertion.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 8-21: Add a brief bullet to the changelog describing the newly
added provenance store tests: mention the seven tests for atom write wiring
(including atom_history_walks_causal_cone,
atom_history_skips_ticks_not_in_causal_cone,
atom_history_terminates_at_creation, atom_history_preserves_within_tick_order)
that exercise LocalProvenanceStore::append_with_writes and atom_writes, and the
three tests for causal-cone behavior that validate
LocalProvenanceStore::atom_history; place this bullet after the existing
API/testing bullets so readers can see the coverage added alongside the API
changes.
In `@crates/echo-dind-harness/tests/digest_golden_vectors.rs`:
- Around line 22-26: The current make_hash() produces mostly uniform 32-byte
values (only h[0] set), making golden vectors weak; change make_hash to produce
non-uniform, position-distinct bytes (e.g., set h[i] = n.wrapping_add(i as u8)
or combine n with the index) so every of the 32 bytes is distinguishable, and
update the other repeated-byte fixtures used in the full-chain test (the other
occurrences around the noted lines) to use this new non-uniform generator so
your golden hash chains exercise all byte positions.
In `@crates/warp-core/src/provenance_store.rs`:
- Around line 908-930: The reproducers only exercise atom writes via
SlotId::Attachment(node_alpha(atom)); add a test case to cover the
SlotId::Node(atom) branch by extending or creating a patch (use
test_patch_with_atom_slots or test_patch_with_atom_mutation) whose out_slots
contains SlotId::Node(<atom>) instead of Attachment(...), then call
append_with_writes(...) with that patch and assert it succeeds and that
atom_history(...) returns the expected entries for that atom; ensure the new
assertion mirrors existing checks so both append_with_writes and atom_history
accept SlotId::Node atom writes.
- Around line 285-305: The append_with_writes API currently stores
caller-provided AtomWrite entries without validating their embedded tick,
leading to inconsistent provenance; update the rustdoc for append_with_writes
(and the same place around lines 321-339) to document the invariant that each
AtomWrite.tick must equal patch.global_tick(), and add a runtime assertion when
processing atom_writes in append_with_writes: iterate atom_writes (type
AtomWriteSet) and assert each AtomWrite.tick == patch.global_tick() alongside
the existing out_slots validation so incorrect ticks are rejected early (this
keeps atom_writes()/atom_history() consistent and references
WorldlineTickPatchV1, AtomWrite.tick, atom_writes(), atom_history(), and
out_slots for location).
- Around line 370-375: The rustdoc for LocalProvenanceStore::atom_history is
inaccurate; update the comment above the atom_history() method to describe the
actual algorithm: it performs a backward O(history_length) scan over the
worldline's patch list, uses each patch's out_slots to filter which ticks to
extract writes for the given atom, and stops early when it encounters the atom's
creation write (origin); explicitly state the complexity (O(history_length)
worst-case) and that out_slots only reduces work by filtering/early termination
rather than avoiding the scan.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e9ffceea-e2d8-4349-9284-c99154ed7e63
📒 Files selected for processing (3)
CHANGELOG.mdcrates/echo-dind-harness/tests/digest_golden_vectors.rscrates/warp-core/src/provenance_store.rs
…m_writes - Fix atom_history() rustdoc: replace misleading "Instead of scanning every tick" with accurate description — the method walks all patches but uses out_slots to filter which ticks' atom writes are examined. - Guard atom_writes() against tick-as-usize truncation on 32-bit targets (WASM): bounds-check tick against Vec length before casting, preventing wrong-index access when tick exceeds usize::MAX.
- Add debug_assert validating AtomWrite.tick == patch.global_tick() in append_with_writes(), catching tick mismatches that would produce self-contradictory provenance. Document the invariant in rustdoc. - Use non-uniform 32-byte fixtures in golden vector tests: make_hash() now produces position-distinct bytes via wrapping arithmetic, catching serializer bugs that drop or reorder trailing zeros. - Add CHANGELOG bullet documenting the 11 provenance store tests.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Cover the skeleton-level provenance path (UpsertNode/DeleteNode) in atom_history() via SlotId::Node, complementing existing tests that only exercise SlotId::Attachment. Addresses CodeRabbit review nit on PR #298.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary
dead_codeallows on digest functions (compute_tick_commit_hash_v2,compute_op_emission_index_digest, etc.) and export them from thewarp-corepublic API. WireLocalProvenanceStore::append_with_writes()to actually store atom writes. Addatom_writes(w, tick)andatom_history(w, atom)query methods for TTD "Show Me Why" provenance tracking. Fixfork()to copy atom writes alongside other history data.atom_history(Paper III §D(v)): Replace the brute-force linear scan with a walk that uses declaredout_slots(Paper III'sOut(μ)) on each tick patch. The walk terminates early at the atom's creation tick — no reverse index needed, the slot declarations ARE the index.Test plan
cargo test)atom_history_walks_causal_cone,atom_history_skips_ticks_not_in_causal_cone,atom_history_terminates_at_creation)Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores