feat: change tool approval message when no parameters are required#596
feat: change tool approval message when no parameters are required#596
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
🤖 LLM Evaluation ResultsOpenAI
❌ Failed EvaluationsShow 2 failuresOPENAI1. TestReactEval/[openai]_react_cat_message
2. TestThreadsOpenQuestionsFromExportedData/[openai]_open_questions_from_eval_timed_dnd.json
Anthropic
❌ Failed EvaluationsShow 2 failuresANTHROPIC1. TestReactEval/[anthropic]_react_cat_message
2. TestThreadsSummarizeFromExportedData/[anthropic]_thread_summarization_from_eval_timed_dnd.json
This comment was automatically generated by the eval CI pipeline. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Security Review: No Issues Found
This PR makes a purely client-side UI presentation change that controls the default collapsed/expanded state of tool call cards when their arguments are empty. No server-side code, authentication/authorization logic, data access paths, or LLM prompt construction is modified. No security concerns identified.
Sent by Cursor Automation: Find vulnerabilities
There was a problem hiding this comment.
Pull request overview
This PR reduces UI noise in the tool approval “call” stage by default-collapsing tool cards when their arguments are empty (or null), so tools without configuration don’t take up unnecessary space.
Changes:
- Add an
hasEmptyArgumentshelper to detect empty/null tool arguments. - Update the collapse-default logic so call-stage tools with empty arguments start collapsed unless the user toggles them open.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdjusted Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
webapp/src/components/tool_approval_set.tsx (1)
239-245: Consider treating[]as empty arguments too.Line 244 excludes arrays, so
arguments: []won’t default-collapse even though it’s effectively empty input. Expanding the helper to include empty arrays would make behavior consistent for all empty JSON argument shapes.Suggested tweak
const hasEmptyArguments = (tool: ToolCall) => { const args = tool.arguments; if (args == null) { return true; } - return typeof args === 'object' && !Array.isArray(args) && Object.keys(args).length === 0; + if (Array.isArray(args)) { + return args.length === 0; + } + return typeof args === 'object' && Object.keys(args).length === 0; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webapp/src/components/tool_approval_set.tsx` around lines 239 - 245, The helper hasEmptyArguments currently treats null and empty plain objects as empty but explicitly excludes arrays, so ToolCall.arguments: [] isn’t considered empty; update hasEmptyArguments to return true when arguments is null OR (Array.isArray(args) && args.length === 0) OR (typeof args === 'object' && !Array.isArray(args) && Object.keys(args).length === 0) so empty arrays are collapsed the same as empty objects; keep the function name hasEmptyArguments and the ToolCall.arguments references to locate and update the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@webapp/src/components/tool_approval_set.tsx`:
- Around line 239-245: The helper hasEmptyArguments currently treats null and
empty plain objects as empty but explicitly excludes arrays, so
ToolCall.arguments: [] isn’t considered empty; update hasEmptyArguments to
return true when arguments is null OR (Array.isArray(args) && args.length === 0)
OR (typeof args === 'object' && !Array.isArray(args) && Object.keys(args).length
=== 0) so empty arrays are collapsed the same as empty objects; keep the
function name hasEmptyArguments and the ToolCall.arguments references to locate
and update the logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c892e24e-50e2-4b09-b233-b58b26904f86
📒 Files selected for processing (1)
webapp/src/components/tool_approval_set.tsx
nickmisasi
left a comment
nickmisasi
left a comment
There was a problem hiding this comment.
Could we enhance the front-end so that rather than {} is just shows something like No arguments provided or something?
I think a lot of people see {} as "the tool is broken" even though it's intended, and this change doesn't fully fix that.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@webapp/src/components/tool_card.tsx`:
- Around line 76-80: NoArgumentsLabel currently applies padding-left: 24px which
duplicates the indentation already provided by ToolCallArguments; remove the
padding-left rule from the NoArgumentsLabel styled component so the
empty-arguments label inherits the existing indentation from ToolCallArguments
(locate the NoArgumentsLabel definition and delete or omit the padding-left
property).
🪄 Autofix (Beta)
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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: ba4612f7-4be5-40aa-833f-9f6edfecc046
📒 Files selected for processing (1)
webapp/src/components/tool_card.tsx
nickmisasi
left a comment
nickmisasi
left a comment
There was a problem hiding this comment.
LGTM! Not blocking - but perhaps poke Asaad to get some recommended padding-top/bottom numbers
Display "No parameters required" inside the tool call code block
instead of an empty JSON object ({}), making it clear the empty
state is intentional and the tool is not broken.
fmartingr
force-pushed
the
feat/tool-config-empty
branch
from
e9dedfc to
c222695
Compare
fmartingr
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@webapp/src/components/tool_card.tsx`:
- Line 392: The string literal "No parameters required" in the assignment to
content (const content = isEmpty ? 'No parameters required' :
JSON.stringify(argumentsValue, null, 2)) must be localized; replace the
hardcoded text with a react-intl message (e.g. useIntl().formatMessage or
<FormattedMessage>) and reference a new message id like "toolCard.noParameters"
with a sensible defaultMessage; update the component to import and use useIntl
(or wrap with <FormattedMessage>) and add the new message key to your
messages/locale files so the fallback/default text remains "No parameters
required".
🪄 Autofix (Beta)
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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: db7050fa-7398-4da8-8655-47f85938737d
📒 Files selected for processing (1)
webapp/src/components/tool_card.tsx
Route the empty-arguments label through react-intl so it can be translated, matching the project's i18n conventions.
Summary
{}) when a tool call has no argumentsSummary by CodeRabbit
Release Notes