tool_max_rounds config silently ignored when falsy, and undocumented in the LLM agent specification #144

New issue

Open

opened 2026-08-21 22:34:59 +00:00 by CoreRasurae · 0 comments

CoreRasurae commented

2026-08-21 22:34:59 +00:00

Member

Metadata

Commit Message: fix(agents): honor tool_max_rounds precedence and document it

Branch: bugfix/m1-tool-max-rounds-precedence

Background and context

LLMAgent._execute_tool_loop() (cleveractors.agents.llm) bounds the multi-turn tool-call loop with a round limit resolved from either the per-agent tool_max_rounds config key or the TOOL_MAX_ROUNDS environment variable. CHANGELOG.md (issue #59 entry) documents the intended behavior as: "a configurable multi-turn loop (default 20 rounds, minimum 1, overridable via tool_max_rounds config or TOOL_MAX_ROUNDS env var; values ≤ 0 are clamped to 1)".

This feature predates the ADR-2030/2031 series and was never added to the normative specification: docs/index.md §4.4 (LLM Agents configuration table) lists exactly 11 fields and tool_max_rounds is not among them, and no ADR documents it. ADR-2031 references "the multi-turn tool-call loop (implemented in #59/#60)" only as pre-existing background while formalizing unrelated token-budget/pruning fields.

The implementation gap and the documentation gap are the same underlying problem: a working, tested feature that was shipped without being correctly specified, and one of its documented behaviors doesn't actually hold in code.

Current behavior

In _execute_tool_loop():

_raw_max_rounds: object = self.config.get(
    "tool_max_rounds"
) or os.environ.get("TOOL_MAX_ROUNDS", "20")
try:
    _TOOL_MAX_ROUNDS = max(1, int(str(_raw_max_rounds)))
except (TypeError, ValueError) as _mre:
    raise ConfigurationError(...)

Because Python's or treats 0 (and "", False) as falsy, an agent that explicitly sets tool_max_rounds: 0 does not get the documented "clamped to 1" behavior — the falsy 0 is discarded and the code falls through to TOOL_MAX_ROUNDS/the "20" default instead. The clamp-to-1 behavior only reliably triggers via the environment-variable path, since a non-empty string like "0" is truthy and reaches max(1, int("0")).

Additionally:

tool_max_rounds receives none of the init-time validation/storage that sibling fields (token_budget_percent, pruning_threshold, pruning_tool_filter, max_retries, etc.) get in LLMAgent.__init__() — it is parsed fresh, unvalidated, on every _execute_tool_loop() call.
docs/index.md §4.4 does not list tool_max_rounds among its documented LLM agent configuration fields.
docs/guides/reasoning-aware-llm-agents.md shows tool_max_rounds in example YAML, but this is non-normative guide content, not the specification, and doesn't describe the config/env precedence or the 0-clamp behavior.

Expected behavior

An explicitly-set tool_max_rounds (including 0 or other falsy-but-present values) is honored and clamped to 1, matching the documented behavior — it must not silently defer to TOOL_MAX_ROUNDS/the default.
Precedence is: explicit per-agent tool_max_rounds (any present value, including 0) > TOOL_MAX_ROUNDS env var > default of 20.
The governing ADR is revised in place (new D-N on the appropriate existing ADR, per project ADR conventions) to formally introduce tool_max_rounds as a documented §4.4 configuration field.
docs/index.md §4.4 configuration table includes a tool_max_rounds row, with a §21.1 Revision History entry attributing the addition to that ADR.
User-facing guide documentation accurately describes the corrected config/env precedence and the 0-clamp behavior.

Acceptance criteria

Setting tool_max_rounds: 0 in an LLM agent config results in exactly 1 permitted tool round, not 20 (or whatever TOOL_MAX_ROUNDS is set to)
Setting tool_max_rounds to a positive integer continues to take precedence over TOOL_MAX_ROUNDS
TOOL_MAX_ROUNDS env var is consulted only when tool_max_rounds is absent from config
_execute_tool_loop() still raises ConfigurationError for a non-integer tool_max_rounds value
docs/index.md §4.4 table lists tool_max_rounds, and §21.1 Revision History has a new row citing the governing ADR
The governing ADR documents tool_max_rounds as a specification-level LLM agent field (revised in place, not a new ADR file)
User-facing guide documentation reflects the corrected config/env precedence and the 0-clamp behavior
nox -s coverage_report remains ≥ 97%

Supporting information

Relevant code: cleveractors.agents.llm.LLMAgent._execute_tool_loop (config parsing at the top of the method). Relevant docs: docs/index.md §4.4 (LLM Agents), CHANGELOG.md (issue #59 entry), docs/adr/ADR-2031-tool-loop-token-budget-and-pruning.md (background reference to the multi-turn loop), docs/guides/reasoning-aware-llm-agents.md (non-normative example usage).

Subtasks

Fix _execute_tool_loop() precedence logic so an explicitly-set tool_max_rounds (including 0) is honored and clamped to 1, distinguishing "absent from config" from "falsy"
Validate and store tool_max_rounds at __init__ time, consistent with sibling config fields
Revise the governing ADR in place with a new D-N formally introducing tool_max_rounds as a §4.4 configuration field
Update docs/index.md §4.4 configuration table with the tool_max_rounds row and a §21.1 Revision History entry attributing the ADR
Update user-facing guide documentation to describe the corrected precedence and 0-clamp behavior
Update CHANGELOG.md with a corrected entry
Tests (Behave): extend features/llm_agent_tool_loop.feature with scenarios covering tool_max_rounds: 0 and absent-vs-falsy precedence
Verify coverage >= 97% via nox -s coverage_report
Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line matches the Commit Message in Metadata exactly.
The commit is pushed to the branch matching the Branch in Metadata exactly.
The commit is submitted as a PR to master, reviewed, and merged.

## Metadata Commit Message: `fix(agents): honor tool_max_rounds precedence and document it` Branch: `bugfix/m1-tool-max-rounds-precedence` ## Background and context `LLMAgent._execute_tool_loop()` (`cleveractors.agents.llm`) bounds the multi-turn tool-call loop with a round limit resolved from either the per-agent `tool_max_rounds` config key or the `TOOL_MAX_ROUNDS` environment variable. `CHANGELOG.md` (issue #59 entry) documents the intended behavior as: "a configurable multi-turn loop (default 20 rounds, minimum 1, overridable via `tool_max_rounds` config or `TOOL_MAX_ROUNDS` env var; values ≤ 0 are clamped to 1)". This feature predates the ADR-2030/2031 series and was never added to the normative specification: `docs/index.md` §4.4 (LLM Agents configuration table) lists exactly 11 fields and `tool_max_rounds` is not among them, and no ADR documents it. ADR-2031 references "the multi-turn tool-call loop (implemented in #59/#60)" only as pre-existing background while formalizing unrelated token-budget/pruning fields. The implementation gap and the documentation gap are the same underlying problem: a working, tested feature that was shipped without being correctly specified, and one of its documented behaviors doesn't actually hold in code. ## Current behavior In `_execute_tool_loop()`: ```python _raw_max_rounds: object = self.config.get( "tool_max_rounds" ) or os.environ.get("TOOL_MAX_ROUNDS", "20") try: _TOOL_MAX_ROUNDS = max(1, int(str(_raw_max_rounds))) except (TypeError, ValueError) as _mre: raise ConfigurationError(...) ``` Because Python's `or` treats `0` (and `""`, `False`) as falsy, an agent that explicitly sets `tool_max_rounds: 0` does **not** get the documented "clamped to 1" behavior — the falsy `0` is discarded and the code falls through to `TOOL_MAX_ROUNDS`/the `"20"` default instead. The clamp-to-1 behavior only reliably triggers via the environment-variable path, since a non-empty string like `"0"` is truthy and reaches `max(1, int("0"))`. Additionally: - `tool_max_rounds` receives none of the init-time validation/storage that sibling fields (`token_budget_percent`, `pruning_threshold`, `pruning_tool_filter`, `max_retries`, etc.) get in `LLMAgent.__init__()` — it is parsed fresh, unvalidated, on every `_execute_tool_loop()` call. - `docs/index.md` §4.4 does not list `tool_max_rounds` among its documented LLM agent configuration fields. - `docs/guides/reasoning-aware-llm-agents.md` shows `tool_max_rounds` in example YAML, but this is non-normative guide content, not the specification, and doesn't describe the config/env precedence or the 0-clamp behavior. ## Expected behavior - An explicitly-set `tool_max_rounds` (including `0` or other falsy-but-present values) is honored and clamped to 1, matching the documented behavior — it must not silently defer to `TOOL_MAX_ROUNDS`/the default. - Precedence is: explicit per-agent `tool_max_rounds` (any present value, including 0) > `TOOL_MAX_ROUNDS` env var > default of 20. - The governing ADR is revised in place (new D-N on the appropriate existing ADR, per project ADR conventions) to formally introduce `tool_max_rounds` as a documented §4.4 configuration field. - `docs/index.md` §4.4 configuration table includes a `tool_max_rounds` row, with a §21.1 Revision History entry attributing the addition to that ADR. - User-facing guide documentation accurately describes the corrected config/env precedence and the 0-clamp behavior. ## Acceptance criteria - [ ] Setting `tool_max_rounds: 0` in an LLM agent config results in exactly 1 permitted tool round, not 20 (or whatever `TOOL_MAX_ROUNDS` is set to) - [ ] Setting `tool_max_rounds` to a positive integer continues to take precedence over `TOOL_MAX_ROUNDS` - [ ] `TOOL_MAX_ROUNDS` env var is consulted only when `tool_max_rounds` is absent from config - [ ] `_execute_tool_loop()` still raises `ConfigurationError` for a non-integer `tool_max_rounds` value - [ ] `docs/index.md` §4.4 table lists `tool_max_rounds`, and §21.1 Revision History has a new row citing the governing ADR - [ ] The governing ADR documents `tool_max_rounds` as a specification-level LLM agent field (revised in place, not a new ADR file) - [ ] User-facing guide documentation reflects the corrected config/env precedence and the 0-clamp behavior - [ ] `nox -s coverage_report` remains ≥ 97% ## Supporting information Relevant code: `cleveractors.agents.llm.LLMAgent._execute_tool_loop` (config parsing at the top of the method). Relevant docs: `docs/index.md` §4.4 (LLM Agents), `CHANGELOG.md` (issue #59 entry), `docs/adr/ADR-2031-tool-loop-token-budget-and-pruning.md` (background reference to the multi-turn loop), `docs/guides/reasoning-aware-llm-agents.md` (non-normative example usage). ## Subtasks - [ ] Fix `_execute_tool_loop()` precedence logic so an explicitly-set `tool_max_rounds` (including `0`) is honored and clamped to 1, distinguishing "absent from config" from "falsy" - [ ] Validate and store `tool_max_rounds` at `__init__` time, consistent with sibling config fields - [ ] Revise the governing ADR in place with a new D-N formally introducing `tool_max_rounds` as a §4.4 configuration field - [ ] Update `docs/index.md` §4.4 configuration table with the `tool_max_rounds` row and a §21.1 Revision History entry attributing the ADR - [ ] Update user-facing guide documentation to describe the corrected precedence and 0-clamp behavior - [ ] Update `CHANGELOG.md` with a corrected entry - [ ] Tests (Behave): extend `features/llm_agent_tool_loop.feature` with scenarios covering `tool_max_rounds: 0` and absent-vs-falsy precedence - [ ] Verify coverage >= 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the first line matches the Commit Message in Metadata exactly. - The commit is pushed to the branch matching the Branch in Metadata exactly. - The commit is submitted as a PR to master, reviewed, and merged.

CoreRasurae added this to the v2.1.0 milestone

2026-08-21 22:34:59 +00:00

CoreRasurae added the

labels