From 1521c4ae8c781725bac4782a641326d50f375959 Mon Sep 17 00:00:00 2001 From: khyari hamza Date: Wed, 4 Mar 2026 19:31:16 +0000 Subject: [PATCH 1/2] feat(acms): add context strategy registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement the ACMS context strategy registry per spec §25162-25233, §28682-28708, §42628-42653, and §43167-43199. - Define ContextStrategy protocol, StrategyCapabilities, BackendSet, PlanContext, StrategyConfig, ContextStrategyResult models - Add 6 built-in stub strategies (simple-keyword, semantic-embedding, breadth-depth-navigator, arce, temporal-archaeology, plan-decision-context) with spec quality scores and feature flags - Add StrategyRegistry with register, register_from_module (plugin discovery), enable/disable, per-strategy config, and validation - Add ContextStrategyResult with deterministic fragment ordering (-relevance_score, uko_node) - Add configuration-driven enabled list with per-project overrides - Add per-strategy timeout, max-fragment, circuit-breaker config - Add registry validation for resource types and backend capabilities - Fix update_config to sync _enabled_order when toggling enabled flag - Fix register_from_module to honour the name parameter as registry key - Fix update_config to re-run Pydantic validators via model_validate - Add docs/reference/context_strategies.md - Add 55 BDD scenarios (Behave), 4 Robot integration tests, ASV benchmarks ISSUES CLOSED: #191 --- CHANGELOG.md | 10 + benchmarks/context_strategy_bench.py | 116 +++ docs/reference/context_strategies.md | 130 +++ features/context_strategy_registry.feature | 454 ++++++++++ .../steps/context_strategy_registry_steps.py | 809 ++++++++++++++++++ robot/context_strategy_registry.robot | 35 + robot/helper_context_strategy_registry.py | 153 ++++ .../application/services/__init__.py | 8 + .../application/services/strategy_registry.py | 442 ++++++++++ .../domain/models/acms/__init__.py | 47 + .../domain/models/acms/strategy.py | 343 ++++++++ .../domain/models/acms/strategy_stubs.py | 394 +++++++++ vulture_whitelist.py | 3 + 13 files changed, 2944 insertions(+) create mode 100644 benchmarks/context_strategy_bench.py create mode 100644 docs/reference/context_strategies.md create mode 100644 features/context_strategy_registry.feature create mode 100644 features/steps/context_strategy_registry_steps.py create mode 100644 robot/context_strategy_registry.robot create mode 100644 robot/helper_context_strategy_registry.py create mode 100644 src/cleveragents/application/services/strategy_registry.py create mode 100644 src/cleveragents/domain/models/acms/strategy.py create mode 100644 src/cleveragents/domain/models/acms/strategy_stubs.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 2473488ee..321baea9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,16 @@ Integrates with all 8 built-in automation profiles. DI-wired as singleton `autonomy_controller`. Includes 48 Behave BDD scenarios, 10 Robot Framework integration tests, and ASV benchmarks. (#546) +- Added context strategy registry with `ContextStrategy` protocol, + `StrategyCapabilities`, `BackendSet`, `PlanContext`, `StrategyConfig`, + `ContextStrategyResult`, and `StrategyRegistryEntry` domain models. + Six built-in stub strategies (simple-keyword, semantic-embedding, + breadth-depth-navigator, arce, temporal-archaeology, plan-decision-context) + with spec-mandated quality scores and backend requirements. + `StrategyRegistry` supports config-driven registration, per-strategy + timeout/max-fragment limits, per-project enable/disable overrides, + plugin discovery from `"module:ClassName"` strings, and validation + that strategies declare supported resource types. (#191) - Fixed `context inspect` to display project-scoped tier fragment counts instead of global counts. Added `ContextTierService.get_scoped_metrics()` which returns fragment population counts filtered to the target project while keeping hit/miss counters as global service diff --git a/benchmarks/context_strategy_bench.py b/benchmarks/context_strategy_bench.py new file mode 100644 index 000000000..e5bd23bfd --- /dev/null +++ b/benchmarks/context_strategy_bench.py @@ -0,0 +1,116 @@ +"""ASV benchmarks for context strategy registry. + +Measures registry lookup, registration, and can_handle performance +against spec targets (can_handle < 20ms per strategy, +specification.md:43245-43261). +""" + +from __future__ import annotations + +import importlib +import sys +from pathlib import Path + +# Ensure src is on sys.path for ASV +_SRC = Path(__file__).resolve().parents[1] / "src" +if str(_SRC) not in sys.path: + sys.path.insert(0, str(_SRC)) + importlib.invalidate_caches() + import cleveragents + + importlib.reload(cleveragents) + +from cleveragents.application.services.strategy_registry import ( # noqa: E402 + StrategyRegistry, +) +from cleveragents.domain.models.acms.crp import ContextRequest # noqa: E402 +from cleveragents.domain.models.acms.strategy import ( # noqa: E402 + BackendSet, + StrategyConfig, +) +from cleveragents.domain.models.acms.strategy_stubs import ( # noqa: E402 + BUILTIN_STRATEGY_CLASSES, + DEFAULT_ENABLED_STRATEGIES, + SimpleKeywordStrategy, +) +from cleveragents.domain.models.acms.stubs import ( # noqa: E402 + InMemoryGraphBackend, + InMemoryTextBackend, + InMemoryVectorBackend, +) + + +class TimeRegistration: + """Benchmark strategy registration performance.""" + + timeout = 10 + repeat = 3 + number = 100 + + def time_register_single(self) -> None: + """Register a single strategy.""" + registry = StrategyRegistry() + registry.register(SimpleKeywordStrategy(), is_builtin=True) + + def time_register_all_builtins(self) -> None: + """Register all 6 built-in strategies.""" + registry = StrategyRegistry() + for cls in BUILTIN_STRATEGY_CLASSES: + inst = cls() + registry.register(inst, is_builtin=True) + + +class TimeLookup: + """Benchmark registry lookup performance.""" + + timeout = 10 + repeat = 3 + number = 1000 + + def setup(self) -> None: + self.registry = StrategyRegistry() + for cls in BUILTIN_STRATEGY_CLASSES: + inst = cls() + enabled = inst.name in DEFAULT_ENABLED_STRATEGIES + self.registry.register( + inst, + config=StrategyConfig(enabled=enabled), + is_builtin=True, + ) + + def time_get_by_name(self) -> None: + """Look up a strategy by name.""" + self.registry.get("simple-keyword") + + def time_list_enabled(self) -> None: + """List enabled strategies.""" + self.registry.list_enabled() + + def time_list_all(self) -> None: + """List all strategies.""" + self.registry.list_all() + + +class TimeCanHandle: + """Benchmark can_handle performance (spec target: < 20ms). + + Spec: specification.md:43245-43261. + """ + + timeout = 10 + repeat = 3 + number = 1000 + + def setup(self) -> None: + self.request = ContextRequest(query="test query") + self.backends_all = BackendSet( + text=InMemoryTextBackend(), + vector=InMemoryVectorBackend(), + graph=InMemoryGraphBackend(), + ) + self.strategies = [cls() for cls in BUILTIN_STRATEGY_CLASSES] + + def time_can_handle_all_strategies(self) -> None: + """Poll can_handle on all 6 built-in strategies.""" + for strategy in self.strategies: + strategy.can_handle(self.request, self.backends_all) diff --git a/docs/reference/context_strategies.md b/docs/reference/context_strategies.md new file mode 100644 index 000000000..2f810d44a --- /dev/null +++ b/docs/reference/context_strategies.md @@ -0,0 +1,130 @@ +# Context Strategy Registry + +The Context Strategy Registry manages pluggable retrieval strategies for the +ACMS Context Assembly Pipeline. Strategies implement the `ContextStrategy` +protocol and are registered via TOML configuration or programmatic API. + +## Architecture + +``` +ContextRequest + ┌─── StrategySelector ───┐ + │ can_handle() polling │ + └──────────┬──────────────┘ + v + ┌─── BudgetAllocator ────┐ + │ proportional alloc │ + └──────────┬──────────────┘ + v + ┌─── StrategyExecutor ───┐ + │ parallel w/ timeout │ + │ circuit breaker │ + └──────────┬──────────────┘ + v + ContextFragment[] +``` + +## ContextStrategy Protocol + +Every strategy implements: + +| Member | Type | Description | +|--------|------|-------------| +| `name` | `property -> str` | Unique strategy name | +| `capabilities` | `property -> StrategyCapabilities` | Backend requirements and quality score | +| `can_handle(request, backends)` | `-> float` | 0.0-1.0 confidence for this request | +| `assemble(request, backends, budget, plan_context)` | `-> list[ContextFragment]` | Execute and return fragments; must respect budget | +| `explain()` | `-> str` | Human-readable description | + +## StrategyCapabilities + +| Field | Type | Default | Description | +|-------|------|---------|-------------| +| `uses_text` | `bool` | `False` | Requires TextBackend | +| `uses_vector` | `bool` | `False` | Requires VectorBackend | +| `uses_graph` | `bool` | `False` | Requires GraphBackend | +| `uses_temporal` | `bool` | `False` | Requires temporal/cold-tier data | +| `resource_types` | `tuple[str, ...]` | `()` | Supported resource types (empty = all) | +| `quality_score` | `float` | `0.5` | Intrinsic quality score (0.0-1.0) | +| `supports_depth_breadth` | `bool` | `False` | Supports depth/breadth projection | +| `supports_plan_hierarchy` | `bool` | `False` | Supports plan hierarchy traversal | +| `supports_temporal` | `bool` | `False` | Supports temporal archaeology queries | + +## Built-in Strategies + +| Strategy | Quality | Backends | Description | +|----------|---------|----------|-------------| +| `simple-keyword` | 0.3 | Text | Basic keyword/regex text search. Universal fallback. | +| `semantic-embedding` | 0.6 | Vector | Vector similarity search. | +| `breadth-depth-navigator` | 0.85 | Graph | Graph-aware UKO traversal. Primary code-aware strategy. | +| `arce` | 0.95 | All | Multi-modal ARCE pipeline. Highest quality. | +| `temporal-archaeology` | 0.5 | Graph + Cold | Historical pattern discovery. | +| `plan-decision-context` | 0.7 | Warm/Cold | Parent/ancestor plan context retrieval. | + +Default enabled (spec `context.strategies.enabled`): +`["simple-keyword", "semantic-embedding", "breadth-depth-navigator"]` + +## Configuration + +### Global (config.toml) + +```toml +[context.strategies] +enabled = ["simple-keyword", "semantic-embedding", "breadth-depth-navigator"] + +[context.strategies.custom] +"my-strategy" = "my_package.strategies:MyStrategy" +``` + +### Per-strategy settings + +| Key | Type | Default | Description | +|-----|------|---------|-------------| +| `timeout_seconds` | int | 30 | Per-strategy assembly timeout | +| `max_fragments` | int | 100 | Maximum fragments per call | +| `max_workers` | int | 4 | Parallel workers for executor | +| `circuit_breaker_threshold` | int | 3 | Failures before circuit opens | +| `enabled` | bool | `true` | Whether the strategy is active | + +### Per-project overrides + +Projects can override the global enabled list using: + +``` +agents project context set --strategy simple-keyword --strategy semantic-embedding +``` + +This replaces the global `context.strategies.enabled` list for that project. + +## ContextStrategyResult + +Output of a single strategy execution: + +| Field | Type | Description | +|-------|------|-------------| +| `strategy_name` | `str` | Name of the producing strategy | +| `fragments` | `tuple[ContextFragment, ...]` | Deterministically ordered fragments | +| `total_fragments` | `int` | Total fragments before limits | +| `tokens_used` | `int` | Total tokens across fragments | +| `execution_time_ms` | `float` | Execution wall-clock time (ms) | +| `errors` | `tuple[str, ...]` | Error messages | +| `stats` | `dict[str, Any]` | Strategy-specific statistics | + +Fragment ordering: `(relevance_score DESC, uko_node ASC)`. + +## Registry Validation + +The registry validates that: + +1. Every enabled strategy is actually registered. +2. Every strategy declares at least one backend capability. +3. Every strategy declares supported resource types. + +## Fallback Degradation Path + +When backends are unavailable, the system degrades gracefully: + +1. `arce` (requires all) → 2. `breadth-depth-navigator` (graph) → +3. `semantic-embedding` (vector) → 4. `simple-keyword` (text only) + +This ensures context is always produced. diff --git a/features/context_strategy_registry.feature b/features/context_strategy_registry.feature new file mode 100644 index 000000000..56ad9ba56 --- /dev/null +++ b/features/context_strategy_registry.feature @@ -0,0 +1,454 @@ +@phase2 @acms @strategy +Feature: Context Strategy Registry + As a CleverAgents developer + I want to register and configure context strategies + So that the ACMS pipeline can select and execute them + + # --------------------------------------------------------------------------- + # Strategy protocol conformance + # --------------------------------------------------------------------------- + + @strategy_protocol + Scenario: All built-in strategies satisfy the ContextStrategy protocol + Given all built-in strategies are instantiated + Then every strategy should satisfy the ContextStrategy protocol + And every strategy should have a non-empty name + And every strategy should have a capabilities object + + @strategy_protocol + Scenario: Built-in strategy quality scores match specification + Given all built-in strategies are instantiated + Then the strategy "simple-keyword" should have quality score 0.3 + And the strategy "semantic-embedding" should have quality score 0.6 + And the strategy "breadth-depth-navigator" should have quality score 0.85 + And the strategy "arce" should have quality score 0.95 + And the strategy "temporal-archaeology" should have quality score 0.5 + And the strategy "plan-decision-context" should have quality score 0.7 + + # --------------------------------------------------------------------------- + # Registry registration + # --------------------------------------------------------------------------- + + @strategy_registry + Scenario: Register a single strategy + Given an empty strategy registry + When I register the "simple-keyword" built-in strategy + Then the registry should contain "simple-keyword" + And the registry size should be 1 + + @strategy_registry + Scenario: Register all built-in strategies + Given an empty strategy registry + When I register all 6 built-in strategies + Then the registry should contain 6 strategies + And the registry should contain "simple-keyword" + And the registry should contain "semantic-embedding" + And the registry should contain "breadth-depth-navigator" + And the registry should contain "arce" + And the registry should contain "temporal-archaeology" + And the registry should contain "plan-decision-context" + + @strategy_registry @error_handling + Scenario: Reject duplicate strategy registration + Given an empty strategy registry + And I register the "simple-keyword" built-in strategy + When I attempt to register "simple-keyword" again + Then a StrategyRegistrationError should be raised + + @strategy_registry @error_handling + Scenario: Reject strategy with empty name + Given an empty strategy registry + When I attempt to register a strategy with an empty name + Then a StrategyRegistrationError should be raised + + # --------------------------------------------------------------------------- + # Registry query + # --------------------------------------------------------------------------- + + @strategy_query + Scenario: Get a registered strategy by name + Given a registry with all built-in strategies + When I get the strategy "simple-keyword" + Then the strategy name should be "simple-keyword" + + @strategy_query @error_handling + Scenario: Get a non-existent strategy raises error + Given a registry with all built-in strategies + When I attempt to get the strategy "nonexistent" + Then a StrategyNotFoundError should be raised + + @strategy_query + Scenario: List all registered strategy names + Given a registry with all built-in strategies + Then the registry should list 6 strategies + And the list should be sorted alphabetically + + # --------------------------------------------------------------------------- + # Enable / disable + # --------------------------------------------------------------------------- + + @strategy_config + Scenario: Default enabled list matches spec + Given a registry with all built-in strategies using default enabled list + Then the enabled strategies should be "simple-keyword", "semantic-embedding", "breadth-depth-navigator" + + @strategy_config + Scenario: Override enabled list per project + Given a registry with all built-in strategies using default enabled list + When I set the enabled list to "simple-keyword", "arce" + Then the enabled strategies should be "simple-keyword", "arce" + + @strategy_config @error_handling + Scenario: Set enabled list with unknown strategy raises error + Given a registry with all built-in strategies using default enabled list + When I attempt to set the enabled list to "nonexistent" + Then a StrategyNotFoundError should be raised + + @strategy_config + Scenario: Disabled strategy excluded from enabled list + Given a registry with all built-in strategies using default enabled list + When I disable the strategy "semantic-embedding" + Then the enabled strategies should not include "semantic-embedding" + + # --------------------------------------------------------------------------- + # Per-strategy configuration + # --------------------------------------------------------------------------- + + @strategy_config + Scenario: Default strategy config values + Given a registry with all built-in strategies + When I get the config for "simple-keyword" + Then the timeout_seconds should be 30 + And the max_fragments should be 100 + And the circuit_breaker_threshold should be 3 + + @strategy_config + Scenario: Update per-strategy timeout + Given a registry with all built-in strategies + When I update config for "simple-keyword" with timeout_seconds 10 + Then the config for "simple-keyword" should have timeout_seconds 10 + + # --------------------------------------------------------------------------- + # can_handle with backends + # --------------------------------------------------------------------------- + + @strategy_can_handle + Scenario: simple-keyword returns quality score with text backend + Given a BackendSet with text backend only + And a default ContextRequest + Then "simple-keyword" can_handle should return 0.3 + + @strategy_can_handle + Scenario: simple-keyword returns 0 without text backend + Given a BackendSet with no backends + And a default ContextRequest + Then "simple-keyword" can_handle should return 0.0 + + @strategy_can_handle + Scenario: semantic-embedding returns quality score with vector backend + Given a BackendSet with vector backend only + And a default ContextRequest + Then "semantic-embedding" can_handle should return 0.6 + + @strategy_can_handle + Scenario: arce requires all backends + Given a BackendSet with all backends + And a default ContextRequest + Then "arce" can_handle should return 0.95 + + @strategy_can_handle + Scenario: arce returns 0 when missing a backend + Given a BackendSet with text backend only + And a default ContextRequest + Then "arce" can_handle should return 0.0 + + @strategy_can_handle + Scenario: plan-decision-context always returns quality score + Given a BackendSet with no backends + And a default ContextRequest + Then "plan-decision-context" can_handle should return 0.7 + + # --------------------------------------------------------------------------- + # Stub assemble (no-op) + # --------------------------------------------------------------------------- + + @strategy_assemble + Scenario: Stub strategies return empty fragment lists + Given all built-in strategies are instantiated + And a BackendSet with all backends + And a default ContextRequest + And a default PlanContext + When each strategy assembles with budget 1000 + Then every strategy should return an empty fragment list + + # --------------------------------------------------------------------------- + # ContextStrategyResult deterministic ordering + # --------------------------------------------------------------------------- + + @strategy_result + Scenario: ContextStrategyResult sorts fragments deterministically + Given a ContextStrategyResult with unordered fragments + Then the fragments should be sorted by relevance_score DESC then uko_node ASC + + @strategy_result + Scenario: ContextStrategyResult with no fragments + Given a ContextStrategyResult with no fragments + Then the total_fragments should be 0 + And the tokens_used should be 0 + + # --------------------------------------------------------------------------- + # Registry validation + # --------------------------------------------------------------------------- + + @strategy_validation + Scenario: Validate registry warns about missing resource types + Given a registry with a strategy that declares no resource types + When I validate the registry + Then validation warnings should mention "resource types" + + @strategy_validation + Scenario: Validate registry warns about missing capabilities + Given a registry with a strategy that declares no backend capabilities + When I validate the registry + Then validation warnings should mention "backend capabilities" + + @strategy_validation + Scenario: Valid registry returns no warnings for built-in strategies + Given a registry with all built-in strategies + When I validate the registry + Then there should be no validation warnings + + # --------------------------------------------------------------------------- + # Unregister + # --------------------------------------------------------------------------- + + # --------------------------------------------------------------------------- + # Plugin discovery (register_from_module) + # --------------------------------------------------------------------------- + + @strategy_registry @plugin + Scenario: Register a strategy from a module path + Given an empty strategy registry + When I register a strategy from module "cleveragents.domain.models.acms.strategy_stubs:SimpleKeywordStrategy" + Then the registry should contain "simple-keyword" + And the registry size should be 1 + + @strategy_registry @plugin @error_handling + Scenario: Reject module path without colon separator + Given an empty strategy registry + When I attempt to register a strategy from module "no_colon_here" + Then a StrategyRegistrationError should be raised + + @strategy_registry @plugin @error_handling + Scenario: Reject module path with missing module + Given an empty strategy registry + When I attempt to register a strategy from module "nonexistent.module:FakeClass" + Then a StrategyRegistrationError should be raised + + @strategy_registry @plugin @error_handling + Scenario: Reject module path with missing class + Given an empty strategy registry + When I attempt to register a strategy from module "cleveragents.domain.models.acms.strategy_stubs:NonExistentClass" + Then a StrategyRegistrationError should be raised + + @strategy_registry @plugin @error_handling + Scenario: Reject module path when instantiation fails + Given an empty strategy registry + When I attempt to register a strategy from module "cleveragents.domain.models.acms.strategy_stubs:BUILTIN_STRATEGY_CLASSES" + Then a StrategyRegistrationError should be raised + + # --------------------------------------------------------------------------- + # Additional query coverage + # --------------------------------------------------------------------------- + + @strategy_query + Scenario: Get entry for a registered strategy + Given a registry with all built-in strategies + When I get the entry for "simple-keyword" + Then the entry name should be "simple-keyword" + And the entry should be marked as builtin + + @strategy_query @error_handling + Scenario: Get entry for non-existent strategy raises error + Given a registry with all built-in strategies + When I attempt to get the entry for "nonexistent" + Then a StrategyNotFoundError should be raised + + @strategy_query + Scenario: List built-in strategies + Given a registry with all built-in strategies + Then the builtin list should contain 6 strategies + + @strategy_query + Scenario: Registry supports the in operator + Given a registry with all built-in strategies + Then "simple-keyword" should be in the registry via contains + And "nonexistent" should not be in the registry via contains + + # --------------------------------------------------------------------------- + # Validation edge case: enabled-but-unregistered + # --------------------------------------------------------------------------- + + @strategy_validation + Scenario: Validate warns when enabled list references unregistered strategy + Given a registry with a stale enabled list entry + When I validate the registry + Then validation warnings should mention "not registered" + + # --------------------------------------------------------------------------- + # Unregister error path + # --------------------------------------------------------------------------- + + @strategy_registry @error_handling + Scenario: Unregister non-existent strategy raises error + Given a registry with all built-in strategies + When I attempt to unregister "nonexistent" + Then a StrategyNotFoundError should be raised + + # --------------------------------------------------------------------------- + # Unregister + # --------------------------------------------------------------------------- + + @strategy_registry + Scenario: Unregister a strategy + Given a registry with all built-in strategies + When I unregister "temporal-archaeology" + Then the registry should contain 5 strategies + And the registry should not contain "temporal-archaeology" + + @strategy_registry + Scenario: Clear the registry + Given a registry with all built-in strategies + When I clear the registry + Then the registry should contain 0 strategies + + # --------------------------------------------------------------------------- + # Boundary tests for Pydantic-validated config fields + # --------------------------------------------------------------------------- + + @strategy_config @boundary + Scenario Outline: Reject invalid StrategyConfig numeric values + When I attempt to create a StrategyConfig with set to + Then a Pydantic ValidationError should be raised + + Examples: + | field | value | + | timeout_seconds | 0 | + | timeout_seconds | -1 | + | max_fragments | 0 | + | max_fragments | -1 | + | max_workers | 0 | + | circuit_breaker_threshold | 0 | + + @strategy_config @boundary + Scenario Outline: Reject invalid quality_score values + When I attempt to create a StrategyCapabilities with quality_score + Then a Pydantic ValidationError should be raised + + Examples: + | value | + | -0.1 | + | 1.1 | + + @strategy_config @boundary + Scenario: Accept quality_score at lower bound 0.0 + When I create a StrategyCapabilities with quality_score 0.0 + Then the quality_score should be 0.0 + + @strategy_config @boundary + Scenario: Accept quality_score at upper bound 1.0 + When I create a StrategyCapabilities with quality_score 1.0 + Then the quality_score should be 1.0 + + # --------------------------------------------------------------------------- + # Bug regression tests + # --------------------------------------------------------------------------- + + @strategy_config @regression + Scenario: update_config enabled=True adds strategy to enabled list + Given an empty strategy registry + When I register the "simple-keyword" built-in strategy with enabled False + Then the enabled strategies should be "" + When I update config for "simple-keyword" with enabled True + Then the enabled strategies should include "simple-keyword" + + @strategy_config @regression + Scenario: update_config enabled=False removes strategy from enabled list + Given a registry with all built-in strategies using default enabled list + When I update config for "simple-keyword" with enabled False + Then the enabled strategies should not include "simple-keyword" + + @strategy_registry @plugin @regression + Scenario: register_from_module uses the provided name parameter + Given an empty strategy registry + When I register a strategy named "custom-search" from module "cleveragents.domain.models.acms.strategy_stubs:SimpleKeywordStrategy" + Then the registry should contain "custom-search" + And the registry size should be 1 + + @strategy_config @regression + Scenario: update_config with max_workers and circuit_breaker_threshold + Given a registry with all built-in strategies + When I update config for "simple-keyword" with max_workers 8 and circuit_breaker_threshold 5 + Then the config for "simple-keyword" should have max_workers 8 + And the config for "simple-keyword" should have circuit_breaker_threshold 5 + + @strategy_registry @error_handling + Scenario: Reject object that does not satisfy ContextStrategy protocol + Given an empty strategy registry + When I attempt to register a non-protocol object + Then a StrategyRegistrationError should be raised + + # --------------------------------------------------------------------------- + # explain() coverage + # --------------------------------------------------------------------------- + + @strategy_protocol + Scenario: Every built-in strategy returns a non-empty explain string + Given all built-in strategies are instantiated + Then every strategy should return a non-empty explain string + + # --------------------------------------------------------------------------- + # can_handle coverage for strategies not tested elsewhere + # --------------------------------------------------------------------------- + + @strategy_can_handle + Scenario: breadth-depth-navigator returns quality score with graph backend + Given a BackendSet with graph backend only + And a default ContextRequest + Then "breadth-depth-navigator" can_handle should return 0.85 + + @strategy_can_handle + Scenario: breadth-depth-navigator returns 0 without graph backend + Given a BackendSet with no backends + And a default ContextRequest + Then "breadth-depth-navigator" can_handle should return 0.0 + + @strategy_can_handle + Scenario: temporal-archaeology returns quality score with graph backend + Given a BackendSet with graph backend only + And a default ContextRequest + Then "temporal-archaeology" can_handle should return 0.5 + + @strategy_can_handle + Scenario: temporal-archaeology returns 0 without graph backend + Given a BackendSet with no backends + And a default ContextRequest + Then "temporal-archaeology" can_handle should return 0.0 + + @strategy_can_handle + Scenario: semantic-embedding returns 0 without vector backend + Given a BackendSet with no backends + And a default ContextRequest + Then "semantic-embedding" can_handle should return 0.0 + + @strategy_config @regression + Scenario: update_config rejects invalid timeout via Pydantic validation + Given a registry with all built-in strategies + When I attempt to update config for "simple-keyword" with timeout_seconds 0 + Then a Pydantic ValidationError should be raised + + @strategy_config @regression + Scenario: update_config rejects negative max_fragments via Pydantic validation + Given a registry with all built-in strategies + When I attempt to update config for "simple-keyword" with max_fragments -1 + Then a Pydantic ValidationError should be raised diff --git a/features/steps/context_strategy_registry_steps.py b/features/steps/context_strategy_registry_steps.py new file mode 100644 index 000000000..b9dc0cac5 --- /dev/null +++ b/features/steps/context_strategy_registry_steps.py @@ -0,0 +1,809 @@ +"""Behave step implementations for context strategy registry feature.""" + +from __future__ import annotations + +from behave import given, then, when +from behave.runner import Context +from pydantic import ValidationError + +from cleveragents.application.services.strategy_registry import ( + StrategyNotFoundError, + StrategyRegistrationError, + StrategyRegistry, +) +from cleveragents.domain.models.acms.crp import ( + ContextFragment, + ContextRequest, + FragmentProvenance, +) +from cleveragents.domain.models.acms.strategy import ( + BackendSet, + ContextStrategy, + ContextStrategyResult, + PlanContext, + StrategyCapabilities, + StrategyConfig, +) +from cleveragents.domain.models.acms.strategy_stubs import ( + BUILTIN_STRATEGY_CLASSES, + DEFAULT_ENABLED_STRATEGIES, +) +from cleveragents.domain.models.acms.stubs import ( + InMemoryGraphBackend, + InMemoryTextBackend, + InMemoryVectorBackend, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _all_builtins() -> dict[str, ContextStrategy]: + """Instantiate all built-in strategies keyed by name.""" + instances: dict[str, ContextStrategy] = {} + for cls in BUILTIN_STRATEGY_CLASSES: + inst = cls() + instances[inst.name] = inst + return instances + + +def _register_all_builtins( + registry: StrategyRegistry, + *, + use_default_enabled: bool = False, +) -> None: + """Register all built-in strategies in the registry.""" + for cls in BUILTIN_STRATEGY_CLASSES: + inst = cls() + enabled = ( + inst.name in DEFAULT_ENABLED_STRATEGIES if use_default_enabled else True + ) + registry.register( + inst, + config=StrategyConfig(enabled=enabled), + is_builtin=True, + ) + if use_default_enabled: + registry.set_enabled(list(DEFAULT_ENABLED_STRATEGIES)) + + +# --------------------------------------------------------------------------- +# Given +# --------------------------------------------------------------------------- + + +@given("all built-in strategies are instantiated") +def step_given_all_builtins(context: Context) -> None: + context.strategies = _all_builtins() + + +@given("an empty strategy registry") +def step_given_empty_registry(context: Context) -> None: + context.registry = StrategyRegistry() + + +@given('I register the "{name}" built-in strategy') +def step_given_register_builtin(context: Context, name: str) -> None: + strategies = _all_builtins() + context.registry.register(strategies[name], is_builtin=True) + + +@given("a registry with all built-in strategies") +def step_given_registry_all(context: Context) -> None: + context.registry = StrategyRegistry() + _register_all_builtins(context.registry) + + +@given("a registry with all built-in strategies using default enabled list") +def step_given_registry_default_enabled(context: Context) -> None: + context.registry = StrategyRegistry() + _register_all_builtins(context.registry, use_default_enabled=True) + + +@given("a BackendSet with text backend only") +def step_given_backends_text(context: Context) -> None: + context.backend_set = BackendSet(text=InMemoryTextBackend()) + + +@given("a BackendSet with vector backend only") +def step_given_backends_vector(context: Context) -> None: + context.backend_set = BackendSet(vector=InMemoryVectorBackend()) + + +@given("a BackendSet with graph backend only") +def step_given_backends_graph(context: Context) -> None: + context.backend_set = BackendSet(graph=InMemoryGraphBackend()) + + +@given("a BackendSet with all backends") +def step_given_backends_all(context: Context) -> None: + context.backend_set = BackendSet( + text=InMemoryTextBackend(), + vector=InMemoryVectorBackend(), + graph=InMemoryGraphBackend(), + ) + + +@given("a BackendSet with no backends") +def step_given_backends_none(context: Context) -> None: + context.backend_set = BackendSet() + + +@given("a default ContextRequest") +def step_given_default_request(context: Context) -> None: + context.request = ContextRequest(query="test query") + + +@given("a default PlanContext") +def step_given_default_plan_context(context: Context) -> None: + context.plan_context = PlanContext() + + +@given("a ContextStrategyResult with unordered fragments") +def step_given_unordered_result(context: Context) -> None: + prov = FragmentProvenance(resource_uri="res://test") + context.result = ContextStrategyResult( + strategy_name="test", + fragments=( + ContextFragment( + uko_node="uko:Bravo", + content="b", + detail_depth=1, + token_count=10, + relevance_score=0.5, + provenance=prov, + ), + ContextFragment( + uko_node="uko:Alpha", + content="a", + detail_depth=1, + token_count=10, + relevance_score=0.9, + provenance=prov, + ), + ContextFragment( + uko_node="uko:Alpha", + content="a2", + detail_depth=1, + token_count=10, + relevance_score=0.5, + provenance=prov, + ), + ), + total_fragments=3, + tokens_used=30, + ) + + +@given("a ContextStrategyResult with no fragments") +def step_given_empty_result(context: Context) -> None: + context.result = ContextStrategyResult(strategy_name="empty") + + +@given("a registry with a stale enabled list entry") +def step_given_stale_enabled(context: Context) -> None: + """Create a registry where the enabled list references a name not registered.""" + context.registry = StrategyRegistry() + _register_all_builtins(context.registry) + # Manually inject a stale name into the internal enabled list + context.registry._enabled_order.append("ghost-strategy") + + +@given("a registry with a strategy that declares no resource types") +def step_given_no_resource_types(context: Context) -> None: + context.registry = StrategyRegistry() + + class _NoResourceTypes: + @property + def name(self) -> str: + return "no-resource-types" + + @property + def capabilities(self) -> StrategyCapabilities: + return StrategyCapabilities( + uses_text=True, + resource_types=(), + quality_score=0.3, + ) + + def can_handle(self, request: ContextRequest, backends: BackendSet) -> float: + return 0.3 + + def assemble( + self, + request: ContextRequest, + backends: BackendSet, + budget: int, + plan_context: PlanContext, + ) -> list[ContextFragment]: + return [] + + def explain(self) -> str: + return "test" + + context.registry.register(_NoResourceTypes()) + + +@given("a registry with a strategy that declares no backend capabilities") +def step_given_no_capabilities(context: Context) -> None: + context.registry = StrategyRegistry() + + class _NoBackend: + @property + def name(self) -> str: + return "no-backends" + + @property + def capabilities(self) -> StrategyCapabilities: + return StrategyCapabilities( + uses_text=False, + uses_vector=False, + uses_graph=False, + uses_temporal=False, + resource_types=("*",), + quality_score=0.1, + ) + + def can_handle(self, request: ContextRequest, backends: BackendSet) -> float: + return 0.1 + + def assemble( + self, + request: ContextRequest, + backends: BackendSet, + budget: int, + plan_context: PlanContext, + ) -> list[ContextFragment]: + return [] + + def explain(self) -> str: + return "test" + + context.registry.register(_NoBackend()) + + +# --------------------------------------------------------------------------- +# When +# --------------------------------------------------------------------------- + + +@when('I register the "{name}" built-in strategy') +def step_when_register_builtin(context: Context, name: str) -> None: + strategies = _all_builtins() + context.registry.register(strategies[name], is_builtin=True) + + +@when("I register all 6 built-in strategies") +def step_when_register_all(context: Context) -> None: + _register_all_builtins(context.registry) + + +@when('I attempt to register "{name}" again') +def step_when_register_duplicate(context: Context, name: str) -> None: + strategies = _all_builtins() + try: + context.registry.register(strategies[name], is_builtin=True) + context.error = None + except StrategyRegistrationError as exc: + context.error = exc + + +@when("I attempt to register a strategy with an empty name") +def step_when_register_empty_name(context: Context) -> None: + class _Empty: + @property + def name(self) -> str: + return "" + + @property + def capabilities(self) -> StrategyCapabilities: + return StrategyCapabilities(uses_text=True, resource_types=("*",)) + + def can_handle(self, request: ContextRequest, backends: BackendSet) -> float: + return 0.0 + + def assemble( + self, + request: ContextRequest, + backends: BackendSet, + budget: int, + plan_context: PlanContext, + ) -> list[ContextFragment]: + return [] + + def explain(self) -> str: + return "" + + try: + context.registry.register(_Empty()) + context.error = None + except StrategyRegistrationError as exc: + context.error = exc + + +@when('I get the strategy "{name}"') +def step_when_get_strategy(context: Context, name: str) -> None: + context.fetched_strategy = context.registry.get(name) + + +@when('I attempt to get the strategy "{name}"') +def step_when_get_nonexistent(context: Context, name: str) -> None: + try: + context.registry.get(name) + context.error = None + except StrategyNotFoundError as exc: + context.error = exc + + +@when('I set the enabled list to "{names}"') +def step_when_set_enabled(context: Context, names: str) -> None: + name_list = [n.strip().strip('"') for n in names.split(",")] + context.registry.set_enabled(name_list) + + +@when('I attempt to set the enabled list to "{names}"') +def step_when_set_enabled_bad(context: Context, names: str) -> None: + name_list = [n.strip().strip('"') for n in names.split(",")] + try: + context.registry.set_enabled(name_list) + context.error = None + except StrategyNotFoundError as exc: + context.error = exc + + +@when('I disable the strategy "{name}"') +def step_when_disable(context: Context, name: str) -> None: + context.registry.update_config(name, enabled=False) + + +@when('I get the config for "{name}"') +def step_when_get_config(context: Context, name: str) -> None: + context.strategy_config = context.registry.get_config(name) + + +@when('I update config for "{name}" with timeout_seconds {timeout:d}') +def step_when_update_config(context: Context, name: str, timeout: int) -> None: + context.registry.update_config(name, timeout_seconds=timeout) + + +@when("each strategy assembles with budget {budget:d}") +def step_when_assemble_all(context: Context, budget: int) -> None: + context.assemble_results = {} + for name, strategy in context.strategies.items(): + result = strategy.assemble( + context.request, + context.backend_set, + budget, + context.plan_context, + ) + context.assemble_results[name] = result + + +@when('I register a strategy from module "{module_path}"') +def step_when_register_from_module(context: Context, module_path: str) -> None: + # Use "simple-keyword" as the name since the scenario asserts on it. + context.registry.register_from_module( + name="simple-keyword", + module_path=module_path, + ) + + +@when('I attempt to register a strategy from module "{module_path}"') +def step_when_register_from_module_fail(context: Context, module_path: str) -> None: + try: + context.registry.register_from_module(name="test", module_path=module_path) + context.error = None + except StrategyRegistrationError as exc: + context.error = exc + + +@when('I get the entry for "{name}"') +def step_when_get_entry(context: Context, name: str) -> None: + context.fetched_entry = context.registry.get_entry(name) + + +@when('I attempt to get the entry for "{name}"') +def step_when_get_entry_fail(context: Context, name: str) -> None: + try: + context.registry.get_entry(name) + context.error = None + except StrategyNotFoundError as exc: + context.error = exc + + +@when('I attempt to unregister "{name}"') +def step_when_unregister_fail(context: Context, name: str) -> None: + try: + context.registry.unregister(name) + context.error = None + except StrategyNotFoundError as exc: + context.error = exc + + +@when('I unregister "{name}"') +def step_when_unregister(context: Context, name: str) -> None: + context.registry.unregister(name) + + +@when("I clear the registry") +def step_when_clear(context: Context) -> None: + context.registry.clear() + + +@when("I validate the registry") +def step_when_validate(context: Context) -> None: + context.validation_warnings = context.registry.validate_registry() + + +# --------------------------------------------------------------------------- +# Then +# --------------------------------------------------------------------------- + + +@then("every strategy should satisfy the ContextStrategy protocol") +def step_then_protocol(context: Context) -> None: + for name, strategy in context.strategies.items(): + assert isinstance(strategy, ContextStrategy), ( + f"Strategy '{name}' does not satisfy ContextStrategy protocol" + ) + + +@then("every strategy should have a non-empty name") +def step_then_nonempty_name(context: Context) -> None: + for name, strategy in context.strategies.items(): + assert strategy.name, f"Strategy has empty name (key={name})" + + +@then("every strategy should have a capabilities object") +def step_then_has_capabilities(context: Context) -> None: + for name, strategy in context.strategies.items(): + caps = strategy.capabilities + assert isinstance(caps, StrategyCapabilities), ( + f"Strategy '{name}' capabilities is not StrategyCapabilities" + ) + + +@then('the strategy "{name}" should have quality score {score:g}') +def step_then_quality_score(context: Context, name: str, score: float) -> None: + strategy = context.strategies[name] + assert strategy.capabilities.quality_score == score, ( + f"Expected {score}, got {strategy.capabilities.quality_score}" + ) + + +@then('the registry should contain "{name}"') +def step_then_contains(context: Context, name: str) -> None: + assert context.registry.is_registered(name), ( + f"'{name}' not in registry: {context.registry.list_all()}" + ) + + +@then('the registry should not contain "{name}"') +def step_then_not_contains(context: Context, name: str) -> None: + assert not context.registry.is_registered(name), ( + f"'{name}' unexpectedly in registry" + ) + + +@then("the registry size should be {size:d}") +def step_then_size(context: Context, size: int) -> None: + assert len(context.registry) == size, ( + f"Expected {size}, got {len(context.registry)}" + ) + + +@then("the registry should contain {count:d} strategies") +def step_then_count(context: Context, count: int) -> None: + assert len(context.registry) == count, ( + f"Expected {count}, got {len(context.registry)}" + ) + + +@then("a StrategyRegistrationError should be raised") +def step_then_reg_error(context: Context) -> None: + assert isinstance(context.error, StrategyRegistrationError), ( + f"Expected StrategyRegistrationError, got {type(context.error)}" + ) + assert str(context.error), "Error message should not be empty" + + +@then("a StrategyNotFoundError should be raised") +def step_then_not_found_error(context: Context) -> None: + assert isinstance(context.error, StrategyNotFoundError), ( + f"Expected StrategyNotFoundError, got {type(context.error)}" + ) + assert str(context.error), "Error message should not be empty" + + +@then('the strategy name should be "{name}"') +def step_then_strategy_name(context: Context, name: str) -> None: + assert context.fetched_strategy.name == name + + +@then("the registry should list {count:d} strategies") +def step_then_list_count(context: Context, count: int) -> None: + all_names = context.registry.list_all() + assert len(all_names) == count, ( + f"Expected {count}, got {len(all_names)}: {all_names}" + ) + + +@then("the list should be sorted alphabetically") +def step_then_sorted(context: Context) -> None: + names = context.registry.list_all() + assert names == sorted(names), f"Names not sorted: {names}" + + +@then('the enabled strategies should be "{names}"') +def step_then_enabled(context: Context, names: str) -> None: + expected = [n.strip().strip('"') for n in names.split(",")] + actual = context.registry.list_enabled() + assert actual == expected, f"Expected {expected}, got {actual}" + + +@then('the enabled strategies should not include "{name}"') +def step_then_not_enabled(context: Context, name: str) -> None: + enabled = context.registry.list_enabled() + assert name not in enabled, f"'{name}' unexpectedly in enabled list: {enabled}" + + +@then("the timeout_seconds should be {val:d}") +def step_then_timeout(context: Context, val: int) -> None: + assert context.strategy_config.timeout_seconds == val + + +@then("the max_fragments should be {val:d}") +def step_then_max_fragments(context: Context, val: int) -> None: + assert context.strategy_config.max_fragments == val + + +@then("the circuit_breaker_threshold should be {val:d}") +def step_then_cb(context: Context, val: int) -> None: + assert context.strategy_config.circuit_breaker_threshold == val + + +@then('the config for "{name}" should have timeout_seconds {val:d}') +def step_then_config_timeout(context: Context, name: str, val: int) -> None: + config = context.registry.get_config(name) + assert config.timeout_seconds == val, ( + f"Expected {val}, got {config.timeout_seconds}" + ) + + +@then('"{name}" can_handle should return {score:g}') +def step_then_can_handle(context: Context, name: str, score: float) -> None: + strategies = _all_builtins() + strategy = strategies[name] + result = strategy.can_handle(context.request, context.backend_set) + assert abs(result - score) < 1e-6, f"Expected {score}, got {result} for '{name}'" + + +@then("every strategy should return an empty fragment list") +def step_then_empty_assemble(context: Context) -> None: + for name, result in context.assemble_results.items(): + assert result == [], f"Strategy '{name}' returned non-empty: {result}" + + +@then("the fragments should be sorted by relevance_score DESC then uko_node ASC") +def step_then_sorted_fragments(context: Context) -> None: + frags = context.result.fragments + assert len(frags) == 3 + # relevance DESC: 0.9 first, then 0.5s; within 0.5: Alpha < Bravo + assert frags[0].relevance_score == 0.9 + assert frags[0].uko_node == "uko:Alpha" + assert frags[1].relevance_score == 0.5 + assert frags[1].uko_node == "uko:Alpha" + assert frags[2].relevance_score == 0.5 + assert frags[2].uko_node == "uko:Bravo" + + +@then("the total_fragments should be {val:d}") +def step_then_total_fragments(context: Context, val: int) -> None: + assert context.result.total_fragments == val + + +@then("the tokens_used should be {val:d}") +def step_then_tokens_used(context: Context, val: int) -> None: + assert context.result.tokens_used == val + + +@then('validation warnings should mention "{text}"') +def step_then_validation_mentions(context: Context, text: str) -> None: + combined = " ".join(context.validation_warnings) + assert text in combined, ( + f"Expected '{text}' in warnings: {context.validation_warnings}" + ) + + +@then('the entry name should be "{name}"') +def step_then_entry_name(context: Context, name: str) -> None: + assert context.fetched_entry.name == name + + +@then("the entry should be marked as builtin") +def step_then_entry_builtin(context: Context) -> None: + assert context.fetched_entry.is_builtin, "Expected entry to be builtin" + + +@then("the builtin list should contain {count:d} strategies") +def step_then_builtin_count(context: Context, count: int) -> None: + builtins = context.registry.list_builtin() + assert len(builtins) == count, ( + f"Expected {count} builtins, got {len(builtins)}: {builtins}" + ) + + +@then('"{name}" should be in the registry via contains') +def step_then_contains_in(context: Context, name: str) -> None: + assert name in context.registry, f"'{name}' not in registry via __contains__" + + +@then('"{name}" should not be in the registry via contains') +def step_then_not_contains_in(context: Context, name: str) -> None: + assert name not in context.registry, ( + f"'{name}' unexpectedly in registry via __contains__" + ) + + +@then("there should be no validation warnings") +def step_then_no_warnings(context: Context) -> None: + assert context.validation_warnings == [], ( + f"Expected no warnings, got: {context.validation_warnings}" + ) + + +# --------------------------------------------------------------------------- +# Boundary test steps (TEST-1) +# --------------------------------------------------------------------------- + + +@when("I attempt to create a StrategyConfig with {field} set to {value:d}") +def step_when_invalid_config(context: Context, field: str, value: int) -> None: + try: + StrategyConfig.model_validate({field: value}) + context.error = None + except ValidationError as exc: + context.error = exc + + +@when("I attempt to create a StrategyCapabilities with quality_score {value:g}") +def step_when_invalid_quality(context: Context, value: float) -> None: + try: + StrategyCapabilities(uses_text=True, quality_score=value) + context.error = None + except ValidationError as exc: + context.error = exc + + +@when("I create a StrategyCapabilities with quality_score {value:g}") +def step_when_valid_quality(context: Context, value: float) -> None: + context.created_capabilities = StrategyCapabilities( + uses_text=True, + quality_score=value, + ) + + +@then("a Pydantic ValidationError should be raised") +def step_then_pydantic_validation_error(context: Context) -> None: + assert context.error is not None, ( + "Expected Pydantic ValidationError but none raised" + ) + assert isinstance(context.error, ValidationError), ( + f"Expected pydantic.ValidationError, got {type(context.error).__name__}" + ) + + +@then("the quality_score should be {value:g}") +def step_then_quality_value(context: Context, value: float) -> None: + assert abs(context.created_capabilities.quality_score - value) < 1e-9, ( + f"Expected {value}, got {context.created_capabilities.quality_score}" + ) + + +# --------------------------------------------------------------------------- +# Bug regression steps +# --------------------------------------------------------------------------- + + +@when( + 'I update config for "{name}" with max_workers {mw:d}' + " and circuit_breaker_threshold {cb:d}" +) +def step_when_update_mw_cb(context: Context, name: str, mw: int, cb: int) -> None: + context.registry.update_config(name, max_workers=mw, circuit_breaker_threshold=cb) + + +@then('the config for "{name}" should have max_workers {val:d}') +def step_then_config_max_workers(context: Context, name: str, val: int) -> None: + config = context.registry.get_config(name) + assert config.max_workers == val, f"Expected {val}, got {config.max_workers}" + + +@then('the config for "{name}" should have circuit_breaker_threshold {val:d}') +def step_then_config_cb(context: Context, name: str, val: int) -> None: + config = context.registry.get_config(name) + assert config.circuit_breaker_threshold == val, ( + f"Expected {val}, got {config.circuit_breaker_threshold}" + ) + + +@when("I attempt to register a non-protocol object") +def step_when_register_non_protocol(context: Context) -> None: + try: + # Pass name explicitly so register() reaches the isinstance check + context.registry.register("not-a-strategy", name="fake") # type: ignore[arg-type] + context.error = None + except StrategyRegistrationError as exc: + context.error = exc + + +@then("every strategy should return a non-empty explain string") +def step_then_explain_nonempty(context: Context) -> None: + for name, strategy in context.strategies.items(): + explanation = strategy.explain() + assert isinstance(explanation, str), ( + f"Strategy '{name}' explain() returned {type(explanation)}, expected str" + ) + assert explanation.strip(), f"Strategy '{name}' returned empty explain string" + + +@when('I register the "{name}" built-in strategy with enabled False') +def step_when_register_builtin_disabled(context: Context, name: str) -> None: + strategies = _all_builtins() + context.registry.register( + strategies[name], + config=StrategyConfig(enabled=False), + is_builtin=True, + ) + + +@when('I update config for "{name}" with enabled True') +def step_when_update_enabled_true(context: Context, name: str) -> None: + context.registry.update_config(name, enabled=True) + + +@when('I update config for "{name}" with enabled False') +def step_when_update_enabled_false(context: Context, name: str) -> None: + context.registry.update_config(name, enabled=False) + + +@then('the enabled strategies should be ""') +def step_then_enabled_empty(context: Context) -> None: + actual = context.registry.list_enabled() + assert actual == [], f"Expected empty enabled list, got {actual}" + + +@then('the enabled strategies should include "{name}"') +def step_then_enabled_includes(context: Context, name: str) -> None: + enabled = context.registry.list_enabled() + assert name in enabled, f"'{name}' not in enabled list: {enabled}" + + +@when('I register a strategy named "{name}" from module "{module_path}"') +def step_when_register_from_module_with_name( + context: Context, name: str, module_path: str +) -> None: + context.registry.register_from_module(name=name, module_path=module_path) + + +@when('I attempt to update config for "{name}" with timeout_seconds {value:d}') +def step_when_update_config_bad_timeout( + context: Context, name: str, value: int +) -> None: + try: + context.registry.update_config(name, timeout_seconds=value) + context.error = None + except ValidationError as exc: + context.error = exc + + +@when('I attempt to update config for "{name}" with max_fragments {value:d}') +def step_when_update_config_bad_fragments( + context: Context, name: str, value: int +) -> None: + try: + context.registry.update_config(name, max_fragments=value) + context.error = None + except ValidationError as exc: + context.error = exc diff --git a/robot/context_strategy_registry.robot b/robot/context_strategy_registry.robot new file mode 100644 index 000000000..1ce9e2948 --- /dev/null +++ b/robot/context_strategy_registry.robot @@ -0,0 +1,35 @@ +*** Settings *** +Documentation Integration smoke tests for context strategy registry +Library Process +Suite Setup Log Context Strategy Registry Robot Tests + +*** Variables *** +${HELPER} ${CURDIR}/helper_context_strategy_registry.py + +*** Test Cases *** +Register All Built-in Strategies + [Documentation] Register 6 built-in strategies with default enabled list + ${result}= Run Process python3 ${HELPER} register-builtins + Log ${result.stdout} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} strategy-ok + +Verify Protocol Conformance + [Documentation] All built-in strategies satisfy ContextStrategy protocol + ${result}= Run Process python3 ${HELPER} protocol-check + Log ${result.stdout} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} strategy-ok + +Test Can Handle With Backends + [Documentation] Verify can_handle returns correct scores per backend availability + ${result}= Run Process python3 ${HELPER} can-handle + Log ${result.stdout} + Should Be Equal As Integers ${result.rc} 0 + +Validate Registry + [Documentation] Validate registry with all built-in strategies passes + ${result}= Run Process python3 ${HELPER} validate + Log ${result.stdout} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} validation passed diff --git a/robot/helper_context_strategy_registry.py b/robot/helper_context_strategy_registry.py new file mode 100644 index 000000000..f9821b536 --- /dev/null +++ b/robot/helper_context_strategy_registry.py @@ -0,0 +1,153 @@ +#!/usr/bin/env python3 +"""Robot Framework helper for context strategy registry smoke tests.""" + +from __future__ import annotations + +import sys +from collections.abc import Callable +from pathlib import Path + +# Ensure src is on sys.path +_SRC = Path(__file__).resolve().parents[1] / "src" +if str(_SRC) not in sys.path: + sys.path.insert(0, str(_SRC)) + +from cleveragents.application.services.strategy_registry import ( # noqa: E402 + StrategyRegistry, +) +from cleveragents.domain.models.acms.crp import ContextRequest # noqa: E402 +from cleveragents.domain.models.acms.strategy import ( # noqa: E402 + BackendSet, + ContextStrategy, + StrategyConfig, +) +from cleveragents.domain.models.acms.strategy_stubs import ( # noqa: E402 + BUILTIN_STRATEGY_CLASSES, + DEFAULT_ENABLED_STRATEGIES, +) +from cleveragents.domain.models.acms.stubs import ( # noqa: E402 + InMemoryGraphBackend, + InMemoryTextBackend, + InMemoryVectorBackend, +) + +# --------------------------------------------------------------------------- +# Commands +# --------------------------------------------------------------------------- + + +def _cmd_register_builtins() -> int: + """Register all built-in strategies and verify count.""" + registry = StrategyRegistry() + for cls in BUILTIN_STRATEGY_CLASSES: + inst = cls() + enabled = inst.name in DEFAULT_ENABLED_STRATEGIES + registry.register(inst, config=StrategyConfig(enabled=enabled), is_builtin=True) + registry.set_enabled(list(DEFAULT_ENABLED_STRATEGIES)) + + if len(registry) != 6: + print(f"strategy-fail: expected 6 strategies, got {len(registry)}") + return 1 + print(f"strategy-ok: registered {len(registry)} strategies") + + enabled = registry.list_enabled() + if len(enabled) != 3: + print(f"strategy-fail: expected 3 enabled, got {len(enabled)}") + return 1 + print(f"strategy-enabled: {enabled}") + return 0 + + +def _cmd_protocol_check() -> int: + """Verify all built-in strategies satisfy ContextStrategy protocol.""" + for cls in BUILTIN_STRATEGY_CLASSES: + inst = cls() + if not isinstance(inst, ContextStrategy): + print(f"strategy-fail: {cls.__name__} doesn't satisfy protocol") + return 1 + if not inst.name: + print(f"strategy-fail: {cls.__name__} has empty name") + return 1 + if inst.capabilities.quality_score <= 0: + print(f"strategy-fail: {cls.__name__} has invalid quality score") + return 1 + print(f"strategy-ok: {inst.name} (quality={inst.capabilities.quality_score})") + return 0 + + +def _cmd_can_handle() -> int: + """Test can_handle with various backend configurations.""" + from cleveragents.domain.models.acms.strategy_stubs import ( + ARCEStrategy, + SimpleKeywordStrategy, + ) + + request = ContextRequest(query="test") + + # simple-keyword with text backend + sk = SimpleKeywordStrategy() + bs_text = BackendSet(text=InMemoryTextBackend()) + score = sk.can_handle(request, bs_text) + if abs(score - 0.3) > 1e-6: + print(f"strategy-fail: simple-keyword expected 0.3, got {score}") + return 1 + print(f"strategy-ok: simple-keyword can_handle with text = {score}") + + # simple-keyword without backend + bs_none = BackendSet() + score = sk.can_handle(request, bs_none) + if score != 0.0: + print( + f"strategy-fail: simple-keyword expected 0.0 without backend, got {score}" + ) + return 1 + print(f"strategy-ok: simple-keyword can_handle without backend = {score}") + + # arce with all backends + arce = ARCEStrategy() + bs_all = BackendSet( + text=InMemoryTextBackend(), + vector=InMemoryVectorBackend(), + graph=InMemoryGraphBackend(), + ) + score = arce.can_handle(request, bs_all) + if abs(score - 0.95) > 1e-6: + print(f"strategy-fail: arce expected 0.95, got {score}") + return 1 + print(f"strategy-ok: arce can_handle with all = {score}") + + return 0 + + +def _cmd_validate() -> int: + """Validate registry with all built-in strategies.""" + registry = StrategyRegistry() + for cls in BUILTIN_STRATEGY_CLASSES: + registry.register(cls(), is_builtin=True) + + warnings = registry.validate_registry() + if warnings: + print(f"strategy-fail: unexpected warnings: {warnings}") + return 1 + print("strategy-ok: registry validation passed with no warnings") + return 0 + + +_COMMANDS: dict[str, Callable[[], int]] = { + "register-builtins": _cmd_register_builtins, + "protocol-check": _cmd_protocol_check, + "can-handle": _cmd_can_handle, + "validate": _cmd_validate, +} + + +def main() -> int: + """Run the specified command.""" + if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS: + print(f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>") + return 2 + return _COMMANDS[sys.argv[1]]() + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/cleveragents/application/services/__init__.py b/src/cleveragents/application/services/__init__.py index 352bda17b..8085f5209 100644 --- a/src/cleveragents/application/services/__init__.py +++ b/src/cleveragents/application/services/__init__.py @@ -111,6 +111,11 @@ from cleveragents.application.services.skeleton_compressor import ( from cleveragents.application.services.skill_registry_service import ( SkillRegistryService, ) +from cleveragents.application.services.strategy_registry import ( + StrategyNotFoundError, + StrategyRegistrationError, + StrategyRegistry, +) from cleveragents.application.services.subplan_execution_service import ( SubplanExecutionOutput, SubplanExecutionResult, @@ -226,6 +231,9 @@ __all__ = [ "SpawnResult", "SpawnValidationError", "SpawnValidationResult", + "StrategyNotFoundError", + "StrategyRegistrationError", + "StrategyRegistry", "SubplanExecutionOutput", "SubplanExecutionResult", "SubplanExecutionService", diff --git a/src/cleveragents/application/services/strategy_registry.py b/src/cleveragents/application/services/strategy_registry.py new file mode 100644 index 000000000..425000e9f --- /dev/null +++ b/src/cleveragents/application/services/strategy_registry.py @@ -0,0 +1,442 @@ +"""Context strategy registry for the ACMS. + +Provides ``StrategyRegistry`` — the central registry for context +strategies. Supports: + +- Registration of built-in and custom strategies +- Configuration-driven enable/disable (spec §25218-25233) +- Per-strategy timeout and max-fragment limits (spec §28706-28708) +- Validation that strategies declare supported resource types +- Per-project strategy overrides via enabled list +- Plugin discovery from ``"module:ClassName"`` strings + +Based on ``docs/specification.md`` §§ 25218-25233, 28682-28708, +42947-42980, and issue #191. +""" + +from __future__ import annotations + +import importlib + +import structlog + +from cleveragents.domain.models.acms.strategy import ( + ContextStrategy, + StrategyConfig, + StrategyRegistryEntry, +) + +logger = structlog.get_logger(__name__) + + +# --------------------------------------------------------------------------- +# Exceptions +# --------------------------------------------------------------------------- + + +class StrategyRegistrationError(Exception): + """Raised when strategy registration fails.""" + + +class StrategyNotFoundError(KeyError): + """Raised when a requested strategy is not in the registry.""" + + +# --------------------------------------------------------------------------- +# StrategyRegistry +# --------------------------------------------------------------------------- + + +class StrategyRegistry: + """Central registry for context strategies. + + Manages built-in and custom strategies with per-strategy + configuration. The registry is the single source of truth for + which strategies are available and how they are configured. + + Strategies are registered via :meth:`register` and discovered + via :meth:`get`, :meth:`list_enabled`, or :meth:`list_all`. + + Configuration-driven registration follows spec §25218-25233: + + - ``[context.strategies] enabled = [...]`` controls the global + enabled list. + - ``[context.strategies.custom] "name" = "module:Class"`` + registers custom strategies from Python modules. + + Per-project overrides replace the global enabled list. + + Lifecycle:: + + registry = StrategyRegistry() + registry.register(my_strategy, config=StrategyConfig(timeout_seconds=10)) + enabled = registry.list_enabled() + result = registry.get("simple-keyword") + """ + + def __init__(self) -> None: + """Initialize an empty registry.""" + self._strategies: dict[str, ContextStrategy] = {} + self._entries: dict[str, StrategyRegistryEntry] = {} + self._enabled_order: list[str] = [] + + # ------------------------------------------------------------------ + # Registration + # ------------------------------------------------------------------ + + def register( + self, + strategy: ContextStrategy, + *, + name: str | None = None, + config: StrategyConfig | None = None, + module_path: str = "", + is_builtin: bool = False, + ) -> None: + """Register a strategy with the registry. + + Args: + strategy: Strategy instance implementing ``ContextStrategy``. + name: Override name for the registry key. If ``None``, + ``strategy.name`` is used. Used by + :meth:`register_from_module` to honour the TOML key. + config: Per-strategy configuration. Defaults to + ``StrategyConfig()`` if not provided. + module_path: Python module path (for custom strategies). + is_builtin: Whether this is a built-in strategy. + + Raises: + StrategyRegistrationError: If the strategy name is empty, + already registered, or the strategy doesn't satisfy + the ``ContextStrategy`` protocol. + """ + name = name or strategy.name + if not name: + raise StrategyRegistrationError("Strategy name must not be empty") + + if name in self._strategies: + raise StrategyRegistrationError(f"Strategy '{name}' is already registered") + + # Validate protocol conformance + if not isinstance(strategy, ContextStrategy): + raise StrategyRegistrationError( + f"Strategy '{name}' does not satisfy the ContextStrategy protocol" + ) + + resolved_config = config or StrategyConfig() + entry = StrategyRegistryEntry( + name=name, + config=resolved_config, + module_path=module_path, + is_builtin=is_builtin, + ) + + self._strategies[name] = strategy + self._entries[name] = entry + + if resolved_config.enabled: + self._enabled_order.append(name) + + logger.debug( + "strategy.registered", + name=name, + builtin=is_builtin, + enabled=resolved_config.enabled, + ) + + def register_from_module( + self, + name: str, + module_path: str, + *, + config: StrategyConfig | None = None, + ) -> None: + """Register a custom strategy from a ``"module:ClassName"`` string. + + Performs plugin discovery per spec §25225-25226: + ``[context.strategies.custom] "name" = "module:Class"``. + + Security: + Module paths should come from trusted configuration only + (e.g., project TOML files). Importing a module executes its + module-level code, and instantiating the class runs its + ``__init__``. Never pass untrusted or user-supplied strings + as ``module_path``. + + Args: + name: Unique name for the strategy. + module_path: Python path in ``"module:ClassName"`` or + ``"module.path:ClassName"`` format. + config: Per-strategy configuration. + + Raises: + StrategyRegistrationError: If import fails, the class + doesn't exist, or the instance doesn't satisfy the + ``ContextStrategy`` protocol. + """ + if ":" not in module_path: + raise StrategyRegistrationError( + f"Invalid module_path '{module_path}': " + f"expected 'module:ClassName' format" + ) + + module_name, class_name = module_path.rsplit(":", 1) + + try: + module = importlib.import_module(module_name) + except ImportError as exc: + raise StrategyRegistrationError( + f"Cannot import module '{module_name}' for strategy '{name}': {exc}" + ) from exc + + cls = getattr(module, class_name, None) + if cls is None: + raise StrategyRegistrationError( + f"Class '{class_name}' not found in module '{module_name}'" + ) + + try: + instance = cls() + except Exception as exc: + raise StrategyRegistrationError( + f"Cannot instantiate '{class_name}' for strategy '{name}': {exc}" + ) from exc + + self.register( + instance, + name=name, + config=config, + module_path=module_path, + is_builtin=False, + ) + + # ------------------------------------------------------------------ + # Query + # ------------------------------------------------------------------ + + def get(self, name: str) -> ContextStrategy: + """Return the strategy with the given name. + + Args: + name: Strategy name. + + Returns: + The registered ``ContextStrategy`` instance. + + Raises: + StrategyNotFoundError: If the name is not registered. + """ + if name not in self._strategies: + raise StrategyNotFoundError( + f"Strategy '{name}' not found in registry. " + f"Available: {sorted(self._strategies)}" + ) + return self._strategies[name] + + def get_entry(self, name: str) -> StrategyRegistryEntry: + """Return the registry entry (metadata + config) for a strategy. + + Raises: + StrategyNotFoundError: If the name is not registered. + """ + if name not in self._entries: + raise StrategyNotFoundError(f"Strategy '{name}' not found in registry") + return self._entries[name] + + def get_config(self, name: str) -> StrategyConfig: + """Return the configuration for a registered strategy. + + Raises: + StrategyNotFoundError: If the name is not registered. + """ + return self.get_entry(name).config + + def list_all(self) -> list[str]: + """Return names of all registered strategies.""" + return sorted(self._strategies) + + def list_enabled(self) -> list[str]: + """Return names of enabled strategies in registration order. + + The order matches the ``[context.strategies] enabled`` list, + which controls priority during selection (spec §28682). + """ + return [ + n + for n in self._enabled_order + if n in self._entries and self._entries[n].config.enabled + ] + + def list_builtin(self) -> list[str]: + """Return names of built-in strategies.""" + return sorted(n for n, e in self._entries.items() if e.is_builtin) + + def is_registered(self, name: str) -> bool: + """Check whether a strategy is registered.""" + return name in self._strategies + + def __len__(self) -> int: + """Return the number of registered strategies.""" + return len(self._strategies) + + def __contains__(self, name: str) -> bool: + """Check whether a strategy name is registered.""" + return name in self._strategies + + # ------------------------------------------------------------------ + # Configuration + # ------------------------------------------------------------------ + + def set_enabled(self, names: list[str]) -> None: + """Replace the enabled strategy list. + + Supports per-project overrides: the ``--strategy`` CLI flag or + project YAML replaces the global ``context.strategies.enabled`` + list (spec §3782). + + Also updates each strategy's ``config.enabled`` flag to match: + strategies in *names* are enabled, all others are disabled. + + Args: + names: Ordered list of strategy names to enable. + + Raises: + StrategyNotFoundError: If any name is not registered. + """ + for name in names: + if name not in self._strategies: + raise StrategyNotFoundError(f"Cannot enable unknown strategy '{name}'") + + # Update config.enabled for all strategies + enabled_set = set(names) + for sname, entry in self._entries.items(): + should_enable = sname in enabled_set + if entry.config.enabled != should_enable: + new_config = entry.config.model_copy( + update={"enabled": should_enable}, + ) + self._entries[sname] = entry.model_copy( + update={"config": new_config}, + ) + + self._enabled_order = list(names) + + def update_config( + self, + name: str, + *, + enabled: bool | None = None, + timeout_seconds: int | None = None, + max_fragments: int | None = None, + max_workers: int | None = None, + circuit_breaker_threshold: int | None = None, + ) -> None: + """Update configuration for a registered strategy. + + Only provided (non-``None``) fields are updated; the rest + retain their current values. + + Args: + name: Strategy name. + enabled: Whether the strategy is enabled. + timeout_seconds: Assembly timeout in seconds (>=1). + max_fragments: Max fragments per call (>=1). + max_workers: Max parallel workers (>=1). + circuit_breaker_threshold: Failures before circuit opens (>=1). + + Raises: + StrategyNotFoundError: If the name is not registered. + pydantic.ValidationError: If the updated values violate + ``StrategyConfig`` constraints (e.g., ``timeout_seconds < 1``). + """ + entry = self.get_entry(name) + updates: dict[str, bool | int] = {} + if enabled is not None: + updates["enabled"] = enabled + if timeout_seconds is not None: + updates["timeout_seconds"] = timeout_seconds + if max_fragments is not None: + updates["max_fragments"] = max_fragments + if max_workers is not None: + updates["max_workers"] = max_workers + if circuit_breaker_threshold is not None: + updates["circuit_breaker_threshold"] = circuit_breaker_threshold + merged = entry.config.model_dump() + merged.update(updates) + new_config = StrategyConfig.model_validate(merged) + new_entry = entry.model_copy(update={"config": new_config}) + self._entries[name] = new_entry + + # Keep _enabled_order in sync with the enabled flag + if enabled is True and name not in self._enabled_order: + self._enabled_order.append(name) + elif enabled is False and name in self._enabled_order: + self._enabled_order = [n for n in self._enabled_order if n != name] + + # ------------------------------------------------------------------ + # Validation + # ------------------------------------------------------------------ + + def validate_registry(self) -> list[str]: + """Validate the registry and return a list of warnings. + + Checks: + - Every enabled strategy is actually registered. + - Every strategy declares at least one capability. + - Every strategy declares supported resource types + (per issue #191 subtask). + + Returns: + List of warning messages (empty = valid). + """ + warnings: list[str] = [] + + for name in self._enabled_order: + if name not in self._strategies: + warnings.append(f"Enabled strategy '{name}' is not registered") + + for name, strategy in self._strategies.items(): + caps = strategy.capabilities + + has_any_backend = ( + caps.uses_text + or caps.uses_vector + or caps.uses_graph + or caps.uses_temporal + ) + if not has_any_backend: + warnings.append(f"Strategy '{name}' declares no backend capabilities") + + if not caps.resource_types: + warnings.append( + f"Strategy '{name}' does not declare supported " + f"resource types (capabilities.resource_types is empty)" + ) + + return warnings + + # ------------------------------------------------------------------ + # Removal (for testing / reconfiguration) + # ------------------------------------------------------------------ + + def unregister(self, name: str) -> None: + """Remove a strategy from the registry. + + Args: + name: Strategy name. + + Raises: + StrategyNotFoundError: If the name is not registered. + """ + if name not in self._strategies: + raise StrategyNotFoundError(f"Cannot unregister unknown strategy '{name}'") + + del self._strategies[name] + del self._entries[name] + self._enabled_order = [n for n in self._enabled_order if n != name] + + def clear(self) -> None: + """Remove all registered strategies.""" + self._strategies.clear() + self._entries.clear() + self._enabled_order.clear() diff --git a/src/cleveragents/domain/models/acms/__init__.py b/src/cleveragents/domain/models/acms/__init__.py index 7ab34236e..297d4bd9f 100644 --- a/src/cleveragents/domain/models/acms/__init__.py +++ b/src/cleveragents/domain/models/acms/__init__.py @@ -17,6 +17,23 @@ BAL types (from :mod:`~cleveragents.domain.models.acms.backends`): - ``VectorBackend`` / ``VectorResult`` -- Embedding similarity search - ``GraphBackend`` / ``GraphResult`` -- SPARQL queries and traversals +Strategy types (from :mod:`~cleveragents.domain.models.acms.strategy`): +- ``ContextStrategy`` -- Pluggable context assembly strategy protocol +- ``StrategyCapabilities`` -- Declares what a strategy can do +- ``BackendSet`` -- Container for available data backends +- ``PlanContext`` -- Lightweight plan hierarchy reference +- ``StrategyConfig`` -- Per-strategy configuration +- ``ContextStrategyResult`` -- Output of a strategy execution +- ``StrategyRegistryEntry`` -- Metadata for a registered strategy + +Strategy stubs (from :mod:`~cleveragents.domain.models.acms.strategy_stubs`): +- ``SimpleKeywordStrategy`` -- Basic keyword/regex text search (quality 0.3) +- ``SemanticEmbeddingStrategy`` -- Vector similarity search (quality 0.6) +- ``BreadthDepthNavigatorStrategy`` -- Graph-aware UKO traversal (quality 0.85) +- ``ARCEStrategy`` -- Multi-modal ARCE pipeline (quality 0.95) +- ``TemporalArchaeologyStrategy`` -- Historical pattern discovery (quality 0.5) +- ``PlanDecisionContextStrategy`` -- Parent/ancestor plan context (quality 0.7) + Stub backends (from :mod:`~cleveragents.domain.models.acms.stubs`): - ``InMemoryTextBackend`` -- Zero-dependency text search stub - ``InMemoryVectorBackend`` -- Zero-dependency vector search stub @@ -68,6 +85,23 @@ from cleveragents.domain.models.acms.crp import ( ) from cleveragents.domain.models.acms.markdown_analyzer import MarkdownAnalyzer from cleveragents.domain.models.acms.python_analyzer import PythonAnalyzer +from cleveragents.domain.models.acms.strategy import ( + BackendSet, + ContextStrategy, + ContextStrategyResult, + PlanContext, + StrategyCapabilities, + StrategyConfig, + StrategyRegistryEntry, +) +from cleveragents.domain.models.acms.strategy_stubs import ( + ARCEStrategy, + BreadthDepthNavigatorStrategy, + PlanDecisionContextStrategy, + SemanticEmbeddingStrategy, + SimpleKeywordStrategy, + TemporalArchaeologyStrategy, +) from cleveragents.domain.models.acms.stubs import ( InMemoryGraphBackend, InMemoryTextBackend, @@ -84,14 +118,19 @@ from cleveragents.domain.models.acms.tiers import ( ) __all__: list[str] = [ + "ARCEStrategy", "ActorContextView", "ActorRole", "AnalyzerProtocol", "AnalyzerRegistry", "AssembledContext", + "BackendSet", + "BreadthDepthNavigatorStrategy", "ContextBudget", "ContextFragment", "ContextRequest", + "ContextStrategy", + "ContextStrategyResult", "ContextTier", "DetailLevelMap", "FragmentProvenance", @@ -101,8 +140,16 @@ __all__: list[str] = [ "InMemoryTextBackend", "InMemoryVectorBackend", "MarkdownAnalyzer", + "PlanContext", + "PlanDecisionContextStrategy", "PythonAnalyzer", "ScopedBackendView", + "SemanticEmbeddingStrategy", + "SimpleKeywordStrategy", + "StrategyCapabilities", + "StrategyConfig", + "StrategyRegistryEntry", + "TemporalArchaeologyStrategy", "TextBackend", "TextResult", "TierBudget", diff --git a/src/cleveragents/domain/models/acms/strategy.py b/src/cleveragents/domain/models/acms/strategy.py new file mode 100644 index 000000000..f8e132cc8 --- /dev/null +++ b/src/cleveragents/domain/models/acms/strategy.py @@ -0,0 +1,343 @@ +"""Context strategy domain models for the ACMS. + +Defines the ``ContextStrategy`` protocol, supporting value objects, and +the ``ContextStrategyResult`` output model used by strategy stubs and +the Context Assembly Pipeline. + +Based on ``docs/specification.md`` §§ 25162-25233 (Context Strategy +Protocol, StrategyCapabilities, Built-in Strategies) and +§§ 28682-28708 (strategy configuration keys). + +All frozen Pydantic models follow ADR-004 immutability rules. +""" + +from __future__ import annotations + +from typing import Any, Protocol, runtime_checkable + +from pydantic import BaseModel, ConfigDict, Field, field_validator + +from cleveragents.domain.models.acms.backends import ( + GraphBackend, + TextBackend, + VectorBackend, +) +from cleveragents.domain.models.acms.crp import ( + ContextFragment, + ContextRequest, +) + +# --------------------------------------------------------------------------- +# BackendSet — container for available data backends +# --------------------------------------------------------------------------- + + +class BackendSet(BaseModel, frozen=True): + """Container holding the set of available data backends. + + Strategies inspect which backends are present (not ``None``) to + determine their ``can_handle`` confidence. + + Based on spec §42851 — ``ContextAssemblyPipeline`` passes a + ``BackendSet`` to strategies during assembly. + """ + + text: TextBackend | None = Field( + default=None, + description="Full-text search backend (e.g., Tantivy, SQLite FTS)", + ) + vector: VectorBackend | None = Field( + default=None, + description="Vector similarity search backend (e.g., FAISS, Qdrant)", + ) + graph: GraphBackend | None = Field( + default=None, + description="Knowledge graph backend (e.g., Blazegraph, Neo4j)", + ) + + model_config = ConfigDict(arbitrary_types_allowed=True) + + +# --------------------------------------------------------------------------- +# PlanContext — lightweight plan hierarchy reference +# --------------------------------------------------------------------------- + + +class PlanContext(BaseModel, frozen=True): + """Lightweight reference to the current plan's hierarchy. + + Passed to ``ContextStrategy.assemble()`` so strategies like + ``plan-decision-context`` can traverse parent/ancestor decisions. + + Based on spec §25182-25186 — ``assemble()`` receives + ``plan_context: PlanContext``. + """ + + plan_id: str = Field( + default="", + description="ULID of the current plan", + ) + parent_plan_id: str = Field( + default="", + description="ULID of the parent plan (empty if root)", + ) + ancestor_plan_ids: tuple[str, ...] = Field( + default=(), + description="Ordered ancestor plan ULIDs (root first)", + ) + + +# --------------------------------------------------------------------------- +# StrategyCapabilities — declares what a strategy can do +# --------------------------------------------------------------------------- + + +class StrategyCapabilities(BaseModel, frozen=True): + """Declares what a strategy is capable of. + + Based on spec §25192-25205. Strategies expose this to the + ``StrategySelector`` so it can match strategies to requests. + """ + + uses_text: bool = Field( + default=False, + description="Requires a TextBackend", + ) + uses_vector: bool = Field( + default=False, + description="Requires a VectorBackend", + ) + uses_graph: bool = Field( + default=False, + description="Requires a GraphBackend", + ) + uses_temporal: bool = Field( + default=False, + description="Requires temporal/cold-tier data", + ) + uko_levels: tuple[str, ...] = Field( + default=(), + description="UKO levels this strategy operates on", + ) + resource_types: tuple[str, ...] = Field( + default=(), + description="Resource types this strategy supports (empty = all)", + ) + supports_depth_breadth: bool = Field( + default=False, + description="Supports depth/breadth projection", + ) + supports_plan_hierarchy: bool = Field( + default=False, + description="Supports plan hierarchy traversal", + ) + supports_temporal: bool = Field( + default=False, + description="Supports temporal archaeology queries", + ) + quality_score: float = Field( + default=0.5, + ge=0.0, + le=1.0, + description="Intrinsic quality score (0.0-1.0)", + ) + + +# --------------------------------------------------------------------------- +# ContextStrategy — the pluggable strategy protocol +# --------------------------------------------------------------------------- + + +@runtime_checkable +class ContextStrategy(Protocol): + """A pluggable context assembly strategy. + + Spec ref: ``specification.md:25166-25189``. + + Strategies are registered via configuration (TOML ``[context.strategies]`` + section) and invoked by the ``StrategyExecutor`` pipeline component + in parallel. + + ``can_handle`` returns 0.0-1.0 confidence that this strategy can + usefully contribute to the given request. ``assemble`` executes the + strategy and **must** respect the ``budget`` parameter. + """ + + @property + def name(self) -> str: + """Strategy's unique name (e.g., 'simple-keyword').""" + ... + + @property + def capabilities(self) -> StrategyCapabilities: + """Declares what backends and features this strategy uses.""" + ... + + def can_handle( + self, + request: ContextRequest, + backends: BackendSet, + ) -> float: + """Return 0.0-1.0 confidence for this request. + + Returns 0.0 if required backends are missing or the request + is outside this strategy's scope. + """ + ... + + def assemble( + self, + request: ContextRequest, + backends: BackendSet, + budget: int, + plan_context: PlanContext, + ) -> list[ContextFragment]: + """Execute the strategy. Must respect the budget.""" + ... + + def explain(self) -> str: + """Return a human-readable explanation of this strategy.""" + ... + + +# --------------------------------------------------------------------------- +# StrategyConfig — per-strategy configuration +# --------------------------------------------------------------------------- + + +class StrategyConfig(BaseModel, frozen=True): + """Per-strategy configuration. + + Controls whether a strategy is enabled, its timeout, and fragment + limits. Based on spec §28706-28708 (pipeline executor config) and + issue #191 acceptance criteria. + """ + + enabled: bool = Field( + default=True, + description="Whether this strategy is enabled", + ) + timeout_seconds: int = Field( + default=30, + ge=1, + description="Timeout for this strategy's assembly call (spec §28706)", + ) + max_fragments: int = Field( + default=100, + ge=1, + description="Maximum fragments this strategy may return per call", + ) + max_workers: int = Field( + default=4, + ge=1, + description="Max parallel workers (spec §28707)", + ) + circuit_breaker_threshold: int = Field( + default=3, + ge=1, + description="Consecutive failures before circuit opens (spec §28708)", + ) + resource_types: tuple[str, ...] = Field( + default=(), + description="Resource types this strategy is limited to (empty = all)", + ) + extra: dict[str, Any] = Field( + default_factory=dict, + description="Strategy-specific extra configuration", + ) + + +# --------------------------------------------------------------------------- +# ContextStrategyResult — output of a strategy execution +# --------------------------------------------------------------------------- + + +class ContextStrategyResult(BaseModel, frozen=True): + """Result of a single strategy's execution. + + Provides fragments, execution statistics, and any errors + encountered. Fragments are stored in deterministic order: + ``(relevance_score DESC, uko_node ASC)``. + + Required by issue #191 acceptance criteria. + """ + + strategy_name: str = Field( + ..., + min_length=1, + description="Name of the strategy that produced this result", + ) + fragments: tuple[ContextFragment, ...] = Field( + default=(), + description="Fragments in deterministic order", + ) + total_fragments: int = Field( + default=0, + ge=0, + description="Total fragments produced (before any limit)", + ) + tokens_used: int = Field( + default=0, + ge=0, + description="Total tokens across all fragments", + ) + execution_time_ms: float = Field( + default=0.0, + ge=0.0, + description="Wall-clock execution time in milliseconds", + ) + errors: tuple[str, ...] = Field( + default=(), + description="Error messages encountered during execution", + ) + stats: dict[str, Any] = Field( + default_factory=dict, + description="Strategy-specific statistics", + ) + + @field_validator("fragments") + @classmethod + def _sort_fragments( + cls: type[ContextStrategyResult], + v: tuple[ContextFragment, ...], + ) -> tuple[ContextFragment, ...]: + """Ensure deterministic ordering: relevance DESC, uko_node ASC.""" + return tuple( + sorted(v, key=lambda f: (-f.relevance_score, f.uko_node)), + ) + + +# --------------------------------------------------------------------------- +# StrategyRegistryEntry — metadata wrapper for registered strategies +# --------------------------------------------------------------------------- + + +class StrategyRegistryEntry(BaseModel, frozen=True): + """Metadata wrapper for a strategy in the registry. + + Combines a strategy name with its resolved configuration. + The actual ``ContextStrategy`` instance is not stored here + (it's a protocol, not a Pydantic model); the registry holds + both this entry and the strategy instance separately. + """ + + name: str = Field( + ..., + min_length=1, + description="Unique strategy name", + ) + config: StrategyConfig = Field( + default_factory=StrategyConfig, + description="Per-strategy configuration", + ) + module_path: str = Field( + default="", + description=( + "Python module path for custom strategies " + "(e.g., 'my_package.strategies:MyStrategy')" + ), + ) + is_builtin: bool = Field( + default=False, + description="Whether this is a built-in strategy", + ) diff --git a/src/cleveragents/domain/models/acms/strategy_stubs.py b/src/cleveragents/domain/models/acms/strategy_stubs.py new file mode 100644 index 000000000..b8f4a2e76 --- /dev/null +++ b/src/cleveragents/domain/models/acms/strategy_stubs.py @@ -0,0 +1,394 @@ +"""Built-in stub context strategies for the ACMS. + +Each strategy implements the ``ContextStrategy`` protocol with no-op +defaults: ``can_handle`` returns the quality score when required backends +are present, and ``assemble`` returns an empty fragment list. + +These stubs are scaffolding for future real implementations. + +Based on ``docs/specification.md`` §25207-25216 (Built-in Strategies +table) and §43167-43199 (Built-in Strategy Catalogue). + +Six built-in strategies per spec: + +| Name | Quality | Backends Required | +|----------------------------|---------|---------------------------| +| ``simple-keyword`` | 0.3 | Text only | +| ``semantic-embedding`` | 0.6 | Vector | +| ``breadth-depth-navigator``| 0.85 | Graph | +| ``arce`` | 0.95 | All | +| ``temporal-archaeology`` | 0.5 | Graph + temporal/cold | +| ``plan-decision-context`` | 0.7 | Warm/cold tiers | +""" + +from __future__ import annotations + +from cleveragents.domain.models.acms.crp import ( + ContextFragment, + ContextRequest, +) +from cleveragents.domain.models.acms.strategy import ( + BackendSet, + PlanContext, + StrategyCapabilities, +) + +# --------------------------------------------------------------------------- +# simple-keyword (spec §25211, §43171-43173) +# --------------------------------------------------------------------------- + + +class SimpleKeywordStrategy: + """Basic keyword/regex text search. Universal fallback. + + Works with any backend, any resource type. No graph or vector + required. Quality score: 0.3. + + Spec: ``specification.md:25211``, ``specification.md:43171-43173``. + """ + + @property + def name(self) -> str: + return "simple-keyword" + + @property + def capabilities(self) -> StrategyCapabilities: + return StrategyCapabilities( + uses_text=True, + uses_vector=False, + uses_graph=False, + uses_temporal=False, + resource_types=("*",), + quality_score=0.3, + ) + + def can_handle( + self, + request: ContextRequest, + backends: BackendSet, + ) -> float: + """Returns quality score if text backend is available.""" + if backends.text is not None: + return self.capabilities.quality_score + return 0.0 + + def assemble( + self, + request: ContextRequest, + backends: BackendSet, + budget: int, + plan_context: PlanContext, + ) -> list[ContextFragment]: + """No-op stub — returns empty list.""" + return [] + + def explain(self) -> str: + return ( + "Basic keyword/regex text search. Universal fallback. " + "Works with any text backend." + ) + + +# --------------------------------------------------------------------------- +# semantic-embedding (spec §25212, §43175-43177) +# --------------------------------------------------------------------------- + + +class SemanticEmbeddingStrategy: + """Vector similarity search for semantically related content. + + Requires a vector backend. Quality score: 0.6. + + Spec: ``specification.md:25212``, ``specification.md:43175-43177``. + """ + + @property + def name(self) -> str: + return "semantic-embedding" + + @property + def capabilities(self) -> StrategyCapabilities: + return StrategyCapabilities( + uses_text=False, + uses_vector=True, + uses_graph=False, + uses_temporal=False, + resource_types=("*",), + quality_score=0.6, + ) + + def can_handle( + self, + request: ContextRequest, + backends: BackendSet, + ) -> float: + if backends.vector is not None: + return self.capabilities.quality_score + return 0.0 + + def assemble( + self, + request: ContextRequest, + backends: BackendSet, + budget: int, + plan_context: PlanContext, + ) -> list[ContextFragment]: + return [] + + def explain(self) -> str: + return ( + "Vector similarity search. Finds semantically related " + "content even without exact keyword matches. " + "Requires a vector backend." + ) + + +# --------------------------------------------------------------------------- +# breadth-depth-navigator (spec §25213, §43179-43181) +# --------------------------------------------------------------------------- + + +class BreadthDepthNavigatorStrategy: + """Graph-aware UKO traversal with depth/breadth projection. + + Primary strategy for code-aware context. Requires a graph backend. + Quality score: 0.85. + + Spec: ``specification.md:25213``, ``specification.md:43179-43181``. + """ + + @property + def name(self) -> str: + return "breadth-depth-navigator" + + @property + def capabilities(self) -> StrategyCapabilities: + return StrategyCapabilities( + uses_text=False, + uses_vector=False, + uses_graph=True, + uses_temporal=False, + resource_types=("*",), + supports_depth_breadth=True, + quality_score=0.85, + ) + + def can_handle( + self, + request: ContextRequest, + backends: BackendSet, + ) -> float: + if backends.graph is not None: + return self.capabilities.quality_score + return 0.0 + + def assemble( + self, + request: ContextRequest, + backends: BackendSet, + budget: int, + plan_context: PlanContext, + ) -> list[ContextFragment]: + return [] + + def explain(self) -> str: + return ( + "Graph-aware UKO traversal with depth/breadth projection. " + "Primary strategy for code-aware context. " + "Supports focus items, hop traversal, and detail depth gradients." + ) + + +# --------------------------------------------------------------------------- +# arce (spec §25214, §43183-43191) +# --------------------------------------------------------------------------- + + +class ARCEStrategy: + """Multi-modal pipeline combining text, vector, and graph. + + Autonomous Reasoning Context Extraction. Requires all backends. + Highest quality. Quality score: 0.95. + + Spec: ``specification.md:25214``, ``specification.md:43183-43191``. + """ + + @property + def name(self) -> str: + return "arce" + + @property + def capabilities(self) -> StrategyCapabilities: + return StrategyCapabilities( + uses_text=True, + uses_vector=True, + uses_graph=True, + uses_temporal=False, + resource_types=("*",), + supports_depth_breadth=True, + quality_score=0.95, + ) + + def can_handle( + self, + request: ContextRequest, + backends: BackendSet, + ) -> float: + if ( + backends.text is not None + and backends.vector is not None + and backends.graph is not None + ): + return self.capabilities.quality_score + return 0.0 + + def assemble( + self, + request: ContextRequest, + backends: BackendSet, + budget: int, + plan_context: PlanContext, + ) -> list[ContextFragment]: + return [] + + def explain(self) -> str: + return ( + "Multi-modal pipeline combining text, vector, and graph " + "with intent analysis. Autonomous Reasoning Context " + "Extraction (ARCE). Highest quality." + ) + + +# --------------------------------------------------------------------------- +# temporal-archaeology (spec §25215, §43193-43195) +# --------------------------------------------------------------------------- + + +class TemporalArchaeologyStrategy: + """Historical pattern discovery from past decisions and archived context. + + Requires graph backend and cold-tier access. Quality score: 0.5. + + Spec: ``specification.md:25215``, ``specification.md:43193-43195``. + """ + + @property + def name(self) -> str: + return "temporal-archaeology" + + @property + def capabilities(self) -> StrategyCapabilities: + return StrategyCapabilities( + uses_text=False, + uses_vector=False, + uses_graph=True, + uses_temporal=True, + resource_types=("*",), + supports_temporal=True, + quality_score=0.5, + ) + + def can_handle( + self, + request: ContextRequest, + backends: BackendSet, + ) -> float: + if backends.graph is not None: + return self.capabilities.quality_score + return 0.0 + + def assemble( + self, + request: ContextRequest, + backends: BackendSet, + budget: int, + plan_context: PlanContext, + ) -> list[ContextFragment]: + return [] + + def explain(self) -> str: + return ( + "Historical pattern discovery from past decisions and " + "archived context. Searches the cold tier for temporal " + "patterns." + ) + + +# --------------------------------------------------------------------------- +# plan-decision-context (spec §25216, §43197-43199) +# --------------------------------------------------------------------------- + + +class PlanDecisionContextStrategy: + """Retrieves context from parent/ancestor plan decisions. + + How child plans 'remember' what their parent decided and why. + Operates on warm/cold tiers. Quality score: 0.7. + + Spec: ``specification.md:25216``, ``specification.md:43197-43199``. + """ + + @property + def name(self) -> str: + return "plan-decision-context" + + @property + def capabilities(self) -> StrategyCapabilities: + return StrategyCapabilities( + uses_text=False, + uses_vector=False, + uses_graph=False, + uses_temporal=True, + resource_types=("*",), + supports_plan_hierarchy=True, + quality_score=0.7, + ) + + def can_handle( + self, + request: ContextRequest, + backends: BackendSet, + ) -> float: + # Works even without graph; reads from warm/cold tiers. + return self.capabilities.quality_score + + def assemble( + self, + request: ContextRequest, + backends: BackendSet, + budget: int, + plan_context: PlanContext, + ) -> list[ContextFragment]: + return [] + + def explain(self) -> str: + return ( + "Retrieves context from parent and ancestor plan " + "decisions. This is how child plans 'remember' what " + "their parent decided and why." + ) + + +# --------------------------------------------------------------------------- +# Registry helper — register all built-in strategies +# --------------------------------------------------------------------------- + +# Canonical ordered list per spec §28682 default: +# ["simple-keyword", "semantic-embedding", "breadth-depth-navigator"] +# Plus the remaining 3 registered but not enabled by default. + +BUILTIN_STRATEGY_CLASSES: tuple[type, ...] = ( + SimpleKeywordStrategy, + SemanticEmbeddingStrategy, + BreadthDepthNavigatorStrategy, + ARCEStrategy, + TemporalArchaeologyStrategy, + PlanDecisionContextStrategy, +) + +# Spec §28682 default enabled list +DEFAULT_ENABLED_STRATEGIES: tuple[str, ...] = ( + "simple-keyword", + "semantic-embedding", + "breadth-depth-navigator", +) diff --git a/vulture_whitelist.py b/vulture_whitelist.py index 183c373e8..0bb07a73a 100644 --- a/vulture_whitelist.py +++ b/vulture_whitelist.py @@ -620,6 +620,9 @@ source_project # noqa: B018, F821 target_project # noqa: B018, F821 dependency_type # noqa: B018, F821 +# Context Strategy Registry — protocol parameter required by ContextStrategy.assemble() +plan_context # noqa: B018, F821 + # ACMS Backend Abstraction Layer — public API (issue #498) TextBackend # noqa: B018, F821 VectorBackend # noqa: B018, F821 -- 2.52.0 From b7effcafc18ed6f1284e4933e695fb2a1f1fbc9e Mon Sep 17 00:00:00 2001 From: Hamza Khyari Date: Thu, 5 Mar 2026 19:56:49 +0000 Subject: [PATCH 2/2] fix(acms): address PR #565 review findings from CoreRasurae MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve 14 review findings across bugs, spec deviations, design gaps, security, performance, thread safety, and test quality: - Fix register() guard ordering: isinstance check before attribute access - Add BackendSet.temporal field for cold-tier backend availability - Fix temporal-archaeology/plan-decision-context can_handle to require temporal backend (spec §43193-43199) - Add threading.RLock to StrategyRegistry for thread safety - Add allowed_module_prefixes (CWE-706) to register_from_module - Change StrategyConfig.extra and ContextStrategyResult.stats to MappingProxyType with field_validator coercion (ADR-004 immutability) - Switch 6 stub capabilities from @property to @functools.cached_property - Add resource_types/extra params to update_config() - Add inject_stale_enabled_entry() public test helper - Rename colliding step patterns to avoid AmbiguousStep with master - Add scenarios: non-protocol without name, concurrent threads, temporal backend regression tests - Document spec §25223 vs §28682 conflict and colon vs dot notation --- features/context_strategy_registry.feature | 105 +++++- .../steps/context_strategy_registry_steps.py | 217 +++++++++++- .../application/services/strategy_registry.py | 316 ++++++++++++------ .../domain/models/acms/strategy.py | 63 +++- .../domain/models/acms/strategy_stubs.py | 40 ++- vulture_whitelist.py | 1 + 6 files changed, 601 insertions(+), 141 deletions(-) diff --git a/features/context_strategy_registry.feature b/features/context_strategy_registry.feature index 56ad9ba56..e4f22151a 100644 --- a/features/context_strategy_registry.feature +++ b/features/context_strategy_registry.feature @@ -33,7 +33,7 @@ Feature: Context Strategy Registry Scenario: Register a single strategy Given an empty strategy registry When I register the "simple-keyword" built-in strategy - Then the registry should contain "simple-keyword" + Then the strategy registry should contain "simple-keyword" And the registry size should be 1 @strategy_registry @@ -41,12 +41,12 @@ Feature: Context Strategy Registry Given an empty strategy registry When I register all 6 built-in strategies Then the registry should contain 6 strategies - And the registry should contain "simple-keyword" - And the registry should contain "semantic-embedding" - And the registry should contain "breadth-depth-navigator" - And the registry should contain "arce" - And the registry should contain "temporal-archaeology" - And the registry should contain "plan-decision-context" + And the strategy registry should contain "simple-keyword" + And the strategy registry should contain "semantic-embedding" + And the strategy registry should contain "breadth-depth-navigator" + And the strategy registry should contain "arce" + And the strategy registry should contain "temporal-archaeology" + And the strategy registry should contain "plan-decision-context" @strategy_registry @error_handling Scenario: Reject duplicate strategy registration @@ -98,6 +98,12 @@ Feature: Context Strategy Registry When I set the enabled list to "simple-keyword", "arce" Then the enabled strategies should be "simple-keyword", "arce" + @strategy_config + Scenario: Set enabled list deduplicates duplicate names + Given a registry with all built-in strategies using default enabled list + When I set the enabled list to "simple-keyword", "arce", "simple-keyword" + Then the enabled strategies should be "simple-keyword", "arce" + @strategy_config @error_handling Scenario: Set enabled list with unknown strategy raises error Given a registry with all built-in strategies using default enabled list @@ -163,8 +169,8 @@ Feature: Context Strategy Registry Then "arce" can_handle should return 0.0 @strategy_can_handle - Scenario: plan-decision-context always returns quality score - Given a BackendSet with no backends + Scenario: plan-decision-context returns quality score with temporal backend + Given a BackendSet with temporal backend only And a default ContextRequest Then "plan-decision-context" can_handle should return 0.7 @@ -230,7 +236,7 @@ Feature: Context Strategy Registry Scenario: Register a strategy from a module path Given an empty strategy registry When I register a strategy from module "cleveragents.domain.models.acms.strategy_stubs:SimpleKeywordStrategy" - Then the registry should contain "simple-keyword" + Then the strategy registry should contain "simple-keyword" And the registry size should be 1 @strategy_registry @plugin @error_handling @@ -314,7 +320,7 @@ Feature: Context Strategy Registry Given a registry with all built-in strategies When I unregister "temporal-archaeology" Then the registry should contain 5 strategies - And the registry should not contain "temporal-archaeology" + And the strategy registry should not contain "temporal-archaeology" @strategy_registry Scenario: Clear the registry @@ -382,7 +388,7 @@ Feature: Context Strategy Registry Scenario: register_from_module uses the provided name parameter Given an empty strategy registry When I register a strategy named "custom-search" from module "cleveragents.domain.models.acms.strategy_stubs:SimpleKeywordStrategy" - Then the registry should contain "custom-search" + Then the strategy registry should contain "custom-search" And the registry size should be 1 @strategy_config @regression @@ -398,6 +404,13 @@ Feature: Context Strategy Registry When I attempt to register a non-protocol object Then a StrategyRegistrationError should be raised + @strategy_registry @error_handling + Scenario: Reject non-protocol object without explicit name + Given an empty strategy registry + When I attempt to register a non-protocol object without a name + Then a StrategyRegistrationError should be raised + And the error message should contain "does not satisfy" + # --------------------------------------------------------------------------- # explain() coverage # --------------------------------------------------------------------------- @@ -424,8 +437,8 @@ Feature: Context Strategy Registry Then "breadth-depth-navigator" can_handle should return 0.0 @strategy_can_handle - Scenario: temporal-archaeology returns quality score with graph backend - Given a BackendSet with graph backend only + Scenario: temporal-archaeology returns quality score with graph and temporal backends + Given a BackendSet with graph and temporal backends And a default ContextRequest Then "temporal-archaeology" can_handle should return 0.5 @@ -452,3 +465,67 @@ Feature: Context Strategy Registry Given a registry with all built-in strategies When I attempt to update config for "simple-keyword" with max_fragments -1 Then a Pydantic ValidationError should be raised + + # --------------------------------------------------------------------------- + # update_config: resource_types and extra parameters + # --------------------------------------------------------------------------- + + @strategy_config @regression + Scenario: update_config with resource_types + Given a registry with all built-in strategies + When I update config for "simple-keyword" with resource_types "code", "doc" + Then the config for "simple-keyword" should have resource_types "code", "doc" + + @strategy_config @regression + Scenario: update_config with extra dict + Given a registry with all built-in strategies + When I update config for "simple-keyword" with extra key "boost" value "2" + Then the config for "simple-keyword" extra should contain key "boost" + And the config for "simple-keyword" extra should be immutable + + # --------------------------------------------------------------------------- + # MappingProxyType coercion validators + # --------------------------------------------------------------------------- + + @strategy_config @boundary + Scenario: StrategyConfig accepts plain dict for extra field + When I create a StrategyConfig with extra as a plain dict + Then the extra field should be a MappingProxyType + + @strategy_config @boundary + Scenario: StrategyConfig rejects non-dict non-MappingProxyType extra + When I attempt to create a StrategyConfig with extra as an integer + Then a Pydantic ValidationError should be raised + + @strategy_result @boundary + Scenario: ContextStrategyResult accepts plain dict for stats field + When I create a ContextStrategyResult with stats as a plain dict + Then the stats field should be a MappingProxyType + + # --------------------------------------------------------------------------- + # Thread safety + # --------------------------------------------------------------------------- + + @strategy_registry @thread_safety + Scenario: Concurrent register and list_enabled do not raise + Given a registry with all built-in strategies + When 10 threads concurrently register and query the registry + Then no thread should have raised an exception + And the registry should contain at least 10 strategies + And every thread-registered strategy should be present + + # --------------------------------------------------------------------------- + # temporal/cold-tier can_handle regression + # --------------------------------------------------------------------------- + + @strategy_can_handle @regression + Scenario: temporal-archaeology returns 0 without temporal backend + Given a BackendSet with graph backend only + And a default ContextRequest + Then "temporal-archaeology" can_handle should return 0.0 + + @strategy_can_handle @regression + Scenario: plan-decision-context returns 0 without temporal backend + Given a BackendSet with no backends + And a default ContextRequest + Then "plan-decision-context" can_handle should return 0.0 diff --git a/features/steps/context_strategy_registry_steps.py b/features/steps/context_strategy_registry_steps.py index b9dc0cac5..8bcffc0d7 100644 --- a/features/steps/context_strategy_registry_steps.py +++ b/features/steps/context_strategy_registry_steps.py @@ -2,6 +2,8 @@ from __future__ import annotations +from types import MappingProxyType + from behave import given, then, when from behave.runner import Context from pydantic import ValidationError @@ -186,8 +188,8 @@ def step_given_stale_enabled(context: Context) -> None: """Create a registry where the enabled list references a name not registered.""" context.registry = StrategyRegistry() _register_all_builtins(context.registry) - # Manually inject a stale name into the internal enabled list - context.registry._enabled_order.append("ghost-strategy") + # Use the public test helper to inject a stale entry + context.registry.inject_stale_enabled_entry("ghost-strategy") @given("a registry with a strategy that declares no resource types") @@ -472,14 +474,14 @@ def step_then_quality_score(context: Context, name: str, score: float) -> None: ) -@then('the registry should contain "{name}"') +@then('the strategy registry should contain "{name}"') def step_then_contains(context: Context, name: str) -> None: assert context.registry.is_registered(name), ( f"'{name}' not in registry: {context.registry.list_all()}" ) -@then('the registry should not contain "{name}"') +@then('the strategy registry should not contain "{name}"') def step_then_not_contains(context: Context, name: str) -> None: assert not context.registry.is_registered(name), ( f"'{name}' unexpectedly in registry" @@ -738,6 +740,20 @@ def step_when_register_non_protocol(context: Context) -> None: context.error = exc +@when("I attempt to register a non-protocol object without a name") +def step_when_register_non_protocol_no_name(context: Context) -> None: + try: + # No name= override — verifies guard ordering: isinstance check + # must happen before strategy.name access (bug 1.1 fix). + context.registry.register(object()) # type: ignore[arg-type] + context.error = None + except StrategyRegistrationError as exc: + context.error = exc + except AttributeError as exc: # pragma: no cover + # This would indicate the bug is still present + context.error = exc + + @then("every strategy should return a non-empty explain string") def step_then_explain_nonempty(context: Context) -> None: for name, strategy in context.strategies.items(): @@ -807,3 +823,196 @@ def step_when_update_config_bad_fragments( context.error = None except ValidationError as exc: context.error = exc + + +# --------------------------------------------------------------------------- +# Thread safety steps +# --------------------------------------------------------------------------- + + +@when("10 threads concurrently register and query the registry") +def step_when_concurrent_register_query(context: Context) -> None: + """Exercise the registry from multiple threads simultaneously.""" + import threading as _threading + + errors: list[Exception] = [] + barrier = _threading.Barrier(10) + + def _worker(idx: int) -> None: + try: + barrier.wait(timeout=5) + # Mix of reads and writes + if idx % 3 == 0: + # Write: register a unique strategy + name = f"thread-strategy-{idx}" + + class _Stub: + @property + def name(self) -> str: + return name + + @property + def capabilities(self) -> StrategyCapabilities: + return StrategyCapabilities( + uses_text=True, resource_types=("*",), quality_score=0.1 + ) + + def can_handle( + self, request: ContextRequest, backends: BackendSet + ) -> float: + return 0.1 + + def assemble( + self, + request: ContextRequest, + backends: BackendSet, + budget: int, + plan_context: PlanContext, + ) -> list[ContextFragment]: + return [] + + def explain(self) -> str: + return f"thread stub {idx}" + + context.registry.register(_Stub()) + elif idx % 3 == 1: + # Read: list enabled + context.registry.list_enabled() + else: + # Read: list all + context.registry.list_all() + except Exception as exc: + errors.append(exc) + + threads = [_threading.Thread(target=_worker, args=(i,)) for i in range(10)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=10) + + context.thread_errors = errors + + +@then("no thread should have raised an exception") +def step_then_no_thread_errors(context: Context) -> None: + assert not context.thread_errors, f"Thread errors: {context.thread_errors}" + + +@then("the registry should contain at least {count:d} strategies") +def step_then_at_least_count(context: Context, count: int) -> None: + actual = len(context.registry) + assert actual >= count, f"Expected at least {count}, got {actual}" + + +@then("every thread-registered strategy should be present") +def step_then_thread_strategies_present(context: Context) -> None: + for idx in [0, 3, 6, 9]: + name = f"thread-strategy-{idx}" + assert context.registry.is_registered(name), ( + f"Thread-registered '{name}' missing from registry" + ) + + +# --------------------------------------------------------------------------- +# Temporal/cold-tier backend steps +# --------------------------------------------------------------------------- + + +# --------------------------------------------------------------------------- +# update_config: resource_types and extra steps +# --------------------------------------------------------------------------- + + +@when('I update config for "{name}" with resource_types "{rt1}", "{rt2}"') +def step_when_update_resource_types( + context: Context, name: str, rt1: str, rt2: str +) -> None: + context.registry.update_config(name, resource_types=(rt1, rt2)) + + +@then('the config for "{name}" should have resource_types "{rt1}", "{rt2}"') +def step_then_resource_types(context: Context, name: str, rt1: str, rt2: str) -> None: + cfg = context.registry.get_config(name) + assert cfg.resource_types == (rt1, rt2), ( + f"Expected ({rt1!r}, {rt2!r}), got {cfg.resource_types}" + ) + + +@when('I update config for "{name}" with extra key "{key}" value "{val}"') +def step_when_update_extra(context: Context, name: str, key: str, val: str) -> None: + context.registry.update_config(name, extra={key: val}) + + +@then('the config for "{name}" extra should contain key "{key}"') +def step_then_extra_contains(context: Context, name: str, key: str) -> None: + cfg = context.registry.get_config(name) + assert key in cfg.extra, f"Key '{key}' not in extra: {cfg.extra}" + + +@then('the config for "{name}" extra should be immutable') +def step_then_extra_immutable(context: Context, name: str) -> None: + cfg = context.registry.get_config(name) + assert isinstance(cfg.extra, MappingProxyType), ( + f"Expected MappingProxyType, got {type(cfg.extra).__name__}" + ) + try: + cfg.extra["hack"] = "should fail" # type: ignore[index] + raise AssertionError("MappingProxyType should reject item assignment") + except TypeError: + pass # Expected — MappingProxyType is immutable + + +# --------------------------------------------------------------------------- +# MappingProxyType coercion validator steps +# --------------------------------------------------------------------------- + + +@when("I create a StrategyConfig with extra as a plain dict") +def step_when_config_plain_dict_extra(context: Context) -> None: + context.created_config = StrategyConfig(extra={"key": "val"}) # type: ignore[arg-type] + + +@then("the extra field should be a MappingProxyType") +def step_then_extra_is_mapping_proxy(context: Context) -> None: + assert isinstance(context.created_config.extra, MappingProxyType), ( + f"Expected MappingProxyType, got {type(context.created_config.extra).__name__}" + ) + + +@when("I attempt to create a StrategyConfig with extra as an integer") +def step_when_config_int_extra(context: Context) -> None: + try: + StrategyConfig(extra=42) # type: ignore[arg-type] + context.error = None + except ValidationError as exc: + context.error = exc + + +@when("I create a ContextStrategyResult with stats as a plain dict") +def step_when_result_plain_dict_stats(context: Context) -> None: + context.created_result = ContextStrategyResult( + strategy_name="test", + stats={"ops": 10}, # type: ignore[arg-type] + ) + + +@then("the stats field should be a MappingProxyType") +def step_then_stats_is_mapping_proxy(context: Context) -> None: + assert isinstance(context.created_result.stats, MappingProxyType), ( + f"Expected MappingProxyType, got {type(context.created_result.stats).__name__}" + ) + + +@given("a BackendSet with graph and temporal backends") +def step_given_backends_graph_temporal(context: Context) -> None: + context.backend_set = BackendSet( + graph=InMemoryGraphBackend(), + temporal=object(), # Presence indicates availability + ) + + +@given("a BackendSet with temporal backend only") +def step_given_backends_temporal_only(context: Context) -> None: + context.backend_set = BackendSet( + temporal=object(), # Presence indicates availability + ) diff --git a/src/cleveragents/application/services/strategy_registry.py b/src/cleveragents/application/services/strategy_registry.py index 425000e9f..3998c2cac 100644 --- a/src/cleveragents/application/services/strategy_registry.py +++ b/src/cleveragents/application/services/strategy_registry.py @@ -17,6 +17,9 @@ Based on ``docs/specification.md`` §§ 25218-25233, 28682-28708, from __future__ import annotations import importlib +import threading +from types import MappingProxyType +from typing import Any import structlog @@ -74,11 +77,39 @@ class StrategyRegistry: result = registry.get("simple-keyword") """ - def __init__(self) -> None: - """Initialize an empty registry.""" + # INVARIANTS (must hold after every public method returns): + # 1. _strategies.keys() == _entries.keys() + # 2. set(_enabled_order) is a subset of _strategies.keys() + # 3. For all n in _enabled_order: _entries[n].config.enabled is True + # 4. For all n not in _enabled_order: _entries[n].config.enabled is False + + # Default module prefix allowlist for register_from_module(). + # Only modules under these prefixes may be dynamically imported. + # Override via the ``allowed_module_prefixes`` constructor parameter. + DEFAULT_ALLOWED_MODULE_PREFIXES: tuple[str, ...] = ("cleveragents.",) + + def __init__( + self, + *, + allowed_module_prefixes: tuple[str, ...] | None = None, + ) -> None: + """Initialize an empty registry. + + Args: + allowed_module_prefixes: Module prefixes permitted for + dynamic import via :meth:`register_from_module`. + Defaults to ``("cleveragents.",)``. Pass an empty + tuple to disable the allowlist (not recommended). + """ + self._lock = threading.RLock() self._strategies: dict[str, ContextStrategy] = {} self._entries: dict[str, StrategyRegistryEntry] = {} self._enabled_order: list[str] = [] + self._allowed_module_prefixes = ( + allowed_module_prefixes + if allowed_module_prefixes is not None + else self.DEFAULT_ALLOWED_MODULE_PREFIXES + ) # ------------------------------------------------------------------ # Registration @@ -110,32 +141,38 @@ class StrategyRegistry: already registered, or the strategy doesn't satisfy the ``ContextStrategy`` protocol. """ + # Validate protocol conformance FIRST — before accessing any + # attributes (e.g., strategy.name) that a non-protocol object + # may not have. + if not isinstance(strategy, ContextStrategy): + label = name or type(strategy).__name__ + raise StrategyRegistrationError( + f"Strategy '{label}' does not satisfy the ContextStrategy protocol" + ) + name = name or strategy.name if not name: raise StrategyRegistrationError("Strategy name must not be empty") - if name in self._strategies: - raise StrategyRegistrationError(f"Strategy '{name}' is already registered") + with self._lock: + if name in self._strategies: + raise StrategyRegistrationError( + f"Strategy '{name}' is already registered" + ) - # Validate protocol conformance - if not isinstance(strategy, ContextStrategy): - raise StrategyRegistrationError( - f"Strategy '{name}' does not satisfy the ContextStrategy protocol" + resolved_config = config or StrategyConfig() + entry = StrategyRegistryEntry( + name=name, + config=resolved_config, + module_path=module_path, + is_builtin=is_builtin, ) - resolved_config = config or StrategyConfig() - entry = StrategyRegistryEntry( - name=name, - config=resolved_config, - module_path=module_path, - is_builtin=is_builtin, - ) + self._strategies[name] = strategy + self._entries[name] = entry - self._strategies[name] = strategy - self._entries[name] = entry - - if resolved_config.enabled: - self._enabled_order.append(name) + if resolved_config.enabled: + self._enabled_order.append(name) logger.debug( "strategy.registered", @@ -156,6 +193,16 @@ class StrategyRegistry: Performs plugin discovery per spec §25225-25226: ``[context.strategies.custom] "name" = "module:Class"``. + Note: + Spec §25226 uses dot notation + (``my_package.strategies.DomainStrategy``) while §25232 + uses colon notation + (``my_extensions.scorers:DomainAwareScorer``). + We use **colon notation** (``module:ClassName``) because + it unambiguously separates the importable module from the + class name, matching Python entry-point conventions. + The spec is internally inconsistent here. + Security: Module paths should come from trusted configuration only (e.g., project TOML files). Importing a module executes its @@ -170,9 +217,10 @@ class StrategyRegistry: config: Per-strategy configuration. Raises: - StrategyRegistrationError: If import fails, the class - doesn't exist, or the instance doesn't satisfy the - ``ContextStrategy`` protocol. + StrategyRegistrationError: If the module prefix is not in + the allowlist, import fails, the class doesn't exist, + or the instance doesn't satisfy the ``ContextStrategy`` + protocol. """ if ":" not in module_path: raise StrategyRegistrationError( @@ -182,6 +230,17 @@ class StrategyRegistry: module_name, class_name = module_path.rsplit(":", 1) + # CWE-706: Restrict dynamic imports to an allowlist of trusted + # module prefixes to prevent arbitrary code execution. + if self._allowed_module_prefixes and not any( + module_name.startswith(prefix) for prefix in self._allowed_module_prefixes + ): + raise StrategyRegistrationError( + f"Module '{module_name}' is not in the allowed prefix list: " + f"{self._allowed_module_prefixes}. Only modules under these " + f"prefixes may be dynamically imported." + ) + try: module = importlib.import_module(module_name) except ImportError as exc: @@ -226,12 +285,13 @@ class StrategyRegistry: Raises: StrategyNotFoundError: If the name is not registered. """ - if name not in self._strategies: - raise StrategyNotFoundError( - f"Strategy '{name}' not found in registry. " - f"Available: {sorted(self._strategies)}" - ) - return self._strategies[name] + with self._lock: + if name not in self._strategies: + raise StrategyNotFoundError( + f"Strategy '{name}' not found in registry. " + f"Available: {sorted(self._strategies)}" + ) + return self._strategies[name] def get_entry(self, name: str) -> StrategyRegistryEntry: """Return the registry entry (metadata + config) for a strategy. @@ -239,9 +299,10 @@ class StrategyRegistry: Raises: StrategyNotFoundError: If the name is not registered. """ - if name not in self._entries: - raise StrategyNotFoundError(f"Strategy '{name}' not found in registry") - return self._entries[name] + with self._lock: + if name not in self._entries: + raise StrategyNotFoundError(f"Strategy '{name}' not found in registry") + return self._entries[name] def get_config(self, name: str) -> StrategyConfig: """Return the configuration for a registered strategy. @@ -253,7 +314,8 @@ class StrategyRegistry: def list_all(self) -> list[str]: """Return names of all registered strategies.""" - return sorted(self._strategies) + with self._lock: + return sorted(self._strategies) def list_enabled(self) -> list[str]: """Return names of enabled strategies in registration order. @@ -261,27 +323,32 @@ class StrategyRegistry: The order matches the ``[context.strategies] enabled`` list, which controls priority during selection (spec §28682). """ - return [ - n - for n in self._enabled_order - if n in self._entries and self._entries[n].config.enabled - ] + with self._lock: + return [ + n + for n in self._enabled_order + if n in self._entries and self._entries[n].config.enabled + ] def list_builtin(self) -> list[str]: """Return names of built-in strategies.""" - return sorted(n for n, e in self._entries.items() if e.is_builtin) + with self._lock: + return sorted(n for n, e in self._entries.items() if e.is_builtin) def is_registered(self, name: str) -> bool: """Check whether a strategy is registered.""" - return name in self._strategies + with self._lock: + return name in self._strategies def __len__(self) -> int: """Return the number of registered strategies.""" - return len(self._strategies) + with self._lock: + return len(self._strategies) - def __contains__(self, name: str) -> bool: + def __contains__(self, name: object) -> bool: """Check whether a strategy name is registered.""" - return name in self._strategies + with self._lock: + return name in self._strategies # ------------------------------------------------------------------ # Configuration @@ -303,23 +370,26 @@ class StrategyRegistry: Raises: StrategyNotFoundError: If any name is not registered. """ - for name in names: - if name not in self._strategies: - raise StrategyNotFoundError(f"Cannot enable unknown strategy '{name}'") + with self._lock: + for name in names: + if name not in self._strategies: + raise StrategyNotFoundError( + f"Cannot enable unknown strategy '{name}'" + ) - # Update config.enabled for all strategies - enabled_set = set(names) - for sname, entry in self._entries.items(): - should_enable = sname in enabled_set - if entry.config.enabled != should_enable: - new_config = entry.config.model_copy( - update={"enabled": should_enable}, - ) - self._entries[sname] = entry.model_copy( - update={"config": new_config}, - ) + # Update config.enabled for all strategies + enabled_set = set(names) + for sname, entry in self._entries.items(): + should_enable = sname in enabled_set + if entry.config.enabled != should_enable: + new_config = entry.config.model_copy( + update={"enabled": should_enable}, + ) + self._entries[sname] = entry.model_copy( + update={"config": new_config}, + ) - self._enabled_order = list(names) + self._enabled_order = list(dict.fromkeys(names)) def update_config( self, @@ -330,6 +400,8 @@ class StrategyRegistry: max_fragments: int | None = None, max_workers: int | None = None, circuit_breaker_threshold: int | None = None, + resource_types: tuple[str, ...] | None = None, + extra: dict[str, Any] | None = None, ) -> None: """Update configuration for a registered strategy. @@ -343,35 +415,42 @@ class StrategyRegistry: max_fragments: Max fragments per call (>=1). max_workers: Max parallel workers (>=1). circuit_breaker_threshold: Failures before circuit opens (>=1). + resource_types: Resource types this strategy is limited to. + extra: Strategy-specific extra configuration. Raises: StrategyNotFoundError: If the name is not registered. pydantic.ValidationError: If the updated values violate ``StrategyConfig`` constraints (e.g., ``timeout_seconds < 1``). """ - entry = self.get_entry(name) - updates: dict[str, bool | int] = {} - if enabled is not None: - updates["enabled"] = enabled - if timeout_seconds is not None: - updates["timeout_seconds"] = timeout_seconds - if max_fragments is not None: - updates["max_fragments"] = max_fragments - if max_workers is not None: - updates["max_workers"] = max_workers - if circuit_breaker_threshold is not None: - updates["circuit_breaker_threshold"] = circuit_breaker_threshold - merged = entry.config.model_dump() - merged.update(updates) - new_config = StrategyConfig.model_validate(merged) - new_entry = entry.model_copy(update={"config": new_config}) - self._entries[name] = new_entry + with self._lock: + entry = self.get_entry(name) + updates: dict[str, Any] = {} + if enabled is not None: + updates["enabled"] = enabled + if timeout_seconds is not None: + updates["timeout_seconds"] = timeout_seconds + if max_fragments is not None: + updates["max_fragments"] = max_fragments + if max_workers is not None: + updates["max_workers"] = max_workers + if circuit_breaker_threshold is not None: + updates["circuit_breaker_threshold"] = circuit_breaker_threshold + if resource_types is not None: + updates["resource_types"] = resource_types + if extra is not None: + updates["extra"] = MappingProxyType(extra) + merged = entry.config.model_dump() + merged.update(updates) + new_config = StrategyConfig.model_validate(merged) + new_entry = entry.model_copy(update={"config": new_config}) + self._entries[name] = new_entry - # Keep _enabled_order in sync with the enabled flag - if enabled is True and name not in self._enabled_order: - self._enabled_order.append(name) - elif enabled is False and name in self._enabled_order: - self._enabled_order = [n for n in self._enabled_order if n != name] + # Keep _enabled_order in sync with the enabled flag + if enabled is True and name not in self._enabled_order: + self._enabled_order.append(name) + elif enabled is False and name in self._enabled_order: + self._enabled_order = [n for n in self._enabled_order if n != name] # ------------------------------------------------------------------ # Validation @@ -389,31 +468,34 @@ class StrategyRegistry: Returns: List of warning messages (empty = valid). """ - warnings: list[str] = [] + with self._lock: + warnings: list[str] = [] - for name in self._enabled_order: - if name not in self._strategies: - warnings.append(f"Enabled strategy '{name}' is not registered") + for name in self._enabled_order: + if name not in self._strategies: + warnings.append(f"Enabled strategy '{name}' is not registered") - for name, strategy in self._strategies.items(): - caps = strategy.capabilities + for name, strategy in self._strategies.items(): + caps = strategy.capabilities - has_any_backend = ( - caps.uses_text - or caps.uses_vector - or caps.uses_graph - or caps.uses_temporal - ) - if not has_any_backend: - warnings.append(f"Strategy '{name}' declares no backend capabilities") - - if not caps.resource_types: - warnings.append( - f"Strategy '{name}' does not declare supported " - f"resource types (capabilities.resource_types is empty)" + has_any_backend = ( + caps.uses_text + or caps.uses_vector + or caps.uses_graph + or caps.uses_temporal ) + if not has_any_backend: + warnings.append( + f"Strategy '{name}' declares no backend capabilities" + ) - return warnings + if not caps.resource_types: + warnings.append( + f"Strategy '{name}' does not declare supported " + f"resource types (capabilities.resource_types is empty)" + ) + + return warnings # ------------------------------------------------------------------ # Removal (for testing / reconfiguration) @@ -428,15 +510,33 @@ class StrategyRegistry: Raises: StrategyNotFoundError: If the name is not registered. """ - if name not in self._strategies: - raise StrategyNotFoundError(f"Cannot unregister unknown strategy '{name}'") + with self._lock: + if name not in self._strategies: + raise StrategyNotFoundError( + f"Cannot unregister unknown strategy '{name}'" + ) - del self._strategies[name] - del self._entries[name] - self._enabled_order = [n for n in self._enabled_order if n != name] + del self._strategies[name] + del self._entries[name] + self._enabled_order = [n for n in self._enabled_order if n != name] def clear(self) -> None: """Remove all registered strategies.""" - self._strategies.clear() - self._entries.clear() - self._enabled_order.clear() + with self._lock: + self._strategies.clear() + self._entries.clear() + self._enabled_order.clear() + + def inject_stale_enabled_entry(self, name: str) -> None: + """Inject a stale name into the enabled list for testing. + + This is a test helper that creates a deliberately inconsistent + state where the enabled list references a name that is not + registered. Used to verify that :meth:`validate_registry` and + :meth:`list_enabled` handle stale entries correctly. + + Args: + name: A strategy name that is NOT currently registered. + """ + with self._lock: + self._enabled_order.append(name) diff --git a/src/cleveragents/domain/models/acms/strategy.py b/src/cleveragents/domain/models/acms/strategy.py index f8e132cc8..832db0675 100644 --- a/src/cleveragents/domain/models/acms/strategy.py +++ b/src/cleveragents/domain/models/acms/strategy.py @@ -13,6 +13,7 @@ All frozen Pydantic models follow ADR-004 immutability rules. from __future__ import annotations +from types import MappingProxyType from typing import Any, Protocol, runtime_checkable from pydantic import BaseModel, ConfigDict, Field, field_validator @@ -27,6 +28,16 @@ from cleveragents.domain.models.acms.crp import ( ContextRequest, ) +__all__ = [ + "BackendSet", + "ContextStrategy", + "ContextStrategyResult", + "PlanContext", + "StrategyCapabilities", + "StrategyConfig", + "StrategyRegistryEntry", +] + # --------------------------------------------------------------------------- # BackendSet — container for available data backends # --------------------------------------------------------------------------- @@ -54,6 +65,14 @@ class BackendSet(BaseModel, frozen=True): default=None, description="Knowledge graph backend (e.g., Blazegraph, Neo4j)", ) + temporal: object | None = Field( + default=None, + description=( + "Temporal/cold-tier backend for historical pattern discovery. " + "Typed as ``object`` until a formal TemporalBackend protocol " + "is defined; presence (not ``None``) indicates availability." + ), + ) model_config = ConfigDict(arbitrary_types_allowed=True) @@ -241,11 +260,27 @@ class StrategyConfig(BaseModel, frozen=True): default=(), description="Resource types this strategy is limited to (empty = all)", ) - extra: dict[str, Any] = Field( - default_factory=dict, - description="Strategy-specific extra configuration", + extra: MappingProxyType[str, Any] = Field( + default_factory=lambda: MappingProxyType({}), + description="Strategy-specific extra configuration (immutable)", ) + model_config = ConfigDict(arbitrary_types_allowed=True) + + @field_validator("extra", mode="before") + @classmethod + def _coerce_extra( + cls: type[StrategyConfig], + v: Any, + ) -> MappingProxyType[str, Any]: + """Wrap plain dicts in MappingProxyType for immutability (ADR-004).""" + if isinstance(v, MappingProxyType): + return v + if isinstance(v, dict): + return MappingProxyType(v) + msg = f"expected dict or MappingProxyType, got {type(v).__name__}" + raise ValueError(msg) + # --------------------------------------------------------------------------- # ContextStrategyResult — output of a strategy execution @@ -290,11 +325,13 @@ class ContextStrategyResult(BaseModel, frozen=True): default=(), description="Error messages encountered during execution", ) - stats: dict[str, Any] = Field( - default_factory=dict, - description="Strategy-specific statistics", + stats: MappingProxyType[str, Any] = Field( + default_factory=lambda: MappingProxyType({}), + description="Strategy-specific statistics (immutable)", ) + model_config = ConfigDict(arbitrary_types_allowed=True) + @field_validator("fragments") @classmethod def _sort_fragments( @@ -306,6 +343,20 @@ class ContextStrategyResult(BaseModel, frozen=True): sorted(v, key=lambda f: (-f.relevance_score, f.uko_node)), ) + @field_validator("stats", mode="before") + @classmethod + def _coerce_stats( + cls: type[ContextStrategyResult], + v: Any, + ) -> MappingProxyType[str, Any]: + """Wrap plain dicts in MappingProxyType for immutability (ADR-004).""" + if isinstance(v, MappingProxyType): + return v + if isinstance(v, dict): + return MappingProxyType(v) + msg = f"expected dict or MappingProxyType, got {type(v).__name__}" + raise ValueError(msg) + # --------------------------------------------------------------------------- # StrategyRegistryEntry — metadata wrapper for registered strategies diff --git a/src/cleveragents/domain/models/acms/strategy_stubs.py b/src/cleveragents/domain/models/acms/strategy_stubs.py index b8f4a2e76..0b9aee7f6 100644 --- a/src/cleveragents/domain/models/acms/strategy_stubs.py +++ b/src/cleveragents/domain/models/acms/strategy_stubs.py @@ -23,6 +23,8 @@ Six built-in strategies per spec: from __future__ import annotations +import functools + from cleveragents.domain.models.acms.crp import ( ContextFragment, ContextRequest, @@ -33,6 +35,17 @@ from cleveragents.domain.models.acms.strategy import ( StrategyCapabilities, ) +__all__ = [ + "BUILTIN_STRATEGY_CLASSES", + "DEFAULT_ENABLED_STRATEGIES", + "ARCEStrategy", + "BreadthDepthNavigatorStrategy", + "PlanDecisionContextStrategy", + "SemanticEmbeddingStrategy", + "SimpleKeywordStrategy", + "TemporalArchaeologyStrategy", +] + # --------------------------------------------------------------------------- # simple-keyword (spec §25211, §43171-43173) # --------------------------------------------------------------------------- @@ -51,7 +64,7 @@ class SimpleKeywordStrategy: def name(self) -> str: return "simple-keyword" - @property + @functools.cached_property def capabilities(self) -> StrategyCapabilities: return StrategyCapabilities( uses_text=True, @@ -106,7 +119,7 @@ class SemanticEmbeddingStrategy: def name(self) -> str: return "semantic-embedding" - @property + @functools.cached_property def capabilities(self) -> StrategyCapabilities: return StrategyCapabilities( uses_text=False, @@ -161,7 +174,7 @@ class BreadthDepthNavigatorStrategy: def name(self) -> str: return "breadth-depth-navigator" - @property + @functools.cached_property def capabilities(self) -> StrategyCapabilities: return StrategyCapabilities( uses_text=False, @@ -217,7 +230,7 @@ class ARCEStrategy: def name(self) -> str: return "arce" - @property + @functools.cached_property def capabilities(self) -> StrategyCapabilities: return StrategyCapabilities( uses_text=True, @@ -276,7 +289,7 @@ class TemporalArchaeologyStrategy: def name(self) -> str: return "temporal-archaeology" - @property + @functools.cached_property def capabilities(self) -> StrategyCapabilities: return StrategyCapabilities( uses_text=False, @@ -293,7 +306,8 @@ class TemporalArchaeologyStrategy: request: ContextRequest, backends: BackendSet, ) -> float: - if backends.graph is not None: + # Requires both graph AND temporal/cold-tier (spec §43193-43195). + if backends.graph is not None and backends.temporal is not None: return self.capabilities.quality_score return 0.0 @@ -332,7 +346,7 @@ class PlanDecisionContextStrategy: def name(self) -> str: return "plan-decision-context" - @property + @functools.cached_property def capabilities(self) -> StrategyCapabilities: return StrategyCapabilities( uses_text=False, @@ -349,8 +363,10 @@ class PlanDecisionContextStrategy: request: ContextRequest, backends: BackendSet, ) -> float: - # Works even without graph; reads from warm/cold tiers. - return self.capabilities.quality_score + # Requires temporal/cold-tier backend (spec §43197-43199). + if backends.temporal is not None: + return self.capabilities.quality_score + return 0.0 def assemble( self, @@ -376,6 +392,12 @@ class PlanDecisionContextStrategy: # Canonical ordered list per spec §28682 default: # ["simple-keyword", "semantic-embedding", "breadth-depth-navigator"] # Plus the remaining 3 registered but not enabled by default. +# +# NOTE: Spec §25223 lists 4 strategies (including "arce") in the default +# enabled config, but §28682 lists only 3 (without "arce"). This is a +# spec internal contradiction. We follow §28682 (the config key table) +# because it is the more specific/operational reference. This decision +# should be confirmed with the spec author. BUILTIN_STRATEGY_CLASSES: tuple[type, ...] = ( SimpleKeywordStrategy, diff --git a/vulture_whitelist.py b/vulture_whitelist.py index 0bb07a73a..12182182b 100644 --- a/vulture_whitelist.py +++ b/vulture_whitelist.py @@ -622,6 +622,7 @@ dependency_type # noqa: B018, F821 # Context Strategy Registry — protocol parameter required by ContextStrategy.assemble() plan_context # noqa: B018, F821 +inject_stale_enabled_entry # noqa: B018, F821 # ACMS Backend Abstraction Layer — public API (issue #498) TextBackend # noqa: B018, F821 -- 2.52.0