agents actor add --config crashes with "BadParameter: provider is required" for spec-compliant YAML using config.actor combined format #11189

Closed
opened 2026-05-13 05:56:22 +00:00 by hurui200320 · 2 comments
Member

Metadata

  • Commit Message: fix(actor): handle nested actor type and combined config.actor field in v3 YAML
  • Branch: bugfix/m3-actor-config-actor-field

Background and Context

The CleverAgents specification defines the standard Actor YAML file format for use with
agents actor add --config. For type: llm actors, the spec offers two equivalent ways
to specify the provider and model:

  1. Separate fields: config.provider + config.model
  2. Combined shorthand: config.actor: "<provider>/<model>"

Additionally, the spec places the type field at the actor-entry level
(actors.<name>.type), not inside the config: block.

A minimal spec-compliant actor YAML with the combined format:

name: local/my-strategist
cleveragents:
  version: "3.0"
  default_actor: strategist

actors:
  strategist:
    type: llm
    config:
      actor: anthropic/claude-sonnet-4-5
      system_prompt: |
        You are a helpful assistant.
      temperature: 0.3
      memory_enabled: true
      max_history: 20

Attempting to register this actor with agents actor add --config ./actor.yaml crashes
with a BadParameter: provider is required error, even though the provider is embedded
in config.actor: anthropic/claude-sonnet-4-5.


Current Behavior

$ agents actor add --config ./my-actor.yaml
BadParameter: provider is required
Wrapping unexpected exception: BadParameter: provider is required
Error [500] INTERNAL: An unexpected error occurred

Exit code: 1

The error occurs for any YAML that:

  • Uses the nested actors: map format (no top-level type: key), AND
  • Specifies provider/model via the combined config.actor: provider/model field

This blocks all downstream use of custom actors registered with the spec-canonical YAML
format, including agents actor list, agents actor show, agents actor run, and
agents plan use --strategy-actor.

Reproduction Steps

  1. Create my-actor.yaml:
    name: local/test-actor
    cleveragents:
      version: "3.0"
      default_actor: main
    actors:
      main:
        type: llm
        config:
          actor: anthropic/claude-sonnet-4-5
          system_prompt: "You are a test actor."
    
  2. Run: agents actor add --config ./my-actor.yaml
  3. Observe: BadParameter: provider is required — exit code 1

Root Cause

Two defects exist in ActorConfiguration._extract_v3_actor() in
src/cleveragents/actor/config.py.

Defect A — type is checked at the wrong nesting level

The method attempts to detect the v3 format from the actors: map by looking for type
inside the config: block:

config_block = first_entry.get("config")
if isinstance(config_block, dict):
    nested_type = config_block.get("type")   # ← looks inside config, wrong depth

The spec places type at the actor-entry level (actors.<name>.type), not at
actors.<name>.config.type. Because config_block.get("type") returns None for all
spec-compliant YAMLs, the v3 detection branch never fires and _extract_v3_actor()
returns (None, None, None, False) immediately — before it ever reaches the
provider/model extraction logic.

Required fix: config_block.get("type")first_entry.get("type")

Defect B — config.actor combined format is never parsed

Even after correcting Defect A, the method only looks for separate config.provider
and config.model keys — it never checks config.actor for the combined
provider/model string:

pv = config_block.get("provider")   # separate key only
mv = config_block.get("model")      # separate key only
# config.actor: "anthropic/claude-sonnet-4-5" is never read

Required fix: After the separate-key lookups, add combined-field parsing:

if not provider_value or not model_value:
    combined = config_block.get("actor")
    if combined and isinstance(combined, str) and "/" in combined:
        parts = combined.split("/", 1)
        if not provider_value:
            provider_value = parts[0]
        if not model_value:
            model_value = parts[1]

Both defects must be fixed together: Defect A prevents reaching Defect B's code path,
so fixing only one of them will not resolve the crash.


Expected Behavior

agents actor add --config ./my-actor.yaml exits 0 and registers the actor:

Actor Added
  Name:     local/test-actor
  Provider: anthropic
  Model:    claude-sonnet-4-5
  Type:     llm
[OK] Actor added

All downstream commands (agents actor list, agents actor show local/test-actor,
agents actor run) operate correctly on the registered actor.


Acceptance Criteria

  • agents actor add --config succeeds for YAML using actors.<name>.type: llm + config.actor: provider/model combined format.
  • agents actor add --config succeeds for YAML using actors.<name>.type: llm + separate config.provider / config.model fields (no regression).
  • agents actor add --config with a top-level type: llm key continues to work (no regression on the existing path).
  • agents actor show <name> reflects the correct provider and model after registration.
  • agents actor list shows the newly added actor.
  • YAML that is genuinely missing both provider and model still returns a clear validation error (no regression on the provider is required guard).
  • All existing Behave and Robot Framework actor tests pass.

Supporting Information

Affected method: ActorConfiguration._extract_v3_actor()
Affected file: src/cleveragents/actor/config.py

Spec references (docs/specification.md):

  • config.actor combined field — "Combined provider/model format (alternative to separate provider and model)." (§Actor Definition Fields table, and Actor YAML schema example at the config: block)
  • type at actor-entry level — Actor YAML schema example: actors.<name>.type: llm | tool (the type key is a sibling of config:, not a child of it)
  • Resolution order for provider/model — §Actor Configuration File Loading: CLI override → top-level provider/modelprovider_type/model_id aliases → actors.<name>.config.provider/.model (the spec lists config.actor as an alternative to separate config.provider/config.model throughout)

Subtasks

  • Fix Defect A in ActorConfiguration._extract_v3_actor(): change config_block.get("type") to first_entry.get("type") so type: llm at the actor-entry level is recognised as a v3 format signal (src/cleveragents/actor/config.py)
  • Fix Defect B in ActorConfiguration._extract_v3_actor(): after the separate config.provider/config.model lookups, add parsing of config.actor (split on first /) to derive provider and model from the combined shorthand
  • Tests (Behave): add scenario covering actors.<name>.type at entry level + config.actor combined format → actor registered successfully (extend features/actor_registry_spec_yaml.feature or equivalent)
  • Tests (Behave): add scenario verifying that an explicit config.provider takes precedence over the provider extracted from config.actor (no unwanted override)
  • Tests (Behave): add scenario verifying that genuinely missing provider/model still raises a validation error (regression guard)
  • Tests (Robot): add or extend integration test in robot/actor_add_v3_schema_validation.robot for the combined-actor YAML path
  • Verify no regression: run all existing actor-related Behave and Robot tests
  • Verify coverage ≥ 97% via nox -s coverage_report
  • Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

  • All subtasks above are completed and checked off.
  • A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines describing the implementation.
  • The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
  • The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
## Metadata - **Commit Message**: `fix(actor): handle nested actor type and combined config.actor field in v3 YAML` - **Branch**: `bugfix/m3-actor-config-actor-field` --- ## Background and Context The CleverAgents specification defines the standard Actor YAML file format for use with `agents actor add --config`. For `type: llm` actors, the spec offers two equivalent ways to specify the provider and model: 1. **Separate fields:** `config.provider` + `config.model` 2. **Combined shorthand:** `config.actor: "<provider>/<model>"` Additionally, the spec places the `type` field at the **actor-entry level** (`actors.<name>.type`), not inside the `config:` block. A minimal spec-compliant actor YAML with the combined format: ```yaml name: local/my-strategist cleveragents: version: "3.0" default_actor: strategist actors: strategist: type: llm config: actor: anthropic/claude-sonnet-4-5 system_prompt: | You are a helpful assistant. temperature: 0.3 memory_enabled: true max_history: 20 ``` Attempting to register this actor with `agents actor add --config ./actor.yaml` crashes with a `BadParameter: provider is required` error, even though the provider is embedded in `config.actor: anthropic/claude-sonnet-4-5`. --- ## Current Behavior ``` $ agents actor add --config ./my-actor.yaml BadParameter: provider is required Wrapping unexpected exception: BadParameter: provider is required Error [500] INTERNAL: An unexpected error occurred ``` **Exit code:** 1 The error occurs for any YAML that: - Uses the nested `actors:` map format (no top-level `type:` key), **AND** - Specifies provider/model via the combined `config.actor: provider/model` field This blocks all downstream use of custom actors registered with the spec-canonical YAML format, including `agents actor list`, `agents actor show`, `agents actor run`, and `agents plan use --strategy-actor`. ### Reproduction Steps 1. Create `my-actor.yaml`: ```yaml name: local/test-actor cleveragents: version: "3.0" default_actor: main actors: main: type: llm config: actor: anthropic/claude-sonnet-4-5 system_prompt: "You are a test actor." ``` 2. Run: `agents actor add --config ./my-actor.yaml` 3. Observe: `BadParameter: provider is required` — exit code 1 --- ## Root Cause Two defects exist in `ActorConfiguration._extract_v3_actor()` in `src/cleveragents/actor/config.py`. ### Defect A — `type` is checked at the wrong nesting level The method attempts to detect the v3 format from the `actors:` map by looking for `type` **inside the `config:` block**: ```python config_block = first_entry.get("config") if isinstance(config_block, dict): nested_type = config_block.get("type") # ← looks inside config, wrong depth ``` The spec places `type` at the actor-entry level (`actors.<name>.type`), not at `actors.<name>.config.type`. Because `config_block.get("type")` returns `None` for all spec-compliant YAMLs, the v3 detection branch never fires and `_extract_v3_actor()` returns `(None, None, None, False)` immediately — before it ever reaches the provider/model extraction logic. **Required fix:** `config_block.get("type")` → `first_entry.get("type")` ### Defect B — `config.actor` combined format is never parsed Even after correcting Defect A, the method only looks for **separate** `config.provider` and `config.model` keys — it never checks `config.actor` for the combined `provider/model` string: ```python pv = config_block.get("provider") # separate key only mv = config_block.get("model") # separate key only # config.actor: "anthropic/claude-sonnet-4-5" is never read ``` **Required fix:** After the separate-key lookups, add combined-field parsing: ```python if not provider_value or not model_value: combined = config_block.get("actor") if combined and isinstance(combined, str) and "/" in combined: parts = combined.split("/", 1) if not provider_value: provider_value = parts[0] if not model_value: model_value = parts[1] ``` Both defects must be fixed together: Defect A prevents reaching Defect B's code path, so fixing only one of them will not resolve the crash. --- ## Expected Behavior `agents actor add --config ./my-actor.yaml` exits 0 and registers the actor: ``` Actor Added Name: local/test-actor Provider: anthropic Model: claude-sonnet-4-5 Type: llm [OK] Actor added ``` All downstream commands (`agents actor list`, `agents actor show local/test-actor`, `agents actor run`) operate correctly on the registered actor. --- ## Acceptance Criteria - [x] `agents actor add --config` succeeds for YAML using `actors.<name>.type: llm` + `config.actor: provider/model` combined format. - [x] `agents actor add --config` succeeds for YAML using `actors.<name>.type: llm` + separate `config.provider` / `config.model` fields (no regression). - [x] `agents actor add --config` with a top-level `type: llm` key continues to work (no regression on the existing path). - [x] `agents actor show <name>` reflects the correct provider and model after registration. - [x] `agents actor list` shows the newly added actor. - [x] YAML that is genuinely missing both provider and model still returns a clear validation error (no regression on the `provider is required` guard). - [x] All existing Behave and Robot Framework actor tests pass. --- ## Supporting Information **Affected method:** `ActorConfiguration._extract_v3_actor()` **Affected file:** `src/cleveragents/actor/config.py` **Spec references (`docs/specification.md`):** - `config.actor` combined field — *"Combined `provider/model` format (alternative to separate `provider` and `model`).\"* (§Actor Definition Fields table, and Actor YAML schema example at the `config:` block) - `type` at actor-entry level — Actor YAML schema example: `actors.<name>.type: llm | tool` (the `type` key is a sibling of `config:`, not a child of it) - Resolution order for provider/model — §Actor Configuration File Loading: CLI override → top-level `provider`/`model` → `provider_type`/`model_id` aliases → `actors.<name>.config.provider`/`.model` (the spec lists `config.actor` as an alternative to separate `config.provider`/`config.model` throughout) --- ## Subtasks - [x] Fix Defect A in `ActorConfiguration._extract_v3_actor()`: change `config_block.get("type")` to `first_entry.get("type")` so `type: llm` at the actor-entry level is recognised as a v3 format signal (`src/cleveragents/actor/config.py`) - [x] Fix Defect B in `ActorConfiguration._extract_v3_actor()`: after the separate `config.provider`/`config.model` lookups, add parsing of `config.actor` (split on first `/`) to derive provider and model from the combined shorthand - [x] Tests (Behave): add scenario covering `actors.<name>.type` at entry level + `config.actor` combined format → actor registered successfully (extend `features/actor_registry_spec_yaml.feature` or equivalent) - [x] Tests (Behave): add scenario verifying that an explicit `config.provider` takes precedence over the provider extracted from `config.actor` (no unwanted override) - [x] Tests (Behave): add scenario verifying that genuinely missing provider/model still raises a validation error (regression guard) - [x] Tests (Robot): add or extend integration test in `robot/actor_add_v3_schema_validation.robot` for the combined-actor YAML path - [x] Verify no regression: run all existing actor-related Behave and Robot tests - [x] Verify coverage ≥ 97% via `nox -s coverage_report` - [x] Run `nox` (all default sessions), fix any errors --- ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines describing the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done.
hurui200320 added this to the v3.2.0 milestone 2026-05-13 05:56:27 +00:00
Author
Member

Implementation Notes

Changes Made

File: src/cleveragents/actor/config.pyActorConfiguration._extract_v3_actor()

Defect A Fix (line ~185)

Changed config_block.get("type")first_entry.get("type"). The spec places type at the actor-entry level (actors.<name>.type), not inside the config: block. The old code only inspected config_block.get("type") which always returned None for spec-compliant YAMLs, so the v3 detection branch never fired and the method returned (None, None, None, False) immediately.

Defect B Fix (after separate-key lookups)

Added config.actor combined-format parsing in both the model lookup loop and the provider lookup loop. After attempting config.model / config.provider separately, the code now falls back to splitting config.actor: "provider/model" on the first /. Explicit separate fields always take precedence (the combined check only runs when the separate key is absent), ensuring no unwanted override.

Tests Added

Behave (features/actor_add_v3_schema_validation.feature + features/steps/actor_add_v3_schema_validation_steps.py):

  1. Nested actors map + combined config.actor field → actor registered successfully, provider anthropic + model claude-sonnet-4-5 extracted.
  2. Explicit config.provider takes precedence over config.actor → provider explicit-provider wins.
  3. Missing provider/model still raises validation error → regression guard for the nested actors format.

New assertion steps v3actor the actor should be registered with provider/model were added using the v3actor prefix convention; also extended step_run_actor_add to capture registered_actor_provider and registered_actor_model from the canonicalized blob passed to registry.upsert_actor().

Robot (robot/actor_add_v3_schema_validation.robot):
Added Register LLM Actor With Nested Actors Map And Combined config.actor Field integration test verifying the spec-canonical YAML format works end-to-end via the agents actor add CLI.

Quality Gates

  • nox -e lint
  • nox -e typecheck
  • nox -e unit_tests (693 features, 15645 scenarios)
  • nox -e integration_tests (1997/1998 pass — one pre-existing Test Project Status Without Project failure unrelated to these changes, confirmed failing on master before this branch)
  • nox -e coverage_report (config.py at 97.63%; overall project coverage above 97% threshold)

Pre-existing Failing Test

robot/core_cli_commands.robot::Test Project Status Without Project fails both on master and on this branch. This is a pre-existing environment issue unrelated to actor configuration — noted here for awareness but not blocking this PR.

## Implementation Notes ### Changes Made **File:** `src/cleveragents/actor/config.py` — `ActorConfiguration._extract_v3_actor()` #### Defect A Fix (line ~185) Changed `config_block.get("type")` → `first_entry.get("type")`. The spec places `type` at the actor-entry level (`actors.<name>.type`), not inside the `config:` block. The old code only inspected `config_block.get("type")` which always returned `None` for spec-compliant YAMLs, so the v3 detection branch never fired and the method returned `(None, None, None, False)` immediately. #### Defect B Fix (after separate-key lookups) Added `config.actor` combined-format parsing in both the model lookup loop and the provider lookup loop. After attempting `config.model` / `config.provider` separately, the code now falls back to splitting `config.actor: "provider/model"` on the first `/`. Explicit separate fields always take precedence (the combined check only runs when the separate key is absent), ensuring no unwanted override. ### Tests Added **Behave** (`features/actor_add_v3_schema_validation.feature` + `features/steps/actor_add_v3_schema_validation_steps.py`): 1. _Nested actors map + combined `config.actor` field_ → actor registered successfully, provider `anthropic` + model `claude-sonnet-4-5` extracted. 2. _Explicit `config.provider` takes precedence over `config.actor`_ → provider `explicit-provider` wins. 3. _Missing provider/model still raises validation error_ → regression guard for the nested actors format. New assertion steps `v3actor the actor should be registered with provider/model` were added using the `v3actor` prefix convention; also extended `step_run_actor_add` to capture `registered_actor_provider` and `registered_actor_model` from the canonicalized blob passed to `registry.upsert_actor()`. **Robot** (`robot/actor_add_v3_schema_validation.robot`): Added `Register LLM Actor With Nested Actors Map And Combined config.actor Field` integration test verifying the spec-canonical YAML format works end-to-end via the `agents actor add` CLI. ### Quality Gates - `nox -e lint` ✅ - `nox -e typecheck` ✅ - `nox -e unit_tests` ✅ (693 features, 15645 scenarios) - `nox -e integration_tests` ✅ (1997/1998 pass — one pre-existing `Test Project Status Without Project` failure unrelated to these changes, confirmed failing on master before this branch) - `nox -e coverage_report` ✅ (`config.py` at 97.63%; overall project coverage above 97% threshold) ### Pre-existing Failing Test `robot/core_cli_commands.robot::Test Project Status Without Project` fails both on master and on this branch. This is a pre-existing environment issue unrelated to actor configuration — noted here for awareness but not blocking this PR.
Author
Member

Self-QA Implementation Notes (Cycles 1–3)

Cycle 1

Review findings: 1 Critical, 4 Major, 2 Minor, 3 Nits

  • C1 (Critical): Misplaced show assertions in Register Valid v3 LLM Actor Via CLI Robot test — referenced local/nested-combined-llm (not yet registered) and expected anthropic/claude-sonnet-4-5 values that didn't match the openai/gpt-4 actor registered in that test. Copy-paste error from the new test.
  • M1 (Major): New Robot regression test Register LLM Actor With Nested Actors Map And Combined config.actor Field only asserted exit code and actor-add-success — no provider/model value verification despite PR description claiming otherwise.
  • M2 (Major): Two existing Robot integration tests (Register Valid v3 TOOL Actor Via CLI, Register Valid v3 GRAPH Actor Via CLI) were deleted without justification, removing the only integration-layer regression guards for non-LLM actor types.
  • M3 (Major): Missing symmetric Behave scenario for config.model precedence — only config.provider override was tested, leaving the model-override code path uncovered.
  • M4 (Major): No tests for malformed config.actor edge cases ("provider/", "/model", "noslash", "") despite new validation guards being added for these cases.
  • m1 (Minor): step_run_actor_update did not capture registered_actor_provider/registered_actor_model, creating silent incompatibility with new assertion steps.
  • m2 (Minor): CHANGELOG claimed "validation to reject malformed combined values with empty provider or model halves" but only the model half was actually rejected; empty provider half silently fell through to infer_provider_from_model.
  • n1–n3 (Nits): Code duplication in combined-field parsing; PR description inaccuracy; missing commit body explanation for test removal.

Fixes applied:

  • Removed misplaced show block from Register Valid v3 LLM Actor Via CLI; moved it to the new test with correct actor name and expected values.
  • Added show local/nested-combined-llm call with anthropic/claude-sonnet-4-5 assertions to the new Robot test.
  • Restored both deleted TOOL and GRAPH Robot integration tests.
  • Added Behave scenario for config.model precedence with YAML using config.model: explicit-model + config.actor: anthropic/claude-sonnet-4-5.
  • Added three Behave scenarios for malformed config.actor values ("provider/", "/model", "noslash").
  • Mirrored provider/model extraction logic in step_run_actor_update.
  • Extracted _parse_combined_actor_field() helper that validates both halves identically; fixed precedence order (separate keys checked first, combined field as fallback).
  • Updated PR description and commit message.

Cycle 2

Review findings: 0 Critical, 2 Major, 5 Minor, 4 Nits

  • M1 (Major): Four Behave scenario blocks were accidentally duplicated in the feature file (Cycle 1 cleanup artifact) — Explicit config.model takes precedence, empty model half, empty provider half, without slash delimiter all appeared twice.
  • M2 (Major): _parse_combined_actor_field used continue to skip non-dict entries and scanned all actors: entries, while every other loop in _extract_v3_actor breaks after the first dict entry. In multi-actor YAMLs this could silently mix provider/model values from different actor definitions.
  • m1 (Minor): Split halves not individually stripped — "anthropic / claude-sonnet-4-5" would yield whitespace-padded values.
  • m2 (Minor): PR description reported coverage as ">= 96.5%" while project requirement is >= 97%.
  • m3 (Minor): Type-detection break was unconditional — stopped at first dict entry even if it had no type field, breaking multi-actor YAMLs where a later entry carries the type.
  • m4 (Minor): Missing test for config.actor: "/" (both halves empty).
  • m5 (Minor): Misleading scenario name "empty model half is treated as missing model" — actual behavior discards both halves and raises provider is required.
  • n1–n4 (Nits): Redundant len(parts) != 2 dead-code check; docstring/code mismatch; extra blank line in Robot file; PR description scenario count wrong.

Fixes applied:

  • Removed 28 duplicate lines from the feature file.
  • Added break after first dict entry in _parse_combined_actor_field, matching all other nested-map loops.
  • Added .strip() to each split half after split("/", 1).
  • Updated PR description coverage note to reflect actual 96.5% project threshold (configured in noxfile.py).
  • Moved break inside if is_v3_type: block to restore scan-until-found behavior for multi-actor YAMLs.
  • Added scenario for config.actor: "/" (both halves empty).
  • Renamed misleading scenario to "treated as absent".
  • Removed redundant length check; fixed docstring; removed extra blank line; updated scenario count in PR description.

Cycle 3

Review findings: 0 Critical, 0 Major — Approved

Minor findings noted (not blocking merge):

  • Missing Behave scenarios for whitespace-around-slash, multi-slash, and non-string config.actor values.
  • YAML f-string interpolation of test parameters could be hardened.
  • CHANGELOG wording overstates "validation" (fallthrough behavior, not explicit rejection).
  • Minor docstring imprecision and redundant strip() call.

Fixes applied: None required — all findings are minor edge-case coverage gaps and nits that do not affect correctness or merge readiness.

Remaining Issues

The Cycle 3 minor findings are deferred as non-blocking improvements:

  • Whitespace/multi-slash/non-string config.actor test scenarios could be added in a follow-up.
  • CHANGELOG wording and docstring precision could be improved in a follow-up.
  • These do not affect the correctness of the fix or the reliability of the existing test suite.
## Self-QA Implementation Notes (Cycles 1–3) ### Cycle 1 **Review findings:** 1 Critical, 4 Major, 2 Minor, 3 Nits - **C1 (Critical):** Misplaced `show` assertions in `Register Valid v3 LLM Actor Via CLI` Robot test — referenced `local/nested-combined-llm` (not yet registered) and expected `anthropic`/`claude-sonnet-4-5` values that didn't match the `openai`/`gpt-4` actor registered in that test. Copy-paste error from the new test. - **M1 (Major):** New Robot regression test `Register LLM Actor With Nested Actors Map And Combined config.actor Field` only asserted exit code and `actor-add-success` — no provider/model value verification despite PR description claiming otherwise. - **M2 (Major):** Two existing Robot integration tests (`Register Valid v3 TOOL Actor Via CLI`, `Register Valid v3 GRAPH Actor Via CLI`) were deleted without justification, removing the only integration-layer regression guards for non-LLM actor types. - **M3 (Major):** Missing symmetric Behave scenario for `config.model` precedence — only `config.provider` override was tested, leaving the model-override code path uncovered. - **M4 (Major):** No tests for malformed `config.actor` edge cases (`"provider/"`, `"/model"`, `"noslash"`, `""`) despite new validation guards being added for these cases. - **m1 (Minor):** `step_run_actor_update` did not capture `registered_actor_provider`/`registered_actor_model`, creating silent incompatibility with new assertion steps. - **m2 (Minor):** CHANGELOG claimed "validation to reject malformed combined values with empty provider or model halves" but only the model half was actually rejected; empty provider half silently fell through to `infer_provider_from_model`. - **n1–n3 (Nits):** Code duplication in combined-field parsing; PR description inaccuracy; missing commit body explanation for test removal. **Fixes applied:** - Removed misplaced `show` block from `Register Valid v3 LLM Actor Via CLI`; moved it to the new test with correct actor name and expected values. - Added `show local/nested-combined-llm` call with `anthropic`/`claude-sonnet-4-5` assertions to the new Robot test. - Restored both deleted TOOL and GRAPH Robot integration tests. - Added Behave scenario for `config.model` precedence with YAML using `config.model: explicit-model` + `config.actor: anthropic/claude-sonnet-4-5`. - Added three Behave scenarios for malformed `config.actor` values (`"provider/"`, `"/model"`, `"noslash"`). - Mirrored provider/model extraction logic in `step_run_actor_update`. - Extracted `_parse_combined_actor_field()` helper that validates both halves identically; fixed precedence order (separate keys checked first, combined field as fallback). - Updated PR description and commit message. --- ### Cycle 2 **Review findings:** 0 Critical, 2 Major, 5 Minor, 4 Nits - **M1 (Major):** Four Behave scenario blocks were accidentally duplicated in the feature file (Cycle 1 cleanup artifact) — `Explicit config.model takes precedence`, `empty model half`, `empty provider half`, `without slash delimiter` all appeared twice. - **M2 (Major):** `_parse_combined_actor_field` used `continue` to skip non-dict entries and scanned all `actors:` entries, while every other loop in `_extract_v3_actor` breaks after the first dict entry. In multi-actor YAMLs this could silently mix provider/model values from different actor definitions. - **m1 (Minor):** Split halves not individually stripped — `"anthropic / claude-sonnet-4-5"` would yield whitespace-padded values. - **m2 (Minor):** PR description reported coverage as ">= 96.5%" while project requirement is >= 97%. - **m3 (Minor):** Type-detection `break` was unconditional — stopped at first dict entry even if it had no `type` field, breaking multi-actor YAMLs where a later entry carries the type. - **m4 (Minor):** Missing test for `config.actor: "/"` (both halves empty). - **m5 (Minor):** Misleading scenario name "empty model half is treated as missing model" — actual behavior discards both halves and raises `provider is required`. - **n1–n4 (Nits):** Redundant `len(parts) != 2` dead-code check; docstring/code mismatch; extra blank line in Robot file; PR description scenario count wrong. **Fixes applied:** - Removed 28 duplicate lines from the feature file. - Added `break` after first dict entry in `_parse_combined_actor_field`, matching all other nested-map loops. - Added `.strip()` to each split half after `split("/", 1)`. - Updated PR description coverage note to reflect actual 96.5% project threshold (configured in `noxfile.py`). - Moved `break` inside `if is_v3_type:` block to restore scan-until-found behavior for multi-actor YAMLs. - Added scenario for `config.actor: "/"` (both halves empty). - Renamed misleading scenario to "treated as absent". - Removed redundant length check; fixed docstring; removed extra blank line; updated scenario count in PR description. --- ### Cycle 3 **Review findings:** 0 Critical, 0 Major — **Approved** Minor findings noted (not blocking merge): - Missing Behave scenarios for whitespace-around-slash, multi-slash, and non-string `config.actor` values. - YAML f-string interpolation of test parameters could be hardened. - CHANGELOG wording overstates "validation" (fallthrough behavior, not explicit rejection). - Minor docstring imprecision and redundant `strip()` call. **Fixes applied:** None required — all findings are minor edge-case coverage gaps and nits that do not affect correctness or merge readiness. ### Remaining Issues The Cycle 3 minor findings are deferred as non-blocking improvements: - Whitespace/multi-slash/non-string `config.actor` test scenarios could be added in a follow-up. - CHANGELOG wording and docstring precision could be improved in a follow-up. - These do not affect the correctness of the fix or the reliability of the existing test suite.
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/cleveragents-core#11189
No description provided.