UAT: Cross-actor subgraph cycle detection broken — reads from config dict instead of actor_ref field #1431

Closed
opened 2026-04-02 17:50:31 +00:00 by freemo · 13 comments
Owner

Bug Report

Feature Area: Actor Graphs (v3.3.0)
Tested by: UAT instance uat-worker-actor-graphs-1

What Was Tested

Cross-actor subgraph cycle detection in the actor compiler.

Expected Behavior (from spec)

The spec requires that cyclic subgraph references across actors are detected at compile time and raise a SubgraphCycleError. If Actor A has a subgraph node referencing Actor B, and Actor B has a subgraph node referencing Actor A, this should be detected as a cycle.

Actual Behavior

The _detect_subgraph_cycles function in src/cleveragents/actor/compiler.py reads the actor reference from node.config.get("actor_ref", "") instead of node.actor_ref. Since actor_ref is stored as a top-level field on NodeDefinition (not inside config), the function always gets an empty string and never detects cross-actor cycles.

Steps to Reproduce

from cleveragents.actor.compiler import _detect_subgraph_cycles
from cleveragents.actor.schema import ActorConfigSchema
import yaml

# Two actors that reference each other (cycle)
actor_a = ActorConfigSchema.model_validate(yaml.safe_load("""
name: test/actor-a
type: graph
model: gpt-4
description: Actor A
route:
  nodes:
    - id: start
      type: agent
      name: Start
      description: Start
      config: {model: gpt-4}
    - id: sub
      type: subgraph
      name: Sub
      description: Subgraph
      actor_ref: test/actor-b
  edges:
    - from_node: start
      to_node: sub
  entry_node: start
  exit_nodes: [sub]
"""))

actor_b = ActorConfigSchema.model_validate(yaml.safe_load("""
name: test/actor-b
type: graph
model: gpt-4
description: Actor B
route:
  nodes:
    - id: start
      type: agent
      name: Start
      description: Start
      config: {model: gpt-4}
    - id: sub
      type: subgraph
      name: Sub
      description: Subgraph
      actor_ref: test/actor-a
  edges:
    - from_node: start
      to_node: sub
  entry_node: start
  exit_nodes: [sub]
"""))

registry = {'test/actor-a': actor_a, 'test/actor-b': actor_b}
cycles = _detect_subgraph_cycles('test/actor-a', actor_a.route.nodes, registry.get, frozenset({'test/actor-a'}))
print(cycles)  # [] — should be non-empty (cycle detected)

Code Location

src/cleveragents/actor/compiler.py, function _detect_subgraph_cycles() line ~197:

ref_name = node.config.get("actor_ref", "")

Should be:

ref_name = node.actor_ref or ""

Severity

High — cross-actor cycle detection is a safety feature. Without it, infinite recursion can occur at runtime when subgraph actors reference each other.


Metadata

  • Branch: bugfix/actor-compiler-cycle-detection-actor-ref
  • Commit Message: fix(actor): read actor_ref from NodeDefinition field in _detect_subgraph_cycles
  • Milestone: v3.3.0
  • Parent Epic: (needs manual linking — Actor Graphs epic)

Subtasks

  • Fix _detect_subgraph_cycles() in src/cleveragents/actor/compiler.py to read node.actor_ref instead of node.config.get("actor_ref", "")
  • Tests (Behave): Add scenario in features/ covering cross-actor cycle detection with actor_ref field
  • Tests (Robot): Add integration test verifying SubgraphCycleError is raised for mutually-referencing actors
  • Verify coverage >= 97% via nox -s coverage_report
  • Run nox (all default sessions), fix any errors

Definition of Done

  • _detect_subgraph_cycles() reads node.actor_ref (not node.config.get("actor_ref", ""))
  • Mutually-referencing actors raise SubgraphCycleError at compile time
  • No false negatives: cycles are reliably detected regardless of how actor_ref is set
  • All nox stages pass
  • Coverage >= 97%
## Bug Report **Feature Area:** Actor Graphs (v3.3.0) **Tested by:** UAT instance uat-worker-actor-graphs-1 ### What Was Tested Cross-actor subgraph cycle detection in the actor compiler. ### Expected Behavior (from spec) The spec requires that cyclic subgraph references across actors are detected at compile time and raise a `SubgraphCycleError`. If Actor A has a subgraph node referencing Actor B, and Actor B has a subgraph node referencing Actor A, this should be detected as a cycle. ### Actual Behavior The `_detect_subgraph_cycles` function in `src/cleveragents/actor/compiler.py` reads the actor reference from `node.config.get("actor_ref", "")` instead of `node.actor_ref`. Since `actor_ref` is stored as a top-level field on `NodeDefinition` (not inside `config`), the function always gets an empty string and never detects cross-actor cycles. ### Steps to Reproduce ```python from cleveragents.actor.compiler import _detect_subgraph_cycles from cleveragents.actor.schema import ActorConfigSchema import yaml # Two actors that reference each other (cycle) actor_a = ActorConfigSchema.model_validate(yaml.safe_load(""" name: test/actor-a type: graph model: gpt-4 description: Actor A route: nodes: - id: start type: agent name: Start description: Start config: {model: gpt-4} - id: sub type: subgraph name: Sub description: Subgraph actor_ref: test/actor-b edges: - from_node: start to_node: sub entry_node: start exit_nodes: [sub] """)) actor_b = ActorConfigSchema.model_validate(yaml.safe_load(""" name: test/actor-b type: graph model: gpt-4 description: Actor B route: nodes: - id: start type: agent name: Start description: Start config: {model: gpt-4} - id: sub type: subgraph name: Sub description: Subgraph actor_ref: test/actor-a edges: - from_node: start to_node: sub entry_node: start exit_nodes: [sub] """)) registry = {'test/actor-a': actor_a, 'test/actor-b': actor_b} cycles = _detect_subgraph_cycles('test/actor-a', actor_a.route.nodes, registry.get, frozenset({'test/actor-a'})) print(cycles) # [] — should be non-empty (cycle detected) ``` ### Code Location `src/cleveragents/actor/compiler.py`, function `_detect_subgraph_cycles()` line ~197: ```python ref_name = node.config.get("actor_ref", "") ``` Should be: ```python ref_name = node.actor_ref or "" ``` ### Severity High — cross-actor cycle detection is a safety feature. Without it, infinite recursion can occur at runtime when subgraph actors reference each other. --- ## Metadata - **Branch**: `bugfix/actor-compiler-cycle-detection-actor-ref` - **Commit Message**: `fix(actor): read actor_ref from NodeDefinition field in _detect_subgraph_cycles` - **Milestone**: v3.3.0 - **Parent Epic**: *(needs manual linking — Actor Graphs epic)* ## Subtasks - [ ] Fix `_detect_subgraph_cycles()` in `src/cleveragents/actor/compiler.py` to read `node.actor_ref` instead of `node.config.get("actor_ref", "")` - [ ] Tests (Behave): Add scenario in `features/` covering cross-actor cycle detection with `actor_ref` field - [ ] Tests (Robot): Add integration test verifying `SubgraphCycleError` is raised for mutually-referencing actors - [ ] Verify coverage >= 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done - [ ] `_detect_subgraph_cycles()` reads `node.actor_ref` (not `node.config.get("actor_ref", "")`) - [ ] Mutually-referencing actors raise `SubgraphCycleError` at compile time - [ ] No false negatives: cycles are reliably detected regardless of how `actor_ref` is set - [ ] All nox stages pass - [ ] Coverage >= 97%
freemo self-assigned this 2026-04-02 18:45:11 +00:00
Author
Owner

PR #1489 Review: Changes Requested

PR #1489 (fix/1431-subgraph) was reviewed and changes were requested. The PR has critical issues:

  1. The actual bug is not fixedsrc/cleveragents/actor/compiler.py is not modified. Lines 197 and 293 still read node.config.get("actor_ref", "") instead of node.actor_ref or "".
  2. Incorrect find-and-replace — The PR replaces "config dict" with "actor_ref" in ~24 files of docstrings/comments/step definitions. These are different concepts ("config dict" = configuration dictionary, "actor_ref" = actor reference field).
  3. Breaks 12+ Behave scenarios — Step decorator strings were changed but .feature files were not updated, causing step definition mismatches.
  4. Grammatically broken text — e.g., "a actor_refionary" (was "a config dictionary").

The PR needs to be reworked from scratch: revert all find-and-replace changes, fix the actual bug in compiler.py, and add proper tests.

See PR #1489 review comment for full details.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## PR #1489 Review: Changes Requested PR #1489 (`fix/1431-subgraph`) was reviewed and **changes were requested**. The PR has critical issues: 1. **The actual bug is not fixed** — `src/cleveragents/actor/compiler.py` is not modified. Lines 197 and 293 still read `node.config.get("actor_ref", "")` instead of `node.actor_ref or ""`. 2. **Incorrect find-and-replace** — The PR replaces "config dict" with "actor_ref" in ~24 files of docstrings/comments/step definitions. These are different concepts ("config dict" = configuration dictionary, "actor_ref" = actor reference field). 3. **Breaks 12+ Behave scenarios** — Step decorator strings were changed but `.feature` files were not updated, causing step definition mismatches. 4. **Grammatically broken text** — e.g., `"a actor_refionary"` (was `"a config dictionary"`). The PR needs to be reworked from scratch: revert all find-and-replace changes, fix the actual bug in `compiler.py`, and add proper tests. See [PR #1489 review comment](https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/1489#issuecomment-81988) for full details. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Author
Owner

PR #1489 Review: Changes Requested (Second Independent Review)

PR #1489 (fix/1431-subgraph) was reviewed independently and changes were requested. The PR has critical issues:

  1. The actual bug is not fixedcompiler.py is not modified at all. The node.config.get("actor_ref", "") bug on lines ~197 and ~293 remains.
  2. Destructive find-and-replace — A blanket replacement of "config dict" → "actor_ref" across 27 files corrupts docstrings, comments, and Gherkin step definitions with nonsensical text (e.g., "actor_refionary").
  3. Broken Gherkin steps — 12+ step definitions no longer match their .feature files, causing CI failures.
  4. Regressions — Reverts legitimate fixes for #1471 (tool.py wrapper key handling), removes CHANGELOG entries, and reverts timeline data.
  5. No tests — Missing required Behave and Robot tests for cycle detection.

The PR needs a complete rewrite to actually fix the bug in compiler.py and remove all the incorrect find-and-replace changes. See the detailed review on PR #1489 for the full list of required actions.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## PR #1489 Review: Changes Requested (Second Independent Review) PR #1489 (`fix/1431-subgraph`) was reviewed independently and **changes were requested**. The PR has critical issues: 1. **The actual bug is not fixed** — `compiler.py` is not modified at all. The `node.config.get("actor_ref", "")` bug on lines ~197 and ~293 remains. 2. **Destructive find-and-replace** — A blanket replacement of "config dict" → "actor_ref" across 27 files corrupts docstrings, comments, and Gherkin step definitions with nonsensical text (e.g., "actor_refionary"). 3. **Broken Gherkin steps** — 12+ step definitions no longer match their `.feature` files, causing CI failures. 4. **Regressions** — Reverts legitimate fixes for #1471 (tool.py wrapper key handling), removes CHANGELOG entries, and reverts timeline data. 5. **No tests** — Missing required Behave and Robot tests for cycle detection. The PR needs a complete rewrite to actually fix the bug in `compiler.py` and remove all the incorrect find-and-replace changes. See the detailed review on PR #1489 for the full list of required actions. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Author
Owner

PR #1489 Review Update — Changes Still Required

PR #1489 has been reviewed for a third time. None of the changes requested in the two prior reviews have been addressed. The PR still contains:

  1. The actual bug is not fixedcompiler.py is not modified at all
  2. Destructive find-and-replace of "config dict" → "actor_ref" across 24 files (semantically wrong, produces nonsensical text)
  3. Broken Gherkin step definitions — step text changed but .feature files not updated (12+ scenarios will fail)
  4. No tests added for the actual bug fix
  5. Incorrect commit message format

The PR needs a complete rework per the detailed feedback in the review comments.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## PR #1489 Review Update — Changes Still Required PR #1489 has been reviewed for a third time. **None of the changes requested in the two prior reviews have been addressed.** The PR still contains: 1. **The actual bug is not fixed** — `compiler.py` is not modified at all 2. **Destructive find-and-replace** of "config dict" → "actor_ref" across 24 files (semantically wrong, produces nonsensical text) 3. **Broken Gherkin step definitions** — step text changed but `.feature` files not updated (12+ scenarios will fail) 4. **No tests added** for the actual bug fix 5. **Incorrect commit message format** The PR needs a complete rework per the detailed feedback in the review comments. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Author
Owner

PR #1489 Review Status: Changes Requested (4th Review)

PR #1489 has been reviewed again. None of the previously requested changes from 3 prior reviews have been addressed. The PR has not been updated since its original push on 2026-04-02.

Outstanding Issues:

  1. The actual bug is not fixedcompiler.py is not modified in the PR at all
  2. Destructive find-and-replace — 24 files have "config dict" incorrectly replaced with "actor_ref" throughout docstrings, comments, and step decorators
  3. 12+ Behave scenarios will break — step decorator strings were changed but .feature files were not updated
  4. No tests added — neither Behave nor Robot tests for cycle detection
  5. Commit message format incorrect — should be fix(actor): ... not fix(v3.7.0): ...

The PR needs a complete rework to address the actual bug described in this issue.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## PR #1489 Review Status: Changes Requested (4th Review) PR #1489 has been reviewed again. **None of the previously requested changes from 3 prior reviews have been addressed.** The PR has not been updated since its original push on 2026-04-02. ### Outstanding Issues: 1. **The actual bug is not fixed** — `compiler.py` is not modified in the PR at all 2. **Destructive find-and-replace** — 24 files have "config dict" incorrectly replaced with "actor_ref" throughout docstrings, comments, and step decorators 3. **12+ Behave scenarios will break** — step decorator strings were changed but `.feature` files were not updated 4. **No tests added** — neither Behave nor Robot tests for cycle detection 5. **Commit message format incorrect** — should be `fix(actor): ...` not `fix(v3.7.0): ...` The PR needs a complete rework to address the actual bug described in this issue. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Author
Owner

PR #1489 has been reviewed for the 5th time. Changes still requested — the PR has not been updated since the original push on 2026-04-02 19:31:48 UTC. The same critical issues persist:

  1. The actual bug is not fixedcompiler.py is not modified in the PR
  2. Destructive find-and-replace of "config dict" → "actor_ref" across 24 files corrupts docstrings and comments
  3. 12+ Behave step definitions are broken — step decorators changed but .feature files not updated
  4. No tests added — Behave and Robot tests required by subtasks are missing
  5. Commit message format incorrect — should be fix(actor): ... with ISSUES CLOSED: #1431 footer

The PR needs to be completely reworked to address the actual bug described in this issue.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

PR #1489 has been reviewed for the 5th time. **Changes still requested** — the PR has not been updated since the original push on 2026-04-02 19:31:48 UTC. The same critical issues persist: 1. **The actual bug is not fixed** — `compiler.py` is not modified in the PR 2. **Destructive find-and-replace** of "config dict" → "actor_ref" across 24 files corrupts docstrings and comments 3. **12+ Behave step definitions are broken** — step decorators changed but `.feature` files not updated 4. **No tests added** — Behave and Robot tests required by subtasks are missing 5. **Commit message format incorrect** — should be `fix(actor): ...` with `ISSUES CLOSED: #1431` footer The PR needs to be completely reworked to address the actual bug described in this issue. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Author
Owner

PR #1489 has been reviewed for the 6th time. Changes requested — the PR has not been updated since the original push on 2026-04-02.

Key issues remain:

  1. compiler.py (the file containing the bug) is not modified at all
  2. A destructive find-and-replace of "config dict" → "actor_ref" across 24 files corrupts docstrings and breaks 12+ Behave step definitions
  3. No tests added for the cycle detection fix
  4. Commit message format does not match issue metadata

The PR needs to be completely reworked per the detailed feedback in the review comments.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

PR #1489 has been reviewed for the 6th time. **Changes requested — the PR has not been updated since the original push on 2026-04-02.** Key issues remain: 1. `compiler.py` (the file containing the bug) is not modified at all 2. A destructive find-and-replace of "config dict" → "actor_ref" across 24 files corrupts docstrings and breaks 12+ Behave step definitions 3. No tests added for the cycle detection fix 4. Commit message format does not match issue metadata The PR needs to be completely reworked per the detailed feedback in the review comments. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Author
Owner

PR #1489 Review Update — Changes Still Required (7th Review)

PR #1489 has been reviewed again. The PR has not been updated since the original push (head SHA d0cd3cfb unchanged since 2026-04-02 19:31:48 UTC). All previously identified issues persist:

  1. The actual bug is not fixedcompiler.py is not modified in the PR
  2. Destructive find-and-replace corrupts 24 files (replaces "config dict" with "actor_ref" — unrelated concepts)
  3. 12+ Gherkin step definitions broken — step decorators changed but .feature files not updated
  4. No tests added for cross-actor cycle detection
  5. Commit message format incorrect

The PR needs to be completely reworked: revert all find-and-replace changes, fix the actual bug in compiler.py lines ~197 and ~293, add required tests, and fix the commit message.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## PR #1489 Review Update — Changes Still Required (7th Review) PR #1489 has been reviewed again. **The PR has not been updated since the original push** (head SHA `d0cd3cfb` unchanged since 2026-04-02 19:31:48 UTC). All previously identified issues persist: 1. **The actual bug is not fixed** — `compiler.py` is not modified in the PR 2. **Destructive find-and-replace** corrupts 24 files (replaces "config dict" with "actor_ref" — unrelated concepts) 3. **12+ Gherkin step definitions broken** — step decorators changed but `.feature` files not updated 4. **No tests added** for cross-actor cycle detection 5. **Commit message format incorrect** The PR needs to be completely reworked: revert all find-and-replace changes, fix the actual bug in `compiler.py` lines ~197 and ~293, add required tests, and fix the commit message. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Author
Owner

PR #1489 Review Outcome: Changes Requested

PR #1489 (fix/1431-subgraph) was reviewed and changes were requested. The PR does not fix the actual bug described in this issue. Instead, it contains a broken mass find-and-replace of "config dict" → "actor_ref" across 24 files that:

  1. Does not modify compiler.py — the file containing the bug (node.config.get("actor_ref", "") on line 197)
  2. Corrupts docstrings and comments — replacing semantically correct "config dict" with incorrect "actor_ref"
  3. Produces broken text — "config dictionary" became "actor_refionary"
  4. Breaks Gherkin step definitions — step text changed in .py files but not in .feature files, causing test failures
  5. Missing required tests — no Behave or Robot tests for cycle detection
  6. Wrong commit message — doesn't match the format specified in this issue

The PR needs to be completely reworked to actually fix the bug in compiler.py and add the required tests.

See full review details: #1489 (comment)


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## PR #1489 Review Outcome: Changes Requested PR #1489 (`fix/1431-subgraph`) was reviewed and **changes were requested**. The PR does not fix the actual bug described in this issue. Instead, it contains a broken mass find-and-replace of "config dict" → "actor_ref" across 24 files that: 1. **Does not modify `compiler.py`** — the file containing the bug (`node.config.get("actor_ref", "")` on line 197) 2. **Corrupts docstrings and comments** — replacing semantically correct "config dict" with incorrect "actor_ref" 3. **Produces broken text** — "config dictionary" became "actor_refionary" 4. **Breaks Gherkin step definitions** — step text changed in `.py` files but not in `.feature` files, causing test failures 5. **Missing required tests** — no Behave or Robot tests for cycle detection 6. **Wrong commit message** — doesn't match the format specified in this issue The PR needs to be completely reworked to actually fix the bug in `compiler.py` and add the required tests. See full review details: https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/1489#issuecomment-99694 --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Author
Owner

PR #1489 Review: Changes Requested

PR #1489 (fix/1431-subgraph) was reviewed and changes were requested. The PR does not fix the actual bug and introduces harmful changes:

  1. The bug in compiler.py line 197 is not fixednode.config.get("actor_ref", "") remains unchanged
  2. Incorrect find-and-replace of "config dict" → "actor_ref" across 24 files breaks semantics in comments/docstrings
  3. Behave step definitions changed without updating feature files — at least 19 step mismatches will cause test failures
  4. Broken word "actor_refionary" introduced (from replacing "dict" inside "dictionary")
  5. No tests added for cross-actor cycle detection

The PR needs to be completely reworked to actually fix the bug and add the required tests.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## PR #1489 Review: Changes Requested PR #1489 (`fix/1431-subgraph`) was reviewed and **changes were requested**. The PR does not fix the actual bug and introduces harmful changes: 1. **The bug in `compiler.py` line 197 is not fixed** — `node.config.get("actor_ref", "")` remains unchanged 2. **Incorrect find-and-replace** of "config dict" → "actor_ref" across 24 files breaks semantics in comments/docstrings 3. **Behave step definitions changed without updating feature files** — at least 19 step mismatches will cause test failures 4. **Broken word "actor_refionary"** introduced (from replacing "dict" inside "dictionary") 5. **No tests added** for cross-actor cycle detection The PR needs to be completely reworked to actually fix the bug and add the required tests. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Author
Owner

PR #1489 Review Outcome: Changes Requested

PR #1489 (fix/1431-subgraph) was reviewed and changes were requested. The PR has critical issues:

  1. The actual bug fix is missingsrc/cleveragents/actor/compiler.py line 197 (node.config.get("actor_ref", "")) is not modified at all
  2. Incorrect mass find-and-replace — The PR replaces "config dict" with "actor_ref" across 24 files, which is semantically wrong (a config dict ≠ an actor_ref)
  3. Broken Behave step definitions — Step definition text was changed but .feature files were not updated, causing step mismatches
  4. No new tests — Required Behave and Robot tests for cycle detection are absent

Full review details are on PR #1489.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## PR #1489 Review Outcome: Changes Requested PR #1489 (`fix/1431-subgraph`) was reviewed and **changes were requested**. The PR has critical issues: 1. **The actual bug fix is missing** — `src/cleveragents/actor/compiler.py` line 197 (`node.config.get("actor_ref", "")`) is not modified at all 2. **Incorrect mass find-and-replace** — The PR replaces "config dict" with "actor_ref" across 24 files, which is semantically wrong (a config dict ≠ an actor_ref) 3. **Broken Behave step definitions** — Step definition text was changed but `.feature` files were not updated, causing step mismatches 4. **No new tests** — Required Behave and Robot tests for cycle detection are absent Full review details are on [PR #1489](https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/1489#issuecomment-105503). --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Author
Owner

PR #1489 Review: Changes Requested

PR #1489 (fix/1431-subgraph) was reviewed and changes were requested. The PR does not fix the reported bug and introduces regressions:

  1. Does not touch compiler.py — the actual bug (node.config.get("actor_ref", "")node.actor_ref) remains unfixed
  2. Performs an incorrect global find-and-replace of "config dict" → "actor_ref" across 24 unrelated files, corrupting docstrings and comments with wrong terminology
  3. Breaks Behave tests — step definitions were renamed but .feature files were not updated, causing at least 19 step matching failures
  4. Introduces grammatical errors (e.g., "actor_refionary")
  5. Missing required tests per the Definition of Done
  6. Commit message lacks ISSUES CLOSED: #1431 footer

Full review details are on PR #1489.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## PR #1489 Review: Changes Requested PR #1489 (`fix/1431-subgraph`) was reviewed and **changes were requested**. The PR does not fix the reported bug and introduces regressions: 1. **Does not touch `compiler.py`** — the actual bug (`node.config.get("actor_ref", "")` → `node.actor_ref`) remains unfixed 2. **Performs an incorrect global find-and-replace** of "config dict" → "actor_ref" across 24 unrelated files, corrupting docstrings and comments with wrong terminology 3. **Breaks Behave tests** — step definitions were renamed but `.feature` files were not updated, causing at least 19 step matching failures 4. **Introduces grammatical errors** (e.g., "actor_refionary") 5. **Missing required tests** per the Definition of Done 6. **Commit message lacks `ISSUES CLOSED: #1431` footer** Full review details are on [PR #1489](https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/1489#issuecomment-107856). --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Author
Owner

PR #1489 Review: Changes Requested

PR #1489 was reviewed and changes were requested. The PR does not fix the actual bug described in this issue. Key findings:

  1. src/cleveragents/actor/compiler.py is not modified — the bug at line 197 (node.config.get("actor_ref", "") instead of node.actor_ref) remains unfixed
  2. The PR instead performs an incorrect mass find-and-replace of "config dict" → "actor_ref" across 24 unrelated files, breaking Behave step definitions and producing nonsensical text
  3. CI is failing across 6+ checks due to the broken changes

The PR needs to be reworked to actually fix the compiler bug and include proper tests.

See PR #1489 review comment for full details.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## PR #1489 Review: Changes Requested PR #1489 was reviewed and **changes were requested**. The PR does not fix the actual bug described in this issue. Key findings: 1. **`src/cleveragents/actor/compiler.py` is not modified** — the bug at line 197 (`node.config.get("actor_ref", "")` instead of `node.actor_ref`) remains unfixed 2. The PR instead performs an incorrect mass find-and-replace of "config dict" → "actor_ref" across 24 unrelated files, breaking Behave step definitions and producing nonsensical text 3. CI is failing across 6+ checks due to the broken changes The PR needs to be reworked to actually fix the compiler bug and include proper tests. See [PR #1489 review comment](https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/1489#issuecomment-112075) for full details. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
Owner

PR #1489 Review: Changes Requested (Architecture-Focused Review)

PR #1489 (fix/1431-subgraph) was reviewed with focus on architecture-alignment, module-boundaries, and interface-contracts. Changes requested — the PR has not been updated since the original push on 2026-04-02 (6 days stale).

Architecture Perspective (New Finding)

The bug in compiler.py is fundamentally an interface contract violation: NodeDefinition defines actor_ref as a first-class Pydantic field with a @field_validator enforcing namespace/name format, but the compiler bypasses this typed interface and reads from the untyped config: dict[str, Any] instead. This means:

  1. The actor_ref field validator is effectively dead code
  2. The compiler violates the module boundary between schema.py (data model) and compiler.py (compilation)
  3. The same pattern exists in three locations in compiler.py, not just the one reported

Status

All previously identified issues persist:

  • compiler.py is not modified (the bug remains)
  • Destructive find-and-replace of "config dict" → "actor_ref" across 24+ files
  • 12+ broken Behave step definitions
  • No tests added
  • Milestone mismatch (v3.7.0 vs v3.3.0)

The branch should be reset and reimplemented from scratch.

See PR #1489 review for full details.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-self-reviewer

## PR #1489 Review: Changes Requested (Architecture-Focused Review) PR #1489 (`fix/1431-subgraph`) was reviewed with focus on **architecture-alignment, module-boundaries, and interface-contracts**. **Changes requested** — the PR has not been updated since the original push on 2026-04-02 (6 days stale). ### Architecture Perspective (New Finding) The bug in `compiler.py` is fundamentally an **interface contract violation**: `NodeDefinition` defines `actor_ref` as a first-class Pydantic field with a `@field_validator` enforcing namespace/name format, but the compiler bypasses this typed interface and reads from the untyped `config: dict[str, Any]` instead. This means: 1. The `actor_ref` field validator is effectively dead code 2. The compiler violates the module boundary between `schema.py` (data model) and `compiler.py` (compilation) 3. The same pattern exists in **three locations** in `compiler.py`, not just the one reported ### Status All previously identified issues persist: - `compiler.py` is not modified (the bug remains) - Destructive find-and-replace of "config dict" → "actor_ref" across 24+ files - 12+ broken Behave step definitions - No tests added - Milestone mismatch (v3.7.0 vs v3.3.0) The branch should be reset and reimplemented from scratch. See [PR #1489 review](https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/1489#issuecomment-143648) for full details. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: pr-self-reviewer
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".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core#1431
No description provided.