Skip to content

fix: dispatch tool metadata on category for cross-agent consistency#116

Merged
wesm merged 2 commits intowesm:mainfrom
clkao:gemini-tool-meta-category
Mar 9, 2026
Merged

fix: dispatch tool metadata on category for cross-agent consistency#116
wesm merged 2 commits intowesm:mainfrom
clkao:gemini-tool-meta-category

Conversation

@clkao
Copy link
Copy Markdown
Contributor

@clkao clkao commented Mar 8, 2026

Summary

  • extractToolParamMeta now dispatches on normalized category (Read, Bash, Edit, etc.) instead of raw tool_name, so Gemini tools (run_command, read_file, grep_search, etc.) render with the same metadata tags as their Claude equivalents
  • Adds cmd meta tag for Bash category showing first line of command, truncated to 80 chars
  • Normalizes param name lookups across agents (file_path/path, pattern/query, command/cmd)
  • Simplifies ToolBlock callsite: single extractToolParamMeta(toolName, params, category) call replaces the try-category-then-fallback-to-toolName pattern

Follow-up to #114. Missing ABOUTME file brief for gemini.go will be addressed in a separate PR.

Test plan

  • Verify Gemini tool calls show metadata tags (file path, command, pattern) in collapsed ToolBlock header
  • Verify Claude tool calls still render identically (no regression)
  • npx vitest run src/lib/utils/tool-params.test.ts — 57 tests pass

🤖 Generated with Claude Code

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 8, 2026

roborev: Combined Review (004de50)

This PR refactors tool metadata extraction to dispatch by category and adds Bash command handling,
but introduces a medium-severity regression risk regarding empty string fallbacks.

Medium

Regression risk: category fallback was removed for metadata extraction

  • Files: frontend/src/lib/utils/tool-params.ts (lines 22-25), frontend/src/lib/components /content/ToolBlock.svelte:106
  • Description: Previously, metadata extraction tried category first and safely handled empty strings (e.g., using toolCall.category || null), falling back to tool_name if no metadata matched. The updated logic uses a single
    dispatch path via the nullish coalescing operator: const cat = category ?? toolName;. Because ?? evaluates an empty string "" as a defined value, it will not fall back to toolName when category is blank. If a parser or stored row yields category: "", metadata extraction
    will now silently fail.
  • Suggested Fix: Change const cat = category ?? toolName; to const cat = category || toolName; to properly handle empty strings, or restore an explicit fallback behavior (category result ?? tool_name result) if the category-based extraction yields
    no tags.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

clkao and others added 2 commits March 8, 2026 22:09
extractToolParamMeta now accepts an optional category parameter and
dispatches on it instead of raw tool_name. This makes Gemini tools
(run_command, read_file, etc.) render with the same metadata tags
as their Claude equivalents (Bash, Read, etc.).

Also adds cmd meta tag for Bash category (first line, truncated)
and normalizes param names (file_path/path, pattern/query) across
agents.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Empty-string categories (from sessions that store blank categories)
caused `category ?? toolName` to preserve the empty string, skipping
all metadata extraction branches. Use `||` instead of `??` so empty
strings fall back to toolName.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@wesm wesm force-pushed the gemini-tool-meta-category branch from 004de50 to 2287ff7 Compare March 9, 2026 03:13
@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 9, 2026

roborev: Combined Review (2287ff7)

Summary Verdict: The PR successfully unifies tool metadata extraction, but introduces regressions in fallback handling and misses category-dispatching for Skill tools.

Findings

Medium

1. Fallback regression when category is non-empty but unrecognized

  • Files: frontend/src/lib/components/content/ToolBlock.svelte#L103, frontend/src/lib/utils/tool-params.ts#L24
  • Problem: Previous logic tried category first, then fell back to tool_name if there was no match. The new logic uses category || toolName once, so unknown/trash category values (including whitespace) suppress metadata that tool_name could have resolved.

Suggested Fix: Restore two-step behavior inside extractToolParamMeta (try category, then fallback to toolName when the category path yields no tags).

2. Skill handling is not category-dispatched

  • Files: frontend/src/lib/utils/tool- params.ts#L105 (to L112)
  • Problem: All major branches switched to the cat variable, but the check for the "Skill" tool still strictly compares against toolName. This creates cross-agent inconsistency and breaks metadata extraction if an agent emits category : "Skill" with a different tool_name (e.g., Gemini's activate_skill tool).
  • Suggested Fix: Change the condition to use the normalized category variable instead of the tool name. Update } else if (toolName === "Skill" || toolName === "skill ") { to } else if (cat === "Skill" || cat === "skill") {.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@wesm
Copy link
Copy Markdown
Owner

wesm commented Mar 9, 2026

Invalid

@wesm wesm merged commit b31cfcc into wesm:main Mar 9, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants