Add tool_agent_class parameter to AgentFactory and create_executor to support runtime-injected ToolAgent subclasses #73

Closed
opened 2026-07-07 06:40:16 +00:00 by hurui200320 · 3 comments
Member

Metadata

Commit Message: feat(agents): add tool_agent_class parameter to AgentFactory and create_executor
Branch: feature/m1-tool-agent-class-injection


Background

cleveractors-core is consumed by cleveragents-webapp (the cleverthis platform). The webapp needs to replace the ToolAgent class used at runtime with a project-controlled subclass — PlatformAwareToolAgent — that overrides _validate_tools() to register native async handlers for platform-approved tools (e.g. cleverthis/safe-http-fetch). This bridges the gap between ToolAgent's Python-sandbox execution model and the webapp's server-side handler registry: without the override, platform tools have no code to run.

Because cleveractors does not currently expose any extension point for customising the ToolAgent implementation, the webapp works around this limitation by monkey-patching five cleveractors module namespaces at import time:

# src/cleverthis/services/platform_tools.py
import cleveractors.agents.factory    as _factory_module
import cleveractors.agents.tool       as _tool_module
import cleveractors.core.application  as _app_module
import cleveractors.langgraph.nodes   as _langgraph_nodes_module
import cleveractors.runtime_dispatch  as _dispatch_module

# Replace the class in the source module (covers late imports)
setattr(_tool_module, "ToolAgent", platform_cls)  # noqa: B010

# Replace the already-bound local names in every module that did
# `from cleveractors.agents.tool import ToolAgent` at its own import time
for _mod in (_factory_module, _dispatch_module, _app_module, _langgraph_nodes_module):
    if getattr(_mod, "ToolAgent", None) is not platform_cls:
        setattr(_mod, "ToolAgent", platform_cls)  # noqa: B010

This is guarded by an _hook_installed flag (idempotent), protected by a try/except, and explicitly documented as a monkey-patch in the module docstring. It works correctly today. However, it carries several structural risks that are best resolved by an upstream API change:

  • Fragility against library evolution. If a new cleveractors module imports ToolAgent at its own import time (e.g. a future workers.py), the patch silently misses it. The failure only surfaces at runtime when that module constructs a ToolAgent instance, and platform tools silently don't execute.
  • Import-order sensitivity. The patch fires when platform_tools is first imported. Any cleveractors module initialised before platform_tools is imported will hold a stale reference for the duration of its module-level setup. This is currently avoided by application startup order, but is a latent ordering hazard.
  • Breaks static analysis. Pyright and IDEs see the original ToolAgent throughout all cleveractors call sites. Types derived from the patched class are invisible to the checker, making strict-mode findings in those modules silently incorrect.
  • Violates the Open/Closed Principle. cleveragents-webapp is coupled to cleveractors's internal module structure rather than a stable public API.

Proposed Solution

Extend AgentFactory.__init__ (and, transitively, create_executor) to accept an optional tool_agent_class keyword argument. When provided, this class is used in place of the default ToolAgent wherever the factory constructs tool agents — in the type registry, in _execute_tool construction, in isinstance checks, and in LangGraph node wiring.

Sketch of the AgentFactory change:

# cleveractors/agents/factory.py
from cleveractors.agents.tool import ToolAgent as _DefaultToolAgent

class AgentFactory:
    def __init__(
        self,
        config: dict,
        credentials: dict | None = None,
        *,
        tool_agent_class: type[_DefaultToolAgent] = _DefaultToolAgent,
        # ... existing params ...
    ) -> None:
        self._tool_agent_class = tool_agent_class
        # ... existing init ...

Every internal site that currently references the module-level ToolAgent symbol is replaced with self._tool_agent_class. create_executor forwards the parameter:

# cleveractors/__init__.py  (or wherever create_executor lives)
def create_executor(
    config_dict: dict,
    credentials: dict | None = None,
    limits: dict | None = None,
    pricing: dict | None = None,
    *,
    tool_agent_class: type[ToolAgent] = ToolAgent,
) -> Executor:
    factory = AgentFactory(
        config_dict,
        credentials,
        tool_agent_class=tool_agent_class,
        limits=limits,
        pricing=pricing,
    )
    return factory.build()

Resulting cleveragents-webapp call site (after platform_tools.py is updated to use the new API):

executor = create_executor(
    config_dict=config,
    credentials=credentials,
    limits=limits,
    pricing=pricing,
    tool_agent_class=PlatformAwareToolAgent,   # replaces the entire monkey-patch
)

The five-module setattr loop and the _install_platform_tool_hook machinery in cleveragents-webapp are deleted entirely once this parameter exists.


Acceptance Criteria

  • AgentFactory.__init__ accepts an optional tool_agent_class: type[ToolAgent] keyword argument, defaulting to ToolAgent.
  • When tool_agent_class is provided, every internal AgentFactory code path that constructs, type-checks (isinstance), or registers a ToolAgent uses the supplied class instead of the module-level default.
  • create_executor accepts and forwards tool_agent_class to AgentFactory.
  • The default behaviour (no argument supplied) is identical to the current behaviour — no regression for callers that do not pass tool_agent_class.
  • The new parameter is fully type-annotated. Pyright strict mode produces no new errors in cleveractors or in the webapp's updated call site.
  • Unit tests cover: default class used when parameter is omitted; supplied subclass used for construction and isinstance checks when parameter is provided.
  • CHANGELOG.md updated with one entry for this commit.

Subtasks

  • Audit all sites within AgentFactory (factory, dispatch, application, langgraph nodes) that reference the ToolAgent symbol and list them.
  • Add tool_agent_class parameter to AgentFactory.__init__; replace all audited sites with self._tool_agent_class.
  • Add tool_agent_class parameter to create_executor; forward to AgentFactory.
  • Write BDD unit tests for the default and override behaviours.
  • Update CHANGELOG.md.
  • Notify cleveragents-webapp team so they can update the pyproject.toml pin and replace _install_platform_tool_hook with the new create_executor argument.

Definition of Done

  • AgentFactory and create_executor accept tool_agent_class with backward-compatible default.
  • All cleveractors quality gates pass (nox suite green, coverage ≥ threshold, Pyright strict clean).
  • PR merged to main / master.
  • cleveragents-webapp team notified (comment on this issue with the new pinned commit SHA or release tag once available).
## Metadata **Commit Message:** `feat(agents): add tool_agent_class parameter to AgentFactory and create_executor` **Branch:** `feature/m1-tool-agent-class-injection` --- ## Background `cleveractors-core` is consumed by `cleveragents-webapp` (the `cleverthis` platform). The webapp needs to replace the `ToolAgent` class used at runtime with a project-controlled subclass — `PlatformAwareToolAgent` — that overrides `_validate_tools()` to register native async handlers for platform-approved tools (e.g. `cleverthis/safe-http-fetch`). This bridges the gap between `ToolAgent`'s Python-sandbox execution model and the webapp's server-side handler registry: without the override, platform tools have no code to run. Because `cleveractors` does not currently expose any extension point for customising the `ToolAgent` implementation, the webapp works around this limitation by **monkey-patching five `cleveractors` module namespaces at import time**: ```python # src/cleverthis/services/platform_tools.py import cleveractors.agents.factory as _factory_module import cleveractors.agents.tool as _tool_module import cleveractors.core.application as _app_module import cleveractors.langgraph.nodes as _langgraph_nodes_module import cleveractors.runtime_dispatch as _dispatch_module # Replace the class in the source module (covers late imports) setattr(_tool_module, "ToolAgent", platform_cls) # noqa: B010 # Replace the already-bound local names in every module that did # `from cleveractors.agents.tool import ToolAgent` at its own import time for _mod in (_factory_module, _dispatch_module, _app_module, _langgraph_nodes_module): if getattr(_mod, "ToolAgent", None) is not platform_cls: setattr(_mod, "ToolAgent", platform_cls) # noqa: B010 ``` This is guarded by an `_hook_installed` flag (idempotent), protected by a `try/except`, and explicitly documented as a monkey-patch in the module docstring. It works correctly today. However, it carries several structural risks that are best resolved by an upstream API change: - **Fragility against library evolution.** If a new `cleveractors` module imports `ToolAgent` at its own import time (e.g. a future `workers.py`), the patch silently misses it. The failure only surfaces at runtime when that module constructs a `ToolAgent` instance, and platform tools silently don't execute. - **Import-order sensitivity.** The patch fires when `platform_tools` is first imported. Any `cleveractors` module initialised before `platform_tools` is imported will hold a stale reference for the duration of its module-level setup. This is currently avoided by application startup order, but is a latent ordering hazard. - **Breaks static analysis.** Pyright and IDEs see the original `ToolAgent` throughout all `cleveractors` call sites. Types derived from the patched class are invisible to the checker, making strict-mode findings in those modules silently incorrect. - **Violates the Open/Closed Principle.** `cleveragents-webapp` is coupled to `cleveractors`'s internal module structure rather than a stable public API. --- ## Proposed Solution Extend `AgentFactory.__init__` (and, transitively, `create_executor`) to accept an optional `tool_agent_class` keyword argument. When provided, this class is used in place of the default `ToolAgent` wherever the factory constructs tool agents — in the type registry, in `_execute_tool` construction, in `isinstance` checks, and in LangGraph node wiring. **Sketch of the `AgentFactory` change:** ```python # cleveractors/agents/factory.py from cleveractors.agents.tool import ToolAgent as _DefaultToolAgent class AgentFactory: def __init__( self, config: dict, credentials: dict | None = None, *, tool_agent_class: type[_DefaultToolAgent] = _DefaultToolAgent, # ... existing params ... ) -> None: self._tool_agent_class = tool_agent_class # ... existing init ... ``` Every internal site that currently references the module-level `ToolAgent` symbol is replaced with `self._tool_agent_class`. `create_executor` forwards the parameter: ```python # cleveractors/__init__.py (or wherever create_executor lives) def create_executor( config_dict: dict, credentials: dict | None = None, limits: dict | None = None, pricing: dict | None = None, *, tool_agent_class: type[ToolAgent] = ToolAgent, ) -> Executor: factory = AgentFactory( config_dict, credentials, tool_agent_class=tool_agent_class, limits=limits, pricing=pricing, ) return factory.build() ``` **Resulting `cleveragents-webapp` call site** (after `platform_tools.py` is updated to use the new API): ```python executor = create_executor( config_dict=config, credentials=credentials, limits=limits, pricing=pricing, tool_agent_class=PlatformAwareToolAgent, # replaces the entire monkey-patch ) ``` The five-module `setattr` loop and the `_install_platform_tool_hook` machinery in `cleveragents-webapp` are deleted entirely once this parameter exists. --- ## Acceptance Criteria - [x] `AgentFactory.__init__` accepts an optional `tool_agent_class: type[ToolAgent]` keyword argument, defaulting to `ToolAgent`. - [x] When `tool_agent_class` is provided, every internal `AgentFactory` code path that constructs, type-checks (`isinstance`), or registers a `ToolAgent` uses the supplied class instead of the module-level default. - [x] `create_executor` accepts and forwards `tool_agent_class` to `AgentFactory`. - [x] The default behaviour (no argument supplied) is identical to the current behaviour — no regression for callers that do not pass `tool_agent_class`. - [x] The new parameter is fully type-annotated. Pyright strict mode produces no new errors in `cleveractors` or in the webapp's updated call site. - [x] Unit tests cover: default class used when parameter is omitted; supplied subclass used for construction and `isinstance` checks when parameter is provided. - [x] `CHANGELOG.md` updated with one entry for this commit. --- ## Subtasks - [x] Audit all sites within `AgentFactory` (factory, dispatch, application, langgraph nodes) that reference the `ToolAgent` symbol and list them. - [x] Add `tool_agent_class` parameter to `AgentFactory.__init__`; replace all audited sites with `self._tool_agent_class`. - [x] Add `tool_agent_class` parameter to `create_executor`; forward to `AgentFactory`. - [x] Write BDD unit tests for the default and override behaviours. - [x] Update `CHANGELOG.md`. - [ ] Notify `cleveragents-webapp` team so they can update the `pyproject.toml` pin and replace `_install_platform_tool_hook` with the new `create_executor` argument. --- ## Definition of Done - `AgentFactory` and `create_executor` accept `tool_agent_class` with backward-compatible default. - All `cleveractors` quality gates pass (`nox` suite green, coverage ≥ threshold, Pyright strict clean). - PR merged to `main` / `master`. - `cleveragents-webapp` team notified (comment on this issue with the new pinned commit SHA or release tag once available).
Author
Member

Implementation Notes — Audit & Design Decisions

Audit: ToolAgent references across cleveractors

Searched all .py files in src/cleveractors/ for ToolAgent references:

File Location Purpose Action
agents/tool.py class definition Base class definition No change (it IS the default)
agents/factory.py L20 import, L99 self.agent_types["tool"] = ToolAgent Registers ToolAgent in type registry Change to self._tool_agent_class
runtime_dispatch.py L36 import, L487 agent = ToolAgent(...) in _execute_tool Constructs ToolAgent directly (bypasses factory) Change to executor.tool_agent_class
core/application.py L23 import, L232 register_agent_type("tool", ToolAgent), L934 type_map ReactiveCleverAgentsApp — separate entry point Out of scope (no create_executor path; note below)
langgraph/nodes.py L279, L510 isinstance(agent, ToolAgent) Type-checking at node execution No change neededisinstance natively supports subclasses via LSP

Design Decisions

1. Scope of the change

The issue asks to fix AgentFactory and create_executor. The ReactiveCleverAgentsApp in core/application.py is a separate, higher-level entry point (CLI/interactive mode) and is not reachable from create_executor. Changing it would require a separate parameter threading path and is out of scope for this issue. A note will be added to the issue for follow-up.

2. Where to carry tool_agent_class on Executor

The cleanest approach is to store tool_agent_class on Executor itself (as self.tool_agent_class). All dispatch functions already receive executor as their first argument, so they can access executor.tool_agent_class without signature changes.

3. isinstance checks in langgraph/nodes.py

isinstance(agent, ToolAgent) already works correctly for subclasses because Python's isinstance checks the MRO. No change is needed — a PlatformAwareToolAgent(ToolAgent) subclass will pass these checks correctly.

4. Type annotation

tool_agent_class: type[ToolAgent] = ToolAgent uses type[ToolAgent] which accepts any subclass of ToolAgent in Pyright strict mode. This is the correct, fully-typed approach.

5. Argument validation

Per CONTRIBUTING.md §Argument Validation, public methods must validate arguments. We'll add issubclass(tool_agent_class, ToolAgent) guard in AgentFactory.__init__ and Executor.__init__ to reject non-ToolAgent classes at the entry boundary (fail-fast).

6. _execute_graph and _execute_graph_stream (AgentFactory creation)

Both functions create an AgentFactory for graph execution. They must pass tool_agent_class=executor.tool_agent_class to the factory so graph nodes using ToolAgent also pick up the subclass.

Files to Modify

  1. src/cleveractors/agents/factory.py — add tool_agent_class param
  2. src/cleveractors/runtime.py — add tool_agent_class to Executor and create_executor
  3. src/cleveractors/runtime_dispatch.py — thread through in _execute_tool, _execute_graph, _execute_graph_stream
  4. features/tool_agent_class_injection.feature — new BDD feature file
  5. features/steps/tool_agent_class_injection_steps.py — new step definitions
  6. CHANGELOG.md — one entry
## Implementation Notes — Audit & Design Decisions ### Audit: ToolAgent references across `cleveractors` Searched all `.py` files in `src/cleveractors/` for `ToolAgent` references: | File | Location | Purpose | Action | |------|----------|---------|--------| | `agents/tool.py` | class definition | Base class definition | No change (it IS the default) | | `agents/factory.py` | L20 import, L99 `self.agent_types["tool"] = ToolAgent` | Registers ToolAgent in type registry | **Change to `self._tool_agent_class`** | | `runtime_dispatch.py` | L36 import, L487 `agent = ToolAgent(...)` in `_execute_tool` | Constructs ToolAgent directly (bypasses factory) | **Change to `executor.tool_agent_class`** | | `core/application.py` | L23 import, L232 `register_agent_type("tool", ToolAgent)`, L934 type_map | `ReactiveCleverAgentsApp` — separate entry point | **Out of scope** (no `create_executor` path; note below) | | `langgraph/nodes.py` | L279, L510 `isinstance(agent, ToolAgent)` | Type-checking at node execution | **No change needed** — `isinstance` natively supports subclasses via LSP | ### Design Decisions **1. Scope of the change** The issue asks to fix `AgentFactory` and `create_executor`. The `ReactiveCleverAgentsApp` in `core/application.py` is a separate, higher-level entry point (CLI/interactive mode) and is **not** reachable from `create_executor`. Changing it would require a separate parameter threading path and is out of scope for this issue. A note will be added to the issue for follow-up. **2. Where to carry `tool_agent_class` on `Executor`** The cleanest approach is to store `tool_agent_class` on `Executor` itself (as `self.tool_agent_class`). All dispatch functions already receive `executor` as their first argument, so they can access `executor.tool_agent_class` without signature changes. **3. `isinstance` checks in `langgraph/nodes.py`** `isinstance(agent, ToolAgent)` already works correctly for subclasses because Python's `isinstance` checks the MRO. No change is needed — a `PlatformAwareToolAgent(ToolAgent)` subclass will pass these checks correctly. **4. Type annotation** `tool_agent_class: type[ToolAgent] = ToolAgent` uses `type[ToolAgent]` which accepts any subclass of `ToolAgent` in Pyright strict mode. This is the correct, fully-typed approach. **5. Argument validation** Per `CONTRIBUTING.md §Argument Validation`, public methods must validate arguments. We'll add `issubclass(tool_agent_class, ToolAgent)` guard in `AgentFactory.__init__` and `Executor.__init__` to reject non-`ToolAgent` classes at the entry boundary (fail-fast). **6. `_execute_graph` and `_execute_graph_stream` (AgentFactory creation)** Both functions create an `AgentFactory` for graph execution. They must pass `tool_agent_class=executor.tool_agent_class` to the factory so graph nodes using `ToolAgent` also pick up the subclass. ### Files to Modify 1. `src/cleveractors/agents/factory.py` — add `tool_agent_class` param 2. `src/cleveractors/runtime.py` — add `tool_agent_class` to `Executor` and `create_executor` 3. `src/cleveractors/runtime_dispatch.py` — thread through in `_execute_tool`, `_execute_graph`, `_execute_graph_stream` 4. `features/tool_agent_class_injection.feature` — new BDD feature file 5. `features/steps/tool_agent_class_injection_steps.py` — new step definitions 6. `CHANGELOG.md` — one entry
Author
Member

Webapp Team Notification

PR #74 (feat(agents): add tool_agent_class parameter to AgentFactory and create_executor) has been submitted for review.

Once this PR is merged, the cleveragents-webapp team can:

  1. Update the pyproject.toml pin to reference the new release tag (or commit SHA) that includes this change.

  2. Replace _install_platform_tool_hook with the new create_executor keyword argument:

# Before (monkey-patch approach — to be deleted):
from cleverthis.services.platform_tools import _install_platform_tool_hook
_install_platform_tool_hook(PlatformAwareToolAgent)

executor = create_executor(config_dict=config, credentials=credentials, ...)

# After (clean API — no monkey-patching needed):
from cleveractors import create_executor
from cleveractors.agents.tool import ToolAgent

class PlatformAwareToolAgent(ToolAgent):
    def _validate_tools(self) -> None:
        # register native async handlers for platform-approved tools
        ...

executor = create_executor(
    config_dict=config,
    credentials=credentials,
    limits=limits,
    pricing=pricing,
    tool_agent_class=PlatformAwareToolAgent,
)

The five-module setattr loop and all _install_platform_tool_hook machinery in cleverthis/services/platform_tools.py can then be removed.

Commit SHA: 20f68e9ae4eacbf050d30efce550f8d9584e44c4 (on branch feature/m1-tool-agent-class-injection, PR #74)

## Webapp Team Notification PR #74 (`feat(agents): add tool_agent_class parameter to AgentFactory and create_executor`) has been submitted for review. Once this PR is merged, the `cleveragents-webapp` team can: 1. **Update the `pyproject.toml` pin** to reference the new release tag (or commit SHA) that includes this change. 2. **Replace `_install_platform_tool_hook`** with the new `create_executor` keyword argument: ```python # Before (monkey-patch approach — to be deleted): from cleverthis.services.platform_tools import _install_platform_tool_hook _install_platform_tool_hook(PlatformAwareToolAgent) executor = create_executor(config_dict=config, credentials=credentials, ...) # After (clean API — no monkey-patching needed): from cleveractors import create_executor from cleveractors.agents.tool import ToolAgent class PlatformAwareToolAgent(ToolAgent): def _validate_tools(self) -> None: # register native async handlers for platform-approved tools ... executor = create_executor( config_dict=config, credentials=credentials, limits=limits, pricing=pricing, tool_agent_class=PlatformAwareToolAgent, ) ``` The five-module `setattr` loop and all `_install_platform_tool_hook` machinery in `cleverthis/services/platform_tools.py` can then be removed. **Commit SHA:** `20f68e9ae4eacbf050d30efce550f8d9584e44c4` (on branch `feature/m1-tool-agent-class-injection`, PR #74)
hurui200320 added this to the v2.1.0 milestone 2026-07-07 08:05:53 +00:00
Author
Member

Self-QA Implementation Notes (Cycles 1–3)

Cycle 1

Review findings: 0 Critical / 1 Major / 3 Minor / 3 Nits

  • Major: LLMAgent internal tool-call loop still imported and constructed base ToolAgent via _TA(...), ignoring the injected subclass. This left the LLM tool-calling path dependent on the monkey-patch.
  • Minor: ReactiveCleverAgentsApp hardcoded ToolAgent in load_configuration and _create_agents; missing test coverage for LLM tool loop with custom subclass; unused imports (asyncio, AsyncMock, MagicMock) in new step file.
  • Nits: Tautological isinstance scenario (direct instantiation instead of factory-created); stale ToolAgent import in runtime_dispatch.py; context parameter shadowed Behave context in _fake_process helper.

Fixes applied:

  • Added tool_agent_class: type[ToolAgent] = ToolAgent keyword-only parameter to LLMAgent.__init__ with fail-fast subclass validation. Stored as self._tool_agent_class and replaced all three _TA(...) constructions in llm.py with self._tool_agent_class(...).
  • Updated AgentFactory._create_agent_instance to forward tool_agent_class when creating "llm"-typed agents.
  • Removed redundant register_agent_type("tool", ToolAgent) and dead type_map re-registration from ReactiveCleverAgentsApp in application.py.
  • Added BDD scenario verifying the LLM tool loop instantiates the custom subclass, plus an LLMAgent rejection scenario for coverage of the new validation branch.
  • Removed unused imports; replaced tautological isinstance scenario with factory-mediated LSP test; removed stale ToolAgent import from runtime_dispatch.py (updated docstring reference) and removed now-dead patch("cleveractors.runtime_dispatch.ToolAgent") mocks in two test files.
  • Renamed _fake_process parameter to **_kwargs to eliminate Behave context shadowing.
  • Updated CHANGELOG.md and PR description.

Cycle 2

Review findings: 0 Critical / 2 Major / 2 Minor / 0 Nits

  • Major: _execute_llm and _execute_llm_stream constructed AgentFactory without forwarding tool_agent_class, so single LLM actors fell back to base ToolAgent.
  • Major: _execute_multi_actor built a sub-Executor without forwarding tool_agent_class, breaking injection for multi-actor bundles.
  • Minor: Missing test coverage for single-LLM and multi-actor injection paths; CHANGELOG overstated dispatch-path coverage as "all three" when six paths exist.

Fixes applied:

  • Added tool_agent_class=executor.tool_agent_class to AgentFactory(...) calls in _execute_llm and _execute_llm_stream (runtime_dispatch.py).
  • Added tool_agent_class=executor.tool_agent_class to sub-Executor(...) constructor in _execute_multi_actor.
  • Added two BDD scenarios: (1) single-LLM executor triggers tool call and asserts custom subclass instantiation, (2) multi-actor bundle forwards subclass to sub-executor. Both were TDD-verified to fail without the source fixes.
  • Updated CHANGELOG.md to accurately list all six dispatch paths.
  • Updated existing credential_executor_paths_steps.py test: capturing_init signature extended to accept and forward tool_agent_class, preventing TypeError from the new forwarding behavior.

Cycle 3

Review findings: 0 Critical / 0 Major / 5 Minor / 3 Nits

  • Minor: Step file tool_agent_class_injection_steps.py exceeds 500-line guideline (720 lines); missing explicit BDD coverage for _execute_graph, _execute_graph_stream, _execute_llm_stream; no Robot integration test for injected subclass; ReactiveCleverAgentsApp still does not expose tool_agent_class; RecordingToolAgent counter increments before super().__init__().
  • Nits: Executor.__init__ missing -> None annotation; several # type: ignore comments; tool_agent_class forwarded only for exact agent_type == "llm" in factory.

Fixes applied: None — all remaining items are polish/extension suggestions rather than correctness defects. The review agent approved the PR.

Remaining Issues

None blocking. The following are acknowledged follow-ups / polish items deferred to future work:

  • Step file could be split for line-count hygiene.
  • Graph and stream dispatch paths have no dedicated BDD scenarios (covered indirectly via factory/executor tests and existing integration suite).
  • No Robot integration test added for tool_agent_class injection.
  • ReactiveCleverAgentsApp does not expose tool_agent_class (platform uses create_executor entry point per ticket scope).
  • RecordingToolAgent counter placement and minor type-annotation nits.

All quality gates pass: 2756 Behave scenarios, 0 failures; 319 integration tests passed; 97.0% coverage (threshold 96.5%). CI run #30677 success.

## Self-QA Implementation Notes (Cycles 1–3) ### Cycle 1 **Review findings:** 0 Critical / 1 Major / 3 Minor / 3 Nits - **Major:** `LLMAgent` internal tool-call loop still imported and constructed base `ToolAgent` via `_TA(...)`, ignoring the injected subclass. This left the LLM tool-calling path dependent on the monkey-patch. - **Minor:** `ReactiveCleverAgentsApp` hardcoded `ToolAgent` in `load_configuration` and `_create_agents`; missing test coverage for LLM tool loop with custom subclass; unused imports (`asyncio`, `AsyncMock`, `MagicMock`) in new step file. - **Nits:** Tautological `isinstance` scenario (direct instantiation instead of factory-created); stale `ToolAgent` import in `runtime_dispatch.py`; `context` parameter shadowed Behave `context` in `_fake_process` helper. **Fixes applied:** - Added `tool_agent_class: type[ToolAgent] = ToolAgent` keyword-only parameter to `LLMAgent.__init__` with fail-fast subclass validation. Stored as `self._tool_agent_class` and replaced all three `_TA(...)` constructions in `llm.py` with `self._tool_agent_class(...)`. - Updated `AgentFactory._create_agent_instance` to forward `tool_agent_class` when creating `"llm"`-typed agents. - Removed redundant `register_agent_type("tool", ToolAgent)` and dead `type_map` re-registration from `ReactiveCleverAgentsApp` in `application.py`. - Added BDD scenario verifying the LLM tool loop instantiates the custom subclass, plus an `LLMAgent` rejection scenario for coverage of the new validation branch. - Removed unused imports; replaced tautological `isinstance` scenario with factory-mediated LSP test; removed stale `ToolAgent` import from `runtime_dispatch.py` (updated docstring reference) and removed now-dead `patch("cleveractors.runtime_dispatch.ToolAgent")` mocks in two test files. - Renamed `_fake_process` parameter to `**_kwargs` to eliminate Behave `context` shadowing. - Updated `CHANGELOG.md` and PR description. ### Cycle 2 **Review findings:** 0 Critical / 2 Major / 2 Minor / 0 Nits - **Major:** `_execute_llm` and `_execute_llm_stream` constructed `AgentFactory` without forwarding `tool_agent_class`, so single LLM actors fell back to base `ToolAgent`. - **Major:** `_execute_multi_actor` built a sub-`Executor` without forwarding `tool_agent_class`, breaking injection for multi-actor bundles. - **Minor:** Missing test coverage for single-LLM and multi-actor injection paths; `CHANGELOG` overstated dispatch-path coverage as "all three" when six paths exist. **Fixes applied:** - Added `tool_agent_class=executor.tool_agent_class` to `AgentFactory(...)` calls in `_execute_llm` and `_execute_llm_stream` (`runtime_dispatch.py`). - Added `tool_agent_class=executor.tool_agent_class` to sub-`Executor(...)` constructor in `_execute_multi_actor`. - Added two BDD scenarios: (1) single-LLM executor triggers tool call and asserts custom subclass instantiation, (2) multi-actor bundle forwards subclass to sub-executor. Both were TDD-verified to fail without the source fixes. - Updated `CHANGELOG.md` to accurately list all six dispatch paths. - Updated existing `credential_executor_paths_steps.py` test: `capturing_init` signature extended to accept and forward `tool_agent_class`, preventing `TypeError` from the new forwarding behavior. ### Cycle 3 **Review findings:** 0 Critical / 0 Major / 5 Minor / 3 Nits - **Minor:** Step file `tool_agent_class_injection_steps.py` exceeds 500-line guideline (720 lines); missing explicit BDD coverage for `_execute_graph`, `_execute_graph_stream`, `_execute_llm_stream`; no Robot integration test for injected subclass; `ReactiveCleverAgentsApp` still does not expose `tool_agent_class`; `RecordingToolAgent` counter increments before `super().__init__()`. - **Nits:** `Executor.__init__` missing `-> None` annotation; several `# type: ignore` comments; `tool_agent_class` forwarded only for exact `agent_type == "llm"` in factory. **Fixes applied:** None — all remaining items are polish/extension suggestions rather than correctness defects. The review agent approved the PR. ### Remaining Issues None blocking. The following are acknowledged follow-ups / polish items deferred to future work: - Step file could be split for line-count hygiene. - Graph and stream dispatch paths have no dedicated BDD scenarios (covered indirectly via factory/executor tests and existing integration suite). - No Robot integration test added for `tool_agent_class` injection. - `ReactiveCleverAgentsApp` does not expose `tool_agent_class` (platform uses `create_executor` entry point per ticket scope). - `RecordingToolAgent` counter placement and minor type-annotation nits. All quality gates pass: 2756 Behave scenarios, 0 failures; 319 integration tests passed; 97.0% coverage (threshold 96.5%). CI run #30677 success.
hurui200320 2026-07-08 09:44:17 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveractors-core#73
No description provided.