Compile basis-bound validated patch requests - #179
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@coderabbitai review |
|
@codex please review |
✅ Action performedReview finished.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary by CodeRabbit
WalkthroughAdds the ChangesValidated workspace patch
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Generator
participant Compiler
participant CoreIR
participant TargetIR
participant Echo
Generator->>Compiler: compile applyValidated source
Compiler->>CoreIR: produce compiler-owned request data
Compiler->>TargetIR: lower one external-action request
TargetIR-->>Echo: provide basis-bound validated patch request
Echo-->>Echo: execute and settle request
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/edict-syntax/tests/external_action_requests.rs`:
- Around line 264-297: Extend
validated_patch_request_compiles_as_non_callable_data to assert the complete
ExternalActionRequest closure in both Core and Target IR: verify exact
digest-locked operation, input, settlement, and reconciliation resource
references, plus the structured input, authority, basis, and budget expressions
using the expected artifact fields and values. Keep the existing operation/count
assertions and ensure the test fails if lowering drops or substitutes any
EXTREQ-TP-019 request data.
In `@docs/topics/external-action-requests/README.md`:
- Around line 110-122: Update the documentation for
workspace.patch.applyValidated@1 to describe the application source as carrying
the authority value/scope, not an authority digest; retain “digest” only for
resource references where that contract applies, and keep the remaining typed
request-expression description unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2d49a75e-4a76-4e5c-b2ec-24900c2f22d2
📒 Files selected for processing (19)
CHANGELOG.mdcrates/edict-syntax/tests/external_action_requests.rsdocs/topics/external-action-requests/README.mddocs/topics/external-action-requests/test-plan.mdfixtures/lawpack/workspace-patch/README.mdfixtures/lawpack/workspace-patch/adapter.cborfixtures/lawpack/workspace-patch/adapter.sha256fixtures/lawpack/workspace-patch/apply-validated-patch.core.cborfixtures/lawpack/workspace-patch/apply-validated-patch.core.sha256fixtures/lawpack/workspace-patch/apply-validated-patch.edictfixtures/lawpack/workspace-patch/apply-validated-patch.target-ir.cborfixtures/lawpack/workspace-patch/apply-validated-patch.target-ir.sha256fixtures/lawpack/workspace-patch/exports.cborfixtures/lawpack/workspace-patch/exports.sha256fixtures/lawpack/workspace-patch/manifest.cborfixtures/lawpack/workspace-patch/manifest.sha256fixtures/lawpack/workspace-patch/request-profile-configuration.cborfixtures/lawpack/workspace-patch/request-profile-configuration.sha256xtask/src/lawpack_goldens.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: rust msrv 1.94.0 (fmt · clippy · test)
- GitHub Check: rust stable (fmt · clippy · test)
- GitHub Check: supply-chain (cargo-deny)
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Never amend Git commits, usegit rebasewithout explicit user approval, or force any Git operation; use new commits and regular merge commits instead.
Do not create draft pull requests, and never use acodexprefix in branch names, pull request titles, or commit messages.
Pull requests for issue work must include GitHub auto-close text such asCloses#123`` for every issue they intend to close.
Usecodex-think --remember --jsonwhen starting a session, entering the repository, or regaining context, and record significant durable events with `codex-think "..." --json`. Treat Think as memory rather than repository truth.
Every pull request body must contain `## Plain-English Walkthrough` with `### TL;DR` and `### Walkthrough`, explaining the prior behavior, new model and dataflow, invariants, failures, compatibility, and verification as applicable.
Use Mermaid diagrams for nontrivial flow, lifecycle, ownership, or component interaction when clearer than prose; every diagram requires an introductory paragraph, the diagram, the exact collapsed caption structure, and a concluding interpretation.
Tag each material technical claim at first occurrence as `[claim:, confidence:]`, cite evidence using repository-relative paths, line numbers, and Git SHAs, and end the explanatory body with a collapsed citations appendix.
If CodeRabbit is actively reviewing, obtain its approval before merge; if unavailable due to limits or credits, request `@codex review please` and wait for the alternate response. Do not treat unavailability as approval unless a maintainer explicitly overrides the gate.
For release preparation, write the release thesis first, reconcile changes from the previous tag, update release policy and tests, verify the milestone has no open issues and no unauthorized crates.io publication occurred, and record a durable release report.
Run `cargo xtask verify` before claiming a branch is ready.
Files:
fixtures/lawpack/workspace-patch/manifest.sha256fixtures/lawpack/workspace-patch/exports.sha256fixtures/lawpack/workspace-patch/exports.cborfixtures/lawpack/workspace-patch/request-profile-configuration.cborfixtures/lawpack/workspace-patch/manifest.cborfixtures/lawpack/workspace-patch/request-profile-configuration.sha256fixtures/lawpack/workspace-patch/apply-validated-patch.core.cborfixtures/lawpack/workspace-patch/adapter.sha256fixtures/lawpack/workspace-patch/apply-validated-patch.edictfixtures/lawpack/workspace-patch/apply-validated-patch.core.sha256fixtures/lawpack/workspace-patch/README.mdfixtures/lawpack/workspace-patch/apply-validated-patch.target-ir.sha256fixtures/lawpack/workspace-patch/apply-validated-patch.target-ir.cborCHANGELOG.mddocs/topics/external-action-requests/README.mddocs/topics/external-action-requests/test-plan.mdfixtures/lawpack/workspace-patch/adapter.cborxtask/src/lawpack_goldens.rscrates/edict-syntax/tests/external_action_requests.rs
**/*.{rs,md}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,md}: Tests must assert software behavior and stable error kinds or structured artifacts, not implementation details, prose, paths, or merelyis_err(); documentation-tool tests may test validator behavior.
For nontrivial behavior, contract, workflow, release, schema, validation, or public-surface changes, follow RED/GREEN TDD: update the owningtest-plan.md, write the deterministic test first, observe the RED failure, implement the smallest coherent fix, then mark the case implemented only after executable evidence exists.
Files:
fixtures/lawpack/workspace-patch/README.mdCHANGELOG.mddocs/topics/external-action-requests/README.mddocs/topics/external-action-requests/test-plan.mdxtask/src/lawpack_goldens.rscrates/edict-syntax/tests/external_action_requests.rs
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{md,mdx}: Documentation pages must have one primary reader job, separate user task help from contributor architecture and evidence maps, use concrete valid examples with expected results when relevant, and keep exact public facts in validated or generated reference material.
Update affected documentation in the same change as behavior, schema, release, workflow, or public-surface changes, or statedocs-impact: nonewith a concise rationale.
Files:
fixtures/lawpack/workspace-patch/README.mdCHANGELOG.mddocs/topics/external-action-requests/README.mddocs/topics/external-action-requests/test-plan.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Use tables for consistent-shape comparisons and evidence, bullets for unordered sets, numbered lists for ordered procedures or states, and focused branch-accurate snippets for exact syntax.
Files:
fixtures/lawpack/workspace-patch/README.mdCHANGELOG.mddocs/topics/external-action-requests/README.mddocs/topics/external-action-requests/test-plan.md
docs/topics/**
📄 CodeRabbit inference engine (AGENTS.md)
docs/topics/**: Topic shelves document landed behavior:README.mddescribes current HEAD truth,test-plan.mdrecords verification and known gaps, and optional architecture or rationale pages contain durable supporting information.
For every nontrivial behavior, contract, workflow, release, schema, validation, or public-surface change, identify or create the owning topic shelf, updatetest-plan.md, add executable evidence, updateREADME.mdonly after behavior exists, and runcargo xtask verify.
Do not update topic shelves for purely mechanical edits that do not change a contract; explain the omission in the pull request or final report.
Files:
docs/topics/external-action-requests/README.mddocs/topics/external-action-requests/test-plan.md
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: For Rust changes, preserve claim integrity by providing executable evidence, keep compiler and validation paths deterministic and free of hidden I/O, and prefer structured public failures with stable error kinds over prose-only diagnostics.
Do not add Rust dependencies without pull-request rationale and contract-impact notes; treat planned lint, dependency, and fuzzing ratchets as planned until executable checks land.
Files:
xtask/src/lawpack_goldens.rscrates/edict-syntax/tests/external_action_requests.rs
🔇 Additional comments (19)
crates/edict-syntax/tests/external_action_requests.rs (1)
52-160: LGTM!Also applies to: 464-624
fixtures/lawpack/workspace-patch/apply-validated-patch.edict (1)
1-34: LGTM!xtask/src/lawpack_goldens.rs (1)
91-137: LGTM!Also applies to: 434-725
fixtures/lawpack/workspace-patch/adapter.cbor (1)
1-1: LGTM!fixtures/lawpack/workspace-patch/adapter.sha256 (1)
1-1: LGTM!fixtures/lawpack/workspace-patch/manifest.cbor (1)
1-1: LGTM!docs/topics/external-action-requests/README.md (1)
137-137: LGTM!docs/topics/external-action-requests/test-plan.md (1)
40-51: LGTM!Also applies to: 95-95
fixtures/lawpack/workspace-patch/README.md (1)
1-35: LGTM!CHANGELOG.md (1)
28-32: LGTM!fixtures/lawpack/workspace-patch/manifest.sha256 (1)
1-1: LGTM!fixtures/lawpack/workspace-patch/request-profile-configuration.cbor (1)
1-1: LGTM!fixtures/lawpack/workspace-patch/request-profile-configuration.sha256 (1)
1-1: LGTM!fixtures/lawpack/workspace-patch/exports.cbor (1)
1-1: LGTM!fixtures/lawpack/workspace-patch/exports.sha256 (1)
1-1: LGTM!fixtures/lawpack/workspace-patch/apply-validated-patch.core.cbor (1)
1-4: LGTM!fixtures/lawpack/workspace-patch/apply-validated-patch.core.sha256 (1)
1-1: LGTM!fixtures/lawpack/workspace-patch/apply-validated-patch.target-ir.cbor (1)
1-4: LGTM!fixtures/lawpack/workspace-patch/apply-validated-patch.target-ir.sha256 (1)
1-1: LGTM!
Plain-English Walkthrough
TL;DR
[claim:request-boundary, confidence:1.00] Edict now compiles
workspace.patch.applyValidated@1as typed, non-callable request data. The request binds patch input, authority, workspace basis, budgets, settlement schema, and reconciliation identity without granting filesystem-write authority.[claim:artifact-ownership, confidence:1.00] The owning lawpack generator publishes the complete canonical closure: source, manifest, exports, adapter, target configuration, Core, Target IR, and digest sidecars.
[claim:runtime-boundary, confidence:1.00] Dynamic patch, path-aperture, basis, and policy-instance admission remains an Echo responsibility. This change performs no patch and introduces no ambient filesystem, shell, Git, network, or model authority.
Walkthrough
The compiler path admits a second concrete external-request family. The focused syntax test proves the exact operation, schemas, dynamic authority values, basis, budgets, and reconciliation law survive Core and Target lowering while callable Target steps remain empty. [claim:typed-request, confidence:1.00] See
crates/edict-syntax/tests/external_action_requests.rs#276@1130df16e08daa5143e4be2a5c05c00a9c7fefe7.The generator builds the closure from canonical lawpack resources, compiles the digest-pinned source, lowers it, and refuses output unless it contains exactly one external request and zero callable steps. [claim:generated-closure, confidence:1.00] See
xtask/src/lawpack_goldens.rs#434@1130df16e08daa5143e4be2a5c05c00a9c7fefe7.The target configuration names the authority, basis, patch, forbidden-path, postcondition, and reconciliation classes that Echo must interpret during admission and execution. [claim:policy-binding, confidence:0.99] See
xtask/src/lawpack_goldens.rs#632@1130df16e08daa5143e4be2a5c05c00a9c7fefe7.The source remains declarative request construction. [claim:no-write-authority, confidence:1.00] See
xtask/src/lawpack_goldens.rs#686@1130df16e08daa5143e4be2a5c05c00a9c7fefe7.The composed ownership boundary is:
flowchart LR S[Edict source] --> C[Canonical Core request] C --> T[Canonical Target request] T --> V[Independent Echo derivation check] V --> A[Echo admission and adapter execution] A --> W[Durable settlement]Caption: Compiler-owned request construction stops before world mutation.
Edict owns the request value and its exact derivation. Echo owns live value admission, durable execution, and settlement. The external adapter alone owns filesystem authority.
The diagram’s boundary is normative: compilation never invokes the adapter, and Echo must not infer a provisional request format.
Verification
cargo xtask lawpack-goldensrejected the absentfixtures/lawpack/workspace-patch/manifest.cbor.cargo test -p edict-syntax --test external_action_requestspassed 15/15 tests.cargo xtask lawpack-goldens --writefollowed bycargo xtask lawpack-goldensreproduced the checked closure.cargo xtask verifypassed formatting, clippy with warnings denied, the all-features workspace suite, doctests, canonical golden checks, provider contract checks, the runtime dependency boundary, 25 topic shelves, andgit diff --check.tests::contract_graph_is_validnow accepts the executable evidence and exact fixture path.Dependency rationale
No dependency was added. Existing compiler, canonical encoding, lawpack, and Target IR surfaces own the implementation.
Documentation impact
The external-action topic now records
EXTREQ-REQ-010, casesEXTREQ-TP-019throughEXTREQ-TP-023, fixture ownership, determinism obligations, and the remaining Echo execution gap. Seedocs/topics/external-action-requests/test-plan.md#40@1130df16e08daa5143e4be2a5c05c00a9c7fefe7anddocs/topics/external-action-requests/README.md#110@1130df16e08daa5143e4be2a5c05c00a9c7fefe7.Appendix: Source citations
crates/edict-syntax/tests/external_action_requests.rs#276@1130df16e08daa5143e4be2a5c05c00a9c7fefe7xtask/src/lawpack_goldens.rs#434@1130df16e08daa5143e4be2a5c05c00a9c7fefe7xtask/src/lawpack_goldens.rs#632@1130df16e08daa5143e4be2a5c05c00a9c7fefe7xtask/src/lawpack_goldens.rs#686@1130df16e08daa5143e4be2a5c05c00a9c7fefe7fixtures/lawpack/workspace-patch/README.md#1@1130df16e08daa5143e4be2a5c05c00a9c7fefe7docs/topics/external-action-requests/test-plan.md#40@1130df16e08daa5143e4be2a5c05c00a9c7fefe7Closes #178