fix(subplan): propagate invariant_enforced decisions to child plans on spawn #10881
No reviewers
Labels
No labels
auto/needs-reevaluation
controller-managed
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
cleveragents/cleveragents-core!10881
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "bugfix/m3-invariant-propagation"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
SubplanService.spawn()to propagate allinvariant_enforceddecisions from the parent plan's decision tree to each child plan's decision tree_propagate_invariant_decisions()helper method toSubplanServicefeatures/tdd_invariant_propagation_subplan.featurewith 4 scenariosCHANGELOG.mdwith fix entryProblem
SubplanService.spawn()created childPlanobjects with no invariants and noinvariant_enforceddecisions. The child plan started Strategize with a completely empty invariant set, violating the spec's propagation requirement:non_overridableglobal invariants enforced on the parent were not guaranteed to be enforced on child plans.Solution
The fix adds
_propagate_invariant_decisions()toSubplanServicewhich:invariant_enforceddecisions fromDecisionServiceDecisionService.record_decision()The
DecisionServiceis already injected intoSubplanService, so no new dependencies are required.Testing
BDD regression coverage added in
features/tdd_invariant_propagation_subplan.featurewith 4 scenarios:Manual verification script confirms the fix works correctly.
Closes #9131
This PR blocks issue #9131
Automated by CleverAgents Bot
Supervisor: Implementation | Agent: task-implementor
invariant_enforceddecisions not propagated to child plans on subplan spawn #9131Review Summary
This PR addresses Bug #9131:
SubplanService.spawn()was creating child plans with noinvariant_enforceddecisions, violating the spec requirement that invariants are "recorded asinvariant_enforceddecisions that propagate to child plans."What was reviewed
subplan_service.py(core fix +_propagate_invariant_decisions()helper),tdd_invariant_propagation_subplan.feature(4 BDD scenarios),tdd_invariant_propagation_subplan_steps.py(step definitions),CHANGELOG.md(entry added)fix(subplan): propagate invariant_enforced decisions to child plans on spawnISSUES CLOSED: #913110-Category Checklist
1. CORRECTNESS✅
The fix correctly queries the parent plan's
invariant_enforceddecisions viaDecisionService.list_by_type()and re-records each on the child plan viaDecisionService.record_decision(). Therecord_decision()API signature matches the arguments passed (plan_id,decision_type,question,chosen_option,rationale,context_snapshot,confidence_score,alternatives_considered). The empty-list fallback when the parent decision hasNoneforalternatives_consideredis handled correctly.2. SPECIFICATION ALIGNMENT✅
The docstring references the spec: Glossary → Invariant ("recorded as
invariant_enforceddecisions that propagate to child plans"). No code departs from spec; implementation aligns exactly.3. TEST QUALITY✅
Four Behave BDD scenarios cover: single invariant propagation, multiple invariants (3, verifying count fidelity), non-overridable invariant (verifies
chosen_optiontext propagation), and clean spawn with no parent invariants (edge case: empty list). Assertions are specific with descriptive error messages referencing bug #9131.4. TYPE SAFETY✅
All function signatures have proper type annotations. No
# type: ignorecomments present. Theif TYPE_CHECKING:import style forDecisionServiceis correctly used.5. READABILITY✅
Method names are descriptive (
_propagate_invariant_decisions,list_by_type). Variable names are clear. The invariant propagation logic is well-commented with spec references inline.6. PERFORMANCE✅
Parent invariant decisions are queried once upfront before spawning all children. No unnecessary allocations.
7. SECURITY✅
No secrets, credentials, or unsafe patterns. Input validation is handled by the existing
DecisionService.8. CODE STYLE✅
dataclass(frozen=True)for value objects9. DOCUMENTATION✅
Module docstring updated with an "Invariant propagation" section. All methods have docstrings. CHANGELOG entry is descriptive.
10. COMMIT AND PR QUALITY✅
fix(subplan): propagate invariant_enforced decisions to child plans on spawnISSUES CLOSED: #9131bugfix/m3-invariant-propagationmatches milestone m3@tdd_issue,@tdd_issue_9131, and@mock_onlytags appropriatelyConclusion
This is a solid, well-scoped bug fix that correctly implements spec-required invariant propagation with meaningful regression test coverage. No blocking issues found.
Suggestion (non-blocking): Consider documenting whether propagating
context_snapshotfrom parent to child invariant decisions is semantically meaningful, since parent context snapshots may contain parent-scoped data.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +1,38 @@@tdd_issue @tdd_issue_9131 @mock_onlyNon-blocking suggestion: the feature file includes
@tdd_issuetags on both the Feature line and individual Scenario lines. Consider whether scenario-level tags serve an independent tracking purpose beyond feature-scoped tags.Suggestion: consider documenting whether propagating
context_snapshotfrom parent to child invariant decisions is semantically correct. Parent context snapshots may contain parent-scoped data that differs from what the child plan should see.Review Summary
This PR fixes Issue #9131:
SubplanService.spawn()was creating child plans with noinvariant_enforceddecisions, violating the spec requirement that invariants are "recorded asinvariant_enforceddecisions that propagate to child plans."What was reviewed
subplan_service.py(core fix +_propagate_invariant_decisions()helper),tdd_invariant_propagation_subplan.feature(4 BDD scenarios),tdd_invariant_propagation_subplan_steps.py(step definitions),CHANGELOG.md(entry added)fix(subplan): propagate invariant_enforced decisions to child plans on spawnbugfix/m3-invariant-propagation10-Category Checklist
1. CORRECTNESS ✅
The fix correctly queries the parent plan's
invariant_enforceddecisions viaDecisionService.list_by_type()and re-records each on the child plan viaDecisionService.record_decision(). All fields from the parent decision (question,chosen_option,rationale,context_snapshot,confidence_score,alternatives_considered) are faithfully propagated. Thealternatives_consideredhandling withlist()conversion is correct.2. SPECIFICATION ALIGNMENT ✅
The docstring references the spec: Glossary → Invariant ("recorded as
_propagate_invariant_decisions()decisions that propagate to child plans"). No code departs from spec.3. TEST QUALITY ✅
Four Behave BDD scenarios cover: single invariant propagation, multiple invariants (3, verifying count fidelity), non-overridable invariant propagation, and clean spawn with no parent invariants (edge case: empty list). Assertions reference bug #9131 with descriptive error messages. The
@tdd_issueand@tdd_issue_9131tags are used on feature and scenario lines.4. TYPE SAFETY ✅
All function signatures have proper type annotations (
str,list[Decision],Nonereturn). No# type: ignorecomments present.5. READABILITY ✅
Method names are descriptive (
_propagate_invariant_decisions). Variable names (parent_invariant_decisions,child_plan_id) are clear. Logic is straightforward and well-commented with spec references inline.6. PERFORMANCE ✅
Parent invariant decisions are queried once upfront before spawning all children. No unnecessary allocations. The
list()call onalternatives_consideredis minimal overhead.7. SECURITY ✅
No secrets, credentials, or unsafe patterns. Input validation handled by the existing
DecisionService.8. CODE STYLE ✅
_propagate_invariant_decisions()invariant_decisions_propagatedin SpawnMetadata dict adds traceability9. DOCUMENTATION ✅
Module docstring updated with an "Invariant propagation" section. Method-level docstrings present. CHANGELOG.md entry is descriptive. Spec references included inline.
10. COMMIT AND PR QUALITY ❌
fix(subplan): propagate invariant_enforced decisions to child plans on spawn✅ISSUES CLOSED: #9131✅bugfix/m3-invariant-propagationmatches milestone m3 ✅labels: []). Per PR requirements, exactly one Type/ label (Type/Bug, Type/Feature, or Type/Task) is mandatory.CI Status — BLOCKING ❌
lint (pull_request): FAILURE (failing after 52s) ❌status-check (pull_request): FAILURE (failing after 7s) ❌coverage (pull_request): SKIPPED ❌Non-Blocking Suggestions
context_snapshotfrom parent to child invariant decisions is semantically meaningful, since parent context snapshots may contain parent-scoped data that is misleading in the child's context. This is a design question, not a bug.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Implementation Attempt — Tier 1: kimi — Success
What was done
Fixed the CI lint failure that was blocking this PR. The
lintCI job runs two steps:nox -s lint(ruff check) — was already passingnox -s format -- --check(ruff format check) — was failingTwo files had ruff formatting violations (line-length issues causing wrapped expressions):
features/steps/tdd_invariant_propagation_subplan_steps.py— 5 wrapped method call expressions reformattedsrc/cleveragents/application/services/subplan_service.py— 1 wrappedlist()call reformattedApplied
nox -s formatto auto-fix both files. No logic changes — pure whitespace/line-length reformatting.Quality gate status
Reviewer feedback
The REQUEST_CHANGES review (id 7071) flagged:
Type/Buglabel manually.The non-blocking suggestions (context_snapshot semantics, scenario-level tags) were not addressed as they are explicitly non-blocking.
Automated by CleverAgents Bot
Supervisor: Implementation | Agent: implementation-worker
CI Status Update — Tier 1: kimi
Lint fix confirmed working ✅
The format fix pushed in the previous attempt resolved the original CI failure:
Remaining integration_tests failure is a CI namespace collision ⚠️
The CI / integration_tests (pull_request): failure status shown against our commit
e9526c1eis NOT caused by this PR. Investigation shows:e9526c1epassed: lint ✅, typecheck ✅, security ✅, quality ✅, unit_tests ✅, e2e_tests ✅, coverage ✅, build ✅, helm ✅, push-validation ✅, docker ✅The status-check failure is a cascade from the cross-contaminated integration_tests status.
Recommendation
This PR is ready for merge. The integration_tests failure is a CI infrastructure issue (status namespace collision from a concurrent PR), not a code issue. A reviewer can verify by checking that run 18437 belongs to PR #9912, not this PR.
The only remaining open item is the missing Type/Bug label, which requires manual addition by a reviewer (label API is restricted for bot accounts).
Automated by CleverAgents Bot
Supervisor: Implementation | Agent: implementation-worker
Re-Review Summary — PR #10881
Prior Feedback Addressed
The previous
REQUEST_CHANGESreview (id 7071) raised two blockers:CI lint failure (ruff format violations) — ✅ ADDRESSED. Commit
e9526c1ecorrectly fixed the ruff format violations insubplan_service.pyandtdd_invariant_propagation_subplan_steps.py. CI now showslint: success(1m14s) on the current head SHA.Missing Type/ label — ❌ NOT ADDRESSED. The PR still has
labels: []. The implementation agent noted the label API is restricted for the bot account and a human must add it manually. This remains a mandatory merge blocker per PR requirements.CI Status (head SHA
e9526c1e)All required CI gates pass:
The
integration_tests: failureand cascadestatus-check: failureare a known CI infrastructure artefact: run 18437 has statuscancelled, and the failure status was written to the commit before cancellation. All other CI jobs running againste9526c1esucceeded. This is NOT a code issue.New Issues Found in This Revision
BLOCKER — Commit history has two commits instead of one
The PR now has two commits:
2622fa56—fix(subplan): propagate invariant_enforced decisions to child plans on spawne9526c1e—style: fix ruff format violations in subplan_service and tdd stepsPer CONTRIBUTING.md, one issue maps to exactly one commit. The style fix commit should be squashed into the original fix commit before merge. The author should run
git rebase -i HEAD~2, squash the two commits into one, and force-push the branch. The resulting single commit should retain the original first line (fix(subplan): propagate invariant_enforced decisions to child plans on spawn) with the ISSUES CLOSED: #9131 footer.BLOCKER — Style commit missing
ISSUES CLOSED:footerCommit
e9526c1e(style: fix ruff format violations...) has noISSUES CLOSED:orRefs:footer. Per CONTRIBUTING.md, every commit footer must reference its issue. This is moot once the commits are squashed (see above), but is a violation in the current state.BLOCKER — Missing Type/ label (carried from prior review)
The PR has
labels: []. Exactly one Type/ label is mandatory for merge. This PR addresses a bug so it requiresType/Bug. A human reviewer or maintainer with label write-access must addType/Bugto the PR.10-Category Checklist (current state)
CORRECTNESS ✅ — The invariant propagation fix is correct.
_propagate_invariant_decisions()queries parentinvariant_enforceddecisions once and re-records each on the child plan. Logic is sound.SPECIFICATION ALIGNMENT ✅ — Implementation matches the spec: Glossary → Invariant: "recorded as
invariant_enforceddecisions that propagate to child plans."TEST QUALITY ✅ — Four BDD scenarios cover single propagation, multiple (3) propagation, non-overridable invariant, and empty parent. Assertions are specific with descriptive error messages referencing bug #9131.
TYPE SAFETY ✅ — All function signatures are annotated. No
# type: ignorecomments added. Correct use oflist[Decision]andNonereturn types.READABILITY ✅ — Method names are descriptive. Inline spec references are helpful. Variable names are clear.
PERFORMANCE ✅ — Parent invariant decisions queried once upfront before the child plan loop. No N+1 pattern.
SECURITY ✅ — No secrets, credentials, or unsafe patterns introduced.
CODE STYLE ✅ — After the format fix, ruff conventions are met. SOLID principles upheld.
DOCUMENTATION ✅ — Module docstring updated with "Invariant propagation" section. Method docstrings present. CHANGELOG entry is descriptive.
COMMIT AND PR QUALITY ❌ — See blockers above: two commits instead of one; style commit lacks issue footer; missing Type/ label.
What Must Be Done Before Approval
Squash the two commits into a single clean commit:
Add
Type/Buglabel to the PR — requires a human with label write-access.Once these two items are addressed, the code is otherwise ready for approval — all 9 other review categories pass cleanly.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
e9526c1e9d55073e20b955073e20b9e8f476b324Re-Review — APPROVED
Prior Blocking Issues Resolved
All three blockers raised in the previous
REQUEST_CHANGESreview (ID 7766) have been fully addressed:e8f476b3(2026-05-12T01:23:02Z). PR now has exactly one commit.Type/BuglabelType/Buglabel now present.ISSUES CLOSED:footerISSUES CLOSED: #9131.10-Category Assessment
CORRECTNESS ✅ —
_propagate_invariant_decisions()correctly queries parentinvariant_enforceddecisions viaDecisionService.list_by_type()once upfront, then re-records each on the child plan inside the spawn loop. All decision fields propagated faithfully:question,chosen_option,rationale,context_snapshot,confidence_score,alternatives_considered. No N+1 pattern.SPECIFICATION ALIGNMENT ✅ — Implementation satisfies the spec requirement verbatim: "recorded as
invariant_enforceddecisions that propagate to child plans." (Glossary → Invariant). Inline spec citations are consistent throughout the docstrings.TEST QUALITY ✅ — Four Behave BDD scenarios cover all meaningful cases: single invariant propagation, 3-invariant propagation (count fidelity), non-overridable invariant propagation (content fidelity via
chosen_optionassertion), and empty-parent clean spawn. Scenarios correctly tagged@tdd_issue @tdd_issue_9131with@tdd_expected_failabsent — correct for the fix PR. RealDecisionServiceused throughout; no mocks. Assertion messages are specific and reference bug #9131.TYPE SAFETY ✅ — All new and modified function signatures carry complete type annotations. No
# type: ignorecomments introduced. Pyright typecheck CI gate passes.READABILITY ✅ — Method names (
_propagate_invariant_decisions,spawn_entries,parent_invariant_decisions) are clear and self-documenting. Inline spec references aid future maintainers.PERFORMANCE ✅ — Parent invariant decisions are queried once before the child-plan loop, not per-child. No N+1 pattern. The
list()conversion ofalternatives_consideredis minimal overhead.SECURITY ✅ — No secrets, credentials, or unsafe patterns. Input validation is handled by the existing
DecisionService.CODE STYLE ✅ — SOLID principles upheld. SRP:
_propagate_invariant_decisionshas a single responsibility. DI:DecisionServiceis injected, not created internally. File is well under 500 lines. ruff conventions met (CI lint passes).DOCUMENTATION ✅ — Module docstring updated with an "Invariant propagation" section including spec citation and design rationale. Method-level docstrings present on all new methods. CHANGELOG entry is descriptive and includes spec reference.
CONTRIBUTORS.mdupdated.COMMIT AND PR QUALITY ✅ — Single atomic commit. First line follows Conventional Changelog format.
ISSUES CLOSED: #9131in footer. Branchbugfix/m3-invariant-propagationmatches milestone m3 (v3.2.0). Milestone v3.2.0 assigned.Type/Buglabel applied.CONTRIBUTORS.mdupdated. CHANGELOG updated. PR description containsCloses #9131andThis PR blocks issue #9131(correct dependency direction).CI Status
All required gates pass (lint, typecheck, security, unit_tests, integration_tests, e2e_tests, coverage, build). CI combined state:
success.Non-Blocking Note
The earlier suggestion (from review 7071) about whether propagating
context_snapshotfrom parent to child invariant decisions is semantically meaningful remains an open design question. The current implementation propagates it, which preserves full provenance. This is a defensible choice and is not a correctness issue.✅ Approved for merge. Note: branch is stale with conflicts — the merge worker will need to perform a local rebase before merging.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
invariant_enforceddecisions not propagated to child plans on subplan spawn #9131e8f476b3249cfa1dd1d7