Normalized tool inputs before rendering so string payloads (like {"fi…#76
Normalized tool inputs before rendering so string payloads (like {"fi…#76hbmartin wants to merge 1 commit into
Conversation
…le_path":…}) are parsed when possible, safely handled when not, and no longer trigger the 'in' operator runtime error; also reuse the normalized display string for the details pane.
Reviewer's GuideThis PR enhances the ChatInterface by normalizing raw tool inputs into a structured Sequence diagram for tool input normalization and renderingsequenceDiagram
participant UI as "ChatInterface UI"
participant Tool as "ToolCallPart"
participant Parser as "Input Parser"
participant Renderer as "Details Renderer"
UI->>Tool: Receives raw tool input
UI->>Parser: Normalize and parse tool input
Parser-->>UI: Returns toolInput and toolInputDisplay
UI->>Renderer: Render tool details using toolInputDisplay
Renderer-->>UI: Display normalized input safely
Class diagram for updated tool input normalization in ChatInterfaceclassDiagram
class ChatInterface {
+providerRegistry: Ref<ProviderRegistry>
+msLogger: Logger
+modelSelectTelementry: ModelPickerTelemetry
+handleNewConversation()
+toolInput: Record<string, unknown> | undefined
+toolInputDisplay: string
}
class ModelSelect {
+storage: Storage
+providerRegistry: ProviderRegistry
+telemetry: ModelPickerTelemetry
}
ChatInterface --> ModelSelect: uses
ModelSelect --> ModelPickerTelemetry: telemetry
ChatInterface --> Logger: msLogger
ChatInterface --> ProviderRegistry: providerRegistry
ChatInterface --> Storage: storage
ChatInterface --> ModelPickerTelemetry: modelSelectTelementry
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughTelemetry instrumentation was added to the ModelSelect component in ChatInterface.tsx, and tool input parsing/rendering logic was hardened. The component now safely handles string/object inputs, formats them for display, extracts fields defensively, and wires telemetry callbacks for provider/model events. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ChatInterface
participant ModelSelect
participant Telemetry
participant Provider
User->>ChatInterface: Open model picker
ChatInterface->>ModelSelect: Render with telemetry config
ModelSelect->>Telemetry: onFetchStart()
ModelSelect->>Provider: Fetch models/config
Provider-->>ModelSelect: Success/Failure
alt Fetch success
ModelSelect->>Telemetry: onFetchSuccess(models)
else Fetch error
ModelSelect->>Telemetry: onFetchError(err)
end
note over ModelSelect,Telemetry: Additional events: onStorageError, onUserModelAdded,<br/>onProviderInitError, onProviderNotFound, onProviderInvalidConfig
sequenceDiagram
autonumber
actor User
participant ChatInterface
participant ToolResult
User->>ChatInterface: View tool result
ToolResult-->>ChatInterface: input (string|object), metadata
ChatInterface->>ChatInterface: Parse input (JSON.parse if string)<br/>Serialize to pretty JSON for display
alt Input details present
ChatInterface-->>User: Render formatted toolInputDisplay
ChatInterface->>ChatInterface: Safely extract theme_name/css info with fallbacks
else No details
ChatInterface-->>User: Omit input block
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
Summary of ChangesHello @hbmartin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness and user experience of displaying tool inputs within the chat interface. It addresses potential runtime errors caused by inconsistent tool input formats by implementing a comprehensive parsing and normalization strategy. Additionally, it integrates telemetry for the ModelSelect component, improving observability and debugging capabilities for model selection processes. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- Consider extracting the rawToolInput parsing/normalization logic into its own utility function to declutter the JSX and make it easier to unit test.
- Remove the stray console.log in the modelSelectTelemetry useMemo (looks like leftover debugging) and correct the telemetry prop/variable name for consistency (e.g. modelSelectTelemetry).
- You’re doing a lot of repetitive checks when accessing toolInput properties—consider using optional chaining with defaults or a small helper to DRY up those property lookups.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the rawToolInput parsing/normalization logic into its own utility function to declutter the JSX and make it easier to unit test.
- Remove the stray console.log in the modelSelectTelemetry useMemo (looks like leftover debugging) and correct the telemetry prop/variable name for consistency (e.g. modelSelectTelemetry).
- You’re doing a lot of repetitive checks when accessing toolInput properties—consider using optional chaining with defaults or a small helper to DRY up those property lookups.
## Individual Comments
### Comment 1
<location> `src/webview/components/Chat/ChatInterface.tsx:122-124` </location>
<code_context>
const providerRegistry = useRef(createDefaultRegistry());
+ const msLogger = useLogger('ModelSelect');
+ const modelSelectTelementry = useMemo(() => {
+ console.log(`msLogger:`, msLogger);
+ return {
+ onFetchStart: (providerId: ProviderId) => msLogger.debug(`onFetchStart: ${providerId}`),
</code_context>
<issue_to_address>
**suggestion:** Remove or guard console.log for production readiness.
Console.log statements in production can expose sensitive information and clutter logs. Please remove or restrict this to development environments.
```suggestion
const modelSelectTelementry = useMemo(() => {
if (process.env.NODE_ENV === 'development') {
// Only log in development
console.log(`msLogger:`, msLogger);
}
return {
```
</issue_to_address>
### Comment 2
<location> `src/webview/components/Chat/ChatInterface.tsx:1119` </location>
<code_context>
<span className='tool-detail__label'>Input:</span>
<div className='tool-detail__value tool-detail__value--result'>
- <pre className='tool-result-content'>{inputString}</pre>
+ <pre className='tool-result-content'>{toolInputDisplay}</pre>
</div>
</div>
</code_context>
<issue_to_address>
**🚨 issue (security):** toolInputDisplay may contain unescaped HTML if input is user-controlled.
If toolInputDisplay includes user input, escape it before rendering to prevent XSS, particularly within the <pre> tag.
</issue_to_address>
### Comment 3
<location> `src/webview/components/Chat/ChatInterface.tsx:868` </location>
<code_context>
- const toolInput = toolCallPart.input as object;
+ const rawToolInput = toolCallPart.input;
+
+ let toolInput: Record<string, unknown> | undefined;
+ let toolInputDisplay = '';
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting tool-input parsing and ModelSelect telemetry setup into separate utility and hook functions to simplify the component.
```suggestion
// Consider moving both the “raw → parsed → display” tool-input logic and the ModelSelect telemetry
// setup out of the render function. This will flatten nesting and keep your component lean.
// 1) Extract tool-input parsing into a util:
// utils/formatToolInput.ts
export function formatToolInput(raw: unknown): {
toolInput?: Record<string, unknown>;
display: string;
} {
if (raw == null) return { display: '' };
// string input
if (typeof raw === 'string') {
try {
const parsed = JSON.parse(raw);
if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
return { toolInput: parsed as Record<string, unknown>, display: JSON.stringify(parsed, null, 2) };
}
} catch {
// fall back to raw string
}
return { display: raw };
}
// object input
if (typeof raw === 'object') {
try {
const obj = raw as Record<string, unknown>;
return { toolInput: obj, display: JSON.stringify(obj, null, 2) };
} catch {
return { display: '[Tool input serialization failed]' };
}
}
// other primitives
return { display: String(raw) };
}
// Then in your renderSingleToolCall:
import { formatToolInput } from '../../utils/formatToolInput';
const { toolInput, display: toolInputDisplay } = formatToolInput(toolCallPart.input);
// …
{hasToolInputDetails && (
<div className="tool-detail">
<span className="tool-detail__label">Input:</span>
<pre className="tool-result-content">{toolInputDisplay}</pre>
</div>
)}
// 2) Extract ModelSelect telemetry into a custom hook:
// hooks/useModelSelectTelemetry.ts
import { useLogger } from 'react-vscode-webview-ipc/client';
import type { ModelPickerTelemetry, ProviderId, ModelId } from 'ai-sdk-react-model-picker';
export function useModelSelectTelemetry(): ModelPickerTelemetry {
const logger = useLogger('ModelSelect');
return useMemo<ModelPickerTelemetry>(
() => ({
onFetchStart: (p) => logger.debug(`onFetchStart: ${p}`),
onFetchSuccess: (p, count) => logger.debug(`onFetchSuccess: ${p}: ${count}`),
onFetchError: (p, e) => logger.error(`onFetchError: ${p}: ${e.message}`, { e }),
onStorageError: (op, key, e) => logger.error(`onStorageError: ${key}: ${op}: ${e.message}`, { e }),
onUserModelAdded: (p, m) => logger.debug(`onUserModelAdded: ${p}: ${m}`),
onProviderInitError: (prov, e) => logger.error(`onProviderInitError: ${prov}: ${e.message}`, { e }),
onProviderNotFound: (p) => logger.error(`onProviderNotFound: ${p}`),
onProviderInvalidConfig: (p) => logger.error(`onProviderInvalidConfig: ${p}`),
}),
[logger],
);
}
// Then in your component:
import { useModelSelectTelemetry } from '../../hooks/useModelSelectTelemetry';
const modelSelectTelemetry = useModelSelectTelemetry();
// …
<ModelSelect
storage={storage.current}
providerRegistry={providerRegistry.current}
telemetry={modelSelectTelemetry}
/>
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const modelSelectTelementry = useMemo(() => { | ||
| console.log(`msLogger:`, msLogger); | ||
| return { |
There was a problem hiding this comment.
suggestion: Remove or guard console.log for production readiness.
Console.log statements in production can expose sensitive information and clutter logs. Please remove or restrict this to development environments.
| const modelSelectTelementry = useMemo(() => { | |
| console.log(`msLogger:`, msLogger); | |
| return { | |
| const modelSelectTelementry = useMemo(() => { | |
| if (process.env.NODE_ENV === 'development') { | |
| // Only log in development | |
| console.log(`msLogger:`, msLogger); | |
| } | |
| return { |
| <span className='tool-detail__label'>Input:</span> | ||
| <div className='tool-detail__value tool-detail__value--result'> | ||
| <pre className='tool-result-content'>{inputString}</pre> | ||
| <pre className='tool-result-content'>{toolInputDisplay}</pre> |
There was a problem hiding this comment.
🚨 issue (security): toolInputDisplay may contain unescaped HTML if input is user-controlled.
If toolInputDisplay includes user input, escape it before rendering to prevent XSS, particularly within the
tag.
| const toolInput = toolCallPart.input as object; | ||
| const rawToolInput = toolCallPart.input; | ||
|
|
||
| let toolInput: Record<string, unknown> | undefined; |
There was a problem hiding this comment.
issue (complexity): Consider extracting tool-input parsing and ModelSelect telemetry setup into separate utility and hook functions to simplify the component.
| let toolInput: Record<string, unknown> | undefined; | |
| // Consider moving both the “raw → parsed → display” tool-input logic and the ModelSelect telemetry | |
| // setup out of the render function. This will flatten nesting and keep your component lean. | |
| // 1) Extract tool-input parsing into a util: | |
| // utils/formatToolInput.ts | |
| export function formatToolInput(raw: unknown): { | |
| toolInput?: Record<string, unknown>; | |
| display: string; | |
| } { | |
| if (raw == null) return { display: '' }; | |
| // string input | |
| if (typeof raw === 'string') { | |
| try { | |
| const parsed = JSON.parse(raw); | |
| if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { | |
| return { toolInput: parsed as Record<string, unknown>, display: JSON.stringify(parsed, null, 2) }; | |
| } | |
| } catch { | |
| // fall back to raw string | |
| } | |
| return { display: raw }; | |
| } | |
| // object input | |
| if (typeof raw === 'object') { | |
| try { | |
| const obj = raw as Record<string, unknown>; | |
| return { toolInput: obj, display: JSON.stringify(obj, null, 2) }; | |
| } catch { | |
| return { display: '[Tool input serialization failed]' }; | |
| } | |
| } | |
| // other primitives | |
| return { display: String(raw) }; | |
| } | |
| // Then in your renderSingleToolCall: | |
| import { formatToolInput } from '../../utils/formatToolInput'; | |
| const { toolInput, display: toolInputDisplay } = formatToolInput(toolCallPart.input); | |
| // … | |
| {hasToolInputDetails && ( | |
| <div className="tool-detail"> | |
| <span className="tool-detail__label">Input:</span> | |
| <pre className="tool-result-content">{toolInputDisplay}</pre> | |
| </div> | |
| )} | |
| // 2) Extract ModelSelect telemetry into a custom hook: | |
| // hooks/useModelSelectTelemetry.ts | |
| import { useLogger } from 'react-vscode-webview-ipc/client'; | |
| import type { ModelPickerTelemetry, ProviderId, ModelId } from 'ai-sdk-react-model-picker'; | |
| export function useModelSelectTelemetry(): ModelPickerTelemetry { | |
| const logger = useLogger('ModelSelect'); | |
| return useMemo<ModelPickerTelemetry>( | |
| () => ({ | |
| onFetchStart: (p) => logger.debug(`onFetchStart: ${p}`), | |
| onFetchSuccess: (p, count) => logger.debug(`onFetchSuccess: ${p}: ${count}`), | |
| onFetchError: (p, e) => logger.error(`onFetchError: ${p}: ${e.message}`, { e }), | |
| onStorageError: (op, key, e) => logger.error(`onStorageError: ${key}: ${op}: ${e.message}`, { e }), | |
| onUserModelAdded: (p, m) => logger.debug(`onUserModelAdded: ${p}: ${m}`), | |
| onProviderInitError: (prov, e) => logger.error(`onProviderInitError: ${prov}: ${e.message}`, { e }), | |
| onProviderNotFound: (p) => logger.error(`onProviderNotFound: ${p}`), | |
| onProviderInvalidConfig: (p) => logger.error(`onProviderInvalidConfig: ${p}`), | |
| }), | |
| [logger], | |
| ); | |
| } | |
| // Then in your component: | |
| import { useModelSelectTelemetry } from '../../hooks/useModelSelectTelemetry'; | |
| const modelSelectTelemetry = useModelSelectTelemetry(); | |
| // … | |
| <ModelSelect | |
| storage={storage.current} | |
| providerRegistry={providerRegistry.current} | |
| telemetry={modelSelectTelemetry} | |
| /> |
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
|
Fixes superdesigndev#70 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces important fixes and enhancements to the chat interface. The primary change is the normalization of tool inputs, which correctly handles string-based payloads and prevents runtime errors. This is a significant improvement for the robustness of tool handling. Additionally, telemetry has been added to the model picker for better observability, which is a valuable enhancement.
My review focuses on improving code consistency and correctness. I've pointed out some leftover debug console.log statements and suggested using the centralized logger for consistency. I've also recommended a small refactoring to make property access safer and less repetitive. Overall, these are solid changes that improve the user experience and maintainability of the code.
| const providerRegistry = useRef(createDefaultRegistry()); | ||
| const msLogger = useLogger('ModelSelect'); | ||
| const modelSelectTelementry = useMemo(() => { | ||
| console.log(`msLogger:`, msLogger); |
There was a problem hiding this comment.
This console.log appears to be for debugging purposes. It should be removed before merging to keep the console clean in production environments.
| toolInputDisplay = JSON.stringify(parsed, null, 2); | ||
| } | ||
| } catch (parseError) { | ||
| console.warn('Failed to parse tool input string:', parseError); |
There was a problem hiding this comment.
It's good practice to use the provided logger instance for logging instead of console.warn. This ensures consistent logging behavior and allows for easier filtering and management of logs. You can replace this with logger.warn(...).
| console.warn('Failed to parse tool input string:', parseError); | |
| logger.warn('Failed to parse tool input string:', parseError); |
| try { | ||
| toolInputDisplay = JSON.stringify(toolInput, null, 2); | ||
| } catch (stringifyError) { | ||
| console.error('Failed to stringify tool input object:', stringifyError); |
There was a problem hiding this comment.
Similar to the previous comment, it's better to use the logger instance for logging errors. This helps centralize logging and makes it easier to handle in different environments. Please replace console.error with logger.error(...).
| console.error('Failed to stringify tool input object:', stringifyError); | |
| logger.error('Failed to stringify tool input object:', stringifyError); |
| const description: string = | ||
| 'description' in toolInput ? (toolInput.description as string) : ''; | ||
| const command: string = 'command' in toolInput ? (toolInput.command as string) : ''; | ||
| const prompt: string = 'prompt' in toolInput ? (toolInput.prompt as string) : ''; | ||
| toolInput !== undefined && 'description' in toolInput && toolInput.description !== undefined | ||
| ? String(toolInput.description) | ||
| : ''; | ||
| const command: string = | ||
| toolInput !== undefined && 'command' in toolInput && toolInput.command !== undefined | ||
| ? String(toolInput.command) | ||
| : ''; | ||
| const prompt: string = | ||
| toolInput !== undefined && 'prompt' in toolInput && toolInput.prompt !== undefined | ||
| ? String(toolInput.prompt) | ||
| : ''; |
There was a problem hiding this comment.
The logic to extract description, command, and prompt is a bit repetitive and can be improved for conciseness and correctness.
- Using
!= nullis a more robust way to check for bothnullandundefinedvalues. The current check for!== undefinedon the property value would incorrectly handle anullvalue by converting it to the string'null'. An empty string would be a better fallback. - The repeated logic could be simplified for better readability.
Here's a suggested improvement that addresses these points.
| const description: string = | |
| 'description' in toolInput ? (toolInput.description as string) : ''; | |
| const command: string = 'command' in toolInput ? (toolInput.command as string) : ''; | |
| const prompt: string = 'prompt' in toolInput ? (toolInput.prompt as string) : ''; | |
| toolInput !== undefined && 'description' in toolInput && toolInput.description !== undefined | |
| ? String(toolInput.description) | |
| : ''; | |
| const command: string = | |
| toolInput !== undefined && 'command' in toolInput && toolInput.command !== undefined | |
| ? String(toolInput.command) | |
| : ''; | |
| const prompt: string = | |
| toolInput !== undefined && 'prompt' in toolInput && toolInput.prompt !== undefined | |
| ? String(toolInput.prompt) | |
| : ''; | |
| const description: string = | |
| toolInput != null && 'description' in toolInput && toolInput.description != null | |
| ? String(toolInput.description) | |
| : ''; | |
| const command: string = | |
| toolInput != null && 'command' in toolInput && toolInput.command != null | |
| ? String(toolInput.command) | |
| : ''; | |
| const prompt: string = | |
| toolInput != null && 'prompt' in toolInput && toolInput.prompt != null | |
| ? String(toolInput.prompt) | |
| : ''; |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to af44597 in 2 minutes and 13 seconds. Click for details.
- Reviewed
172lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/webview/components/Chat/ChatInterface.tsx:871
- Draft comment:
Good job safely normalizing the tool input with JSON parsing. Consider extracting this logic into a helper function if similar processing is needed elsewhere, to improve readability and reduce duplication. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. src/webview/components/Chat/ChatInterface.tsx:1118
- Draft comment:
Reusing the normalized 'toolInputDisplay' for the details pane is an effective improvement over inline serialization. This consolidates error handling and ensures consistent display. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. src/webview/components/Chat/ChatInterface.tsx:122
- Draft comment:
Typographical error: 'modelSelectTelementry' appears to be misspelled. Consider renaming it to 'modelSelectTelemetry' for clarity. - Reason this comment was not posted:
Marked as duplicate.
4. src/webview/components/Chat/ChatInterface.tsx:1391
- Draft comment:
Typo in variable name: 'modelSelectTelementry' appears to be a misspelling. Consider renaming to 'modelSelectTelemetry'. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_bd8KewE1P3J23ZJA
You can customize 
| ); | ||
| const providerRegistry = useRef(createDefaultRegistry()); | ||
| const msLogger = useLogger('ModelSelect'); | ||
| const modelSelectTelementry = useMemo(() => { |
There was a problem hiding this comment.
Typo: Rename 'modelSelectTelementry' to 'modelSelectTelemetry' for clarity.
| const modelSelectTelementry = useMemo(() => { | |
| const modelSelectTelemetry = useMemo(() => { |
| const providerRegistry = useRef(createDefaultRegistry()); | ||
| const msLogger = useLogger('ModelSelect'); | ||
| const modelSelectTelementry = useMemo(() => { | ||
| console.log(`msLogger:`, msLogger); |
There was a problem hiding this comment.
Remove the debug console.log in the telemetry useMemo block to avoid unnecessary logging in production.
| console.log(`msLogger:`, msLogger); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/webview/components/Chat/ChatInterface.tsx(8 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/webview/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Implement React-based Canvas and Chat UI components in src/webview/components with clear structure (CanvasView, Chat, DesignFrame, ConnectionLines)
Files:
src/webview/components/Chat/ChatInterface.tsx
src/webview/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/webview/**/*.{ts,tsx}: Decouple backend logic from frontend React; use a structured webview message protocol and validate/sanitize messages
Avoid large payloads over postMessage; use chunked streaming or on-demand fetching
Use vscode webview state (setState/getState) to preserve UI state across reloads
Clean up listeners/timers in webviews to avoid leaks (dispose on unmount/dispose)
Debug webviews with Developer Tools; support VS Code themes in webview UI
Ensure accessibility: keyboard navigation, ARIA labels, focus management, screen reader support, reduced motion
Files:
src/webview/components/Chat/ChatInterface.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Prefer const over let; never use var
Use explicit return types for public APIs
Leverage type inference for local variables
Define interfaces for complex objects instead of type aliases; create specific interfaces per concern
Use enums sparingly; prefer const assertions or union types
Implement proper error types (custom classes) instead of generic Error
Use optional chaining (?.) and nullish coalescing (??)
Prefer readonly arrays and properties where applicable
Use generic constraints for reusable components
Avoid any; use unknown when the type is truly unknown
Implement discriminated unions for state management
Use template literal types for string patterns
Maintain type safety throughout the codebase
Register commands in code via vscode.commands.registerCommand with consistent names (extension.commandName)
Register all disposables in activate() via context.subscriptions and implement proper deactivate() cleanup
Defer heavy initialization until needed; lazy-load modules with dynamic imports
Cache expensive computations at module level
Validate command arguments before execution; return promises from handlers for testability
Use VS Code configuration API with typed interfaces; listen to onDidChangeConfiguration and validate values
Use vscode.window.withProgress for long-running operations
Leverage vscode.Uri and vscode.workspace.fs for file operations; avoid direct Node fs for workspace files
Implement TextDocumentContentProvider for virtual documents where needed
Use diagnostic collections to report problems
Implement cancellation tokens for long operations; debounce frequent events like onDidChangeTextDocument
Batch workspace edits and cache parsed ASTs/computations where beneficial
Wrap async operations in try-catch; log errors to output channel; provide user-friendly messages with actions
Use custom error classes with error codes; implement retry with exponential backoff for network failures
Validate external data and API responses
Never store s...
Files:
src/webview/components/Chat/ChatInterface.tsx
src/webview/**/*.{ts,tsx,html}
📄 CodeRabbit inference engine (CLAUDE.md)
Sanitize all webview content and enforce Content Security Policy with nonces; avoid eval/dynamic code
Files:
src/webview/components/Chat/ChatInterface.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: hbmartin/secure-design#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-28T01:08:35.717Z
Learning: Applies to **/*.{ts,tsx} : Stream responses for real-time chat; support model selection per provider via unified interface
🧬 Code graph analysis (1)
src/webview/components/Chat/ChatInterface.tsx (1)
src/providers/types.ts (1)
ProviderId(30-30)
🪛 GitHub Actions: CI
src/webview/components/Chat/ChatInterface.tsx
[error] 28-28: TS2305: Module 'ai-sdk-react-model-picker' has no exported member 'ModelPickerTelemetry'.
🪛 GitHub Check: lint-and-typecheck
src/webview/components/Chat/ChatInterface.tsx
[failure] 28-28:
Module '"ai-sdk-react-model-picker"' has no exported member 'ModelPickerTelemetry'.
[failure] 1391-1391:
Type '{ storage: { get: (key: string) => PromiseLike<Record<string, string> | undefined>; set: (key: string, value: Record<string, string>) => PromiseLike; remove: (key: string) => PromiseLike<...>; }; providerRegistry: ProviderRegistry; telemetry: ModelPickerTelemetry; }' is not assignable to type 'IntrinsicAttributes & ModelSelectProps'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (4)
src/webview/components/Chat/ChatInterface.tsx (4)
866-894: LGTM: Robust tool input parsing and display logic.The refactored tool input handling safely parses string payloads, handles JSON parsing errors gracefully, and provides appropriate fallbacks for serialization failures. This directly addresses the PR objective of normalizing tool inputs before rendering.
924-938: LGTM: Safe field extraction with guarded access.The extraction of
theme_name,cssSheet, andcssFilePathnow uses proper guarded access patterns to handle undefinedtoolInputand mixed types, with appropriate fallbacks from tool result metadata.
1008-1018: LGTM: Safe field extraction for description, command, and prompt.The guarded access patterns properly handle undefined
toolInputand ensure type safety with explicit checks before property access and string coercion.
1057-1059: LGTM: Conditional rendering based on tool input details.The
hasToolInputDetailsflag correctly determines when to render the Input block, andtoolInputDisplayis appropriately used for rendering the formatted tool input.Also applies to: 1115-1122
| import { | ||
| createDefaultRegistry, | ||
| ModelId, | ||
| ModelSelect, | ||
| ProviderId, | ||
| type ModelPickerTelemetry, | ||
| } from 'ai-sdk-react-model-picker'; |
There was a problem hiding this comment.
🧩 Analysis chain
Critical: ModelPickerTelemetry type is not exported by the library.
The TypeScript compiler and static analysis confirm that ai-sdk-react-model-picker does not export ModelPickerTelemetry. This causes a compilation failure and the telemetry prop on line 1391 is also rejected by TypeScript.
Based on learnings, the library version is 0.2.0. Please verify whether this version exports the telemetry type and accepts a telemetry prop:
Until confirmed, consider removing the telemetry instrumentation or checking the library documentation for the correct API.
Also applies to: 122-141, 1391-1391
🌐 Web query:
Does ai-sdk-react-model-picker version 0.2.0 support telemetry configuration with a ModelPickerTelemetry type?
💡 Result:
Short answer: No — ai-sdk-react-model-picker@0.2.0 does not expose a ModelPickerTelemetry type or documented telemetry configuration. See the package docs / npm listing (no telemetry API or ModelPickerTelemetry mentioned). [1][2]
Sources:
[1] ai-sdk-react-model-picker package listing and docs. [libraries.io/npm/ai-sdk-react-model-picker].
[2] ai-sdk-react-model-picker package analysis/docs (npm/socket.dev).
Remove unsupported telemetry instrumentation
Remove the import of ModelPickerTelemetry from ai-sdk-react-model-picker (lines 23–29) and the telemetry prop on the ModelSelect component (line 1391). ai-sdk-react-model-picker@0.2.0 does not export or support telemetry configuration.
🧰 Tools
🪛 GitHub Actions: CI
[error] 28-28: TS2305: Module 'ai-sdk-react-model-picker' has no exported member 'ModelPickerTelemetry'.
🪛 GitHub Check: lint-and-typecheck
[failure] 28-28:
Module '"ai-sdk-react-model-picker"' has no exported member 'ModelPickerTelemetry'.
🤖 Prompt for AI Agents
In src/webview/components/Chat/ChatInterface.tsx around lines 23–29 and at line
1391, remove the unsupported telemetry instrumentation: delete
ModelPickerTelemetry from the import list from 'ai-sdk-react-model-picker' and
remove the telemetry={...} prop passed to the ModelSelect component at ~line
1391; if the telemetry identifier was only used for typing, also remove any
local types/usages tied to it or replace with supported types so the file
compiles cleanly.
| const providerRegistry = useRef(createDefaultRegistry()); | ||
| const msLogger = useLogger('ModelSelect'); | ||
| const modelSelectTelementry = useMemo(() => { | ||
| console.log(`msLogger:`, msLogger); |
There was a problem hiding this comment.
Remove console.log from production code.
The console.log statement should be removed or replaced with the msLogger.debug call. Console logs are not appropriate for production code and can expose internal state.
Apply this diff:
- console.log(`msLogger:`, msLogger);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log(`msLogger:`, msLogger); |
🤖 Prompt for AI Agents
In src/webview/components/Chat/ChatInterface.tsx around line 123, remove the
console.log(`msLogger:`, msLogger) and replace it with a msLogger.debug call;
specifically call msLogger.debug with a clear message and the msLogger object
(or JSON.stringify(msLogger) if it contains non-serializable data), and ensure
msLogger is available in scope (import or prop) before using it so there are no
runtime errors.
1 participant
User description
…le_path":…}) are parsed when possible, safely handled when not, and no longer trigger the 'in' operator runtime error; also reuse the normalized display string for the details pane.
PR Type
Bug fix, Enhancement
Description
Fixed runtime error from 'in' operator on string tool inputs
Added safe parsing and normalization for tool input data
Enhanced model picker with telemetry logging integration
Improved error handling for tool input serialization
Diagram Walkthrough
File Walkthrough
ChatInterface.tsx
Tool input normalization and telemetry integrationsrc/webview/components/Chat/ChatInterface.tsx
Important
Normalize tool inputs in
ChatInterface.tsxto handle string payloads safely and improve display consistency.renderSingleToolCall()to handle string payloads by attempting JSON parsing and safely handling errors.toolInputDisplayfor consistent display in the details pane.modelSelectTelementrylogging inChatInterfaceto track model selection events.ModelPickerTelemetrytype import inChatInterface.tsx.This description was created by
for af44597. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit