Conversation
Add support browser config in model catalogs
Add image uploading
Feat/image uploading
There was a problem hiding this comment.
Pull request overview
This PR is a hotfix release on the MCP (Model Context Protocol) that includes two main areas of enhancement: (1) extending the media placeholder pattern to support both formats with and without the base64, marker, and (2) adding a new supports_browser capability flag to the model catalog system.
Key Changes
- Updated media placeholder regex pattern to accept both
data:image/jpeg;jan_xxxanddata:image/jpeg;base64,jan_xxxformats across media-api and llm-api services - Added
supports_browserboolean field to model catalogs with full database migration, domain model, API, and repository support - Enhanced chat message handling to support flexible content formats from different clients (OpenAI format and browser-mcp format)
- Improved tool message handling by properly setting
tool_call_idand using appropriate content types - Removed verbose error logging in streaming operations (simplified to silent error suppression)
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
services/media-api/internal/domain/media/service.go |
Updated regex pattern to accept optional base64, marker in jan_* placeholders |
services/media-api/docs/swagger/swagger.json |
Updated API documentation to reflect flexible placeholder formats and changed payload types from array to object |
services/media-api/README.md |
Updated documentation examples to show both placeholder format variations |
services/llm-api/migrations/000015_add_supports_browser.up.sql |
Added migration to create supports_browser column with index on model_catalogs table |
services/llm-api/migrations/000015_add_supports_browser.down.sql |
Added rollback migration to remove supports_browser column and index |
services/llm-api/internal/domain/model/model_catalog.go |
Added SupportsBrowser field to ModelCatalog domain model and filter |
services/llm-api/internal/infrastructure/database/dbschema/model_catalog.go |
Added SupportsBrowser field mapping in database schema with proper default |
services/llm-api/internal/infrastructure/database/gormgen/model_catalogs.gen.go |
Regenerated GORM code to include SupportsBrowser field |
services/llm-api/internal/infrastructure/database/repository/modelrepo/model_catalog_repository.go |
Added filter support for SupportsBrowser in repository queries |
services/llm-api/internal/interfaces/httpserver/requests/models/model.go |
Added SupportsBrowser to UpdateModelCatalogRequest |
services/llm-api/internal/interfaces/httpserver/responses/model/model.go |
Added SupportsBrowser to ModelCatalogResponse |
services/llm-api/internal/interfaces/httpserver/handlers/modelhandler/model_catalog_handler.go |
Added handling for SupportsBrowser field in catalog update logic |
services/llm-api/internal/interfaces/httpserver/requests/chat/chat.go |
Added FlexibleContentPart to handle multiple client formats, custom UnmarshalJSON for chat requests, and helper functions (including duplicate min function) |
services/llm-api/internal/interfaces/httpserver/handlers/chathandler/chat_handler.go |
Enhanced tool message handling with proper tool_call_id mapping, removed verbose logging function, added duplicate min function |
services/llm-api/internal/utils/httpclients/chat/chat_completion_client.go |
Simplified error handling by removing verbose logging, added chunksReceived increment, added duplicate min function, introduced duplicate comment |
services/llm-api/internal/infrastructure/mediaresolver/resolver.go |
Updated regex pattern to match media-api changes for flexible placeholder formats |
Comments suppressed due to low confidence (1)
services/llm-api/internal/utils/httpclients/chat/chat_completion_client.go:328
- The variable chunksReceived is incremented twice for data lines: once at line 293 for every line received, and again at line 328 only for lines that have a data prefix. This results in data chunks being double-counted. If the intention is to count all lines including non-data lines, remove the increment at line 328. If the intention is to count only data chunks, remove the increment at line 293.
chunksReceived++
// Check if this is the [DONE] marker BEFORE writing it
// Check if this is the [DONE] marker BEFORE writing it
if data, found := strings.CutPrefix(line, dataPrefix); found {
if data == doneMarker {
// Call the beforeDone callback BEFORE sending [DONE]
if beforeDone != nil {
_ = beforeDone(reqCtx)
}
// Now write the [DONE] marker
if err := c.writeSSELine(reqCtx, line); err != nil {
cancel()
wg.Wait()
span.RecordError(err)
span.SetStatus(codes.Error, "failed to write SSE done marker")
return nil, platformerrors.AsError(ctx, platformerrors.LayerDomain, err, "unable to write SSE line")
}
streamingComplete = true
cancel()
break
}
}
// Write the line for non-[DONE] events
if err := c.writeSSELine(reqCtx, line); err != nil {
cancel()
wg.Wait()
span.RecordError(err)
span.SetStatus(codes.Error, "failed to write SSE line")
return nil, platformerrors.AsError(ctx, platformerrors.LayerDomain, err, "unable to write SSE line")
}
// Process the data chunk
if data, found := strings.CutPrefix(line, dataPrefix); found {
chunksReceived++
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // min returns the minimum of two integers | ||
| func min(a, b int) int { | ||
| if a < b { | ||
| return a | ||
| } | ||
| return b | ||
| } |
There was a problem hiding this comment.
Since Go 1.21, the min function is built into the language. This custom implementation can be replaced with the built-in min function. The codebase is using Go 1.25.0, so the built-in function is available. Note that this same helper function is duplicated in multiple files.
| // min returns the minimum of two integers | |
| func min(a, b int) int { | |
| if a < b { | |
| return a | |
| } | |
| return b | |
| } |
|
|
||
| // min returns the minimum of two integers | ||
| func min(a, b int) int { | ||
| if a < b { | ||
| return a | ||
| } | ||
| return b | ||
| } |
There was a problem hiding this comment.
Since Go 1.21, the min function is built into the language. This custom implementation can be replaced with the built-in min function. The codebase is using Go 1.25.0, so the built-in function is available. Note that this same helper function is duplicated in multiple files.
| // min returns the minimum of two integers | |
| func min(a, b int) int { | |
| if a < b { | |
| return a | |
| } | |
| return b | |
| } |
| // min returns the minimum of two integers | ||
| func min(a, b int) int { | ||
| if a < b { | ||
| return a | ||
| } | ||
| return b | ||
| } |
There was a problem hiding this comment.
Since Go 1.21, the min function is built into the language. This custom implementation can be replaced with the built-in min function. The codebase is using Go 1.25.0, so the built-in function is available. Note that this same helper function is duplicated in multiple files.
| chunksReceived++ | ||
|
|
||
| // Check if this is the [DONE] marker BEFORE writing it | ||
| // Check if this is the [DONE] marker BEFORE writing it |
There was a problem hiding this comment.
Duplicate comment. The same comment appears on both lines 295 and 296.
| // Check if this is the [DONE] marker BEFORE writing it |
Release on mcp, hotfix