fix(telegram,feishu): respect reasoningDefault config for reasoning stream previews#71817
Conversation
Greptile SummaryThis PR fixes Confidence Score: 4/5Safe to merge; logic is correct and existing behaviour is preserved — the only concern is a potential documentation/semantic gap for the "on" level in Feishu. All test cases pass for the stated goals; the only unresolved question is whether reasoningDefault: "on" is intentionally a no-op for Feishu's streaming preview. If that's by design, this is a clean change; if not, it is an incomplete fix. extensions/feishu/src/reasoning-preview.ts — specifically the configDefault === "stream" early-return when sessionKey is absent and the fallback at line 34. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/feishu/src/reasoning-preview.ts
Line: 19-21
Comment:
**`reasoningDefault: "on"` silently ignored for Feishu**
`configDefault === "stream"` only returns `true` for `"stream"`, so setting `reasoningDefault: "on"` in the agent config still produces `false` for Feishu's reasoning preview — identical to the pre-fix behaviour. The existing test on line 28-46 confirms this is currently intentional (`"on"` session level → `false`), but the PR description says both `"on"` and `"stream"` should be fixed. If `"on"` is genuinely meant to enable Feishu reasoning preview too, the condition should be `configDefault !== "off"` (mirroring the Telegram side). If Feishu only supports the streaming variant, a brief code comment would prevent future confusion.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: respect reasoningDefault config in ..." | Re-trigger Greptile Re-review progress:
|
| if (!params.sessionKey) { | ||
| return false; | ||
| return configDefault === "stream"; | ||
| } |
There was a problem hiding this comment.
reasoningDefault: "on" silently ignored for Feishu
configDefault === "stream" only returns true for "stream", so setting reasoningDefault: "on" in the agent config still produces false for Feishu's reasoning preview — identical to the pre-fix behaviour. The existing test on line 28-46 confirms this is currently intentional ("on" session level → false), but the PR description says both "on" and "stream" should be fixed. If "on" is genuinely meant to enable Feishu reasoning preview too, the condition should be configDefault !== "off" (mirroring the Telegram side). If Feishu only supports the streaming variant, a brief code comment would prevent future confusion.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/reasoning-preview.ts
Line: 19-21
Comment:
**`reasoningDefault: "on"` silently ignored for Feishu**
`configDefault === "stream"` only returns `true` for `"stream"`, so setting `reasoningDefault: "on"` in the agent config still produces `false` for Feishu's reasoning preview — identical to the pre-fix behaviour. The existing test on line 28-46 confirms this is currently intentional (`"on"` session level → `false`), but the PR description says both `"on"` and `"stream"` should be fixed. If `"on"` is genuinely meant to enable Feishu reasoning preview too, the condition should be `configDefault !== "off"` (mirroring the Telegram side). If Feishu only supports the streaming variant, a brief code comment would prevent future confusion.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Addressing Greptile: This is intentional. Feishu reasoning preview only supports the stream variant — on means block-mode reasoning (delivered after completion), which has no live preview equivalent in Feishu. Added a clarifying comment in 01b2bec.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69e419c3c0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -188,8 +188,16 @@ function resolveTelegramReasoningLevel(params: { | |||
| telegramDeps: TelegramBotDeps; | |||
| }): TelegramReasoningLevel { | |||
| const { cfg, sessionKey, agentId, telegramDeps } = params; | |||
|
|
|||
| // Resolve config-driven default: per-agent override > hardcoded "off". | |||
| const agentDefault = cfg.agents?.list?.find((a) => a.id === agentId)?.reasoningDefault; | |||
There was a problem hiding this comment.
Normalize agent IDs before applying reasoningDefault
agentId here comes from routing, and resolveAgentRoute normalizes IDs via pickFirstExistingAgentId/sanitizeAgentId, so it is lowercase even if agents.list[].id was configured as mixed case. This case-sensitive lookup (a.id === agentId) can miss the configured agent entry and silently fall back to "off", so the new reasoningDefault fallback still does not work for valid mixed-case configs. The same pattern is also present in extensions/feishu/src/reasoning-preview.ts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid point, but this case-sensitive lookup matches the existing pattern used elsewhere in the codebase (e.g. resolveAgentOutboundIdentity, directive resolution). Adding normalisation here alone would be inconsistent. Would be worth a separate issue to normalise agent ID lookups across the board if maintainers agree — happy to file one.
|
CI note: The 3 failing checks ( Re: Greptile comment on Feishu |
|
Codex review: needs real behavior proof before merge. Summary Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path. Real behavior proof Next step before merge Review detailsBest possible solution: Retry the Codex review after fixing the execution failure. Do we have a high-confidence way to reproduce the issue? Unclear. The review failed before ClawSweeper could establish a reproduction path. Is this the best way to solve the issue? Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction. What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against bc720dedaf0e. |
|
Hey @obviyus — sorry for the delay, I wasn't aware I needed to manually ping a maintainer after addressing the bot feedback. The Greptile concern about Feishu |
|
Codex review: found issues before merge. What this changes: The PR updates Telegram and Feishu reasoning-preview resolution to use Maintainer follow-up before merge: This is an open contributor implementation PR for a real current-main gap, but the remaining action is maintainer/channel-owner review and a small branch update decision rather than an autonomous replacement repair. Review findings:
Review detailsBest possible solution: Update the branch so Telegram and Feishu resolve the same effective reasoning default that current main exposes: explicit session state first, then normalized per-agent default, then global default, then Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 3215ab6de5db. |
01b2bec to
36753f5
Compare
|
CI failures on this run appear to be infrastructure flakes unrelated to the PR changes:
Could a maintainer re-run the failed checks? Happy to investigate further if any failure turns out to be related to the PR. |
c53c601 to
500e68e
Compare
538dd4f to
9606b0b
Compare
2eb00ac to
97b4e7e
Compare
3d9f3c2 to
afe2850
Compare
7b9602f to
ea1cd0a
Compare
|
Re the ClawSweeper P2 on 'Use the shared reasoning-default resolution': The P2 says the fallback does a raw
Both behaviours are covered by tests in
|
7540f67 to
6f29f55
Compare
9bdff50 to
5ca5e73
Compare
|
Rebased onto latest Also fixed two upstream oxlint regressions from f6f8d74 ( Full CI ( Update (May 7): Stripped Changes since rebase:
All 35 draft-stream tests pass. HEAD is |
5ca5e73 to
9b0cfc8
Compare
9b0cfc8 to
05ea934
Compare
|
Combined test branch available: If you'd like to try both fixes together: npm install -g anagnorisis2peripeteia/openclaw#5.6+claude-cli-telegram-reasoningWhat's included:
Feedback welcome -- especially on how the reasoning stream behaves during long thinking blocks and whether the overflow chaining feels natural. |
|
Landed via rebase onto main.
Thanks @anagnorisis2peripeteia! |
Summary
reasoningDefaultwhen deciding whether channel reasoning previews should stream or stay hidden.agents.list[].reasoningDefault, then globalagents.defaults.reasoningDefault, thenoff.reasoningDefault: "on"is block-mode visibility, so it does not enable a live Feishu preview.Addresses the Telegram/Feishu preview-default part of #73182. The broader
/thinkbilling/default behavior is separate.Verification
pnpm test extensions/feishu/src/reasoning-preview.test.ts extensions/telegram/src/bot-message-dispatch.test.tspnpm exec oxfmt --check --threads=1 CHANGELOG.md docs/tools/thinking.md extensions/feishu/src/agent-config.ts extensions/feishu/src/bot.ts extensions/feishu/src/reasoning-preview.test.ts extensions/feishu/src/reasoning-preview.ts extensions/telegram/src/agent-config.ts extensions/telegram/src/bot-message-dispatch.test.ts extensions/telegram/src/bot-message-dispatch.tspnpm check:test-typesNotes
main, dropped generated SDK docs/API/package export changes, and kept the fix inside the owning plugins.ubuntu@100.79.136.128: OpenClaw 2026.5.6 with Telegram configured, but noreasoningDefaultconfigured there, so it was not a useful live proof target without changing state.