cleveragents/cleveractors-core

Fork

You've already forked cleveractors-core

Code Issues 22 Pull requests 7 Projects Releases Packages Wiki Activity Actions 373

test(agents): capture tool_max_rounds zero-value fallback (#144) #146

Open

CoreRasurae wants to merge 1 commit from tdd/m1-tool-max-rounds-precedence into master

pull from: tdd/m1-tool-max-rounds-precedence

merge into: cleveragents:master

cleveragents:master

cleveragents:feature/m1-rename-reasoning-config-property

cleveragents:feature/m1-inherit-tool-timeout-ceilings

cleveragents:bugfix/m1-executor-validate-configuration

cleveragents:bugfix/m1-registry-content-fetch

cleveragents:tdd/m1-registry-content-fetch

cleveragents:tdd/m1-registry-resolve-full-type-names

cleveragents:bugfix/m1-inline-sandbox-import-restriction

cleveragents:feature/m2-reasoning-model-provider-support

cleveragents:feature/skill-package-support

cleveragents:feature/76-budget-pre-flight

cleveragents:feature/17-public-api

cleveragents:feature/m1-registry-http-client

cleveragents:master-backup

Conversation 1 Commits 1 Files changed 2 +56

CoreRasurae commented

2026-08-21 23:06:23 +00:00

Member

Copy link

Summary

Companion TDD issue-capture test for #144 ("tool_max_rounds config silently ignored when falsy, and undocumented in the LLM agent specification"). Adds a Behave regression scenario proving the bug: _execute_tool_loop() resolves the round limit via self.config.get("tool_max_rounds") or os.environ.get("TOOL_MAX_ROUNDS", "20"), and Python's or treats an explicitly configured 0 as falsy, so it silently falls through to TOOL_MAX_ROUNDS/the "20" default instead of being clamped to 1 as CHANGELOG.md (issue #59 entry) documents.

New scenario in features/llm_agent_tool_loop.feature, steps added to the existing shared features/steps/llm_agent_tool_loop_steps.py (no new step file).
Tagged @tdd_issue @tdd_issue_144 @tdd_expected_fail.
Configures tool_max_rounds: 0 with a mock that always returns tool calls, and asserts the main tool-call loop executes exactly 1 round before the stuck-model synthesis calls.
Confirmed locally that the assertion fails with AssertionError (main_rounds=20, not 1) reproducing the bug; with the tag present, TddExpectedFailPolicy inverts the result so CI passes.

Closes #145

Test plan

nox -s lint — green
nox -s format -- --check — green
nox -s typecheck — green (pre-existing unrelated warning: langchain_google_genai import in llm_imports.py)
nox -s security_scan — green (bandit + semgrep, 0 findings)
nox -s dead_code — green
nox -s unit_tests — 154 features / 3084 scenarios / 14279 steps green (includes the new scenario passing via TDD tag inversion)
nox -s coverage_report — 96.9% (>= repo gate 96.5%)
nox -s integration_tests — skipped per explicit instruction; test-only change to features/, no src/ or robot/ code touched
nox -s benchmark_regression — skipped per explicit instruction; test-only change, no ASV-relevant code touched
Confirmed the assertion fails via AssertionError (not ValueError/RuntimeError) when @tdd_expected_fail is removed locally

## Summary Companion TDD issue-capture test for #144 ("tool_max_rounds config silently ignored when falsy, and undocumented in the LLM agent specification"). Adds a Behave regression scenario proving the bug: `_execute_tool_loop()` resolves the round limit via `self.config.get("tool_max_rounds") or os.environ.get("TOOL_MAX_ROUNDS", "20")`, and Python's `or` treats an explicitly configured `0` as falsy, so it silently falls through to `TOOL_MAX_ROUNDS`/the `"20"` default instead of being clamped to 1 as `CHANGELOG.md` (issue #59 entry) documents. - New scenario in `features/llm_agent_tool_loop.feature`, steps added to the existing shared `features/steps/llm_agent_tool_loop_steps.py` (no new step file). - Tagged `@tdd_issue @tdd_issue_144 @tdd_expected_fail`. - Configures `tool_max_rounds: 0` with a mock that always returns tool calls, and asserts the main tool-call loop executes exactly 1 round before the stuck-model synthesis calls. - Confirmed locally that the assertion fails with `AssertionError` (`main_rounds=20`, not 1) reproducing the bug; with the tag present, `TddExpectedFailPolicy` inverts the result so CI passes. Closes #145 ## Test plan - [x] `nox -s lint` — green - [x] `nox -s format -- --check` — green - [x] `nox -s typecheck` — green (pre-existing unrelated warning: `langchain_google_genai` import in `llm_imports.py`) - [x] `nox -s security_scan` — green (bandit + semgrep, 0 findings) - [x] `nox -s dead_code` — green - [x] `nox -s unit_tests` — 154 features / 3084 scenarios / 14279 steps green (includes the new scenario passing via TDD tag inversion) - [x] `nox -s coverage_report` — 96.9% (>= repo gate 96.5%) - [ ] `nox -s integration_tests` — skipped per explicit instruction; test-only change to `features/`, no `src/` or `robot/` code touched - [ ] `nox -s benchmark_regression` — skipped per explicit instruction; test-only change, no ASV-relevant code touched - [x] Confirmed the assertion fails via `AssertionError` (not `ValueError`/`RuntimeError`) when `@tdd_expected_fail` is removed locally

CoreRasurae added 1 commit

2026-08-21 23:06:27 +00:00

test(agents): capture tool_max_rounds zero-value fallback (#144 )

Some checks failed

CI / lint (pull_request) Successful in 52s

Details

CI / typecheck (pull_request) Successful in 1m24s

Details

CI / quality (pull_request) Successful in 58s

Details

CI / security (pull_request) Successful in 2m17s

Details

CI / build (pull_request) Successful in 1m53s

Details

CI / integration_tests (pull_request) Successful in 2m58s

Details

CI / unit_tests (pull_request) Successful in 5m38s

Details

CI / coverage (pull_request) Failing after 11m6s

Details

CI / benchmark (pull_request) Failing after 21m27s

Details

CI / status-check (pull_request) Has been cancelled

Details

daea4ddd8d

_execute_tool_loop() resolves the round limit via
self.config.get("tool_max_rounds") or os.environ.get("TOOL_MAX_ROUNDS", "20").
Python's `or` treats an explicitly configured 0 as falsy, so it silently
falls through to TOOL_MAX_ROUNDS/the "20" default instead of being clamped
to 1, as CHANGELOG.md (issue #59 entry) documents ("values <= 0 are
clamped to 1"). An agent configured with tool_max_rounds: 0 therefore runs
far more tool-call rounds than intended.

Adds a new scenario to features/llm_agent_tool_loop.feature (extending the
existing shared llm_agent_tool_loop_steps.py rather than a new step file)
that configures tool_max_rounds: 0 with a mock that always returns tool
calls, and asserts the main loop executes exactly 1 round before the
stuck-model synthesis calls. Confirmed locally that the assertion fails
with AssertionError (main_rounds=20, not 1) when @tdd_expected_fail is
removed, proving the bug is genuinely reproduced; with the tag present,
TddExpectedFailPolicy inverts the failure so nox -s unit_tests stays green.

ISSUES CLOSED: #145

CoreRasurae added a new dependency

2026-08-21 23:07:14 +00:00

#145 TDD: tool_max_rounds config silently ignored when falsy, and undocumented in the LLM agent specification

CoreRasurae added this to the v2.1.0 milestone

2026-08-21 23:07:41 +00:00

CoreRasurae added the

Type

Testing

label

2026-08-21 23:07:52 +00:00

CoreRasurae referenced this pull request

2026-08-21 23:09:08 +00:00

TDD: tool_max_rounds config silently ignored when falsy, and undocumented in the LLM agent specification #145

hurui200320 approved these changes

2026-08-22 06:19:39 +00:00

hurui200320 left a comment

Copy link

PR Review: !146 (Ticket #145)

Verdict: Approve

This is a clean, well-focused TDD issue-capture PR for bug #144. The new Behave scenario correctly reproduces the tool_max_rounds: 0 precedence bug, uses the required @tdd_issue @tdd_issue_144 @tdd_expected_fail tag combination, fails via AssertionError (so TddExpectedFailPolicy inverts it as expected), and isolates the environment from ambient TOOL_MAX_ROUNDS. No critical or major issues were found.

Critical Issues

None.

Major Issues

None.

Minor Issues

None.

Nits

None.

Summary

features/llm_agent_tool_loop.feature adds a single tagged scenario that documents #144 and asserts the documented clamp-to-1 behavior for tool_max_rounds: 0.
features/steps/llm_agent_tool_loop_steps.py extends the existing shared step file with an env-var isolation step and a main_rounds = counter[0] - 2 assertion that matches the current _execute_tool_loop() stuck-model synthesis flow (one main round + synthesis + follow-up).
The assertion logic was verified against src/cleveractors/agents/llm.py: with the bug present the loop runs 20 main rounds, so the assertion fails with main_rounds=20; after the fix it will pass with main_rounds=1.
The patch.dict(os.environ, …) patcher is registered in context._active_patches, which after_scenario already cleans up, so the test is deterministic across environments.
This is a test-only change; it does not touch src/ or robot/, and it correctly defers the actual precedence fix and specification updates to the companion bugfix PR.

Approved as a valid TDD capture for #144.

## PR Review: !146 (Ticket #145) ### Verdict: Approve This is a clean, well-focused TDD issue-capture PR for bug #144. The new Behave scenario correctly reproduces the `tool_max_rounds: 0` precedence bug, uses the required `@tdd_issue @tdd_issue_144 @tdd_expected_fail` tag combination, fails via `AssertionError` (so `TddExpectedFailPolicy` inverts it as expected), and isolates the environment from ambient `TOOL_MAX_ROUNDS`. No critical or major issues were found. ### Critical Issues None. ### Major Issues None. ### Minor Issues None. ### Nits None. ### Summary - `features/llm_agent_tool_loop.feature` adds a single tagged scenario that documents #144 and asserts the documented clamp-to-1 behavior for `tool_max_rounds: 0`. - `features/steps/llm_agent_tool_loop_steps.py` extends the existing shared step file with an env-var isolation step and a `main_rounds = counter[0] - 2` assertion that matches the current `_execute_tool_loop()` stuck-model synthesis flow (one main round + synthesis + follow-up). - The assertion logic was verified against `src/cleveractors/agents/llm.py`: with the bug present the loop runs 20 main rounds, so the assertion fails with `main_rounds=20`; after the fix it will pass with `main_rounds=1`. - The `patch.dict(os.environ, …)` patcher is registered in `context._active_patches`, which `after_scenario` already cleans up, so the test is deterministic across environments. - This is a test-only change; it does not touch `src/` or `robot/`, and it correctly defers the actual precedence fix and specification updates to the companion bugfix PR. Approved as a valid TDD capture for #144.

Some checks failed

CI / lint (pull_request) Successful in 52s

Details

CI / typecheck (pull_request) Successful in 1m24s

Details

CI / quality (pull_request) Successful in 58s

Details

CI / security (pull_request) Successful in 2m17s

Details

CI / build (pull_request) Successful in 1m53s

Details

CI / integration_tests (pull_request) Successful in 2m58s

Details

CI / unit_tests (pull_request) Successful in 5m38s

Details

CI / coverage (pull_request) Failing after 11m6s

Details

CI / benchmark (pull_request) Failing after 21m27s

Details

CI / status-check (pull_request) Has been cancelled

Details

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin tdd/m1-tool-max-rounds-precedence:tdd/m1-tool-max-rounds-precedence

git switch tdd/m1-tool-max-rounds-precedence

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch master

git merge --no-ff tdd/m1-tool-max-rounds-precedence

git switch tdd/m1-tool-max-rounds-precedence

git rebase master

git switch master

git merge --ff-only tdd/m1-tool-max-rounds-precedence

git switch tdd/m1-tool-max-rounds-precedence

git rebase master

git switch master

git merge --no-ff tdd/m1-tool-max-rounds-precedence

git switch master

git merge --squash tdd/m1-tool-max-rounds-precedence