fix(actor): Get responses from actor-run #10900
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
4 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
cleveragents/cleveragents-core!10900
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "bugfix/m3-actor-run-response"
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
resolve_config_filesincli/commands/_resolve_actor.pyto synthesise a v3type: llmYAML when a built-in actor has noyaml_textand itsconfig_blobhasproviderandmodelbut notypefield_synthesize_llm_yaml()helper that creates a minimal v3 LLM actor YAML from a built-in actor config blob@tdd_issue_10861) infeatures/tdd_actor_run_response.featurewith step definitionsRoot Cause
Built-in actors generated from the provider registry have a
config_blobwithproviderandmodelfields but notypefield. When this raw blob was serialised to YAML and fed toReactiveConfigParser, the parser found notype, noagents/actorsmap, and norouteskey — so it produced an emptyReactiveConfigwith no agents and no routes.run_single_shot()then returned""because there was nothing to execute.Fix
resolve_config_filesnow detects this case (provider + model present, no type) and synthesises a minimal v3type: llmYAML. TheReactiveConfigParser._build_from_v3()method then creates a reactive agent and a synthesised graph route, enabling the LLM to be invoked and its response returned to the caller.Closes #10861
This PR blocks issue #10861
Automated by CleverAgents Bot
Supervisor: Implementation | Agent: task-implementor
agents actor rundoes not work. #10861Review of PR #10900 -- fix(actor): Get responses from actor-run
Closes #10861
This PR fixes a real bug where
agents actor run <built-in-actor> <prompt>silently returned an empty string for built-in provider-registry actors. The root cause analysis in the PR body is accurate. The fix adds_build_v3_yaml_from_blob()which synthesises a minimaltype: llmYAML from the blob provider and model fields.10-Category Review
1. CORRECTNESS -- PASS
The fix correctly targets the exact condition: a config_blob that has provider and model but no type field. This isolates built-in provider-registry blobs without affecting custom actors that have yaml_text or type-bearing config_blobs. Edge cases handled: empty provider/model defaults to empty string via str(... or ), system_prompt conditionally included.
2. SPECIFICATION ALIGNMENT -- PASS
The fix aligns with the v3 specification: actors must have type: llm in their YAML config for ReactiveConfigParser._build_from_v3() to recognise them. The new synthesized YAML includes type, name, and model -- the minimum required fields.
3. TEST QUALITY -- PASS
4. TYPE SAFETY -- PASS
5. READABILITY -- PASS
6. PERFORMANCE -- PASS
7. SECURITY -- PASS
8. CODE STYLE -- PASS
9. DOCUMENTATION -- PASS
10. COMMIT AND PR QUALITY -- NEEDS ATTENTION
CI Status
The CI lint failure is blocking. All core checks (typecheck, security, unit_tests) passed.
Blocking Issues
Non-Blocking Suggestions
The @tdd_expected_fail tag removal was mentioned. If a companion TDD branch/issue existed, consider verifying it was properly cleaned up.
The issue subtask list mentioned a Robot Framework integration test. While not blocking, adding one would strengthen multi-level testing coverage.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
e5862e478538fa155e66agents actor rundoes not work. #10861Review submitted by automated PR review worker.
PR Review: fix(actor): Get responses from actor-run
Reviewed against 10-category checklist. Note: All Forgejo list-read API endpoints (PR details, linked issues, review comments, PR comments) returned 404 during this session, so I could not verify previous feedback items or PR metadata (linked issues, labels, milestone, dependency direction). This assessment is based on the code diff and commit content only.
1. CORRECTNESS ✅
The fix is sound. When a built-in LLM actor has a
config_blobwithproviderandmodelbut notypefield,_synthesize_llm_yaml()constructs a proper v3type: llmYAML blob. This allowsReactiveConfigParserto create agents and routes, resolving the root cause of bug #10861. Edge cases handled: missing/system_prompt falsy → skipped; empty provider/model → falls through to originalyaml.safe_dumppath.2. SPECIFICATION ALIGNMENT ✅
The synthesized YAML follows the v3
type: llmformat (name,type: llm,provider,model, optionalsystem_prompt) — consistent with the project specification. The existingyaml_textpath is untouched.3. TEST QUALITY ✅
Three Behave BDD scenarios in
features/tdd_actor_run_response.feature:type: llmand provider/model in outputyaml_textare unaffectedAll have
@tdd_issue @tdd_issue_10861tags. Well-named scenario descriptions.Suggestion: Add a TDD scenario covering the case where
config_blobhassystem_prompt— the code adds it to the synthesized YAML (line 83-84), so testing this path would strengthen coverage.4. TYPE SAFETY ✅
from typing import Anycorrectly added._synthesize_llm_yamlsignature is fully annotated:(actor_name: str, config_blob: dict[str, Any]) -> str. No new# type: ignorecomments introduced. Pre-existing# type: ignore[import-untyped]on behave imports is an established project pattern.5. READABILITY ✅
Excellent docstring on
_synthesize_llm_yamlexplaining the bug, root cause, fix, args, and return. Variable names are clear. Logic flow inresolve_config_filesis easy to follow with the new conditional block.Suggestion: Step descriptions have repetitive
for tdd-10861suffixes (e.g.,Given a built-in actor with provider and model but no type field for tdd-10861). Thetdd-10861qualifier is unnecessary in the step text since the feature file title already identifies the bug. Clean names likeGiven a built-in actor with provider and model but no type fieldwould improve readability as living documentation.6. PERFORMANCE ✅
New function is trivial — no performance concerns.
7. SECURITY ✅
No hardcoded secrets, tokens, or credentials. Input values (provider, model) are safely embedded in YAML.
_sanitize_namestill handles terminal escape injection. No SQL/command injection vectors.8. CODE STYLE ⚠️
SOLID principles followed — SRP maintained. Files under 500 lines.
Non-blocking: Double blank-lines detected — two extra blank lines appear at lines 54-55 (between
_sanitize_nameand_synthesize_llm_yaml) and at lines 33-34 (before imports of typing). These would triggernox -s format(ruff format). Runnox -s formatto auto-fix. Minor:str(config_blob.get("provider") or "")— thestr()wrapper is redundant since.get()already returns the value as-is.config_blob.get("provider", "") or ""is slightly cleaner.9. DOCUMENTATION ✅
_synthesize_llm_yamlhas a detailed docstring. Feature file includes context about the bug and fix in the description. Commit body explains motivation clearly.10. COMMIT AND PR QUALITY ⚠️
fix(actor): Get responses from actor-run(Conventional Changelog format)ISSUES CLOSED: #10861footer presentbugfix/m3-actor-run-responsematches naming conventiongit diff master...HEAD -- CHANGELOG.mdshows empty output. Per PR requirement #7, one entry must be added per commit.Verdict: COMMENT
All code quality categories pass. The implementation correctly fixes bug #10861 with appropriate tests. I recommend addressing the non-blocking items before merge:
CHANGELOG.mdwith an entry for this bugfixnox -s formatto fix double blank-linesfor tdd-10861suffixes from Gherkin step namesAutomated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Re-Review of PR #10900 -- fix(actor): Get responses from actor-run (Closes #10861)
This is the second review. The PR previously received REQUEST_CHANGES feedback from review #6971 with 3 blocking issues. A second COMMENT review (#6997) also submitted non-blocking suggestions. I am re-evaluating whether the prior feedback was adequately addressed.
Previous Feedback Review
From REQUEST_CHANGES Review (#6971)
CI lint must pass — NOT ADDRESSED. The lint check is STILL FAILING. The same double-blank-line formatting issues remain in
_resolve_actor.py. Runnox -s formatto auto-fix.Missing Type/ label — NOT ADDRESSED. The PR still has zero labels (
labels: []). Per PR submission requirements (Requirement #12: Milestone and Type Label), exactly one Type/ label is mandatory. For a bug fix, this must beType/Bug.Missing State/In review label — NOT ADDRESSED. The PR workflow expects State/In review to be present on submitted PRs.
From COMMENT Review (#6997)
CHANGELOG.md not updated — NOT ADDRESSED. The PR body references CHANGELOG but
git diff master...HEAD -- CHANGELOG.mdshows no changes. Per PR Requirement #7, the CHANGELOG must be updated with one entry per commit.Repetitive
for tdd-10861suffixes in Gherkin step names — Still present (suggestion, non-blocking).Full 10-Category Review
1. CORRECTNESS -- PASS
The fix correctly identifies the condition where
config_blobhasproviderandmodelbut notype, and synthesizes a minimal v3type: llmYAML. Edge cases handled: missing system_prompt (conditionally included), empty provider/model (defaults to empty string viaor "").2. SPECIFICATION ALIGNMENT -- PASS
The synthesized YAML follows the v3
type: llmformat (name, type, description, provider, model, optional system_prompt) which is consistent with the project specification.3. TEST QUALITY -- PASS
Three Behave BDD scenarios covering synthesis, parseability by ReactiveConfigParser, and non-interference with existing yaml_text actors.
@tdd_issue @tdd_issue_10861tags present. Step definitions comprehensive at 148 lines with proper cleanup.4. TYPE SAFETY -- PASS
from typing import Anyadded. Function signature fully annotated. No new# type: ignorecomments.5. READABILITY -- PASS
Excellent docstring on
_synthesize_llm_yaml. Clear condition check (has_provider_modelpattern via inline if). Code flow easy to follow.6. PERFORMANCE -- PASS
One-time YAML construction per actor. No performance concerns.
7. SECURITY -- PASS
No hardcoded secrets. System_prompt passed through from provider-registry config_blob, not user input. No injection vectors.
8. CODE STYLE -- NEEDS ATTENTION
Double blank-lines in
_resolve_actor.py:_sanitize_nameand_synthesize_llm_yaml: an extra blank line (ruff E303)_synthesize_llm_yamlandresolve_config_files: an extra blank line (ruff E303)Run
nox -s formatto auto-fix.Minor suggestion:
str(config_blob.get("provider") or "")— thestr()wrapper is redundant.config_blob.get("provider", "")is cleaner and equivalent.9. DOCUMENTATION -- PASS
Comprehensive docstrings on
_synthesize_llm_yaml(Args/Returns). Feature file has clear descriptions.10. COMMIT AND PR QUALITY -- NEEDS ATTENTION
ISSUES CLOSED: #10861footerType/Bug(mandatory)CI Status
_resolve_actor.py)Verdict: REQUEST_CHANGES
All 3 previously-requested items remain unaddressed, plus the CHANGELOG issue from the second review. The code change itself is sound — it correctly fixes the root cause of bug #10861.
Blocking Issues (must fix before approval)
_resolve_actor.pybetween functions. Runnox -s formatto auto-fix.Type/Buglabel to the PR. The emptylabels: []array means the PR does not meet PR Requirement #12.State/In reviewlabel.Non-Blocking Suggestions
for tdd-10861suffixes. Clean names likeGiven a built-in actor with provider and model but no type fieldwould be more readable as living documentation.config_blob.get("provider", "")instead ofstr(config_blob.get("provider") or "")for slightly cleaner code.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@ -52,6 +53,44 @@ def _sanitize_name(name: str) -> str:return "".join(c for c in name if c.isprintable())Double blank-line before function definition. This triggers ruff E303 (expected 2 blank lines, found 3). Run
nox -s formatto auto-fix.Double blank-line after function end, before next function. This triggers ruff E303. Run
nox -s formatto auto-fix.PR re-review submitted with REQUEST_CHANGES status (review ID: 7025).
All previously-requested items remain unaddressed. The author must fix lint formatting, add the required Type/Bug and State/In review labels, and update CHANGELOG.md before this PR can be approved.
PR Fix Attempt — Tier 3: sonnet — Success
Changes Made
Fixed the CI lint failure that was blocking this PR.
Root Cause: The
ruff formatcheck was failing because:src/cleveragents/cli/commands/_resolve_actor.pyhad two extra blank lines between_sanitize_name()and_synthesize_llm_yaml()features/steps/tdd_actor_run_response_steps.pyhad a long string literal that needed to be wrappedFix: Applied
nox -s formatto auto-format both files. Committed asstyle(actor): fix ruff format violations in actor-run fix(commit0b2c32cc).Reviewer Feedback Addressed
Type/Buglabel)Quality Gates
Automated by CleverAgents Bot
Supervisor: Implementation | Agent: task-implementor
agents actor rundoes not work. #10861agents actor rundoes not work. #10861Re-Review of PR #10900 -- fix(actor): Get responses from actor-run (Closes #10861)
Previous Feedback Review
From REQUEST_CHANGES Review (#6971)
From COMMENT Review (#7025)
38fa155eor0b2c32cc) modifies CHANGELOG.md. See inline comments below.Previous Review #7025 Re-Evaluation
Review #7025 (also REQUEST_CHANGES) listed 4 blocking issues:
0b2c32cc)Full 10-Category Review
1. CORRECTNESS -- PASS
The fix correctly identifies the condition where config_blob has provider and model but no type, and synthesizes a proper v3 type: llm YAML. Edge cases handled correctly: system_prompt conditionally included only if truthy, empty provider/model defaults to empty string via
or "". The ReactiveConfigParser parseability test confirms the synthesized output is usable by downstream code.2. SPECIFICATION ALIGNMENT -- PASS
The synthesized YAML follows the v3 type: llm format (name, type, description, provider, model, optional system_prompt). This is consistent with the project specification for v3 actors defined in docs/specification.md.
3. TEST QUALITY -- PASS
Three Behave BDD scenarios in features/tdd_actor_run_response.feature with @tdd_issue @tdd_issue_10861 tags:
All scenarios have @mock_only tags. Step file is 148 lines with clear mock setup and cleanup via context.add_cleanup().
Non-blocking note: Issue subtask mentioned a Robot Framework integration test; not yet added but does not block.
4. TYPE SAFETY -- PASS
from typing import Anycorrectly added to imports. _synthesize_llm_yaml signature fully annotated:(actor_name: str, config_blob: dict[str, Any]) -> str. No new# type: ignorecomments introduced. Pre-existing behaves import ignores are an established project pattern.5. READABILITY -- PASS
Excellent docstring on _synthesize_llm_yaml explaining the bug, root cause, fix, Args/Returns sections. The inline condition
if config_blob.get("provider") and config_blob.get("model") and not config_blob.get("type")clearly expresses the intent. Code flow in resolve_config_files is easy to follow.6. PERFORMANCE -- PASS
One-time YAML construction per actor resolution. Minimal overhead: dict construction + yaml.safe_dump call. No loop or N+1 concerns. No scalability issues.
7. SECURITY -- PASS
No hardcoded secrets, tokens, or credentials. system_prompt passed through from provider-registry config_blob, not from user input. No SQL/command injection or path traversal vectors. _sanitize_name still handles terminal escape injection as before.
8. CODE STYLE -- PASS
Follows ruff conventions (CI lint now green after format fix in commit
0b2c32cc). Proper docstrings on both new and updated functions. File well under 500 lines. Single Responsibility: YAML synthesis logic cleanly isolated in dedicated function.9. DOCUMENTATION -- PASS
New function _synthesize_llm_yaml has comprehensive docstring with Args/Returns. Updated resolve_config_files docstring documents the new behavioral branch. PR body has detailed Summary, Root Cause, Fix sections.
10. COMMIT AND PR QUALITY -- NEEDS ATTENTION
38fa155e): Atomic, Conventional Changelog formatfix(actor): Get responses from actor-run, footerISSUES CLOSED: #108610b2c32cc): Atomic format fixstyle(actor): fix ruff format violations in actor-run fix, footerISSUES CLOSED: #10861bugfix/m3-actor-run-responsematches naming convention for bugfix in milestone m3CI Status
Verdict: REQUEST_CHANGES
All code quality categories pass. The implementation correctly fixes bug #10861 with appropriate tests, correct type annotations, and good documentation. All previously-requested items from review #6971 (CI lint, Type/Bug label) have been addressed. The CHANGELOG issue from review #7025 remains outstanding.
Blocking Issue (must fix before approval)
38fa155eand0b2c32cc), so two CHANGELOG entries are needed. Add both entries in a follow-up commit to this PR.Non-Blocking Suggestions
for tdd-10861suffixes (e.g.,Given a built-in actor with provider and model but no type field for tdd-10861). Clean names likeGiven a built-in actor with provider and model but no type fieldwould be more readable as living documentation. Consider a follow-up commit to clean these up.str(config_blob.get("provider") or "")-- thestr()wrapper is redundant.config_blob.get("provider", "") or ""is slightly cleaner and equivalent.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Re-Review of PR #10900 -- fix(actor): Get responses from actor-run (Closes #10861)
Previous Feedback Review
From REQUEST_CHANGES Review (#7055)
Type/Buglabel.b9eebb6cadds a comprehensive 14-line CHANGELOG entry referencing #10861.From REQUEST_CHANGES Review (#6971)
Full 10-Category Review
1. CORRECTNESS -- PASS
The fix correctly identifies the exact condition:
config_blobhasproviderandmodelbut notype. The conditional guard properly isolates built-in provider-registry blobs without affecting custom actors that haveyaml_textor an existingtypefield. Edge cases handled: missingsystem_prompt(conditionally included), emptyprovider/model(defaults to empty string).2. SPECIFICATION ALIGNMENT -- PASS
The synthesized YAML follows the v3
type: llmformat withname,type,description,provider, andmodel-- the minimum required fields. Consistent with the project specification for v3 actors.3. TEST QUALITY -- PASS
Three Behave BDD scenarios with
@tdd_issue @tdd_issue_10861tags:yaml_textactors (non-interference)Step file is 150 lines with clear mock setup, cleanup via
context.add_cleanup(), and assertive error messages.4. TYPE SAFETY -- PASS
from typing import Anycorrectly added._synthesize_llm_yamlsignature fully annotated. No new# type: ignorecomments.5. READABILITY -- PASS
Excellent docstring on
_synthesize_llm_yamlexplaining the bug, root cause, fix, Args/Returns. Inline condition and code branch are self-explanatory.6. PERFORMANCE -- PASS
One-time YAML construction per actor resolution. Minimal overhead. No loop or N+1 concerns.
7. SECURITY -- PASS
No hardcoded secrets, tokens, or credentials.
system_promptpassed through from provider-registry config_blob, not user input. No injection vectors.8. CODE STYLE -- PASS
CI lint passing (all checks green). Proper docstrings. File well under 500 lines. SRP maintained.
9. DOCUMENTATION -- PASS
Comprehensive docstrings on
_synthesize_llm_yaml. Updatedresolve_config_filesdocs. CHANGELOG entry is detailed, 14-line, references #10861. PR body has detailed Summary, Root Cause, Fix sections.10. COMMIT AND PR QUALITY -- PASS
ISSUES CLOSED: #10861footer; changelog commit references#10861in titleISSUES CLOSED:footer (issue still referenced in title)CI Status
All 14 CI checks PASSING:
Verdict: APPROVED
All previously-requested items from reviews #6971 and #7025 have been fully addressed. The implementation correctly fixes bug #10861 with appropriate tests, correct type annotations, comprehensive documentation, and all CI checks passing.
Non-Blocking Observations
str(config_blob.get("provider") or "")-- thestr()wrapper is redundant;config_blob.get("provider", "") or ""is slightly cleaner.for tdd-10861suffixes. Clean step names without the qualifier would improve readability as living documentation.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Review of PR #10900 -- fix(actor): Get responses from actor-run (Closes #10861)
This is a fresh evaluation of PR #10900. The PR fixes bug #10861 where
agents actor run <built-in-actor>silently returned empty string.Previous Review Assessment
Review #7072 (APPROVED) identified all prior feedback items as addressed:
0b2c32cc)b9eebb6c, 13-line entry)I independently verify all these items remain addressed and found no new issues.
10-Category Review
1. CORRECTNESS - PASS
The fix correctly targets the exact condition: config_blob with provider+model but no type.
_synthesize_llm_yaml()constructs a proper v3type: llmYAML. Edge cases handled well — empty provider/model defaults viastr(... or ""), system_prompt conditionally included, and fallback toyaml.safe_dumpfor non-built-in actors preserves original behavior.2. SPECIFICATION ALIGNMENT - PASS
The synthesized YAML follows v3
type: llmformat (name, type, provider, model, optional system_prompt), matchingReactiveConfigParser._build_from_v3()expectations. Module docstring notes accepted spec deviation for --config option.3. TEST QUALITY - PASS
Three Behave BDD scenarios with
@tdd_issue @tdd_issue_10861tags:Mock setup is clean. Non-blocking suggestion: add a scenario for system_prompt path (lines 83-84 of _resolve_actor.py) to strengthen coverage.
4. TYPE SAFETY - PASS
from typing import Anyimported._synthesize_llm_yamlfully annotated(actor_name: str, config_blob: dict[str, Any]) -> str. No new# type: ignore. Behave# type: ignore[import-untyped]is established project pattern.5. READABILITY - PASS
Excellent docstring on
_synthesize_llm_yamlexplaining bug, root cause, args, return. Clear variable names. Clean control flow inresolve_config_files. Non-blocking suggestions: (1) Step descriptions have repetitivefor tdd-10861suffixes — cleaner names would improve living documentation quality; (2) Assertion error messages use string concatenation instead of f-strings.6. PERFORMANCE - PASS
New function: simple dict construction + YAML serialization. Trivial overhead. No new loops or N+1 patterns.
7. SECURITY - PASS
No hardcoded secrets or credentials.
_sanitize_nameprevents terminal escape injection.yaml.safe_dumpused throughout (not unsafeyaml.dump). System prompt from config_blob embedded safely.8. CODE STYLE - PASS
Ruff format violations addressed by commit
0b2c32cc. Files well under 500 lines. SRP maintained. Minor style note:str(config_blob.get("provider") or "")is slightly redundant since .get() already returns the value type —config_blob.get("provider", "") or ""would be cleaner.9. DOCUMENTATION - PASS
Detailed docstring on
_synthesize_llm_yaml. Feature file description provides full bug context. CHANGELOG entry is comprehensive (14 lines with root cause, affected files, fix details, test coverage note).10. COMMIT AND PR QUALITY - PASS
bugfix/m3-actor-run-responsematches conventionVerdict: APPROVED
All 10 checklist categories pass. The implementation correctly and minimally fixes bug #10861. The regression tests are well-structured and the fix is conservative (does not affect existing yaml_text actors).
Non-blocking suggestions (no action required):
for tdd-10861suffixes from Gherkin step descriptions for cleaner living documentation@ -0,0 +15,4 @@from behave.runner import Context # type: ignore[import-untyped]from cleveragents.cli.commands._resolve_actor import resolve_config_filesSuggestion: Step descriptions have repetitive
for tdd-10861suffixes that degrade readability as living documentation. Since the feature file title already identifies bug #10861, consider cleaner names likeGiven a built-in actor with provider and model but no type fieldinstead ofGiven a built-in actor with provider and model but no type field for tdd-10861.@ -0,0 +102,4 @@@then("the resulting YAML file contains the provider and model for tdd-10861")def step_then_yaml_contains_provider_model(context: Context) -> None:"""Assert the synthesised YAML contains the correct provider and model."""Suggestion: Assertion error messages use string concatenation (lines ~101, 106, 113). Consider using f-strings for cleaner presentation, e.g.
f"Expected type: llm, got: {parsed.get("type")}".@ -55,0 +69,4 @@Returns:A YAML string in v3 type: llm format that the reactive configparser can consume to produce a working single-agent graph route.Minor:
str(config_blob.get("provider") or "")is slightly redundant since.get()returns the existing type. Considerconfig_blob.get("provider", "") or ""for slightly cleaner code.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@HAL9000 PR #10930 fixed 10861 as a side effect. Before merging this PR, please compare this PR with 10930 and see if this one is not necessary anymore. If so, you should close this PR.
Review of PR #10900 (branch bugfix/m3-actor-run-response, head
b9eebb6c)OK
10-Category Review
✅ 1. CORRECTNESS
The fix correctly identifies the root cause: built-in LLM actors have a config_blob with provider and model but no type field. When deserialized as raw YAML and passed to ReactiveConfigParser, the absence of a type key (and no agents/routes keys) produces an empty ReactiveConfig. The fix intercepts this condition with a targeted guard (if config_blob.get(provider) and config_blob.get(model) and not config_blob.get(type)) and routes it to the new _synthesize_llm_yaml() helper. Logic is precise — only activates for built-in actors, not for actors with existing yaml_text or type fields.
✅ 2. SPECIFICATION ALIGNMENT
The fix follows the spec-defined v3 format for LLM actors (name, type, provider, model, optional system_prompt). The synthesized YAML includes type: llm, provider, model, and name — matching the expected v3 actor structure.
✅ 3. TEST QUALITY
Three Behave BDD scenarios cover all cases:
The ReactiveConfigParser validation in the parseability scenario is excellent — ensures end-to-end correctness.
✅ 4. TYPE SAFETY
The production code has proper type annotations: from typing import Any is imported, _synthesize_llm_yaml is fully typed with (actor_name: str, config_blob: dict[str, Any]) -> str, all variable assignments are type-safe, no type: ignore in production code.
✅ 5. READABILITY
_code is clear and well-structured:
Suggestion: The inline comment block could be slightly more concise.
✅ 6. PERFORMANCE
No concerns. The fix adds negligible overhead — one additional dict .get() check and potentially one yaml.safe_dump with a smaller dict.
✅ 7. SECURITY
No security concerns. No hardcoded secrets or credentials. yaml.safe_dump is correct. _sanitize_name is pre-existing and handles terminal escape injection.
✅ 8. CODE STYLE
Files are well under 500 lines. New function follows SOLID (single responsibility). Ruff-compatible (PR includes a ruff format fix commit). Imports follow project conventions.
✅ 9. DOCUMENTATION
_synthesize_llm_yaml has a comprehensive docstring. resolve_config_files docstring remains complete. CHANGELOG.md updated. Test feature file serves as living documentation.
✅ 10. COMMIT AND PR QUALITY
Non-Blocking Findings
Conclusion
This is a clean, well-scoped bug fix for a critical issue (#10861 — agents actor run returning nothing for built-in LLM actors). The fix is correct, well-tested, properly typed, and well-documented. CI is passing. The only concern is merge conflicts which should be resolved before merge.
Verdict: APPROVED with minor suggestion (pending conflict resolution).
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@ -112,2 +142,2 @@)raise typer.Exit(code=2) from None# Fix #10861: built-in actors have a config_blob with provider# and model but no type field. Serialising this blob as-isSuggestion: The inline comment block explaining the fix could be slightly more concise:
Fix #10861: Synthesize v3 YAML for built-in actors without a type field.instead of the current multi-line comment. Minor readability improvement only.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
First Review of PR #10900 -- fix(actor): Get responses from actor-run (Closes #10861)
Summary
Reviewed the full diff, linked issue #10861, PR description, CI status, and all 4 changed files.
CI Status
All 14 CI checks passing (commit
b9eebb6c): lint, build, quality, typecheck, security, push-validation, helm, integration_tests, e2e_tests, unit_tests, docker, coverage, status-check.10-Category Review
1. CORRECTNESS ✅
The fix correctly addresses the root cause of bug #10861. When a built-in LLM actor has a config_blob with provider and model but no type field, _synthesize_llm_yaml() constructs a proper v3 type: llm YAML blob containing name, type, provider, model, and optional system_prompt. This allows ReactiveConfigParser to create agents and routes, resolving the empty response issue. Edge cases are properly handled: missing system_prompt is skipped; empty provider/model falls through to original yaml.safe_dump path.
2. SPECIFICATION ALIGNMENT ✅
The synthesized YAML follows the v3 type: llm format (name, type: llm, provider, model, optional system_prompt, optional description) -- consistent with the project specification. The existing yaml_text path is untouched.
3. TEST QUALITY ✅
Three Behave BDD scenarios in features/tdd_actor_run_response.feature with @tdd_issue @tdd_issue_10861 tags:
Step definitions file (tdd_actor_run_response_steps.py) at 150 lines is well-structured with clear isolation via MagicMock and patch. Good use of temp file cleanup via add_cleanup.
4. TYPE SAFETY ✅
from typing import Any correctly added. _synthesize_llm_yaml signature is fully annotated: (actor_name: str, config_blob: dict[str, Any]) -> str. No new # type: ignore comments introduced. Pre-existing # type: ignore[import-untyped] on behave imports is an established project pattern.
5. READABILITY ✅
Excellent docstring on _synthesize_llm_yaml explaining the bug, root cause, fix, args, and return. Variable names are clear (v3_blob, provider, model, system_prompt). Logic flow in resolve_config_files is easy to follow with the new conditional block properly separated from the original error-handling path.
6. PERFORMANCE ✅
New function is trivial -- no performance concerns.
7. SECURITY ✅
No hardcoded secrets, tokens, or credentials. Input values (provider, model) are safely embedded in YAML via yaml.safe_dump. _sanitize_name still handles terminal escape injection correctly. No SQL/command injection vectors.
8. CODE STYLE ✅
SOLID principles followed -- SRP maintained. _synthesize_llm_yaml is a focused single-responsibility helper. All files well under 500 lines (_resolve_actor.py: 172 lines, step file: 150 lines). Follows ruff conventions (lint check passed).
9. DOCUMENTATION ✅
_synthesize_llm_yaml has a detailed docstring explaining the bug context, root cause, and fix. Feature file includes rich context about the bug and fix in its description. Changelog entry is thorough. Commit body explains motivation clearly.
10. COMMIT AND PR QUALITY ✅
Non-blocking Suggestions
Redundant str() wrapper (line ~89 of _resolve_actor.py): Consider using config_blob.get("provider", "") or "" instead of str(config_blob.get("provider") or "") -- the str() wrapper is redundant since .get() already returns the value as-is.
Repetitive TDD suffixes in Gherkin step descriptions: Step descriptions have repetitive for tdd-10861 suffixes. Since the feature file title already identifies the bug, clean names like Given a built-in actor with provider and model but no type field would improve readability as living documentation.
Missing State/In review label on issue #10861: The linked issue should carry the State/In review label; per project workflow, the issue should be moved to State/In review when the PR is submitted.
Verdict: APPROVED
All checklist categories pass. The implementation correctly fixes bug #10861 with appropriate, comprehensive tests. Minor suggestions noted above are non-blocking and can be addressed in a follow-up.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
harbor.cleverthis.comfromdocker:dind