diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cf791e12..1acecf207 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,18 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). message listing available built-in profiles. The resolved profile name is also logged at debug level for observability. +- **Invariant precedence chain omitted action scope tier** (#9003): Updated the + invariant precedence chain documentation and implementation from the incorrect + three-tier order (plan > project > global) to the correct four-tier hierarchy + (**plan > action > project > global**) as defined in specification §92. Changes + include: module docstring and `InvariantScope` class docstring corrections, + `merge_invariants()` function updated to accept and process `action_invariants` + parameter, `InvariantSet.merge()` class method propagated with the new parameter, + `InvariantService.get_effective_invariants()` expanded to collect and pass + action-scoped invariants, Behave step definitions extended for action-invariant + scenarios, benchmark suite updated for four-tier merge performance testing, and + Robot Framework helper functions supplied with empty action_invariants lists. + ### Added - **TDD Issue-Capture Test Activation** (#7025): Replaced 234 bare `@skip` tags diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 31d4c882f..4973816ff 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -16,4 +16,5 @@ Below are some of the specific details of various contributions. * Brent E. Edwards has contributed quality assurance, test coverage, and CI pipeline improvements. * HAL 9000 has contributed automated implementation, bug fixes, and feature development as part of the CleverAgents automation pool. * HAL 9000 has contributed the plan concurrency race-condition fix (#7989): wired `LockService` into the plan lifecycle, guarding `execute_plan()` and `apply_plan()` with plan-level advisory locks and unique per-invocation owner identities to prevent silent concurrent state corruption. +* HAL 9000 has contributed the invariant precedence chain action scope fix (issue #9003 / PR #9240): corrected the invariant precedence chain from three-tier (plan > project > global) to four-tier (plan > action > project > global) in docstrings, merge functions, and BDD test steps as required by specification §92. * This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc. diff --git a/benchmarks/invariant_merge_bench.py b/benchmarks/invariant_merge_bench.py index d68db24cf..f468f8f46 100644 --- a/benchmarks/invariant_merge_bench.py +++ b/benchmarks/invariant_merge_bench.py @@ -69,12 +69,13 @@ class MergeSmallSuite: def setup(self) -> None: """Create small invariant lists.""" self.plan = _make_invariants(InvariantScope.PLAN, 3) + self.action = _make_invariants(InvariantScope.ACTION, 2) self.project = _make_invariants(InvariantScope.PROJECT, 5) self.global_invs = _make_invariants(InvariantScope.GLOBAL, 5) def time_merge_small(self) -> None: - """Benchmark merge with ~13 invariants.""" - merge_invariants(self.plan, self.project, self.global_invs) + """Benchmark merge with ~15 invariants.""" + merge_invariants(self.plan, self.action, self.project, self.global_invs) class MergeMediumSuite: @@ -83,12 +84,13 @@ class MergeMediumSuite: def setup(self) -> None: """Create medium invariant lists.""" self.plan = _make_invariants(InvariantScope.PLAN, 10) + self.action = _make_invariants(InvariantScope.ACTION, 5) self.project = _make_invariants(InvariantScope.PROJECT, 20) self.global_invs = _make_invariants(InvariantScope.GLOBAL, 20) def time_merge_medium(self) -> None: - """Benchmark merge with ~50 invariants.""" - merge_invariants(self.plan, self.project, self.global_invs) + """Benchmark merge with ~55 invariants.""" + merge_invariants(self.plan, self.action, self.project, self.global_invs) class MergeLargeSuite: @@ -97,12 +99,13 @@ class MergeLargeSuite: def setup(self) -> None: """Create large invariant lists.""" self.plan = _make_invariants(InvariantScope.PLAN, 50) + self.action = _make_invariants(InvariantScope.ACTION, 25) self.project = _make_invariants(InvariantScope.PROJECT, 100) self.global_invs = _make_invariants(InvariantScope.GLOBAL, 100) def time_merge_large(self) -> None: - """Benchmark merge with ~250 invariants.""" - merge_invariants(self.plan, self.project, self.global_invs) + """Benchmark merge with ~275 invariants.""" + merge_invariants(self.plan, self.action, self.project, self.global_invs) class MergeDeduplicationSuite: @@ -118,6 +121,14 @@ class MergeDeduplicationSuite: ) for i in range(20) ] + self.action = [ + Invariant( + text=f"Shared constraint {i}", + scope=InvariantScope.ACTION, + source_name="action-001", + ) + for i in range(20) + ] self.project = [ Invariant( text=f"Shared constraint {i}", @@ -136,8 +147,8 @@ class MergeDeduplicationSuite: ] def time_merge_dedup(self) -> None: - """Benchmark merge with 60 invariants, all duplicates.""" - merge_invariants(self.plan, self.project, self.global_invs) + """Benchmark merge with 80 invariants, all duplicates.""" + merge_invariants(self.plan, self.action, self.project, self.global_invs) class InvariantSetMergeSuite: @@ -146,12 +157,13 @@ class InvariantSetMergeSuite: def setup(self) -> None: """Create invariant lists.""" self.plan = _make_invariants(InvariantScope.PLAN, 5) + self.action = _make_invariants(InvariantScope.ACTION, 3) self.project = _make_invariants(InvariantScope.PROJECT, 10) self.global_invs = _make_invariants(InvariantScope.GLOBAL, 10) def time_invariant_set_merge(self) -> None: """Benchmark InvariantSet.merge().""" - InvariantSet.merge(self.plan, self.project, self.global_invs) + InvariantSet.merge(self.plan, self.action, self.project, self.global_invs) class ServiceEffectiveSuite: @@ -164,9 +176,13 @@ class ServiceEffectiveSuite: self.service.add_invariant(f"Global {i}", InvariantScope.GLOBAL, "system") for i in range(10): self.service.add_invariant(f"Project {i}", InvariantScope.PROJECT, "myapp") + for i in range(5): + self.service.add_invariant(f"Action {i}", InvariantScope.ACTION, "action-001") for i in range(5): self.service.add_invariant(f"Plan {i}", InvariantScope.PLAN, "plan-001") def time_get_effective(self) -> None: """Benchmark get_effective_invariants().""" - self.service.get_effective_invariants(plan_id="plan-001", project_name="myapp") + self.service.get_effective_invariants( + plan_id="plan-001", project_name="myapp", action_name="action-001" + ) diff --git a/features/steps/invariant_models_steps.py b/features/steps/invariant_models_steps.py index b1909d647..22d1aa847 100644 --- a/features/steps/invariant_models_steps.py +++ b/features/steps/invariant_models_steps.py @@ -135,6 +135,11 @@ def step_plan_invariants(context): context.plan_invariants = _parse_invariant_table(context, InvariantScope.PLAN) +@given("I have action invariants") +def step_action_invariants(context): + context.action_invariants = _parse_invariant_table(context, InvariantScope.ACTION) + + @given("I have project invariants") def step_project_invariants(context): context.project_invariants = _parse_invariant_table(context, InvariantScope.PROJECT) @@ -160,6 +165,7 @@ def step_plan_invariants_inactive(context): def step_merge(context): context.merged = merge_invariants( getattr(context, "plan_invariants", []), + getattr(context, "action_invariants", []), getattr(context, "project_invariants", []), getattr(context, "global_invariants", []), ) @@ -189,6 +195,7 @@ def step_merged_scope(context, idx, scope): def step_merge_invariant_set(context): inv_set = InvariantSet.merge( getattr(context, "plan_invariants", []), + getattr(context, "action_invariants", []), getattr(context, "project_invariants", []), getattr(context, "global_invariants", []), ) @@ -386,6 +393,7 @@ def step_service_all_scopes(context): context.service = InvariantService() context.service.add_invariant("Global rule", InvariantScope.GLOBAL, "system") context.service.add_invariant("Project rule", InvariantScope.PROJECT, "proj1") + context.service.add_invariant("Action rule", InvariantScope.ACTION, "action1") context.service.add_invariant("Plan rule", InvariantScope.PLAN, "plan1") diff --git a/robot/helper_m3_e2e_verification.py b/robot/helper_m3_e2e_verification.py index bfd06a911..84e9d78c1 100644 --- a/robot/helper_m3_e2e_verification.py +++ b/robot/helper_m3_e2e_verification.py @@ -867,6 +867,7 @@ def invariants_enforced_during_strategize() -> None: merged = merge_invariants( plan_invariants=[plan_inv], + action_invariants=[], project_invariants=[project_inv], global_invariants=[global_inv], ) @@ -890,6 +891,7 @@ def invariants_enforced_during_strategize() -> None: invariant_set = InvariantSet.merge( plan_invariants=[plan_inv], + action_invariants=[], project_invariants=[project_inv], global_invariants=[global_inv], ) diff --git a/src/cleveragents/application/services/invariant_service.py b/src/cleveragents/application/services/invariant_service.py index e941b999e..0f6a01572 100644 --- a/src/cleveragents/application/services/invariant_service.py +++ b/src/cleveragents/application/services/invariant_service.py @@ -11,7 +11,7 @@ a dict keyed by invariant ID. ## Merge Precedence -Effective invariants are computed using plan > project > global order. +Effective invariants are computed using plan > action > project > global order. See ``merge_invariants`` for de-duplication semantics. Based on ``docs/specification.md`` and implementation plan Stage M3.5. @@ -168,17 +168,20 @@ class InvariantService: self, plan_id: str | None = None, project_name: str | None = None, + action_name: str | None = None, ) -> list[Invariant]: - """Return the merged precedence chain for a plan/project context. + """Return the merged precedence chain for a plan/project/action context. Collects active invariants from each scope tier and merges them - using plan > project > global precedence. + using plan > action > project > global precedence. Args: plan_id: Optional plan identifier to collect plan-scoped invariants. project_name: Optional project name to collect project-scoped invariants. + action_name: Optional action name to collect action-scoped + invariants. Returns: Merged, de-duplicated list of effective invariants. @@ -191,6 +194,12 @@ class InvariantService: if inv.scope == InvariantScope.PLAN and (plan_id is None or inv.source_name == plan_id) ] + action_invs = [ + inv + for inv in active + if inv.scope == InvariantScope.ACTION + and (action_name is None or inv.source_name == action_name) + ] project_invs = [ inv for inv in active @@ -199,7 +208,7 @@ class InvariantService: ] global_invs = [inv for inv in active if inv.scope == InvariantScope.GLOBAL] - return merge_invariants(plan_invs, project_invs, global_invs) + return merge_invariants(plan_invs, action_invs, project_invs, global_invs) def enforce_invariants( self, diff --git a/src/cleveragents/domain/models/core/invariant.py b/src/cleveragents/domain/models/core/invariant.py index 1880bb668..3b7e480af 100644 --- a/src/cleveragents/domain/models/core/invariant.py +++ b/src/cleveragents/domain/models/core/invariant.py @@ -4,7 +4,7 @@ Invariants are natural-language constraints on plan execution, scoped at global, project, action, or plan level. When an action is used, its invariants are promoted to plan-level. The runtime precedence chain is: - plan > project > global + plan > action > project > global They are reconciled by the Invariant Reconciliation Actor at the start of Strategize and recorded as ``invariant_enforced`` decisions. @@ -21,7 +21,7 @@ of Strategize and recorded as ``invariant_enforced`` decisions. ## Merge Precedence When computing the effective set of invariants for a plan, the merge -order is **plan > project > global**. Duplicate texts (case-insensitive) +order is **plan > action > project > global**. Duplicate texts (case-insensitive) are de-duplicated, keeping the highest-precedence copy. Based on ``docs/specification.md`` and implementation plan Stage M3.5. @@ -39,7 +39,7 @@ from ulid import ULID class InvariantScope(StrEnum): """Scope at which an invariant applies. - Precedence (highest to lowest): PLAN > PROJECT > GLOBAL. + Precedence (highest to lowest): PLAN > ACTION > PROJECT > GLOBAL. ACTION invariants are promoted to PLAN scope at ``plan use`` time. """ @@ -137,10 +137,11 @@ class InvariantSet(BaseModel): def merge( cls, plan_invariants: list[Invariant], + action_invariants: list[Invariant], project_invariants: list[Invariant], global_invariants: list[Invariant], ) -> InvariantSet: - """Merge invariants respecting plan > project > global precedence. + """Merge invariants respecting plan > action > project > global precedence. De-duplicates by text (case-insensitive), keeping the copy from the highest-precedence tier. Within each tier, source ordering @@ -148,6 +149,7 @@ class InvariantSet(BaseModel): Args: plan_invariants: Plan-level invariants (highest precedence). + action_invariants: Action-level invariants. project_invariants: Project-level invariants. global_invariants: Global-level invariants (lowest precedence). @@ -156,7 +158,12 @@ class InvariantSet(BaseModel): """ return cls( invariants=tuple( - merge_invariants(plan_invariants, project_invariants, global_invariants) + merge_invariants( + plan_invariants, + action_invariants, + project_invariants, + global_invariants, + ) ) ) @@ -165,10 +172,11 @@ class InvariantSet(BaseModel): def merge_invariants( plan_invariants: list[Invariant], + action_invariants: list[Invariant], project_invariants: list[Invariant], global_invariants: list[Invariant], ) -> list[Invariant]: - """Merge invariants implementing plan > project > global precedence. + """Merge invariants implementing plan > action > project > global precedence. De-duplicates by text (case-insensitive). The first occurrence (from the highest-precedence tier) wins. Within each tier, the @@ -176,6 +184,7 @@ def merge_invariants( Args: plan_invariants: Plan-level invariants (highest precedence). + action_invariants: Action-level invariants. project_invariants: Project-level invariants. global_invariants: Global-level invariants (lowest precedence). @@ -185,7 +194,12 @@ def merge_invariants( seen: set[str] = set() result: list[Invariant] = [] - for inv_list in (plan_invariants, project_invariants, global_invariants): + for inv_list in ( + plan_invariants, + action_invariants, + project_invariants, + global_invariants, + ): for inv in inv_list: if not inv.active: continue