feat(acms): plan execution leverages ACMS context for LLM calls #1163

Merged
aditya merged 1 commit from feature/m5-acms-llm-integration into master 2026-03-30 10:12:06 +00:00
Member

Summary

  • Integrates ACMS execute-phase context assembly into LLMExecuteActor so execute-time LLM prompts use assembled, scoped context rather than raw file data.
  • Wires execute-phase context assembly through plan CLI composition (_get_plan_executor) using policy-aware views and budget constraints.
  • Adds Behave and Robot coverage for execute-context injection, fallback-on-failure behavior, and empty-context handling.

Why

M5 requires plan execution to consume ACMS-assembled context with execute-phase view constraints and budget enforcement, improving relevance and debuggability of LLM execution calls.

Validation

  • nox -e lint
  • nox -e typecheck
  • nox -e unit_tests
  • nox -e integration_tests
  • nox -e coverage_report (97%)

Closes #850

## Summary - Integrates ACMS execute-phase context assembly into `LLMExecuteActor` so execute-time LLM prompts use assembled, scoped context rather than raw file data. - Wires execute-phase context assembly through plan CLI composition (`_get_plan_executor`) using policy-aware views and budget constraints. - Adds Behave and Robot coverage for execute-context injection, fallback-on-failure behavior, and empty-context handling. ## Why M5 requires plan execution to consume ACMS-assembled context with execute-phase view constraints and budget enforcement, improving relevance and debuggability of LLM execution calls. ## Validation - `nox -e lint` - `nox -e typecheck` - `nox -e unit_tests` - `nox -e integration_tests` - `nox -e coverage_report` (97%) Closes #850
aditya added this to the v3.4.0 milestone 2026-03-26 09:55:52 +00:00
aditya requested review from freemo 2026-03-26 13:18:01 +00:00
aditya force-pushed feature/m5-acms-llm-integration from 6155982275
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 25s
CI / lint (pull_request) Successful in 3m19s
CI / quality (pull_request) Successful in 3m49s
CI / typecheck (pull_request) Successful in 4m3s
CI / security (pull_request) Successful in 4m4s
CI / integration_tests (pull_request) Successful in 8m55s
CI / unit_tests (pull_request) Successful in 9m9s
CI / e2e_tests (pull_request) Successful in 10m13s
CI / docker (pull_request) Successful in 1m7s
CI / coverage (pull_request) Successful in 11m59s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-regression (pull_request) Successful in 54m19s
to 25e78474af
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 27s
CI / lint (pull_request) Successful in 4m10s
CI / typecheck (pull_request) Successful in 4m49s
CI / quality (pull_request) Successful in 4m50s
CI / security (pull_request) Successful in 5m1s
CI / integration_tests (pull_request) Successful in 9m38s
CI / unit_tests (pull_request) Successful in 10m4s
CI / e2e_tests (pull_request) Successful in 10m12s
CI / docker (pull_request) Successful in 1m11s
CI / coverage (pull_request) Successful in 11m25s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-regression (pull_request) Successful in 57m7s
2026-03-26 13:20:45 +00:00
Compare
Owner

Code Review Note

Unable to review — the branch feature/m5-acms-llm-integration was not found on the remote. Please verify the branch exists and has been pushed.

## Code Review Note **Unable to review** — the branch `feature/m5-acms-llm-integration` was not found on the remote. Please verify the branch exists and has been pushed.
freemo left a comment

Review: REQUEST CHANGES

Issue 1: llm_actors.py Exceeds 500-Line Limit

With the ACMSExecutePhaseContextAssembler addition (~200 lines), llm_actors.py is now ~600+ lines. Per CONTRIBUTING.md §Modular Design: "Keep files under 500 lines. Break large files into focused, cohesive modules."

Suggestion: Extract ACMSExecutePhaseContextAssembler into its own module (e.g., acms_context_assembler.py). It has a distinct responsibility (context assembly) separate from LLM actor execution.

Issue 2: Raw SQL Instead of Repository Pattern

_read_policy_blob uses raw SQL queries (session.execute(text("SELECT ..."))) instead of going through the repository layer. Per CONTRIBUTING.md §Clean Architecture: "Separate concerns, maintain clear boundaries between layers."

Use the existing NamespacedProjectRepository or ConfigService to read project policy data instead of raw SQL in the assembler.

Issue 3: Type Safety Regression

ProviderRegistry type import was changed from a TYPE_CHECKING-guarded import to Any — this loses type safety. Restore the proper type annotation.

Minor Notes

  • PurePath.full_match is Python 3.13+ — verify this matches the project's compatibility target (the project does target 3.13, so this is likely fine).
  • The graceful fallback on assembly failure (log and proceed without context) is a good pattern.
  • BDD scenarios and Robot integration test are included.
## Review: REQUEST CHANGES ### Issue 1: `llm_actors.py` Exceeds 500-Line Limit With the `ACMSExecutePhaseContextAssembler` addition (~200 lines), `llm_actors.py` is now **~600+ lines**. Per CONTRIBUTING.md §Modular Design: "Keep files under 500 lines. Break large files into focused, cohesive modules." **Suggestion:** Extract `ACMSExecutePhaseContextAssembler` into its own module (e.g., `acms_context_assembler.py`). It has a distinct responsibility (context assembly) separate from LLM actor execution. ### Issue 2: Raw SQL Instead of Repository Pattern `_read_policy_blob` uses raw SQL queries (`session.execute(text("SELECT ..."))`) instead of going through the repository layer. Per CONTRIBUTING.md §Clean Architecture: "Separate concerns, maintain clear boundaries between layers." Use the existing `NamespacedProjectRepository` or `ConfigService` to read project policy data instead of raw SQL in the assembler. ### Issue 3: Type Safety Regression `ProviderRegistry` type import was changed from a `TYPE_CHECKING`-guarded import to `Any` — this loses type safety. Restore the proper type annotation. ### Minor Notes - `PurePath.full_match` is Python 3.13+ — verify this matches the project's compatibility target (the project does target 3.13, so this is likely fine). - The graceful fallback on assembly failure (log and proceed without context) is a good pattern. - BDD scenarios and Robot integration test are included.
Owner

Day 50 Planning — Branch availability required.

The branch feature/m5-acms-llm-integration was reported as not found on remote since Day 48. This PR enables plan execution to leverage ACMS context for LLM calls (v3.4.0).

@freemo — Please push the branch or confirm status. This is critical for v3.4.0 which is already 24+ days overdue. Reviewers assigned.

Day 50 Planning — **Branch availability required.** The branch `feature/m5-acms-llm-integration` was reported as not found on remote since Day 48. This PR enables plan execution to leverage ACMS context for LLM calls (v3.4.0). @freemo — Please push the branch or confirm status. This is critical for v3.4.0 which is already 24+ days overdue. Reviewers assigned.
aditya force-pushed feature/m5-acms-llm-integration from 25e78474af
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 27s
CI / lint (pull_request) Successful in 4m10s
CI / typecheck (pull_request) Successful in 4m49s
CI / quality (pull_request) Successful in 4m50s
CI / security (pull_request) Successful in 5m1s
CI / integration_tests (pull_request) Successful in 9m38s
CI / unit_tests (pull_request) Successful in 10m4s
CI / e2e_tests (pull_request) Successful in 10m12s
CI / docker (pull_request) Successful in 1m11s
CI / coverage (pull_request) Successful in 11m25s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-regression (pull_request) Successful in 57m7s
to f0442e835d
All checks were successful
CI / typecheck (pull_request) Successful in 50s
CI / lint (pull_request) Successful in 3m31s
CI / security (pull_request) Successful in 4m14s
CI / build (pull_request) Successful in 26s
CI / quality (pull_request) Successful in 3m45s
CI / helm (pull_request) Successful in 40s
CI / integration_tests (pull_request) Successful in 3m57s
CI / unit_tests (pull_request) Successful in 4m24s
CI / docker (pull_request) Successful in 1m29s
CI / coverage (pull_request) Successful in 10m54s
CI / benchmark-publish (pull_request) Has been skipped
CI / e2e_tests (pull_request) Successful in 15m35s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-regression (pull_request) Successful in 1h0m6s
2026-03-30 09:25:39 +00:00
Compare
Author
Member

Addressed @freemo's requested changes on f0442e835dc8154911459fa91c67e3ba98ea022b after rebasing feature/m5-acms-llm-integration onto the latest master.

  • Extracted execute-phase ACMS assembly into src/cleveragents/application/services/execute_phase_context_assembler.py so llm_actors.py stays focused and under the file-size guideline.
  • Replaced the assembler's direct SQL lookup with NamespacedProjectRepository.get_context_policy(...), so project policy reads now go through the repository layer.
  • Restored typed ProviderRegistry annotations in the LLM actors and updated the test doubles accordingly.
  • Kept the existing execute-context Behave coverage and the targeted Robot verification for execute-phase ACMS prompt injection aligned with the refactor.
  • Re-pushed the PR branch, so the branch-availability comments should now be resolved as well.

Targeted validation run locally:

  • TEST_PROCESSES=9 nox -e lint
  • TEST_PROCESSES=9 nox -e typecheck
  • TEST_PROCESSES=9 nox -e unit_tests
  • TEST_PROCESSES=9 nox -e integration_tests
Addressed @freemo's requested changes on `f0442e835dc8154911459fa91c67e3ba98ea022b` after rebasing `feature/m5-acms-llm-integration` onto the latest `master`. - Extracted execute-phase ACMS assembly into `src/cleveragents/application/services/execute_phase_context_assembler.py` so `llm_actors.py` stays focused and under the file-size guideline. - Replaced the assembler's direct SQL lookup with `NamespacedProjectRepository.get_context_policy(...)`, so project policy reads now go through the repository layer. - Restored typed `ProviderRegistry` annotations in the LLM actors and updated the test doubles accordingly. - Kept the existing execute-context Behave coverage and the targeted Robot verification for execute-phase ACMS prompt injection aligned with the refactor. - Re-pushed the PR branch, so the branch-availability comments should now be resolved as well. Targeted validation run locally: - `TEST_PROCESSES=9 nox -e lint` - `TEST_PROCESSES=9 nox -e typecheck` - `TEST_PROCESSES=9 nox -e unit_tests` - `TEST_PROCESSES=9 nox -e integration_tests`
aditya scheduled this pull request to auto merge when all checks succeed 2026-03-30 09:52:35 +00:00
aditya merged commit a2546b06b1 into master 2026-03-30 10:12:06 +00:00
aditya deleted branch feature/m5-acms-llm-integration 2026-03-30 10:12:06 +00:00
Author
Member

Hey @freemo , I merged the PR after addressing all comments, but realized it didn’t get formal approval. Please take a look. If there are any remaining issues in this PR, feel free to raise a new issue and I’ll be happy to work on it.

Hey @freemo , I merged the PR after addressing all comments, but realized it didn’t get formal approval. Please take a look. If there are any remaining issues in this PR, feel free to raise a new issue and I’ll be happy to work on it.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core!1163
No description provided.