docs: add dag loop - #2390
Conversation
Resolve ambiguities that blocked unambiguous conformance tests: - define an iteration's terminal status as the DAG-run status the iteration's step statuses would produce - reject empty while/until condition arrays - define backoff: false as equivalent to omitted backoff - keep Spec 007 dependency-ordering for step-output references across iterations; unordered earlier-iteration data flows only through the feedback file, loop environment, and filesystem - treat waiting states as non-terminal for the continuation decision - guarantee feedback file content: per-entry values, met/not-met result tokens, and a tail cap for value-match actual values
📝 WalkthroughWalkthroughAdds a complete DAG Loop specification covering configuration, bounded iteration behavior, continuation conditions, state isolation, failure handling, composition, validation, and examples. The specification is listed in the conformance table as not implemented. ChangesDAG Loop specification
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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 `@specs/024-dag-loop.md`:
- Around line 141-150: Update the loop configuration rules around loop.backoff
and loop.max_interval_sec so backoff: false and an omitted backoff validate
identically. Define presence based on the effective backoff value, or apply the
same max_interval_sec allowance or rejection to both cases, while preserving the
documented equivalence of backoff: false and omission.
- Around line 663-671: Update the expected behavior in the harness example so
its assertions match what record-agent.sh records: either add separate logging
for the relevant loop environment variables and assert them there, or remove the
claim that the invocations differ only by environment. Keep the existing
argument-vector assertions unchanged.
- Around line 318-328: Update the feedback-file requirements in the loop
condition reporting section to bound total output size, not just per-entry
command output and actual values. Define a deterministic aggregate byte limit
and truncation behavior covering expected values and all entries, or explicitly
enforce a maximum condition-list length; preserve source order and the existing
met/not met markers.
🪄 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: CHILL
Plan: Pro
Run ID: 846ac582-5493-480e-93ef-d1ca5c4e7615
📒 Files selected for processing (2)
specs/024-dag-loop.mdspecs/README.md
| - `loop.backoff` accepts `true`, which means `2.0`, or a number greater | ||
| than `1.0`. | ||
|
|
||
| - `loop.backoff: false` is valid and has the same behavior as an omitted | ||
| `backoff`. | ||
|
|
||
| - `loop.max_interval_sec` is optional. | ||
|
|
||
| - `loop.max_interval_sec` is valid only when `loop.backoff` is present. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make backoff: false consistent with omission.
backoff: false is declared equivalent to an omitted backoff, but max_interval_sec is valid only when backoff is present. Therefore, equivalent configurations validate differently. Define “present” as an effective backoff, or explicitly allow/reject max_interval_sec in both cases.
🧰 Tools
🪛 LanguageTool
[style] ~149-~149: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...is optional. - loop.max_interval_sec is valid only when loop.backoff is prese...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for 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.
In `@specs/024-dag-loop.md` around lines 141 - 150, Update the loop configuration
rules around loop.backoff and loop.max_interval_sec so backoff: false and an
omitted backoff validate identically. Define presence based on the effective
backoff value, or apply the same max_interval_sec allowance or rejection to both
cases, while preserving the documented equivalence of backoff: false and
omission.
| - For each loop condition entry, in source order, the feedback file | ||
| includes the entry's captured combined stdout and stderr for command | ||
| checks, or the entry's actual value and expected value for value | ||
| matches. | ||
|
|
||
| - For each loop condition entry, the feedback file marks the entry's | ||
| result with the token `met` or `not met`. | ||
|
|
||
| - Per entry, at most the last 64 KiB of captured command-check output is | ||
| retained, and at most the last 64 KiB of a value-match actual value is | ||
| retained. |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Bound the total feedback file size.
Only command output and actual values are capped per entry; expected values and the aggregate size are unbounded, and condition-list length has no stated maximum. A workflow with many entries can therefore create arbitrarily large feedback files. Define a total byte limit with deterministic truncation, or bound the number of entries.
🤖 Prompt for 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.
In `@specs/024-dag-loop.md` around lines 318 - 328, Update the feedback-file
requirements in the loop condition reporting section to bound total output size,
not just per-entry command output and actual values. Define a deterministic
aggregate byte limit and truncation behavior covering expected values and all
entries, or explicitly enforce a maximum condition-list length; preserve source
order and the existing met/not met markers.
| With `record-agent.sh` appending its argument vector as one line to | ||
| `invocations.log` and exiting `0`: | ||
|
|
||
| Expected behavior: | ||
|
|
||
| - `invocations.log` contains two lines. | ||
| - The second line is byte-identical to the first line. | ||
| - Neither line contains a session-resume or continuation argument. | ||
| - The two invocations differ only in their loop environment variable |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the harness example’s observable assertion consistent.
The recorder logs only the argument vector, while the example says the invocations differ only in loop environment variables. Environment changes cannot be verified from that log. Log the relevant environment values separately, or remove that assertion.
🤖 Prompt for 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.
In `@specs/024-dag-loop.md` around lines 663 - 671, Update the expected behavior
in the harness example so its assertions match what record-agent.sh records:
either add separate logging for the relevant loop environment variables and
assert them there, or remove the claim that the invocations differ only by
environment. Keep the existing argument-vector assertions unchanged.
- cap loop condition lists at 100 entries and extend the 64 KiB tail cap to every reported feedback value, so the total feedback file size has a deterministic bound - require an enabled backoff (true or a number) for max_interval_sec so that backoff: false validates the same as an omitted backoff - drop an assertion about environment differences from the harness recorder example that its log cannot observe
- delegate feedback-file transport for remote and containerized executors to executor specs, mirroring the step output file contract - classify DAGU_LOOP_* names as execution-attempt Dagu-managed values and exclude them from the environment passed to child DAG runs - report feedback result tokens after negation, matching list passage - persist singular output variables across iterations like published step outputs - bound interval fields, require max_interval_sec with enabled backoff, and require saturating wait computation
1 participant
Summary
Adds Spec 024: DAG Loop — a normative contract for a new root
loopfield that repeats a whole DAG step body until a condition is reached. Status is Not implemented (target conformance behavior per specs/README.md), registered in the status table.The primary use case is a convergence loop: steps attempt work, a deterministic check decides whether the goal is reached, and the next iteration can observe why the previous check did not pass. This supports agent-style goal loops while keeping every loop bounded by construction.
Design
while/until, using the Spec 023 condition-entry shape (string shortcut,condition/eval,expected,negate, AND list). Empty arrays are invalid.max_iterationsis required (1–1000). Unbounded repetition belongs to the scheduler, not one DAG run.on_exhausted: fail(default) makes an unmet goal a failed run even when every iteration succeeded;succeedopts out.DAGU_LOOP_ITERATION,DAGU_LOOP_MAX_ITERATIONS,DAGU_LOOP_FEEDBACK_FILE. The feedback file is replaced at each continuation decision and carries per-entry results and captured verifier output with a 64 KiB tail cap.max_iterations.Commits
spec: dag loop— initial spec and README status row.spec: tighten dag loop contracts for conformance— resolves ambiguities found in a conformance-readability pass: iteration terminal status definition, empty condition arrays,backoff: false, cross-iteration reference ordering, waiting states, feedback file content guarantees.Nine examples are written to double as black-box fixtures for
conformance/dag_loop/testdata.Summary by cubic
Add Spec 024: DAG Loop — defines a root
loopthat repeats a whole DAG until a condition is met, with strict bounds and clear conformance. Also adds a full conformance test suite and harness support for start/interrupt; status is Not implemented and registered inspecs/README.md.New Features
loopwithwhile/until, requiredmax_iterations(1–1000),interval_sec, optionalbackoff(true or >1) with requiredmax_interval_sec, andon_exhausted(faildefault orsucceed).DAGU_LOOP_ITERATION,DAGU_LOOP_MAX_ITERATIONS,DAGU_LOOP_FEEDBACK_FILE; stable feedback file carries per-entry values and met/not met tokens with a 64 KiB tail cap; executor specs handle transport; loop env is attempt-scoped and not passed to child DAGs.Conformance
outputvars persist with Spec 007 ordering; latest published values are used; filesystem is not reset; empty condition arrays invalid; lists capped at 100; waiting states are non-terminal; evaluation errors fail the run; backoff wait saturates at cap;max_interval_secrequires enabled backoff;backoff: falseequals omitted; loops compose inside child DAGs.conformance/spec024_dag_loop/*covers validation, lifecycle, timeout/abort (including abort during interval), interval/backoff (cap and overflow), env/feedback semantics, isolation (harness and chat), sub-DAG composition, and runtime bounds;conformance/harness/runner.goaddsRunner.Start/Interrupt/Waitto drive in-progress runs.Written for commit 1576d36. Summary will update on new commits.
Summary by CodeRabbit