Skip to content

feat: batch-parallel per-unit waves with parallel foreground reviewers (2.5.67) - #617

Merged
apackeer merged 4 commits into
awslabs:v2from
jstrunk:feat/610-batch-parallel-waves
Aug 12, 2026
Merged

feat: batch-parallel per-unit waves with parallel foreground reviewers (2.5.67)#617
apackeer merged 4 commits into
awslabs:v2from
jstrunk:feat/610-batch-parallel-waves

Conversation

@jstrunk

@jstrunk jstrunk commented Jul 20, 2026

Copy link
Copy Markdown

Implements dependency-safe per-Unit design waves for the default stage-major Construction walk.

What

For functional-design, nfr-requirements, nfr-design, and infrastructure-design, the engine may emit directive.wave for the first unsettled Bolt-DAG batch. Each entry carries fully resolved Unit-local inputs, outputs, memory, build state, completion state, and paired-review progress. The conductor never reads or reconstructs the cached runtime graph.

Current-v2 integration

  • Wave membership comes from one cache-validated, self-healed BoltDagResolution.
  • Builders inherit the parent stage file, inline persona/knowledge roster, context warnings, steering content, sensors, reviewer configuration, and effective review class.
  • Reviews use paired REVIEW_REQUESTED / REVIEW_COMPLETED evidence. none, advisory, adversarial repair, unmatched-request retry, and terminal NOT-READY behavior follow the current review-class contract.
  • Parallel builders do not enter the serial one-active-Unit lifecycle. After build and review settle, aidlc-state.ts unit complete --wave verifies the live entry, fans Unit diary entries into the parent diary with deterministic deduplication, then emits UNIT_COMPLETED.
  • Dependent batches and the stage gate cannot overtake build evidence, review evidence, diary fan-in, or completion receipts.
  • Blocking builders withhold a kind-applicable required_produces path. Optional or kind-inapplicable outputs cannot keep the Unit open.
  • Code Generation and unit-major iteration remain wave-free.
  • All seven shipped harnesses receive the same branch-before-gate contract. Reviewers remain serialized where reviewer-scope enforcement uses the single dispatch record.

Compatibility and release metadata

  • Rebased onto current v2.
  • Version re-slotted to 2.5.67.
  • Wave regression coverage renumbered to t278.
  • All generated harness and plugin distributions regenerated from authored sources.

Verification

  • bun run check
  • bun tests/gen-coverage-registry.ts --check
  • Full smoke + unit + integration landing suite

The stronger future phases from the RFC, including plugin-extensible wave eligibility and background reviewer dispatch, remain out of scope.

@apackeer

Copy link
Copy Markdown
Contributor

Reviewed at head 951fda8. The mechanism is sound and the engine facts the PR leans on all check out; the findings below are prose-shaped and none require engine changes, but 1 and 2 should block as written.

Verified before the findings:

  • bun scripts/package.ts --check green on all five harness trees at the head; focused suites (new t244, t68 version sync, t217 reviewer read scope) pass 17/17 assertions.
  • Per-unit coverage really is a stateless disk scan (unitCovered, core/tools/aidlc-orchestrate.ts:2249), so units landing between next calls are skipped as claimed.
  • bolt_dag.batches is read from runtime-graph.json (readBoltDagSnapshot, aidlc-orchestrate.ts:763), so the conductor as a second consumer is legitimate.
  • The reviewer dispatch record is a single file (reviewerDispatchPath, aidlc-lib.ts:2311), so serializing per-unit reviews on the enforcement harnesses is correct, and Kiro IDE's reviewerScopeRegistration: "unsupported" in the harness matrix matches its parallel-reviewer allowance.
  • t244 itself is well built: it pins authored and dist surfaces through HARNESS_MATRIX and branches on the capability flag, so a new harness cannot ship without the wave paragraph.

1. P1: carve non-autonomous code-generation out of wave eligibility

The wave paragraphs apply to any gate: false per-unit directive, and the per-unit iteration prose (plus this PR's own docs/reference/03-orchestrator.md addition) explicitly includes non-autonomous code-generation. Two independent reasons it cannot ride a wave:

  • The disjointness argument covers only the construction/<unit>/<stage>/ record dirs. code-generation carries workspace_requires: true and writes source into the shared workspace, so concurrent wave builders collide in the working tree. The swarm path uses per-unit worktrees for exactly this reason, and the floor deliberately has none.
  • code-generation's Step 3 Plan Approval is "a mandatory hard stop in every execution mode, including Bolt execution" (core/aidlc-common/stages/construction/code-generation.md:130 plus the orchestrator-managed completion gating note at :195). That directly contradicts the wave rule that human-blocking questions ride in the builder's return message instead of stopping.

Fix: restrict the wave prose to the four inline design stages (or exclude workspace_requires stages), on every surface that carries the paragraph, and pin it in t244.

2. P1: sibling-unit resolution is unspecified; mixed-kind batches get wrong artifact sets

The engine resolves paths, kind-filtered produces, and the consumes present/absent split only for directive.unit. The wave prose tells the conductor to take "that batch's units whose artifacts for THIS stage are not yet on disk" but never says how to derive the sibling units' produce sets or paths. Two concrete gaps:

  • "artifacts not on disk" is not the engine's coverage predicate. unitCovered filters required produces by the unit's kind via produces_kinds (a spec unit is vacuously covered by infrastructure-design) and exempts optional_produces such as shared-infrastructure.md. A literal conductor dispatches builders for kind-exempt units and writes artifacts the matrix says do not apply to them.
  • Naive substitution of the sibling unit name into the one resolved directive is wrong across kinds.

This is closable in prose: bolt_dag.units[].kind is already in the same runtime-graph.json snapshot the wave reads, and produces_kinds is in the stage frontmatter the conductor holds. One or two sentences specifying kind-aware wave membership and per-sibling path derivation would do it. (Engine-emitted batch directives would also close it, but that is the RFC's stronger phase, deliberately out of scope here.)

3. P2: the crash-recovery sentence overclaims; covered-but-unreviewed units reach the gate

"A session that crashes mid-wave recovers via the same disk scan" is true for builds but not for reviews. Coverage is artifact-existence only, the gate flips to gate: true purely from the disk scan, and the approve-path guard checks unit coverage and ensemble contribution files but never reviewer verdicts (aidlc-orchestrate.ts:3290). So if the wave's builders finish and the session dies before the reviewer pass, re-entry scans every unit as covered and the engine presents the gate; "the gate is never presented with a reviewer outstanding" is unenforceable in exactly the crash case the sentence claims to handle. (t186 pins artifact-only coverage as the contract.)

To be fair, the hole exists on the serial path today too: a crash between a unit's artifact write and its review skips that review identically. But a wave widens the exposure from one unit to a whole batch, and the recovery sentence claims a correctness it does not have. Cheap prose fix: the verdict is durable evidence (the 12a step 2 reviewer appends ## Review to the primary artifact), so require that on crash re-entry the conductor run the 12a step for any covered wave unit whose primary artifact lacks a verdict, before the gate. Deterministic enforcement of this class of gap is PR #569's territory; a cross-reference would help.

4. P2: a builder that finishes its artifacts and returns a human-blocking question strands the question

If a wave builder writes all of its produces AND returns a blocking question, the unit is covered, the engine never re-hands it, and the answer can no longer shape the artifact. The prose should require a blocked builder to withhold at least one required artifact (so the unit stays uncovered), or require the conductor to re-run that unit's body after the human answers.

5. P3: learnings and Q&A fidelity

The section 13 ritual surfaces the stage-level memory.md entries verbatim (aidlc-learnings.ts reads the path built by memoryPathFor, aidlc-orchestrate.ts:611). The wave moves builder notes into per-unit dirs and has the conductor write one consolidated diary entry, so the ritual now surfaces a paraphrase instead of the builders' verbatim entries. Same question for per-unit questions vs. the single stage Q&A file the 12a reviewer is passed. Worth one sentence saying the conductor's consolidated entry carries the unit entries over verbatim (or that the ritual reads the unit files too).

6. Process

  • The PR is CONFLICTING against v2 and the 2.5.3 slot is stale: v2 is at 2.5.2 and 2.5.3 is contended by other in-flight PRs. Rebase, re-bump to the next free patch per the CHANGELOG conflict policy, and fix the heading date while there (a 2026-07-20 heading would sort above 2.5.2's 2026-07-21).
  • Test slot: t244 is free on v2 today, but feat: reviewer turn backstop (maxTurns) + §12a missing-verdict guard (2.5.14) #613 has been asked to renumber its new test into t244; whichever of the two lands second renumbers again.

None of this questions the design itself: MAY-not-MUST, the unit-major exclusion, the reviewer-serialization split by harness capability, and the wave-as-loop-parallelization carve-out in section 5 are all consistent with the engine as shipped. Findings 1-4 are each a sentence or two of prose plus matching t244 pins.

jstrunk added a commit to jstrunk/aidlc-workflows that referenced this pull request Jul 28, 2026
…aware sibling resolution (awslabs#617 review)

Addresses review findings on PR awslabs#617 (a field engagement's request for
batch-parallel per-unit waves). Both P1s were correctness bugs in the
wave prose, not the engine:

- P1: the wave paragraph applied to any gate:false per-unit directive,
  including non-autonomous code-generation. That's wrong on two counts
  - code-generation carries workspace_requires:true and writes into
  the shared workspace, so concurrent wave builders would collide in
  the working tree (the swarm path uses per-Unit worktrees for exactly
  this isolation; the wave floor has none); and code-generation's Step
  3 Plan Approval is a mandatory hard stop in every execution mode,
  which contradicts the wave rule that blocking questions ride in the
  builder's return message instead of stopping. Wave eligibility is
  now restricted to the four inline per-unit design stages
  (functional-design, nfr-requirements, nfr-design,
  infrastructure-design); any workspace_requires:true stage is never
  wave-eligible.

- P1: sibling-unit resolution for a wave was unspecified for
  mixed-kind batches. The wave prose said to take "units whose
  artifacts for THIS stage are not yet on disk" but never said how to
  derive a sibling's produce set - the engine's actual coverage
  predicate (unitCovered) filters required produces by the unit's
  kind via produces_kinds, so a kind exempt from every one of a
  stage's produces is vacuously covered and must never be a wave
  member. Naive name substitution into the one resolved directive is
  also wrong across kinds. The wave prose now specifies: cross-
  reference each candidate unit's bolt_dag.units[].kind against the
  stage's produces_kinds map to decide membership, and resolve each
  included sibling's own paths/produces/consumes from its own kind.

- P2: the crash-recovery sentence overclaimed - disk-scan coverage is
  artifact-existence only, so a wave that crashes between builder
  completion and the reviewer pass could reach the gate with a
  covered-but-unreviewed unit. On crash re-entry the conductor now
  runs the §12a reviewer step for any wave unit that reads as covered
  but carries no `## Review` verdict yet, before presenting the gate.
  Full deterministic closure of this class of gap is awslabs#569's territory;
  this is the wave-specific instance of that same contract.

- P2: a builder that writes all its produces AND returns a blocking
  question would strand the question (the unit reads covered and the
  engine never re-hands it). Fixed via the smaller of the two options:
  a blocked builder must withhold at least one required produce so
  its unit still reads as uncovered; the conductor re-hands the unit
  after the human answers, consistent with how the serial loop already
  treats an incomplete unit.

- P3: the consolidated per-wave diary entry and any per-unit Q&A now
  explicitly carry the unit-level content verbatim, not a paraphrase.

Applied to every surface that carries the wave paragraph: the shared
core/aidlc-common/protocols/stage-protocol.md §3 contract, all five
authored harness/*/skills/aidlc/SKILL.md files, and
docs/reference/03-orchestrator.md, 04-stages/construction.md, and
13-runtime-graph.md. dist/ regenerated from the authored sources (no
hand-edits).

tests/unit/t249-per-unit-wave.test.ts (renamed from a rebase-time
t244 collision) pins every load-bearing sentence above on every
carrying surface, plus two grounding tests against real stage
frontmatter: code-generation actually carries workspace_requires:true
while the four inline design stages don't, and
infrastructure-design's produces_kinds map actually vacuously exempts
a spec-kind unit from every required produce while a service-kind
unit stays fully required (apackeer's own example, grounded rather
than only asserted in prose).

No engine, hook, or state-format changes - conductor-prose only.
@jstrunk
jstrunk force-pushed the feat/610-batch-parallel-waves branch from 951fda8 to 21ef329 Compare July 28, 2026 15:52
@jstrunk jstrunk changed the title feat: batch-parallel per-unit waves with parallel foreground reviewers (2.5.3) feat: batch-parallel per-unit waves with parallel foreground reviewers (2.5.15) Jul 28, 2026
@jstrunk

jstrunk commented Jul 28, 2026

Copy link
Copy Markdown
Author

Thanks for the thorough review — rebased onto current v2 and addressed all six points. Summary below; commit 21ef329 carries the substantive fixes on top of the rebased/re-bumped c497beb.

6 (process). Rebased onto v2 (was conflicting). Version bumped 2.5.32.5.15 (six copies of aidlc-version.ts + CHANGELOG heading + README badge) to avoid colliding with sibling PRs rebasing in parallel. Test file renamed t244t249 (both the old number and my prior number are now taken by unrelated merged work); internal t244 references inside the file updated to t249.

1 (P1, blocking) — carved non-autonomous code-generation out of wave eligibility. Confirmed: code-generation.md carries workspace_requires: true and its Step 3 Plan Approval is a mandatory hard stop in every execution mode (:130, :195), while the four inline design stages (functional-design, nfr-requirements, nfr-design, infrastructure-design) are mode: inline with no workspace_requires. The wave paragraph now explicitly restricts eligibility to those four stages and states the two reasons code-generation is excluded (shared-workspace collision; the Plan Approval hard stop can't fold into a builder's return message) — applied to core/aidlc-common/protocols/stage-protocol.md §3, all five harness/*/skills/aidlc/SKILL.md files, and docs/reference/03-orchestrator.md / 04-stages/construction.md / 13-runtime-graph.md. Pinned in t249 with regex assertions on every surface, plus a grounding test that reads the real frontmatter and asserts code-generation actually has workspace_requires: true while the four inline stages don't (so this can't silently drift back).

2 (P1, blocking) — kind-aware sibling resolution. Added the missing specification: wave membership is now derived by cross-referencing each candidate unit's bolt_dag.units[].kind (from the same runtime-graph.json snapshot the wave already reads) against the stage's produces_kinds frontmatter map — a unit whose kind is exempt from every one of the stage's required produces is vacuously covered and is never a wave member. Each included sibling's paths/produces/consumes are resolved from its own kind, explicitly never by substituting the sibling's name into the one already-resolved directive. Pinned in t249 with a grounding test against infrastructure-design.md's real produces_kinds map, reproducing your own example: a spec-kind unit is absent from every required produce's kind list (vacuously covered, never a wave member) while a service-kind unit is present on all of them (a genuine wave member).

3 (P2) — crash-recovery overclaim. Fixed: the recovery sentence now states the disk scan is artifact-existence only and proves nothing about review, and requires that on crash re-entry the conductor run the §12a reviewer step for any wave unit that reads as covered but whose primary artifact carries no ## Review verdict yet — before presenting the gate. Cross-references #569 as the deterministic-closure territory for this class of gap; this is the wave-specific instance of that same contract. Applied to stage-protocol.md §3 and all five SKILL.md files; pinned in t249.

4 (P2) — stranded blocking question. Went with the withhold-artifact fix (the smaller change, and consistent with how the engine's coverage predicate already works): a wave builder with a human-blocking question MUST withhold at least one of the stage's required produces so its unit still reads as uncovered on the engine's disk scan, and — symmetrically — once a builder has written every required produce it MUST NOT raise a blocking question, since the engine would then read the unit as done and never re-hand it. The conductor re-hands an uncovered unit after the human answers, the same way the serial per-unit loop already treats an incomplete unit. Applied everywhere the wave paragraph lives; pinned in t249.

5 (P3) — learnings/Q&A fidelity. Added the one-sentence fix: the conductor's consolidated per-wave diary entry now explicitly carries every unit's memory-note content verbatim, not a paraphrase, and the §13 learnings ritual (or a reviewer needing per-unit Q&A) may read the unit files directly wherever a verbatim source matters.

Verification:

  • bun scripts/package.ts --check — clean (dist regenerated from core/ + harness/ sources, no hand-edits).
  • bun tests/gen-coverage-registry.ts --check — OK.
  • bun run check (package --check + typecheck + biome lint) — clean.
  • bash tests/run-tests.sh --smoke --unit — 179 files, 4405 assertions, 0 failures.
  • t249-per-unit-wave.test.ts — 9 pass / 0 fail / 412 assertions (up from 6/213 pre-review, now including the two P1 grounding tests).
  • t68-version-changelog-sync.test.ts — 7 pass / 0 fail.

Force-pushed as 21ef329 on top of the rebased/re-bumped c497beb.

@apackeer

Copy link
Copy Markdown
Contributor

Re-reviewed at head 21ef329c. All six round-1 points are genuinely addressed (verified against the code on disk, details below), and the two grounding tests in t249 are a nice touch: pinning the prose's claims against the real frontmatter is exactly the right defense. However, the re-review surfaced two new findings in the fixed prose itself, one of which I'd block on. Sorry to volley this back once more; both have small, prose-only fixes.

Verified before the findings:

  • bun scripts/package.ts --check clean on all five harness trees; bun run typecheck and the coverage-registry check clean.
  • Full smoke + unit tier at the head: 179 files, 4405 assertions, 0 failures. Focused t249 (9/9), t68 (7/7), t217 (4/4), t215 (all pass).
  • Round-1 fix verification: (1) the carve-out names the four inline stages on every surface and the grounding test reads the real workspace_requires/mode frontmatter; (2) kind-aware membership matches the engine (unitCovered filters via applicableProduceNames/filterProducesByKind, and bolt_dag.units[].kind is parsed in resolveBoltDag); (3) the recovery sentence now scopes the disk scan to build coverage and requires the section-12a pass for covered-but-unverdicted units, matching the ## Review durable-evidence contract; (4) the withhold rule plus its symmetric no-question-after-last-produce rule are on every surface; (5) verbatim diary carry + direct-read allowance present; (6) rebase, 2.5.15, t249 rename all check out.

New finding 1 (P1, blocking): the wave reads raw runtime-graph.json, but the engine's self-heal is in-memory only, so the wave can act on a stale DAG the directive no longer matches.

The wave prose (stage-protocol.md section 3 and each SKILL.md) instructs the conductor to "read bolt_dag.batches from the intent's runtime-graph.json". The engine deliberately does NOT trust that file: resolveBoltDag (aidlc-lib.ts, the boltDagMatches check) validates the cached node against the authored unit-of-work-dependency.md and, on mismatch, returns the authored parse without writing the graph back; resolveBoltBatches is documented "Pure in-memory: never writes the graph (next stays read-only); the runtime-compile hook repairs the cache on the next transition" (aidlc-orchestrate.ts, above resolveBoltBatches). So after a heal, the directive the conductor holds and the JSON file the wave prose points it at describe different unit sets.

This is not hypothetical; t215 test 6b ("a valid but outdated cached bolt_dag heals from the authored artifact") constructs exactly this state: the cached graph says [alpha], the authored artifact says alpha + beta, and next emits directive.unit: beta while the raw JSON still contains no beta at all. A conductor following the wave paragraph on that state hits one of two failure shapes:

  • directive.unit is absent from every batch in the file. The prose's "find the batch containing directive.unit" has no defined answer; its only stated fallback covers an absent bolt_dag node, not a stale one.
  • Worse, the inverse skew: a stale cache still batches two units together that the authored artifact now declares dependent. The conductor waves them concurrently, and no engine invariant catches it: per-unit coverage checks artifact existence, never ordering, so the "batch units are dependency-free by construction" safety sentence is only true of a fresh snapshot.

This also grazes the round-1 kind-membership fix: the sibling kinds are read from the same possibly-stale snapshot, so a kind edit that healed in memory is invisible to the wave.

Suggested fix (prose-only, keeps the floor framing): make the serial fallback fire on staleness, not just absence. E.g.: before forming a wave, cross-check the snapshot against the directive: if directive.unit is missing from bolt_dag.batches, or the batch's units are not all present in the same snapshot's units[], treat the DAG as stale and run the serial loop (the engine re-hands units one at a time from its healed in-memory view; correctness is preserved, only the concurrency is lost). Alternatively require the conductor to derive batches from unit-of-work-dependency.md itself, but the mismatch-means-serial rule is smaller and self-limiting. Worth a t249 sentence-pin either way.

New finding 2 (P2): the withhold-a-produce rule is kind-blind; on a mixed-kind unit the withheld artifact can be one the engine never checks.

The new rule says a blocked builder "MUST withhold at least one of the stage's required produces so its unit still reads as uncovered". But coverage only checks produces applicable to the unit's kind: unitCovered filters the required set through produces_kinds (filterProducesByKind: a mapped artifact is kept only when its kind list includes the unit's kind). Concrete case from functional-design's own frontmatter: for a ui unit, only business-logic-model applies; business-rules and domain-entities are [service, spec, library]. A ui-unit builder that "withholds" domain-entities while writing business-logic-model has satisfied the rule as written, yet the engine reads the unit as covered and never re-hands it. That is the exact stranded-question failure round-1 point 4 was closing, reintroduced through the kind axis.

One-phrase fix on every surface (and the section 3 paragraph): "withhold at least one of the stage's required produces that applies to its unit's kind". Since t249 already pins the withhold sentence by regex, the pin needs the same update; and the existing grounding test for (g) could gain one assertion that the withheld-produce example uses a kind-applicable artifact.


Smaller notes (non-blocking):

With finding 1's serial-on-mismatch rule and finding 2's one-phrase kind qualifier (plus the matching t249 pin updates), this is ready.

@apackeer

apackeer commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Thanks, Jeff. Your 21ef329 update addressed the first-pass findings, and the batch-wave direction remains useful. Please do not rebase or revise this branch while we complete the second pass.

@leandrodamascena, could you check how this should integrate with current v2, particularly:

  1. Should wave membership and complete sibling directives come from the engine’s healed DAG snapshot rather than being reconstructed by the conductor?
  2. Should crash recovery use fresh per-unit REVIEW_COMPLETED receipts and settle reviews before a dependent batch begins?
  3. Does the blocked-builder rule need to withhold a required produce applicable to that unit’s kind, and are current steering, context, and memory-path contracts preserved?

The core constraints still look sound: optional waves, the four inline design stages only, default stage-major execution, serialized reviewers where enforcement requires it, and no gate with reviews outstanding. If changes are needed, maintainers should handle the current-v2 integration rather than asking Jeff to chase the moving branch.

@leandrodamascena leandrodamascena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against current v2. The wave direction is sound, but integration needs engine support before merge.

P1: Engine must emit wave membership

core/aidlc-common/protocols/stage-protocol.md:411 reads raw runtime-graph.json, while resolveBoltDag() may heal a stale DAG only in memory. This can leave directive.unit absent from the cached batches or concurrently dispatch units the authored DAG now declares dependent. Wave membership and complete per-unit directives should come from the engine’s healed snapshot.

P1: Reviews must settle before dependent batches

Crash recovery checks for ## Review, but completion requires a fresh REVIEW_COMPLETED receipt, and later artifact writes invalidate that receipt. The engine should keep the current DAG batch active until every applicable unit has a fresh terminal review receipt; otherwise a dependent batch can be built against design that the delayed review subsequently changes.

P1: Preserve current steering and context contracts

Every wave builder must receive the accumulated load-steering.rules_content bundle verbatim. Inline support perspectives and context_warnings must also remain effective. The current wave prose predates and does not preserve these current-v2 contracts.

P2: Resolve unit-local memory paths

Current directives expose the shared stage-level memory_path, while wave builders are prohibited from writing that diary. Engine-emitted wave entries need an explicit unit-local memory path; the parent directive should retain the stage diary consumed by the learnings tool.

P2: Make blocked-builder withholding kind-aware

Require withholding at least one required produce applicable to the unit’s kind. Withholding an inapplicable artifact still lets the engine mark the unit covered and strands the blocking question.

Recommended integration shape: add an optional engine-emitted wave containing complete per-unit entries derived from one healed BoltDagResolution, including resolved unit, consumes, consumes_absent, produces, unit memory path, and review state. Keep steering, context, and stage-level memory on the parent directive.

Verification passed locally: package parity, typecheck, lint, coverage registry, t249, t215, t208, and t205.

@apackeer
apackeer force-pushed the feat/610-batch-parallel-waves branch from 21ef329 to ef2c4cd Compare August 6, 2026 21:07
apackeer pushed a commit to jstrunk/aidlc-workflows that referenced this pull request Aug 6, 2026
…aware sibling resolution (awslabs#617 review)

Addresses review findings on PR awslabs#617 (a field engagement's request for
batch-parallel per-unit waves). Both P1s were correctness bugs in the
wave prose, not the engine:

- P1: the wave paragraph applied to any gate:false per-unit directive,
  including non-autonomous code-generation. That's wrong on two counts
  - code-generation carries workspace_requires:true and writes into
  the shared workspace, so concurrent wave builders would collide in
  the working tree (the swarm path uses per-Unit worktrees for exactly
  this isolation; the wave floor has none); and code-generation's Step
  3 Plan Approval is a mandatory hard stop in every execution mode,
  which contradicts the wave rule that blocking questions ride in the
  builder's return message instead of stopping. Wave eligibility is
  now restricted to the four inline per-unit design stages
  (functional-design, nfr-requirements, nfr-design,
  infrastructure-design); any workspace_requires:true stage is never
  wave-eligible.

- P1: sibling-unit resolution for a wave was unspecified for
  mixed-kind batches. The wave prose said to take "units whose
  artifacts for THIS stage are not yet on disk" but never said how to
  derive a sibling's produce set - the engine's actual coverage
  predicate (unitCovered) filters required produces by the unit's
  kind via produces_kinds, so a kind exempt from every one of a
  stage's produces is vacuously covered and must never be a wave
  member. Naive name substitution into the one resolved directive is
  also wrong across kinds. The wave prose now specifies: cross-
  reference each candidate unit's bolt_dag.units[].kind against the
  stage's produces_kinds map to decide membership, and resolve each
  included sibling's own paths/produces/consumes from its own kind.

- P2: the crash-recovery sentence overclaimed - disk-scan coverage is
  artifact-existence only, so a wave that crashes between builder
  completion and the reviewer pass could reach the gate with a
  covered-but-unreviewed unit. On crash re-entry the conductor now
  runs the §12a reviewer step for any wave unit that reads as covered
  but carries no `## Review` verdict yet, before presenting the gate.
  Full deterministic closure of this class of gap is awslabs#569's territory;
  this is the wave-specific instance of that same contract.

- P2: a builder that writes all its produces AND returns a blocking
  question would strand the question (the unit reads covered and the
  engine never re-hands it). Fixed via the smaller of the two options:
  a blocked builder must withhold at least one required produce so
  its unit still reads as uncovered; the conductor re-hands the unit
  after the human answers, consistent with how the serial loop already
  treats an incomplete unit.

- P3: the consolidated per-wave diary entry and any per-unit Q&A now
  explicitly carry the unit-level content verbatim, not a paraphrase.

Applied to every surface that carries the wave paragraph: the shared
core/aidlc-common/protocols/stage-protocol.md §3 contract, all five
authored harness/*/skills/aidlc/SKILL.md files, and
docs/reference/03-orchestrator.md, 04-stages/construction.md, and
13-runtime-graph.md. dist/ regenerated from the authored sources (no
hand-edits).

tests/unit/t249-per-unit-wave.test.ts (renamed from a rebase-time
t244 collision) pins every load-bearing sentence above on every
carrying surface, plus two grounding tests against real stage
frontmatter: code-generation actually carries workspace_requires:true
while the four inline design stages don't, and
infrastructure-design's produces_kinds map actually vacuously exempts
a spec-kind unit from every required produce while a service-kind
unit stays fully required (apackeer's own example, grounded rather
than only asserted in prose).

No engine, hook, or state-format changes - conductor-prose only.
@apackeer

apackeer commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Addressed the current-v2 integration review and force-pushed the rebased branch at ef2c4cde (2.5.54).

Review findings

  • Engine-owned wave membership: eligible stage-major design directives now carry optional directive.wave, derived from one healed BoltDagResolution. Each entry includes its resolved unit/kind, present and absent consumes, all produces, the kind-applicable required-produce subset, unit-local memory path, build state, review state, and review iteration. The conductor no longer reads or reconstructs runtime-graph.json; large batches remain dependency-local and are emitted as deterministic prefixes under the 28 KiB transport limit.
  • Review settlement before dependent work: the engine keeps the first unsettled DAG batch active until every applicable unit has build coverage, summary confirmation, and a fresh fingerprint-bound terminal REVIEW_COMPLETED. Later artifact changes reopen only the owning unit's batch. Below-cap NOT-READY now emits repair-required with its prior iteration so crash recovery resumes lead repair before the next review; NOT-READY is terminal only at reviewer_max_iterations. Autonomous swarm finalize uses the same terminal-verdict predicate.
  • Steering/context preservation: the parent directive retains the stage diary, accumulated load-steering.rules_content, inline_context_paths, context_warnings, reviewer settings, and sensors. single -> wave -> gate/unit is now an exclusive branch order, so parent compatibility fields are not executed twice. Every builder receives the exact steering bundle and inline context roster.
  • Unit-local memory: wave builders write only entry.unit_memory_path; the parent memory_path remains the stage diary consumed by learnings. Fully settled siblings are omitted from repeated waves, preventing duplicate diary fan-in.
  • Kind-aware blocking and inputs: blocked builders must withhold an entry.required_produces path applicable to their unit kind. Optional or kind-inapplicable outputs cannot keep coverage open. Downstream consumes are also pruned when the per-unit producer excludes that artifact for the unit kind.
  • Scope boundaries preserved: waves remain optional, limited to the four inline design stages, absent under unit-major iteration, and excluded from Code Generation. Reviewer dispatch remains serialized on enforcement harnesses and may run foreground-parallel only where no reviewer-scope record exists.

Verification

  • bun run check — package parity, all TypeScript configs, and Biome pass.
  • bun tests/gen-coverage-registry.ts --check — fresh; ratchet held.
  • Final deterministic default gate: 300 files, 6,225 assertions, 0 failures (smoke + unit + integration, --debug -P 8 --no-llm).
  • Focused wave/state/swarm slice: 147 assertions, 0 failures, including stale-DAG healing, mixed-kind resolution, crash repair, receipt invalidation, large-batch prefixing, and swarm NOT-READY iterations 1/2.
  • Independent post-fix review found no remaining issues.

I also attempted the Claude SDK live integration slice. This sandbox blocks writes to /home/ubuntu/.claude.json, so those tests failed uniformly at SDK startup with EROFS before workflow assertions; the deterministic integration suite above is green.

@leandrodamascena

Copy link
Copy Markdown
Contributor

Hey @jstrunk pls check the last review.

@jstrunk

jstrunk commented Aug 12, 2026

Copy link
Copy Markdown
Author

Hey @jstrunk pls check the last review.

I thought that @apackeer resolved everything. I'm missing what is pending for me to do.

@leandrodamascena
leandrodamascena self-requested a review August 12, 2026 19:59
@apackeer

Copy link
Copy Markdown
Contributor

@jstrunk Now Leandro has approved, i will rebase and merge. Thank you again!

jstrunk and others added 4 commits August 12, 2026 22:09
…s (2.5.15)

Implements the floor version of awslabs#610: on the default stage-major
Construction walk, the conductor MAY process a gate:false per-unit
design directive as a Bolt-DAG batch wave - one concurrent stage-body
dispatch per uncovered batch unit, each running the stage's lead
persona for that ONE unit, confined to construction/<unit>/<stage>/
(no stage-level diary; the conductor appends one consolidated entry
per wave; human-blocking questions surface in the builder's return
message). Reviewers run per wave unit at wave end - parallel
FOREGROUND dispatches only where no reviewer-scope dispatch record is
active; on enforcement harnesses (Claude Code, Kiro CLI, Codex,
opencode) the single-file record serializes per-unit reviews. Every
NOT-READY revision loop resolves within its iteration budget before
the single stage gate - the wait is deferred, not skipped. Waves never
apply under Construction Iteration: unit-major, and harnesses without
a parallel dispatch primitive stay on the serial loop (MAY, not MUST).

Conductor-prose only - no engine, hook, or dispatch-record changes:
per-unit coverage is a stateless disk scan (nextUncoveredUnit), batch
units are dependency-free with disjoint artifact dirs, and a crashed
session mid-wave recovers via the same scan.

- Wave paragraph in all five harness/*/skills/aidlc/SKILL.md, worded
  per each harness's native dispatch (Task / subagent crew / codex
  exec workers / task tool)
- stage-protocol.md: section 3 harness-neutral wave paragraph +
  section 5 per-unit-wave carve-out (a wave parallelizes the engine's
  per-unit loop, not a communication topology; mode: inline's
  "supports are voices" folds into each builder's brief)
- Docs: 03-orchestrator (per-unit wave in Construction Execution),
  04-stages/construction.md (wave note), 13-runtime-graph.md (the
  conductor as a bolt_dag.batches consumer)
- New prose pin tests/unit/t249-per-unit-wave.test.ts (HARNESS_MATRIX
  style; pins MAY-not-MUST, unit-major exclusion, write confinement,
  never-present-gate-with-outstanding-reviewer, and the enforcement
  serialization constraint on authored + dist surfaces)
- Version 2.5.15, CHANGELOG entry, README badge; dist regenerated
…aware sibling resolution (awslabs#617 review)

Addresses review findings on PR awslabs#617 (a field engagement's request for
batch-parallel per-unit waves). Both P1s were correctness bugs in the
wave prose, not the engine:

- P1: the wave paragraph applied to any gate:false per-unit directive,
  including non-autonomous code-generation. That's wrong on two counts
  - code-generation carries workspace_requires:true and writes into
  the shared workspace, so concurrent wave builders would collide in
  the working tree (the swarm path uses per-Unit worktrees for exactly
  this isolation; the wave floor has none); and code-generation's Step
  3 Plan Approval is a mandatory hard stop in every execution mode,
  which contradicts the wave rule that blocking questions ride in the
  builder's return message instead of stopping. Wave eligibility is
  now restricted to the four inline per-unit design stages
  (functional-design, nfr-requirements, nfr-design,
  infrastructure-design); any workspace_requires:true stage is never
  wave-eligible.

- P1: sibling-unit resolution for a wave was unspecified for
  mixed-kind batches. The wave prose said to take "units whose
  artifacts for THIS stage are not yet on disk" but never said how to
  derive a sibling's produce set - the engine's actual coverage
  predicate (unitCovered) filters required produces by the unit's
  kind via produces_kinds, so a kind exempt from every one of a
  stage's produces is vacuously covered and must never be a wave
  member. Naive name substitution into the one resolved directive is
  also wrong across kinds. The wave prose now specifies: cross-
  reference each candidate unit's bolt_dag.units[].kind against the
  stage's produces_kinds map to decide membership, and resolve each
  included sibling's own paths/produces/consumes from its own kind.

- P2: the crash-recovery sentence overclaimed - disk-scan coverage is
  artifact-existence only, so a wave that crashes between builder
  completion and the reviewer pass could reach the gate with a
  covered-but-unreviewed unit. On crash re-entry the conductor now
  runs the §12a reviewer step for any wave unit that reads as covered
  but carries no `## Review` verdict yet, before presenting the gate.
  Full deterministic closure of this class of gap is awslabs#569's territory;
  this is the wave-specific instance of that same contract.

- P2: a builder that writes all its produces AND returns a blocking
  question would strand the question (the unit reads covered and the
  engine never re-hands it). Fixed via the smaller of the two options:
  a blocked builder must withhold at least one required produce so
  its unit still reads as uncovered; the conductor re-hands the unit
  after the human answers, consistent with how the serial loop already
  treats an incomplete unit.

- P3: the consolidated per-wave diary entry and any per-unit Q&A now
  explicitly carry the unit-level content verbatim, not a paraphrase.

Applied to every surface that carries the wave paragraph: the shared
core/aidlc-common/protocols/stage-protocol.md §3 contract, all five
authored harness/*/skills/aidlc/SKILL.md files, and
docs/reference/03-orchestrator.md, 04-stages/construction.md, and
13-runtime-graph.md. dist/ regenerated from the authored sources (no
hand-edits).

tests/unit/t249-per-unit-wave.test.ts (renamed from a rebase-time
t244 collision) pins every load-bearing sentence above on every
carrying surface, plus two grounding tests against real stage
frontmatter: code-generation actually carries workspace_requires:true
while the four inline design stages don't, and
infrastructure-design's produces_kinds map actually vacuously exempts
a spec-kind unit from every required produce while a service-kind
unit stays fully required (apackeer's own example, grounded rather
than only asserted in prose).

No engine, hook, or state-format changes - conductor-prose only.
@apackeer
apackeer force-pushed the feat/610-batch-parallel-waves branch from ef2c4cd to d38ca2d Compare August 12, 2026 22:10
@apackeer apackeer changed the title feat: batch-parallel per-unit waves with parallel foreground reviewers (2.5.15) feat: batch-parallel per-unit waves with parallel foreground reviewers (2.5.67) Aug 12, 2026
@apackeer
apackeer merged commit 327d362 into awslabs:v2 Aug 12, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants