docs(adr): draft ADR-0008 — shell calls join the ^action model - #72
Merged
Merged
Conversation
Decides the design note's §4.5 open question (a/b/c) by dissolving the apparent tension: ADR-0006 already lists the top-level runner and `do` as effect boundaries where actions execute (its own §6 example runs `echo`/`cp` inside a `do` block with no explicit `run`). So "immediate at the top level" isn't a special case bolted onto "always ^action" -- it's what "boundary" already implies once you take ADR-0006's list literally. This ADR gives shell calls zero carve-out: - (vim start) builds an ^action ^t exactly like cp/echo -- no separate immediate-exec primitive, matching LANGUAGE.md's own Phase 2 roadmap. - It runs wherever ADR-0006's boundaries already say any action runs: bare at the top-level runner, inside `do`, via `run`, via `then`, or boundary `;`. Nowhere else. - Defines "top-level" precisely (the design note's own ask): the single expression submitted as one unit to the runner -- not a do-body, not a function body, not an argument position. - Capturing the action via let/set suppresses auto-run even at the top level, per ADR-0006's own `let :copy be cp "a" "b"` example applied literally -- not a new rule for shell calls specifically. Status: Proposed (acceptance = editing the Status line, same convention as ADR-0006/0007). Updates design-note.md's remaining §2/§5/ §6/§7 "ADR 4.5"/"ADR-0008 (planned)" placeholders to real links, and execution-queue.md's epic 007 criteria likewise. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012SbjL7643FUSoVuwCGtkJv
Reviewer's GuideThis documentation-only PR drafts ADR-0008 and resolves shell executable semantics by making them ordinary File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request defines shell executable calls as ChangesShell executable call effects
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Other Merge Risk: ⚪ Minimal · up to The documentation changes are consistent and do not introduce a merge-blocking issue. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Up to standards ✅🟢 Issues
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="specs/006-strictness-effects-spike/design-note.md" line_range="196" />
<code_context>
| `to adder :n do fn do :n + 1 end end` | — | pure (definition) | none | Yes — defines closure, no side effects. |
| Graph `let ^bool :x be -true` | strict | effectful (mutation) | graph commit | Yes — modifies graph binding engine. |
-| `(vim start)` (shell) | strict | effectful | process IO | Effectful is settled; *when* it runs (immediately vs `^action` at a boundary) is **ADR 4.5**. |
+| `(vim start)` (shell) | strict | effectful | process IO | Yes — builds an `^action ^t`, runs at the same boundaries as any action (top-level runner, `do`, …), per [ADR-0008](../decisions/ADR-0008-shell-executable-calls.md). |
| `` `[ a ~:b _:c d ] `` (syntax-quote) | — | pure (compile-time) | none | Yes — macro template, no runtime effect. |
| `map %{ a->1, b->2 }` | strict | pure | none | Yes — literal constructor, pure per contract. |
</code_context>
<issue_to_address>
**issue:** The §6 verification table classifies `(vim start)` as `effectful`, while the new ADR and the preceding taxonomy state that constructing it is pure and only running its returned action is effectful. This contradiction can lead implementers to execute the shell command during expression construction instead of producing an unrun `^action` value.
**Suggested fix:** Change the table's purity entry to `pure (builds action)` and describe process IO as occurring only when the action reaches an effect boundary.
```suggestion
| `(vim start)` (shell) | strict | pure (builds action) | process IO only when the action reaches an effect boundary | Yes — builds an `^action ^t`, runs at the same boundaries as any action (top-level runner, `do`, …), per [ADR-0008](../decisions/ADR-0008-shell-executable-calls.md). |
```
</issue_to_address>
### Comment 2
<location path="specs/decisions/ADR-0008-shell-executable-calls.md" line_range="149-151" />
<code_context>
+- Epic 007 (IK1 core language) implements shell-mode form resolution
+ against this model: build an `^action ^t`, run only at the named
+ boundaries.
+- Update [`specs/execution-queue.md`](../execution-queue.md) epic 007 start
+ criteria to drop "ADR-0008 (planned)" in favor of a direct reference,
+ once accepted.
</code_context>
<issue_to_address>
**nitpick:** The ADR's Follow-ups says the execution queue still needs to be updated once ADR-0008 is accepted, but this diff has already changed that queue to the direct ADR link while the ADR remains Proposed. The follow-up is stale and leaves an already-completed documentation task appearing unfinished.
**Suggested fix:** Remove this follow-up, or change it to state that the execution-queue reference has already been updated and only the ADR Status line remains to be changed on acceptance.
```suggestion
```
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: specs/006-strictness-effects-spike/design-note.md:196
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- design-note.md §6's (vim start) row still said Purity: effectful / Effect: process IO, contradicting the row's own Notes column (and ADR-0008) which say building the action is pure and only running it performs IO. Fixed to `pure (builds action)` / `process IO only when the action reaches an effect boundary`, matching sourcery's suggestion and the §2 row already updated correctly in this same PR. - ADR-0008's Follow-ups told the reader to update execution-queue.md "once accepted", but that update already happened in this PR's own diff -- a completed task described as pending. Reworded to say so directly; only the Status line itself remains for acceptance. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012SbjL7643FUSoVuwCGtkJv
§3 grouped `set` with `let` as an action-capturing form that "suppresses auto-run," but ADR-0007 (decided alongside this one) classifies `set` as an immediate evaluation-time mutation effect, explicitly exempt from the ^action pattern with no build-then-run split at all. ADR-0006's only worked example (let :copy be cp "a" "b") is let-only, so claiming the suppression mechanism extends to set adds something the ADRs don't actually establish and contradicts ADR-0007's own framing. Rescoped §3 to let (the actual evidence) and added a note that set's effect status comes entirely from ADR-0007's mutation classification, independent of whatever its RHS evaluates to -- not a parallel capturing mechanism. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012SbjL7643FUSoVuwCGtkJv
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Drafts ADR-0008, deciding the design note's §4.5 open question — your
pick, option (c) (immediate at the top level only) — by showing it isn't
actually a separate mechanism from option (b):
doas effectboundaries where actions execute (its own §6 example runs
echo/cpinside a
doblock with no explicitrun). So "immediate at the toplevel" is what "boundary" already implies once you take that list
literally — not a carve-out bolted onto "always
^action".(vim start)builds an^action ^texactly likecp/echo— noseparate immediate-exec primitive, matching
LANGUAGE.md's own Phase 2roadmap ("port shell/file/network built-ins to return
^action ^t").at the top-level runner, inside
do, viarun, viathen, or boundary;. Nowhere else.expression submitted as one unit to the runner — not a
do-body, not afunction body, not an argument position.
let/setsuppresses auto-run even at the toplevel, per ADR-0006's own
let :copy be cp "a" "b"example appliedliterally — not a new rule invented for shell calls.
Status: Proposed — acceptance = editing the Status line, same
convention as ADR-0006/0007.
Also updates design-note.md's remaining §2/§5/§6/§7 "ADR 4.5" / "ADR-0008
(planned)" placeholders to real links, and
execution-queue.md's epic 007criteria likewise.
With this, all three ADRs the epic 006 design note flagged (§4.1–4.3,
§4.4, §4.5) are drafted. Remaining on the epic 007 unblock path: the IK1
grammar ADR (
fn/cond/repeat), then epic 008.Summary by Sourcery
Adopt the existing
^actioneffect model for shell executable calls and document the decision in ADR-0008.Enhancements:
^action ^tvalues that execute only at the existing effect boundaries, including the top-level runner, without a shell-specific immediate-execution exception.Documentation:
Summary by cubic
Drafts ADR-0008, resolving the design note's §4.5 open question: shell-mode executable calls like
(vim start)build an^action ^texactly likecp/echoand run only at ADR-0006's existing effect boundaries, with no immediate-execution carve-out. Replaces the remaining "ADR 4.5" / "ADR-0008 (planned)" placeholders indesign-note.mdandexecution-queue.mdwith links to the new ADR.doare already effect boundaries.dobody, function body, or argument position.letcapturing suppresses auto-run even at top level, per ADR-0006's existing example;setis not an action-capturing form and stays governed by ADR-0007's mutation classification.(vim start)row now says construction is pure, matching its notes column; the ADR's follow-up no longer lists the execution-queue update as pending.Written for commit 597d948. Summary will update on new commits.
Summary by CodeRabbit
run,do,then,let, andset.