fix: local-model reliability — compaction hang, Qwen3 reasoning toggle, stuck-loop backstop#188
Merged
Merged
Conversation
Compaction summarization had no output-length limit, so a slow or degenerate completion from a local model could run unbounded and hang the soul loop indefinitely (observed as a stuck "Compacting..." / "Bloviating..." state). Add capped_chat_provider() to cap the summary call via the correct provider-specific kwarg (max_tokens or max_output_tokens), and wire it into SimpleCompaction. Also fixes ScriptedEchoChatProvider, which was missing with_generation_kwargs entirely.
…ffort Qwen3.x models expose a binary enable_thinking chat-template toggle, not the tiered low/medium/high reasoning_effort values OpenAI/Anthropic providers use. Self-hosted openai_legacy endpoints (llama.cpp/vLLM/LM Studio) were rejecting the tiered value for these models and silently promoting every configured effort level to full reasoning. Detect Qwen3.x models (mirroring the existing Kimi/GLM special-casing) and send chat_template_kwargs.enable_thinking instead.
The existing max_consecutive_failures backstop only trips when every tool call in a batch reports is_error=True, so it can't catch a degenerate loop where a tool falsely reports success on a call that never made progress (observed: a stuck agent burning 15 minutes and 142k tokens of context with no backstop firing). Add max_consecutive_identical_calls (default 10), tracked from the toolset's own identical-argument repeat streak rather than each call's reported success/failure, as a second independent backstop.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a capped output-token helper for compaction, switches Qwen3.x openai_legacy thinking to a binary chat-template toggle, and introduces a consecutive-identical-tool-call backstop with config, toolset, runtime, tests, and changelog updates. ChangesCompaction cap, Qwen3 toggle, and stuck backstop
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SimpleCompaction
participant capped_chat_provider
participant ChatProvider
SimpleCompaction->>capped_chat_provider: capped_chat_provider(llm, 4000)
capped_chat_provider->>ChatProvider: with_generation_kwargs(max_tokens=4000)
ChatProvider-->>SimpleCompaction: capped provider
SimpleCompaction->>ChatProvider: generate() summary request
sequenceDiagram
participant PythinkerSoul
participant PythinkerToolset
participant StepOutcome
PythinkerSoul->>PythinkerToolset: consecutive_repeat_count
PythinkerToolset-->>PythinkerSoul: streak length
PythinkerSoul->>PythinkerSoul: compare to max_consecutive_identical_calls
PythinkerSoul->>StepOutcome: stop_reason="stuck"
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pythinker_code/llm.py`:
- Around line 64-85: The kwarg override map in capped_chat_provider does not
handle the openai_codex provider type, so it falls back to max_tokens instead of
max_output_tokens and the cap can be ignored. Update
_MAX_OUTPUT_TOKENS_KWARG_OVERRIDES in llm.py to include the openai_codex key,
matching the OpenAIResponses-backed path used by create_llm, and add a test for
capped_chat_provider or test_compaction_caps_output_tokens that covers
provider_config.type == "openai_codex" to verify the capped kwarg is applied
correctly.
In `@tests/core/test_pythinkersoul_stuck_loop.py`:
- Around line 163-199: The `_make_soul_with_pythinker_toolset` helper duplicates
the full `Runtime(...)` rebuild already done in `_make_soul`; extract that
shared construction into a small helper (for example, a runtime-rebuild
function) and have both helpers call it. Keep the differing parts in
`_make_soul_with_pythinker_toolset` limited to the `PythinkerToolset`, `Agent`,
and `PythinkerSoul` setup so the duplicated runtime field wiring is centralized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3257809c-88d3-4b09-816e-e97f27dbbc9d
📒 Files selected for processing (14)
CHANGELOG.mdpackages/pythinker-core/src/pythinker_core/chat_provider/echo/scripted_echo.pypackages/pythinker-core/tests/test_scripted_echo_chat_provider.pysrc/pythinker_code/config.pysrc/pythinker_code/llm.pysrc/pythinker_code/soul/compaction.pysrc/pythinker_code/soul/pythinkersoul.pysrc/pythinker_code/soul/toolset.pytests/core/test_compaction_overflow.pytests/core/test_config.pytests/core/test_model_switch_carryover.pytests/core/test_notifications.pytests/core/test_openai_provider.pytests/core/test_pythinkersoul_stuck_loop.py
- Add openai_codex to the max_output_tokens kwarg-override map: it builds the same OpenAIResponses provider as openai_responses, so the compaction cap was silently no-op'ing (falling back to max_tokens) for ChatGPT/Codex-backed sessions. - Add direct capped_chat_provider coverage for all provider-type -> kwarg mappings, including the openai_codex case above. - Extract the duplicated Runtime(...) rebuild in test_pythinkersoul_stuck_loop.py into a shared helper.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/core/test_compaction_overflow.py`:
- Around line 75-82: The test currently ignores the return value from
capped_chat_provider() and only verifies the fake mutates in place, which
couples it to _FakeChatProvider.with_generation_kwargs() instead of the helper
contract. Update test_capped_chat_provider_picks_kwarg_by_provider_type to have
_FakeChatProvider.with_generation_kwargs() return a fresh provider instance,
capture the result of capped_chat_provider(), and assert on that returned
provider’s generation_kwargs rather than the original fake.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 831f1b8c-c2e5-46ef-956b-bcfeb5ca1c83
📒 Files selected for processing (3)
src/pythinker_code/llm.pytests/core/test_compaction_overflow.pytests/core/test_pythinkersoul_stuck_loop.py
_FakeChatProvider.with_generation_kwargs mutated self and returned self, so the new provider-type-mapping test only observed the fake's internal state rather than capped_chat_provider's actual return value -- a regression that discarded the capped copy would have passed unnoticed. Make the fake return a fresh instance (matching the real providers' copy-on-write contract) and assert on the returned/used provider in both the mapping test and test_compaction_caps_output_tokens.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three related fixes for local-model reliability issues reported against LM Studio-hosted models:
capped_chat_provider()now caps the summary call via the correct provider-specific kwarg. Also fixesScriptedEchoChatProvider, which was missingwith_generation_kwargsentirely and would have crashed once the cap was wired in.enable_thinkingchat-template toggle, not tieredreasoning_effortlevels. Self-hostedopenai_legacyendpoints (llama.cpp/vLLM/LM Studio) were silently promoting every configured effort level to full reasoning for these models. Detects Qwen3.x models (mirroring the existing Kimi/GLM special-casing) and sendschat_template_kwargs.enable_thinkinginstead.max_consecutive_failuresbackstop only trips when every tool call in a batch reportsis_error=True, so it can't catch a loop where a tool falsely reports success on a call that never made progress (observed: an agent burning 15 minutes / 142k tokens with no backstop firing). Addsmax_consecutive_identical_calls(default 10) as a second, independent backstop tracked from the toolset's own identical-argument repeat streak rather than each call's reported success/failure.Test plan
make check-pythinker-code— ruff/format/pyright/ty all passmake check-pythinker-core— ruff/pyright pass (ty non-blocking, pre-existing SDK-stub noise unrelated to this change)tests+tests_e2esuite green (6556 passed)packages/pythinker-coredoctest suite green (314 passed)enable_thinkingtoggle, identical-call stuck-loop backstop (fires + disables-at-zero)Summary by CodeRabbit
New Features
max_consecutive_identical_callsconsecutive identical tool-call arguments (default: 10;0disables).with_generation_kwargs(...).Bug Fixes