feat(studio): Studio copilot reasoning trace - #1106
Merged
htolentino-nvidia merged 2 commits intoAug 7, 2026
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughChangesReasoning streaming and chat sequencing
Sequence Diagram(s)sequenceDiagram
participant Model
participant ReasoningStreamHandler
participant StudioCopilot
participant CopilotChat
Model->>ReasoningStreamHandler: reasoning_content
ReasoningStreamHandler->>StudioCopilot: NAT reasoning intermediate step
StudioCopilot->>CopilotChat: reasoning assistant event
CopilotChat->>CopilotChat: render reasoning as assistant text
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
services/studio/src/nmp/studio/copilot.py (1)
1559-1588: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the duplicate tool-input definitions.
_parse_tool_step_inputis already defined at lines 1493-1517. This second definition replaces it at import time. Keep one parser and one_TOOL_INPUT_INTERNAL_KEYSdeclaration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/studio/src/nmp/studio/copilot.py` around lines 1559 - 1588, Remove the duplicate _parse_tool_step_input definition and duplicate _TOOL_INPUT_INTERNAL_KEYS declaration shown in this section, retaining the existing earlier definitions. Ensure the module has only one parser implementation and one internal-key constant without changing their behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@services/studio/src/nmp/studio/copilot.py`:
- Around line 1559-1588: Remove the duplicate _parse_tool_step_input definition
and duplicate _TOOL_INPUT_INTERNAL_KEYS declaration shown in this section,
retaining the existing earlier definitions. Ensure the module has only one
parser implementation and one internal-key constant without changing their
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3167a490-30f8-4698-ba9c-4301a718e842
📒 Files selected for processing (8)
agents/nemo-studio-copilot/src/nemo_studio_copilot/register.pyagents/nemo-studio-copilot/src/nemo_studio_copilot/wrapper.pyservices/studio/src/nmp/studio/copilot.pyservices/studio/tests/unit/test_copilot.pyweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.ts
Contributor
|
htolentino-nvidia
force-pushed
the
studio-copilot-reasoning-trace/htolentino
branch
from
August 6, 2026 13:31
a481b10 to
e4fd9aa
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
services/studio/src/nmp/studio/copilot.py (1)
1559-1588: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the duplicate tool-input definitions.
Lines 1559-1588 redefine
_parse_tool_step_inputand_TOOL_INPUT_INTERNAL_KEYS. Retain the earlier definitions. Duplicate implementations can diverge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/studio/src/nmp/studio/copilot.py` around lines 1559 - 1588, Remove the later duplicate definitions of _parse_tool_step_input and _TOOL_INPUT_INTERNAL_KEYS, retaining the earlier implementations as the single source of truth. Leave surrounding tool-input handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/studio/src/nmp/studio/copilot.py`:
- Around line 1667-1675: Update the reasoning branch in the step-processing flow
around _parse_reasoning_step_output and _reasoning_stream_event to track
completed reasoning step["id"] values and skip duplicate completed events. Only
record an ID after parsing returns nonempty text, and do not mark paired start
events as seen. Add coverage that submits the same completed reasoning event
twice and verifies only one reasoning block is emitted.
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.ts`:
- Around line 235-244: Update the final returned-result path around
completeActiveAssistantMessage to call ensureAssistantMessage() before
completing result.text or result.content, preserving the result under a newly
created assistant message when prepareForUserInput cleared the ID and avoiding
reuse of an old message above the picker answer. Add a regression test covering
a returned result after appendUserMessage.
---
Nitpick comments:
In `@services/studio/src/nmp/studio/copilot.py`:
- Around line 1559-1588: Remove the later duplicate definitions of
_parse_tool_step_input and _TOOL_INPUT_INTERNAL_KEYS, retaining the earlier
implementations as the single source of truth. Leave surrounding tool-input
handling unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9192f5a8-9510-4b2a-8fb6-c793c20917a5
📒 Files selected for processing (8)
agents/nemo-studio-copilot/src/nemo_studio_copilot/register.pyagents/nemo-studio-copilot/src/nemo_studio_copilot/wrapper.pyservices/studio/src/nmp/studio/copilot.pyservices/studio/tests/unit/test_copilot.pyweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- web/packages/studio/src/routes/agents/CopilotChatRoute/stream.test.ts
- agents/nemo-studio-copilot/src/nemo_studio_copilot/register.py
- web/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.test.ts
- agents/nemo-studio-copilot/src/nemo_studio_copilot/wrapper.py
- web/packages/studio/src/routes/agents/CopilotChatRoute/stream.ts
Nemotron and other vLLM/NIM-served models stream their chain of thought in a ``reasoning_content`` delta, but it never reached the UI. Each layer dropped it: - langchain-openai 1.4.x only understands OpenAI's o-series ``reasoning`` block, so it discards ``reasoning_content`` on both the streaming and the non-streaming path. ``_preserve_reasoning_content`` re-attaches it to the message. - NAT's ChatResponseChunk carries only ``content``, so the trace rides the same ``intermediate_data:`` channel the tool-call trace already uses. It is published per LLM run rather than per token, since one step per token would be thousands of frames for a single answer. NAT's StepAdaptor forwards only LLM, TOOL and FUNCTION categories and drops an end step whose start carries no input, hence the paired LLM steps. - Studio relays it as a message part. It deliberately bypasses the tool dedup: the pair shares one id, so the start would claim it and the end -- which carries the trace -- would be dropped. - The UI renders it as ordinary assistant text, so it reads as narration between tool calls rather than hiding in a collapsed block. Also fix tool-call ordering, which the same rendering path exposed: answering a blocking picker appends a user message mid-run, and a tool step arriving before the answer re-attached the run to the assistant message above it, so later tool calls rendered before the answer that triggered them. Signed-off-by: Henrique Tolentino <htolentino@nvidia.com>
htolentino-nvidia
force-pushed
the
studio-copilot-reasoning-trace/htolentino
branch
from
August 6, 2026 14:08
e4fd9aa to
89dd457
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
services/studio/src/nmp/studio/copilot.py (1)
1559-1590: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the duplicate
_parse_tool_step_inputdefinition. Keep one implementation to prevent future definitions from diverging.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/studio/src/nmp/studio/copilot.py` around lines 1559 - 1590, Remove the duplicate _parse_tool_step_input definition, retaining the existing single implementation and its current parsing behavior. Ensure callers continue resolving to that one function and do not alter the nearby _TOOL_INPUT_INTERNAL_KEYS declaration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@services/studio/src/nmp/studio/copilot.py`:
- Around line 1559-1590: Remove the duplicate _parse_tool_step_input definition,
retaining the existing single implementation and its current parsing behavior.
Ensure callers continue resolving to that one function and do not alter the
nearby _TOOL_INPUT_INTERNAL_KEYS declaration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1bf01763-89c4-4085-9fac-e015b59c927c
📒 Files selected for processing (8)
agents/nemo-studio-copilot/src/nemo_studio_copilot/register.pyagents/nemo-studio-copilot/src/nemo_studio_copilot/wrapper.pyservices/studio/src/nmp/studio/copilot.pyservices/studio/tests/unit/test_copilot.pyweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- agents/nemo-studio-copilot/src/nemo_studio_copilot/register.py
- web/packages/studio/src/routes/agents/CopilotChatRoute/stream.test.ts
- web/packages/studio/src/routes/agents/CopilotChatRoute/stream.ts
- web/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.ts
- agents/nemo-studio-copilot/src/nemo_studio_copilot/wrapper.py
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
services/studio/src/nmp/studio/copilot.py (1)
1559-1588: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep one tool-input parser definition.
Remove or reconcile the earlier definitions of
_parse_tool_step_inputand_TOOL_INPUT_INTERNAL_KEYS. Python uses the later definitions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/studio/src/nmp/studio/copilot.py` around lines 1559 - 1588, Consolidate `_parse_tool_step_input` and `_TOOL_INPUT_INTERNAL_KEYS` into single definitions, removing or reconciling any earlier duplicates so the intended parser and internal-key set are unambiguous. Preserve the existing parsing behavior and filtering of `studio_session_id`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@services/studio/src/nmp/studio/copilot.py`:
- Around line 1559-1588: Consolidate `_parse_tool_step_input` and
`_TOOL_INPUT_INTERNAL_KEYS` into single definitions, removing or reconciling any
earlier duplicates so the intended parser and internal-key set are unambiguous.
Preserve the existing parsing behavior and filtering of `studio_session_id`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dda7beac-0084-4a77-86e0-f947d408d1d3
📒 Files selected for processing (8)
agents/nemo-studio-copilot/src/nemo_studio_copilot/register.pyagents/nemo-studio-copilot/src/nemo_studio_copilot/wrapper.pyservices/studio/src/nmp/studio/copilot.pyservices/studio/tests/unit/test_copilot.pyweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- web/packages/studio/src/routes/agents/CopilotChatRoute/stream.test.ts
- agents/nemo-studio-copilot/src/nemo_studio_copilot/register.py
- web/packages/studio/src/routes/agents/CopilotChatRoute/stream.ts
- web/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.ts
- web/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.test.ts
htolentino-nvidia
enabled auto-merge
August 6, 2026 17:40
htolentino-nvidia
disabled auto-merge
August 6, 2026 20:15
htolentino-nvidia
enabled auto-merge
August 6, 2026 20:15
htolentino-nvidia
disabled auto-merge
August 6, 2026 20:18
htolentino-nvidia
marked this pull request as draft
August 6, 2026 20:18
htolentino-nvidia
marked this pull request as ready for review
August 6, 2026 20:18
htolentino-nvidia
enabled auto-merge
August 6, 2026 20:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Nemotron and other vLLM/NIM-served models stream their chain of thought in a
reasoning_contentdelta, but it never reached the UI. Each layer dropped it:reasoningblock, so it discardsreasoning_contenton both the streaming and the non-streaming path._preserve_reasoning_contentre-attaches it to the message.content, so the trace rides the sameintermediate_data:channel the tool-call trace already uses. It is published per LLM run rather than per token, since one step per token would be thousands of frames for a single answer. NAT's StepAdaptor forwards only LLM, TOOL and FUNCTION categories and drops an end step whose start carries no input, hence the paired LLM steps.Also fix tool-call ordering, which the same rendering path exposed: answering a blocking picker appends a user message mid-run, and a tool step arriving before the answer re-attached the run to the assistant message above it, so later tool calls rendered before the answer that triggered them.
Summary by CodeRabbit
New Features
Bug Fixes
Tests