From a54bf5f2befe00cc52116a150d2bdca7afd77595 Mon Sep 17 00:00:00 2001 From: HAL9000 Date: Tue, 14 Apr 2026 12:19:19 +0000 Subject: [PATCH] fix(domain): correct invariant precedence chain to include action scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The invariant precedence chain is four-tier per specification §92: plan > action > project > global This fix updates: 1. Module docstring in invariant.py to document the correct four-tier precedence 2. InvariantScope class docstring to reflect PLAN > ACTION > PROJECT > GLOBAL 3. merge_invariants() function to accept action_invariants parameter 4. InvariantSet.merge() class method to accept and pass action_invariants 5. InvariantService.get_effective_invariants() to collect and pass action invariants 6. BDD test steps to include action invariants in merge operations 7. Benchmark suite to include action invariants in performance tests 8. Robot Framework helper to pass action_invariants to merge functions 9. CHANGELOG.md entry under [Unreleased]/### Fixed section 10. CONTRIBUTORS.md entry documenting HAL 9000 contribution All docstrings now correctly document the four-tier precedence chain, and the merge logic properly handles action-scope invariants between plan and project scopes. ISSUES CLOSED: #9003 --- CHANGELOG.md | 12 +++++++ CONTRIBUTORS.md | 1 + benchmarks/invariant_merge_bench.py | 36 +++++++++++++------ features/steps/invariant_models_steps.py | 8 +++++ robot/helper_m3_e2e_verification.py | 2 ++ .../application/services/invariant_service.py | 17 ++++++--- .../domain/models/core/invariant.py | 28 +++++++++++---- 7 files changed, 83 insertions(+), 21 deletions(-) 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