From 16f4d9b50f3a3d5aa7b2f645511cd34e40948ab1 Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Sat, 18 Apr 2026 17:12:02 -0400
Subject: [PATCH] Address PR #1212 code review feedback

- Fix hasAnyAnnotation checking annotations.title which is never rendered
- Reset formValues when switching tools via onSelectTool handler
- Extract ToolProgress type from ToolDetailPanel, reuse in ToolsScreen
- Fix contradictory readOnlyHint + destructiveHint in story fixture
- Replace listMaxHeight function with LIST_MAX_HEIGHT constant

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 .../groups/ToolControls/ToolControls.tsx      |  7 +++----
 .../ToolDetailPanel.stories.tsx               |  1 -
 .../ToolDetailPanel/ToolDetailPanel.tsx       | 13 ++++++------
 .../screens/ToolsScreen/ToolsScreen.tsx       | 21 +++++++++----------
 4 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/clients/web/src/components/groups/ToolControls/ToolControls.tsx b/clients/web/src/components/groups/ToolControls/ToolControls.tsx
index cc533276..8779e617 100644
--- a/clients/web/src/components/groups/ToolControls/ToolControls.tsx
+++ b/clients/web/src/components/groups/ToolControls/ToolControls.tsx
@@ -12,9 +12,8 @@ export interface ToolControlsProps {
   onSelectTool: (name: string) => void;
 }
 
-function listMaxHeight(): string {
-  return "calc(100vh - var(--app-shell-header-height, 0px) - var(--mantine-spacing-xl) * 2 - 160px)";
-}
+const LIST_MAX_HEIGHT =
+  "calc(100vh - var(--app-shell-header-height, 0px) - var(--mantine-spacing-xl) * 2 - 160px)";
 
 export function ToolControls({
   tools,
@@ -44,7 +43,7 @@ export function ToolControls({
         value={searchText}
         onChange={(e) => setSearchText(e.currentTarget.value)}
       />
-      <ScrollArea.Autosize mah={listMaxHeight()}>
+      <ScrollArea.Autosize mah={LIST_MAX_HEIGHT}>
         <Stack gap="xs">
           {filteredTools.map((tool) => (
             <ToolListItem
diff --git a/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.stories.tsx b/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.stories.tsx
index 9760b8c1..9dbee5da 100644
--- a/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.stories.tsx
+++ b/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.stories.tsx
@@ -50,7 +50,6 @@ const deleteRecordsTool: Tool = {
   name: "delete_records",
   description: "Deletes records matching the given criteria",
   annotations: {
-    readOnlyHint: true,
     destructiveHint: true,
   },
   inputSchema: {
diff --git a/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx b/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx
index 72b5f1c3..3cd88f7c 100644
--- a/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx
+++ b/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx
@@ -8,14 +8,16 @@ import { AnnotationBadge } from "../../elements/AnnotationBadge/AnnotationBadge"
 import { ProgressDisplay } from "../../elements/ProgressDisplay/ProgressDisplay";
 import { SchemaForm } from "../SchemaForm/SchemaForm";
 
+export type ToolProgress = Pick<
+  ProgressNotification["params"],
+  "progress" | "total" | "message"
+>;
+
 export interface ToolDetailPanelProps {
   tool: Tool;
   formValues: Record<string, unknown>;
   isExecuting: boolean;
-  progress?: Pick<
-    ProgressNotification["params"],
-    "progress" | "total" | "message"
-  >;
+  progress?: ToolProgress;
   onFormChange: (values: Record<string, unknown>) => void;
   onExecute: () => void;
   onCancel: () => void;
@@ -44,8 +46,7 @@ function resolveTitle(name: string, title?: string): string {
 function hasAnyAnnotation(annotations?: ToolAnnotations): boolean {
   return !!(
     annotations &&
-    (annotations.title ||
-      annotations.readOnlyHint ||
+    (annotations.readOnlyHint ||
       annotations.destructiveHint ||
       annotations.idempotentHint ||
       annotations.openWorldHint)
diff --git a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx
index fbfdad05..e4458eee 100644
--- a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx
+++ b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx
@@ -1,22 +1,18 @@
 import { useState } from "react";
 import { Card, Flex, Stack, Text } from "@mantine/core";
-import type {
-  CallToolResult,
-  ProgressNotification,
-  Tool,
-} from "@modelcontextprotocol/sdk/types.js";
+import type { CallToolResult, Tool } from "@modelcontextprotocol/sdk/types.js";
 import { ToolControls } from "../../groups/ToolControls/ToolControls";
-import { ToolDetailPanel } from "../../groups/ToolDetailPanel/ToolDetailPanel";
+import {
+  ToolDetailPanel,
+  type ToolProgress,
+} from "../../groups/ToolDetailPanel/ToolDetailPanel";
 import { ToolResultPanel } from "../../groups/ToolResultPanel/ToolResultPanel";
 
 export interface ToolCallState {
   status: "idle" | "pending" | "ok" | "error";
   result?: CallToolResult;
   error?: string;
-  progress?: Pick<
-    ProgressNotification["params"],
-    "progress" | "total" | "message"
-  >;
+  progress?: ToolProgress;
 }
 
 export interface ToolsScreenProps {
@@ -86,7 +82,10 @@ export function ToolsScreen({
             selectedName={selectedToolName}
             listChanged={listChanged}
             onRefreshList={onRefreshList}
-            onSelectTool={onSelectTool}
+            onSelectTool={(name) => {
+              setFormValues({});
+              onSelectTool(name);
+            }}
           />
         </SidebarCard>
       </Sidebar>