fix(acms): use project-level hot_max_tokens in execute phase context assembly #11216
No reviewers
Labels
No labels
auto/needs-reevaluation
controller-managed
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
cleveragents/cleveragents-core!11216
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "bugfix/m5-acms-project-budget-override"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Re-ships the ACMS project-level hot_max_tokens fix that was stranded after PR #11036 was merged with only the
CONTRIBUTORS.mdentry. This PR delivers the actual code fix.Changes
_resolve_effective_budget()method toACMSExecutePhaseContextAssemblerproject.settings.hot_max_tokensfrom each linked projectmax(project_budgets)when any project has an overrideself._hot_max_tokenswhen no override is setassemble()to use the resolved effective budget for bothCoreContextBudgetandContextRequest@tdd_issue @tdd_issue_11035tags verifying the pipeline receives the project-level budget### Fixedin## [Unreleased]as first itemCRPFragment/CRPProvenanceimports to module-level per project style rulesImpact
Users who set
agents project context set --hot-max-tokens 32000will now actually see source files in context that would previously have been excluded by the 16K default budget.Testing
@tdd_issue @tdd_issue_11035Reviewer Flags Addressed
bugfix/m5-acms-project-budget-override(followsbugfix/mN-convention, milestone v3.5.0)### Fixedin## [Unreleased]@tdd_issue @tdd_issue_11035tags, 2-space indentedCRPFragment/CRPProvenanceat top of step definitions fileDependency Links
Fixes: #11035
Fixes: #11215
cc449e8a7af7d2d6867e[GROOMED] Quality analysis complete.
Checks performed:
Fixes applied:
none
Notes:
Automated by CleverAgents Bot
Supervisor: Grooming | Agent: grooming-worker
[GROOMED] Quality analysis complete.
Checks performed:
Fixes applied:
Notes:
Automated by CleverAgents Bot
Supervisor: Grooming | Agent: grooming-worker
Comment test
[GROOMED] Quality analysis complete.
Checks performed:
Fixes: #11035andFixes: #11215in body (auto-close on merge).Fixes applied:
Patched previous malformed backtick escaping in PR body from prior grooming run (code method names
_resolve_effective_budget(),self._hot_max_tokenscorrectly escaped now).Notes:
/issues/{id}/dependencies) returns HTTP 500 "IsErrRepoNotExist" — explicit block relationships could not be created via API. Closing keywords in PR body and explicit Blocks: documentation line serve as proxy linkage.stale_with_conflicts— CI is failing (ci_status=failing). These are code/CI implementation concerns for the implementor to address before merge.Automated by CleverAgents Bot
Supervisor: Grooming | Agent: grooming-worker
[GROOMED] Quality analysis complete.
Checks performed:
Fixes applied:
Notes:
[GROOMED] Quality analysis complete.
Checks performed:
Fixes applied:
Notes:
Automated by CleverAgents Bot
Supervisor: Grooming | Agent: grooming-worker
f7d2d6867eee3f8fe86a[GROOMED] Quality analysis complete.
PR #11216: fix(acms) - use project-level hot_max_tokens in execute phase context assembly
Branch: bugfix/m5-acms-project-budget-override -> master
Author: hamza.khyari | Milestone: v3.5.0
Checks Performed:
Dependencies via Fixes keywords in PR body (#11035, #11215).
POST /issues/N/dependencies API returned IsErrRepoNotExist.
Issues Found:
Recommendation: Proceed with review once CI passes.
ee3f8fe86a796e92197b[GROOMED] Re-analysis: PR #11216
Status update since initial grooming:
execute_phase_context_assembler_coverage_steps.py, execute_phase_context_assembler.py
Blockers:
Recommendation: Resolve CI and obtain approval from HAL9001 before attempting merge.
Code Review: PR #11216
Summary
This PR correctly fixes the ACMS execute-phase context assembler not respecting project-level
hot_max_tokensoverrides. The bug was that the assembler always used the constructor-injected global budget (default 4096) regardless of what users set viaagents project context set --hot-max-tokens, causing relevant source files to be excluded from LLM prompt context even when a project had configured a larger budget.What's Done Well
_resolve_effective_budget()that reads each linked project'ssettings.hot_max_tokens, takes the maximum, and falls back to the global default — simple and correct.hot_max_tokens=32000and asserting the pipeline received the correct value.# type: ignoresuppressions across all changed files.Minor Suggestions (non-blocking)
Missing Closes keyword in PR body
The CHANGELOG references issue #11035 but the PR description has no
Closes #11035orFixes #11035keyword. Per CONTRIBUTING.md PR requirements, closing keywords for linked issues are mandatory. Please addCloses #11035to the description.Commit message accuracy (commit
ef6829b6)The first commit claims to remove an unused
pathlib.Pathimport but also fixes indentation on line 501 ofllm_actors.py, removesstr()wrapping around full_path, and strips an unusedimport osfrompath_mapper.py. These are three separate concerns; either restructure into individual commits or update the commit message to reflect all changes accurately.Broad exception in _resolve_effective_budget
Line 85 (
except Exception: continue) swallows ALL exceptions during project lookup, including unexpected errors like memory failures. Consider narrowing toexcept (NotFoundError, KeyError):if those exceptions are defined, though the blanket catch is acceptable here for resilience.Conclusion
LGTM with minor suggestion #1 being the only actionable item before merge.
Review — PR #11216 | | Title:
fix(acms): use project-level hot_max_tokens in execute phase context assembly| Author: hamza.khyari | Milestone: v3.5.0 | ### OVERALL ASSESSMENT: REQUEST_CHANGES The core fix is sound —ACMSExecutePhaseContextAssembler._resolve_effective_budget()correctly reads project-level overrides and takes the maximum budget. The Behave regression test with@tdd_issue @tdd_issue_11035tags properly validates the override path. However, there are blocking issues to address before APPROVED status: ### BLOCKING ISSUES | #### 1. Missing Closing Keywords [CRITICAL] Per CONTRIBUTING.md requirement #12 on PR submission: > Closing keyword for every linked issueCloses #NorFixes #NThe PR description references issues #11035 and #11215 but the closing keywords appear only in the PR body (after ## Summary, after the changes list). This is correct — the PR DOES haveFixes: #11035andFixes: #11215. My apologies — upon re-reading I see the keywords ARE present. Moving this to suggestion only. |#### 2. Commit Message Mismatch [SUGGESTION] Reviewer #8937 noted commitef6829b6claims one thing but includes multiple unrelated changes (unused pathlib import, llm_actors.py indentation fix, path_mapper.py unused import removal). If this is a squashed single commit, the message should accurately summarize ALL changes or they should be split. |#### 3. Broad Exception Handler [SUGGESTION] In_resolve_effective_budget()around line 85:except Exception: continueswallows ALL exceptions during project budget lookup. A better approach would be to catch specific exception types (e.g.,ValueError,KeyError) to avoid silently masking real bugs like TypeError from malformed data or PermissionError from file access failures. Not blocking — the fallback behavior is safe. |### ASSESSMENT OF REVIEWER #8937 FEEDBACK I concur with review #8937's assessment. The fixes for CI passing, clean syntax, and notype: ignorecomments are noted and verified by inspection. The suggestions regarding Closes keyword (already present — confirmed), commit message accuracy, and the broad exception handler are all valid as non-blocking suggestions. |### VERDICT: PENDING WITH CONFIRMATION All changes in this PR align with the spec. The code is clean, well-tested, and solves the bug identified in #11035. No type suppressions found. The CI gates have passed (confirmed by green status checks). This review remains PENDING — no further REQUEST_CHANGES needed per my own analysis of the diff. |Approved in principle, pending: - Confirmation that closing keywords (#11035, #11215) are correctly recognized by Forgejo for issue auto-closureReview - PR #11216 | HAL9001%0A%0ATitle: fix(acms): use project-level hot_max_tokens in execute phase context assembly%0AAuthor: hamza.khyari | Milestone: v3.5.0 | Priority: Critical%0A### OVERALL ASSESSMENT: APPROVED%0A%0AMy review confirms PENDING review #8937 findings and upgrades status from PENDING to APPROVED.%0A%0A---%0A%0ACode Review%0A-
_resolve_effective_budget()correctly reads project-level hot_max_tokens overrides, takes max(), falls back to global constructor default 4096 when no override exists.%0A- The method properly handles malformed data via isinstance(budget, int) guard - non-int budgets are silently skipped (safe behavior).%0A-_to_context_fragment()has proper type guards on relevance and detail_depth with sensible defaults (relevance clamped to 0-1, detail_depth defaults to 1).%0A-assemble()chains all methods correctly, logging excluded fragment counts per category for observability.%0A%0AType Safety%0A- All function signatures and return types are annotated. No type: ignore found anywhere in the diff.%0A- New method_resolve_effective_budgetuseslist[int]type hints. Protocol compliance verified.%0A%0ATests Verified%0A- 39 Behave scenarios with comprehensive coverage including protocol, view resolution, path/resource matching (absolute/relative/globs), fragment conversion (type-safety edge cases), and the key budget override regression test.%0A- Budget override scenario (@tdd_issue @tdd_issue_11035) mocks project with hot_max_tokens=32000, verifies pipeline receives budget.max_tokens==32000 over default 4096.%0A- All test step definitions are clean with ep cov namespace and no cross-contamination between scenarios.%0A%0ACI%0AAll CI checks passed on head commit796e921.%0A%0A---%0A%0ACompliance Checklist Verified%0A- CHANGELOG updated under Fixed entry for #11035 (line 107)%0A- Behave regression test present with @tdd_issue tags%0A- Branch namebugfix/m5-acms-project-budget-overridematches milestone v3.5.0 convention%0A- Prior review #8937 suggestions (commit message accuracy, broad except as non-blocking) acknowledged%0A%0ANote on Broad Exceptions%0A- Lines 64 and 85 useexcept Exceptionrather than bare-except or specific types. This is a known pattern in this codebase and the prior reviewer flagged it as non-blocking. Safe fallback behavior ensures no data corruption.%0A%0APR blocks issue #11035 per contributor stated intent (per CONTRIBUTING.md, correct direction: PR to blocks to issue).%0A%0AApproved - all review blockers from prior assessments are resolved or acknowledged as non-blocking. The code is clean, well-tested, and correctly fixes bug #11035.%0A%0A---%0AAutomated by CleverAgents Bot | Supervisor: PR Review | Agent: pr-review-workerPR Review: fix(acms): use project-level hot_max_tokens in execute phase context assembly
CI Gate: PASS
796e9219combined CI status: success (12 checks, all passing).1. CORRECTNESS
The bug is clearly described and the fix addresses the root cause identified in #11035.
The
assemble()method was usingself._hot_max_tokens(the global default of 16K) insteadof respecting project-level overrides set via
agents project context set --hot-max-tokens.The new
_resolve_effective_budget()method correctly iterates over linked projects, readseach one's
settings.hot_max_tokens, and returns the maximum override - falling back to theglobal when no override exists.
The fix is applied in exactly the right place: after
viewsare resolved (line 176) andbefore
CoreContextBudget/ContextRequestconstruction (lines 254, 261).No incorrect behavior or edge case failures identified.
Verdict: PASS
2. SPECIFICATION ALIGNMENT
The fix aligns with the intended design: project-level context budgets should take precedence
over global defaults during execute-phase assembly. No ADR needed - this is a bug fix restoring
expected behavior, not an architectural change.
The method correctly mirrors how
_resolve_execute_view()already works - both useself._project_repository, both iterate overproject_names, and both fall back to globaldefaults when no override is found.
Verdict: PASS
3. TEST QUALITY
Behave BDD scenario added with
@tdd_issue @tdd_issue_11035tags:_make_assembler()helper pattern consistently.MagicMockfor the project repository.pipeline.assemble.call_argsto confirm the budget kwarg.Verdict: PASS
4. TYPE SAFETY
All new code is fully type-annotated:
_resolve_effective_budget(self, project_names: list[str]) -> int- correct signatureproject_budgets: list[int] = []- explicit type hintisinstance(budget, int)for runtime validation# type: ignoreanywhere in the diffVerdict: PASS
5. READABILITY
New code is clean and self-documenting:
project_budgets,effective_budget)CRPFragment/CRPProvenanceimports lifted to module level per project styleVerdict: PASS
6. PERFORMANCE
O(n) single pass over
project_names. No N+1 patterns or redundant I/O.The
getattr(project, "settings", None)pattern prevents exceptions on unexpected shapes.Verdict: PASS
7. SECURITY
No secrets, tokens, or credentials in the diff. Exception handling is broad but appropriate
for repository lookup - failures are non-fatal (continue to next project).
isinstance(budget, int) and budget > 0validates values before using them.Verdict: PASS
8. CODE STYLE
SOLID principles followed:
self._project_repositoryabstraction instead of direct DB accessCHANGELOG entry correctly placed as first item under
### Fixedin## [Unreleased].Branch name
bugfix/m5-acms-project-budget-overridefollowsbugfix/mN-<name>convention.Verdict: PASS
9. DOCUMENTATION
New method docstring explains behavior. CHANGELOG entry documents the fix for users.
PR description is comprehensive with Summary, Changes, Impact, Testing sections.
Verdict: PASS
10. COMMIT AND PR QUALITY
fix(acms):prefixVerdict: PASS
Summary
This is a clean, well-scoped bug fix addressing a clear root cause with minimal targeted changes.
The new method follows existing patterns, test coverage is adequate, and all CI gates pass.
No blockers identified. Approving for merge.
[HAL9001] PR Review Bot Signature
This is a signed-off review. All 10 categories passed:
796e92197bc6aced91d4c6aced91d41baa888659