-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: Morph Fast Apply (tolerant multi-file apply_diff) + settings #7828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -394,17 +394,18 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |
|
|
||
| // Only set up diff strategy if diff is enabled. | ||
| if (this.diffEnabled) { | ||
| // Default to old strategy, will be updated if experiment is enabled. | ||
| // Default to legacy single-file strategy | ||
| this.diffStrategy = new MultiSearchReplaceDiffStrategy(this.fuzzyMatchThreshold) | ||
|
|
||
| // Check experiment asynchronously and update strategy if needed. | ||
| // Prefer MultiFile when Morph Fast Apply is enabled, otherwise respect experiment flag. | ||
| provider.getState().then((state) => { | ||
| const isMultiFileApplyDiffEnabled = experiments.isEnabled( | ||
| state.experiments ?? {}, | ||
| EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF, | ||
| ) | ||
| const isMorphFastApplyEnabled = state.apiConfiguration?.morphFastApplyEnabled === true | ||
|
|
||
| if (isMultiFileApplyDiffEnabled) { | ||
| if (isMorphFastApplyEnabled || isMultiFileApplyDiffEnabled) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logic here gives Morph Fast Apply precedence over the experiment flag, but this priority isn't documented. Could we add a comment explaining why morphFastApplyEnabled takes priority? Something like: |
||
| this.diffStrategy = new MultiFileSearchReplaceDiffStrategy(this.fuzzyMatchThreshold) | ||
| } | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,15 +27,20 @@ export const generateSystemPrompt = async (provider: ClineProvider, message: Web | |
| maxConcurrentFileReads, | ||
| } = await provider.getState() | ||
|
|
||
| // Check experiment to determine which diff strategy to use | ||
| // Determine which diff strategy to use: | ||
| // - If Morph Fast Apply is enabled, force MultiFile strategy for best tolerance and batching | ||
| // - Otherwise, fall back to experiment flag to choose between MultiFile and single-file strategies | ||
| const isMultiFileApplyDiffEnabled = experimentsModule.isEnabled( | ||
| experiments ?? {}, | ||
| EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF, | ||
| ) | ||
| const isMorphFastApplyEnabled = apiConfiguration?.morphFastApplyEnabled === true | ||
|
|
||
| const diffStrategy = isMultiFileApplyDiffEnabled | ||
| const diffStrategy = isMorphFastApplyEnabled | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the same strategy selection logic as in Task.ts. Consider extracting this to a shared utility function to avoid duplication and ensure consistency. Something like: |
||
| ? new MultiFileSearchReplaceDiffStrategy(fuzzyMatchThreshold) | ||
| : new MultiSearchReplaceDiffStrategy(fuzzyMatchThreshold) | ||
| : isMultiFileApplyDiffEnabled | ||
| ? new MultiFileSearchReplaceDiffStrategy(fuzzyMatchThreshold) | ||
| : new MultiSearchReplaceDiffStrategy(fuzzyMatchThreshold) | ||
|
|
||
| const cwd = provider.cwd | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||
| import React, { memo, useCallback, useEffect, useMemo, useState } from "react" | ||||||
| import { convertHeadersToObject } from "./utils/headers" | ||||||
| import { useDebounce } from "react-use" | ||||||
| import { VSCodeLink, VSCodeButton } from "@vscode/webview-ui-toolkit/react" | ||||||
| import { VSCodeLink, VSCodeButton, VSCodeCheckbox, VSCodeTextField } from "@vscode/webview-ui-toolkit/react" | ||||||
| import { ExternalLinkIcon } from "@radix-ui/react-icons" | ||||||
|
|
||||||
| import { | ||||||
|
|
@@ -765,6 +765,32 @@ const ApiOptions = ({ | |||||
| fuzzyMatchThreshold={apiConfiguration.fuzzyMatchThreshold} | ||||||
| onChange={(field, value) => setApiConfigurationField(field, value)} | ||||||
| /> | ||||||
|
|
||||||
| {/* Morph Fast Apply - provider-scoped controls */} | ||||||
| <div className="pl-3 border-l-2 border-vscode-button-background space-y-2"> | ||||||
| <label className="block font-medium">Morph Fast Apply</label> | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing i18n for these UI strings. The rest of the application uses translation keys from the i18n system, but these new controls have hardcoded English text:
Consider adding these to the translation files for consistency.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider replacing the hardcoded label text 'Morph Fast Apply' with a translatable string (e.g. using t('settings:providers.morphFastApply.title')) to support internationalization.
Suggested change
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX. |
||||||
| <div className="flex flex-col gap-1"> | ||||||
| <VSCodeCheckbox | ||||||
| checked={Boolean(apiConfiguration.morphFastApplyEnabled)} | ||||||
| onChange={(e: any) => | ||||||
| setApiConfigurationField("morphFastApplyEnabled", e.target.checked) | ||||||
| }> | ||||||
| Enable Morph Fast Apply (fuzzy multi-file) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace the checkbox text 'Enable Morph Fast Apply (fuzzy multi-file)' with a translatable string to ensure consistency with the app’s localization strategy.
Suggested change
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX. |
||||||
| </VSCodeCheckbox> | ||||||
|
|
||||||
| <VSCodeTextField | ||||||
| value={apiConfiguration?.morphApiKey || ""} | ||||||
| type="password" | ||||||
| onInput={(e: any) => setApiConfigurationField("morphApiKey", e.target.value)} | ||||||
| placeholder="Optional API key"> | ||||||
| Morph API key (optional) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace the inner text 'Morph API key (optional)' in the VSCodeTextField with a call to the translation function (e.g. t('settings:providers.morphApiKey.label')) to support i18n.
Suggested change
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX. |
||||||
| </VSCodeTextField> | ||||||
| <div className="text-sm text-vscode-descriptionForeground"> | ||||||
| When enabled, apply_diff uses faster tolerant matching across multiple files. An API | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace the hardcoded description text with a translatable string so that this user-facing message can be localized. This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX. |
||||||
| key is only required if using a hosted Morph service. | ||||||
| </div> | ||||||
| </div> | ||||||
| </div> | ||||||
| {selectedModelInfo?.supportsTemperature !== false && ( | ||||||
| <TemperatureControl | ||||||
| value={apiConfiguration.modelTemperature} | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment formatting here doesn't match the established pattern in this file. Other optional sections use a format like:
But this uses:
Minor inconsistency, but worth keeping the style uniform.