cleveragents/cleveragents-core

Fork 3

fix(autonomy): correct child ID linkage in _build_hierarchy() #1225

New issue

Open

opened 2026-03-31 09:58:47 +00:00 by brent.edwards · 4 comments

brent.edwards commented

2026-03-31 09:58:47 +00:00

Member

Metadata

Commit Message: fix(autonomy): correct child ID linkage in _build_hierarchy()
Branch: bugfix/m6-build-hierarchy-child-ids

Background and Context

During review of PR #1201 (parallel execution scaling, issue #855), @freemo identified that a bug fix to _build_hierarchy() in decomposition_service.py was bundled with the feature implementation. Per CONTRIBUTING.md §Atomic Commits ("One logical change per commit"), the bug fix was reverted from PR #1201 and split into this separate issue.

Current Behavior

In DecompositionService._build_hierarchy(), the child linkage code uses:

children_ids.append(nodes[-1].node_id)

This appends the node ID of the last node in the nodes list, which is only correct when exactly one child is added per iteration. When multiple children are added in a loop, nodes[-1] always refers to the most recently appended node, causing all prior children to be linked to the wrong node ID.

Expected Behavior

The code should use the child_node_id returned from each recursive _build_hierarchy() call:

children_ids.append(child_node_id)

This ensures each child is linked to the correct node ID returned from its own recursive decomposition.

Acceptance Criteria

_build_hierarchy() uses the return value from recursive calls for child ID linkage
Existing decomposition tests continue to pass
Coverage >= 97%

Supporting Information

Discovered during review of PR #1201 (review #2909 by @freemo, Issue 2)
Affects src/cleveragents/application/services/decomposition_service.py, method _build_hierarchy()

Subtasks

Code: Fix children_ids.append(nodes[-1].node_id) to use the child node ID returned from the recursive call
Code: Update _build_hierarchy() return type to tuple[int, str] to return both max_depth and node_id
Tests (Behave): Verify existing decomposition scenarios pass with the fix
Quality: Verify coverage >= 97% via nox -s coverage_report
Quality: Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

All subtasks above 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 fix.
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.

## Metadata - **Commit Message**: `fix(autonomy): correct child ID linkage in _build_hierarchy()` - **Branch**: `bugfix/m6-build-hierarchy-child-ids` ## Background and Context During review of PR #1201 (parallel execution scaling, issue #855), @freemo identified that a bug fix to `_build_hierarchy()` in `decomposition_service.py` was bundled with the feature implementation. Per CONTRIBUTING.md §Atomic Commits ("One logical change per commit"), the bug fix was reverted from PR #1201 and split into this separate issue. ## Current Behavior In `DecompositionService._build_hierarchy()`, the child linkage code uses: ```python children_ids.append(nodes[-1].node_id) ``` This appends the node ID of the last node in the `nodes` list, which is only correct when exactly one child is added per iteration. When multiple children are added in a loop, `nodes[-1]` always refers to the most recently appended node, causing all prior children to be linked to the wrong node ID. ## Expected Behavior The code should use the `child_node_id` returned from each recursive `_build_hierarchy()` call: ```python children_ids.append(child_node_id) ``` This ensures each child is linked to the correct node ID returned from its own recursive decomposition. ## Acceptance Criteria - [x] `_build_hierarchy()` uses the return value from recursive calls for child ID linkage - [x] Existing decomposition tests continue to pass - [x] Coverage >= 97% ## Supporting Information - Discovered during review of PR #1201 (review #2909 by @freemo, Issue 2) - Affects `src/cleveragents/application/services/decomposition_service.py`, method `_build_hierarchy()` ## Subtasks - [x] Code: Fix `children_ids.append(nodes[-1].node_id)` to use the child node ID returned from the recursive call - [x] Code: Update `_build_hierarchy()` return type to `tuple[int, str]` to return both `max_depth` and `node_id` - [x] Tests (Behave): Verify existing decomposition scenarios pass with the fix - [x] Quality: Verify coverage >= 97% via `nox -s coverage_report` - [x] Quality: Run `nox` (all default sessions), fix any errors ## Definition of Done This issue is complete when: - All subtasks above 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 fix. - 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.

brent.edwards added this to the v3.5.0 milestone

2026-03-31 09:59:31 +00:00

brent.edwards added the

labels

2026-03-31 09:59:32 +00:00

brent.edwards referenced this issue from a commit

2026-03-31 11:06:05 +00:00

fix(autonomy): correct child ID linkage in _build_hierarchy()

brent.edwards referenced this issue from a pull request that will close it,

2026-03-31 11:06:19 +00:00

fix(autonomy): correct child ID linkage in _build_hierarchy() #1227

brent.edwards commented

2026-03-31 11:07:21 +00:00

Author

Member

Implementation Notes

Changes Made

File: src/cleveragents/application/services/decomposition_service.py

Return type change — DecompositionService._build_hierarchy() return type changed from int to tuple[int, str]. The tuple now returns (max_depth, node_id), propagating the root node ID created by each recursive call back to the caller.
Child ID linkage fix — In the recursive loop within _build_hierarchy(), the call now unpacks as child_depth, child_node_id = self._build_hierarchy(...) and uses children_ids.append(child_node_id) instead of the fragile children_ids.append(nodes[-1].node_id).
Caller update — DecompositionService.decompose() now unpacks the return as max_depth, _ = self._build_hierarchy(...) (the root node_id is not needed at the top level).
All return paths updated — All three return statements in _build_hierarchy() (leaf node, fallback, internal node) now return (depth_value, node_id).

Design Rationale

The previous implementation relied on nodes[-1] being the child root after each recursive call. While this happens to work for leaf nodes (only one node appended), it is semantically incorrect for internal nodes in a multi-level subtree — the last node appended in a deep recursion is the deepest internal node, not the immediate child root. The fix makes the child ID linkage explicit via the return value, eliminating the dependency on list ordering.

Quality Gate Results

Gate	Result
`nox -s lint`	✅ Pass
`nox -s typecheck` (Pyright)	✅ Pass
`nox -s unit_tests` (Behave)	✅ Pass
`nox -s coverage_report`	✅ Pass — 97.0% (threshold: 97%)
`nox -s integration_tests` (Robot)	✅ Pass

PR

PR #1227 — bugfix/m6-build-hierarchy-child-ids → master
Commit: 6e3cfcca

## Implementation Notes ### Changes Made **File:** `src/cleveragents/application/services/decomposition_service.py` 1. **Return type change** — `DecompositionService._build_hierarchy()` return type changed from `int` to `tuple[int, str]`. The tuple now returns `(max_depth, node_id)`, propagating the root node ID created by each recursive call back to the caller. 2. **Child ID linkage fix** — In the recursive loop within `_build_hierarchy()`, the call now unpacks as `child_depth, child_node_id = self._build_hierarchy(...)` and uses `children_ids.append(child_node_id)` instead of the fragile `children_ids.append(nodes[-1].node_id)`. 3. **Caller update** — `DecompositionService.decompose()` now unpacks the return as `max_depth, _ = self._build_hierarchy(...)` (the root node_id is not needed at the top level). 4. **All return paths updated** — All three return statements in `_build_hierarchy()` (leaf node, fallback, internal node) now return `(depth_value, node_id)`. ### Design Rationale The previous implementation relied on `nodes[-1]` being the child root after each recursive call. While this happens to work for leaf nodes (only one node appended), it is semantically incorrect for internal nodes in a multi-level subtree — the last node appended in a deep recursion is the deepest internal node, not the immediate child root. The fix makes the child ID linkage explicit via the return value, eliminating the dependency on list ordering. ### Quality Gate Results | Gate | Result | |------|--------| | `nox -s lint` | ✅ Pass | | `nox -s typecheck` (Pyright) | ✅ Pass | | `nox -s unit_tests` (Behave) | ✅ Pass | | `nox -s coverage_report` | ✅ Pass — 97.0% (threshold: 97%) | | `nox -s integration_tests` (Robot) | ✅ Pass | ### PR PR #1227 — `bugfix/m6-build-hierarchy-child-ids` → `master` Commit: `6e3cfcca`

brent.edwards added

and removed

labels

2026-03-31 11:07:25 +00:00

freemo self-assigned this

2026-04-02 06:13:54 +00:00

freemo commented

2026-04-02 16:51:11 +00:00

Owner

PR #1227 Review Outcome: Changes Requested

The core bugfix (tuple return type for _build_hierarchy() to propagate child node IDs correctly) is well-implemented and correct. However, PR #1227 has merge conflicts with master that must be resolved before it can be merged.

Key conflicts:

Leaf condition in _build_hierarchy() has diverged — master has a newer version
Fallback guard condition has diverged — master added a non-progress guard
CHANGELOG.md conflicts

The implementer needs to rebase onto latest master, resolve conflicts (keeping master's newer logic while applying the tuple return fix), re-run quality gates, and force-push.

See PR #1227 review for full details.

**PR #1227 Review Outcome: Changes Requested** The core bugfix (tuple return type for `_build_hierarchy()` to propagate child node IDs correctly) is well-implemented and correct. However, PR #1227 has **merge conflicts with `master`** that must be resolved before it can be merged. Key conflicts: 1. Leaf condition in `_build_hierarchy()` has diverged — master has a newer version 2. Fallback guard condition has diverged — master added a non-progress guard 3. CHANGELOG.md conflicts The implementer needs to rebase onto latest `master`, resolve conflicts (keeping master's newer logic while applying the tuple return fix), re-run quality gates, and force-push. See [PR #1227 review](https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/1227#issuecomment-77832) for full details.

freemo referenced this issue

2026-04-02 17:29:22 +00:00

fix(autonomy): correct child ID linkage in _build_hierarchy() #1227

freemo referenced this issue

2026-04-02 17:41:02 +00:00

fix(autonomy): correct child ID linkage in _build_hierarchy() #1227

freemo commented

2026-04-02 17:41:10 +00:00

Owner

PR #1227 Review Status Update

PR #1227 (fix(autonomy): correct child ID linkage in _build_hierarchy()) was closed without merge by an automated groomer that incorrectly identified it as a duplicate. The PR was intended to close this issue via Closes #1225.

Review Findings

The core bugfix is correct and well-designed — changing _build_hierarchy() to return tuple[int, str] and using child_node_id directly instead of the fragile nodes[-1].node_id is the right approach.

However, the PR has unresolved merge conflicts with master (identified in review #3139):

Leaf condition divergence in decomposition_service.py
Fallback guard divergence (non-progress guard added to master)
CHANGELOG.md conflicts

Required Action

The implementing agent needs to:

Reopen PR #1227 or create a fresh PR from a rebased branch
Rebase bugfix/m6-build-hierarchy-child-ids onto latest master
Resolve all conflicts (keeping master's updated leaf condition and non-progress guard)
Re-run nox quality gates
Push for re-review

This issue remains open and in State/In Review until the rebased PR is submitted and merged.

## PR #1227 Review Status Update PR #1227 (`fix(autonomy): correct child ID linkage in _build_hierarchy()`) was **closed without merge** by an automated groomer that incorrectly identified it as a duplicate. The PR was intended to close this issue via `Closes #1225`. ### Review Findings The core bugfix is **correct and well-designed** — changing `_build_hierarchy()` to return `tuple[int, str]` and using `child_node_id` directly instead of the fragile `nodes[-1].node_id` is the right approach. However, the PR has **unresolved merge conflicts** with `master` (identified in review #3139): - Leaf condition divergence in `decomposition_service.py` - Fallback guard divergence (non-progress guard added to master) - CHANGELOG.md conflicts ### Required Action The implementing agent needs to: 1. Reopen PR #1227 or create a fresh PR from a rebased branch 2. Rebase `bugfix/m6-build-hierarchy-child-ids` onto latest `master` 3. Resolve all conflicts (keeping master's updated leaf condition and non-progress guard) 4. Re-run `nox` quality gates 5. Push for re-review This issue remains **open** and in `State/In Review` until the rebased PR is submitted and merged.

freemo removed the

labels

2026-04-03 00:30:01 +00:00

freemo commented

2026-04-03 03:55:49 +00:00

Owner

⚠️ State/In Review compliance check: This issue is marked State/In Review but no open PR was found for branch bugfix/m6-build-hierarchy-child-ids. All subtasks are checked off, suggesting the work may be complete.

Please either:

Create a PR for branch bugfix/m6-build-hierarchy-child-ids if the code is ready for review, OR
If the PR was already merged, close this issue and update the state to State/Completed

Automated by CleverAgents Bot
Supervisor: Backlog Grooming | Agent: ca-backlog-groomer

⚠️ **State/In Review compliance check**: This issue is marked `State/In Review` but no open PR was found for branch `bugfix/m6-build-hierarchy-child-ids`. All subtasks are checked off, suggesting the work may be complete. Please either: 1. Create a PR for branch `bugfix/m6-build-hierarchy-child-ids` if the code is ready for review, OR 2. If the PR was already merged, close this issue and update the state to `State/Completed` --- **Automated by CleverAgents Bot** Supervisor: Backlog Grooming | Agent: ca-backlog-groomer

freemo referenced this issue

2026-04-03 03:56:02 +00:00

[Automated] Product Build Session State — v3.7.0 Production Sprint #2006