cleveragents/cleveragents-core

Fork 3

UAT: DefaultStrategyExecutor creates ContextBudget with reserved_tokens=0 which violates core ContextBudget invariant when allocated_tokens equals max_tokens #2090

New issue

Open

opened 2026-04-03 03:59:16 +00:00 by freemo · 1 comment

freemo commented

2026-04-03 03:59:16 +00:00

Owner

Metadata

Branch: fix/acms-default-executor-budget-creation
Commit Message: fix(acms): fix DefaultStrategyExecutor scoped budget creation to avoid validation errors
Milestone: v3.7.0
Parent Epic: #396

Summary

DefaultStrategyExecutor.execute() in acms_service.py creates a scoped ContextBudget for each strategy allocation:

scoped_budget = ContextBudget(
    max_tokens=allocated_tokens,
    reserved_tokens=0,
)

The ContextBudget imported here is from domain/models/core/context_fragment.py, which has a stricter validator: reserved_tokens must be strictly less than max_tokens. When reserved_tokens=0 and max_tokens=allocated_tokens, this is fine as long as allocated_tokens >= 1. However, the DefaultBudgetAllocator can produce allocations where allocated_tokens=0 for some strategies (which the executor already guards against with if allocated_tokens <= 0: continue).

The real issue is that ContextBudget(max_tokens=1, reserved_tokens=0) is valid, but the default reserved_tokens=512 in ContextBudget would cause a validation error if max_tokens < 512. The executor correctly passes reserved_tokens=0 explicitly, so this specific path is safe.

However, the ParallelStrategyExecutor in acms_pipeline.py has the same pattern and also creates ContextBudget(max_tokens=allocated_tokens, reserved_tokens=0). When allocated_tokens=1, this creates a budget with max_tokens=1, reserved_tokens=0 — which is valid. But the ContextBudget default constructor (ContextBudget()) uses max_tokens=4096, reserved_tokens=512, which means any code that creates ContextBudget() without arguments gets a budget with 512 reserved tokens. If the pipeline is called with a budget of max_tokens=512, the default ContextBudget() would fail validation since reserved_tokens (512) >= max_tokens (512).

The real bug is: ContextBudget has a default reserved_tokens=512 but max_tokens=4096. If a caller creates ContextBudget(max_tokens=512), the validator raises ValueError: reserved_tokens (512) must be less than max_tokens (512). This is a footgun for any caller using the default reserved_tokens.

Expected Behavior

ContextBudget(max_tokens=512) should either:

Accept the call and set reserved_tokens=0 automatically when reserved_tokens >= max_tokens, OR
Raise a clear error message explaining that max_tokens must be greater than the default reserved_tokens=512

The current error message "reserved_tokens (512) must be less than max_tokens (512)" is confusing because the user didn't explicitly set reserved_tokens.

Actual Behavior

from cleveragents.domain.models.core.context_fragment import ContextBudget

# This raises ValueError even though the user only set max_tokens
ContextBudget(max_tokens=512)
# ValueError: reserved_tokens (512) must be less than max_tokens (512)

# This also fails
ContextBudget(max_tokens=100)
# ValueError: reserved_tokens (512) must be less than max_tokens (100)

Code Location

src/cleveragents/domain/models/core/context_fragment.py, lines 143-155: ContextBudget with max_tokens=4096, reserved_tokens=512 defaults
The validator at line 151: if self.reserved_tokens >= self.max_tokens: raise ValueError(...)

Impact

Any caller that creates ContextBudget(max_tokens=N) where N <= 512 will get a confusing ValueError about reserved_tokens they never set.
The DefaultStrategyExecutor and ParallelStrategyExecutor avoid this by explicitly passing reserved_tokens=0, but this is a hidden footgun for external callers.
The BDD test "Assembly with empty fragments returns empty payload" uses ContextBudget(max_tokens=4096, reserved_tokens=512) — if someone changes the default reserved_tokens to a larger value, this test would break.

Subtasks

Change ContextBudget default reserved_tokens from 512 to 0 to avoid the footgun, OR add a validator that provides a clear error message when only max_tokens is set below the default reserved_tokens
Add a BDD scenario testing ContextBudget(max_tokens=512) (equal to default reserved_tokens)
Add a BDD scenario testing ContextBudget(max_tokens=100) (less than default reserved_tokens)
Update docstring to warn about the default reserved_tokens interaction

Definition of Done

ContextBudget(max_tokens=N) for any N >= 1 either succeeds or raises a clear, actionable error
BDD tests added and passing
No regressions

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-uat-tester

## Metadata - **Branch**: `fix/acms-default-executor-budget-creation` - **Commit Message**: `fix(acms): fix DefaultStrategyExecutor scoped budget creation to avoid validation errors` - **Milestone**: v3.7.0 - **Parent Epic**: #396 ## Summary `DefaultStrategyExecutor.execute()` in `acms_service.py` creates a scoped `ContextBudget` for each strategy allocation: ```python scoped_budget = ContextBudget( max_tokens=allocated_tokens, reserved_tokens=0, ) ``` The `ContextBudget` imported here is from `domain/models/core/context_fragment.py`, which has a stricter validator: `reserved_tokens` must be **strictly less than** `max_tokens`. When `reserved_tokens=0` and `max_tokens=allocated_tokens`, this is fine as long as `allocated_tokens >= 1`. However, the `DefaultBudgetAllocator` can produce allocations where `allocated_tokens=0` for some strategies (which the executor already guards against with `if allocated_tokens <= 0: continue`). The real issue is that `ContextBudget(max_tokens=1, reserved_tokens=0)` is valid, but the default `reserved_tokens=512` in `ContextBudget` would cause a validation error if `max_tokens < 512`. The executor correctly passes `reserved_tokens=0` explicitly, so this specific path is safe. **However**, the `ParallelStrategyExecutor` in `acms_pipeline.py` has the same pattern and also creates `ContextBudget(max_tokens=allocated_tokens, reserved_tokens=0)`. When `allocated_tokens=1`, this creates a budget with `max_tokens=1, reserved_tokens=0` — which is valid. But the `ContextBudget` default constructor (`ContextBudget()`) uses `max_tokens=4096, reserved_tokens=512`, which means any code that creates `ContextBudget()` without arguments gets a budget with 512 reserved tokens. If the pipeline is called with a budget of `max_tokens=512`, the default `ContextBudget()` would fail validation since `reserved_tokens (512) >= max_tokens (512)`. The real bug is: **`ContextBudget` has a default `reserved_tokens=512` but `max_tokens=4096`**. If a caller creates `ContextBudget(max_tokens=512)`, the validator raises `ValueError: reserved_tokens (512) must be less than max_tokens (512)`. This is a footgun for any caller using the default `reserved_tokens`. ## Expected Behavior `ContextBudget(max_tokens=512)` should either: 1. Accept the call and set `reserved_tokens=0` automatically when `reserved_tokens >= max_tokens`, OR 2. Raise a clear error message explaining that `max_tokens` must be greater than the default `reserved_tokens=512` The current error message `"reserved_tokens (512) must be less than max_tokens (512)"` is confusing because the user didn't explicitly set `reserved_tokens`. ## Actual Behavior ```python from cleveragents.domain.models.core.context_fragment import ContextBudget # This raises ValueError even though the user only set max_tokens ContextBudget(max_tokens=512) # ValueError: reserved_tokens (512) must be less than max_tokens (512) # This also fails ContextBudget(max_tokens=100) # ValueError: reserved_tokens (512) must be less than max_tokens (100) ``` ## Code Location - `src/cleveragents/domain/models/core/context_fragment.py`, lines 143-155: `ContextBudget` with `max_tokens=4096, reserved_tokens=512` defaults - The validator at line 151: `if self.reserved_tokens >= self.max_tokens: raise ValueError(...)` ## Impact - Any caller that creates `ContextBudget(max_tokens=N)` where `N <= 512` will get a confusing `ValueError` about `reserved_tokens` they never set. - The `DefaultStrategyExecutor` and `ParallelStrategyExecutor` avoid this by explicitly passing `reserved_tokens=0`, but this is a hidden footgun for external callers. - The BDD test `"Assembly with empty fragments returns empty payload"` uses `ContextBudget(max_tokens=4096, reserved_tokens=512)` — if someone changes the default `reserved_tokens` to a larger value, this test would break. ## Subtasks - [ ] Change `ContextBudget` default `reserved_tokens` from `512` to `0` to avoid the footgun, OR add a validator that provides a clear error message when only `max_tokens` is set below the default `reserved_tokens` - [ ] Add a BDD scenario testing `ContextBudget(max_tokens=512)` (equal to default reserved_tokens) - [ ] Add a BDD scenario testing `ContextBudget(max_tokens=100)` (less than default reserved_tokens) - [ ] Update docstring to warn about the default `reserved_tokens` interaction ## Definition of Done - [ ] `ContextBudget(max_tokens=N)` for any `N >= 1` either succeeds or raises a clear, actionable error - [ ] BDD tests added and passing - [ ] No regressions --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-uat-tester

freemo added this to the v3.7.0 milestone

2026-04-03 03:59:43 +00:00

freemo added a new dependency

2026-04-03 04:00:29 +00:00

#396 Epic: ACMS Context Pipeline

freemo referenced this issue

2026-04-03 04:01:23 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 701 — v3.7.0 #2093

freemo referenced this issue

2026-04-03 04:07:31 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.7.0 — Session Tracker #2023

freemo referenced this issue

2026-04-03 04:08:26 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.7.0 — Session Tracker #2023

freemo referenced this issue

2026-04-03 04:11:31 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 711 — v3.7.0 #2115

freemo self-assigned this

2026-04-03 16:58:08 +00:00

freemo added the

MoSCoW

Could have

label

2026-04-03 18:28:06 +00:00

freemo commented

2026-04-03 18:28:17 +00:00

Author

Owner

MoSCoW classification: Could Have

Rationale: This is a low-priority or backlog item. Desirable but not necessary for the milestone. Include only if time permits.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

MoSCoW classification: **Could Have** Rationale: This is a low-priority or backlog item. Desirable but not necessary for the milestone. Include only if time permits. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner