diff --git a/features/acms_advanced_strategies_coverage_boost.feature b/features/acms_advanced_strategies_coverage_boost.feature index 4e71ffc5c..a87a4bb26 100644 --- a/features/acms_advanced_strategies_coverage_boost.feature +++ b/features/acms_advanced_strategies_coverage_boost.feature @@ -52,9 +52,9 @@ Feature: ACMS advanced strategies coverage boost Scenario: TemporalArchaeologyStrategy capabilities include temporal and graph support Given a TemporalArchaeologyStrategy instance - Then its capabilities have supports_temporal_archaeology true - And its capabilities have supports_graph_navigation true - And its capabilities have supports_semantic_search false + Then its capabilities have uses_temporal true + And its capabilities have uses_graph true + And its capabilities have uses_vector false # ----------------------------------------------------------------------- # PlanDecisionContextStrategy.capabilities (lines 364-365) @@ -62,6 +62,6 @@ Feature: ACMS advanced strategies coverage boost Scenario: PlanDecisionContextStrategy capabilities include temporal support only Given a PlanDecisionContextStrategy instance - Then its capabilities have supports_temporal_archaeology true - And its capabilities have supports_graph_navigation false - And its capabilities have supports_semantic_search false + Then its capabilities have uses_temporal true + And its capabilities have uses_graph false + And its capabilities have uses_vector false diff --git a/features/acms_pipeline.feature b/features/acms_pipeline.feature index 7377a5e7c..930a929a1 100644 --- a/features/acms_pipeline.feature +++ b/features/acms_pipeline.feature @@ -514,11 +514,11 @@ Feature: ACMS v1 Context Assembly Pipeline Scenario: Each built-in strategy declares distinct capabilities Given the ACMS pipeline modules are available When I inspect the capabilities of "relevance" - Then the strategy should declare supports_semantic_search as true + Then the strategy should declare uses_vector as true When I inspect the capabilities of "recency" - Then the strategy should declare supports_temporal_archaeology as true + Then the strategy should declare uses_temporal as true When I inspect the capabilities of "tiered" - Then the strategy should declare supports_semantic_search as false + Then the strategy should declare uses_vector as false @acms_strategy @introspection Scenario: Strategy confidence ranking determines selector priority diff --git a/features/acms_strategy_capabilities_regression.feature b/features/acms_strategy_capabilities_regression.feature new file mode 100644 index 000000000..d52a10778 --- /dev/null +++ b/features/acms_strategy_capabilities_regression.feature @@ -0,0 +1,119 @@ +@acms @strategy @regression +Feature: ACMS StrategyCapabilities consolidation regression tests + As a developer maintaining the ACMS pipeline + I want to verify that all 6 built-in strategies use the consolidated + domain-model StrategyCapabilities with spec-compliant fields + So that validate_registry() produces no warnings for any built-in strategy + + # Regression tests for issue #9161: + # Consolidate StrategyCapabilities to single domain model with spec-compliant fields. + # All 6 built-in strategies must declare: + # - At least one backend capability (uses_text, uses_vector, uses_graph, uses_temporal) + # - Non-empty resource_types (AC#7 from issue #9161) + + # --------------------------------------------------------------------------- + # validate_registry() — all 6 built-in strategies produce no warnings + # --------------------------------------------------------------------------- + + @strategy_validation + Scenario: validate_registry produces no warnings when all 6 built-in strategies are registered + Given a registry with all built-in strategies + When I validate the registry + Then there should be no validation warnings + + # --------------------------------------------------------------------------- + # Individual strategy: resource_types must be non-empty (AC#7) + # --------------------------------------------------------------------------- + + @strategy_validation + Scenario: simple-keyword strategy declares non-empty resource_types + Given all built-in strategies are instantiated + Then the strategy "simple-keyword" should declare non-empty resource_types + + @strategy_validation + Scenario: semantic-embedding strategy declares non-empty resource_types + Given all built-in strategies are instantiated + Then the strategy "semantic-embedding" should declare non-empty resource_types + + @strategy_validation + Scenario: breadth-depth-navigator strategy declares non-empty resource_types + Given all built-in strategies are instantiated + Then the strategy "breadth-depth-navigator" should declare non-empty resource_types + + @strategy_validation + Scenario: arce strategy declares non-empty resource_types + Given all built-in strategies are instantiated + Then the strategy "arce" should declare non-empty resource_types + + @strategy_validation + Scenario: temporal-archaeology strategy declares non-empty resource_types + Given all built-in strategies are instantiated + Then the strategy "temporal-archaeology" should declare non-empty resource_types + + @strategy_validation + Scenario: plan-decision-context strategy declares non-empty resource_types + Given all built-in strategies are instantiated + Then the strategy "plan-decision-context" should declare non-empty resource_types + + # --------------------------------------------------------------------------- + # Individual strategy: at least one backend capability declared + # --------------------------------------------------------------------------- + + @strategy_validation + Scenario: simple-keyword strategy declares at least one backend capability + Given all built-in strategies are instantiated + Then the strategy "simple-keyword" should declare at least one backend capability + + @strategy_validation + Scenario: semantic-embedding strategy declares at least one backend capability + Given all built-in strategies are instantiated + Then the strategy "semantic-embedding" should declare at least one backend capability + + @strategy_validation + Scenario: breadth-depth-navigator strategy declares at least one backend capability + Given all built-in strategies are instantiated + Then the strategy "breadth-depth-navigator" should declare at least one backend capability + + @strategy_validation + Scenario: arce strategy declares at least one backend capability + Given all built-in strategies are instantiated + Then the strategy "arce" should declare at least one backend capability + + @strategy_validation + Scenario: temporal-archaeology strategy declares at least one backend capability + Given all built-in strategies are instantiated + Then the strategy "temporal-archaeology" should declare at least one backend capability + + @strategy_validation + Scenario: plan-decision-context strategy declares at least one backend capability + Given all built-in strategies are instantiated + Then the strategy "plan-decision-context" should declare at least one backend capability + + # --------------------------------------------------------------------------- + # StrategyCapabilities uses spec-compliant field names (not legacy names) + # --------------------------------------------------------------------------- + + @strategy_validation + Scenario: StrategyCapabilities uses spec-compliant field names + Given all built-in strategies are instantiated + Then every strategy capabilities should use spec-compliant field names + + # --------------------------------------------------------------------------- + # Pipeline strategies (RelevanceStrategy, RecencyStrategy, TieredStrategy) + # also declare non-empty resource_types (AC#7) + # --------------------------------------------------------------------------- + + @strategy_validation + Scenario: RelevanceStrategy declares non-empty resource_types + Given the ACMS pipeline built-in strategies are instantiated + Then the pipeline strategy "relevance" should declare non-empty resource_types + + @strategy_validation + Scenario: RecencyStrategy declares non-empty resource_types + Given the ACMS pipeline built-in strategies are instantiated + Then the pipeline strategy "recency" should declare non-empty resource_types + + @strategy_validation + Scenario: TieredStrategy declares non-empty resource_types + Given the ACMS pipeline built-in strategies are instantiated + Then the pipeline strategy "tiered" should declare non-empty resource_types diff --git a/features/steps/acms_advanced_strategies_coverage_boost_steps.py b/features/steps/acms_advanced_strategies_coverage_boost_steps.py index 1a94a57bf..01f2c5d30 100644 --- a/features/steps/acms_advanced_strategies_coverage_boost_steps.py +++ b/features/steps/acms_advanced_strategies_coverage_boost_steps.py @@ -275,19 +275,19 @@ def step_temporal_instance(context: Context) -> None: context.strategy = TemporalArchaeologyStrategy() -@then("its capabilities have supports_temporal_archaeology true") +@then("its capabilities have uses_temporal true") def step_cap_temporal(context: Context) -> None: - assert context.strategy.capabilities.supports_temporal_archaeology is True + assert context.strategy.capabilities.uses_temporal is True -@then("its capabilities have supports_graph_navigation true") +@then("its capabilities have uses_graph true") def step_cap_graph_true(context: Context) -> None: - assert context.strategy.capabilities.supports_graph_navigation is True + assert context.strategy.capabilities.uses_graph is True -@then("its capabilities have supports_semantic_search false") -def step_cap_semantic_false(context: Context) -> None: - assert context.strategy.capabilities.supports_semantic_search is False +@then("its capabilities have uses_vector false") +def step_cap_vector_false(context: Context) -> None: + assert context.strategy.capabilities.uses_vector is False # =================================================================== @@ -300,6 +300,6 @@ def step_plan_decision_instance(context: Context) -> None: context.strategy = PlanDecisionContextStrategy() -@then("its capabilities have supports_graph_navigation false") +@then("its capabilities have uses_graph false") def step_cap_graph_false(context: Context) -> None: - assert context.strategy.capabilities.supports_graph_navigation is False + assert context.strategy.capabilities.uses_graph is False diff --git a/features/steps/acms_pipeline_phase3_steps.py b/features/steps/acms_pipeline_phase3_steps.py index f2259a436..195b3c361 100644 --- a/features/steps/acms_pipeline_phase3_steps.py +++ b/features/steps/acms_pipeline_phase3_steps.py @@ -364,7 +364,7 @@ def step_arce_name(context: Context, expected: str) -> None: @then("the ArceStrategy should support semantic search") def step_arce_supports_semantic(context: Context) -> None: - assert context.arce_strategy.capabilities.supports_semantic_search + assert context.arce_strategy.capabilities.uses_vector @then('the ArceStrategy explain should contain "{text}"') diff --git a/features/steps/acms_strategy_capabilities_regression_steps.py b/features/steps/acms_strategy_capabilities_regression_steps.py new file mode 100644 index 000000000..79e8fa86f --- /dev/null +++ b/features/steps/acms_strategy_capabilities_regression_steps.py @@ -0,0 +1,176 @@ +"""Step definitions for acms_strategy_capabilities_regression.feature. + +Regression tests for issue #9161: Consolidate StrategyCapabilities to single +domain model with spec-compliant fields. + +Verifies that: +- All 6 built-in strategies (from strategy_stubs.py) declare non-empty + resource_types (AC#7) and at least one backend capability. +- validate_registry() produces no warnings for any built-in strategy. +- StrategyCapabilities uses spec-compliant field names (uses_text, uses_vector, + uses_graph, uses_temporal) rather than legacy names. +- Pipeline strategies (RelevanceStrategy, RecencyStrategy, TieredStrategy) + also declare non-empty resource_types. +""" + +from __future__ import annotations + +from behave import given, then +from behave.runner import Context + +from cleveragents.application.services.acms_service import ( + RelevanceStrategy, + RecencyStrategy, + TieredStrategy, +) +from cleveragents.application.services.strategy_registry import ( + StrategyRegistry, +) +from cleveragents.domain.models.acms.strategy import ( + StrategyCapabilities, + StrategyConfig, +) +from cleveragents.domain.models.acms.strategy_stubs import ( + BUILTIN_STRATEGY_CLASSES, +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _all_builtins() -> dict[str, object]: + """Instantiate all 6 built-in strategies keyed by name.""" + instances: dict[str, object] = {} + for cls in BUILTIN_STRATEGY_CLASSES: + inst = cls() + instances[inst.name] = inst + return instances + + +def _register_all_builtins(registry: StrategyRegistry) -> None: + """Register all 6 built-in strategies in the registry.""" + for cls in BUILTIN_STRATEGY_CLASSES: + inst = cls() + registry.register( + inst, + config=StrategyConfig(enabled=True), + is_builtin=True, + ) + + +# --------------------------------------------------------------------------- +# Given +# --------------------------------------------------------------------------- + + +@given("the ACMS pipeline built-in strategies are instantiated") +def step_given_pipeline_builtins(context: Context) -> None: + """Instantiate the 3 pipeline built-in strategies.""" + context.pipeline_strategies = { + "relevance": RelevanceStrategy(), + "recency": RecencyStrategy(), + "tiered": TieredStrategy(), + } + + +# --------------------------------------------------------------------------- +# Then +# --------------------------------------------------------------------------- + + +@then('the strategy "{name}" should declare non-empty resource_types') +def step_then_nonempty_resource_types(context: Context, name: str) -> None: + """Verify that the named strategy declares non-empty resource_types.""" + strategy = context.strategies[name] + caps = strategy.capabilities + assert isinstance(caps, StrategyCapabilities), ( + f"Strategy '{name}' capabilities is not StrategyCapabilities, " + f"got {type(caps).__name__}" + ) + assert caps.resource_types, ( + f"Strategy '{name}' has empty resource_types — violates AC#7 of issue #9161. " + f"All built-in strategies must declare at least one supported resource type." + ) + + +@then('the strategy "{name}" should declare at least one backend capability') +def step_then_has_backend_capability(context: Context, name: str) -> None: + """Verify that the named strategy declares at least one backend capability.""" + strategy = context.strategies[name] + caps = strategy.capabilities + assert isinstance(caps, StrategyCapabilities), ( + f"Strategy '{name}' capabilities is not StrategyCapabilities" + ) + has_any = ( + caps.uses_text or caps.uses_vector or caps.uses_graph or caps.uses_temporal + ) + assert has_any, ( + f"Strategy '{name}' declares no backend capabilities " + f"(uses_text={caps.uses_text}, uses_vector={caps.uses_vector}, " + f"uses_graph={caps.uses_graph}, uses_temporal={caps.uses_temporal}). " + f"Every strategy must declare at least one backend capability." + ) + + +@then("every strategy capabilities should use spec-compliant field names") +def step_then_spec_compliant_fields(context: Context) -> None: + """Verify that StrategyCapabilities uses spec-compliant field names. + + The consolidated domain model uses: + - uses_text (not supports_semantic_search) + - uses_vector (not supports_semantic_search) + - uses_graph (not supports_graph_navigation) + - uses_temporal (not supports_temporal_archaeology) + + This test verifies that the legacy field names are NOT present on the + StrategyCapabilities model, confirming the consolidation is complete. + """ + for name, strategy in context.strategies.items(): + caps = strategy.capabilities + assert isinstance(caps, StrategyCapabilities), ( + f"Strategy '{name}' capabilities is not StrategyCapabilities" + ) + # Verify spec-compliant fields exist + assert hasattr(caps, "uses_text"), ( + f"Strategy '{name}' StrategyCapabilities missing 'uses_text' field" + ) + assert hasattr(caps, "uses_vector"), ( + f"Strategy '{name}' StrategyCapabilities missing 'uses_vector' field" + ) + assert hasattr(caps, "uses_graph"), ( + f"Strategy '{name}' StrategyCapabilities missing 'uses_graph' field" + ) + assert hasattr(caps, "uses_temporal"), ( + f"Strategy '{name}' StrategyCapabilities missing 'uses_temporal' field" + ) + # Verify legacy field names are NOT present + assert not hasattr(caps, "supports_semantic_search"), ( + f"Strategy '{name}' StrategyCapabilities still has legacy " + f"'supports_semantic_search' field — consolidation incomplete" + ) + assert not hasattr(caps, "supports_graph_navigation"), ( + f"Strategy '{name}' StrategyCapabilities still has legacy " + f"'supports_graph_navigation' field — consolidation incomplete" + ) + assert not hasattr(caps, "supports_temporal_archaeology"), ( + f"Strategy '{name}' StrategyCapabilities still has legacy " + f"'supports_temporal_archaeology' field — consolidation incomplete" + ) + + +@then('the pipeline strategy "{name}" should declare non-empty resource_types') +def step_then_pipeline_nonempty_resource_types(context: Context, name: str) -> None: + """Verify that the named pipeline strategy declares non-empty resource_types.""" + strategy = context.pipeline_strategies[name] + caps = strategy.capabilities + assert isinstance(caps, StrategyCapabilities), ( + f"Pipeline strategy '{name}' capabilities is not StrategyCapabilities, " + f"got {type(caps).__name__}" + ) + assert caps.resource_types, ( + f"Pipeline strategy '{name}' has empty resource_types — violates AC#7 of " + f"issue #9161. All pipeline strategies must declare at least one supported " + f"resource type." + ) diff --git a/features/steps/context_strategies_steps.py b/features/steps/context_strategies_steps.py index 9e4c263bb..226da338c 100644 --- a/features/steps/context_strategies_steps.py +++ b/features/steps/context_strategies_steps.py @@ -239,8 +239,8 @@ def step_strategy_confidence(context: Context, expected: float) -> None: @then("the SimpleKeywordStrategy should not support semantic search") def step_simple_keyword_no_semantic(context: Context) -> None: caps = context.strategy.capabilities - assert not caps.supports_semantic_search, ( - "SimpleKeywordStrategy should NOT support semantic search" + assert not caps.uses_vector, ( + "SimpleKeywordStrategy should NOT support vector/semantic search" ) @@ -253,8 +253,8 @@ def step_simple_keyword_name(context: Context, name: str) -> None: @then("the SemanticEmbeddingStrategy should support semantic search") def step_semantic_supports_search(context: Context) -> None: caps = context.strategy.capabilities - assert caps.supports_semantic_search, ( - "SemanticEmbeddingStrategy should support semantic search" + assert caps.uses_vector, ( + "SemanticEmbeddingStrategy should support vector/semantic search" ) @@ -267,7 +267,7 @@ def step_semantic_name(context: Context, name: str) -> None: @then("the BreadthDepthNavigatorStrategy should support graph navigation") def step_breadth_depth_supports_graph(context: Context) -> None: caps = context.strategy.capabilities - assert caps.supports_graph_navigation, ( + assert caps.uses_graph, ( "BreadthDepthNavigatorStrategy should support graph navigation" ) diff --git a/robot/helper_acms_pipeline_phase3.py b/robot/helper_acms_pipeline_phase3.py index 5122961bf..f284881f4 100644 --- a/robot/helper_acms_pipeline_phase3.py +++ b/robot/helper_acms_pipeline_phase3.py @@ -122,7 +122,7 @@ def _test_arce_strategy() -> None: strategy = ArceStrategy(max_iterations=3) assert strategy.name == "arce" assert strategy.can_handle({}) == 0.95 - assert strategy.capabilities.supports_semantic_search + assert strategy.capabilities.uses_vector frags = [ _frag("project://app/io.py", "async IO handler", 0.7, 20, 5), diff --git a/robot/helper_context_strategies.py b/robot/helper_context_strategies.py index 79383bcb8..3a8c006cb 100644 --- a/robot/helper_context_strategies.py +++ b/robot/helper_context_strategies.py @@ -229,13 +229,13 @@ def _cmd_capabilities() -> int: se = SemanticEmbeddingStrategy() bd = BreadthDepthNavigatorStrategy() - if sk.capabilities.supports_semantic_search: - print("FAIL: SimpleKeyword should not support semantic search") + if sk.capabilities.uses_vector: + print("FAIL: SimpleKeyword should not support vector/semantic search") return 1 - if not se.capabilities.supports_semantic_search: - print("FAIL: SemanticEmbedding should support semantic search") + if not se.capabilities.uses_vector: + print("FAIL: SemanticEmbedding should support vector/semantic search") return 1 - if not bd.capabilities.supports_graph_navigation: + if not bd.capabilities.uses_graph: print("FAIL: BreadthDepth should support graph navigation") return 1 if sk.name != "simple-keyword":