feat(studio): Copilot chat history - #1092
Merged
Merged
Conversation
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesCopilot conversations now use durable Entity Store persistence with workspace and owner checks, deletion, bounded request context, optimistic concurrency, and legacy history replay. Studio chat routes, APIs, types, runtime, history UI, tool rendering, navigation, and tests now use Copilot-specific names and integrations. The NeMo client now prefers Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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.
Actionable comments posted: 3
🧹 Nitpick comments (6)
web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/HistoryPanelContents.tsx (1)
21-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd an explicit component return type.
HistoryPanelContentsis an exported complex component. Declare itsJSX.Elementreturn type.As per coding guidelines, “Use explicit return types for public APIs and complex functions in TypeScript.”
🤖 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 `@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/HistoryPanelContents.tsx` around lines 21 - 26, Add an explicit JSX.Element return type to the exported HistoryPanelContents component declaration, preserving its existing props and implementation.Source: Coding guidelines
web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/HistorySessionButton.tsx (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a type-only React import.
Reactonly providesReact.JSX.Element. Change this toimport type React from 'react'.As per coding guidelines, “Use
import typefor type-only imports in TypeScript.”🤖 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 `@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/HistorySessionButton.tsx` at line 13, Change the React import in HistorySessionButton.tsx to a type-only import, since React is used only for the React.JSX.Element type.Source: Coding guidelines
web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.ts (1)
58-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd an explicit return type.
getClaudeCodeHistorySessionsQueryKeyis an exported API. Declare its readonly tuple return type.As per coding guidelines, “Use explicit return types for public APIs and complex functions in TypeScript.”
🤖 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 `@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.ts` around lines 58 - 59, Update the exported getClaudeCodeHistorySessionsQueryKey function with an explicit readonly tuple return type matching its four elements: the two string literals, the sessions literal, and the workspace string.Source: Coding guidelines
services/studio/tests/unit/test_copilot.py (2)
310-322: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAssert the response status.
test_create_session_persists_workspace_and_ownerreadsresponse.json()without checkingresponse.status_code. A 4xx response fails later with a confusingKeyError.Proposed fix
+ assert response.status_code == 200 session_id = response.json()["session_id"]🤖 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/tests/unit/test_copilot.py` around lines 310 - 322, Add an explicit successful status assertion to test_create_session_persists_workspace_and_owner immediately after the service_client.post call, before reading response.json()["session_id"], while preserving the existing persistence and owner/message assertions.
79-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the conflict and persistence-failure branches.
FakeEntityStore.deletediscardsexpected_db_version, so theEntityConflictError→ 409 branch indelete_session_historyis untested. TheEntityStoreError→ streamed error branch in_stream_copilotis also untested.Make the fake honor
expected_db_versionand raiseEntityConflictErroron mismatch, and add a test whereupdateraisesEntityStoreError.🤖 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/tests/unit/test_copilot.py` around lines 79 - 91, Update FakeEntityStore.delete to honor expected_db_version and raise EntityConflictError when it differs from the stored entity version, then add coverage for delete_session_history’s conflict/409 path. Add a _stream_copilot test using an update implementation that raises EntityStoreError and assert the streamed error response.services/studio/src/nmp/studio/entities.py (1)
27-28: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider bounding persisted
messagesgrowth.
messagesgrows without limit. Each completed turn rewrites the whole conversation entity throughentity_store.update, so per-turn write size grows linearly with history length. Long sessions will produce large entity payloads and slow updates.Consider capping stored turns, or storing messages as separate append-only entities.
🤖 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/entities.py` around lines 27 - 28, Bound persisted conversation history represented by the messages field on the relevant entity, retaining only the most recent configured turns before entity_store.update writes it. Use an existing configuration or limit symbol if available, and preserve the current chat_artifacts behavior; do not allow messages to grow without limit.
🤖 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 `@agents/nemo-studio-copilot/Dockerfile`:
- Around line 58-63: Update the NEMO_BASE_URL default to use an HTTPS loopback
URL so SDK requests cannot send bearer tokens over plaintext HTTP. Preserve the
existing port and deployment override behavior, and ensure the configured base
URL remains compatible with local runtime replacement.
In `@agents/nemo-studio-copilot/tests/test_nemo_studio_copilot.py`:
- Around line 187-201: Update test_sdk_prefers_agent_base_url_override to clear
NMP_WORKSPACE with monkeypatch.delenv(..., raising=False) before calling
_get_client, ensuring the assertion for workspace="default" is unaffected by
ambient environment state.
In `@services/studio/src/nmp/studio/copilot.py`:
- Around line 1692-1699: The conversation update flow after appending messages
must handle cross-replica EntityConflictError by reloading the latest
conversation, reapplying this turn, and retrying entity_store.update within the
same request so both turns are preserved. Update the relevant exception handling
around entity_store.update and the existing EntityStoreError path; do not rely
on the per-process _session_streams guard or the local 409 check for distributed
concurrency.
---
Nitpick comments:
In `@services/studio/src/nmp/studio/entities.py`:
- Around line 27-28: Bound persisted conversation history represented by the
messages field on the relevant entity, retaining only the most recent configured
turns before entity_store.update writes it. Use an existing configuration or
limit symbol if available, and preserve the current chat_artifacts behavior; do
not allow messages to grow without limit.
In `@services/studio/tests/unit/test_copilot.py`:
- Around line 310-322: Add an explicit successful status assertion to
test_create_session_persists_workspace_and_owner immediately after the
service_client.post call, before reading response.json()["session_id"], while
preserving the existing persistence and owner/message assertions.
- Around line 79-91: Update FakeEntityStore.delete to honor expected_db_version
and raise EntityConflictError when it differs from the stored entity version,
then add coverage for delete_session_history’s conflict/409 path. Add a
_stream_copilot test using an update implementation that raises EntityStoreError
and assert the streamed error response.
In `@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.ts`:
- Around line 58-59: Update the exported getClaudeCodeHistorySessionsQueryKey
function with an explicit readonly tuple return type matching its four elements:
the two string literals, the sessions literal, and the workspace string.
In
`@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/HistoryPanelContents.tsx`:
- Around line 21-26: Add an explicit JSX.Element return type to the exported
HistoryPanelContents component declaration, preserving its existing props and
implementation.
In
`@web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/HistorySessionButton.tsx`:
- Line 13: Change the React import in HistorySessionButton.tsx to a type-only
import, since React is used only for the React.JSX.Element type.
🪄 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: b3b5e61e-b6b8-4a45-a201-e1f2240ba540
📒 Files selected for processing (18)
agents/nemo-studio-copilot/Dockerfileagents/nemo-studio-copilot/src/nemo_studio_copilot/register.pyagents/nemo-studio-copilot/tests/test_nemo_studio_copilot.pyservices/studio/src/nmp/studio/copilot.pyservices/studio/src/nmp/studio/entities.pyservices/studio/src/nmp/studio/service.pyservices/studio/tests/unit/test_copilot.pyweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeHistoryPanel.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/ClaudeCodeLayout.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.test.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/api.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/ClaudeCodeChatProvider.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/HistoryPanelContents.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/HistorySessionButton.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/historyPanel/types.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.test.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/useClaudeCodeChatRuntime.ts
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Contributor
|
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
steramae-nvidia
approved these changes
Aug 5, 2026
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Signed-off-by: Danielle Ali <44468613+dmariali@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/packages/studio/src/routes/agents/CopilotChatRoute/useCopilotChatRuntime.ts (1)
64-76: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCancel stale history loads when the selected session is already active.
If session
Ais loading while sessionBremains active, navigation back toBdoes not callloadSessioninCopilotChatRoute/index.tsxbecauseB === sessionId.requestedSessionIdReftherefore remainsA, and the response forApasses the stale-request check and temporarily replacesB.Invalidate or abort the pending request when the selected session changes back to the active session. Add a regression test for
B → A → Bnavigation beforeAresolves.🤖 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 `@web/packages/studio/src/routes/agents/CopilotChatRoute/useCopilotChatRuntime.ts` around lines 64 - 76, Update useCopilotChatRuntime so changing the selected session back to the active session invalidates or aborts any pending load tracked by requestedSessionIdRef, preventing a stale response from session A from replacing active session B. Preserve loadSession behavior for genuinely new sessions, and add a regression test covering B → A → B navigation before A resolves.
🧹 Nitpick comments (4)
web/packages/studio/src/routes/agents/CopilotChatRoute/blockingInputRequest.ts (1)
29-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit return contract.
getBlockingInputRequestis exported. Define an interface for its result and use it as the return type.Proposed change
+interface BlockingInputRequestDetails { + readonly id: string; + readonly title: string; + readonly description: string; +} + -export const getBlockingInputRequest = (request: CopilotInputRequest) => { +export const getBlockingInputRequest = ( + request: CopilotInputRequest +): BlockingInputRequestDetails => {🤖 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 `@web/packages/studio/src/routes/agents/CopilotChatRoute/blockingInputRequest.ts` at line 29, Define a named interface describing the result returned by getBlockingInputRequest, then annotate the exported function’s return type with that interface. Ensure the interface captures the function’s existing return shape without changing its runtime behavior.Source: Coding guidelines
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillCard.tsx (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine a props interface and an explicit return type.
Replace the inline
{ skill: CopilotSkill }contract withSkillCardProps. Add an explicit return type toSkillCard.As per coding guidelines, prefer interfaces for object contracts and use explicit return types for public APIs.
🤖 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 `@web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillCard.tsx` at line 9, Update the SkillCard component’s inline props contract to a named SkillCardProps interface containing skill: CopilotSkill, and annotate SkillCard with an explicit JSX return type while preserving its current rendered output.Source: Coding guidelines
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx (1)
21-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit return types to exported components.
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx#L21-L26: add theHistoryPanelContentsreturn type.web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelSkeletons.tsx#L6-L20: add return types to both skeleton components.web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/FloatingPanel.tsx#L15-L15: add theFloatingPanelreturn type.web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillsPanelContents.tsx#L15-L15: add theSkillsPanelContentsreturn type.As per coding guidelines, use explicit return types for public APIs and complex functions.
🤖 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 `@web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx` around lines 21 - 26, Declare explicit React return types for the exported HistoryPanelContents component and both skeleton components in web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx:21-26 and web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelSkeletons.tsx:6-20; also add the corresponding return types to FloatingPanel in web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/FloatingPanel.tsx:15 and SkillsPanelContents in web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillsPanelContents.tsx:15.Source: Coding guidelines
web/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.ts (1)
212-212: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant trailing
\bafter the lookahead.The
\bafter(?!...)asserts at the same position as the\bfollowingagent. Remove it.♻️ Proposed cleanup
- /\b(build|create) (an? )?agent\b(?!\s+(class|component|helper|test|function|module))\b/i, + /\b(build|create) (an? )?agent\b(?!\s+(class|component|helper|test|function|module))/i,🤖 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 `@web/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.ts` at line 212, Remove the redundant trailing word-boundary assertion from the agent-matching regular expression in the navigation suggestions pattern, keeping the existing lookahead and preceding boundary 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
`@web/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.ts`:
- Around line 236-251: Update the catalog ordering or regex patterns so the
job-list entries safe-synthesizer and data-designer are selected for prompts
mentioning existing jobs. Prefer placing these entries before
safe-synthesizer-new and data-designer-new, or constrain the creation patterns
to exclude “jobs”; preserve creation routing for prompts that do not refer to
jobs.
---
Outside diff comments:
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/useCopilotChatRuntime.ts`:
- Around line 64-76: Update useCopilotChatRuntime so changing the selected
session back to the active session invalidates or aborts any pending load
tracked by requestedSessionIdRef, preventing a stale response from session A
from replacing active session B. Preserve loadSession behavior for genuinely new
sessions, and add a regression test covering B → A → B navigation before A
resolves.
---
Nitpick comments:
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/blockingInputRequest.ts`:
- Line 29: Define a named interface describing the result returned by
getBlockingInputRequest, then annotate the exported function’s return type with
that interface. Ensure the interface captures the function’s existing return
shape without changing its runtime behavior.
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx`:
- Around line 21-26: Declare explicit React return types for the exported
HistoryPanelContents component and both skeleton components in
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx:21-26
and
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelSkeletons.tsx:6-20;
also add the corresponding return types to FloatingPanel in
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/FloatingPanel.tsx:15
and SkillsPanelContents in
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillsPanelContents.tsx:15.
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillCard.tsx`:
- Line 9: Update the SkillCard component’s inline props contract to a named
SkillCardProps interface containing skill: CopilotSkill, and annotate SkillCard
with an explicit JSX return type while preserving its current rendered output.
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.ts`:
- Line 212: Remove the redundant trailing word-boundary assertion from the
agent-matching regular expression in the navigation suggestions pattern, keeping
the existing lookahead and preceding boundary 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: 4e358fe3-867f-46d9-9cbc-0a8a59488ccd
📒 Files selected for processing (73)
web/packages/studio/src/components/Layouts/GlobalNav/index.test.tsxweb/packages/studio/src/components/Layouts/GlobalNav/index.tsxweb/packages/studio/src/routes/DashboardLandingRoute/index.test.tsxweb/packages/studio/src/routes/DashboardLandingRoute/index.tsxweb/packages/studio/src/routes/DashboardLandingRoute/skillActionSuggestions.tsweb/packages/studio/src/routes/DashboardLandingRoute/skillActionTemplates.test.tsweb/packages/studio/src/routes/DashboardLandingRoute/skillDisplayName.test.tsweb/packages/studio/src/routes/DashboardLandingRoute/skillDisplayName.tsweb/packages/studio/src/routes/PageLayout/index.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/useClaudeCodeChatContext.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/types.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/BlockingInputComposer.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/BlockingInputComposer.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotChatThread.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotChatThread.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotHistoryPanel.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotHistoryPanel.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotLayout.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotStudioLink.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotStudioLink.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotStudioLinkTarget.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotToolCallPart.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotToolCallPart.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotTopBarChat.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotTopBarChat.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/JobProgressToolCall.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/JobProgressToolCall.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/activeSessionStorage.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/api.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/api.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/artifacts.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/artifacts.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/blockingInputRequest.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/context/CopilotChatProvider.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/context/CopilotChatProvider.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/context/useCopilotChatContext.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/ArtifactSections.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/CopilotArtifactsPane.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/FloatingPanel.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelSkeletons.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistorySessionButton.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillCard.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillsPanelContents.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/helpers.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/helpers.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/types.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/index.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/index.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/jobProgressConsts.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/CollapsedThinkingToolCall.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/FileChangeToolCallCard.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/SubtleToolCallRow.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/constants.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/helpers.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/types.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/toolParts.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/toolParts.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/types.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCopilotChatRuntime.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCopilotChatRuntime.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/util.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/util.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/utils/jobProgress.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/utils/jobProgress.tsweb/packages/studio/src/routes/groups/dashboardRoutes.tsxweb/packages/studio/src/util/localStorage.ts
💤 Files with no reviewable changes (2)
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/types.ts
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/useClaudeCodeChatContext.ts
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/packages/studio/src/routes/agents/CopilotChatRoute/useCopilotChatRuntime.ts (1)
64-76: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCancel stale history loads when the selected session is already active.
If session
Ais loading while sessionBremains active, navigation back toBdoes not callloadSessioninCopilotChatRoute/index.tsxbecauseB === sessionId.requestedSessionIdReftherefore remainsA, and the response forApasses the stale-request check and temporarily replacesB.Invalidate or abort the pending request when the selected session changes back to the active session. Add a regression test for
B → A → Bnavigation beforeAresolves.🤖 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 `@web/packages/studio/src/routes/agents/CopilotChatRoute/useCopilotChatRuntime.ts` around lines 64 - 76, Update useCopilotChatRuntime so changing the selected session back to the active session invalidates or aborts any pending load tracked by requestedSessionIdRef, preventing a stale response from session A from replacing active session B. Preserve loadSession behavior for genuinely new sessions, and add a regression test covering B → A → B navigation before A resolves.
🧹 Nitpick comments (4)
web/packages/studio/src/routes/agents/CopilotChatRoute/blockingInputRequest.ts (1)
29-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit return contract.
getBlockingInputRequestis exported. Define an interface for its result and use it as the return type.Proposed change
+interface BlockingInputRequestDetails { + readonly id: string; + readonly title: string; + readonly description: string; +} + -export const getBlockingInputRequest = (request: CopilotInputRequest) => { +export const getBlockingInputRequest = ( + request: CopilotInputRequest +): BlockingInputRequestDetails => {🤖 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 `@web/packages/studio/src/routes/agents/CopilotChatRoute/blockingInputRequest.ts` at line 29, Define a named interface describing the result returned by getBlockingInputRequest, then annotate the exported function’s return type with that interface. Ensure the interface captures the function’s existing return shape without changing its runtime behavior.Source: Coding guidelines
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillCard.tsx (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine a props interface and an explicit return type.
Replace the inline
{ skill: CopilotSkill }contract withSkillCardProps. Add an explicit return type toSkillCard.As per coding guidelines, prefer interfaces for object contracts and use explicit return types for public APIs.
🤖 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 `@web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillCard.tsx` at line 9, Update the SkillCard component’s inline props contract to a named SkillCardProps interface containing skill: CopilotSkill, and annotate SkillCard with an explicit JSX return type while preserving its current rendered output.Source: Coding guidelines
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx (1)
21-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit return types to exported components.
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx#L21-L26: add theHistoryPanelContentsreturn type.web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelSkeletons.tsx#L6-L20: add return types to both skeleton components.web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/FloatingPanel.tsx#L15-L15: add theFloatingPanelreturn type.web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillsPanelContents.tsx#L15-L15: add theSkillsPanelContentsreturn type.As per coding guidelines, use explicit return types for public APIs and complex functions.
🤖 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 `@web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx` around lines 21 - 26, Declare explicit React return types for the exported HistoryPanelContents component and both skeleton components in web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx:21-26 and web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelSkeletons.tsx:6-20; also add the corresponding return types to FloatingPanel in web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/FloatingPanel.tsx:15 and SkillsPanelContents in web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillsPanelContents.tsx:15.Source: Coding guidelines
web/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.ts (1)
212-212: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant trailing
\bafter the lookahead.The
\bafter(?!...)asserts at the same position as the\bfollowingagent. Remove it.♻️ Proposed cleanup
- /\b(build|create) (an? )?agent\b(?!\s+(class|component|helper|test|function|module))\b/i, + /\b(build|create) (an? )?agent\b(?!\s+(class|component|helper|test|function|module))/i,🤖 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 `@web/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.ts` at line 212, Remove the redundant trailing word-boundary assertion from the agent-matching regular expression in the navigation suggestions pattern, keeping the existing lookahead and preceding boundary 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
`@web/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.ts`:
- Around line 236-251: Update the catalog ordering or regex patterns so the
job-list entries safe-synthesizer and data-designer are selected for prompts
mentioning existing jobs. Prefer placing these entries before
safe-synthesizer-new and data-designer-new, or constrain the creation patterns
to exclude “jobs”; preserve creation routing for prompts that do not refer to
jobs.
---
Outside diff comments:
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/useCopilotChatRuntime.ts`:
- Around line 64-76: Update useCopilotChatRuntime so changing the selected
session back to the active session invalidates or aborts any pending load
tracked by requestedSessionIdRef, preventing a stale response from session A
from replacing active session B. Preserve loadSession behavior for genuinely new
sessions, and add a regression test covering B → A → B navigation before A
resolves.
---
Nitpick comments:
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/blockingInputRequest.ts`:
- Line 29: Define a named interface describing the result returned by
getBlockingInputRequest, then annotate the exported function’s return type with
that interface. Ensure the interface captures the function’s existing return
shape without changing its runtime behavior.
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx`:
- Around line 21-26: Declare explicit React return types for the exported
HistoryPanelContents component and both skeleton components in
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsx:21-26
and
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelSkeletons.tsx:6-20;
also add the corresponding return types to FloatingPanel in
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/FloatingPanel.tsx:15
and SkillsPanelContents in
web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillsPanelContents.tsx:15.
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillCard.tsx`:
- Line 9: Update the SkillCard component’s inline props contract to a named
SkillCardProps interface containing skill: CopilotSkill, and annotate SkillCard
with an explicit JSX return type while preserving its current rendered output.
In
`@web/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.ts`:
- Line 212: Remove the redundant trailing word-boundary assertion from the
agent-matching regular expression in the navigation suggestions pattern, keeping
the existing lookahead and preceding boundary 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: 4e358fe3-867f-46d9-9cbc-0a8a59488ccd
📒 Files selected for processing (73)
web/packages/studio/src/components/Layouts/GlobalNav/index.test.tsxweb/packages/studio/src/components/Layouts/GlobalNav/index.tsxweb/packages/studio/src/routes/DashboardLandingRoute/index.test.tsxweb/packages/studio/src/routes/DashboardLandingRoute/index.tsxweb/packages/studio/src/routes/DashboardLandingRoute/skillActionSuggestions.tsweb/packages/studio/src/routes/DashboardLandingRoute/skillActionTemplates.test.tsweb/packages/studio/src/routes/DashboardLandingRoute/skillDisplayName.test.tsweb/packages/studio/src/routes/DashboardLandingRoute/skillDisplayName.tsweb/packages/studio/src/routes/PageLayout/index.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/useClaudeCodeChatContext.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/types.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/BlockingInputComposer.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/BlockingInputComposer.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotChatThread.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotChatThread.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotHistoryPanel.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotHistoryPanel.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotLayout.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotStudioLink.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotStudioLink.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotStudioLinkTarget.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotToolCallPart.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotToolCallPart.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotTopBarChat.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/CopilotTopBarChat.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/JobProgressToolCall.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/JobProgressToolCall.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/activeSessionStorage.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/api.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/api.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/artifacts.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/artifacts.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/blockingInputRequest.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/context/CopilotChatProvider.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/context/CopilotChatProvider.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/context/useCopilotChatContext.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/ArtifactSections.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/CopilotArtifactsPane.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/FloatingPanel.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelContents.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistoryPanelSkeletons.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/HistorySessionButton.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillCard.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/SkillsPanelContents.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/helpers.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/helpers.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/historyPanel/types.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/index.test.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/index.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/jobProgressConsts.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/stream.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/CollapsedThinkingToolCall.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/FileChangeToolCallCard.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/SubtleToolCallRow.tsxweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/constants.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/helpers.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/toolCall/types.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/toolParts.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/toolParts.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/types.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCopilotChatRuntime.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCopilotChatRuntime.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/useCustomAssistantChatRuntime.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/util.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/util.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/utils/jobProgress.test.tsweb/packages/studio/src/routes/agents/CopilotChatRoute/utils/jobProgress.tsweb/packages/studio/src/routes/groups/dashboardRoutes.tsxweb/packages/studio/src/util/localStorage.ts
💤 Files with no reviewable changes (2)
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/types.ts
- web/packages/studio/src/routes/agents/ClaudeCodeChatRoute/context/useClaudeCodeChatContext.ts
🛑 Comments failed to post (1)
web/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.ts (1)
236-251: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Earlier catalog entries shadow these two destinations.
Matching runs in catalog order and returns the first match.
safe-synthesizer-new(line 55,/\bsafe[-\s]?synthesizer\b/i) matches "safe synthesizer jobs", sosafe-synthesizernever handles that phrase.data-designer-new(line 109,/\bdata designer\b/i) matches every string that/\bdata designer jobs?\b/imatches, sodata-designeris unreachable. Prompts asking about existing jobs route to the creation UIs.Move the job-list destinations before the creation destinations, or make the creation patterns exclude "jobs".
♻️ Example: exclude job phrasing from the creation patterns
- /\bsafe[-\s]?synthesizer\b/i, + /\bsafe[-\s]?synthesizer\b(?!\s+jobs?\b)/i,- /\bdata designer\b/i, + /\bdata designer\b(?!\s+jobs?\b)/i,🤖 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 `@web/packages/studio/src/routes/agents/CopilotChatRoute/studioUiNavigationSuggestions.ts` around lines 236 - 251, Update the catalog ordering or regex patterns so the job-list entries safe-synthesizer and data-designer are selected for prompts mentioning existing jobs. Prefer placing these entries before safe-synthesizer-new and data-designer-new, or constrain the creation patterns to exclude “jobs”; preserve creation routing for prompts that do not refer to jobs.
This was referenced Aug 5, 2026
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.
Summary by CodeRabbit