-
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
Conversation
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.
Reviewing my own code is like proofreading my own typos - I'll miss the obvious ones.
|
|
||
| {/* 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> |
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.
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:
- "Morph Fast Apply"
- "Enable Morph Fast Apply (fuzzy multi-file)"
- "Morph API key (optional)"
- "When enabled, apply_diff uses faster tolerant matching..."
Consider adding these to the translation files for consistency.
| diffEnabled: z.boolean().optional(), | ||
| todoListEnabled: z.boolean().optional(), | ||
| fuzzyMatchThreshold: z.number().optional(), | ||
| // Morph Fast Apply - optional provider-scoped controls |
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.
| const isMorphFastApplyEnabled = state.apiConfiguration?.morphFastApplyEnabled === true | ||
|
|
||
| if (isMultiFileApplyDiffEnabled) { | ||
| if (isMorphFastApplyEnabled || isMultiFileApplyDiffEnabled) { |
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 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:
| const isMorphFastApplyEnabled = apiConfiguration?.morphFastApplyEnabled === true | ||
|
|
||
| const diffStrategy = isMultiFileApplyDiffEnabled | ||
| const diffStrategy = isMorphFastApplyEnabled |
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.
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:
|
|
||
| {/* 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> |
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.
Consider replacing the hardcoded label text 'Morph Fast Apply' with a translatable string (e.g. using t('settings:providers.morphFastApply.title')) to support internationalization.
| <label className="block font-medium">Morph Fast Apply</label> | |
| <label className="block font-medium">{t('settings:providers.morphFastApply.title')}</label> |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
| onChange={(e: any) => | ||
| setApiConfigurationField("morphFastApplyEnabled", e.target.checked) | ||
| }> | ||
| Enable Morph Fast Apply (fuzzy multi-file) |
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.
Replace the checkbox text 'Enable Morph Fast Apply (fuzzy multi-file)' with a translatable string to ensure consistency with the app’s localization strategy.
| Enable Morph Fast Apply (fuzzy multi-file) | |
| {t('settings:advancedSettings.enableMorphFastApply')} |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
| type="password" | ||
| onInput={(e: any) => setApiConfigurationField("morphApiKey", e.target.value)} | ||
| placeholder="Optional API key"> | ||
| Morph API key (optional) |
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.
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.
| Morph API key (optional) | |
| {t('settings:providers.morphApiKey.label')} |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
| Morph API key (optional) | ||
| </VSCodeTextField> | ||
| <div className="text-sm text-vscode-descriptionForeground"> | ||
| When enabled, apply_diff uses faster tolerant matching across multiple files. An API |
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.
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.
Adds morphFastApplyEnabled and morphApiKey provider settings with UI controls; chooses MultiFile diff strategy when enabled; updates prompt wiring. No external Morph service required. Includes secret storage for morphApiKey.
Important
Add Morph Fast Apply feature with settings and UI controls for a multi-file diff strategy.
morphFastApplyEnabledandmorphApiKeytoprovider-settings.ts.morphApiKeyinSECRET_STATE_KEYSinglobal-settings.ts.morphFastApplyEnabledandmorphApiKeyinApiOptions.tsx.Task.ts, preferMultiFileSearchReplaceDiffStrategyifmorphFastApplyEnabledis true.generateSystemPrompt.ts, chooseMultiFileSearchReplaceDiffStrategyifmorphFastApplyEnabledis true.This description was created by
for cc6a3b8. You can customize this summary. It will automatically update as commits are pushed.