bug(acms): dual incompatible `ContextStrategy` protocols — domain model protocol never used by ACMS pipeline #2375

New issue

Open

opened 2026-04-03 17:25:30 +00:00 by freemo · 1 comment

freemo commented

2026-04-03 17:25:30 +00:00

Owner

Metadata

Branch: fix/acms-unified-context-strategy-protocol
Commit Message: fix(acms): unify ContextStrategy protocol to domain model definition (strategy.py)
Milestone: v3.4.0
Parent Epic: #396

Background

The codebase defines two incompatible ContextStrategy protocols that cannot be used interchangeably. The spec (§25166-25189) designates the domain model version in strategy.py as the authoritative interface, but the ACMS pipeline exclusively uses a simplified, incompatible protocol defined inline in acms_service.py.

Protocol 1 — `acms_service.py` (used by the pipeline, non-authoritative):

class ContextStrategy(Protocol):
    def can_handle(self, request: dict[str, Any]) -> float: ...
    def assemble(self, fragments: Sequence[ContextFragment], budget: ContextBudget) -> Sequence[ContextFragment]: ...

Protocol 2 — `domain/models/acms/strategy.py` (domain model, authoritative per spec, never used):

class ContextStrategy(Protocol):
    def can_handle(self, request: ContextRequest, backends: BackendSet) -> float: ...
    def assemble(self, request: ContextRequest, backends: BackendSet, budget: int, plan_context: PlanContext) -> list[ContextFragment]: ...

The spec requires strategies to receive:

A typed ContextRequest (not a raw dict) for can_handle
A BackendSet for backend availability detection
A PlanContext for plan hierarchy traversal
An integer budget (not a ContextBudget object) for assemble

Impact

All built-in strategies (SimpleKeywordStrategy, SemanticEmbeddingStrategy, BreadthDepthNavigatorStrategy, ArceStrategy, TemporalArchaeologyStrategy, PlanDecisionContextStrategy) implement the simplified acms_service.py protocol and cannot use BackendSet to detect available backends or PlanContext for hierarchy traversal.
The StrategyRegistryEntry in strategy.py references the domain model protocol but the StrategyRegistry in strategy_registry.py uses the service protocol — creating a type inconsistency that Pyright cannot catch across module boundaries.
Custom strategies registered via the plugin system cannot know which protocol to implement.
The strategy.py domain model is dead code that is never exercised.

Code Locations

src/cleveragents/application/services/acms_service.py lines 96–125 (service protocol — to be removed)
src/cleveragents/domain/models/acms/strategy.py lines 190–220 (domain model protocol — authoritative)
src/cleveragents/application/services/context_strategies.py (implements service protocol — must be migrated)
src/cleveragents/application/services/acms_advanced_strategies.py (implements service protocol — must be migrated)
src/cleveragents/application/services/strategy_registry.py (uses service protocol — must be migrated)

Subtasks

Remove the duplicate ContextStrategy protocol from acms_service.py (lines 96–125)
Update all imports in acms_service.py to reference ContextStrategy from domain/models/acms/strategy.py
Migrate SimpleKeywordStrategy in context_strategies.py to implement the domain model protocol (accept ContextRequest, BackendSet, PlanContext, int budget)
Migrate SemanticEmbeddingStrategy in context_strategies.py to implement the domain model protocol
Migrate BreadthDepthNavigatorStrategy in context_strategies.py to implement the domain model protocol
Migrate ArceStrategy in acms_advanced_strategies.py to implement the domain model protocol
Migrate TemporalArchaeologyStrategy in acms_advanced_strategies.py to implement the domain model protocol
Migrate PlanDecisionContextStrategy in acms_advanced_strategies.py to implement the domain model protocol
Update StrategyRegistry in strategy_registry.py to use the domain model ContextStrategy protocol
Update the ACMS pipeline in acms_service.py to pass ContextRequest, BackendSet, and PlanContext when invoking strategies
Update plugin system documentation/interface to reference the canonical domain model protocol
Write/update Behave unit tests in features/ covering the unified protocol contract for all six built-in strategies
Run nox -e typecheck and confirm zero Pyright errors related to ContextStrategy
Run nox -e unit_tests and confirm all tests pass
Run nox -e coverage_report and confirm coverage ≥ 97%

Definition of Done

Only one ContextStrategy protocol exists in the codebase — the domain model definition in src/cleveragents/domain/models/acms/strategy.py
All six built-in strategies implement the domain model protocol signature (ContextRequest, BackendSet, int, PlanContext)
StrategyRegistry and the ACMS pipeline use the domain model protocol exclusively
No references to the old dict[str, Any] / ContextBudget-based protocol remain in strategy-related code
nox -e typecheck passes with zero errors
All nox stages pass
Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-uat-tester

## Metadata - **Branch**: `fix/acms-unified-context-strategy-protocol` - **Commit Message**: `fix(acms): unify ContextStrategy protocol to domain model definition (strategy.py)` - **Milestone**: v3.4.0 - **Parent Epic**: #396 ## Background The codebase defines two incompatible `ContextStrategy` protocols that cannot be used interchangeably. The spec (§25166-25189) designates the domain model version in `strategy.py` as the authoritative interface, but the ACMS pipeline exclusively uses a simplified, incompatible protocol defined inline in `acms_service.py`. ### Protocol 1 — `acms_service.py` (used by the pipeline, **non-authoritative**): ```python class ContextStrategy(Protocol): def can_handle(self, request: dict[str, Any]) -> float: ... def assemble(self, fragments: Sequence[ContextFragment], budget: ContextBudget) -> Sequence[ContextFragment]: ... ``` ### Protocol 2 — `domain/models/acms/strategy.py` (domain model, **authoritative per spec**, never used): ```python class ContextStrategy(Protocol): def can_handle(self, request: ContextRequest, backends: BackendSet) -> float: ... def assemble(self, request: ContextRequest, backends: BackendSet, budget: int, plan_context: PlanContext) -> list[ContextFragment]: ... ``` The spec requires strategies to receive: - A typed `ContextRequest` (not a raw `dict`) for `can_handle` - A `BackendSet` for backend availability detection - A `PlanContext` for plan hierarchy traversal - An integer `budget` (not a `ContextBudget` object) for `assemble` ### Impact 1. All built-in strategies (`SimpleKeywordStrategy`, `SemanticEmbeddingStrategy`, `BreadthDepthNavigatorStrategy`, `ArceStrategy`, `TemporalArchaeologyStrategy`, `PlanDecisionContextStrategy`) implement the simplified `acms_service.py` protocol and cannot use `BackendSet` to detect available backends or `PlanContext` for hierarchy traversal. 2. The `StrategyRegistryEntry` in `strategy.py` references the domain model protocol but the `StrategyRegistry` in `strategy_registry.py` uses the service protocol — creating a type inconsistency that Pyright cannot catch across module boundaries. 3. Custom strategies registered via the plugin system cannot know which protocol to implement. 4. The `strategy.py` domain model is dead code that is never exercised. ### Code Locations - `src/cleveragents/application/services/acms_service.py` lines 96–125 (service protocol — to be removed) - `src/cleveragents/domain/models/acms/strategy.py` lines 190–220 (domain model protocol — authoritative) - `src/cleveragents/application/services/context_strategies.py` (implements service protocol — must be migrated) - `src/cleveragents/application/services/acms_advanced_strategies.py` (implements service protocol — must be migrated) - `src/cleveragents/application/services/strategy_registry.py` (uses service protocol — must be migrated) ## Subtasks - [ ] Remove the duplicate `ContextStrategy` protocol from `acms_service.py` (lines 96–125) - [ ] Update all imports in `acms_service.py` to reference `ContextStrategy` from `domain/models/acms/strategy.py` - [ ] Migrate `SimpleKeywordStrategy` in `context_strategies.py` to implement the domain model protocol (accept `ContextRequest`, `BackendSet`, `PlanContext`, `int` budget) - [ ] Migrate `SemanticEmbeddingStrategy` in `context_strategies.py` to implement the domain model protocol - [ ] Migrate `BreadthDepthNavigatorStrategy` in `context_strategies.py` to implement the domain model protocol - [ ] Migrate `ArceStrategy` in `acms_advanced_strategies.py` to implement the domain model protocol - [ ] Migrate `TemporalArchaeologyStrategy` in `acms_advanced_strategies.py` to implement the domain model protocol - [ ] Migrate `PlanDecisionContextStrategy` in `acms_advanced_strategies.py` to implement the domain model protocol - [ ] Update `StrategyRegistry` in `strategy_registry.py` to use the domain model `ContextStrategy` protocol - [ ] Update the ACMS pipeline in `acms_service.py` to pass `ContextRequest`, `BackendSet`, and `PlanContext` when invoking strategies - [ ] Update plugin system documentation/interface to reference the canonical domain model protocol - [ ] Write/update Behave unit tests in `features/` covering the unified protocol contract for all six built-in strategies - [ ] Run `nox -e typecheck` and confirm zero Pyright errors related to `ContextStrategy` - [ ] Run `nox -e unit_tests` and confirm all tests pass - [ ] Run `nox -e coverage_report` and confirm coverage ≥ 97% ## Definition of Done - [ ] Only one `ContextStrategy` protocol exists in the codebase — the domain model definition in `src/cleveragents/domain/models/acms/strategy.py` - [ ] All six built-in strategies implement the domain model protocol signature (`ContextRequest`, `BackendSet`, `int`, `PlanContext`) - [ ] `StrategyRegistry` and the ACMS pipeline use the domain model protocol exclusively - [ ] No references to the old `dict[str, Any]` / `ContextBudget`-based protocol remain in strategy-related code - [ ] `nox -e typecheck` passes with zero errors - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-uat-tester

freemo added this to the v3.4.0 milestone

2026-04-03 17:25:36 +00:00

freemo added a new dependency

2026-04-03 17:26:11 +00:00

#396 Epic: ACMS Context Pipeline

freemo referenced this issue

2026-04-03 17:35:14 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.6.0 — Session Tracker #2360

freemo commented

2026-04-03 18:23:25 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: High — Two incompatible ContextStrategy protocols means the domain model protocol (which the spec designates as authoritative) is dead code. The ACMS pipeline uses a different, simpler protocol, creating a spec-implementation divergence.
Milestone: v3.4.0 (as specified in issue metadata)
MoSCoW: Must Have — The spec designates the domain model ContextStrategy as the authoritative interface. Having the ACMS pipeline use an incompatible protocol means the context pipeline does not conform to the spec's architecture.
Parent Epic: #396 (ACMS Context Pipeline)

Well-described with clear spec references and protocol analysis. Valid and actionable.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: High — Two incompatible `ContextStrategy` protocols means the domain model protocol (which the spec designates as authoritative) is dead code. The ACMS pipeline uses a different, simpler protocol, creating a spec-implementation divergence. - **Milestone**: v3.4.0 (as specified in issue metadata) - **MoSCoW**: Must Have — The spec designates the domain model `ContextStrategy` as the authoritative interface. Having the ACMS pipeline use an incompatible protocol means the context pipeline does not conform to the spec's architecture. - **Parent Epic**: #396 (ACMS Context Pipeline) Well-described with clear spec references and protocol analysis. Valid and actionable. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo added the

MoSCoW

Must have

label

2026-04-03 18:23:40 +00:00

freemo removed the

MoSCoW

Must have

label

2026-04-05 04:23:22 +00:00

freemo removed this from the v3.4.0 milestone

2026-04-06 21:01:26 +00:00