Skip to content

feat(config): add [reasoning_only] section for retry count and custom… - #5867

Open
Gabriel-Degret wants to merge 6 commits into
Hmbown:mainfrom
Gabriel-Degret:feat/reasoning-only-config
Open

feat(config): add [reasoning_only] section for retry count and custom…#5867
Gabriel-Degret wants to merge 6 commits into
Hmbown:mainfrom
Gabriel-Degret:feat/reasoning-only-config

Conversation

@Gabriel-Degret

@Gabriel-Degret Gabriel-Degret commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Add a [reasoning_only] config section to make the reasoning-only retry behavior user-configurable.

Before: MAX_REASONING_ONLY_REPROMPTS = 2 was hardcoded. When a reasoning model returned only hidden thinking with no answer or tool call, the engine silently retried exactly twice, then failed.

After: Users can configure the retry count and optionally inject a custom reprompt message to nudge the model.

[reasoning_only]
# Maximum number of automatic re-requests (default: 2). Set to 0 to disable.
max_reprompts = 2

# Optional message sent as a user turn before each retry.
reprompt_message = "So, what's up ? Keep running !"

When reprompt_message is set, the engine inserts it as a runtime user message before re-issuing the request. When unset, the original cached-prefix retry behaviour is preserved (no synthetic message).

Testing

  • cargo fmt --all -- --check — clean
  • cargo clippy --workspace --all-targets --all-features --locked — warning-free under the CI allow list
  • cargo test --workspace --all-features --locked — passes, including 8 existing reasoning_only tests and 1 new config test

Checklist

  • Updated docs or comments as needed (docs/CONFIGURATION.md)
  • Added or updated tests where relevant (crates/tui/src/config/tests.rs)
  • Verified TUI behavior manually if UI changes — no UI changes
  • Harvested/co-authored credit uses a GitHub numeric noreply address

No-Issue: add section [reasoning_only] in config.toml

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Thanks @Gabriel-Degret for taking the time to contribute.

This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered.

Please read CONTRIBUTING.md for the expected contribution shape. A maintainer can grant recurring PR access by commenting /lgtm on a pull request.

@Gabriel-Degret
Gabriel-Degret force-pushed the feat/reasoning-only-config branch from 0922990 to e361f6c Compare September 3, 2026 13:38
@Gabriel-Degret
Gabriel-Degret force-pushed the feat/reasoning-only-config branch from e361f6c to cd21201 Compare September 3, 2026 14:03
@Gabriel-Degret

Copy link
Copy Markdown
Contributor Author

CI failure — pre-existing, unrelated to this PR

The single Windows CI failure:

runtime_api::tests::marketplace_add_rejects_symlink_documents_over_http

... is pre-existing on main and completely unrelated to this PR.

  • Not our code — our changes are in config.rs, engine.rs, turn_loop.rs, frame.rs, runtime_threads.rs, exec_agent.rs, config/tests.rs, and docs/CONFIGURATION.md. None of these files are touched by the failing test.
  • The failing test (crates/tui/src/runtime_api/tests.rs:11195) is about plugin marketplace HTTP symlink handling on Windows — a different subsystem entirely.
  • On Unix (Linux/macOS) the test passes fine. On Windows, the server doesn't return the expected status code, which is a pre-existing Windows-specific issue.

The test exists identically on origin/main and is not modified by this PR.

8 reasoning-only-related tests and 1 new config test all pass ✅.

@Hmbown

Hmbown commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Hi @Gabriel-Degret — I'm Claude Opus 5, working with @Hmbown on the 0.9.12 release. Thank you for this: you found a real gap, and the idea is landing in 0.9.12.

What we took, as yours. The whole config surface is in — the [reasoning_only] table, max_reprompts, reprompt_message, the accessors, the wiring through every EngineConfig construction site, config.example.toml, and the docs/CONFIGURATION.md section. That ceiling being a hard-coded 2 was a genuine limitation and you were right to make it configurable. It's committed on our 0.9.12 line carrying Co-authored-by: and Harvested-from: PR #5867, so once that reaches main the credit and the auto-close both point back here.

The one thing we implemented differently, and why. The nudge was delivered with add_session_message, which does session.add_message() + emit_session_updated(). That writes a user-role message the operator never typed permanently into the session — it persists to disk, shows up in the transcript and exports, and the model re-reads it on every subsequent turn of that conversation. It also turns the retry into a cache miss: the comment three lines above the code says the retry is deliberately "an exact cached-prefix retry — no synthetic message, no prefix churn," which is what made recovery nearly free.

So we kept your config surface and changed only the delivery: the nudge is now attached to a single outbound request and dropped, never reaching session.messages. It also escalates rather than firing every time — attempt 1 stays the bare cached retry (usually enough, and free), and the nudge rides along only from attempt 2, where an identical request has already come back answerless and repeating it would just reproduce the same reply.

There's a regression test, the_reasoning_only_nudge_rides_one_request_and_never_joins_the_session, that tells the two apart by message counts across four requests: persisted, they accumulate (n, n, n+1, n+2); request-scoped, each nudged request carries exactly one extra over the same baseline. Pointing the branch back at add_session_message fails it precisely there.

Two smaller notes, both cosmetic and neither a criticism of the work: we changed the default nudge text to "Continue: give your answer, or make the next tool call." to match the product's voice, and left out the link_link rename in plugins/marketplace/document.rs, which was this branch's only merge conflict. Your red CI was plugin_e2e_acceptance, unrelated to your change and green on current main — your branch point just predated the fix, which is our churn, not yours.

We'd genuinely welcome your follow-up here as 0.9.12 goes out. If the escalation policy is wrong for the models you hit this on — if the bare retry is never enough for a particular provider and the nudge should fire from attempt 1, or be configurable per route — you have better data on that than we do, and we want this properly fixed as much as you do. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants