feat(subplan): execute and merge subplans #184

Closed
opened 2026-02-22 23:39:55 +00:00 by freemo · 3 comments
Owner

Metadata

  • Commit Message: feat(subplan): execute and merge subplans
  • Branch: feature/m4-subplan-execution

Background

Subplan execution supports sequential, parallel, and dependency_ordered scheduling modes. Merge strategies (git three-way, sequential apply, fail on conflict, last wins) use sandbox outputs to combine subplan results into the parent plan.

Acceptance Criteria

  • Implement subplan execution scheduler for sequential/parallel/dependency_ordered modes.
  • Implement merge strategies (git three-way, sequential apply, fail on conflict, last wins) using sandbox outputs.
  • Update docs/reference/subplans.md with execution modes and merge outcomes.

Definition of Done

This issue is complete when:

  • All subtasks below are completed and checked off.
  • A Git commit is created where the first line of the commit message matches
    the Commit Message in Metadata exactly, followed by a blank line, then
    additional lines providing relevant details about the implementation. The
    commit body should be appropriate in size for a commit message and relatively
    complete in describing what was done.
  • The commit is pushed to the remote on the branch matching the Branch in
    Metadata exactly.
  • The commit is submitted as a pull request to master, reviewed, and
    merged before this issue is marked done.

Subtasks

  • Implement subplan execution scheduler for sequential/parallel/dependency_ordered modes.
  • Implement merge strategies (git three-way, sequential apply, fail on conflict, last wins) using sandbox outputs.
  • Update docs/reference/subplans.md with execution modes and merge outcomes.
  • Tests (Behave): Add scenarios for parallel execution, conflict handling, and merge results.
  • Tests (Robot): Add Robot test for subplan execution and merge summaries.
  • Tests (ASV): Add benchmarks/subplan_execution_bench.py for scheduler overhead.
  • Verify coverage >=97% via nox -s coverage_report.
  • Run nox (all default sessions, including benchmark), fix any errors if needed ensuring nox passes across entire code base.

Section: #### M4: Corrections + Subplans + Checkpoints (Day 22)
Status: In Review (PR #444)

## Metadata - **Commit Message**: `feat(subplan): execute and merge subplans` - **Branch**: `feature/m4-subplan-execution` ## Background Subplan execution supports sequential, parallel, and dependency_ordered scheduling modes. Merge strategies (git three-way, sequential apply, fail on conflict, last wins) use sandbox outputs to combine subplan results into the parent plan. ## Acceptance Criteria - [x] Implement subplan execution scheduler for sequential/parallel/dependency_ordered modes. - [x] Implement merge strategies (git three-way, sequential apply, fail on conflict, last wins) using sandbox outputs. - [x] Update `docs/reference/subplans.md` with execution modes and merge outcomes. ## Definition of Done This issue is complete when: - All subtasks below are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. The commit body should be appropriate in size for a commit message and relatively complete in describing what was done. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. ## Subtasks - [x] Implement subplan execution scheduler for sequential/parallel/dependency_ordered modes. - [x] Implement merge strategies (git three-way, sequential apply, fail on conflict, last wins) using sandbox outputs. - [x] Update `docs/reference/subplans.md` with execution modes and merge outcomes. - [x] Tests (Behave): Add scenarios for parallel execution, conflict handling, and merge results. - [x] Tests (Robot): Add Robot test for subplan execution and merge summaries. - [x] Tests (ASV): Add `benchmarks/subplan_execution_bench.py` for scheduler overhead. - [x] Verify coverage >=97% via `nox -s coverage_report`. - [x] Run `nox` (all default sessions, including benchmark), fix any errors if needed ensuring nox passes across **entire** code base. **Section**: #### M4: Corrections + Subplans + Checkpoints (Day 22) **Status**: In Review (PR #444)
freemo added this to the v3.3.0 milestone 2026-02-22 23:39:55 +00:00
Author
Owner

Expected completion updated (Day 15 rebaseline): Day 32 / 2026-03-12 (previously Day 25 / 2026-03-05)

**Expected completion updated (Day 15 rebaseline):** Day 32 / 2026-03-12 (previously Day 25 / 2026-03-05)
freemo added the due date 2026-03-04 2026-02-23 18:41:38 +00:00
Member

Implementation Notes

Design Decisions

  1. Two-service architecture: Split the implementation into SubplanExecutionService (scheduling/orchestration) and SubplanMergeService (merge strategy execution). This follows SRP and keeps each file under 500 lines.

  2. Scheduler design: The execution scheduler dispatches based on ExecutionMode:

    • SEQUENTIAL: Executes subplans one-by-one in order, stopping on failure if fail_fast is set
    • PARALLEL: Uses asyncio.gather with max_parallel concurrency limit via semaphore
    • DEPENDENCY_ORDERED: Topological sort of subplan dependencies, executing independent subplans concurrently
  3. Merge strategy implementation: All four strategies implemented:

    • GIT_THREE_WAY: Leverages git merge-base for conflict detection
    • SEQUENTIAL_APPLY: Applies changesets in subplan completion order
    • FAIL_ON_CONFLICT: Rejects any overlapping file changes
    • LAST_WINS: Latest subplan result takes precedence for conflicting files
  4. Failure handling: Uses existing SubplanFailureHandler for retry/stop-others decisions. Retry attempts are recorded in SubplanAttempt entries.

  5. Dependency injection: Both services accept dependencies as constructor parameters for testability.

Key Code Locations

  • src/cleveragents/application/services/subplan_execution_service.py — Scheduler with sequential/parallel/dependency_ordered modes
  • src/cleveragents/application/services/subplan_merge_service.py — Merge strategy implementations
  • features/subplan_execution.feature — 21 BDD scenarios covering all execution modes and merge strategies
  • robot/subplan_execution.robot — 11 Robot integration test cases
  • benchmarks/subplan_execution_bench.py — ASV benchmarks for scheduler overhead
  • docs/reference/subplans.md — Reference documentation for execution modes and merge outcomes

Test Coverage

  • All 21 Behave scenarios pass (88 steps)
  • All 11 Robot integration tests pass
  • ASV benchmarks run successfully
  • Pyright typecheck: 0 errors, 0 warnings
  • Lint/format: clean

Commit

Branch: feature/m4-subplan-execution
Commit: cad1678feat(subplan): execute and merge subplans

## Implementation Notes ### Design Decisions 1. **Two-service architecture**: Split the implementation into `SubplanExecutionService` (scheduling/orchestration) and `SubplanMergeService` (merge strategy execution). This follows SRP and keeps each file under 500 lines. 2. **Scheduler design**: The execution scheduler dispatches based on `ExecutionMode`: - `SEQUENTIAL`: Executes subplans one-by-one in order, stopping on failure if `fail_fast` is set - `PARALLEL`: Uses `asyncio.gather` with `max_parallel` concurrency limit via semaphore - `DEPENDENCY_ORDERED`: Topological sort of subplan dependencies, executing independent subplans concurrently 3. **Merge strategy implementation**: All four strategies implemented: - `GIT_THREE_WAY`: Leverages git merge-base for conflict detection - `SEQUENTIAL_APPLY`: Applies changesets in subplan completion order - `FAIL_ON_CONFLICT`: Rejects any overlapping file changes - `LAST_WINS`: Latest subplan result takes precedence for conflicting files 4. **Failure handling**: Uses existing `SubplanFailureHandler` for retry/stop-others decisions. Retry attempts are recorded in `SubplanAttempt` entries. 5. **Dependency injection**: Both services accept dependencies as constructor parameters for testability. ### Key Code Locations - `src/cleveragents/application/services/subplan_execution_service.py` — Scheduler with sequential/parallel/dependency_ordered modes - `src/cleveragents/application/services/subplan_merge_service.py` — Merge strategy implementations - `features/subplan_execution.feature` — 21 BDD scenarios covering all execution modes and merge strategies - `robot/subplan_execution.robot` — 11 Robot integration test cases - `benchmarks/subplan_execution_bench.py` — ASV benchmarks for scheduler overhead - `docs/reference/subplans.md` — Reference documentation for execution modes and merge outcomes ### Test Coverage - All 21 Behave scenarios pass (88 steps) - All 11 Robot integration tests pass - ASV benchmarks run successfully - Pyright typecheck: 0 errors, 0 warnings - Lint/format: clean ### Commit Branch: `feature/m4-subplan-execution` Commit: `cad1678` — `feat(subplan): execute and merge subplans`
Member

Code Review Fixes Applied

A code review of commit cad1678b identified 13 findings. The following fixes have been applied to feature/m4-subplan-execution:

Bug Fixes

  • BDD assertion orand: subplan_execution_steps.py step "merged file should contain changes from both subplans" now correctly asserts both changes are present (was or, now and)
  • Narrowed exception handling: SubplanExecutionService.execute_all() merge failure catch narrowed from bare except Exception to except MergeConflictError, per CONTRIBUTING.md exception propagation guidelines. Programming errors now propagate instead of being silently swallowed.

Performance

  • Topological sort uses collections.deque: _topological_sort replaced list.pop(0) (O(n)) with deque.popleft() (O(1))

Correctness

  • _sequential_apply merge semantics: Fixed three-way merge call from merge(current, current, content) to merge(base_content, current, content) — base argument now correctly references the original base content

Test Coverage (87% → 97%)

Added 7 new BDD scenarios covering:

  • Property accessors (config, merge_service, strategy)
  • Non-retriable error handling (no retry for ConfigurationError)
  • Retry exhaustion (max retries exceeded → permanent failure)
  • Executor exception retry path (exception raised, retried, succeeds)
  • Executor exception non-retriable (exception raised, not retried)
  • Merge conflict during execution (fail_on_conflict strategy)

Added 3 new Robot integration tests:

  • retry-non-retriable: Non-retriable error verification
  • retry-exception: Exception retry and recovery
  • merge-conflict-exec: Merge conflict during execution

Documentation

  • Added "Design Decisions and Specification Alignment" section to docs/reference/subplans.md documenting:
    • Execution mode via config enum vs. decision tree (spec deviation rationale)
    • Single merge strategy per plan vs. per-resource-type (spec deviation rationale)
    • Global plan.concurrency and plan.max-child-depth not enforced locally (orchestration-layer concern)

Quality Gates

  • Pyright: 0 errors
  • Ruff lint: all checks passed
  • Ruff format: all files formatted
  • Vulture dead code: passed
  • BDD: 28/28 scenarios passed
  • Robot helpers: 14/14 passed
  • Coverage: 97% (execution service 96%, merge service 97%)

Items Not Fixed (by design)

  • DI container registration: SubplanExecutionService and SubplanMergeService are parameterized per plan execution (take runtime config/strategy). They are not singletons and are intentionally not registered in the DI container.
  • Lines 300-303 and 197: Unreachable defensive code providing safety nets for edge cases in parallel cancellation and merge content lookup. Left in place as defensive programming.
  • Security (pre-existing): GitMergeStrategy subprocess invocation is pre-existing infrastructure code, not introduced by this commit.
## Code Review Fixes Applied A code review of commit `cad1678b` identified 13 findings. The following fixes have been applied to `feature/m4-subplan-execution`: ### Bug Fixes - **BDD assertion `or` → `and`**: `subplan_execution_steps.py` step "merged file should contain changes from both subplans" now correctly asserts both changes are present (was `or`, now `and`) - **Narrowed exception handling**: `SubplanExecutionService.execute_all()` merge failure catch narrowed from bare `except Exception` to `except MergeConflictError`, per CONTRIBUTING.md exception propagation guidelines. Programming errors now propagate instead of being silently swallowed. ### Performance - **Topological sort uses `collections.deque`**: `_topological_sort` replaced `list.pop(0)` (O(n)) with `deque.popleft()` (O(1)) ### Correctness - **`_sequential_apply` merge semantics**: Fixed three-way merge call from `merge(current, current, content)` to `merge(base_content, current, content)` — base argument now correctly references the original base content ### Test Coverage (87% → 97%) Added 7 new BDD scenarios covering: - Property accessors (`config`, `merge_service`, `strategy`) - Non-retriable error handling (no retry for `ConfigurationError`) - Retry exhaustion (max retries exceeded → permanent failure) - Executor exception retry path (exception raised, retried, succeeds) - Executor exception non-retriable (exception raised, not retried) - Merge conflict during execution (fail_on_conflict strategy) Added 3 new Robot integration tests: - `retry-non-retriable`: Non-retriable error verification - `retry-exception`: Exception retry and recovery - `merge-conflict-exec`: Merge conflict during execution ### Documentation - Added "Design Decisions and Specification Alignment" section to `docs/reference/subplans.md` documenting: - Execution mode via config enum vs. decision tree (spec deviation rationale) - Single merge strategy per plan vs. per-resource-type (spec deviation rationale) - Global `plan.concurrency` and `plan.max-child-depth` not enforced locally (orchestration-layer concern) ### Quality Gates - Pyright: 0 errors - Ruff lint: all checks passed - Ruff format: all files formatted - Vulture dead code: passed - BDD: 28/28 scenarios passed - Robot helpers: 14/14 passed - Coverage: 97% (execution service 96%, merge service 97%) ### Items Not Fixed (by design) - **DI container registration**: `SubplanExecutionService` and `SubplanMergeService` are parameterized per plan execution (take runtime config/strategy). They are not singletons and are intentionally not registered in the DI container. - **Lines 300-303 and 197**: Unreachable defensive code providing safety nets for edge cases in parallel cancellation and merge content lookup. Left in place as defensive programming. - **Security (pre-existing)**: `GitMergeStrategy` subprocess invocation is pre-existing infrastructure code, not introduced by this commit.
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

2026-03-04

Blocks
#368 Epic: Subplans & Parallelism
cleveragents/cleveragents-core
Reference
cleveragents/cleveragents-core#184
No description provided.