Skip to content

Improve prompts for cross os work#170

Merged
wonderwhy-er merged 56 commits into
mainfrom
improve-prompts-for-cross-os-work
Jul 2, 2025
Merged

Improve prompts for cross os work#170
wonderwhy-er merged 56 commits into
mainfrom
improve-prompts-for-cross-os-work

Conversation

@wonderwhy-er

@wonderwhy-er wonderwhy-er commented Jul 2, 2025

Copy link
Copy Markdown
Owner

Summary of improvements

🖥️ Platform-Specific Prompts so that chatbot can run right platform specific commands faster

Dynamic OS detection: Automatically detects Windows/macOS/Linux and adjusts tool descriptions
Platform-specific guidance: Windows PowerShell vs CMD troubleshooting, macOS Homebrew notes, Linux package manager guidance
Smart shell selection: Better default shell handling per platform

⚡ Enhanced Process Management

Faster REPL detection: Immediate recognition of Python/Node.js prompts (>>>, >, $)
Early exit optimization: Eliminates unnecessary timeouts when processes are ready for input
Improved state detection: Better analysis of when processes are waiting vs finished
Enhanced output formatting: Cleaner process responses with consistent status indicators

🔗 SSH Improvements

Auto-enhancement: Automatically adds -t flag to SSH commands for better terminal compatibility
Better terminal support: Improved TERM environment variable handling

🔍 Code Search Fixes

Result capping: Limits search results to 900KB to prevent API timeouts
Progressive truncation: Shows partial results with helpful truncation messages instead of failing completely
Performance boost: No more "results too big" errors - always returns useful data

- test_enhanced_read.js: Tests enhanced file reading with offset/length parameters
- test_improved_search_truncation.js: Tests improved search result truncation
- test_search_truncation.js: Tests basic search result truncation

These files are automatically included in the test suite via run-all-tests.js
…k' into improve-prompts-for-cross-os-work

# Conflicts:
#	src/server.ts
#	src/tools/filesystem.ts
#	src/tools/improved-process-tools.ts
#	test/test_output/node_repl_debug.txt
#	test/test_output/repl_test_output.txt
@coderabbitai

coderabbitai Bot commented Jul 2, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The changes introduce enhanced system-aware guidance and status messaging, improved file and process handling, and new utility modules. File operation constants and utilities are consolidated, search result truncation is made safer, and process interaction is more responsive. Multiple new and extended tests validate large file handling, search truncation, and REPL interaction. No exported function signatures are changed.

Changes

File(s) Change Summary
src/handlers/edit-search-handlers.ts Improved search result capping and truncation logic, increasing default and max limits, and introducing output size-based truncation.
src/server.ts Tool descriptions now use dynamic, system-aware guidance; redundant warnings removed; system info added to config tool.
src/terminal-manager.ts SSH command preprocessing, environment variable setup, and prompt detection improved; output and session handling refined.
src/tools/config.ts getConfig now includes detailed system info from new utility module.
src/tools/edit.ts Uses validated file path for reading during search-replace operations.
src/tools/filesystem.ts Refactored to use centralized constants/utilities for timeouts, limits, MIME, line counting, and enhanced status messages.
src/tools/improved-process-tools.ts Process prompt detection is now event-driven; shell info included in process start; output formatting improved.
src/utils/system-info.ts New module: Provides platform detection, system info, and OS-specific guidance utilities.
test/modified-test.js New test: Measures performance and correctness of editing large files with different line endings.
test/test-enhanced-repl.js Extended tests for enhanced REPL interaction, including prompt detection and multi-line code.
test/test_enhanced_read.js New test: Validates enhanced file reading for various offsets and lengths.
test/test_improved_search_truncation.js, test/test_search_truncation.js New tests: Validate improved search result truncation logic and output safety.
test/test_output/node_repl_debug.txt, test/test_output/repl_test_output.txt New test outputs: Capture expected REPL session outputs for Python and Node.js.
.gitignore Added plans/ directory to ignore list.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Server
    participant SystemInfoUtil
    participant FileSystemUtil
    participant TerminalManager
    participant ProcessTools

    User->>Server: Request tool list/config
    Server->>SystemInfoUtil: getSystemInfo()
    SystemInfoUtil-->>Server: System info & guidance
    Server-->>User: Tool list/config with dynamic guidance

    User->>Server: Search code / edit file / run process
    Server->>FileSystemUtil: Validate path, read file, search files
    FileSystemUtil-->>Server: Results with enhanced status/truncation
    Server->>TerminalManager: Execute command (e.g., SSH)
    TerminalManager->>ProcessTools: Start/interact with process
    ProcessTools-->>TerminalManager: Output with prompt detection
    TerminalManager-->>Server: Command/process results
    Server-->>User: Final response
Loading

Possibly related PRs

  • Allow reading urls and some cleanup #63: Modifies handleSearchCode for timeout handling and process management, closely related to the current PR's search result capping and truncation improvements.
  • File write read improvements #150: Refactors file reading functions and introduces readFileInternal, directly related to the current PR's use and enhancement of file reading and validation logic.

Suggested labels

size:L

Suggested reviewers

  • serg33v

Poem

🐇
A hop and a skip through the code I go,
System-aware guidance now on show!
Search results trimmed, no more a fright,
File reads and prompts now work just right.
With tests that leap and outputs neat,
This bunny’s work is quite complete!
🥕

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🔭 Outside diff range comments (1)
test/test-enhanced-repl.js (1)

12-28: Improve error handling for process startup failures.

The test assumes that the Python process will start successfully but doesn't handle potential startup failures gracefully.

   // Start Python in interactive mode
   console.log('Starting Python REPL...');
-  const result = await startProcess({
-    command: 'python -i',
-    timeout_ms: 10000
-  });
+  let result;
+  try {
+    result = await startProcess({
+      command: 'python -i',
+      timeout_ms: 10000
+    });
+  } catch (error) {
+    console.error('Failed to start Python process:', error);
+    return false;
+  }
   
   console.log('Result from start_process:', result);
   
   // Extract PID from the result text
   const pidMatch = result.content[0].text.match(/Process started with PID (\d+)/);
   const pid = pidMatch ? parseInt(pidMatch[1]) : null;
   
   if (!pid) {
-    console.error("Failed to get PID from Python process");
+    console.error("Failed to get PID from Python process. Output:", result.content[0].text);
     return false;
   }
♻️ Duplicate comments (2)
test/test_enhanced_read.js (2)

17-17: Same hardcoded path issue as above.


24-24: Same hardcoded path issue as above.

🧹 Nitpick comments (9)
test/test_enhanced_read.js (1)

4-35: Consider adding error handling improvements.

The test function has basic error handling, but could benefit from more specific error reporting and test assertions.

Consider enhancing the test with:

  • Specific assertions for expected content
  • More detailed error messages indicating which test failed
  • Verification that the test file exists before running tests
src/tools/config.ts (2)

13-27: Refactor: Eliminate redundant destructuring and reconstruction.

The system information is being destructured from getSystemInfo() and then immediately reconstructed into a new object. This is unnecessary and can be simplified.

-    // Add system information to the config response
-    const systemInfo = getSystemInfo();
-    const configWithSystemInfo = {
-      ...config,
-      systemInfo: {
-        platform: systemInfo.platform,
-        platformName: systemInfo.platformName,
-        defaultShell: systemInfo.defaultShell,
-        pathSeparator: systemInfo.pathSeparator,
-        isWindows: systemInfo.isWindows,
-        isMacOS: systemInfo.isMacOS,
-        isLinux: systemInfo.isLinux,
-        examplePaths: systemInfo.examplePaths
-      }
-    };
+    // Add system information to the config response
+    const configWithSystemInfo = {
+      ...config,
+      systemInfo: getSystemInfo()
+    };

6-6: Update function comment to be more descriptive.

The comment should better reflect that this function enriches the config with system information.

-/**
- * Get the entire config including system information
- */
+/**
+ * Get the entire configuration enriched with system information
+ */
test/test_search_truncation.js (1)

22-30: Improve test result validation logic.

The current validation logic could be more robust and provide better feedback about what actually happened.

-        if (result.content[0].text.length > 1000000) {
-            console.log('❌ Results not truncated - this would exceed Claude limits');
-        } else if (result.content[0].text.includes('Results truncated')) {
-            console.log('✅ Results properly truncated with warning message');
-            const truncationIndex = result.content[0].text.indexOf('Results truncated');
-            console.log('Truncation message:', result.content[0].text.substring(truncationIndex, truncationIndex + 100));
-        } else {
-            console.log('✅ Results under limit, no truncation needed');
-        }
+        const resultLength = result.content[0].text.length;
+        const isTruncated = result.content[0].text.includes('Results truncated');
+        
+        if (resultLength > 1000000) {
+            console.log(`❌ Results not truncated - ${resultLength.toLocaleString()} chars exceed Claude limits`);
+        } else if (isTruncated) {
+            console.log(`✅ Results properly truncated at ${resultLength.toLocaleString()} chars`);
+            const truncationIndex = result.content[0].text.indexOf('Results truncated');
+            console.log('Truncation message:', result.content[0].text.substring(truncationIndex, truncationIndex + 100));
+        } else {
+            console.log(`✅ Results under limit (${resultLength.toLocaleString()} chars), no truncation needed`);
+        }
test/test_improved_search_truncation.js (1)

4-4: Consider consolidating duplicate test files.

This test file appears to be very similar to test_search_truncation.js with minor differences. Consider consolidating them to reduce maintenance overhead.

The two test files (test_search_truncation.js and test_improved_search_truncation.js) have significant overlap. Consider merging them into a single comprehensive test file that covers all the test scenarios.

test/test-enhanced-repl.js (1)

104-110: Add error handling for process termination.

The process termination should be wrapped in error handling to ensure the test doesn't fail if termination encounters issues.

   // Terminate the session
   console.log("Terminating session...");
-  await forceTerminate({ pid });
-  console.log('Python session terminated');
+  try {
+    await forceTerminate({ pid });
+    console.log('Python session terminated');
+  } catch (error) {
+    console.warn('Failed to terminate process cleanly:', error);
+    // Don't fail the test for termination issues
+  }
src/tools/improved-process-tools.ts (2)

143-144: Use optional chaining for safer property access.

The condition can be simplified using optional chaining for better readability and safety.

-      const session = terminalManager.getSession(pid);
-      if (session && session.process && session.process.stdout && session.process.stderr) {
+      const session = terminalManager.getSession(pid);
+      if (session?.process?.stdout && session?.process?.stderr) {

323-324: Use optional chaining for safer property access.

Similar to the previous function, optional chaining would improve safety and readability.

-        const session = terminalManager.getSession(pid);
-        if (session && session.process && session.process.stdout && session.process.stderr) {
+        const session = terminalManager.getSession(pid);
+        if (session?.process?.stdout && session?.process?.stderr) {
src/tools/filesystem.ts (1)

30-89: Well-designed utility functions that effectively eliminate code duplication.

The extraction of these common operations into dedicated functions improves code organization and maintainability. The size check in getFileLineCount is a thoughtful performance optimization.

One minor consideration: The error handling in getFileLineCount (lines 54-56) silently returns undefined for all errors. While this may be intentional, consider at least logging permission errors or other unexpected failures for debugging purposes.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14baa9a and 2a898e0.

📒 Files selected for processing (16)
  • .gitignore (1 hunks)
  • src/handlers/edit-search-handlers.ts (2 hunks)
  • src/server.ts (8 hunks)
  • src/terminal-manager.ts (4 hunks)
  • src/tools/config.ts (1 hunks)
  • src/tools/edit.ts (1 hunks)
  • src/tools/filesystem.ts (29 hunks)
  • src/tools/improved-process-tools.ts (6 hunks)
  • src/utils/system-info.ts (1 hunks)
  • test/modified-test.js (1 hunks)
  • test/test-enhanced-repl.js (1 hunks)
  • test/test_enhanced_read.js (1 hunks)
  • test/test_improved_search_truncation.js (1 hunks)
  • test/test_output/node_repl_debug.txt (1 hunks)
  • test/test_output/repl_test_output.txt (1 hunks)
  • test/test_search_truncation.js (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
test/modified-test.js (1)
test/test.js (1)
  • fs (41-41)
src/server.ts (1)
src/utils/system-info.ts (4)
  • getSystemInfo (21-81)
  • getOSSpecificGuidance (86-125)
  • getDevelopmentToolGuidance (130-158)
  • getPathGuidance (163-165)
src/tools/config.ts (2)
src/config-manager.ts (2)
  • getConfig (148-151)
  • configManager (212-212)
src/utils/system-info.ts (1)
  • getSystemInfo (21-81)
test/test_search_truncation.js (2)
test/test_improved_search_truncation.js (3)
  • searchArgs (9-14)
  • result (18-18)
  • truncationIndex (36-36)
src/handlers/edit-search-handlers.ts (1)
  • handleSearchCode (25-112)
🪛 LanguageTool
test/test_output/node_repl_debug.txt

[style] ~49-~49: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...> ... ... Hello, User 0! Hello, User 1! Hello, User 2! undefined > > �[0m �[34mFound ...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)


[style] ~53-~53: Using many exclamation marks might seem excessive (in this case: 11 exclamation marks for a text that’s 1399 characters long)
Context: ... > > �[0m �[34mFound "Hello from Node.js!": true�[0m �[34mFound greetings: true�[...

(EN_EXCESSIVE_EXCLAMATION)

🪛 Biome (1.9.4)
src/tools/improved-process-tools.ts

[error] 144-144: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


[error] 324-324: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🔇 Additional comments (25)
.gitignore (1)

41-42: LGTM! Good practice to exclude local planning directories.

The addition of plans/ to .gitignore with a descriptive comment follows best practices for keeping local documentation out of version control.

src/tools/edit.ts (1)

125-125: LGTM! Improved path handling consistency.

Using validPath instead of the raw filePath ensures consistency with the updated file system utilities and improves security by using the validated path.

test/test_output/repl_test_output.txt (1)

1-15: LGTM! Useful test documentation.

This test output file provides clear documentation of expected REPL behavior for both Python and Node.js, which is valuable for testing and development reference.

test/test_output/node_repl_debug.txt (1)

1-57: LGTM! Comprehensive REPL interaction documentation.

This test output file provides detailed documentation of Node.js REPL interaction including startup, command execution, and multi-line input handling. The colored terminal codes and output format are appropriate for debugging and testing purposes.

test/test-enhanced-repl.js (1)

72-102: Excellent test coverage for multi-line code execution.

The multi-line code test effectively validates the enhanced REPL's ability to handle complex Python constructs with proper prompt detection and output capture.

src/handlers/edit-search-handlers.ts (2)

29-31: Reasonable default and cap for search results.

The increase to 2000 default results and 5000 maximum cap provides a good balance between comprehensive search results and response size management.


78-99: Well-implemented search result truncation logic.

The truncation mechanism effectively prevents response size issues by:

  • Setting a conservative 900KB limit (well below the 1MB API limit)
  • Checking size incrementally before adding each result
  • Providing informative truncation messages with result count and estimated remaining size

The implementation correctly handles file headers and maintains accurate result counts.

src/terminal-manager.ts (3)

62-67: Smart SSH command enhancement.

Automatically adding the -t option to SSH commands improves interactive session handling. The implementation correctly preserves existing options and logs the enhancement.


105-128: Efficient immediate prompt detection.

The quick prompt pattern matching on stdout provides responsive detection of REPL readiness. The patterns cover common shells and REPLs effectively.


137-150: Robust periodic state checking.

The 100ms periodic check using analyzeProcessState provides a reliable fallback for complex prompt patterns that might not match the quick patterns. The combination of immediate and periodic detection ensures both responsiveness and reliability.

src/server.ts (2)

10-16: Excellent dynamic system guidance implementation.

Replacing hardcoded guidance with dynamic, OS-specific content generated at startup significantly improves user experience across different platforms. The guidance is generated once and reused efficiently.


97-99: Valuable system info addition to configuration.

Adding the systemInfo field to the configuration response provides useful runtime environment details that can help with debugging and platform-specific handling.

src/tools/improved-process-tools.ts (2)

59-62: Helpful shell information in process startup.

Including the shell information in the response message helps users understand the execution environment, especially useful when dealing with cross-platform issues.

Also applies to: 75-75


431-442: Clear and consistent response formatting.

The improved response formatting with structured sections (Input executed, Output, Status) and consistent emoji usage enhances readability and user experience.

src/utils/system-info.ts (2)

21-81: Comprehensive system detection implementation.

The getSystemInfo function provides thorough platform detection with:

  • Clear platform identification for Windows, macOS, and Linux
  • Appropriate default shells for each platform
  • Representative example paths
  • Fallback handling for other Unix-like systems

The implementation is well-structured and maintainable.


86-125: Valuable OS-specific troubleshooting guidance.

The guidance provides practical, actionable advice for common platform-specific issues. The Windows section is particularly detailed with shell compatibility notes, which addresses a common pain point.

src/tools/filesystem.ts (9)

11-29: Excellent refactoring of magic numbers into well-organized constants.

The consolidation of timeouts, size limits, and performance thresholds into typed constant objects improves maintainability and makes the codebase more configurable. The use of as const ensures type safety.


328-372: Excellent addition of enhanced status messages with line count information.

The function provides clear, informative status messages that help users understand exactly what portion of the file is being read. The handling of both tail operations and normal reads with graceful degradation when total line count is unavailable is well implemented.


385-416: Good integration of line counting and constants throughout the smart positioning logic.

The changes properly retrieve the total line count once and pass it through to all reader functions, enabling enhanced status messages. The replacement of magic numbers with named constants improves code clarity.


424-601: Consistent updates across all reader functions for enhanced status messaging.

All reader functions have been properly updated to:

  • Accept the optional fileTotalLines parameter
  • Use the centralized constants instead of magic numbers
  • Generate enhanced status messages with line count information

The changes maintain backward compatibility while improving user feedback.


624-687: Proper utilization of new utilities and constants in readFileFromDisk.

The function now consistently uses the extracted utility functions and constants, improving code maintainability and reducing duplication.


699-701:


716-723: Consistent use of utilities in readFileInternal.

The function properly uses the new utility functions for default read length, file extension, and MIME type detection, maintaining consistency across the module.


797-801: Good use of utilities for telemetry in writeFile.

The function now uses the centralized utilities for file extension extraction and line counting, providing consistent telemetry data.


939-950: Smart optimization in getFileInfo using utilities and constants.

The function now properly:

  • Uses the centralized size limit constant for consistency
  • Checks MIME type to avoid counting lines in image files
  • Uses the utility functions for MIME detection and line counting

This prevents unnecessary processing of binary files.


// Test 1: Read first 10 lines
console.log('\n=== Test 1: Read first 10 lines ===');
const result1 = await readFileFromDisk('/Users/eduardruzga/work/ClaudeServerCommander/new_folder/file_with_1500_lines.txt', 0, 10);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix hardcoded absolute paths for portability.

The test uses hardcoded absolute paths that are specific to one developer's machine, making it non-portable.

Apply this approach to make the test portable:

-        const result1 = await readFileFromDisk('/Users/eduardruzga/work/ClaudeServerCommander/new_folder/file_with_1500_lines.txt', 0, 10);
+        const result1 = await readFileFromDisk('./test/fixtures/file_with_1500_lines.txt', 0, 10);

Consider:

  • Moving test files to a test/fixtures/ directory
  • Using relative paths from the project root
  • Using environment variables for test data paths if needed
📝 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.

Suggested change
const result1 = await readFileFromDisk('/Users/eduardruzga/work/ClaudeServerCommander/new_folder/file_with_1500_lines.txt', 0, 10);
const result1 = await readFileFromDisk('./test/fixtures/file_with_1500_lines.txt', 0, 10);
🤖 Prompt for AI Agents
In test/test_enhanced_read.js at line 10, replace the hardcoded absolute file
path with a relative path pointing to a test file located in a test/fixtures/
directory within the project. Adjust the path to be relative from the project
root or use path utilities to construct it dynamically. This change will make
the test portable across different environments without relying on specific
machine paths.

Comment thread test/modified-test.js
Comment on lines +1 to +59
/**
* Test performance with large files of different line ending types
* This is a modified version to work with the 100-line limit
*/
async function testLargeFilePerformance() {
console.log('\nTest 6: Performance with large files');

const LARGE_FILE_LF = path.join(TEST_DIR, 'large_lf.txt');
const LARGE_FILE_CRLF = path.join(TEST_DIR, 'large_crlf.txt');

try {
// Create large test files (but stay within 100-line limit)
const lines = Array(90).fill('This is a line in a large file.\n');
lines[45] = 'TARGET LINE TO FIND AND REPLACE\n';

// LF version - write in smaller chunks to respect line limit
// First chunk
await fs.writeFile(LARGE_FILE_LF, lines.join(''));

// CRLF version - also respect line limit
const crlfLines = lines.map(line => line.replace('\n', '\r\n'));
await fs.writeFile(LARGE_FILE_CRLF, crlfLines.join(''));

// Test LF file
const startLF = Date.now();
let result = await handleEditBlock({
file_path: LARGE_FILE_LF,
old_string: 'TARGET LINE TO FIND AND REPLACE',
new_string: 'REPLACED TARGET LINE IN LF FILE',
expected_replacements: 1
});
const timeLF = Date.now() - startLF;

assert.ok(
result.content[0].text.includes('Successfully applied 1 edit'),
'Should handle large LF file'
);

// Test CRLF file
const startCRLF = Date.now();
result = await handleEditBlock({
file_path: LARGE_FILE_CRLF,
old_string: 'TARGET LINE TO FIND AND REPLACE',
new_string: 'REPLACED TARGET LINE IN CRLF FILE',
expected_replacements: 1
});
const timeCRLF = Date.now() - startCRLF;

assert.ok(
result.content[0].text.includes('Successfully applied 1 edit'),
'Should handle large CRLF file'
);

console.log(`✓ Performance test passed (LF: ${timeLF}ms, CRLF: ${timeCRLF}ms)`);
} catch (error) {
console.error('❌ Test failed:', error);
throw error;
}
} No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix missing imports and improve test robustness.

The test function references several undefined variables and modules that need to be imported or defined.

The following issues need to be addressed:

  1. Missing imports: path, fs, assert, handleEditBlock, TEST_DIR are not imported or defined
  2. No cleanup: Test files are created but never cleaned up
  3. Potential race conditions: File operations may not be properly awaited
+import assert from 'assert';
+import path from 'path';
+import fs from 'fs/promises';
+import { handleEditBlock } from '../path/to/edit-handler.js';
+
+const TEST_DIR = './test-temp';
+
 /**
  * Test performance with large files of different line ending types
  * This is a modified version to work with the 100-line limit
  */
 async function testLargeFilePerformance() {
   console.log('\nTest 6: Performance with large files');
   
   const LARGE_FILE_LF = path.join(TEST_DIR, 'large_lf.txt');
   const LARGE_FILE_CRLF = path.join(TEST_DIR, 'large_crlf.txt');
   
   try {
     // Create large test files (but stay within 100-line limit)
     const lines = Array(90).fill('This is a line in a large file.\n');
     lines[45] = 'TARGET LINE TO FIND AND REPLACE\n';
     
     // LF version - write in smaller chunks to respect line limit
     // First chunk
     await fs.writeFile(LARGE_FILE_LF, lines.join(''));
     
     // CRLF version - also respect line limit
     const crlfLines = lines.map(line => line.replace('\n', '\r\n'));
     await fs.writeFile(LARGE_FILE_CRLF, crlfLines.join(''));
     
     // Test LF file
     const startLF = Date.now();
     let result = await handleEditBlock({
       file_path: LARGE_FILE_LF,
       old_string: 'TARGET LINE TO FIND AND REPLACE',
       new_string: 'REPLACED TARGET LINE IN LF FILE',
       expected_replacements: 1
     });
     const timeLF = Date.now() - startLF;
     
     assert.ok(
       result.content[0].text.includes('Successfully applied 1 edit'),
       'Should handle large LF file'
     );
     
     // Test CRLF file
     const startCRLF = Date.now();
     result = await handleEditBlock({
       file_path: LARGE_FILE_CRLF,
       old_string: 'TARGET LINE TO FIND AND REPLACE',
       new_string: 'REPLACED TARGET LINE IN CRLF FILE',
       expected_replacements: 1
     });
     const timeCRLF = Date.now() - startCRLF;
     
     assert.ok(
       result.content[0].text.includes('Successfully applied 1 edit'),
       'Should handle large CRLF file'
     );
     
     console.log(`✓ Performance test passed (LF: ${timeLF}ms, CRLF: ${timeCRLF}ms)`);
   } catch (error) {
     console.error('❌ Test failed:', error);
     throw error;
+  } finally {
+    // Cleanup test files
+    try {
+      await fs.unlink(LARGE_FILE_LF).catch(() => {});
+      await fs.unlink(LARGE_FILE_CRLF).catch(() => {});
+    } catch (cleanupError) {
+      console.warn('Failed to cleanup test files:', cleanupError);
+    }
   }
 }
🤖 Prompt for AI Agents
In test/modified-test.js lines 1 to 59, the test function uses undefined
variables and modules such as path, fs, assert, handleEditBlock, and TEST_DIR,
which must be imported or defined at the top of the file. Add the necessary
import statements for these modules and constants. Also, ensure that all
asynchronous file operations are properly awaited to avoid race conditions.
Finally, implement cleanup logic to delete the created test files after the test
completes, ideally in a finally block to guarantee cleanup regardless of test
success or failure.

Comment on lines +9 to +14
const searchArgs = {
path: '/Users/eduardruzga/work/ClaudeServerCommander',
pattern: 'function|const|let|var', // This should match many lines
maxResults: 50000, // Very high limit to get lots of results
ignoreCase: true
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix hardcoded path for cross-platform compatibility.

The test uses a hardcoded path that's specific to one developer's machine, making it non-portable and likely to fail in other environments.

-        const searchArgs = {
-            path: '/Users/eduardruzga/work/ClaudeServerCommander',
-            pattern: 'function|const|let|var',  // This should match many lines
-            maxResults: 50000,  // Very high limit to get lots of results
-            ignoreCase: true
-        };
+        const searchArgs = {
+            path: process.cwd(),  // Use current working directory
+            pattern: 'function|const|let|var',  // This should match many lines
+            maxResults: 50000,  // Very high limit to get lots of results
+            ignoreCase: true
+        };
📝 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.

Suggested change
const searchArgs = {
path: '/Users/eduardruzga/work/ClaudeServerCommander',
pattern: 'function|const|let|var', // This should match many lines
maxResults: 50000, // Very high limit to get lots of results
ignoreCase: true
};
const searchArgs = {
path: process.cwd(), // Use current working directory
pattern: 'function|const|let|var', // This should match many lines
maxResults: 50000, // Very high limit to get lots of results
ignoreCase: true
};
🤖 Prompt for AI Agents
In test/test_search_truncation.js around lines 9 to 14, replace the hardcoded
absolute path with a relative path or use a dynamic method to determine the base
directory, such as using __dirname or process.cwd(), to ensure the test works
across different machines and environments without relying on a specific user's
file system structure.

Comment on lines +9 to +14
const searchArgs = {
path: '/Users/eduardruzga/work/ClaudeServerCommander',
pattern: '.', // Match almost every line - this should be a lot of results
maxResults: 50000, // Very high limit to get lots of results, but capped at 5000
ignoreCase: true
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix hardcoded path and overly broad pattern.

The test suffers from the same portability issue as the previous test file, and the pattern is too broad which could lead to unpredictable results.

-        const searchArgs = {
-            path: '/Users/eduardruzga/work/ClaudeServerCommander',
-            pattern: '.',  // Match almost every line - this should be a lot of results
-            maxResults: 50000,  // Very high limit to get lots of results, but capped at 5000
-            ignoreCase: true
-        };
+        const searchArgs = {
+            path: process.cwd(),  // Use current working directory
+            pattern: 'function|const|let|var|import|export',  // More predictable pattern
+            maxResults: 50000,  // Very high limit to get lots of results, but capped at 5000
+            ignoreCase: true
+        };
📝 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.

Suggested change
const searchArgs = {
path: '/Users/eduardruzga/work/ClaudeServerCommander',
pattern: '.', // Match almost every line - this should be a lot of results
maxResults: 50000, // Very high limit to get lots of results, but capped at 5000
ignoreCase: true
};
const searchArgs = {
path: process.cwd(), // Use current working directory
pattern: 'function|const|let|var|import|export', // More predictable pattern
maxResults: 50000, // Very high limit to get lots of results, but capped at 5000
ignoreCase: true
};
🤖 Prompt for AI Agents
In test/test_improved_search_truncation.js around lines 9 to 14, replace the
hardcoded absolute path with a relative or dynamically resolved path to ensure
portability across environments. Also, refine the search pattern to be more
specific and less broad than '.', which currently matches almost every line and
may cause unpredictable test results. Adjust the pattern to target a meaningful
subset of lines relevant to the test scenario.

@wonderwhy-er wonderwhy-er merged commit 1b1e55e into main Jul 2, 2025
2 checks passed
@wonderwhy-er wonderwhy-er deleted the improve-prompts-for-cross-os-work branch August 15, 2025 06:40
@coderabbitai coderabbitai Bot mentioned this pull request Aug 25, 2025
@coderabbitai coderabbitai Bot mentioned this pull request Oct 1, 2025
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.

1 participant