From 0556358560f065dec5e35fd5b0544b18ddcc4495 Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Fri, 2 May 2025 14:39:39 -0700 Subject: [PATCH] Cleanup low value comments. (#248) --- .../cli/src/ui/components/InputPrompt.tsx | 2 +- .../src/ui/components/SuggestionsDisplay.tsx | 7 +------ .../components/messages/ToolGroupMessage.tsx | 3 --- packages/cli/src/ui/hooks/useGeminiStream.ts | 6 +++--- .../cli/src/ui/utils/MarkdownRenderer.tsx | 1 - packages/server/src/core/client.ts | 2 +- packages/server/src/core/turn.ts | 19 +++++-------------- packages/server/src/tools/edit.ts | 6 +++--- packages/server/src/tools/glob.ts | 9 ++++----- packages/server/src/tools/tools.ts | 1 - packages/server/src/tools/web-fetch.ts | 7 ++----- packages/server/src/tools/write-file.ts | 19 ++++++------------- .../src/utils/BackgroundTerminalAnalyzer.ts | 15 ++------------- packages/server/src/utils/errors.ts | 2 -- .../server/src/utils/getFolderStructure.ts | 7 ++----- packages/server/src/utils/paths.ts | 2 +- packages/server/src/utils/schemaValidator.ts | 1 - 17 files changed, 31 insertions(+), 78 deletions(-) diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index 1c8cff4d..a7f06bce 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -17,7 +17,7 @@ interface InputPromptProps { setInputKey: React.Dispatch>; onSubmit: (value: string) => void; showSuggestions: boolean; - suggestions: Suggestion[]; // Changed to Suggestion[] + suggestions: Suggestion[]; activeSuggestionIndex: number; navigateUp: () => void; navigateDown: () => void; diff --git a/packages/cli/src/ui/components/SuggestionsDisplay.tsx b/packages/cli/src/ui/components/SuggestionsDisplay.tsx index 8c9cf377..02dea70e 100644 --- a/packages/cli/src/ui/components/SuggestionsDisplay.tsx +++ b/packages/cli/src/ui/components/SuggestionsDisplay.tsx @@ -48,12 +48,7 @@ export function SuggestionsDisplay({ const visibleSuggestions = suggestions.slice(startIndex, endIndex); return ( - + {scrollOffset > 0 && ▲} {visibleSuggestions.map((suggestion, index) => { diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index 2d4982c2..401b8ee0 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -66,9 +66,6 @@ export const ToolGroupMessage: React.FC = ({ )} ))} - {/* Optional: Add padding below the last item if needed, - though ToolMessage already has some vertical space implicitly */} - {/* {tools.length > 0 && } */} ); }; diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index 9e2901ec..bd2b617c 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -27,12 +27,12 @@ import { IndividualToolCallDisplay, ToolCallStatus, } from '../types.js'; -import { isAtCommand } from '../utils/commandUtils.js'; // Import the @ command checker +import { isAtCommand } from '../utils/commandUtils.js'; import { useSlashCommandProcessor } from './slashCommandProcessor.js'; import { useShellCommandProcessor } from './shellCommandProcessor.js'; import { usePassthroughProcessor } from './passthroughCommandProcessor.js'; -import { handleAtCommand } from './atCommandProcessor.js'; // Import the @ command handler -import { findSafeSplitPoint } from '../utils/markdownUtilities.js'; // Import the split point finder +import { handleAtCommand } from './atCommandProcessor.js'; +import { findSafeSplitPoint } from '../utils/markdownUtilities.js'; const addHistoryItem = ( setHistory: React.Dispatch>, diff --git a/packages/cli/src/ui/utils/MarkdownRenderer.tsx b/packages/cli/src/ui/utils/MarkdownRenderer.tsx index c9053728..6441dcb6 100644 --- a/packages/cli/src/ui/utils/MarkdownRenderer.tsx +++ b/packages/cli/src/ui/utils/MarkdownRenderer.tsx @@ -24,7 +24,6 @@ export class MarkdownRenderer { private static _renderInline(text: string): React.ReactNode[] { const nodes: React.ReactNode[] = []; let lastIndex = 0; - // UPDATED Regex: Added .*?<\/u> pattern const inlineRegex = /(\*\*.*?\*\*|\*.*?\*|_.*?_|~~.*?~~|\[.*?\]\(.*?\)|`+.+?`+|.*?<\/u>)/g; let match; diff --git a/packages/server/src/core/client.ts b/packages/server/src/core/client.ts index d4bcdeca..29712db5 100644 --- a/packages/server/src/core/client.ts +++ b/packages/server/src/core/client.ts @@ -19,7 +19,7 @@ import { getFolderStructure } from '../utils/getFolderStructure.js'; import { Turn, ServerGeminiStreamEvent } from './turn.js'; import { Config } from '../config/config.js'; import { getCoreSystemPrompt } from './prompts.js'; -import { ReadManyFilesTool } from '../tools/read-many-files.js'; // Import ReadManyFilesTool +import { ReadManyFilesTool } from '../tools/read-many-files.js'; import { getResponseText } from '../utils/generateContentResponseUtilities.js'; export class GeminiClient { diff --git a/packages/server/src/core/turn.ts b/packages/server/src/core/turn.ts index 47ca051b..ad461d74 100644 --- a/packages/server/src/core/turn.ts +++ b/packages/server/src/core/turn.ts @@ -12,14 +12,12 @@ import { FunctionCall, FunctionDeclaration, } from '@google/genai'; -// Removed UI type imports import { ToolCallConfirmationDetails, ToolResult, ToolResultDisplay, -} from '../tools/tools.js'; // Keep ToolResult for now +} from '../tools/tools.js'; import { getResponseText } from '../utils/generateContentResponseUtilities.js'; -// Removed gemini-stream import (types defined locally) // --- Types for Server Logic --- @@ -27,7 +25,7 @@ import { getResponseText } from '../utils/generateContentResponseUtilities.js'; interface ServerToolExecutionOutcome { callId: string; name: string; - args: Record; // Use unknown for broader compatibility + args: Record; result?: ToolResult; error?: Error; confirmationDetails: ToolCallConfirmationDetails | undefined; @@ -36,16 +34,14 @@ interface ServerToolExecutionOutcome { // Define a structure for tools passed to the server export interface ServerTool { name: string; - schema: FunctionDeclaration; // Schema is needed + schema: FunctionDeclaration; // The execute method signature might differ slightly or be wrapped execute(params: Record): Promise; shouldConfirmExecute( params: Record, ): Promise; - // validation and description might be handled differently or passed } -// Redefine necessary event types locally export enum GeminiEventType { Content = 'content', ToolCallRequest = 'tool_call_request', @@ -80,15 +76,13 @@ export type ServerGeminiStreamEvent = value: ServerToolCallConfirmationDetails; }; -// --- Turn Class (Refactored for Server) --- - // A turn manages the agentic loop turn within the server context. export class Turn { - private readonly availableTools: Map; // Use passed-in tools + private readonly availableTools: Map; private pendingToolCalls: Array<{ callId: string; name: string; - args: Record; // Use unknown + args: Record; }>; private fnResponses: Part[]; private confirmationDetails: ToolCallConfirmationDetails[]; @@ -206,7 +200,6 @@ export class Turn { } } - // Generates a ToolCallRequest event to signal the need for execution private handlePendingFunctionCall( fnCall: FunctionCall, ): ServerGeminiStreamEvent | null { @@ -257,12 +250,10 @@ export class Turn { return this.confirmationDetails; } - // Allows the service layer to get the responses needed for the next API call getFunctionResponses(): Part[] { return this.fnResponses; } - // Debugging information (optional) getDebugResponses(): GenerateContentResponse[] { return this.debugResponses; } diff --git a/packages/server/src/tools/edit.ts b/packages/server/src/tools/edit.ts index 3b317a08..72c25603 100644 --- a/packages/server/src/tools/edit.ts +++ b/packages/server/src/tools/edit.ts @@ -53,10 +53,10 @@ interface CalculatedEdit { } /** - * Implementation of the Edit tool logic (moved from CLI) + * Implementation of the Edit tool logic */ export class EditTool extends BaseTool { - static readonly Name = 'replace'; // Keep static name + static readonly Name = 'replace'; private shouldAlwaysEdit = false; /** @@ -371,7 +371,7 @@ export class EditTool extends BaseTool { editData.newContent, 'Current', 'Proposed', - { context: 3 }, // Removed ignoreWhitespace for potentially more accurate display diff + { context: 3 }, ); displayResult = { fileDiff }; } diff --git a/packages/server/src/tools/glob.ts b/packages/server/src/tools/glob.ts index d9d91c7a..9e7df0e8 100644 --- a/packages/server/src/tools/glob.ts +++ b/packages/server/src/tools/glob.ts @@ -27,11 +27,10 @@ export interface GlobToolParams { } /** - * Implementation of the Glob tool logic (moved from CLI) + * Implementation of the Glob tool logic */ export class GlobTool extends BaseTool { - static readonly Name = 'glob'; // Keep static name - + static readonly Name = 'glob'; /** * Creates a new instance of the GlobLogic * @param rootDirectory Root directory to ground this tool in. @@ -39,8 +38,8 @@ export class GlobTool extends BaseTool { constructor(private rootDirectory: string) { super( GlobTool.Name, - 'FindFiles', // Display name handled by CLI wrapper - 'Efficiently finds files matching specific glob patterns (e.g., `src/**/*.ts`, `**/*.md`), returning absolute paths sorted by modification time (newest first). Ideal for quickly locating files based on their name or path structure, especially in large codebases.', // Description handled by CLI wrapper + 'FindFiles', + 'Efficiently finds files matching specific glob patterns (e.g., `src/**/*.ts`, `**/*.md`), returning absolute paths sorted by modification time (newest first). Ideal for quickly locating files based on their name or path structure, especially in large codebases.', { properties: { pattern: { diff --git a/packages/server/src/tools/tools.ts b/packages/server/src/tools/tools.ts index 57a388d8..ac04450d 100644 --- a/packages/server/src/tools/tools.ts +++ b/packages/server/src/tools/tools.ts @@ -5,7 +5,6 @@ */ import { FunctionDeclaration, Schema } from '@google/genai'; -// Removed import for ../ui/types.js as confirmation is UI-specific /** * Interface representing the base Tool functionality diff --git a/packages/server/src/tools/web-fetch.ts b/packages/server/src/tools/web-fetch.ts index 415dc033..12584231 100644 --- a/packages/server/src/tools/web-fetch.ts +++ b/packages/server/src/tools/web-fetch.ts @@ -19,7 +19,7 @@ export interface WebFetchToolParams { } /** - * Implementation of the WebFetch tool logic (moved from CLI) + * Implementation of the WebFetch tool logic */ export class WebFetchTool extends BaseTool { static readonly Name: string = 'web_fetch'; @@ -70,8 +70,6 @@ export class WebFetchTool extends BaseTool { return `Fetching content from ${displayUrl}`; } - // Removed shouldConfirmExecute - handled by CLI - async execute(params: WebFetchToolParams): Promise { const validationError = this.validateParams(params); if (validationError) { @@ -86,10 +84,9 @@ export class WebFetchTool extends BaseTool { try { const response = await fetch(url, { headers: { - // Identify the client making the request 'User-Agent': 'GeminiCode-ServerLogic/1.0', }, - signal: AbortSignal.timeout(15000), // Use AbortSignal for timeout + signal: AbortSignal.timeout(15000), }); if (!response.ok) { diff --git a/packages/server/src/tools/write-file.ts b/packages/server/src/tools/write-file.ts index d24080d2..c9a47296 100644 --- a/packages/server/src/tools/write-file.ts +++ b/packages/server/src/tools/write-file.ts @@ -6,7 +6,7 @@ import fs from 'fs'; import path from 'path'; -import * as Diff from 'diff'; // Keep for result generation +import * as Diff from 'diff'; import { BaseTool, ToolResult, @@ -14,11 +14,10 @@ import { ToolEditConfirmationDetails, ToolConfirmationOutcome, ToolCallConfirmationDetails, -} from './tools.js'; // Updated import (Removed ToolResultDisplay) +} from './tools.js'; import { SchemaValidator } from '../utils/schemaValidator.js'; // Updated import import { makeRelative, shortenPath } from '../utils/paths.js'; // Updated import -import { isNodeError } from '../utils/errors.js'; // Import isNodeError - +import { isNodeError } from '../utils/errors.js'; /** * Parameters for the WriteFile tool */ @@ -35,7 +34,7 @@ export interface WriteFileToolParams { } /** - * Implementation of the WriteFile tool logic (moved from CLI) + * Implementation of the WriteFile tool logic */ export class WriteFileTool extends BaseTool { static readonly Name: string = 'write_file'; @@ -49,7 +48,6 @@ export class WriteFileTool extends BaseTool { { properties: { file_path: { - // Renamed from filePath in original schema description: "The absolute path to the file to write to (e.g., '/home/user/project/file.txt'). Relative paths are not supported.", type: 'string', @@ -59,7 +57,7 @@ export class WriteFileTool extends BaseTool { type: 'string', }, }, - required: ['file_path', 'content'], // Use correct param names + required: ['file_path', 'content'], type: 'object', }, ); @@ -97,15 +95,13 @@ export class WriteFileTool extends BaseTool { return null; } - // Removed shouldConfirmExecute - handled by CLI - getDescription(params: WriteFileToolParams): string { const relativePath = makeRelative(params.file_path, this.rootDirectory); return `Writing to ${shortenPath(relativePath)}`; } /** - * Handles the confirmation prompt for the WriteFile tool in the CLI. + * Handles the confirmation prompt for the WriteFile tool. */ async shouldConfirmExecute( params: WriteFileToolParams, @@ -203,7 +199,6 @@ export class WriteFileTool extends BaseTool { ? `Successfully created and wrote to new file: ${params.file_path}` : `Successfully overwrote file: ${params.file_path}`; - // The returnDisplay contains the diff const displayResult: FileDiff = { fileDiff }; return { @@ -218,6 +213,4 @@ export class WriteFileTool extends BaseTool { }; } } - - // ensureParentDirectoriesExist logic moved into execute } diff --git a/packages/server/src/utils/BackgroundTerminalAnalyzer.ts b/packages/server/src/utils/BackgroundTerminalAnalyzer.ts index 06cefd9b..ebb60aad 100644 --- a/packages/server/src/utils/BackgroundTerminalAnalyzer.ts +++ b/packages/server/src/utils/BackgroundTerminalAnalyzer.ts @@ -12,7 +12,6 @@ import { promises as fs } from 'fs'; import { exec as _exec } from 'child_process'; import { promisify } from 'util'; -// Define the AnalysisStatus type alias type AnalysisStatus = | 'Running' | 'SuccessReported' @@ -20,7 +19,6 @@ type AnalysisStatus = | 'Unknown' | 'AnalysisFailed'; -// Promisify child_process.exec for easier async/await usage const execAsync = promisify(_exec); // Identifier for the background process (e.g., PID) @@ -33,20 +31,17 @@ export interface AnalysisResult { inferredStatus: 'Running' | 'SuccessReported' | 'ErrorReported' | 'Unknown'; } -// Represents the structure returned when the LLM analysis itself fails export interface AnalysisFailure { error: string; inferredStatus: 'AnalysisFailed'; } -// Type guard to check if the result is a failure object function isAnalysisFailure( result: AnalysisResult | AnalysisFailure, ): result is AnalysisFailure { return (result as AnalysisFailure).inferredStatus === 'AnalysisFailed'; } -// Represents the final outcome after polling is complete (or failed/timed out) export interface FinalAnalysisOutcome { status: string; // e.g., 'Completed_SuccessReported', 'TimedOut_Running', 'AnalysisFailed' summary: string; // Final summary or error message @@ -60,7 +55,7 @@ export class BackgroundTerminalAnalyzer { private initialDelayMs: number; constructor( - config: Config, // Accept Config object + config: Config, options: { pollIntervalMs?: number; maxAttempts?: number; @@ -68,7 +63,6 @@ export class BackgroundTerminalAnalyzer { } = {}, ) { try { - // Initialize Gemini client using config this.geminiClient = new GeminiClient(config); } catch (error) { console.error( @@ -262,7 +256,6 @@ export class BackgroundTerminalAnalyzer { return { status: finalStatus, summary: finalSummary }; } - // --- Actual Implementation of isProcessRunning --- /** * Checks if the background process is still running using OS-specific methods. * @param pid Process handle/identifier (expects a number for standard checks). @@ -312,7 +305,6 @@ export class BackgroundTerminalAnalyzer { } } - // --- LLM Analysis Method (largely unchanged but added validation robustness) --- private async performLlmAnalysis( stdoutContent: string, stderrContent: string, @@ -433,7 +425,6 @@ Based *only* on the provided stdout and stderr: 'Unknown', ]; - // Cast the unknown value to string before checking with includes const statusString = resultJson?.inferredStatus as string; const inferredStatus = validStatuses.includes( statusString as Exclude, @@ -441,15 +432,13 @@ Based *only* on the provided stdout and stderr: ? (statusString as Exclude) : 'Unknown'; - // Explicitly construct the object matching AnalysisResult type const analysisResult: AnalysisResult = { summary, inferredStatus }; return analysisResult; } catch (error: unknown) { console.error(`LLM Analysis Request Failed for PID ${pid}:`, error); - // Return the AnalysisFailure type const analysisFailure: AnalysisFailure = { error: `[Analysis failed: ${getErrorMessage(error)}]`, - inferredStatus: 'AnalysisFailed', // This matches the AnalysisStatus type + inferredStatus: 'AnalysisFailed', }; return analysisFailure; } diff --git a/packages/server/src/utils/errors.ts b/packages/server/src/utils/errors.ts index 15417c83..32139c1a 100644 --- a/packages/server/src/utils/errors.ts +++ b/packages/server/src/utils/errors.ts @@ -12,12 +12,10 @@ export function getErrorMessage(error: unknown): string { if (error instanceof Error) { return error.message; } else { - // Attempt to convert the non-Error value to a string for logging try { const errorMessage = String(error); return errorMessage; } catch { - // If String() itself fails (highly unlikely) return 'Failed to get error details'; } } diff --git a/packages/server/src/utils/getFolderStructure.ts b/packages/server/src/utils/getFolderStructure.ts index 0caabe0f..1f7f0fcd 100644 --- a/packages/server/src/utils/getFolderStructure.ts +++ b/packages/server/src/utils/getFolderStructure.ts @@ -66,7 +66,6 @@ async function readFullStructure( options: MergedFolderStructureOptions, ): Promise { const name = path.basename(folderPath); - // Initialize with isIgnored: false const folderInfo: Omit = { name, path: folderPath, @@ -97,7 +96,7 @@ async function readFullStructure( subFolders: [], totalChildren: 0, // No children explored totalFiles: 0, // No files explored - isIgnored: true, // Mark as ignored + isIgnored: true, }; folderInfo.subFolders.push(ignoredFolderInfo); // Skip recursion for this folder @@ -122,7 +121,6 @@ async function readFullStructure( for (const entry of entries) { if (entry.isFile()) { const fileName = entry.name; - // Include if no pattern or if pattern matches if ( !options.fileIncludePattern || options.fileIncludePattern.test(fileName) @@ -156,7 +154,7 @@ async function readFullStructure( } return { - ...(folderInfo as FullFolderInfo), // Cast needed after conditional assignment check + ...folderInfo, totalChildren: totalChildrenCount, totalFiles: totalFileCount, }; @@ -285,7 +283,6 @@ function countReducedItems(node: ReducedFolderNode): number { /** * Formats the reduced folder structure into a tree-like string. - * (No changes needed in this function) * @param node The current node in the reduced structure. * @param indent The current indentation string. * @param isLast Sibling indicator. diff --git a/packages/server/src/utils/paths.ts b/packages/server/src/utils/paths.ts index 6da3d4ab..4ff74897 100644 --- a/packages/server/src/utils/paths.ts +++ b/packages/server/src/utils/paths.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import path from 'node:path'; // Import the 'path' module +import path from 'node:path'; /** * Shortens a path string if it exceeds maxLen, prioritizing the start and end segments. diff --git a/packages/server/src/utils/schemaValidator.ts b/packages/server/src/utils/schemaValidator.ts index 107ccc85..34ed5843 100644 --- a/packages/server/src/utils/schemaValidator.ts +++ b/packages/server/src/utils/schemaValidator.ts @@ -6,7 +6,6 @@ /** * Simple utility to validate objects against JSON Schemas - * In a real implementation, you would use a library like Ajv */ export class SchemaValidator { /**