Freeze canonical Target IR artifacts - #108
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)**⚙️ CodeRabbit configuration file
Files:
docs/topics/**/test-plan.md📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (4)
Summary by CodeRabbit
WalkthroughAdds canonical CBOR encoding and digest framing for TargetIrArtifact, reviewed golden fixtures for Echo and git-warp, an xtask golden check/write command wired into ChangesCanonical Target IR Encoding, Digests, Goldens, and Bundle Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
@codex review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 827c0c5276
ℹ️ 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".
|
@codex self-audit found additional issue before fixes:
|
|
Summary
Testing
|
Code Lawyer Activity Summary
Verification:
|
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/src/contract_bundle.rs`:
- Around line 527-531: Validate target_ir_artifact.target_profile before calling
digest_target_ir_artifact, so ContractBundleAssemblyError reports the stable
InvalidDigest failure for target_ir_artifact.target_profile instead of a generic
canonical-digest error on target_ir_artifact. Reorder the logic in the
contract_bundle assembly path to invoke target_profile_from_target_ir_artifact
first, then hash only after that reference is known to be valid, preserving the
existing error conversion and symbol names used in this flow.
In `@docs/topics/target-ir/test-plan.md`:
- Around line 77-83: The test-plan rows for TIR-TP-018 and TIR-TP-019 are marked
implemented without matching executable evidence in the PR. Update the rows in
the test-plan and the related coverage in crates/edict-syntax/tests/target_ir.rs
so they only claim behavior that is actually covered, or add the missing
RED/GREEN tests (especially step-order digest sensitivity) and then keep the
rows as implemented. Use the existing target_ir_* test names and the
TIR-TP-018/TIR-TP-019 entries to align the wording with the real evidence before
merge.
🪄 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
Run ID: cdb633cf-b5c5-4faf-a86d-668568cfea92
📒 Files selected for processing (17)
CHANGELOG.mdcrates/edict-syntax/src/canonical.rscrates/edict-syntax/src/contract_bundle.rscrates/edict-syntax/src/lib.rscrates/edict-syntax/tests/contract_bundle.rscrates/edict-syntax/tests/target_ir.rsdocs/design/canonical-target-ir-v0.11.mddocs/design/contract-bundle-assembly-v0.11.mddocs/topics/contract-bundles/README.mddocs/topics/contract-bundles/test-plan.mddocs/topics/target-ir/README.mddocs/topics/target-ir/test-plan.mdfixtures/target-ir/canonical/echo-effectful.target-ir.cborfixtures/target-ir/canonical/echo-effectful.target-ir.sha256fixtures/target-ir/canonical/gitwarp-append.target-ir.cborfixtures/target-ir/canonical/gitwarp-append.target-ir.sha256xtask/src/main.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**
⚙️ CodeRabbit configuration file
**: # AGENTSGit Rules
NEVER amend git commits. Make a new commit instead.
NEVER use
git rebaseunless the user explicitly approves a rare exception.
Use regular merge commits.NEVER force any git operation. If a force operation appears necessary, stop and
explain what happened and what options remain.NEVER create draft pull requests.
NEVER use a
codexprefix in branch names, PR titles, or commit messages.Pull request bodies for issue work MUST include GitHub auto-close text such as
Closes#123`` for every issue the PR is intended to close.Think
Think is durable memory for cross-session coordination.
- Use
codex-think --remember --jsonwhen starting a new session, changing into
this repository, or regaining context after a context shift.- Use
codex-think "..." --jsonwhen a cycle closes or a significant event
should survive across turns.- Treat Think as memory, not repo truth. Anchor strong claims back to files,
commits, commands, issues, or pull requests.- Claude memories are read-only. Use
claude-think --remember --jsononly for
additional context.Topic Shelves
docs/topics/contains the living contract graph for landed behavior. Topic
shelves are not proposals, retrospectives, or design archaeology.Each shelf may contain:
README.md: what is true in HEAD.test-plan.md: how those truths are verified, including requirements, cases,
fixtures, oracles, implemented evidence, planned cases, and known gaps.architecture.md: optional structure or dataflow notes when the machinery
earns a separate page.rationale.md: optional still-relevant tradeoffs and rejected approaches.When To Update Topic Shelves
For every nontrivial behavior, contract, workflow, release, schema, validation,
or public-surface change:
- Identify the owning topic shelf before editing code.
- If no shelf owns durable behavior, create one.
- Update
test-plan.mdbefore or alongside tests with requirement IDs, case
IDs,...
Files:
fixtures/target-ir/canonical/gitwarp-append.target-ir.sha256fixtures/target-ir/canonical/echo-effectful.target-ir.sha256docs/design/canonical-target-ir-v0.11.mdCHANGELOG.mddocs/design/contract-bundle-assembly-v0.11.mddocs/topics/target-ir/test-plan.mdcrates/edict-syntax/src/contract_bundle.rsxtask/src/main.rscrates/edict-syntax/tests/contract_bundle.rscrates/edict-syntax/src/lib.rsdocs/topics/contract-bundles/README.mddocs/topics/target-ir/README.mddocs/topics/contract-bundles/test-plan.mdcrates/edict-syntax/src/canonical.rscrates/edict-syntax/tests/target_ir.rs
docs/topics/**/test-plan.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/topics/**/test-plan.md: Update a topic shelftest-plan.mdbefore or alongside tests with requirement IDs, case IDs, fixtures, oracles, implemented evidence, planned cases, and known gaps.
Mark planned cases as implemented only when executable evidence exists.
Do not usepolicyrows in topic-shelf test plans to avoid writing behavior tests for software behavior.
Negative tests in topic-shelf test plans should assert stable error kinds or structured artifacts, not merelyis_err()or diagnostic text.
For nontrivial behavior, contract, workflow, release, schema, validation, or public-surface changes, update the owning topictest-plan.mdwith planned requirement and case rows before or alongside the first test.
Mark planned rows as implemented only after executable evidence exists.
Files:
docs/topics/target-ir/test-plan.mddocs/topics/contract-bundles/test-plan.md
docs/topics/**/README.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/topics/**/README.md: Topic shelfREADME.mdfiles must describe only what is true in HEAD.
Update the topic shelfREADME.mdonly after the behavior exists in the branch; do not describe intended future behavior there.
Do not churn topic-shelf README files for purely mechanical edits that do not change a contract.
Files:
docs/topics/contract-bundles/README.mddocs/topics/target-ir/README.md
🔇 Additional comments (10)
CHANGELOG.md (1)
38-47: LGTM!docs/design/canonical-target-ir-v0.11.md (1)
1-103: LGTM!docs/design/contract-bundle-assembly-v0.11.md (1)
15-16: LGTM!Also applies to: 29-39, 88-109, 125-125, 142-146, 156-157
docs/topics/contract-bundles/README.md (1)
9-11: LGTM!Also applies to: 22-25, 68-77, 104-105
crates/edict-syntax/src/canonical.rs (1)
1-6: LGTM!Also applies to: 19-19, 30-32, 183-219, 451-540
crates/edict-syntax/src/lib.rs (1)
34-37: LGTM!Also applies to: 96-112
xtask/src/main.rs (1)
12-17: LGTM!Also applies to: 56-72, 98-98, 248-434, 1107-1140
fixtures/target-ir/canonical/echo-effectful.target-ir.sha256 (1)
1-1: LGTM!fixtures/target-ir/canonical/gitwarp-append.target-ir.sha256 (1)
1-1: LGTM!crates/edict-syntax/tests/target_ir.rs (1)
7-18: LGTM!Also applies to: 81-86, 902-1122
Code Lawyer Activity Summary
Self-audit: Verification:
|
Summary
Closes #105.
This freezes canonical Target IR artifact bytes and digests for the current Echo and git-warp Target IR envelope without expanding into runtime execution, verifier reports, general target plugin dispatch, admission execution, or canonical
ContractBundleManifestbytes.edict.target-ir.artifact/v1canonical value/digest frame and publicencode_target_ir_artifact/digest_target_ir_artifactAPIs.fixtures/target-ir/canonical/pluscargo xtask target-ir-goldens --check/--write, wired intocargo xtask verify.assemble_contract_bundle_from_target_ir(...)so bundle assembly can computetargetIrDigestfrom a realTargetIrArtifactwithout a caller-supplied target IR digest field.Scope Boundaries
ContractBundleManifestbytes.RED
cargo test -p edict-syntax target_ir_artifactfailed becausedigest_target_ir_artifact,encode_target_ir_artifact, andTARGET_IR_ARTIFACT_DIGEST_DOMAINdid not exist.cargo xtask target-ir-goldens --checkfailed first because the command was unknown, then failed because the Target IR golden fixtures were absent.cargo test -p edict-syntax assembled_bundle_from_real_target_ir_computes_target_ir_digestfailed becauseassemble_contract_bundle_from_target_irandContractBundleAssemblyFromTargetIrInputdid not exist.GREEN
cargo test -p edict-syntax target_ircargo test -p edict-syntax contract_bundle_assemblycargo test -p xtask target_ir_goldens_match_executable_encodercargo xtask target-ir-goldens --checkcargo xtask contract-checkcargo xtask verifyGolden Digests
sha256:f5a81ddfb84a704c180c9c0e03210445da1873bea397edc8ac683738710ed2b7sha256:032dba1b21bc18970ccfb05141d9ad1fd2f1ab94b6de0cd6e853c04e76eeb116