-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: prepare VS Code LM API for image support (#8123) #8132
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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -170,7 +170,9 @@ describe("convertToVsCodeLmMessages", () => { | |
|
|
||
| expect(result).toHaveLength(1) | ||
| const imagePlaceholder = result[0].content[1] as MockLanguageModelTextPart | ||
| expect(imagePlaceholder.value).toContain("[Image (base64): image/png not supported by VSCode LM API]") | ||
| expect(imagePlaceholder.value).toContain( | ||
| "[Image placeholder: base64 (image/png) - VS Code LM API image support pending]", | ||
| ) | ||
|
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 test update correctly matches the new placeholder format. Consider adding edge case tests for:
This would ensure our placeholder handling is robust even with unexpected input. |
||
| }) | ||
| }) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,8 +85,12 @@ export function convertToVsCodeLmMessages( | |
| // Convert non-tool messages to TextParts after tool messages | ||
| ...nonToolMessages.map((part) => { | ||
| if (part.type === "image") { | ||
| // TODO: When VS Code LM API adds LanguageModelImagePart support, convert images properly | ||
| // For now, we provide a placeholder text description | ||
| const imageType = part.source?.type || "unknown" | ||
| const mediaType = part.source?.media_type || "unknown" | ||
| return new vscode.LanguageModelTextPart( | ||
| `[Image (${part.source?.type || "Unknown source-type"}): ${part.source?.media_type || "unknown media-type"} not supported by VSCode LM API]`, | ||
| `[Image placeholder: ${imageType} (${mediaType}) - VS Code LM API image support pending]`, | ||
|
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. Good improvement on the placeholder message! However, I notice the placeholder messages differ between user context (line 93) and assistant context (line 138). For consistency, should we standardize them? Both could use the same format: |
||
| ) | ||
| } | ||
| return new vscode.LanguageModelTextPart(part.text) | ||
|
|
@@ -129,7 +133,10 @@ export function convertToVsCodeLmMessages( | |
| // Convert non-tool messages to TextParts after tool messages | ||
| ...nonToolMessages.map((part) => { | ||
| if (part.type === "image") { | ||
| return new vscode.LanguageModelTextPart("[Image generation not supported by VSCode LM API]") | ||
| // Assistant-generated images are not yet supported | ||
| return new vscode.LanguageModelTextPart( | ||
| "[Image generation not yet supported by VS Code LM API]", | ||
|
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. Is the different message intentional here? Assistant-generated images use "not yet supported" while user images use "support pending". If we want to distinguish between the two cases, that's fine, but it might be worth a comment explaining why. |
||
| ) | ||
| } | ||
| return new vscode.LanguageModelTextPart(part.text) | ||
| }), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ | |
| "theme": "dark" | ||
| }, | ||
| "engines": { | ||
| "vscode": "^1.84.0", | ||
| "vscode": "^1.95.0", | ||
|
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. Could we consider:
|
||
| "node": "20.19.2" | ||
| }, | ||
| "author": { | ||
|
|
@@ -531,7 +531,7 @@ | |
| "@types/string-similarity": "^4.0.2", | ||
| "@types/tmp": "^0.2.6", | ||
| "@types/turndown": "^5.0.5", | ||
| "@types/vscode": "^1.84.0", | ||
| "@types/vscode": "^1.104.0", | ||
| "@vscode/test-electron": "^2.5.2", | ||
| "@vscode/vsce": "3.3.2", | ||
| "esbuild": "^0.25.0", | ||
|
|
||
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 TODO comment is helpful, but could be more specific. Consider expanding it to:
This would help future maintainers understand exactly what we're waiting for.