Lower obstruction continuations into Core - #130
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. 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, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds Core IR representation for ChangesCore lowering for require obstruction failure arms
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Source as require ... else source
participant Compiler as edict-syntax compiler
participant CoreIR as CoreNode::Require
participant Canonical as canonical encoder
participant CLI as edict-cli review
Source->>Compiler: parse Stmt::Require (terminal or continue obstructed)
Compiler->>Compiler: validate predicate, reason kind, payload fields
Compiler-->>Compiler: reject duplicate payload fields (DuplicateObstructionPayloadField)
Compiler->>CoreIR: build CoreRequireFailureArm + CoreObstructionReason
CoreIR->>Canonical: encode kind/predicate/onFailure into canonical map
Canonical-->>CLI: canonical bytes consumed by review serialization
CLI->>CLI: emit require/onFailure JSON via core_node_review
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 |
![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
🤖 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/compiler.rs`:
- Around line 816-817: The shared payload handling in check_reason_payload is
incorrectly stripping reason for all record payloads, which causes legal payload
fields to be dropped for terminal arms. Update check_terminal_require_reason so
it preserves reason when lowering terminal records like else domain.Blocked({
reason: ... }), and limit the reason-stripping behavior to the
ContinueObstructedArm path only; use the check_reason_payload and
ContinueObstructedArm symbols to keep the fix scoped correctly.
In `@crates/edict-syntax/tests/compiler_spine.rs`:
- Around line 745-759: `assert_reason` is doing an order-sensitive comparison of
payload keys, even though it should validate the key set regardless of ordering.
Update the helper in `compiler_spine.rs` so `reason.payload` and the expected
`payload_keys` are compared in an order-independent way, using `assert_reason`
as the target and keeping the check focused on key membership rather than
iteration order.
- Around line 646-705: The digest-stability test in
obstruction_reason_mutations_move_core_digest can silently pass if the string
replace does not actually modify CONTINUE_OBSTRUCTED_REQUIRE. Update the
mutation setup for reordered_payload and reformatted to first verify the fixture
contains the expected substring before replacing it, or otherwise assert that
the mutated source differs from the baseline input. Keep the existing baseline
digest checks, but make the mutation step self-defending so the test fails when
the fixture drifts.
🪄 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: df2ad46e-5c98-4009-aff4-74265ac40d52
📒 Files selected for processing (20)
CHANGELOG.mdcrates/edict-cli/src/main.rscrates/edict-syntax/src/canonical.rscrates/edict-syntax/src/compiler.rscrates/edict-syntax/src/core_ir.rscrates/edict-syntax/src/lib.rscrates/edict-syntax/src/target_ir.rscrates/edict-syntax/tests/compiler_spine.rsdocs/abi/edict-core.cddldocs/design/obstruction-strands-v0.mddocs/topics/README.mddocs/topics/compiler-spine/README.mddocs/topics/compiler-spine/test-plan.mddocs/topics/core-ir/README.mddocs/topics/core-ir/canonical-encoding.mddocs/topics/core-ir/test-plan.mddocs/topics/obstruction-strands/README.mddocs/topics/obstruction-strands/test-plan.mddocs/topics/syntax/test-plan.mdfixtures/obstruction-strands/v0/stale-basis/README.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: supply-chain (cargo-deny)
🧰 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/obstruction-strands/v0/stale-basis/README.mddocs/topics/README.mddocs/topics/core-ir/README.mddocs/topics/syntax/test-plan.mdcrates/edict-syntax/src/target_ir.rsdocs/abi/edict-core.cddldocs/design/obstruction-strands-v0.mddocs/topics/core-ir/test-plan.mdcrates/edict-syntax/src/lib.rsCHANGELOG.mddocs/topics/core-ir/canonical-encoding.mddocs/topics/obstruction-strands/README.mddocs/topics/obstruction-strands/test-plan.mdcrates/edict-syntax/src/canonical.rscrates/edict-syntax/src/core_ir.rsdocs/topics/compiler-spine/README.mdcrates/edict-syntax/tests/compiler_spine.rscrates/edict-cli/src/main.rsdocs/topics/compiler-spine/test-plan.mdcrates/edict-syntax/src/compiler.rs
docs/topics/**/{README.md,test-plan.md,architecture.md,rationale.md}
📄 CodeRabbit inference engine (AGENTS.md)
docs/topics/**/{README.md,test-plan.md,architecture.md,rationale.md}: For every nontrivial behavior, contract, workflow, release, schema, validation, or public-surface change, identify the owning topic shelf before editing code; create one if none exists; updatetest-plan.mdbefore or alongside tests with requirement IDs, case IDs, fixtures, and oracles; write executable evidence; update the topicREADME.mdonly after behavior exists; mark planned cases implemented only when executable evidence exists; and runcargo xtask verifybefore claiming the shelf is current.
Do not churn topic shelves for purely mechanical edits that do not change a contract; when a change intentionally does not update a topic shelf, state why in the pull request body or final report.
TopicREADME.mdfiles must not describe intended behavior before it lands;test-plan.mdmay include planned cases and known gaps;policyrows are for human-review workflow contracts and must not be used to avoid writing behavior tests; tests must assert code behavior and stable contract artifacts, not prose; negative tests should assert stable error kinds or structured artifacts, not merelyis_err()or diagnostic text; release, CI, and publication workflows count as behavior when they define a project contract; avoid ceremonial documentation and update shelves because the contract changed, not because a path changed.
Files:
docs/topics/README.mddocs/topics/core-ir/README.mddocs/topics/syntax/test-plan.mddocs/topics/core-ir/test-plan.mddocs/topics/obstruction-strands/README.mddocs/topics/obstruction-strands/test-plan.mddocs/topics/compiler-spine/README.mddocs/topics/compiler-spine/test-plan.md
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
When creating or changing documentation, give each page one primary reader job; keep user-facing task help separate from contributor architecture and evidence maps; use concrete, valid examples and show expected results when the result matters; put exact public facts in reference material and validate or generate them from authoritative sources when practical; and update affected documentation in the same change as behavior, schema, release, workflow, or public-surface changes, or state
docs-impact: nonewith a concise rationale.
Files:
docs/topics/README.mddocs/topics/core-ir/README.mddocs/topics/syntax/test-plan.mddocs/design/obstruction-strands-v0.mddocs/topics/core-ir/test-plan.mddocs/topics/core-ir/canonical-encoding.mddocs/topics/obstruction-strands/README.mddocs/topics/obstruction-strands/test-plan.mddocs/topics/compiler-spine/README.mddocs/topics/compiler-spine/test-plan.md
🔇 Additional comments (24)
crates/edict-cli/src/main.rs (1)
15-19: LGTM!Also applies to: 1091-1091, 1238-1242, 1261-1284
crates/edict-syntax/src/target_ir.rs (1)
411-416: LGTM!CHANGELOG.md (1)
19-26: LGTM!docs/design/obstruction-strands-v0.md (1)
165-180: LGTM!Also applies to: 182-187
docs/topics/README.md (1)
60-62: LGTM!docs/topics/compiler-spine/README.md (1)
41-44: LGTM!Also applies to: 65-70, 85-85
docs/topics/syntax/test-plan.md (1)
106-109: LGTM!fixtures/obstruction-strands/v0/stale-basis/README.md (1)
15-19: LGTM!docs/topics/compiler-spine/test-plan.md (1)
16-16: LGTM!Also applies to: 47-48, 75-76, 93-94
docs/topics/core-ir/README.md (1)
48-52: LGTM!Also applies to: 79-83
docs/topics/core-ir/canonical-encoding.md (1)
56-58: LGTM!Also applies to: 91-99
docs/topics/core-ir/test-plan.md (1)
16-17: LGTM!Also applies to: 50-50, 94-94
docs/topics/obstruction-strands/README.md (1)
4-17: LGTM!Also applies to: 51-65, 76-81
docs/topics/obstruction-strands/test-plan.md (1)
4-19: LGTM!Also applies to: 33-42, 59-63, 76-76
crates/edict-syntax/tests/compiler_spine.rs (5)
10-13: LGTM!
89-111: LGTM!
126-138: LGTM!
600-630: LGTM!
734-743: LGTM!crates/edict-syntax/src/core_ir.rs (1)
222-249: LGTM!crates/edict-syntax/src/lib.rs (1)
120-122: LGTM!docs/abi/edict-core.cddl (1)
239-265: LGTM!crates/edict-syntax/src/compiler.rs (1)
756-766: LGTM!Also applies to: 768-808, 835-847
crates/edict-syntax/src/canonical.rs (1)
743-747: LGTM!Also applies to: 768-798
![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: 1b7e835498
ℹ️ 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 please sanity-check this self-discovered audit item.
|
Activity Summary
Resolved review threads: Local evidence:
Current gate note: remote CI and CodeRabbit restarted for head |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Code Lawyer Re-Audit SummaryCurrent head:
No new self-discovered code issues in this re-audit. Merge judgment: code and CI are clean, but the strict automated check surface still contains the failing CodeRabbit status context. This appears to be an account-credit/status-context failure, not a code failure. |
1 participant
Summary
Lower parsed obstruction-strand
requirearms into explicit Core require nodes.This PR implements the Core slice for obstruction strands:
CoreNode::Require;CoreRequireFailureArm::TerminalandCoreRequireFailureArm::ContinueObstructed;CoreObstructionReasonenvelope with stable reason kind plus canonical payload map;Closes #129
Refs #116
Claims
requiresubset.continue obstructedare Core-distinct.Explicit Non-Goals
RED
cargo test -p edict-syntax --test compiler_spine obstructionObserved failure before implementation: missing
CoreNode::Require,CoreRequireFailureArm,CoreObstructionReason, andDuplicateObstructionPayloadField.GREEN
Docs
Updated the Core ABI CDDL, compiler-spine shelf, Core IR shelf, obstruction-strands shelf, syntax test-plan note, fixture corridor note, design note, topics index, and changelog.