Skip to content

Conversation

@flyingrobots
Copy link
Owner

Closes #147

Implements Paper II “blocking causality” witness for tick receipts:

  • Tick receipts now record, for each rejected candidate, the indices of earlier applied candidates that blocked it.
  • Digest/commit hash semantics are unchanged: TickReceipt::digest() still commits only to accept/reject outcomes + coarse rejection code, not blocker metadata.

Tests:

  • cargo test --workspace
  • cargo clippy --workspace --all-targets -- -D warnings -D missing_docs

Docs:

  • Updated docs/aion-papers-bridge.md, docs/spec-merkle-commit.md, docs/decision-log.md, docs/execution-plan.md.

Notes:

  • Blocker detection mirrors the scheduler’s current overlap checks (nodes/edges/ports) instead of Footprint::independent while factor_mask is still a placeholder in some footprints.

Record which applied candidates block rejected rewrites (Paper II poset edges) without changing the decision digest format.

Refs #147
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 29, 2025

Summary by CodeRabbit

  • New Features

    • Tick receipts now include per-entry "blocked by" information so rejected entries show which prior candidates blocked them.
  • Tests

    • Added tests validating multi-blocker scenarios and asserting correct blocker attribution for rejected entries.
  • Documentation

    • Updated specs, decision log, and design docs to describe blocking-causality witnesses and stable digest semantics.
  • Chores

    • Public receipt API updated to expose blocker data (constructor/accessor surfaced).

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

The PR extends TickReceipt to record blocking causality for rewrites rejected due to footprint conflicts by adding per-entry blocker lists. It refactors digest computation into helpers, introduces footprint conflict detection, updates TickReceipt’s constructor and accessor, and adds multi-blocker tests and documentation updates. Decision_digest remains unchanged.

Changes

Cohort / File(s) Summary
Engine core logic
crates/warp-core/src/engine_impl.rs
Refactored inline digest hashing into compute_plan_digest and compute_rewrites_digest; added conflict detection (footprints_conflict) and per-entry blocker tracking during reservation; enumerate rewrites with indices and convert indices to u32 for receipts; propagate blocked_by into TickReceipt::new.
Receipt API and structure
crates/warp-core/src/receipt.rs
Added private blocked_by: Vec<Vec<u32>> field; changed constructor to new(tx, entries, blocked_by) with length parity assertion; added public blocked_by(&self, idx) -> &[u32]; clarified that blocker metadata is excluded from decision_digest.
Tests
crates/warp-core/tests/tick_receipt_tests.rs
Expanded imports and deterministic helpers; adjusted digest composition to match new plan metadata; added multi-blocker test (commit_with_receipt_records_multi_blocker_causality) asserting rejected entries include multiple blocker indices; updated existing tests to assert blocker invariants.
Documentation
docs/aion-papers-bridge.md, docs/decision-log.md, docs/spec-merkle-commit.md, docs/execution-plan.md
Updated docs and decision log to record blockers in TickReceipt semantics, note digest stability (blockers excluded), and describe intended behavior and tests.
Scheduler docs
crates/warp-core/src/scheduler.rs
Added/expanded documentation on reserve return contract across scheduler implementations (no behavior change).

Sequence Diagram(s)

sequenceDiagram
    participant Scheduler
    participant RewriteSet
    participant ConflictDetector
    participant Receipt

    Note over Scheduler,Receipt: Tick execution with blocking-causality tracking

    Scheduler->>RewriteSet: drain batch of rewrite candidates
    loop per candidate (canonical plan order)
        RewriteSet->>ConflictDetector: request conflict check (footprint, reserved set)
        alt conflict with reserved entries
            ConflictDetector->>ConflictDetector: compute blocker indices (u32)
            ConflictDetector->>Receipt: record Rejected entry with blocked_by = [idx...]
        else no conflict
            ConflictDetector->>RewriteSet: mark candidate reserved/accepted
            ConflictDetector->>Receipt: record Applied entry with blocked_by = []
        end
    end

    Note over Receipt: entries aligned with blocked_by vectors (same length)
    Scheduler->>Receipt: construct TickReceipt(tx, entries, blocked_by)
    Note over Receipt: decision_digest excludes blocked_by metadata
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🧩 Three rewrites marched in canonical line,
Two stood firm — the third crossed footprints fine.
The receipt now notes, in tidy array,
Who blocked who on that fateful day.
A poset whisper: "Here are the causes."

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title directly and clearly summarizes the main change: implementing tick receipt blocking causality, which is the primary focus of all code modifications.
Description check ✅ Passed The description comprehensively covers the linked issue #147 requirements: blocking causality implementation, digest stability, test coverage, and documentation updates.
Linked Issues check ✅ Passed All primary coding requirements from issue #147 are met: TickReceipt extended with blocked_by field [#147], footprint conflict blocker detection implemented [#147], digest stability maintained [#147], and multi-blocker tests added [#147].
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #147: receipt structure, engine blocker computation, tests, and documentation. The scheduler.rs documentation-only additions are tangential but supportive of blocker detection semantics.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch echo/tick-receipt-blockers

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 99304b2 and 28bc3a8.

📒 Files selected for processing (4)
  • crates/warp-core/src/engine_impl.rs
  • crates/warp-core/src/receipt.rs
  • crates/warp-core/src/scheduler.rs
  • crates/warp-core/tests/tick_receipt_tests.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Every public API across crates (warp-core, warp-ffi, warp-wasm, etc.) must carry rustdoc comments that explain intent, invariants, and usage. Treat missing docs as a failing test.
Run cargo clippy --all-targets -- -D missing_docs before every PR; CI will expect a zero-warning, fully documented surface.
Every source file must start with exactly: // SPDX-License-Identifier: Apache-2.0 and // © James Ross Ω FLYING•ROBOTS <https://github.com/flyingrobots>

Files:

  • crates/warp-core/src/receipt.rs
  • crates/warp-core/src/scheduler.rs
  • crates/warp-core/tests/tick_receipt_tests.rs
  • crates/warp-core/src/engine_impl.rs
🧠 Learnings (1)
📚 Learning: 2025-12-28T23:14:28.083Z
Learnt from: CR
Repo: flyingrobots/echo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T23:14:28.083Z
Learning: Applies to **/*.rs : Every public API across crates (warp-core, warp-ffi, warp-wasm, etc.) must carry rustdoc comments that explain intent, invariants, and usage. Treat missing docs as a failing test.

Applied to files:

  • crates/warp-core/src/scheduler.rs
  • crates/warp-core/tests/tick_receipt_tests.rs
  • crates/warp-core/src/engine_impl.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Dependency Policy (cargo-deny)
🔇 Additional comments (11)
crates/warp-core/src/scheduler.rs (3)

120-129: Documentation addresses prior review concern — LGTM.

The return value contract is now explicit: true means reserved, false means footprint conflict exclusively. The future extensibility note about upgrading to an enum is a nice touch for maintainability.


556-571: Contract documentation mirrors RadixScheduler — consistent API semantics.

The identical documentation across RadixScheduler, LegacyScheduler, and DeterministicScheduler ensures callers can treat the scheduler as a single abstraction with guaranteed behavior.


662-674: Wrapper documentation complete.

The forwarding semantics are clear, and the contract is correctly documented.

crates/warp-core/src/receipt.rs (1)

95-97: Digest stability guarantee clearly documented.

The explicit note that blocker metadata is excluded ensures future maintainers understand this is intentional, not an oversight.

crates/warp-core/src/engine_impl.rs (3)

197-198: Docstring updated to reference Paper II blocking-causality witness.

Clear and accurate documentation of the new behavior.


220-223: Overflow guard for entry index — good defensive programming.

The u32::try_from with error propagation prevents silent truncation on massive tick batches (though unlikely in practice, the invariant is worth enforcing).


404-429: Digest helpers extracted cleanly.

The hash field ordering differs between compute_plan_digest (scope_hash, rule_id) and compute_rewrites_digest (rule_id, scope_hash, scope.0) — this is intentional since they serve different purposes. The empty-case handling uses the canonical DIGEST_LEN0_U64 constant consistently.

crates/warp-core/tests/tick_receipt_tests.rs (4)

86-122: Deterministic test ID selection is correct but dense.

The algorithm correctly ensures C sorts last by iterating candidates in descending scope_hash order and finding compatible A and B. The panic! at line 121 is appropriate — if no valid combination exists, the test design is fundamentally broken.

One observation: the function doesn't guarantee a specific order between A and B, only that both precede C. This is fine since the test only cares that blocked_by(2) == [0, 1] — both A and B block C regardless of their relative order.


203-286: Multi-blocker test correctly validates Paper II blocking-causality witness.

The test constructs a scenario where:

  • Rule A writes scope_a
  • Rule B writes scope_b (= other_of(scope_a))
  • Rule C writes both scope_a AND scope_b

Since A and B are applied first (by deterministic ordering), C is correctly rejected with blocked_by(2) == [0, 1]. This validates the multi-blocker attribution required by issue #147.


183-191: Existing test extended with blocker assertions — validates single-blocker scenario.

The assertions correctly verify that applied entries have no blockers and rejected entries reference their blockers by plan index.


55-78: Test helpers are clean and focused.

fp_write_scope and fp_write_scope_and_other provide deterministic footprints with non-zero factor_mask (avoiding the fast-path issue mentioned in footprints_conflict). The other_of derivation via blake3 hash ensures distinct but reproducible node IDs.


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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0b32fbf and 99304b2.

📒 Files selected for processing (7)
  • crates/warp-core/src/engine_impl.rs
  • crates/warp-core/src/receipt.rs
  • crates/warp-core/tests/tick_receipt_tests.rs
  • docs/aion-papers-bridge.md
  • docs/decision-log.md
  • docs/execution-plan.md
  • docs/spec-merkle-commit.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Every public API across crates (warp-core, warp-ffi, warp-wasm, etc.) must carry rustdoc comments that explain intent, invariants, and usage. Treat missing docs as a failing test.
Run cargo clippy --all-targets -- -D missing_docs before every PR; CI will expect a zero-warning, fully documented surface.
Every source file must start with exactly: // SPDX-License-Identifier: Apache-2.0 and // © James Ross Ω FLYING•ROBOTS <https://github.com/flyingrobots>

Files:

  • crates/warp-core/src/receipt.rs
  • crates/warp-core/src/engine_impl.rs
  • crates/warp-core/tests/tick_receipt_tests.rs
🧠 Learnings (3)
📚 Learning: 2025-12-28T23:14:28.083Z
Learnt from: CR
Repo: flyingrobots/echo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T23:14:28.083Z
Learning: Every meaningful design choice should land in `docs/` (specs, diagrams, memorials) or other durable repo artifacts (e.g. `docs/decision-log.md`).

Applied to files:

  • docs/decision-log.md
📚 Learning: 2025-12-28T23:14:28.083Z
Learnt from: CR
Repo: flyingrobots/echo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T23:14:28.083Z
Learning: Start each session by updating *Today's Intent* in `docs/execution-plan.md` and capture milestones, blockers, and decisions in the Decision Log.

Applied to files:

  • docs/execution-plan.md
📚 Learning: 2025-12-28T23:14:28.083Z
Learnt from: CR
Repo: flyingrobots/echo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T23:14:28.083Z
Learning: When a PR touches non-doc code, update `docs/execution-plan.md` and `docs/decision-log.md` in the same PR.

Applied to files:

  • docs/execution-plan.md
🧬 Code graph analysis (2)
crates/warp-core/src/engine_impl.rs (2)
crates/warp-core/tests/tick_receipt_tests.rs (2)
  • compute_plan_digest (26-34)
  • compute_rewrites_digest (36-51)
crates/warp-core/src/receipt.rs (3)
  • blocked_by (79-81)
  • new (32-45)
  • tx (49-51)
crates/warp-core/tests/tick_receipt_tests.rs (4)
crates/warp-core/src/payload.rs (1)
  • encode_motion_payload (19-25)
crates/warp-core/src/ident.rs (2)
  • make_node_id (37-42)
  • make_type_id (29-34)
crates/warp-core/src/engine_impl.rs (2)
  • scope_hash (426-431)
  • new (74-76)
crates/warp-core/src/receipt.rs (4)
  • new (32-45)
  • blocked_by (79-81)
  • tx (49-51)
  • entries (55-57)
🔇 Additional comments (11)
docs/execution-plan.md (1)

38-43: LGTM — execution plan entry is thorough.

The entry accurately captures scope, exit criteria, and evidence for the blocking causality feature. It correctly notes that decision_digest remains stable and that multi-blocker tests were added.

docs/spec-merkle-commit.md (1)

67-70: Excellent clarification on digest scope.

This note is critical for implementers and future maintainers: the digest commits only to outcomes, not blocker metadata. This preserves hash stability as rejection explanations evolve. Well-documented.

docs/aion-papers-bridge.md (2)

63-63: Paper II mapping is accurate.

The backlog table correctly reflects that tick receipts now implement blocking attribution with the caveat that richer rejection reasons are pending. The touchpoints reference the correct source files.


138-138: Notable gap section correctly describes the blocking witness.

The description accurately captures that blocker indices are in canonical plan order and represent a minimal poset edge list. This aligns with the implementation in engine_impl.rs.

docs/decision-log.md (1)

9-9: Decision log entry is comprehensive.

The entry clearly documents: (1) what changed (blocking-causality witness), (2) why (Paper II alignment, deterministic debugging), (3) how (indices of applied candidates, canonical plan order), and (4) consequences (tools can surface explanations, commit IDs remain stable). Based on learnings, this follows the project convention for recording design decisions.

crates/warp-core/src/receipt.rs (1)

59-75: Rustdoc is excellent — semantics and invariants are crystal clear.

The documentation precisely specifies:

  • Indices are into entries()
  • Sorted, no duplicates, strictly less than idx
  • Empty for Applied, non-empty for Rejected(FootprintConflict)
  • Not included in digest

This is the documentation standard every public API should meet. As per coding guidelines, treating missing docs as a failing test — this passes with honors.

crates/warp-core/src/engine_impl.rs (3)

197-198: Documentation accurately describes the blocking causality witness.

The comment correctly notes this is a "minimal blocking-causality witness / poset edge list, per Paper II". Good alignment with the spec documentation.


392-417: Digest helpers are clean and correctly handle empty cases.

Both compute_plan_digest and compute_rewrites_digest:

  • Return canonical empty digest for empty inputs
  • Use consistent length-prefixed encoding
  • Hash fields in deterministic order

This matches the spec in docs/spec-merkle-commit.md and the test helpers in tick_receipt_tests.rs.


220-223: Good: u32::try_from with proper error propagation.

Unlike the as u64 casts elsewhere, this correctly handles the theoretical case of more than u32::MAX entries. The error message is clear. This is the right pattern.

crates/warp-core/tests/tick_receipt_tests.rs (2)

195-278: Multi-blocker test is thorough and correctly validates the feature.

The test:

  1. Sets up two scopes with distinct nodes
  2. Creates three rules with carefully chosen IDs for deterministic ordering
  3. Applies all three, expecting A and B to succeed, C to be rejected
  4. Verifies C is blocked by exactly [0, 1] (both prior candidates)

This directly validates the PR's core feature: multi-blocker causality tracking. The assertions on lines 271-277 are precise and cover the key invariants.


175-183: Good: existing test updated to verify blocker semantics.

The additions correctly assert:

  • Applied entries have empty blockers
  • Rejected entries have non-empty blockers pointing to the applied candidate(s)

This ensures backwards compatibility while adding coverage for the new feature.

Address CodeRabbit review: enforce receipt invariants in release builds, document scheduler reserve rejection contract, and clarify blocker attribution assumptions.

Refs #147
@flyingrobots
Copy link
Owner Author

Addressed CodeRabbit review items:

  • Enforced TickReceipt parallel-array invariant in all builds (assert_eq).
  • Documented scheduler reserve() rejection contract (false == footprint conflict) across radix/legacy + wrapper.
  • Added explicit attribution comments + complexity note + scheduler conflict cross-reference.
  • Added test helper comments/docstrings to reduce drift risk.

Re-ran: cargo test --workspace; cargo clippy --workspace --all-targets -- -D warnings -D missing_docs.

@flyingrobots flyingrobots merged commit 4d4ccd2 into main Dec 29, 2025
13 checks passed
@flyingrobots flyingrobots deleted the echo/tick-receipt-blockers branch December 29, 2025 15:08
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.

TickReceipt: blocking causality (poset edges)

2 participants