fix(ai): keep response content when a stray </think> follows a real think block - #836
Conversation
…hink block stripThinkingTags ran its orphan </think> pass unconditionally. For models that emit a proper <think>…</think> block and then leak a stray </think> later in the answer, everything between the real close and the stray close was reclassified as thinking and dropped from the visible response. Only apply the orphan separator when the original text had no opening tag (the Nemotron case it is meant for); otherwise leave any remaining </think> to the existing stray-tag stripping. Adds regression tests covering the stray-close case plus the preserved Nemotron, long-form, and plain-content paths.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2ab61eae3
ℹ️ 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".
| let hasOpeningThinkTag = text.range(of: "<think>") != nil | ||
| || text.range(of: "<thinking>") != nil |
There was a problem hiding this comment.
When Nemotron-style non-streaming output uses reasoning</think>answer, this flag now disables the orphan-separator pass if the visible answer happens to contain the literal text <think> or <thinking> anywhere. For example, reasoning</think>Explain the <think> tag is returned as visible content with the reasoning leaked at the front instead of extracting reasoning, because there was no real opening tag before the separator—only tag text in the answer. Consider basing this guard on whether the proper-pair regex actually removed a real block before skipping orphan parsing.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR corrects non-streaming thinking-token extraction so a stray closing tag after a valid thinking block no longer causes answer content to be reclassified as reasoning and discarded.
|
2 participants
Description
LLMClient.stripThinkingTags(the non-streaming thinking-token extractor used by every non-streaming LLM response) ran its orphan</think>pass unconditionally. That pass is meant for Nemotron-style output that has no opening tag and uses</think>as the separator (thoughts</think>response). But when a model emits a proper<think>…</think>block and then leaks a second, stray</think>later in its answer, the orphan pass reclassified every character between the real close and the stray close as thinking — silently dropping it from the visible response. Example:<think>reasoning here</think>The literal </think> tag is stray.reasoning hereThe literal, content =tag is stray.(the answer textThe literalis lost)reasoning here, content =The literal tag is stray.The fix only applies the orphan separator when the original text had no opening
<think>/<thinking>tag (the Nemotron case it is meant for). When an opening tag was present, any remaining</think>is stray markup and is left to the existing stray-tag stripping, preserving the answer.Type of Change
Related Issue or Discussion
Same thinking-token extraction area as the reasoning-content regression in #445 and the Anthropic thinking/temperature handling in #285. No standalone issue exists for the stray-close case, so linking the adjacent regressions.
Testing
swiftlint --strict --config .swiftlint.yml Sourceson the edited files → 0 violationsswiftformat --config .swiftformat Sources(not a CI check; skipped to avoid unrelated formatting churn)xcodebuild test -project Fluid.xcodeproj -scheme Fluid -destination 'platform=macOS,arch=arm64' -only-testing:FluidDictationIntegrationTests/StripThinkingTagsTests→ 5 tests, 0 failures.testStrayCloseAfterRealThinkBlockKeepsContentfails on the previous unconditional orphan pass and passes with the opening-tag guard; the Nemotron (testOrphanCloseWithoutOpeningTagStillSplits,…SupportsLongFormTag), well-formed-block, and plain-content cases are unchanged.Screenshots / Video
Notes
Non-streaming path only; streaming uses the per-model
ThinkingParserfinalize (which already strips stray</think>without reclassifying content), so this brings the non-streaming path to the same behavior. The genuinely ambiguous case (a stray</think>in content from a model that never emits an opening tag) is intentionally left as-is — it is indistinguishable from Nemotron's separator.