Skip to content

fix: don't discard model output past a literal reasoning close tag#5395

Open
nolanchic wants to merge 1 commit into
Aider-AI:mainfrom
nolanchic:fix/reasoning-literal-close-tag
Open

fix: don't discard model output past a literal reasoning close tag#5395
nolanchic wants to merge 1 commit into
Aider-AI:mainfrom
nolanchic:fix/reasoning-literal-close-tag

Conversation

@nolanchic

Copy link
Copy Markdown

Problem

For models with a reasoning_tag (e.g. DeepSeek-R1 → think), remove_reasoning_content silently discarded part of the model's real answer whenever the answer contained a literal closing tag (e.g. </think> inside code, markup, or prose) and the response also contained a real reasoning block.

What happens: the function first strips balanced <think>...</think> blocks, then has a fallback that drops everything before any remaining </think> (intended for the continuation case where the opening tag was emitted in a prior chunk). That fallback fired unconditionally, so a literal </think> in the answer was mistaken for an orphan reasoning close.

in:  <think>r</think>Here is code: print('</think>')
was: ')
now: Here is code: print('</think>')

This function runs on every assistant response for reasoning-tag models (Coder.remove_reasoning_content) and on every commit-message / chat-summarization call (Model.simple_send_with_retries), so the truncation was silent and the shortened text was what reached edit parsing / commit-message generation.

Fix

aider/reasoning_tags.py — only treat a remaining </tag> as an orphan close when the response contained no opening tag at all (the continuation case). When an opening tag was present, any </tag> left after the balanced-block substitution is a literal occurrence in the answer and is preserved.

The legitimate orphan-close behaviour (continuation whose <tag> was in a prior chunk) is unchanged.

Test plan

  • Added test_remove_reasoning_content_literal_closing_tag — reproduces the truncation on main (returns ')'), passes with the fix.
  • Added test_remove_reasoning_content_orphan_closing_tag_stripped — locks in that a genuine orphan close (no opening tag) is still stripped.
  • pytest tests/basic/test_reasoning.py tests/basic/test_models.py — 34 passed (incl. the existing test_remove_reasoning_content and the integration tests that drive coder.remove_reasoning_content() with mocked streaming).
  • pre-commit run --files aider/reasoning_tags.py tests/basic/test_reasoning.py — isort / black / flake8 / codespell all pass.

remove_reasoning_content strips <tag>...</tag> reasoning blocks, then has a
fallback that drops everything before a remaining </tag> (for the case where
the opening tag was emitted in a prior chunk, e.g. a length-limited
continuation). That fallback fired for *any* remaining </tag>, so when the
model's actual answer contained a literal </tag> (inside code, markup, or
prose) AND the response also had a real reasoning block, the real answer up
to that literal </tag> was silently discarded.

Example (reasoning_tag="think"):
  in:  <think>r</think>Here is code: print('</think>')
  was: ')
  now: Here is code: print('</think>')

This runs on every assistant response for reasoning-tag models and on every
commit-message / summarization call, so the truncation was silent and
affected downstream edit parsing.

Only treat a remaining </tag> as an orphan close when the response had no
opening tag at all. When it did, any </tag> left after the balanced-block
substitution is a literal occurrence and is preserved.

Adds regression tests for both the literal case and the orphan case.
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.

1 participant