From e1dc9aa283dd2d965400aabf9bdd193fd9104419 Mon Sep 17 00:00:00 2001 From: HAL9000 Date: Fri, 8 May 2026 09:01:10 +0000 Subject: [PATCH] fix(acms): resolve AmbiguousStep regression, lint violations, and type safety issues Resolve blocking issues identified in final review (ID 7998) for PR #9671: 1. AmbiguousStep fix: Renamed duplicate step '@then("the policy should not be applied")' to '@then("the policy should not be applied to the LLM context")' in acms_plan_execution_integration_steps.py and updated corresponding scenario in features/acms_plan_execution_integration.feature 2. lint fix (SIM102): Combined nested if statements into single compound condition in step_set_policy_priority() to remove ruff SIM102 violation 3. Type safety fix: Changed 'policy: Any' to 'policy: ContextPolicyConfig' in ACMSContextAssembler._apply_policy() for proper Pyright type safety, added ContextPolicyConfig to module imports This resolves the unit_tests CI failure caused by AmbiguousStep and fixes the lint CI failure introduced by commit 3457fc61. ISSUES CLOSED: #9584 --- .../acms_plan_execution_integration.feature | 2 +- .../acms_plan_execution_integration_steps.py | 19 +++++++++++-------- .../acms/plan_execution_integration.py | 5 ++++- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/features/acms_plan_execution_integration.feature b/features/acms_plan_execution_integration.feature index 289363695..be36065cb 100644 --- a/features/acms_plan_execution_integration.feature +++ b/features/acms_plan_execution_integration.feature @@ -59,7 +59,7 @@ Feature: Plan Execution ACMS Integration When I prepare LLM context with file_type "python" Then the policy should be applied When I prepare LLM context with file_type "javascript" - Then the policy should not be applied + Then the policy should not be applied to the LLM context Scenario: PlanExecutor accepts ACMS integration via dependency injection Given I have a plan execution ACMS integration with a policy configuration diff --git a/features/steps/acms_plan_execution_integration_steps.py b/features/steps/acms_plan_execution_integration_steps.py index 8cf5f3dc3..437771f64 100644 --- a/features/steps/acms_plan_execution_integration_steps.py +++ b/features/steps/acms_plan_execution_integration_steps.py @@ -244,13 +244,16 @@ def step_have_scope_rules(context: Any) -> None: def step_set_policy_priority(context: Any, count: int, weight: float) -> None: """Set the priority weight for a specific policy by index.""" idx = count - 1 - if context.policy_config and context.policy_config.policies: - if 0 <= idx < len(context.policy_config.policies): - context.policy_config.policies[idx].priority_weight = float(weight) - # Rebuild integration with updated config - context.integration = PlanExecutionACMSIntegration( - policy_config=context.policy_config, - ) + if ( + context.policy_config + and context.policy_config.policies + and 0 <= idx < len(context.policy_config.policies) + ): + context.policy_config.policies[idx].priority_weight = float(weight) + # Rebuild integration with updated config + context.integration = PlanExecutionACMSIntegration( + policy_config=context.policy_config, + ) @given("the policy has budget_override {amount:int}") @@ -302,7 +305,7 @@ def step_check_budget(context: Any, amount: int) -> None: assert assembled_data["budget"] == int(amount) -@then("the policy should not be applied") +@then("the policy should not be applied to the LLM context") def step_policy_not_applied(context: Any) -> None: """Check that the policy was NOT applied.""" assert context.llm_context is not None diff --git a/src/cleveragents/acms/plan_execution_integration.py b/src/cleveragents/acms/plan_execution_integration.py index adb8ee7c5..7ad829d89 100644 --- a/src/cleveragents/acms/plan_execution_integration.py +++ b/src/cleveragents/acms/plan_execution_integration.py @@ -10,6 +10,7 @@ from __future__ import annotations from typing import Any from cleveragents.acms.context_policy_loader import ( + ContextPolicyConfig, ContextPolicyConfigurationLoader, PolicyScope, ViewPolicyConfiguration, @@ -80,7 +81,9 @@ class ACMSContextAssembler: return all(scope.matches(context) for scope in scopes) - def _apply_policy(self, policy: Any, raw_context: dict[str, Any]) -> dict[str, Any]: + def _apply_policy( + self, policy: ContextPolicyConfig, raw_context: dict[str, Any] + ) -> dict[str, Any]: """Apply a policy to the raw context. Args: