fix(aichat): dont use diff based edits on script mode

[centdix]

No description provided.

[claude]

Claude finished @centdix's task —— View job

---

PR Review: Fix AI Chat Script Mode - Disable Diff-Based Edits

Summary

This PR disables diff-based edit functionality for all AI providers in the script chat mode by emptying the DIFF_BASED_EDIT_PROVIDERS array. Previously, OpenAI, Anthropic, Google AI, and Azure OpenAI were configured to use diff-based editing.

Changes Analysis

File: frontend/src/lib/components/copilot/chat/script/core.ts (+2/-7)

The change replaces a populated array of AI providers with an empty array and adds a TODO comment:

// Before:
export const DIFF_BASED_EDIT_PROVIDERS: AIProvider[] = [
  'openai',
  'anthropic', 
  'googleai',
  'azure_openai'
]

// After:
// TODO: Explore this again when we have better diff-based edit providers
export const DIFF_BASED_EDIT_PROVIDERS: AIProvider[] = []

Code Quality Assessment

Positive Aspects

✅ Clean and focused change - The modification is minimal and surgical, affecting only the configuration constant

✅ Backwards compatible - The empty array is handled gracefully by existing code through .includes() checks

✅ Self-documenting - The TODO comment explains the intention to revisit this feature

✅ No breaking changes - The system falls back to full-file replacement mode, which is still functional

Concerns and Suggestions

1. Missing Context in PR Description ⚠️

The PR has no description explaining:

• What problem does this solve?
• Were there bugs with diff-based edits?
• What makes the current implementation insufficient?
• Are there any related issues or user reports?

Recommendation: Add a PR description explaining the motivation for this change.

2. TODO Comment Lacks Specificity ⚠️

The TODO comment is vague: "when we have better diff-based edit providers"

Questions:

• What specific improvements are needed?
• What are the current limitations?
• Are there tracking issues for these improvements?

Recommendation: Make the TODO more actionable:

// TODO: Re-enable diff-based edits once the following issues are resolved:
// - [Issue link] Diff matching fails on whitespace variations
// - [Issue link] Poor handling of large code blocks
// See: [design doc/discussion link]

3. Behavioral Impact Analysis 🔍

This change affects how the AI assistant modifies code:

Before (Diff mode):

• AI provides targeted diffs: {old_string: "...", new_string: "..."}
• Only modified sections are specified
• More token-efficient for small changes
• Requires exact string matching (fragile)

After (Full replacement mode):

• AI provides complete updated file
• Uses more tokens for large files
• More reliable (no string matching issues)
• Better for comprehensive refactors

Impact:

• Users may see different AI behavior in the script editor chat
• Potentially higher token usage for small edits
• More reliable edits (likely the reason for this change)

4. Scope Limitation 🔍

The change only affects script mode (script/core.ts). I verified that:

• Flow mode doesn't have similar functionality
• This is the only file using DIFF_BASED_EDIT_PROVIDERS

Question: Is this intentional, or should other chat modes also be reviewed?

5. No Test Coverage Changes ⚠️

There are no tests in this PR. While the change is simple:

• Are there existing tests that cover this code path?
• Should tests be updated to verify both modes still work?
• Are there integration tests for the AI chat functionality?

Recommendation: Verify test coverage for the prepareScriptTools and buildChatSystemPrompt functions.

6. Performance Implications 📊

Full-file replacement will:

• Increase token usage (more expensive API calls)
• May increase latency for large files
• Could affect the user experience for users with large scripts

Recommendation: Consider monitoring metrics after deployment:

• Average tokens per edit request
• User feedback on edit quality
• API costs

Security Assessment

✅ No security concerns - This is a configuration change that affects AI behavior but doesn't introduce vulnerabilities.

Testing the Changes

To verify this change works correctly:

In the Windmill app, navigate to any script editor (Scripts → Create Script or open an existing script). Open the AI chat panel (typically in the sidebar or bottom panel). Start a conversation with the AI assistant and request code modifications, such as "Add error handling to the main function" or "Refactor this to use async/await". The AI should now provide complete file replacements instead of targeted diffs. Verify that the code changes are applied correctly and that you can accept or reject them. Test with various AI providers (OpenAI, Anthropic, Google AI) to ensure they all use the full-replacement mode. Check that the test run functionality still works after AI edits are applied.

Recommendation

APPROVE with minor suggestions:

The change is technically sound and achieves its goal of disabling diff-based edits. However:

1. Add a PR description explaining the motivation
2. Enhance the TODO comment with specific improvement criteria (see inline comment)
3. Consider documenting the trade-offs between diff mode and full-replacement mode
4. Monitor token usage and user feedback post-deployment

The lack of context makes it difficult to fully evaluate whether this is the right approach, but the implementation itself is clean and safe.

---

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	405b89c
Status:	✅  Deploy successful!
Preview URL:	https://59772e05.windmill.pages.dev
Branch Preview URL:	https://fg-nodiffedit.windmill.pages.dev

View logs

[claude]

Consider making this TODO more actionable by specifying what "better diff-based edit providers" means. For example:

// TODO: Re-enable diff-based edits once we resolve:
// 1. String matching issues with whitespace variations
// 2. Token efficiency vs reliability trade-offs
// 3. User feedback on edit quality
// Track: [issue/discussion link]

This will help future developers understand what improvements are needed before re-enabling this feature.