feat(llmloop): backfill comment thinking from turn output by yingjiexu2002 · Pull Request #773 · alibaba/open-code-review

yingjiexu2002 · 2026-08-07T13:44:08Z

Description

Comments parsed from code_comment tool calls have a runtime-only thinking field that most models never populate (it is deliberately not advertised in the tool schema). This PR backfills it from the model's output of the current turn, so --format json output carries the review reasoning:

Add ChatResponse.ReasoningContent() to expose the native reasoning content of a response (OpenAI reasoning_content in both streaming and non-streaming paths; Anthropic thinking blocks).
In the agent loop, compute the turn's thinking text as reasoning content, falling back to the assistant message for models that do not expose reasoning, and backfill it into each collected comment whose thinking is empty. Explicit model-provided thinking always wins.
Terminal output is unchanged (renderComment does not render thinking); only the JSON output gains the field.

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Refactoring (no functional changes)
Documentation update
CI / Build / Tooling

How Has This Been Tested?

make test passes locally
Manual testing (describe below)

Added unit tests: backfill uses the turn's thinking text when a comment has no explicit thinking; explicit thinking is preserved; no backfill when the turn has no thinking text. Also verified end-to-end with a local review run (--format json) against providers with and without native reasoning output.

Checklist

My code follows the project's coding style (go fmt, go vet)
I have performed a self-review of my code
I have added tests that prove my fix is effective or my feature works
New and existing unit tests pass locally with my changes
I have updated the documentation accordingly (if applicable)
I have signed the CLA

Related Issues

N/A

… message Expose ChatResponse.ReasoningContent and backfill per-comment thinking with the current turn's reasoning content, falling back to the assistant message for models that do not expose reasoning, so --format json output carries thinking even when the model omits it.

github-actions · 2026-08-07T13:45:24Z

✅ OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s).

lizhengfeng101 · 2026-08-08T04:38:21Z

On the content fallback for thinking

The backfill prefers reasoning_content and falls back to the turn's assistant message (content) when there's no native reasoning. I'd suggest verifying this fallback is actually worth keeping before we ship it.

The concern: for a tool-calling turn, the assistant message is often not real reasoning — it tends to be a short, user-facing preamble like "I'll now leave a comment on this file" or "Here's my feedback". Backfilling that into thinking (and copying it across every comment in the turn) adds noise to the JSON output rather than the review reasoning we actually want, and it's the part most likely to read as misleading.

Could you run a few review passes against a couple of models without native reasoning_content and check what content actually contains in those turns? If it's mostly these low-value summaries, the fallback buys us little and I'd lean toward only backfilling from reasoning_content and leaving thinking empty otherwise. If you find that some models do put substantive reasoning in content, then keeping the fallback makes sense — ideally noted with an example in the PR description.

Either way, capturing what you observe would make the fallback's necessity clear.

wu21-web

You idea is great and we appreciate it. Here are several suggestions.
By the way, im not sure if this is true, but it is worth considering: JSON output marshals the whole LlmComment and terminal renderComment never renders it.

wu21-web · 2026-08-08T13:33:52Z

+		if thinking == "" {
+			thinking = content
+		}


Suggested change

if thinking == "" {

thinking = content

}

Capture the model's native reasoning content for this turn.

wu21-web · 2026-08-08T13:35:50Z

 	"context"
 	"strings"
 	"testing"



Suggested change

"github.com/alibaba/open-code-review/internal/config/template"

Tests are needed to lock in the new behavior and regression tests that would fail if the fallback returns.
Tip on L11: this is a package you need to include for new tests, e.g TestRunPerFile_BackfillsThinkingFromReasoningContent
is a test to verify the full wiring

Added in 2b18430: TestRunPerFile_BackfillsThinkingFromReasoningContent drives the full RunPerFile loop via a scripted fake client and asserts the comment gets the turn's reasoning_content; TestRunPerFile_NoFallbackToContent asserts thinking stays empty when only content exists — verified it fails if the fallback is re-added.

wu21-web · 2026-08-08T13:39:41Z

Russian site documentation is missing, use a translator to translate this to pages/scr/content/docs/ru/tools.md

wu21-web · 2026-08-08T13:40:34Z

+		// Prefer the model's native reasoning content; fall back to the
+		// assistant message of this turn for models that do not expose it.


Suggested change

// Prefer the model's native reasoning content; fall back to the

// assistant message of this turn for models that do not expose it.

// Capture the model's native reasoning content for this turn. Models

// without a reasoning channel leave it empty.

wu21-web · 2026-08-08T13:41:51Z

-captures the model's reasoning and is preserved on the comment but not
-shown in the final review output.
+captures the model's reasoning and is preserved on the comment; OCR
+backfills it from the reasoning content the model emits on the current
+turn (or the turn's message when there is none), and includes it in
+the JSON output (the terminal output does not
+render it).


If you applied the suggestion here: https://github.com/alibaba/open-code-review/pull/773/changes#r3740773775
This means you have to likely update the documentation here.

wu21-web · 2026-08-08T13:42:01Z

-モデルが明示的に設定する必要はほとんどありません。`thinking`（コメントごと）はモデルの推論を捕捉し、コメントに保持されますが、
-最終的なレビュー出力には表示されません。
+モデルが明示的に設定する必要はほとんどありません。`thinking`（コメントごと）はモデルの推論を捕捉し、コメントに保持されます。
+OCR はモデルが現在のターンで出力した推論内容（推論内容がない場合はそのターンの本文）で自動的に補完し、JSON 出力に含めます（ターミナル出力には表示されません）。


wu21-web · 2026-08-08T13:42:11Z

-模型极少需要显式设置。`thinking`（按评论）捕获模型推理，保留在评论上，但不会
-在最终评审输出中显示。
+模型极少需要显式设置。`thinking`（按评论）捕获模型推理，保留在评论上；OCR 会用
+模型当轮输出的推理内容（无推理内容时用当轮正文）自动回填，并包含在 JSON 输出中（终端输出不渲染）。


The turn's assistant message is usually a short user-facing preamble rather than real reasoning, so backfill per-comment thinking only from the model's native reasoning_content and leave it empty otherwise. Add a full-wiring RunPerFile test for the reasoning backfill and a regression test that fails if the content fallback returns. Sync the thinking docs across en/zh/ja/ru.

yingjiexu2002 · 2026-08-09T10:44:53Z

On the content fallback for thinking

The backfill prefers reasoning_content and falls back to the turn's assistant message (content) when there's no native reasoning. I'd suggest verifying this fallback is actually worth keeping before we ship it.

The concern: for a tool-calling turn, the assistant message is often not real reasoning — it tends to be a short, user-facing preamble like "I'll now leave a comment on this file" or "Here's my feedback". Backfilling that into thinking (and copying it across every comment in the turn) adds noise to the JSON output rather than the review reasoning we actually want, and it's the part most likely to read as misleading.

Could you run a few review passes against a couple of models without native reasoning_content and check what content actually contains in those turns? If it's mostly these low-value summaries, the fallback buys us little and I'd lean toward only backfilling from reasoning_content and leaving thinking empty otherwise. If you find that some models do put substantive reasoning in content, then keeping the fallback makes sense — ideally noted with an example in the PR description.

Either way, capturing what you observe would make the fallback's necessity clear.

Removed. In an opus 4.6 run without thinking, the fallback fired on only 8/23 comments and just 3 of those were valuable — the rest were preambles like "I'll now leave a comment on this file". thinking is now backfilled only from reasoning_content and left empty otherwise; docs and tests updated.

lizhengfeng101

@yingjiexu2002 When multiple tool calls share the same thinking behavior in the same turn, it is recommended to explicitly state in the code comments that this is an intentional design to avoid future maintainers mistakenly thinking it is a bug.

Document in the main loop and at the code_comment backfill site that the model emits reasoning once per turn, so every tool call and comment in the same turn intentionally shares the same thinking.

yingjiexu2002 · 2026-08-10T05:08:13Z

@yingjiexu2002 When multiple tool calls share the same thinking behavior in the same turn, it is recommended to explicitly state in the code comments that this is an intentional design to avoid future maintainers mistakenly thinking it is a bug.

Done, documented at both sites in loop.go

lizhengfeng101

LGTM

* feat(llmloop): backfill comment thinking from the turn's reasoning or message Expose ChatResponse.ReasoningContent and backfill per-comment thinking with the current turn's reasoning content, falling back to the assistant message for models that do not expose reasoning, so --format json output carries thinking even when the model omits it. * fix(llmloop): drop content fallback for comment thinking backfill The turn's assistant message is usually a short user-facing preamble rather than real reasoning, so backfill per-comment thinking only from the model's native reasoning_content and leave it empty otherwise. Add a full-wiring RunPerFile test for the reasoning backfill and a regression test that fails if the content fallback returns. Sync the thinking docs across en/zh/ja/ru. * docs(llmloop): note that turn-level thinking is shared by design Document in the main loop and at the code_comment backfill site that the model emits reasoning once per turn, so every tool call and comment in the same turn intentionally shares the same thinking.

wu21-web suggested changes Aug 8, 2026

View reviewed changes

KlausFan mentioned this pull request Aug 9, 2026

Thinking state is parsed but dropped when assistant tool-call turns are replayed #804

Closed

Fanzzzd mentioned this pull request Aug 9, 2026

Thinking state is parsed but dropped when assistant tool-call turns are replayed #805

Open

lizhengfeng101 reviewed Aug 10, 2026

View reviewed changes

docs(llmloop): note that turn-level thinking is shared by design

373b8c3

Document in the main loop and at the code_comment backfill site that the model emits reasoning once per turn, so every tool call and comment in the same turn intentionally shares the same thinking.

wu21-web approved these changes Aug 10, 2026

View reviewed changes

lizhengfeng101 approved these changes Aug 10, 2026

View reviewed changes

lizhengfeng101 merged commit 71d2981 into alibaba:main Aug 10, 2026
13 checks passed



	"github.com/alibaba/open-code-review/internal/config/template"

		// Prefer the model's native reasoning content; fall back to the
		// assistant message of this turn for models that do not expose it.

Uh oh!

Conversation

yingjiexu2002 commented Aug 7, 2026

Description

Type of Change

How Has This Been Tested?

Checklist

Related Issues

Uh oh!

github-actions Bot commented Aug 7, 2026

Uh oh!

lizhengfeng101 commented Aug 8, 2026

Uh oh!

wu21-web left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wu21-web Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yingjiexu2002 commented Aug 9, 2026

Uh oh!

lizhengfeng101 left a comment

Choose a reason for hiding this comment

Uh oh!

yingjiexu2002 commented Aug 10, 2026

Uh oh!

lizhengfeng101 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wu21-web Aug 8, 2026 •

edited

Loading