cleveragents/cleveractors-core

Fork 0

refactor(agents): rename reasoning config field to reasoning_feedback #143

Open

CoreRasurae wants to merge 1 commit from feature/m1-rename-reasoning-config-property into master

CoreRasurae commented

2026-08-21 21:56:40 +00:00

Member

Summary

Renames the type: llm agent configuration field reasoning to reasoning_feedback (ADR-2036 D-10, accepted in place). Semantics, default (false), validation, and routing effect are unchanged — the field still selects the reasoning-aware client (ReasoningChatModel) that round-trips a non-native provider's reasoning_content, only its name changes.

docs/adr/ADR-2036-reasoning-aware-provider-routing.md: adds accepted decision D-10 documenting the rename and its rationale; D-1–D-9 are left unchanged as the historical record, and a Revision History section is added.
docs/index.md §4.4 / §4.4.1: field name and example updated to reasoning_feedback. Since the field never shipped in a release, the existing 1.3.0 §21.1 entry is corrected in place — no Version bump.
src/cleveractors/agents/llm_client.py: config.get("reasoning_feedback", False), the boolean-validation error message, and the reasoning_feedback: bool parameter through build_chat_model/_build_from_credentials.
docs/guides/reasoning-aware-llm-agents.md: operator guide examples updated.
features/reasoning_provider_routing.feature + features/steps/reasoning_provider_routing_steps.py, robot/ReasoningRoutingTestLib.py: Behave/Robot tests exercise reasoning_feedback.
CHANGELOG.md: the still-[Unreleased] entry that introduced this field is corrected in place (no new entry, consistent with no release having occurred).

Explicitly out of scope (per ADR-2036 D-10): reasoning_content/reasoning_details (the wire-protocol fields the flag round-trips), ReasoningChatModel, and the unrelated "reasoning" §4.11 capabilities-list identifier (LLMAgent.get_capabilities, features/llm_agent_coverage.feature) are all untouched.

Closes #142

Tests

Behave: features/reasoning_provider_routing.feature (9 scenarios, all passing) exercising the renamed field.
Robot: robot/reasoning_provider_routing.robot (ReasoningRoutingTestLib.py) — passing.
nox (lint, format --check, typecheck, security_scan, dead_code, unit_tests, coverage_report, integration_tests, docs, build, complexity) all green:
- unit_tests: 154 features / 3083 scenarios / 14274 steps passed.
- coverage_report: 96.9% (threshold 96.5%).
- integration_tests: 362 Robot tests passed (pabot).
- benchmark / benchmark_regression: not run — no performance-sensitive code changed (config-key rename only); pre-existing ASV machine-registration gap in this sandbox is unrelated and was confirmed to reproduce on unmodified master.

## Summary Renames the `type: llm` agent configuration field `reasoning` to `reasoning_feedback` (ADR-2036 D-10, accepted in place). Semantics, default (`false`), validation, and routing effect are unchanged — the field still selects the reasoning-aware client (`ReasoningChatModel`) that round-trips a non-native provider's `reasoning_content`, only its name changes. - `docs/adr/ADR-2036-reasoning-aware-provider-routing.md`: adds accepted decision **D-10** documenting the rename and its rationale; D-1–D-9 are left unchanged as the historical record, and a Revision History section is added. - `docs/index.md` §4.4 / §4.4.1: field name and example updated to `reasoning_feedback`. Since the field never shipped in a release, the existing 1.3.0 §21.1 entry is corrected in place — no Version bump. - `src/cleveractors/agents/llm_client.py`: `config.get("reasoning_feedback", False)`, the boolean-validation error message, and the `reasoning_feedback: bool` parameter through `build_chat_model`/`_build_from_credentials`. - `docs/guides/reasoning-aware-llm-agents.md`: operator guide examples updated. - `features/reasoning_provider_routing.feature` + `features/steps/reasoning_provider_routing_steps.py`, `robot/ReasoningRoutingTestLib.py`: Behave/Robot tests exercise `reasoning_feedback`. - `CHANGELOG.md`: the still-`[Unreleased]` entry that introduced this field is corrected in place (no new entry, consistent with no release having occurred). **Explicitly out of scope** (per ADR-2036 D-10): `reasoning_content`/`reasoning_details` (the wire-protocol fields the flag round-trips), `ReasoningChatModel`, and the unrelated `"reasoning"` §4.11 capabilities-list identifier (`LLMAgent.get_capabilities`, `features/llm_agent_coverage.feature`) are all untouched. Closes #142 ## Tests - Behave: `features/reasoning_provider_routing.feature` (9 scenarios, all passing) exercising the renamed field. - Robot: `robot/reasoning_provider_routing.robot` (`ReasoningRoutingTestLib.py`) — passing. - `nox` (lint, format --check, typecheck, security_scan, dead_code, unit_tests, coverage_report, integration_tests, docs, build, complexity) all green: - `unit_tests`: 154 features / 3083 scenarios / 14274 steps passed. - `coverage_report`: 96.9% (threshold 96.5%). - `integration_tests`: 362 Robot tests passed (pabot). - `benchmark` / `benchmark_regression`: not run — no performance-sensitive code changed (config-key rename only); pre-existing ASV machine-registration gap in this sandbox is unrelated and was confirmed to reproduce on unmodified `master`.

CoreRasurae added 1 commit

2026-08-21 21:56:43 +00:00

refactor(agents): rename reasoning config field to reasoning_feedback

CI / lint (pull_request) Successful in 53s

Details

CI / typecheck (pull_request) Successful in 1m29s

Details

CI / security (pull_request) Successful in 2m20s

Details

CI / quality (pull_request) Successful in 59s

Details

CI / build (pull_request) Successful in 1m56s

Details

CI / integration_tests (pull_request) Successful in 2m56s

Details

CI / unit_tests (pull_request) Successful in 5m3s

Details

CI / coverage (pull_request) Successful in 5m40s

Details

CI / status-check (pull_request) Successful in 7s

Details

CI / benchmark (pull_request) Failing after 15m55s

Details

a16db7a596

Renames the LLM agent configuration field `reasoning` to
`reasoning_feedback` (ADR-2036 D-10, accepted in place). The old name read
as though it enabled a reasoning-capable model or "thinking mode" on the
model itself; the field actually selects a client that feeds the
provider's own `reasoning_content` back to it on the next turn.
`reasoning_feedback` names that round-trip behaviour without ambiguity.

Since the field never shipped in a release, this is a pre-release
correction, not a versioned spec revision: docs/index.md's existing
1.3.0 §21.1 entry is corrected in place and the Version header is
unchanged. Semantics, default (`false`), validation, and routing effect
are unchanged — only the name changes, in the spec, the config-reading
code in llm_client.py, the operator guide, and the Behave/Robot tests
that exercise it.

ISSUES CLOSED: #142

CoreRasurae added this to the v2.1.0 milestone

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

CoreRasurae added the

Type

Refactor

label

2026-08-21 21:58:13 +00:00

CoreRasurae added a new dependency

2026-08-21 21:58:28 +00:00

#142 Rename LLM agent reasoning config property to reasoning_feedback

hurui200320 approved these changes

2026-08-22 06:33:30 +00:00

hurui200320 left a comment

PR Review: !143 (Ticket #142)

Verdict: Approve

This is a clean, mechanical rename of the LLM agent configuration key from reasoning to reasoning_feedback. The implementation is consistent across the code, tests, operator guide, changelog, and ADR. The semantics, default value, validation behavior, and routing effect are preserved exactly as required by ADR-2036 D-10. The PR author has also clearly documented the items that are intentionally out of scope (wire-protocol fields, ReasoningChatModel internals, and the "reasoning" capability identifier), and those boundaries are respected.

Critical Issues

None.

Major Issues

None.

Minor Issues

None.

Nits

Coverage threshold discrepancy (pre-existing, not blocking). The PR reports coverage_report: 96.9% (threshold 96.5%), which passes the value actually enforced in noxfile.py (COVERAGE_THRESHOLD = 96.5). However, CONTRIBUTING.md states the project threshold is 97%, and the coverage_report docstring also says "Coverage threshold is enforced at >=97.0%." This inconsistency is unrelated to the rename and appears to pre-date the PR, but it may be worth aligning noxfile.py and CONTRIBUTING.md in a separate housekeeping change so CI and documentation agree.

Summary

A focused, well-documented refactor. All changed files keep the new naming consistent, the ADR revision is correctly recorded as accepted, and the test suite exercises the renamed field including validation and error paths. No behavior is altered beyond the configuration key name. I recommend merging.

## PR Review: !143 (Ticket #142) ### Verdict: Approve This is a clean, mechanical rename of the LLM agent configuration key from `reasoning` to `reasoning_feedback`. The implementation is consistent across the code, tests, operator guide, changelog, and ADR. The semantics, default value, validation behavior, and routing effect are preserved exactly as required by ADR-2036 D-10. The PR author has also clearly documented the items that are intentionally out of scope (wire-protocol fields, `ReasoningChatModel` internals, and the `"reasoning"` capability identifier), and those boundaries are respected. ### Critical Issues None. ### Major Issues None. ### Minor Issues None. ### Nits - **Coverage threshold discrepancy (pre-existing, not blocking).** The PR reports `coverage_report: 96.9% (threshold 96.5%)`, which passes the value actually enforced in `noxfile.py` (`COVERAGE_THRESHOLD = 96.5`). However, `CONTRIBUTING.md` states the project threshold is **97%**, and the `coverage_report` docstring also says "Coverage threshold is enforced at >=97.0%." This inconsistency is unrelated to the rename and appears to pre-date the PR, but it may be worth aligning `noxfile.py` and `CONTRIBUTING.md` in a separate housekeeping change so CI and documentation agree. ### Summary A focused, well-documented refactor. All changed files keep the new naming consistent, the ADR revision is correctly recorded as accepted, and the test suite exercises the renamed field including validation and error paths. No behavior is altered beyond the configuration key name. I recommend merging.

CI / lint (pull_request) Successful in 53s

Details

CI / typecheck (pull_request) Successful in 1m29s

Details

CI / security (pull_request) Successful in 2m20s

Details

CI / quality (pull_request) Successful in 59s

Details

CI / build (pull_request) Successful in 1m56s

Details

CI / integration_tests (pull_request) Successful in 2m56s

Details

CI / unit_tests (pull_request) Successful in 5m3s

Details

CI / coverage (pull_request) Successful in 5m40s

Details

CI / status-check (pull_request) Successful in 7s

Details

CI / benchmark (pull_request) Failing after 15m55s

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 feature/m1-rename-reasoning-config-property:feature/m1-rename-reasoning-config-property

git switch feature/m1-rename-reasoning-config-property

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 feature/m1-rename-reasoning-config-property

git switch feature/m1-rename-reasoning-config-property

git rebase master

git switch master

git merge --ff-only feature/m1-rename-reasoning-config-property

git switch feature/m1-rename-reasoning-config-property

git rebase master

git switch master

git merge --no-ff feature/m1-rename-reasoning-config-property

git switch master

git merge --squash feature/m1-rename-reasoning-config-property