test(actor): add regression test for issue 4321 nested config extraction #11126
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
3 participants
Notifications
Due date
No due date set.
Blocks
#4321 ActorRegistry.add() fails to extract provider and model from nested actor config
cleveragents/cleveragents-core
Reference
cleveragents/cleveragents-core!11126
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/actor-registry-provider-extraction"
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
typeis only at the nestedactors.<name>level (not at top level), provider and model are correctly extracted from the nestedconfigblockChanges
features/actor_registry_spec_yaml.feature: Added scenario with@tdd_issue @tdd_issue_4321tagsfeatures/steps/actor_registry_spec_yaml_steps.py: Added step definition for new scenarioCHANGELOG.md: Added entry documenting the regression testTesting
actor_registry_spec_yaml.featurepassnox -s lint: All checks passednox -s typecheck: 0 errors, 3 warnings (optional langchain deps)Closes #4321
219f49d74178b6b49974First Review — PR #11126:
test(actor): add regression test for issue 4321 nested config extractionThank you for the clear PR description and well-structured test. The regression test scenario itself is correctly written and follows the existing patterns in the codebase. All required CI gates pass (lint, typecheck, security, unit_tests, coverage, integration_tests, status-check). However, I have found four blocking issues that must be resolved before this PR can be approved.
❌ Blockers (must be resolved)
1. Commit message does not match the issue Metadata prescription
Issue #4321's
## Metadatasection prescribes the exact commit message:The actual commit uses:
Per CONTRIBUTING.md, the commit first line must be used verbatim from the issue Metadata. The current message also has the wrong
typeprefix (testvsfix) and a different scope.2. No companion TDD issue created for bug #4321
Issue #4321 is
Type/Bug. CONTRIBUTING.md requires that everyType/Bugissue have a companionType/Testingissue (with aTDD:title prefix), and the bug issue must depend on the TDD issue (TDD blocks the bug fix). No such companion issue was created for #4321. The current PR adds a test, which is good, but the process requires the TDD issue to exist separately so the dependency chain is explicit and traceable.3. Wrong branch prefix for the work being done
Branch
fix/actor-registry-provider-extractionuses thefix/prefix. Per CONTRIBUTING.md,fix/mN-branches are for bug fix implementation andtdd/mN-branches are for TDD test-only work. This PR only adds a test with no production code changes. The correct prefix for a test-only regression guard istdd/m6-actor-registry-provider-extraction.Note: I acknowledge the issue Metadata says
Branch: fix/actor-registry-provider-extraction. If the original intention was to fix the bug AND add the test in a single commit on this branch, but the fix ended up in #4300 instead, the issue Metadata needs to be updated to reflect the current state of the work.4.
Type/Bugissue closed by a test-only PR without verifying all acceptance criteriaIssue #4321 has six acceptance criteria — all of them describe behavioral requirements that the fix should satisfy (provider/model extraction working, actors registerable, plan execution not failing, etc.). This PR only adds a regression test. It relies on the fix from #4300 already being in master. Closing a
Type/Bugissue withCloses #4321on aType/TestingPR bypasses the acceptance criteria verification process.To properly close #4321, either:
fix/actor-registry-provider-extractionwork) should carryCloses #4321, and this test PR should reference a separate TDD issue with aRefs #4321note only⚠️ CI Failure — benchmark-regression
The
CI / benchmark-regressionjob is failing (run #20256, job #1). While this check is NOT in the required-for-mergestatus-checkgate, a failing CI job should always be explained. Please confirm whether this failure pre-dates this PR (i.e., was failing on master before this branch was cut) or was introduced by these changes. Given that this PR adds only test/changelog/feature files with no production code changes, this failure is most likely a pre-existing flake, but it should be explicitly acknowledged.✅ What is correct
@tdd_issue_4300scenario for comparison)@tdd_issue @tdd_issue_4321tags are used consistently with the codebase conventionType/Testinglabel is present ✅ISSUES CLOSED: #4321is present ✅Summary
This PR is close to correct but has four process violations that must be addressed. The most critical are the missing companion TDD issue (per the required workflow for bug tickets) and the commit message mismatch with the issue Metadata. Please address all four blockers and push a corrected commit.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
BLOCKER — Wrong branch prefix and missing companion TDD issue
Two related process violations:
Branch name:
fix/actor-registry-provider-extractionuses thefix/prefix which is reserved for bug fix implementation PRs (production code). This PR only adds a test. Per CONTRIBUTING.md, test-only work for a bug regression guard should use atdd/mN-branch. The correct name would betdd/m6-actor-registry-provider-extraction.Missing companion TDD issue: Issue #4321 is
Type/Bug. CONTRIBUTING.md requires a companionType/Testingissue (titledTDD: <same title>) to be created for every bug, and the bug issue must depend on (be blocked by) the TDD issue. This companion issue was never created. Please create the TDD companion issue and link it correctly before this test work can be merged.Note: The
@tdd_issue_4321tag itself is correct — it correctly identifies this scenario as the regression guard for issue #4321.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
BLOCKER —
Type/Bugissue should not be closed by a test-only PRCloses #4321in the PR body will close issue #4321 (Type/Bug) when this PR is merged. However:Closes #4321Why this is a problem: Closing a
Type/Bugissue via a test-only PR means there is no traceable record that the acceptance criteria were validated as part of the fix work. The proper workflow is:Closes #4321with evidence the acceptance criteria were metRefs #4321only (or reference the separate TDD companion issue)Please update the PR body to replace
Closes #4321withRefs #4321, and ensure the bug issue is closed by the PR that implements the actual fix.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
That request cannot be satisfied. the PR #4300 has already been merged to master and cannot be changed anymore. Consider opening an exception for this case, where the current master fails to have a regression test for part of what it implemented. It is better to have a test even if later, than nothing, as it is in master right now.
BLOCKER — Commit message mismatch with issue Metadata
Issue #4321
## Metadatasection prescribes this exact commit message:The actual commit on this PR uses:
Per CONTRIBUTING.md, the commit first line must match verbatim the
Commit Messagefield in the issue Metadata section. Please amend the commit to use the prescribed message exactly.If the prescribed message no longer accurately describes this PR (since the fix was done in #4300 and this PR only adds the test), then the issue Metadata should first be updated to reflect the actual work, and then the commit message should match that updated Metadata.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
While that is true, this commit does not include code changes, only tests, so it is better to use the more accurate description, maybe issue #4321 metadata should be updated instead, to math what we are actually doing.
Formal review submitted (review ID: 8630) — REQUEST_CHANGES.
Four blocking issues were identified. See the review for full details and inline comments on the specific violations. Once all blockers are addressed, please re-request review.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@tdd_issue_4321the issues is not related to the test78b6b49974ad55c3a8e9ad55c3a8e9f6ca9b7291