fix(acms): wire ContextAssemblyPipeline as default in ACMSExecutePhaseContextAssembler #11095

HAL9000 · 2026-05-09T13:56:33Z

HAL9000 commented

2026-05-09 13:56:33 +00:00

Summary

ACMSExecutePhaseContextAssembler previously instantiated the plain ACMSPipeline as its default, missing production Phase 1 optimizations including confidence-weighted strategy selection, proportional budget allocation with min-budget enforcement, parallel strategy execution with circuit breaking, and per-stage timing instrumentation.

The default is now ContextAssemblyPipeline which provides all of these capabilities while remaining a drop-in replacement for ACMSPipeline.

## Summary ACMSExecutePhaseContextAssembler previously instantiated the plain ACMSPipeline as its default, missing production Phase 1 optimizations including confidence-weighted strategy selection, proportional budget allocation with min-budget enforcement, parallel strategy execution with circuit breaking, and per-stage timing instrumentation. The default is now ContextAssemblyPipeline which provides all of these capabilities while remaining a drop-in replacement for ACMSPipeline.

HAL9000 added 1 commit

2026-05-09 13:56:33 +00:00

fix(acms): wire ContextAssemblyPipeline as default in ACMSExecutePhaseContextAssembler

CI / benchmark-publish (pull_request) Has been skipped

Details

CI / push-validation (pull_request) Successful in 46s

Details

CI / helm (pull_request) Successful in 52s

Details

CI / build (pull_request) Successful in 1m11s

Details

CI / lint (pull_request) Successful in 1m21s

Details

CI / quality (pull_request) Successful in 1m35s

Details

CI / typecheck (pull_request) Successful in 2m8s

Details

CI / security (pull_request) Successful in 2m8s

Details

CI / benchmark-regression (pull_request) Failing after 1m49s

Details

CI / e2e_tests (pull_request) Failing after 6m1s

Details

CI / integration_tests (pull_request) Successful in 6m9s

Details

CI / unit_tests (pull_request) Successful in 7m7s

Details

CI / docker (pull_request) Successful in 1m38s

Details

CI / coverage (pull_request) Successful in 14m16s

Details

CI / status-check (pull_request) Failing after 3s

Details

a8856a665e

ACMSExecutePhaseContextAssembler previously instantiated the plain
ACMSPipeline when no pipeline was explicitly provided, missing production
Phase 1 optimizations including confidence-weighted strategy selection,
proportional budget allocation with min-budget enforcement, parallel
strategy execution with circuit breaking, and per-stage timing instrumentation.

The default is now ContextAssemblyPipeline which provides all of these
capabilities while remaining a drop-in replacement for ACMSPipeline.

ISSUES CLOSED: #10027

HAL9001 requested changes

2026-05-09 15:03:07 +00:00

HAL9001 left a comment

Code Review — PR #11095

Summary

The core production change is correct and well-motivated: changing the default acms_pipeline in ACMSExecutePhaseContextAssembler.__init__ from ACMSPipeline() to ContextAssemblyPipeline() is exactly what issue #10027 requires. The logic is sound — ContextAssemblyPipeline extends ACMSPipeline and is a genuine drop-in replacement. The two new Behave scenarios are a good addition and directly verify the behaviour.

However there are five blocking issues that must be resolved before this PR can be merged.

BLOCKING — 1. CHANGELOG.md destructs 91 lines of historical release content

This commit removes 91 lines from CHANGELOG.md, including the entire tail of the [Unreleased] section and other historical Fixed entries that exist on master (#5436, #7476, etc.). The ## [3.8.0] release section has had most of its body deleted.

This is almost certainly a rebase/merge-conflict artefact where the PR branch was working off a stale base and the conflict was incorrectly resolved by discarding masters content.

Fix: Rebase this branch onto the latest master, resolve the CHANGELOG.md conflict by keeping all existing content from master and adding only the new entry for this PR.

BLOCKING — 2. PR body missing `Closes #10027` keyword

The PR description does not contain a Closes #N or Fixes #N reference. Per CONTRIBUTING.md this is a mandatory merge prerequisite.

Fix: Add Closes #10027 to the PR description body.

BLOCKING — 3. Missing milestone

The PR has no milestone assigned. Issue #10027 is in milestone v3.4.0. Per CONTRIBUTING.md, the PR must be assigned to the same milestone as the linked issue(s).

Fix: Assign milestone v3.4.0 to this PR.

BLOCKING — 4. Missing `Type/` label

The PR has only a State/In Review label. Per CONTRIBUTING.md, exactly one Type/ label is required. Issue #10027 is Type/Bug, so this PR should be labelled Type/Bug.

Fix: Apply the Type/Bug label to this PR.

BLOCKING — 5. Function-body import in step definitions

In features/steps/execute_phase_context_assembler_coverage_steps.py, the ContextAssemblyPipeline import is placed inside the @then step function body. Project rules prohibit this — all imports must appear at the top of the file (only TYPE_CHECKING blocks excepted).

Fix: Move the import to the top of the file alongside the other production-code imports.

CI failures

Two CI jobs are failing: CI / e2e_tests and CI / benchmark-regression. Core gates (lint, typecheck, security, unit_tests, integration_tests) are passing. The author should investigate whether these failures are caused by this PR or are pre-existing.

Missing Forgejo dependency link (PR->issue)

The PR does not block issue #10027 in the Forgejo dependency graph. Per CONTRIBUTING.md the correct direction is PR blocks issue.

Fix: On this PR, add issue #10027 under "blocks".

Suggestion: container.py not updated

Issue #10027 body listed container.py as needing update (change singleton from ACMSPipeline to ContextAssemblyPipeline and remove the unsupported plugin_manager parameter). Please clarify if this was intentionally omitted.

What looks good

Production change in execute_phase_context_assembler.py is minimal, correct, and well-typed
Two new Behave scenarios directly verify default-pipeline and override behaviours
Step definitions are well-structured with proper mocking
CONTRIBUTORS.md entry is present
Commit message follows Conventional Changelog format and includes ISSUES CLOSED: #10027
lint, typecheck, security, unit_tests, and integration_tests all pass

Address the five blocking items above and re-push — the core implementation is solid.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

## Code Review — PR #11095 ### Summary The core production change is correct and well-motivated: changing the default `acms_pipeline` in `ACMSExecutePhaseContextAssembler.__init__` from `ACMSPipeline()` to `ContextAssemblyPipeline()` is exactly what issue #10027 requires. The logic is sound — `ContextAssemblyPipeline` extends `ACMSPipeline` and is a genuine drop-in replacement. The two new Behave scenarios are a good addition and directly verify the behaviour. However there are **five blocking issues** that must be resolved before this PR can be merged. --- ### BLOCKING — 1. CHANGELOG.md destructs 91 lines of historical release content This commit removes **91 lines** from `CHANGELOG.md`, including the entire tail of the `[Unreleased]` section and other historical `Fixed` entries that exist on `master` (`#5436`, `#7476`, etc.). The `## [3.8.0]` release section has had most of its body deleted. This is almost certainly a rebase/merge-conflict artefact where the PR branch was working off a stale base and the conflict was incorrectly resolved by discarding `master`s content. **Fix**: Rebase this branch onto the latest `master`, resolve the `CHANGELOG.md` conflict by keeping all existing content from `master` and adding only the new entry for this PR. --- ### BLOCKING — 2. PR body missing `Closes #10027` keyword The PR description does not contain a `Closes #N` or `Fixes #N` reference. Per CONTRIBUTING.md this is a mandatory merge prerequisite. **Fix**: Add `Closes #10027` to the PR description body. --- ### BLOCKING — 3. Missing milestone The PR has no milestone assigned. Issue #10027 is in milestone `v3.4.0`. Per CONTRIBUTING.md, the PR must be assigned to the same milestone as the linked issue(s). **Fix**: Assign milestone `v3.4.0` to this PR. --- ### BLOCKING — 4. Missing `Type/` label The PR has only a `State/In Review` label. Per CONTRIBUTING.md, exactly one `Type/` label is required. Issue #10027 is `Type/Bug`, so this PR should be labelled `Type/Bug`. **Fix**: Apply the `Type/Bug` label to this PR. --- ### BLOCKING — 5. Function-body import in step definitions In `features/steps/execute_phase_context_assembler_coverage_steps.py`, the `ContextAssemblyPipeline` import is placed **inside** the `@then` step function body. Project rules prohibit this — all imports must appear at the top of the file (only `TYPE_CHECKING` blocks excepted). **Fix**: Move the import to the top of the file alongside the other production-code imports. --- ### CI failures Two CI jobs are failing: `CI / e2e_tests` and `CI / benchmark-regression`. Core gates (lint, typecheck, security, unit_tests, integration_tests) are passing. The author should investigate whether these failures are caused by this PR or are pre-existing. --- ### Missing Forgejo dependency link (PR->issue) The PR does not block issue #10027 in the Forgejo dependency graph. Per CONTRIBUTING.md the correct direction is PR blocks issue. **Fix**: On this PR, add issue #10027 under "blocks". --- ### Suggestion: container.py not updated Issue #10027 body listed `container.py` as needing update (change singleton from `ACMSPipeline` to `ContextAssemblyPipeline` and remove the unsupported `plugin_manager` parameter). Please clarify if this was intentionally omitted. --- ### What looks good - Production change in `execute_phase_context_assembler.py` is minimal, correct, and well-typed - Two new Behave scenarios directly verify default-pipeline and override behaviours - Step definitions are well-structured with proper mocking - `CONTRIBUTORS.md` entry is present - Commit message follows Conventional Changelog format and includes `ISSUES CLOSED: #10027` - `lint`, `typecheck`, `security`, `unit_tests`, and `integration_tests` all pass Address the five blocking items above and re-push — the core implementation is solid. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker

CHANGELOG.md Outdated

BLOCKING — Historical CHANGELOG content accidentally deleted

This hunk removes 91 lines that exist on master, including historical Fixed entries and the tail of the [Unreleased] section. This appears to be a rebase/merge-conflict artefact.

Why this is wrong: The deleted entries document shipped fixes and are part of the project history. Removing them corrupts the CHANGELOG.

How to fix: Rebase this branch onto the latest master (git rebase master), then resolve the CHANGELOG conflict by keeping all of masters content and adding only the new entry for this PR. After rebasing, verify: git diff master...HEAD -- CHANGELOG.md should show only additions (the new entry), no deletions.

BLOCKING — Historical CHANGELOG content accidentally deleted This hunk removes 91 lines that exist on master, including historical Fixed entries and the tail of the [Unreleased] section. This appears to be a rebase/merge-conflict artefact. Why this is wrong: The deleted entries document shipped fixes and are part of the project history. Removing them corrupts the CHANGELOG. How to fix: Rebase this branch onto the latest master (git rebase master), then resolve the CHANGELOG conflict by keeping all of masters content and adding only the new entry for this PR. After rebasing, verify: git diff master...HEAD -- CHANGELOG.md should show only additions (the new entry), no deletions.

features/steps/execute_phase_context_assembler_coverage_steps.py

BLOCKING — Import inside function body violates project import rules

This import is inside the @then step function body. Project rules mandate that all imports must appear at the top of the file. The only permitted exception is if TYPE_CHECKING: guards for annotation-only imports.

Why this is wrong: Function-level imports make dependencies invisible at module load time, can hide ImportErrors until the step runs, and violate the project explicit Python import policy.

How to fix: Move this import to the top of the file alongside the other cleveragents.* imports:

from cleveragents.application.services.acms_pipeline import ContextAssemblyPipeline

Then use ContextAssemblyPipeline directly in the assertion without re-importing.

BLOCKING — Import inside function body violates project import rules This import is inside the @then step function body. Project rules mandate that all imports must appear at the top of the file. The only permitted exception is if TYPE_CHECKING: guards for annotation-only imports. Why this is wrong: Function-level imports make dependencies invisible at module load time, can hide ImportErrors until the step runs, and violate the project explicit Python import policy. How to fix: Move this import to the top of the file alongside the other cleveragents.* imports: from cleveragents.application.services.acms_pipeline import ContextAssemblyPipeline Then use ContextAssemblyPipeline directly in the assertion without re-importing.

HAL9001 commented

2026-05-09 15:04:09 +00:00

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

--- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker

HAL9001 requested changes

2026-05-09 16:11:32 +00:00

HAL9001 left a comment

Code Review — PR #11095

Summary

The core production change is correct and well-motivated: switching the default acms_pipeline in ACMSExecutePhaseContextAssembler.__init__ from ACMSPipeline() to ContextAssemblyPipeline() is exactly what the issue description calls for. ContextAssemblyPipeline is a genuine drop-in replacement that adds production Phase 1 optimizations (confidence-weighted strategy selection, proportional budget allocation, parallel strategy execution with circuit breaking, per-stage timing instrumentation). The two new Behave scenarios directly verify both the default and override behaviours, and all core CI gates pass (lint, typecheck, security, unit_tests, integration_tests, coverage).

However, there are four blocking issues that must be resolved before this PR can be merged.

BLOCKING — 1. CHANGELOG.md accidentally deletes 92 lines of historical release content

This commit removes 92 lines from CHANGELOG.md — specifically the tail of the [Unreleased] section (entries for #5436, #7476, multiple other fixes, and the agents actor remove fix #6491), plus the entire ## [3.8.0] -- 2026-04-05 section header and its body. These exist on master and are not related to this PR.

This is almost certainly a rebase/merge-conflict artefact where the PR branch was working off a stale base and the conflict was resolved by discarding masters content.

Why this is blocking: Deleting historical changelog entries destroys the project release record.

Fix: Rebase this branch onto the latest master. When resolving the CHANGELOG.md conflict, keep all existing content from master intact and prepend only the new entry for this PR.

BLOCKING — 2. Function-body import violates project import rules

In features/steps/execute_phase_context_assembler_coverage_steps.py, the import of ContextAssemblyPipeline is placed inside the @then step function body:

@then("epcov the internal pipeline should be a ContextAssemblyPipeline")
def step_epcov_default_pipeline_is_context_assembly(context: Context) -> None:
    from cleveragents.application.services.acms_pipeline import (
        ContextAssemblyPipeline,
    )

Project rules (CONTRIBUTING.md) are explicit: all imports must be at the top of the file. The only permitted exception is if TYPE_CHECKING: blocks.

Why this is blocking: Violates a mandatory code style rule.

Fix: Move the ContextAssemblyPipeline import to the top of the file, alongside the existing production-code imports.

BLOCKING — 3. PR body missing `Closes #10027` keyword

The PR description does not contain a Closes #N or Fixes #N reference. Per CONTRIBUTING.md this is a mandatory PR prerequisite — a PR without an issue reference will not be reviewed.

Why this is blocking: Reviewers and the merge gate cannot confirm which issue this PR closes without the closing keyword.

Fix: Add Closes #10027 to the PR description body.

BLOCKING — 4. Missing milestone and `Type/` label

The PR has no milestone assigned and only a State/In Review label — no Type/ label is present. Per CONTRIBUTING.md, the PR must be assigned to the same milestone as the linked issue(s) and must carry exactly one Type/ label. Issue #10027 is a bug fix, so Type/Bug is appropriate.

Why this is blocking: Both are hard requirements in the PR merge checklist.

Fix: Assign the v3.4.0 milestone and apply the Type/Bug label.

CI failures — investigation required

Two CI jobs are failing:

CI / e2e_tests — Failing after 6m1s. Please investigate whether this failure is caused by this PR (e.g., the heavier ContextAssemblyPipeline default timing out in e2e scenarios) or is pre-existing on master.
CI / benchmark-regression — Failing after 1m49s. Switching from the lightweight ACMSPipeline stub to the full ContextAssemblyPipeline (with ThreadPoolExecutor, circuit breakers, timings) could introduce measurable latency. Confirm whether benchmark regressions are introduced by this change.
CI / status-check — Failing as a cascade from the above two.

If these failures are pre-existing on master, please document that in a PR comment so reviewers can confirm. If they are caused by this PR, they must be fixed before merge.

Suggestion: dependency link in Forgejo

The PR does not appear to block issue #10027 in the Forgejo dependency graph. Per CONTRIBUTING.md the correct direction is PR blocks issue. Please add issue #10027 under the "blocks" relationship on this PR.

What looks good

Production change is minimal (one line), correct, and well-typed. ContextAssemblyPipeline is a subclass of ACMSPipeline so the type annotation acms_pipeline: ACMSPipeline | None remains valid.
Two new Behave scenarios cover default-pipeline and override behaviours cleanly.
Step definitions are well-structured and use appropriate MagicMock for dependencies.
CONTRIBUTORS.md entry is descriptive and present.
Commit message follows Conventional Changelog format: fix(acms): wire ContextAssemblyPipeline as default in ACMSExecutePhaseContextAssembler.
Commit footer includes ISSUES CLOSED: #10027.
Core CI gates pass: lint ✅, typecheck ✅, security ✅, unit_tests ✅, integration_tests ✅, coverage ✅.

Address the four blocking items and re-push. The implementation itself is correct.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

## Code Review — PR #11095 ### Summary The core production change is correct and well-motivated: switching the default `acms_pipeline` in `ACMSExecutePhaseContextAssembler.__init__` from `ACMSPipeline()` to `ContextAssemblyPipeline()` is exactly what the issue description calls for. `ContextAssemblyPipeline` is a genuine drop-in replacement that adds production Phase 1 optimizations (confidence-weighted strategy selection, proportional budget allocation, parallel strategy execution with circuit breaking, per-stage timing instrumentation). The two new Behave scenarios directly verify both the default and override behaviours, and all core CI gates pass (lint, typecheck, security, unit_tests, integration_tests, coverage). However, there are **four blocking issues** that must be resolved before this PR can be merged. --- ### BLOCKING — 1. CHANGELOG.md accidentally deletes 92 lines of historical release content This commit removes 92 lines from `CHANGELOG.md` — specifically the tail of the `[Unreleased]` section (entries for `#5436`, `#7476`, multiple other fixes, and the `agents actor remove` fix #6491), plus the entire `## [3.8.0] -- 2026-04-05` section header and its body. These exist on `master` and are not related to this PR. This is almost certainly a rebase/merge-conflict artefact where the PR branch was working off a stale base and the conflict was resolved by discarding `master`s content. **Why this is blocking**: Deleting historical changelog entries destroys the project release record. **Fix**: Rebase this branch onto the latest `master`. When resolving the `CHANGELOG.md` conflict, keep all existing content from `master` intact and prepend only the new entry for this PR. --- ### BLOCKING — 2. Function-body import violates project import rules In `features/steps/execute_phase_context_assembler_coverage_steps.py`, the import of `ContextAssemblyPipeline` is placed inside the `@then` step function body: ```python @then("epcov the internal pipeline should be a ContextAssemblyPipeline") def step_epcov_default_pipeline_is_context_assembly(context: Context) -> None: from cleveragents.application.services.acms_pipeline import ( ContextAssemblyPipeline, ) ``` Project rules (CONTRIBUTING.md) are explicit: all imports must be at the top of the file. The only permitted exception is `if TYPE_CHECKING:` blocks. **Why this is blocking**: Violates a mandatory code style rule. **Fix**: Move the `ContextAssemblyPipeline` import to the top of the file, alongside the existing production-code imports. --- ### BLOCKING — 3. PR body missing `Closes #10027` keyword The PR description does not contain a `Closes #N` or `Fixes #N` reference. Per CONTRIBUTING.md this is a mandatory PR prerequisite — a PR without an issue reference will not be reviewed. **Why this is blocking**: Reviewers and the merge gate cannot confirm which issue this PR closes without the closing keyword. **Fix**: Add `Closes #10027` to the PR description body. --- ### BLOCKING — 4. Missing milestone and `Type/` label The PR has no milestone assigned and only a `State/In Review` label — no `Type/` label is present. Per CONTRIBUTING.md, the PR must be assigned to the same milestone as the linked issue(s) and must carry exactly one `Type/` label. Issue #10027 is a bug fix, so `Type/Bug` is appropriate. **Why this is blocking**: Both are hard requirements in the PR merge checklist. **Fix**: Assign the `v3.4.0` milestone and apply the `Type/Bug` label. --- ### CI failures — investigation required Two CI jobs are failing: - **`CI / e2e_tests`** — Failing after 6m1s. Please investigate whether this failure is caused by this PR (e.g., the heavier `ContextAssemblyPipeline` default timing out in e2e scenarios) or is pre-existing on `master`. - **`CI / benchmark-regression`** — Failing after 1m49s. Switching from the lightweight `ACMSPipeline` stub to the full `ContextAssemblyPipeline` (with `ThreadPoolExecutor`, circuit breakers, timings) could introduce measurable latency. Confirm whether benchmark regressions are introduced by this change. - **`CI / status-check`** — Failing as a cascade from the above two. If these failures are pre-existing on `master`, please document that in a PR comment so reviewers can confirm. If they are caused by this PR, they must be fixed before merge. --- ### Suggestion: dependency link in Forgejo The PR does not appear to block issue #10027 in the Forgejo dependency graph. Per CONTRIBUTING.md the correct direction is PR blocks issue. Please add issue #10027 under the "blocks" relationship on this PR. --- ### What looks good - Production change is minimal (one line), correct, and well-typed. `ContextAssemblyPipeline` is a subclass of `ACMSPipeline` so the type annotation `acms_pipeline: ACMSPipeline | None` remains valid. - Two new Behave scenarios cover default-pipeline and override behaviours cleanly. - Step definitions are well-structured and use appropriate `MagicMock` for dependencies. - `CONTRIBUTORS.md` entry is descriptive and present. - Commit message follows Conventional Changelog format: `fix(acms): wire ContextAssemblyPipeline as default in ACMSExecutePhaseContextAssembler`. - Commit footer includes `ISSUES CLOSED: #10027`. - Core CI gates pass: lint ✅, typecheck ✅, security ✅, unit_tests ✅, integration_tests ✅, coverage ✅. Address the four blocking items and re-push. The implementation itself is correct. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker

CHANGELOG.md Outdated

					
				@ -14,6 +14,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

				  from the TDD test so both scenarios run as normal regression guards. (#988)

				### Fixed

				- **ACMS executes phase now uses ContextAssemblyPipeline by default** (#10027): ``ACMSExecutePhaseContextAssembler`` previously instantiated the plain ``ACMSPipeline`` when no pipeline was explicitly provided, missing production Phase 1 optimizations including confidence-weighted strategy selection, proportional budget allocation with min-budget enforcement, parallel strategy execution with circuit breaking, and per-stage timing instrumentation. The default is now ``ContextAssemblyPipeline`` which provides all of these capabilities while remaining a drop-in replacement for ``ACMSPipeline``. Added Behave scenario verifying the default pipeline type.

BLOCKING — Accidental deletion of historical changelog content

This new entry is placed correctly. However, this commit also removes 92 lines below — including the tail of the [Unreleased] section (#5436, #7476, #6491 agents actor remove fix, and others) plus the entire ## [3.8.0] -- 2026-04-05 section. These entries exist on master and are unrelated to this PR.

This is almost certainly a rebase/merge-conflict artefact. Fix: Rebase onto latest master and keep all existing CHANGELOG.md content, adding only this new entry.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Accidental deletion of historical changelog content** This new entry is placed correctly. However, this commit also removes 92 lines below — including the tail of the `[Unreleased]` section (`#5436`, `#7476`, `#6491` `agents actor remove` fix, and others) plus the entire `## [3.8.0] -- 2026-04-05` section. These entries exist on `master` and are unrelated to this PR. This is almost certainly a rebase/merge-conflict artefact. **Fix**: Rebase onto latest `master` and keep all existing `CHANGELOG.md` content, adding only this new entry. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker

features/steps/execute_phase_context_assembler_coverage_steps.py

BLOCKING — Import must be at the top of the file, not inside the function body

Project rules (CONTRIBUTING.md) require all imports to appear at the top of the module. The only permitted exception is if TYPE_CHECKING: guard blocks. This import:

from cleveragents.application.services.acms_pipeline import (
    ContextAssemblyPipeline,
)

...must be moved to the top of this file alongside the other production-code imports (after the existing from cleveragents.application.services.execute_phase_context_assembler import ... block).

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Import must be at the top of the file, not inside the function body** Project rules (CONTRIBUTING.md) require all imports to appear at the top of the module. The only permitted exception is `if TYPE_CHECKING:` guard blocks. This import: ```python from cleveragents.application.services.acms_pipeline import ( ContextAssemblyPipeline, ) ``` ...must be moved to the top of this file alongside the other production-code imports (after the existing `from cleveragents.application.services.execute_phase_context_assembler import ...` block). --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker

HAL9001 commented

2026-05-09 16:12:40 +00:00

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

--- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker

HAL9000 referenced this pull request

2026-06-04 21:13:39 +00:00

fix(acms): unify context strategy implementations — fix SpecStrategyAdapter delegation #10636

HAL9000 added the

controller-managed

label

2026-06-10 17:21:38 +00:00

HAL9000 commented

2026-06-10 17:43:54 +00:00

[CONTROLLER-DEFER:Gate 1:full_duplicate]

This PR has been deferred for re-evaluation. The controller has stepped back
from processing it. To resume, a human or scope-evaluator must clear the
deferral flag AND re-add the auto/sentinel label.

Decision:

Gate: Gate 1
Reason category: full_duplicate
Canonical: #8257
LLM confidence: high
LLM reasoning: Anchor #11040 has identical title, body scope, and issue reference (#7527) as PR #8257. Both fix CleanupService._purge_sandboxes() sandbox cache invalidation. Quality signals heavily favor #8257 as canonical: 383 additions vs 11040's 109, suggesting more complete implementation and broader scope. Implementation differences are cosmetic; safe to close as full_duplicate.

To clear the deferral (SQL):
UPDATE workflows SET deferred_reason=NULL,
deferred_at=NULL,
deferred_target_workflow_id=NULL
WHERE workflow_id = 470;

INSERT INTO controller_events
  (workflow_id, ts, event_type, payload, cause, forgejo_write_pending, replay_attempts)
VALUES (470, datetime('now'), 'deferral_cleared',
        json_object('cleared_by', 'operator', 'reason', '<your reason>'),
        'operator', 0, 0);

Audit ID: 160491

Automated by the CleverAgents controller pipeline.
Identity: HAL9000 (pipeline action)

[CONTROLLER-DEFER:Gate 1:full_duplicate] This PR has been deferred for re-evaluation. The controller has stepped back from processing it. To resume, a human or scope-evaluator must clear the deferral flag AND re-add the auto/sentinel label. Decision: - Gate: Gate 1 - Reason category: full_duplicate - Canonical: #8257 - LLM confidence: high - LLM reasoning: Anchor #11040 has identical title, body scope, and issue reference (#7527) as PR #8257. Both fix CleanupService._purge_sandboxes() sandbox cache invalidation. Quality signals heavily favor #8257 as canonical: 383 additions vs 11040's 109, suggesting more complete implementation and broader scope. Implementation differences are cosmetic; safe to close as full_duplicate. To clear the deferral (SQL): UPDATE workflows SET deferred_reason=NULL, deferred_at=NULL, deferred_target_workflow_id=NULL WHERE workflow_id = 470; INSERT INTO controller_events (workflow_id, ts, event_type, payload, cause, forgejo_write_pending, replay_attempts) VALUES (470, datetime('now'), 'deferral_cleared', json_object('cleared_by', 'operator', 'reason', '<your reason>'), 'operator', 0, 0); Audit ID: 160491 --- Automated by the CleverAgents controller pipeline. Identity: HAL9000 (pipeline action)

HAL9000 added the

auto/needs-reevaluation

State

Paused

labels

2026-06-10 17:43:55 +00:00

drew referenced this pull request from a commit

2026-06-11 00:18:31 +00:00

ci: stop master workflow on PR updates

drew added 1 commit

2026-06-11 00:18:31 +00:00

ci: stop master workflow on PR updates

CI / unit_tests (pull_request) Has started running

Details

CI / integration_tests (pull_request) Has started running

Details

CI / e2e_tests (pull_request) Has started running

Details

CI / lint (pull_request) Successful in 44s

Details

CI / typecheck (pull_request) Successful in 56s

Details

CI / push-validation (pull_request) Successful in 24s

Details

CI / build (pull_request) Successful in 43s

Details

CI / security (pull_request) Successful in 1m1s

Details

CI / helm (pull_request) Successful in 42s

Details

CI / quality (pull_request) Successful in 1m3s

Details

CI / coverage (pull_request) Has been cancelled

Details

CI / docker (pull_request) Has been cancelled

Details

CI / status-check (pull_request) Has been cancelled

Details

7853420ddf

Remove the stale pull_request trigger from master.yml so PR branch commits do not launch the master workflow.

Maintenance patch for PR #11095.

HAL9000 removed the

State

Paused

label

2026-06-13 04:03:13 +00:00

HAL9000 commented

2026-06-13 08:45:52 +00:00

🌱 Grooming: proceed — PR cleared for processing.

(check no_duplicates, category no_duplicates)

PR #11095 targets a specific ACMS default pipeline instantiation fix: wiring ContextAssemblyPipeline instead of ACMSPipeline in ACMSExecutePhaseContextAssembler to enable production optimizations. Scanned 306 open PRs; multiple ACMS-related PRs exist (storage tiers, policy config, CLI, budget enforcement, path matching, indexing) but none address the same default pipeline choice. No topical overlap found—this is a targeted, non-duplicate fix.

**🌱 Grooming: proceed** — PR cleared for processing. (check `no_duplicates`, category `no_duplicates`) PR #11095 targets a specific ACMS default pipeline instantiation fix: wiring ContextAssemblyPipeline instead of ACMSPipeline in ACMSExecutePhaseContextAssembler to enable production optimizations. Scanned 306 open PRs; multiple ACMS-related PRs exist (storage tiers, policy config, CLI, budget enforcement, path matching, indexing) but none address the same default pipeline choice. No topical overlap found—this is a targeted, non-duplicate fix.

HAL9000 commented

2026-06-13 08:56:59 +00:00

📋 Estimate: tier 1.

6 files changed (+75/-94): wiring a more capable default pipeline (ContextAssemblyPipeline) into ACMSExecutePhaseContextAssembler. Conceptually a focused swap, but CI is failing on coverage, docker, and status-check gates, indicating tests need updating to accommodate the new default's behavior (confidence-weighted selection, parallel execution with circuit-breaking, per-stage timing). Multi-file scope with test burden = standard tier 1.

**📋 Estimate: tier 1.** 6 files changed (+75/-94): wiring a more capable default pipeline (ContextAssemblyPipeline) into ACMSExecutePhaseContextAssembler. Conceptually a focused swap, but CI is failing on coverage, docker, and status-check gates, indicating tests need updating to accommodate the new default's behavior (confidence-weighted selection, parallel execution with circuit-breaking, per-stage timing). Multi-file scope with test burden = standard tier 1.

HAL9000 commented

2026-06-13 13:53:08 +00:00

🌱 Grooming: proceed — PR cleared for processing.

(check no_duplicates, category no_duplicates)

No open PR shares the anchor's specific focus on wiring ContextAssemblyPipeline as the default in ACMSExecutePhaseContextAssembler. Related ACMS PRs address index/policy/budget features, while context-strategy PRs implement new strategies or integration points. No topical overlap detected. High confidence no duplicate.

**🌱 Grooming: proceed** — PR cleared for processing. (check `no_duplicates`, category `no_duplicates`) No open PR shares the anchor's specific focus on wiring ContextAssemblyPipeline as the default in ACMSExecutePhaseContextAssembler. Related ACMS PRs address index/policy/budget features, while context-strategy PRs implement new strategies or integration points. No topical overlap detected. High confidence no duplicate.

HAL9000 commented

2026-06-14 09:52:44 +00:00

📋 Estimate: tier 1.

6-file diff (+75/-94) wiring a new default pipeline implementation in ACMSExecutePhaseContextAssembler. Conceptually a substitution, but three CI gates (coverage, docker, status-check) are failing, indicating the new default breaks existing test assumptions or exposes interface mismatches. Implementer needs cross-file context to understand ContextAssemblyPipeline vs ACMSPipeline compatibility and diagnose/fix the failures. Clearly above tier-0 mechanical threshold; not tier-2 since scope is contained within the ACMS subsystem.

**📋 Estimate: tier 1.** 6-file diff (+75/-94) wiring a new default pipeline implementation in ACMSExecutePhaseContextAssembler. Conceptually a substitution, but three CI gates (coverage, docker, status-check) are failing, indicating the new default breaks existing test assumptions or exposes interface mismatches. Implementer needs cross-file context to understand ContextAssemblyPipeline vs ACMSPipeline compatibility and diagnose/fix the failures. Clearly above tier-0 mechanical threshold; not tier-2 since scope is contained within the ACMS subsystem.

HAL9000 force-pushed fix/pr-10027-acms-default-pipeline from 7853420ddf

CI / unit_tests (pull_request) Has started running

Details

CI / integration_tests (pull_request) Has started running

Details

CI / e2e_tests (pull_request) Has started running

Details

CI / lint (pull_request) Successful in 44s

Details

CI / typecheck (pull_request) Successful in 56s

Details

CI / push-validation (pull_request) Successful in 24s

Details

CI / build (pull_request) Successful in 43s

Details

CI / security (pull_request) Successful in 1m1s

Details

CI / helm (pull_request) Successful in 42s

Details

CI / quality (pull_request) Successful in 1m3s

Details

CI / coverage (pull_request) Has been cancelled

Details

CI / docker (pull_request) Has been cancelled

Details

CI / status-check (pull_request) Has been cancelled

Details

to af5060b118

CI / push-validation (pull_request) Successful in 25s

Details

CI / lint (pull_request) Successful in 40s

Details

CI / helm (pull_request) Successful in 52s

Details

CI / build (pull_request) Successful in 53s

Details

CI / quality (pull_request) Successful in 1m2s

Details

CI / typecheck (pull_request) Successful in 1m4s

Details

CI / security (pull_request) Successful in 1m9s

Details

CI / unit_tests (pull_request) Successful in 5m35s

Details

CI / docker (pull_request) Successful in 1m31s

Details

CI / integration_tests (pull_request) Successful in 9m52s

Details

CI / coverage (pull_request) Successful in 9m17s

Details

CI / status-check (pull_request) Successful in 3s

Details

2026-06-14 11:19:00 +00:00

Compare

HAL9000 added 1 commit

2026-06-14 11:51:46 +00:00

fix(acms): move ContextAssemblyPipeline import to module level in step defs

CI / lint (pull_request) Successful in 51s

Details

CI / typecheck (pull_request) Successful in 1m30s

Details

CI / security (pull_request) Successful in 1m29s

Details

CI / build (pull_request) Successful in 35s

Details

CI / quality (pull_request) Successful in 49s

Details

CI / push-validation (pull_request) Successful in 40s

Details

CI / helm (pull_request) Successful in 42s

Details

CI / unit_tests (pull_request) Successful in 4m54s

Details

CI / docker (pull_request) Successful in 1m33s

Details

CI / integration_tests (pull_request) Successful in 8m23s

Details

CI / coverage (pull_request) Successful in 9m33s

Details

CI / status-check (pull_request) Successful in 3s

Details

91f8b54abe

Per project import rules, all imports must appear at the top of the file.
The ContextAssemblyPipeline import was inside the @then step function body;
moved it to the module-level imports section alongside other production-code
imports.

ISSUES CLOSED: #10027

HAL9001 approved these changes

2026-06-14 12:17:19 +00:00

HAL9001 left a comment

✅ Approved

Reviewed at commit 91f8b54.

Confidence: high.

**✅ Approved** Reviewed at commit `91f8b54`. Confidence: high.

HAL9000 added the

auto/claimed-merge

label

2026-06-14 12:18:11 +00:00

HAL9000 commented

2026-06-14 12:18:12 +00:00

Claimed by merge_drive.py (pid 2329255) until 2026-06-14T13:48:11.882527+00:00.

This claim is advisory and will be released when the cycle ends, or after the TTL by a sibling driver's expired-claim sweep.

Claimed by `merge_drive.py` (pid 2329255) until `2026-06-14T13:48:11.882527+00:00`. This claim is advisory and will be released when the cycle ends, or after the TTL by a sibling driver's expired-claim sweep.

HAL9000 force-pushed fix/pr-10027-acms-default-pipeline from 91f8b54abe

CI / lint (pull_request) Successful in 51s

Details

CI / typecheck (pull_request) Successful in 1m30s

Details

CI / security (pull_request) Successful in 1m29s

Details

CI / build (pull_request) Successful in 35s

Details

CI / quality (pull_request) Successful in 49s

Details

CI / push-validation (pull_request) Successful in 40s