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.

Garfield1985 pushed a commit to Garfield1985/DeepSeek-TUI that referenced this pull request Sep 5, 2026
…udge

A reasoning model that closes a turn having emitted only hidden thinking —
no answer text, no tool call — is recovered by re-requesting. That ceiling
was a hard-coded 2. It is now `[reasoning_only] max_reprompts`, and the
nudge that rides a retry is `[reasoning_only] reprompt_message`.

Harvested from @Gabriel-Degret's PR Hmbown#5867, which found the gap and built the
config surface, the accessors, the wiring through every EngineConfig
construction site, and the docs — all taken here. One thing is implemented
differently.

The submitted version delivered the nudge 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, it shows in the transcript and exports, and the model re-reads it on
every later turn. It also contradicts the comment three lines above the code
it changed, which is still there and still true — "Nothing was persisted for
this response (a bare Thinking block is not sendable), so re-issuing the
request is an exact cached-prefix retry — no synthetic message, no prefix
churn."

So the nudge is now turn-scoped: it is attached to exactly one outbound
request and dropped, never reaching `session.messages`. It also escalates
rather than firing every time. Attempt 1 stays the bare cached-prefix retry
the comment describes, which is nearly free and usually enough. Only from
attempt 2 — where an identical request has already come back answerless, so
repeating it would only reproduce the same reply — does the nudge ride along.

`the_reasoning_only_nudge_rides_one_request_and_never_joins_the_session`
distinguishes the two by message counts across four requests: persisted, they
accumulate (n, n, n+1, n+2); request-scoped, the nudged requests each carry
one extra over the same baseline. Re-pointing the branch at
`add_session_message` fails it exactly there ("the nudge did not accumulate",
left: 3, right: 2).

Two smaller departures: the default nudge text is now
"Continue: give your answer, or make the next tool call." rather than
"So, what's up ? Keep running !", to match the product's voice; and
`MAX_REASONING_ONLY_REPROMPTS` is deleted rather than left unused, since
`config::DEFAULT_REASONING_ONLY_REPROMPTS` is now its single owner. An
unrelated drive-by in `plugins/marketplace/document.rs` (renaming `link` to
`_link`) is not taken — it was the branch's only merge conflict.

The PR's red CI is `plugin_e2e_acceptance`, unrelated to this change and
green on current main; its branch point predates the fix.

Gates: fmt PASS · clippy -p codewhale-tui PASS · nextest -p codewhale-tui
11989 passed / 1 failed / 13 skipped, the failure being the known
exec_persistent_service 120s timeout under parallel load (3/3 in isolation) ·
nextest -p codewhale-config 634 passed.

Co-authored-by: Gabriel Degret <28120444+Gabriel-Degret@users.noreply.github.com>
Harvested-from: PR Hmbown#5867 by @Gabriel-Degret
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QCLLenseqfQdVaZUBqn9yG
Garfield1985 pushed a commit to Garfield1985/DeepSeek-TUI that referenced this pull request Sep 5, 2026
0.9.12 had no `### Contributors` section, `web/lib/release-credits.ts`
still held the 0.9.11 cohort, and the contributor doc band was missing
five people whose work landed. Three surfaces that must agree, disagreeing
three ways.

Everyone whose work reached main this cycle is now named in all three,
derived from `Co-authored-by` and `Harvested-from` trailers on
v0.9.11..HEAD rather than from memory. The five who were missing:

- @qiuYliangM (秋月凉梦) — co-author on Hmbown#5621, Hmbown#5622, Hmbown#5623
- @whp233Hmbown#5716, landed as Hmbown#5719
- @Gabriel-DegretHmbown#5867
- @huangxianzhanHmbown#5868
- @zhuowpHmbown#5869

@aboimpinto's entry gained Hmbown#5825 and Hmbown#5865, @M-Maciej's gained Hmbown#5533/Hmbown#5831.

`requiredCandidateCredits` in public-surface-facts.json moves from the
0.9.11 cohort to this one — that guard is meant to hold the *current*
release's credits in every surface, and it had been pinning the previous
release's.

`RELEASE_HELPERS` is deliberately empty. Every credited contribution this
cycle arrived as code that landed; a name there that no landed change can
point at would be a nicer-looking ledger and a less true one.

AUTHOR_MAP gains canonical numeric-noreply identities for all five, since
the trailers on their harvest commits used raw emails and old-style
noreply addresses.

web: 386 tests, 386 passed. check:facts, check:docs, check:tokens,
check:locales all PASS.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QCLLenseqfQdVaZUBqn9yG
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