feat(frontend): add copilot tool chain UI - #13773
Conversation
|
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:
WalkthroughThe PR adds a feature-flagged Copilot tool-chain UI with specialized result cards, interactive question handling, debug playback pages, animation components, and backend provider and web-fetch metadata. ChangesCopilot tool-chain rendering
Backend metadata
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 3 conflict(s), 0 medium risk, 4 low risk (out of 7 PRs with file overlap) Auto-generated on push. Ignores: |
|
/review |
|
/review |
|
Queued a review for PR #13773 at b03d2e6. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #13773 +/- ##
==========================================
+ Coverage 77.64% 77.68% +0.04%
==========================================
Files 2859 2921 +62
Lines 217035 218626 +1591
Branches 20673 21058 +385
==========================================
+ Hits 168513 169842 +1329
- Misses 44011 44134 +123
- Partials 4511 4650 +139
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (7)
autogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/useToolUiDebugPage.ts (1)
33-62: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCancel playback on unmount and resolve the pending answer promise when playback restarts.
Two cases leave the loop running or suspended:
- The hook never cancels a run on unmount. After navigation away, the loop continues sleeping and calling
setMessages/setStatusfor the remaining script (tens of seconds).- During an
await-userpausestatusis"ready", so the play button stays enabled. A secondplay()overwritesanswerRef.current, so the first loop stays suspended on its promise forever.Add an unmount cleanup that bumps
runRefand resolves any pending answer, and resolve the previous answer promise at the start ofplay().♻️ Proposed fix
-import { useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; @@ + useEffect(() => { + return () => { + runRef.current += 1; + const resume = answerRef.current; + answerRef.current = null; + resume?.(""); + }; + }, []); + async function play() { const runId = ++runRef.current; + const pending = answerRef.current; + answerRef.current = null; + pending?.(""); setMessages([]);🤖 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 `@autogpt_platform/frontend/src/app/`(platform)/copilot/tool-ui-debug/useToolUiDebugPage.ts around lines 33 - 62, Update the useToolUiDebugPage playback lifecycle around play and its effect cleanup: resolve any existing answerRef.current before assigning a new run so a restarted play() releases the prior await-user loop, then clear the resolver and bump runRef during unmount cleanup so pending playback exits without further state updates.autogpt_platform/frontend/src/app/(platform)/copilot/test-ui/sampleTools.ts (1)
44-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffSplit the large fixture modules. Both files hold all demo fixture data in one module and exceed the ~200-line limit for files under
autogpt_platform/frontend/src. Splitting them by domain keeps each file reviewable and lets the debug pages import only the sections they render.
autogpt_platform/frontend/src/app/(platform)/copilot/test-ui/sampleTools.ts#L44-L44: move eachCATALOG_SECTIONSgroup (agents, blocks, web, files, memory, folders, schedules, docs, integrations, misc) into its own fixture file and keepSampleTool/toPartin this module.autogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/sampleScript.ts#L97-L98: splitbuildSampleEventsinto per-phase builders (research phase, building phase) in separate files and compose them here.As per coding guidelines: "Keep files under ~200 lines; extract sub-components or hooks into their own files when a file grows beyond this".
🤖 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 `@autogpt_platform/frontend/src/app/`(platform)/copilot/test-ui/sampleTools.ts at line 44, Split autogpt_platform/frontend/src/app/(platform)/copilot/test-ui/sampleTools.ts#L44-L44 by moving each CATALOG_SECTIONS domain group into its own fixture file, while keeping SampleTool and toPart in sampleTools.ts and updating imports/exports accordingly. Split autogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/sampleScript.ts#L97-L98 by moving research-phase and building-phase logic from buildSampleEvents into separate builder files, then compose those builders in sampleScript.ts so each file stays under approximately 200 lines.Source: Coding guidelines
autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ToolResult.tsx (1)
134-280: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffSplit the tool dispatch table into smaller modules.
The file is 314 lines and
toolCardalone spans about 146 lines. The coding guidelines require frontendsrc/**/*.{ts,tsx}files to stay under ~200 lines. Group the cases by domain (agents, blocks, workspace/docs, schedules, misc) into separate modules, then dispatch through a lookup map keyed byrow.tool.As per coding guidelines: "Keep files under ~200 lines; extract sub-components or hooks into their own files when a file grows beyond this".
🤖 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 `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/ToolResult.tsx around lines 134 - 280, Split the large toolCard dispatch in ToolResult into domain-specific modules for agents, blocks, workspace/docs, schedules, and miscellaneous tools, with each module exposing handlers keyed by tool name. Replace the monolithic switch with a lookup-map dispatch that passes row and output to the selected handler, preserving all existing rendering and null behavior while keeping ToolResult.tsx and extracted files under roughly 200 lines.Source: Coding guidelines
autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/__tests__/helpers.test.ts (1)
61-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd helper tests for the new result normalization.
The suite covers
buildChainSegments,toChainRow, andgetChainHeading.resultHelpers.tsandisDiffText/parseUnifiedDiffinFileDiff.tsxcarry the branch-heavy logic that drives card selection and are untested. Add cases forasObjectwith array and non-JSON strings,dictToOutputItemswith single-element arrays, and a unified diff that contains---/+++headers.🤖 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 `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/__tests__/helpers.test.ts around lines 61 - 91, Extend the test suite in helpers.test.ts to add test cases covering the untested branch-heavy logic currently missing. Create test cases for asObject from resultHelpers.ts with array and non-JSON string inputs, dictToOutputItems from resultHelpers.ts with single-element arrays, and parseUnifiedDiff or isDiffText from FileDiff.tsx with a unified diff containing --- and +++ headers. These new test blocks should validate the edge cases that drive the card selection logic but are not currently exercised by the existing toChainRow and getChainHeading tests.autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ToolChain.tsx (2)
29-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the local identifier to
panelID.Line 29 uses
Idin a symbol name. RenamepanelIdand its references topanelID.As per coding guidelines, fully capitalize acronyms in symbols.
🤖 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 `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/ToolChain.tsx at line 29, Rename the local identifier panelId returned by useId to panelID, and update every reference to it within the ToolChain component without changing its behavior.Source: Coding guidelines
51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeclare handlers before JSX.
These arrow functions handle DOM events. Declare named handlers and pass them to the event props. Keep arrow functions only for small collection callbacks.
autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ToolChain.tsx#L51-L51: declare a toggle handler.autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ChainRowView.tsx#L85-L85: declare a row-toggle handler.autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/RowIcon.tsx#L117-L117: declare an image-error handler.As per coding guidelines, use function declarations for components and handlers.
🤖 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 `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/ToolChain.tsx at line 51, Replace the inline DOM event callbacks with named function-declaration handlers: add a toggle handler in ToolChain.tsx for the expanded state, a row-toggle handler in ChainRowView.tsx, and an image-error handler in RowIcon.tsx, then pass those handlers to the corresponding event props while retaining arrow functions only for small collection callbacks. Apply the changes at ToolChain.tsx lines 51-51, ChainRowView.tsx lines 85-85, and RowIcon.tsx lines 117-117.Source: Coding guidelines
autogpt_platform/frontend/src/app/(platform)/copilot/components/PixelGridLoader/PixelGridLoader.module.css (1)
1-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Tailwind utilities for the loader animation.
The CSS module defines presentation and reduced-motion behavior outside Tailwind. Move the
pixel-onkeyframes and animation utility intoautogpt_platform/frontend/tailwind.config.ts. Replace the CSS-module import andstyles.cellwith Tailwind utilities, includingmotion-reduce:animate-none.
autogpt_platform/frontend/src/app/(platform)/copilot/components/PixelGridLoader/PixelGridLoader.module.css#L1-L21: remove the CSS-module animation rules.autogpt_platform/frontend/src/app/(platform)/copilot/components/PixelGridLoader/PixelGridLoader.tsx#L2-L2: remove the CSS-module import.autogpt_platform/frontend/src/app/(platform)/copilot/components/PixelGridLoader/PixelGridLoader.tsx#L51-L55: apply the Tailwind animation classes.As per coding guidelines, use Tailwind CSS only for styling.
🤖 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 `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/PixelGridLoader/PixelGridLoader.module.css around lines 1 - 21, Move the pixel-on keyframes and matching animation utility into autogpt_platform/frontend/tailwind.config.ts; remove the animation rules from autogpt_platform/frontend/src/app/(platform)/copilot/components/PixelGridLoader/PixelGridLoader.module.css lines 1-21. In PixelGridLoader.tsx lines 2 and 51-55, remove the CSS-module import and replace styles.cell with Tailwind animation utilities, including motion-reduce:animate-none, while preserving the existing loader behavior.Source: Coding guidelines
🤖 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 `@autogpt_platform/backend/backend/copilot/tools/helpers.py`:
- Around line 174-178: Update the provider normalization before this helper so
CredentialsFieldInfo.provider is declared as the provider-slug type, then change
the providers comprehension to use each provider value directly. Remove the
getattr-based runtime shape check while preserving the resulting provider-slug
set behavior.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ChatMessagesContainer/__tests__/ChatMessagesContainer.test.tsx:
- Around line 87-89: Add a test covering the feature-flagged chain-rendering
branch in the ChatMessagesContainer tests: enable Flag.NEW_TOOL_UI, provide a
chainable tool part so renderChainSegments uses the ToolChain renderer, and set
forceOldToolUI to preserve the legacy renderer behavior. Keep the existing
default fixture unchanged and assert the expected rendered output.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsx:
- Around line 108-140: Extract renderChainSegments and the new-tool-UI dispatch
from ChatMessagesContainer into a dedicated renderer component or module,
preserving their existing props, keys, streaming behavior, and rendering output.
Update ChatMessagesContainer to use the extracted renderer so it no longer owns
the second rendering strategy.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/ChainRowView.tsx:
- Around line 83-90: Update the row rendering around the button in ChainRowView
so rows with hasContent false are rendered as static content rather than as a
button; keep the existing expandable button behavior, onClick toggle, and
aria-expanded state for rows with content.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/FileDiff.tsx:
- Around line 21-41: Update the diff-row parsing loop to skip unified-diff file
header lines beginning with "---" or "+++", and ignore the "\ No newline at end
of file" marker before classifying rows. Keep actual additions, deletions,
context lines, and hunk-header handling unchanged in the parser responsible for
building the rows.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/helpers.ts:
- Around line 146-162: The completed-chain path in getChainHeading must check
for the latest output-error row before building CATEGORY_SUMMARY phrases. Return
that error row’s label when present, while preserving the existing streaming
running-row behavior and completed-category fallback.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/QuestionRowForm.tsx:
- Around line 15-25: Update getQuestions to filter the raw asItems results for
entries whose question and keyword fields are strings before passing them to
normalizeClarifyingQuestions; remove the unsafe array cast and preserve the
existing output/input fallback behavior.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/resultHelpers.ts:
- Around line 1-13: The asObject function has inconsistent array handling
between its JSON-string and direct-value branches. The JSON parse branch returns
arrays without rejection, while the direct-value branch explicitly excludes them
with !Array.isArray(value). This causes JSON strings like "[1,2]" to return as
arrays typed as Record<string, unknown>, breaking downstream rendering in
KeyValueList and stripBaseFields. Update the ternary operator in the JSON parse
branch to also check !Array.isArray(parsed) before returning the parsed result,
aligning both branches to reject arrays consistently.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/toolCatalog.ts:
- Around line 44-334: Split COPILOT_TOOL_CATALOG into domain-specific catalog
modules so each file stays under roughly 200 lines, and add a small entry point
that merges those modules into the existing catalog export. Move label/subject
construction helpers such as quoted and str into a separate helper module,
updating catalog entries to reuse them while preserving the current ToolMeta
behavior and public export.
- Around line 336-355: Update the error-label construction in getCatalogLabel so
it forms grammatical text with the catalog’s existing gerund labels, avoiding
combinations such as “Couldn't running block.” Preserve the running and done
branches, category, subject suffix, and null behavior unchanged.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/ToolChain.tsx:
- Around line 76-109: Update the panel containers in ToolChain.tsx around the
open state to apply inert and aria-hidden whenever open is false, while
preserving the existing close animation and mounted content. In
ChainRowView.tsx, apply the same accessibility state to the result panel based
on showContent so hidden question rows and specialized-card controls cannot
receive keyboard focus; update both listed sites accordingly.
- Around line 49-63: Update the disclosure button in the ToolChain component to
use the forced-open state `open` for both `aria-expanded` and the caret’s
rotation class, while preserving `expanded` for the click toggle behavior.
---
Nitpick comments:
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/PixelGridLoader/PixelGridLoader.module.css:
- Around line 1-21: Move the pixel-on keyframes and matching animation utility
into autogpt_platform/frontend/tailwind.config.ts; remove the animation rules
from
autogpt_platform/frontend/src/app/(platform)/copilot/components/PixelGridLoader/PixelGridLoader.module.css
lines 1-21. In PixelGridLoader.tsx lines 2 and 51-55, remove the CSS-module
import and replace styles.cell with Tailwind animation utilities, including
motion-reduce:animate-none, while preserving the existing loader behavior.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/__tests__/helpers.test.ts:
- Around line 61-91: Extend the test suite in helpers.test.ts to add test cases
covering the untested branch-heavy logic currently missing. Create test cases
for asObject from resultHelpers.ts with array and non-JSON string inputs,
dictToOutputItems from resultHelpers.ts with single-element arrays, and
parseUnifiedDiff or isDiffText from FileDiff.tsx with a unified diff containing
--- and +++ headers. These new test blocks should validate the edge cases that
drive the card selection logic but are not currently exercised by the existing
toChainRow and getChainHeading tests.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/ToolChain.tsx:
- Line 29: Rename the local identifier panelId returned by useId to panelID, and
update every reference to it within the ToolChain component without changing its
behavior.
- Line 51: Replace the inline DOM event callbacks with named
function-declaration handlers: add a toggle handler in ToolChain.tsx for the
expanded state, a row-toggle handler in ChainRowView.tsx, and an image-error
handler in RowIcon.tsx, then pass those handlers to the corresponding event
props while retaining arrow functions only for small collection callbacks. Apply
the changes at ToolChain.tsx lines 51-51, ChainRowView.tsx lines 85-85, and
RowIcon.tsx lines 117-117.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ToolChain/ToolResult.tsx:
- Around line 134-280: Split the large toolCard dispatch in ToolResult into
domain-specific modules for agents, blocks, workspace/docs, schedules, and
miscellaneous tools, with each module exposing handlers keyed by tool name.
Replace the monolithic switch with a lookup-map dispatch that passes row and
output to the selected handler, preserving all existing rendering and null
behavior while keeping ToolResult.tsx and extracted files under roughly 200
lines.
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/test-ui/sampleTools.ts:
- Line 44: Split
autogpt_platform/frontend/src/app/(platform)/copilot/test-ui/sampleTools.ts#L44-L44
by moving each CATALOG_SECTIONS domain group into its own fixture file, while
keeping SampleTool and toPart in sampleTools.ts and updating imports/exports
accordingly. Split
autogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/sampleScript.ts#L97-L98
by moving research-phase and building-phase logic from buildSampleEvents into
separate builder files, then compose those builders in sampleScript.ts so each
file stays under approximately 200 lines.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/tool-ui-debug/useToolUiDebugPage.ts:
- Around line 33-62: Update the useToolUiDebugPage playback lifecycle around
play and its effect cleanup: resolve any existing answerRef.current before
assigning a new run so a restarted play() releases the prior await-user loop,
then clear the resolver and bump runRef during unmount cleanup so pending
playback exits without further state updates.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5b370c87-1aef-49e8-9072-e60b65d0b6ca
📒 Files selected for processing (43)
autogpt_platform/backend/backend/copilot/tools/find_block.pyautogpt_platform/backend/backend/copilot/tools/helpers.pyautogpt_platform/backend/backend/copilot/tools/helpers_test.pyautogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/web_fetch.pyautogpt_platform/backend/backend/copilot/tools/web_fetch_test.pyautogpt_platform/frontend/next.config.mjsautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/__tests__/ChatMessagesContainer.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/components/ThinkingIndicator.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/helpers.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/PixelGridLoader/PixelGridLoader.module.cssautogpt_platform/frontend/src/app/(platform)/copilot/components/PixelGridLoader/PixelGridLoader.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/AgentCards.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/BlockCards.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ChainRowView.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ExecutionCard.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/FileDiff.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/InfoCards.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ListCards.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/QuestionRowForm.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ResultCards.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/RowIcon.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ShimmerText.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/SwapText.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ToolChain.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ToolResult.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ToolResultViews.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/accordion.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/helpers.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/resultHelpers.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/toolCatalog.tsautogpt_platform/frontend/src/app/(platform)/copilot/test-ui/page.tsxautogpt_platform/frontend/src/app/(platform)/copilot/test-ui/sampleTools.tsautogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/components/NewChatView.tsxautogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/components/StreamingText.tsxautogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/helpers.tsautogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/page.tsxautogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/sampleScript.tsautogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/useToolUiDebugPage.tsautogpt_platform/frontend/src/services/feature-flags/use-get-flag.tsautogpt_platform/frontend/tailwind.config.ts
There was a problem hiding this comment.
📋 Automated Review — PR #13773
PR #13773 — feat(frontend): add copilot tool chain UI
Author: Abhi1992002 | Files: 44
🎯 Verdict: REQUEST_CHANGES
PR Description Quality
✅ Has Why + What + How — the description explains the feature-flagged tool-chain renderer, backend metadata additions, and debug pages. continue_run_block_test — which is exactly the suite that catches the regression below.
What This PR Does
Adds a new, feature-flagged (NEW_TOOL_UI) Copilot renderer that displays tool calls as an animated "tool chain" — status rows, provider icons, result cards, file diffs, and interactive question forms — while preserving the legacy renderer as a fallback toggle. On the backend it adds optional response metadata (provider on block summaries; title, content_length, truncated on web_fetch) and two internal debug/comparison pages. The truncated change (len(content) > _MAX_CONTENT_BYTES instead of hardcoded False) is a genuine correctness fix.
Specialist Findings
🛡️ Security ✅ — No critical/high issues. web_fetch SSRF protection is unchanged, no dangerouslySetInnerHTML, no new auth/DB/credential surface. Main concerns are a third-party favicon privacy leak and an unsanitized provider-slug interpolated into an asset path.
🟠 Google-favicon leak sends every result hostname to www.google.com/s2/favicons from the user's browser (ResultCards.tsx:23).
🏗️ Architecture get_block_provider uses getattr(...) for type dispatch against the backend "no duck-typing" rule, provider-slug logic is duplicated, and debug routes ship unguarded.
🟠 getattr(provider, "value", str(provider)) dispatch and an unguarded comprehension in helpers.py:174-181 (root cause of the blocker below).
⚡ Performance find_block.py:265), unmemoized full-chain recompute on every streamed token (ToolChain.tsx:40, O(n²)/turn), and unbounded/undeduped favicon fan-out.
🧪 Testing web_fetch_test.py, helpers_test.py), but the headline ~40-file frontend feature ships with one thin helper test. The core renderChainSegments branch, QuestionRowForm, the ~40-way toolCard dispatch, and FileDiff parsing are all uncovered; the container test actively mocks the new path out of existence.
📖 Quality ✅ — Readable and well-organized (score B). Divergences from documented conventions: props declared inline instead of interface Props, className string-concat instead of cn(), and hardcoded hex/rgba instead of design tokens (FileDiff.tsx:52). All quick cleanups, none blocking.
📦 Product
📬 Discussion test (3.11/3.12/3.13) all fail on a PR-introduced regression, codecov/patch fails, review decision is REVIEW_REQUIRED with zero human reviews, and 14+ bot findings (Sentry/CodeRabbit) have no author response.
🔎 QA ✅ — Live end-to-end validation passed: new tool chain renders on debug pages and real /copilot, expand/collapse works, legacy toggle works, and web_fetch returned title="Example Domain", content_length=168, truncated=false live. Backend unit subset 6 passed. Caveat: find_block's new provider field couldn't be exercised (empty search index in the env) — note this QA run did not execute continue_run_block_test, so it did not surface the CI regression.
🔴 Blockers
get_block_providerunguarded.values()crashes block execution (backend/backend/copilot/tools/helpers.py:176) — The provider-collection comprehension (infos.values()/info.provider) sits outside thetry/exceptthat only wrapsget_credentials_fields_info(). Wheninfosis a list rather than a dict,infos.values()raisesAttributeError, which propagates intoexecute_blockand converts a successful block run into anErrorResponse. GitHub CI confirms this:continue_run_block_test::test_approved_review_executes_blockfails identically on Python 3.11, 3.12, and 3.13 ('list' object has no attribute 'values'). Move the whole providers computation inside thetry(returnNoneon failure), replace thegetattrdispatch with a typed accessor, and add a test exercisingexecute_blockwith the block type used bycontinue_run_block_test. (Flagged by: discussion — CI-confirmed; root cause also noted by architect)
🟠 Should Fix
- Debug routes ship to production ungated (
copilot/test-ui/page.tsx:1,copilot/tool-ui-debug/page.tsx:1) — Both sit in the(platform)route group with noNODE_ENV/flag/notFound()guard, so any authenticated user can reach them. Gate behind a dev/flag check or move out of the shipped tree. (Flagged by: architect, quality, product, security — 4 specialists) - Favicon fetch leaks result domains to Google (
ToolChain/ResultCards.tsx:23, host allowlisted innext.config.mjs:84) — Every surfaced hostname is sent towww.google.com/s2/faviconsfrom the user's browser, disclosing query-derived activity to a third party. Proxy through the backend or bundle icons. (Flagged by: security, architect, performance, product — 4 specialists) content_lengthis character count, not bytes, but rendered as KB (web_fetch.py:160) — Set from the original byte length (len(response.content)) or rename the field and adjust the frontend display. (Flagged by: discussion/Sentry)isDiffTextruns before object parsing (ToolResult.tsx:293,FileDiff.tsx) — A tool's stringified JSON output containing+/-lines is misclassified and rendered as a file diff. Parse structured output first, or require a real@@hunk header. (Flagged by: discussion/Sentry, testing)- Thinking indicator no longer announced to screen readers (
ThinkingIndicator.tsx:31) — The loader isaria-hiddenand the text is optional; whenstatusMessageis absent, assistive tech gets nothing. Add ansr-onlyrole="status""Thinking…" fallback. (Flagged by: product) - Unsanitized provider slug in asset path (
ToolChain/helpers.ts:30,BlockCards.tsx:24) — The transform collapses whitespace/dashes but not/,.,..; a model-influenced provider value can resolve to an unintended same-origin path. Whitelist[a-z0-9_]. (Flagged by: security, architect) - Core feature is effectively untested (
ChatMessagesContainer.test.tsx:88,QuestionRowForm.tsx:65,ToolResult.tsx:134) — Add coverage for theNEW_TOOL_UI-on rendering branch, the interactive question form (submit gating +onSendmessage), and representativetoolCarddispatch cases. Test gaps do not by themselves drive this verdict (the blocker does), but they should land in this PR. (Flagged by: testing) ToolChainrebuilds the whole chain per streamed token (ToolChain.tsx:40) — Memoizerowskeyed onparts; this is the repo's sanctioneduseMemoperformance carve-out. (Flagged by: performance)NEW_TOOL_UIdefaults totrue(use-get-flag.ts:65) — A net-new renderer fails open on a LaunchDarkly outage/missing key, undercutting the "legacy preserved as fallback" intent. Confirm rollout intent or default tofalse. (Flagged by: architect, product)
🟡 Nice to Have
- Cache provider slug per block type (
find_block.py:265) — Memoize introspection so it runs once per block class, not per response row. (performance) FileDiffsingle-pass counts + row cap/virtualization (FileDiff.tsx:48) — Deferred behind a collapsed accordion, so low priority. (performance)- Extract shared
integrationIconSrc()andsafeHostname()helpers — Kills the duplicated slug logic and repeatedtry { new URL(url).hostname } catch {}. (architect, quality) getChainHeadinglabels an errored chain "completed" (ToolChain/helpers.ts:162) — Surface a failed heading when any row is inerrorstate. (discussion/CodeRabbit)
🔵 Nits
- Props declared inline instead of
interface Props(AgentCards.tsx:28and siblings) — inconsistent even within this PR. (quality) classNamestring-concat instead ofcn()(ChainRowView.tsx:40) — (quality)- Hardcoded hex/rgba instead of design tokens (
FileDiff.tsx:52-59) — breaks dark-mode/theming. (quality) ---/+++diff headers parsed as content rows (FileDiff.tsx:41) — skip file-header lines before classifying. (discussion/CodeRabbit)- Collapsed accordion descendants stay focusable (
ToolChain.tsx:109) — gate withhidden/inertwhen closed. (discussion/CodeRabbit)
QA Screenshots
Human Review Needed
NO — This is frontend UI plus additive, backward-compatible backend metadata; no authentication, credential-handling, or trust-boundary changes (web_fetch SSRF protection is untouched). The blocker is a mechanical exception-scoping regression that CI already pinpoints, not a security-boundary concern.
Risk Assessment
Merge risk: MEDIUM | Rollback: EASY — The renderer is feature-flagged and additive; the one hard risk is the block-execution regression, which is a one-scope fix and blocks CI today. Rollback is a flag flip / small revert.
CI Status
GitHub CI: FAILING — test (3.11), test (3.12), test (3.13) all fail on the PR-introduced AttributeError in helpers.py:176; codecov/patch/platform-frontend fails on insufficient patch coverage; mergeable: MERGEABLE (no conflicts); REVIEW_REQUIRED with 0 human reviews.
Local harness (review sandbox, not CI): 4/5 pass — frontend lint ✅, backend lint ✅, frontend typecheck ✅, frontend build ✅; frontend test:unit ❌ (backend test suite not run in-harness). Both the harness and GitHub agree the test suites are red.
UI Testing — Variant Results
✅ local: Feature-flagged copilot tool-chain UI renders correctly on debug pages and real /copilot, legacy renderer still works, and web_fetch metadata verified live; only unverified item is find_block provider due to an empty search index in this env.
✅ hosted: New copilot tool-chain UI renders and behaves correctly across all surfaces (chains, result cards, interactive question form, live streaming, legacy fallback) and backend metadata tests pass; only note is ungated debug routes.
- low: The tool-ui-debug developer comparison harness is shipped as a real production route reachable by any authenticated non-admin user (verified by loading it as a freshly signed-up regular user with no flag/admin gate).
- low: The test-ui sample-catalog page is a production route accessible to any authenticated user with no gating; it is an internal QA/preview surface.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
autogpt_platform/backend/backend/copilot/tools/helpers.py (2)
190-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the public entry point before its private helper.
Line 190 defines
get_block_providerafter_get_input_schema_providerat Line 169. Move the public function above the private helper.As per coding guidelines, “Use top-down ordering — define the main/public function or class first, then the helpers it uses below.”
Proposed change
+def get_block_provider(block: AnyBlockSchema) -> str | None: + """Sole integration provider slug for a block, or None when the block + uses zero or multiple providers.""" + return _get_input_schema_provider(block.input_schema) + `@lru_cache`(maxsize=None) def _get_input_schema_provider(input_schema: type[BlockSchemaInput]) -> str | None: ... - -def get_block_provider(block: AnyBlockSchema) -> str | None: - """Sole integration provider slug for a block, or None when the block - uses zero or multiple providers.""" - return _get_input_schema_provider(block.input_schema)🤖 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 `@autogpt_platform/backend/backend/copilot/tools/helpers.py` around lines 190 - 193, Reorder the definitions in helpers.py so the public get_block_provider function appears before the private _get_input_schema_provider helper it calls. Keep both implementations and behavior unchanged.Source: Coding guidelines
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
cachefor the unboundedfunctoolscache.
@lru_cache(maxsize=None)should be@cache.Proposed change
-from functools import lru_cache +from functools import cache ... -@lru_cache(maxsize=None) +@cache🤖 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 `@autogpt_platform/backend/backend/copilot/tools/helpers.py` at line 9, Replace the unbounded functools cache usage with functools.cache: import cache instead of lru_cache and update the affected decorator to use `@cache`, preserving the existing caching behavior.Source: Linters/SAST tools
🤖 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 `@autogpt_platform/backend/backend/copilot/tools/helpers.py`:
- Around line 190-193: Reorder the definitions in helpers.py so the public
get_block_provider function appears before the private
_get_input_schema_provider helper it calls. Keep both implementations and
behavior unchanged.
- Line 9: Replace the unbounded functools cache usage with functools.cache:
import cache instead of lru_cache and update the affected decorator to use
`@cache`, preserving the existing caching behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 20631085-def4-4132-9dec-6dca4a7bbdeb
📒 Files selected for processing (5)
autogpt_platform/backend/backend/copilot/tools/helpers.pyautogpt_platform/backend/backend/copilot/tools/helpers_test.pyautogpt_platform/backend/backend/copilot/tools/models.pyautogpt_platform/backend/backend/copilot/tools/web_fetch.pyautogpt_platform/backend/backend/copilot/tools/web_fetch_test.py
🚧 Files skipped from review as they are similar to previous changes (4)
- autogpt_platform/backend/backend/copilot/tools/models.py
- autogpt_platform/backend/backend/copilot/tools/web_fetch.py
- autogpt_platform/backend/backend/copilot/tools/web_fetch_test.py
- autogpt_platform/backend/backend/copilot/tools/helpers_test.py
📜 Review details
⏰ Context from checks skipped due to timeout. (18)
- GitHub Check: integration_test
- GitHub Check: lint
- GitHub Check: check API types
- GitHub Check: Seer Code Review
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (typescript)
- GitHub Check: check-overlaps
- GitHub Check: types
- GitHub Check: lint
- GitHub Check: end-to-end tests
- GitHub Check: Check PR Status
- GitHub Check: type-check (3.13)
- GitHub Check: test (3.12)
- GitHub Check: type-check (3.12)
- GitHub Check: test (3.13)
- GitHub Check: test (3.11)
- GitHub Check: lint
- GitHub Check: type-check (3.11)
🧰 Additional context used
📓 Path-based instructions (2)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
🧠 Learnings (18)
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-03-04T08:04:35.881Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12273
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:216-220
Timestamp: 2026-03-04T08:04:35.881Z
Learning: In the AutoGPT Copilot backend, ensure that SVG images are not treated as vision image types by excluding 'image/svg+xml' from INLINEABLE_MIME_TYPES and MULTIMODAL_TYPES in tool_adapter.py; the Claude API supports PNG, JPEG, GIF, and WebP for vision. SVGs (XML text) should be handled via the text path instead, not the vision path.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-04-01T04:17:41.600Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12632
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-01T04:17:41.600Z
Learning: When reviewing AutoGPT Copilot tool implementations, accept that `readOnlyHint=True` (provided via `ToolAnnotations`) may be applied unconditionally to *all* tools—even tools that have side effects (e.g., `bash_exec`, `write_workspace_file`, or other write/save operations). Do **not** flag these tools for having `readOnlyHint=True`; this is intentional to enable fully-parallel dispatch by the Anthropic SDK/CLI and has been E2E validated. Only flag `readOnlyHint` issues if they conflict with the established `ToolAnnotations` behavior (e.g., missing/incorrect propagation relative to the intended annotation mechanism).
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-06-06T12:22:37.648Z
Learnt from: anvyle
Repo: Significant-Gravitas/AutoGPT PR: 13302
File: autogpt_platform/backend/backend/copilot/tools/agent_search.py:579-583
Timestamp: 2026-06-06T12:22:37.648Z
Learning: When writing LLM-facing instruction strings that trigger tool calls in this AutoGPT codebase, use the exact registered tool name `view_agent_output` (as defined in `backend/copilot/tools/agent_output.py` via its `name` property and exported via `TOOL_REGISTRY`). Do not reference the bare name `agent_output`, since it is not a valid tool name and will cause tool invocation to fail.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-03-04T12:19:39.243Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12279
File: autogpt_platform/backend/backend/copilot/tools/base.py:184-188
Timestamp: 2026-03-04T12:19:39.243Z
Learning: In autogpt_platform/backend/backend/copilot/tools/, ensure that anonymous users always pass user_id=None to tool execution methods. The anon_ prefix (e.g., anon_123) is used only for PostHog/analytics distinct_id and must not be used as an actual user_id. Use a simple truthiness check on user_id (e.g., if user_id: ... else: ... or a dedicated is_authenticated flag) to distinguish anonymous from authenticated users, and review all tool execution call sites within this directory to prevent accidentally forwarding an anon_ user_id to tools.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-03-31T14:22:26.566Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12622
File: autogpt_platform/backend/backend/copilot/tools/agent_search.py:223-236
Timestamp: 2026-03-31T14:22:26.566Z
Learning: In files under autogpt_platform/backend/backend/copilot/tools/, ensure agent graph enrichment uses the typed Pydantic model `backend.data.graph.Graph` for `AgentInfo.graph` (i.e., `Graph | None`), not `dict[str, Any]`. When enriching with graph data (e.g., `_enrich_agents_with_graph`), prefer calling `graph_db().get_graph(graph_id, version=None, user_id=user_id)` directly to retrieve the typed `Graph` object rather than routing through JSON conversions like `get_agent_as_json()` / `graph_to_json()`.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-03-15T15:30:02.282Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12385
File: autogpt_platform/backend/backend/copilot/tools/helpers.py:149-185
Timestamp: 2026-03-15T15:30:02.282Z
Learning: In autogpt_platform/backend/backend/copilot/tools/helpers.py, inside execute_block: when InsufficientBalanceError occurs after post-execution credit charging (i.e., balance drained concurrently after pre-check passed), treat as a non-fatal billing leak. Log at ERROR level with structured JSON: {"billing_leak": True, "user_id": ..., "cost": ...} for monitoring/alerting, then return BlockOutputResponse normally (do not discard the output). Do not perform a second get_user_credit_model call; reuse the credit_model obtained during the pre-execution balance check (guarded by if cost > 0 and credit_model:). This guidance improves UX by not discarding results and provides observable billing leak signals.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-05-23T05:29:43.085Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13200
File: autogpt_platform/backend/backend/executor/scheduler.py:590-593
Timestamp: 2026-05-23T05:29:43.085Z
Learning: When reviewing Python code that uses Pydantic discriminated/tagged unions (e.g., `Annotated[Union[...], Field(discriminator="kind")]`), recognize that using `isinstance(x, SomeVariantInfo)` to narrow the union is an intentional and correct runtime guard and should also enable static type narrowing in tools like Pyright. Do not recommend replacing such `isinstance`-based narrowing with `cast(...)` when the check already proves the variant at runtime.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-04-22T11:46:04.431Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/config.py:0-0
Timestamp: 2026-04-22T11:46:04.431Z
Learning: Do not flag the Claude Sonnet 4.6 model ID as incorrect when it uses the project’s established hyphenated convention: `anthropic/claude-sonnet-4-6`. This hyphen form is the intentional, production convention and should be treated as valid (including in files like llm.py, blocks tests, reasoning.py, `_is_anthropic_model` tests, and config defaults). Note that OpenRouter also accepts the dot variant `anthropic/claude-sonnet-4.6`, so either form may be tolerated, but `anthropic/claude-sonnet-4-6` should be considered the standard to match project usage.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-04-22T11:46:12.892Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/baseline/service.py:322-332
Timestamp: 2026-04-22T11:46:12.892Z
Learning: In this codebase (Significant-Gravitas/AutoGPT), OpenRouter-routed Anthropic model IDs should use the hyphen-separated convention (e.g., `anthropic/claude-sonnet-4-6`, `anthropic/claude-opus-4-6`). Although OpenRouter may accept both hyphen and dot variants, treat the hyphen-separated form as the intended, correct codebase-wide convention and do not flag it as an error. Only flag the dot-separated variant (e.g., `anthropic/claude-sonnet-4.6`) as incorrect when reviewing/validating model ID strings for OpenRouter-routed Anthropic models.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-05-07T18:48:14.242Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13040
File: autogpt_platform/backend/backend/blocks/llm.py:0-0
Timestamp: 2026-05-07T18:48:14.242Z
Learning: In this repository, isort may split imports from the same module into separate blocks when some imports are aliased (e.g., `from module import X as Y`) and others are not. Preserve the two-block layout when it results from isort (such as keeping `from openai.types.chat import ChatCompletion as OpenAIChatCompletion` separate from non-aliased imports from `openai.types.chat`). Do not treat that split as a style issue during review; merging them into a single block can fail CI with `Imports are incorrectly sorted and/or formatted`.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-05-26T14:24:34.866Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 13217
File: autogpt_platform/backend/backend/api/features/search/service.py:137-137
Timestamp: 2026-05-26T14:24:34.866Z
Learning: In the Significant-Gravitas/AutoGPT backend, treat `user_id` (an opaque UUID used only for correlation/tracing) as non-PII. Do not flag direct logging of `user_id` in `logger.warning`/`logger.info` statements as a PII exposure issue, as the established convention is to log `user_id` for tracing while reserving PII for fields like email or display name.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-06-11T19:39:10.493Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 13337
File: autogpt_platform/backend/backend/copilot/graphiti/reranker.py:0-0
Timestamp: 2026-06-11T19:39:10.493Z
Learning: In the Significant-Gravitas/AutoGPT Python backend, when calling the OpenAI Python client `chat.completions.create`, construct the `messages` payload using the concrete typed-dict variants from `openai.types.chat` (e.g., `ChatCompletionSystemMessageParam`, `ChatCompletionUserMessageParam`, etc.) rather than trying to instantiate `ChatCompletionMessageParam` directly. `ChatCompletionMessageParam` is a `Union` alias and is not constructible, so `ChatCompletionMessageParam(role=..., content=...)` should fail type checking. Build each message element with the appropriate concrete typed dict and then annotate the resulting list as `list[ChatCompletionMessageParam]` (e.g., `messages: list[ChatCompletionMessageParam] = [ChatCompletionSystemMessageParam(...), ...]`).
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
📚 Learning: 2026-06-17T12:07:57.197Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 13373
File: autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py:395-399
Timestamp: 2026-06-17T12:07:57.197Z
Learning: In autogpt_platform/backend/backend/copilot/tools/run_mcp_tool.py (and the sibling helper in copilot/tools/helpers.py), when handling FileRefExpansionError, ensure the exception text is passed through verbatim into ErrorResponse.message. The referenced “path” originates from the model’s own `@agptfile`: input token (not server-side directory structure) and is intentionally echoed back so the model can self-correct and retry. During review, do NOT flag this as a directory-structure/path-leak issue and do NOT recommend sanitizing it (e.g., adding os.path.basename) because that would break the self-correction loop. Only treat it as a leak when the message is actually derived from server filesystem paths rather than the model-provided `@agptfile` content.
Applied to files:
autogpt_platform/backend/backend/copilot/tools/helpers.py
🪛 Ruff (0.16.1)
autogpt_platform/backend/backend/copilot/tools/helpers.py
[warning] 169-169: Use @functools.cache instead of @functools.lru_cache(maxsize=None)
Rewrite with `@functools.cache
(UP033)
🔇 Additional comments (2)
autogpt_platform/backend/backend/copilot/tools/helpers.py (2)
10-15: LGTM!Also applies to: 36-36
170-187: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
autogpt_platform/frontend/src/app/(platform)/copilot/test-ui/page.tsx (1)
4-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for both production guards.
A future change can expose these internal routes again. Add Vitest and RTL tests that assert
notFound()runs in production and the page component renders outside production. Run the tests withpnpm test:unit.
autogpt_platform/frontend/src/app/(platform)/copilot/test-ui/page.tsx#L4-L6: add an adjacent page test for theTestUiPageroute guard.autogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/page.tsx#L4-L6: add an adjacent page test for theToolUiDebugPageroute guard.As per coding guidelines, write page tests in
__tests__/next topage.tsxwith Vitest and RTL for new pages and features.🤖 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 `@autogpt_platform/frontend/src/app/`(platform)/copilot/test-ui/page.tsx around lines 4 - 6, Add adjacent Vitest and RTL page tests under __tests__/ for both TestUiPage and ToolUiDebugPage, covering production behavior where notFound() is called and non-production behavior where the page component renders. Apply the tests to both specified page files and verify them with pnpm test:unit.Source: Coding guidelines
autogpt_platform/frontend/src/app/(platform)/copilot/test-ui/TestUiPage.tsx (1)
24-188: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit the test catalog into focused components.
TestUiPagecontains about 96 lines of render logic. The local components also use inline prop object types. Extract the catalog sections into focused components, then define a non-exportedinterface Propsin each component module.As per coding guidelines, use non-exported component prop interfaces and keep render functions under about 50 lines.
🤖 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 `@autogpt_platform/frontend/src/app/`(platform)/copilot/test-ui/TestUiPage.tsx around lines 24 - 188, Split the render sections in TestUiPage into focused non-exported components, keeping TestUiPage and each render function under about 50 lines. Move each extracted component into its own module as appropriate, replacing inline prop object types with a non-exported interface Props, and preserve the existing catalog, raw-data toggle, and sent-message behavior.Source: Coding guidelines
autogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/ToolUiDebugPage.tsx (1)
10-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine a named props interface.
VariantToggledeclares its props with an inline object type. Define a non-exportedinterface Propsbefore the component and use it for the parameter.Proposed change
+interface Props { + variant: ToolUiVariant; + onChange: (variant: ToolUiVariant) => void; +} + function VariantToggle({ variant, onChange, -}: { - variant: ToolUiVariant; - onChange: (variant: ToolUiVariant) => void; -}) { +}: Props) {As per coding guidelines, component props should use a non-exported
interface Props.🤖 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 `@autogpt_platform/frontend/src/app/`(platform)/copilot/tool-ui-debug/ToolUiDebugPage.tsx around lines 10 - 16, Define a non-exported interface Props before VariantToggle containing the variant and onChange fields, then replace the component’s inline props object type with Props while preserving the existing field types and behavior.Source: Coding guidelines
🤖 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
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ChatMessagesContainer/components/ChainMessageParts.tsx:
- Around line 33-35: Update the isStreaming calculation in ChainMessageParts so
it compares segmentIndex with the index of the final segment whose kind is
"chain", rather than segments.length - 1. Preserve the isCurrentlyStreaming
condition so only the active final chain segment is marked streaming.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/tool-ui-debug/ToolUiDebugPage.tsx:
- Around line 20-32: Add aria-pressed={variant === option} to each toggle button
in the variant selector so assistive technologies can identify the active
renderer, while preserving the existing onClick and styling behavior.
---
Nitpick comments:
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/test-ui/page.tsx:
- Around line 4-6: Add adjacent Vitest and RTL page tests under __tests__/ for
both TestUiPage and ToolUiDebugPage, covering production behavior where
notFound() is called and non-production behavior where the page component
renders. Apply the tests to both specified page files and verify them with pnpm
test:unit.
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/test-ui/TestUiPage.tsx:
- Around line 24-188: Split the render sections in TestUiPage into focused
non-exported components, keeping TestUiPage and each render function under about
50 lines. Move each extracted component into its own module as appropriate,
replacing inline prop object types with a non-exported interface Props, and
preserve the existing catalog, raw-data toggle, and sent-message behavior.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/tool-ui-debug/ToolUiDebugPage.tsx:
- Around line 10-16: Define a non-exported interface Props before VariantToggle
containing the variant and onChange fields, then replace the component’s inline
props object type with Props while preserving the existing field types and
behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ca7500ca-e97b-47a2-b42f-89fc8ca86e70
📒 Files selected for processing (35)
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/__tests__/ChatMessagesContainer.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/components/ChainMessageParts.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/components/ThinkingIndicator.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/AgentCards.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/BlockCards.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ChainRowView.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ExecutionCard.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/FileDiff.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/InfoCards.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ListCards.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/QuestionRowForm.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ResultCards.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/RowIcon.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ShimmerText.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/SwapText.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ToolChain.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ToolResult.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ToolResultViews.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/__tests__/FileDiff.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/__tests__/QuestionRowForm.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/__tests__/ToolResult.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/__tests__/resultHelpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/fileDiffHelpers.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/helpers.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/resultHelpers.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/toolCatalog.agent.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/toolCatalog.platform.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/toolCatalog.shared.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/toolCatalog.tsautogpt_platform/frontend/src/app/(platform)/copilot/test-ui/TestUiPage.tsxautogpt_platform/frontend/src/app/(platform)/copilot/test-ui/page.tsxautogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/ToolUiDebugPage.tsxautogpt_platform/frontend/src/app/(platform)/copilot/tool-ui-debug/page.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/components/ThinkingIndicator.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/BlockCards.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/SwapText.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ShimmerText.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/RowIcon.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ChainRowView.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ExecutionCard.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ListCards.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ToolChain.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/ToolResultViews.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/helpers.ts
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/QuestionRowForm.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ToolChain/InfoCards.tsx
|
/review |
|
/review |
|
!deploy |
|
🚀 Deploying PR #13773 to development environment... |
|
🔄 Auto-redeploying: new commits pushed to a PR with an active deployment. Refreshing development environment for PR #13773. |
…n connectors table - Proceed/Add never auto-sends: drafts the combined reply of ready cards into the chat input via setInitialPrompt; unready cards no longer block - Questions card is a per-question stepper with ring-dot pager, chevrons, animated step swap and a round advance/send action button; Skip in header - run_mcp_tool setup rows lift into the chain action card: hidden MCPSetupCard registers an mcp entry and McpConnectorRow renders its OAuth/manual-token flow inside the connectors table (deduped by server) - connectors-only cards render no Proceed: connecting is the whole ask
|
🔄 Auto-redeploying: new commits pushed to a PR with an active deployment. Refreshing development environment for PR #13773. |
- MCPSetupCard: wrap handleManualToken in a callback so onClick typechecks - autopilot_permissions_test: patch DISABLED_LEGACY_TOOL_NAMES instead of relying on ask_question, which is a first-class tool again - tool_schema_test: raise char budget for the restored ask_question schema - helpers: top-down ordering, functools.cache over lru_cache(maxsize=None) - StreamingText: blink the caret while streaming, not after - ToolChain: aria-expanded follows the panel's real visibility - ToolResultViews: fetch favicons from the result's own origin instead of Google, with no-referrer
|
🔄 Auto-redeploying: new commits pushed to a PR with an active deployment. Refreshing development environment for PR #13773. |
|
Addressed the review — status of each item as of bbb3cca: Blocker
Should Fix
Nice to have / nits
CI |
|
🔄 Auto-redeploying: new commits pushed to a PR with an active deployment. Refreshing development environment for PR #13773. |
Proceed only drafts into the composer, so the onSent callbacks registered by SetupRequirementsCard and QuestionDock never ran — the setup card never collapsed to "Connected. Continuing…" and the question dock was never dismissed. The store now counts real sends; ToolChain records which cards it drafted and fires their onSent on the next send.
|
🔄 Auto-redeploying: new commits pushed to a PR with an active deployment. Refreshing development environment for PR #13773. |
|
/review |
There was a problem hiding this comment.
📋 Automated Review — PR #13773
PR #13773 — feat(frontend): add copilot tool chain UI
Author: Abhi1992002 | Files: 113
🎯 Verdict: APPROVE
PR Description Quality
✅ Has Why + What + How — the description explains the feature-flagged unified tool-chain renderer, backend metadata additions, and preserved legacy path. The checklist is filled and CI-relevant items are checked.
What This PR Does
Adds a new, feature-flagged (NEW_TOOL_UI) copilot "tool chain" UI that renders assistant tool activity as a unified, provider-aware chain with animated status narration, specialized result cards, and a clarifying-question dock — while keeping the legacy renderer fully intact behind a forceOldToolUI escape hatch. On the backend it makes small additive changes: extra streaming status chunks ("Message received…" → "Setting up your environment…"), web-fetch page metadata (title/length/truncated), a cached block-provider slug lookup, and re-enables the ask_question tool. Debug/sample pages are production-guarded via notFound().
Specialist Findings
🛡️ Security ✅ — No injection, auth, or secrets exposure introduced. Tool output renders through JSX text nodes (React auto-escaping), SSRF protection in web_fetch is retained, and sdk/service.py adds real hardening (strict-mcp-config, empty setting sources). 🔵 One nit: MCP token input lacks autoComplete="off" (McpConnectorRow.tsx:73).
🏗️ Architecture ✅/ChatContainer.tsx:175) and 🟠 stray root-level screenshots. Notes standing coupling debt between the frontend catalog mirror and backend TOOL_REGISTRY.
⚡ Performance ✅/@cache-backed provider lookup). 🟠 Two client re-render amplifiers on the streaming hot path: ToolChain subscribes to the whole Zustand store with no selector (ToolChain.tsx:55), and the unstable PendingQuestionsContext value (ChatContainer.tsx:175).
🧪 Testing ✅/ChainActionCard/ subtree (helpers + stepper) has no tests; web_fetch truncation test over-promises relative to its assertions.
📖 Quality ✅/eslint-disable react-hooks/exhaustive-deps (ConnectorRow.tsx:41) against the explicit "no linter suppressors" repo rule, masking a stale-deps risk. 🔵 Minor duplicated string.
📦 Product ✅ — Behavior matches the description; legacy renderer preserved, debug pages prod-guarded, good accessibility (aria-live, sr-only, keyboard). 🔵 Minor UX polish: wordless silent-gap state, hostname-derived MCP icon, hardcoded text-purple-700.
📬 Discussion ✅/autogpt-pr-reviewer CHANGES_REQUESTED (its blocker since fixed + regression test added) still gates reviewDecision and should be dismissed/re-run. Zero human reviews yet (Pwuts, Swiftyos requested).
🔎 QA ✅ — Drove the real copilot flow end-to-end: web_fetch extracted "Example Domain" title and rendered the new collapsed→expanded chain card; find_block chain rendered; backend PR tests 10/10 pass; ask_question confirmed re-registered; prod-guarded debug page correctly 404s; negative cases returned 401/401/422. No functional defects found.
🟠 Should Fix
- Unmemoized
PendingQuestionsContextvalue (ChatContainer.tsx:175) —getPendingQuestions(messages)builds a fresh object each render, invalidating allToolChainconsumers on every streaming tick/keystroke while a question card is open. Memoize keyed on messages. (Flagged by: architect, performance — 2 specialists) ToolChainsubscribes to the whole Zustand store (ToolChain.tsx:55) — no selector means every store mutation (incl. per-mousemovepanel resize) re-renders every chain in the transcript. UseuseCopilotUIStore((s) => …)selectors. (performance)eslint-disableviolates explicit repo rule (ConnectorRow.tsx:41) — AGENTS.md forbids suppressors; the effect readssavedCredential.provider/type/titlebut depends only on?.id, so a credential mutated without an id change applies stale values. Fix the deps honestly. (quality)ChainActionCard/subtree has zero tests (ChainActionCard/helpers.ts:78,33) —toConnectorRows(dedup/drop/fan-out) andformatInputsTitle(regex name mangling) are cheap, high-value pure functions with no coverage; stepper state machine also untested. Addhelpers.test.ts+ a stepper RTL test. (testing)- Remove 8 dev screenshots committed to repo root (
chain-expanded.png,dock-filled.png,spinner-on-border.png,spinner-small.png,streaming-early.png,streaming-thinking.png,thought-open.png,tool-page-awaiting.png) — unreferenced binaries that permanently bloat git history; attach to the PR/issue instead. (Flagged by: architect, quality — 2 specialists)
🟡 Nice to Have
- Guard
NEXT_PUBLIC_FORCE_ALL_FLAGSbehind non-production (use-get-flag.ts:172) — build-time env var that silently enables all gated features if it leaks into a non-dev build. (architect) - Strengthen
web_fetchtruncation test (web_fetch_test.py:22) — assertlen(content) <= _MAX_CONTENT_BYTESand add a non-HTMLcontent_typebranch (title staysNone). (testing) - Catalog-drift guard — a lightweight test asserting frontend catalog keys are a subset of backend tool names would catch silent registry drift. (architect)
🔵 Nits
- MCP token input lacks
autoComplete="off"(McpConnectorRow.tsx:73) — password managers may capture the long-lived API token. (security) - Duplicated
"Fill in the details"string (InputsSection.tsx:32,36) — hoist to a named const. (quality) - Change-relative comments (
tool_schema_test.py:422,SetupRequirementsCard.tsx:57) — rewrite "Bumped X→Y" / "no longer renders" to state the standing invariant. (architect) - Wordless silent-gap thinking state (
ThinkingIndicator.tsx:40) — consider a minimal visible fallback label when no status message is present. (product) - Hostname-derived MCP provider icon (
McpConnectorRow.tsx:26) — can resolve to a misleading brand icon for arbitrary hosts; match known slugs first. (product) - Hardcoded
text-purple-700"Read more" toggle (UserMessageClamp.tsx:26) — use a design token for palette consistency. (product)
QA Screenshots
Human Review Needed
NO — This is a feature-flagged frontend UI change with small additive backend metadata; it touches no authentication, credential storage, or cross-service trust boundary. QA verified it end-to-end and all findings are non-blocking. (Note separately: two process items — dismiss the stale bot CHANGES_REQUESTED and the requested maintainer reviews — gate merge in GitHub, but neither is a code concern.)
Risk Assessment
Merge risk: LOW | Rollback: EASY (feature-flag-gated; disable NEW_TOOL_UI to fully revert to legacy renderer)
CI Status
GitHub CI (per discussion review): ✅ all checks green on the current head — test (3.11/3.12/3.13), type-check, lint, e2e, integration, CodeQL, codecov all passing; branch MERGEABLE. Local harness: lint ✅, backend lint ✅, typecheck ✅, build ✅ — but pnpm test:unit failed in the review sandbox. Because GitHub CI ran the same frontend suite green on this SHA, the local failure is treated as environment skew (per repo-CI-authoritative policy), not a defect. Note: the only thing still reporting reviewDecision: CHANGES_REQUESTED is a stale bot review whose blocker was already fixed — it should be dismissed or re-run.
UI Testing — Variant Results
✅ local: New copilot tool-chain UI, web_fetch/block-provider metadata, and re-enabled ask_question tool all verified working end-to-end with passing backend tests and correct negative-case handling.
⚠️ hosted: New copilot tool-chain UI, StreamStatus narration, and web_fetch metadata all work live; the two new production-guarded debug pages crash with a client-side exception instead of rendering a clean 404.
- medium: In the production build (container NODE_ENV=production) navigating to /copilot/tool renders a client-side exception ('Application error: a client-side exception has occurred') instead of the intended 404 from notFound(). The real /copilot page renders fine in the same authenticated session, so the crash is specific to this new route.
- medium: Same as /copilot/tool: /copilot/ui throws a client-side exception in the production build instead of rendering a 404 via the NODE_ENV production guard.
Superseded by a newer automated review.
|
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |










Why / What / How
Copilot tool activity needs a clearer, compact presentation that remains useful while streaming and exposes rich results without hiding interactive tools. This adds a feature-flagged, unified tool-chain renderer with animated status, provider-aware Hugeicons, specialized result cards, and guarded debug surfaces while preserving the legacy renderer. Backend tool responses provide the block-provider and web-page metadata required by those cards, with regression coverage across frontend rendering and backend execution paths.
Changes 🏗️
Checklist 📋
For code changes:
pnpm format,pnpm lint, andpnpm typespnpm test:unitsuiteExample test plan
For configuration changes:
.env.defaultis updated or already compatible with my changesdocker-compose.ymlis updated or already compatible with my changesExamples of configuration changes