fix(telegram): suppress bare Reasoning: prefix leak during Gemini streaming#37892
fix(telegram): suppress bare Reasoning: prefix leak during Gemini streaming#37892okuyam2y wants to merge 2 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR fixes a prefix leak in the Telegram Gemini streaming path where bare Changes:
Issue found:
Confidence Score: 4/5
Last reviewed commit: e71a7ec |
| // Suppress partial "Reasoning:" arriving during streaming before any | ||
| // actual reasoning content follows (trim() collapses "Reasoning:\n" to | ||
| // "Reasoning:" which would otherwise fall through to answerText). | ||
| if (/^Reasoning:?\s*$/.test(trimmed)) { |
There was a problem hiding this comment.
The regex /^Reasoning:?\s*$/ makes the colon optional, so it also suppresses the bare string "Reasoning" (no colon). This is broader than the stated intent ("suppress partial Reasoning: chunks") and would silently drop any streaming chunk that is exactly the word "Reasoning".
Since REASONING_MESSAGE_PREFIX is "Reasoning:\n" (colon required), the guard should only match the colon form:
| if (/^Reasoning:?\s*$/.test(trimmed)) { | |
| if (/^Reasoning:\s*$/.test(trimmed)) { |
Additionally, \s* before $ is redundant here because this is applied to trimmed (the result of text.trim()), which already has no trailing whitespace. The guard still works correctly because \s* matches zero characters, but /^Reasoning:$/.test(trimmed) is equally correct and more self-documenting for this specific case.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/telegram/reasoning-lane-coordinator.ts
Line: 74
Comment:
The regex `/^Reasoning:?\s*$/` makes the colon optional, so it also suppresses the bare string `"Reasoning"` (no colon). This is broader than the stated intent ("suppress partial `Reasoning:` chunks") and would silently drop any streaming chunk that is exactly the word `"Reasoning"`.
Since `REASONING_MESSAGE_PREFIX` is `"Reasoning:\n"` (colon required), the guard should only match the colon form:
```suggestion
if (/^Reasoning:\s*$/.test(trimmed)) {
```
Additionally, `\s*` before `$` is redundant here because this is applied to `trimmed` (the result of `text.trim()`), which already has no trailing whitespace. The guard still works correctly because `\s*` matches zero characters, but `/^Reasoning:$/.test(trimmed)` is equally correct and more self-documenting for this specific case.
How can I resolve this? If you propose a fix, please make it concise.|
Fixed in 6d1bbba — changed Good catch, thanks. |
|
This pull request has been automatically marked as stale due to inactivity. |
|
Closing due to inactivity. |
Summary
Fixes #37890
Reasoning:/Reasoning:\nduring streaming partialstrim()collapses"Reasoning:\n"to"Reasoning:"which doesn't matchREASONING_MESSAGE_PREFIXand falls through toanswerText, leaking the prefix into user-visible output/^Reasoning:?\s*$/catches these partial chunks and returns{}(suppress) before they reach the answer pathTest plan
"Reasoning:","Reasoning:\n", and"Reasoning: \n"reasoningTextpnpm exec vitest run src/telegram/reasoning-lane-coordinator.test.ts— 6 tests passpnpm checkpassescc @AyaanZaidi — you recently worked on draft materialize in this area (#36746)