Merge pull request 'revert(acms): revert PR #11216 — _resolve_effective_budget reads wrong attribute' (#11228) from bugfix/m5-revert-acms-budget-assembler into master

Reviewed-on: #11228
Reviewed-by: Rui Hu <rui.hu@cleverthis.com>

CHANGELOG.md

@@ -107,12 +107,6 @@ Changed `wf10_batch.robot` to be less likely to create files, and
 - **Plan Rollback Command** (#8557): Implemented `agents plan rollback <plan-id> [<checkpoint-id>]` for checkpoint-based plan state restoration in Epic #8493. The command restores a plan's sandbox to the state captured at a given checkpoint, discarding all decisions made after that checkpoint. The checkpoint can be specified as an optional positional second argument or via the `--to-checkpoint` named option. Supports `--yes/-y` flag to skip confirmation prompts and `--format/-f` for output format selection (rich/plain/json/yaml). Included with comprehensive BDD test coverage (>= 97%) and spec-aligned output formatting showing rollback summary, changes reverted, impact analysis, and post-rollback state panels.
 
 ### Fixed
-- **ACMS execute-phase assembler now respects project-level hot_max_tokens** (#11035): The
-  ``ACMSExecutePhaseContextAssembler._resolve_effective_budget()`` method was added to read
-  each linked project's ``settings.hot_max_tokens`` and use the maximum override value as the
-  pipeline budget. Falls back to the constructor-injected global when no override is set. This
-  ensures users who configure ``agents project context set --hot-max-tokens 32000`` actually see
-  source files in context that would previously have been excluded by the 16K default budget.
 - **Guard cleanup_stale against execute/processing and execute/complete plans** (#11121):
   ``_create_sandbox_for_plan()`` in ``src/cleveragents/cli/commands/plan.py`` now
   skips ``GitWorktreeSandbox.cleanup_stale()`` when the plan is in

features/execute_phase_context_assembler_coverage.feature

@@ -235,13 +235,3 @@ Feature: Execute-phase context assembler coverage
     When epcov I call assemble on the plan
     Then epcov the assembled result should be an AssembledContext
     And epcov the assembled result should have fragments and metadata
-
-  # ---- assemble: project-level hot_max_tokens budget override ----
-
-  @tdd_issue @tdd_issue_11035
-  Scenario: epcov assemble uses project-level hot_max_tokens when set
-    Given epcov an assembler with project hot_max_tokens set to 32000
-    And epcov a plan with project links
-    And epcov scoped fragments that pass all filters
-    When epcov I call assemble on the plan
-    Then epcov the assembled budget should be 32000

features/steps/execute_phase_context_assembler_coverage_steps.py

@@ -22,8 +22,6 @@ from cleveragents.application.services.execute_phase_context_assembler import (
     ACMSExecutePhaseContextAssembler,
 )
 from cleveragents.domain.models.acms.crp import AssembledContext
-from cleveragents.domain.models.acms.crp import ContextFragment as CRPFragment
-from cleveragents.domain.models.acms.crp import FragmentProvenance as CRPProvenance
 from cleveragents.domain.models.acms.tiers import TieredFragment
 from cleveragents.domain.models.core.context_policy import (
     ContextView,
@@ -879,6 +877,13 @@ def step_epcov_scoped_frags_all_excluded(context: Context) -> None:
 
 @given("epcov an assembler with default policy and pipeline")
 def step_epcov_assembler_full_pipeline(context: Context) -> None:
+    from cleveragents.domain.models.acms.crp import (
+        ContextFragment as CRPFragment,
+    )
+    from cleveragents.domain.models.acms.crp import (
+        FragmentProvenance as CRPProvenance,
+    )
+
     mock_frag = CRPFragment(
         uko_node="res:good",
         content="good content here",
@@ -932,63 +937,3 @@ def step_epcov_assembled_has_data(context: Context) -> None:
     assert result.budget_used >= 0.0, "Expected non-negative budget_used"
     assert result.context_hash, "Expected non-empty context_hash"
     assert result.strategies_used, "Expected non-empty strategies_used"
-
-
-# ---------------------------------------------------------------------------
-# assemble: project-level hot_max_tokens budget override
-# ---------------------------------------------------------------------------
-
-
-@given("epcov an assembler with project hot_max_tokens set to 32000")
-def step_epcov_assembler_project_budget(context: Context) -> None:
-    """Create an assembler whose project repo returns hot_max_tokens=32000."""
-    mock_frag = CRPFragment(
-        uko_node="res:good",
-        content="good content here",
-        detail_depth=1,
-        token_count=10,
-        relevance_score=0.5,
-        provenance=CRPProvenance(
-            resource_uri="res:good",
-            location="src/good.py",
-            strategy="execute_phase_context",
-        ),
-    )
-    pr = _make_pipeline_result()
-    pr.fragments = (mock_frag,)
-    pr.total_tokens = 10
-    pr.budget_used = 0.25
-    pr.strategies_used = ("relevance",)
-    pr.context_hash = "sha256hash"
-    pr.preamble = "Context preamble"
-    pr.provenance_map = {"frag-1": {"resource_uri": "res:test"}}
-
-    # Build assembler with global budget of 4096
-    assembler = _make_assembler(pipeline_result=pr)
-
-    # Mock the project repo's get() to return a project with hot_max_tokens=32000
-    mock_project = MagicMock()
-    mock_project.settings.hot_max_tokens = 32000
-    assembler._project_repository.get.return_value = mock_project
-
-    context.epcov_assembler = assembler
-
-
-@then("epcov the assembled budget should be 32000")
-def step_epcov_assembled_budget_32k(context: Context) -> None:
-    """Verify the pipeline was called with the project-level budget (32000)."""
-    if context.epcov_error is not None:
-        raise AssertionError(f"Unexpected error: {context.epcov_error}")
-    # Check the budget passed to the pipeline.assemble() call
-    pipeline = context.epcov_assembler._pipeline
-    assert pipeline.assemble.called, "Pipeline.assemble() was not called"
-    call_kwargs = pipeline.assemble.call_args
-    # The budget kwarg should reflect the project-level override
-    budget = call_kwargs.kwargs.get("budget") or call_kwargs[1].get("budget")
-    if budget is None:
-        # Try positional args
-        budget = call_kwargs[0][2] if len(call_kwargs[0]) > 2 else None
-    assert budget is not None, "Could not find budget in pipeline.assemble() call"
-    assert budget.max_tokens == 32000, (
-        f"Expected budget max_tokens=32000, got {budget.max_tokens}"
-    )

src/cleveragents/application/services/execute_phase_context_assembler.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import fnmatch
+import json
 from pathlib import PurePath
 from typing import Any, Protocol
 
@@ -70,28 +71,70 @@ class ACMSExecutePhaseContextAssembler(ExecutePhaseContextAssembler):
             return ProjectContextPolicy().resolve_view("execute")
         return policy.resolve_view("execute")
 
-    def _resolve_effective_budget(self, project_names: list[str]) -> int:
-        """Resolve the effective hot-tier budget from project settings.
+    def _resolve_hot_max_tokens(self, project_names: list[str]) -> int:
+        """Resolve the effective ``hot_max_tokens`` budget for *project_names*.
 
-        Checks each project's ``settings.hot_max_tokens`` and uses the
-        maximum value found.  Falls back to the constructor-injected
-        global ``self._hot_max_tokens`` when no project-level override
-        is set.
+        Looks up each project's :attr:`ContextConfig.hot_max_tokens` from its
+        stored context configuration.  Projects that do not have an explicit
+        setting are excluded from the aggregation so they do not affect
+        the computed value.
+
+        Returns the **maximum** of all explicitly-set project-level values,
+        falling back to :attr:`_hot_max_tokens` (the caller-passed global
+        default) when no project overrides are present.
         """
-        project_budgets: list[int] = []
-        for name in project_names:
+        from typing import cast
+
+        candidates: list[int] = []
+        for namespaced_name in project_names:
+            row = None
             try:
-                project = self._project_repository.get(name)
+                session = self._project_repository._session()  # type: ignore[attr-defined]
+                from cleveragents.infrastructure.database.models import (
+                    NamespacedProjectModel,
+                )
+
+                row = (
+                    session.query(NamespacedProjectModel)
+                    .filter_by(namespaced_name=namespaced_name)
+                    .first()
+                )
+                session.close()
+            except AttributeError:
+                self._logger.warning(
+                    "hot_max_tokens_session_factory_missing",
+                    project_name=namespaced_name,
+                )
+                continue
             except Exception:
-                continue
-            settings = getattr(project, "settings", None)
-            if settings is None:
-                continue
-            budget = getattr(settings, "hot_max_tokens", None)
-            if isinstance(budget, int) and budget > 0:
-                project_budgets.append(budget)
-        if project_budgets:
-            return max(project_budgets)
+                self._logger.warning(
+                    "hot_max_tokens_lookup_failed",
+                    project_name=namespaced_name,
+                    exc_info=True,
+                )
+
+            if row is not None and row.context_policy_json is not None:
+                try:
+                    config_dict = json.loads(cast(str, row.context_policy_json))
+                    tokens = config_dict.get("hot_max_tokens")
+                    if tokens is not None and isinstance(tokens, int) and tokens > 0:
+                        candidates.append(tokens)
+                except (ValueError, TypeError):
+                    self._logger.warning(
+                        "hot_max_tokens_parse_failed",
+                        project_name=namespaced_name,
+                    )
+
+        if candidates:
+            effective = max(candidates)
+            self._logger.info(
+                "hot_max_tokens_resolved_from_projects",
+                project_names=project_names,
+                project_values=candidates,
+                effective=effective,
+            )
+            return effective
+        # No project overrides --- use the caller-passed global default.
         return self._hot_max_tokens
 
     @staticmethod
@@ -174,7 +217,6 @@ class ACMSExecutePhaseContextAssembler(ExecutePhaseContextAssembler):
             return None
 
         views = {name: self._resolve_execute_view(name) for name in project_names}
-        effective_budget = self._resolve_effective_budget(project_names)
         scoped = self._tier.get_scoped_view(project_names)
         if not scoped:
             return None
@@ -251,14 +293,21 @@ class ACMSExecutePhaseContextAssembler(ExecutePhaseContextAssembler):
             )
             return None
 
-        budget = CoreContextBudget(max_tokens=effective_budget, reserved_tokens=0)
+        # Resolve effective hot_max_tokens: project-level overrides take precedence
+        # over the global default.  When multiple projects have explicit values,
+        # use the maximum so all projects can contribute within their biggest budget.
+        effective_hot_max_tokens = self._resolve_hot_max_tokens(project_names)
+
+        budget = CoreContextBudget(
+            max_tokens=effective_hot_max_tokens, reserved_tokens=0
+        )
         request = ContextRequest(
             query=(
                 f"Execute-phase context for plan {plan.identity.plan_id} "
                 f"({', '.join(project_names)})"
             ),
             purpose="llm_execute_phase_prompt",
-            max_tokens=effective_budget,
+            max_tokens=effective_hot_max_tokens,
         )
         payload = self._pipeline.assemble(
             plan_id=plan.identity.plan_id,