mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-13 20:14:50 +00:00
fix(vision): file-ref mode + media pipeline fixes for image visibility (#511)
* fix(vision): file-ref mode + media pipeline fixes for image visibility
Replace inline base64 image loading with file path references when
read_image provider is configured. LLM calls read_image(path=...)
instead of receiving 25K-250K tokens of base64 per image.
Changes:
- Agent loop: skip base64 context storage in file-ref mode, only
load historical images for inline fallback
- New enrichImagePaths() enriches ALL user messages with file paths
(not just current turn) so historical images are accessible
- System prompt: imperative tool summaries ("REQUIRED when you see
<media:image> tags") + dedicated "Media Files" section
- Slack: store mediaPaths in pending history + CollectMedia on mention
- Feishu: add CollectMedia for group context history media
- RC-1 fix: save enriched content (with media IDs/paths) to DB
instead of raw req.Message
Closes #509
* fix(vision): resolve Claude review — Feishu early media + skip loadImages in file-ref mode
- Feishu: download media BEFORE mention gate (step 4) and store file
paths in pending history via Media field. Reuse early-resolved
MediaInfo at step 10 to avoid double-download. CollectMedia now
returns actual paths instead of empty.
- Agent loop: guard loadImages() behind !deferToReadImageTool so
file-ref mode avoids unnecessary disk I/O + base64 encoding.
This commit is contained in:
1 parent
4dafd70c83
commit
231bc9684e
6 files changed
+159
-33
No files matched your search
+30
-19
@@ -89,26 +89,21 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
|
||||
imageFiles = append(imageFiles, bus.MediaFile{Path: ref.Path, MimeType: ref.MimeType})
|
||||
}
|
||||
}
|
||||
if images := loadImages(imageFiles); len(images) > 0 {
|
||||
if deferToReadImageTool {
|
||||
// Tool mode: store in context only — agent calls read_image tool.
|
||||
ctx = tools.WithMediaImages(ctx, images)
|
||||
slog.Info("vision: deferring to read_image tool", "count", len(images), "agent", l.id)
|
||||
} else {
|
||||
// Inline mode: attach to message + context.
|
||||
messages[len(messages)-1].Images = images
|
||||
ctx = tools.WithMediaImages(ctx, images)
|
||||
slog.Info("vision: attached images inline to main provider", "count", len(images), "agent", l.id)
|
||||
}
|
||||
if deferToReadImageTool {
|
||||
// File-ref mode: skip base64 encoding entirely — images accessible via read_image(path=...).
|
||||
slog.Info("vision: file-ref mode, images accessible via read_image tool",
|
||||
"count", len(imageFiles), "agent", l.id)
|
||||
} else if images := loadImages(imageFiles); len(images) > 0 {
|
||||
// Inline mode: read files, base64 encode, attach to message + context.
|
||||
messages[len(messages)-1].Images = images
|
||||
ctx = tools.WithMediaImages(ctx, images)
|
||||
slog.Info("vision: attached images inline to main provider", "count", len(images), "agent", l.id)
|
||||
}
|
||||
}
|
||||
|
||||
// 2a. Load historical images into context for read_image tool.
|
||||
// Without this, read_image can only see current-turn images, not previous turns.
|
||||
// Both inline and tool-deferred modes need this — inline mode attaches images to
|
||||
// messages for the main LLM, but the read_image tool also needs context access
|
||||
// to analyze historical images on demand.
|
||||
if l.mediaStore != nil {
|
||||
// 2a. In inline mode, also load historical images into context for read_image tool.
|
||||
// In file-ref mode (deferToReadImageTool), images are accessed via path — no base64 needed.
|
||||
if !deferToReadImageTool && l.mediaStore != nil {
|
||||
ctx = l.loadHistoricalImagesForTool(ctx, mediaRefs, messages)
|
||||
}
|
||||
|
||||
@@ -178,6 +173,12 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
|
||||
// knows images were received and stored (consistent with audio/video enrichment).
|
||||
l.enrichImageIDs(messages, mediaRefs)
|
||||
|
||||
// 2e-ii. In file-ref mode, enrich ALL user messages' image tags with file paths.
|
||||
// This enables read_image(path=...) for both current and historical images.
|
||||
if deferToReadImageTool {
|
||||
l.enrichImagePaths(messages)
|
||||
}
|
||||
|
||||
// 2f. Collect all media file paths for team workspace auto-collect.
|
||||
// When the leader calls team_tasks(create), these paths are copied to the
|
||||
// team workspace so members can access attached files.
|
||||
@@ -287,12 +288,22 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
|
||||
// 3. Buffer new messages — write to session only AFTER the run completes.
|
||||
// This prevents concurrent runs from seeing each other's in-progress messages.
|
||||
// NOTE: pendingMsgs stores text + lightweight MediaRefs (not base64 images).
|
||||
// Initialized here before rs is created; moved to rs after construction.
|
||||
// Use enriched content (with media IDs and paths) from the messages array
|
||||
// instead of raw req.Message, so historical messages retain full refs (RC-1 fix).
|
||||
var initPendingMsgs []providers.Message
|
||||
if !req.HideInput {
|
||||
enrichedContent := req.Message
|
||||
if len(messages) > 0 {
|
||||
for i := len(messages) - 1; i >= 0; i-- {
|
||||
if messages[i].Role == "user" {
|
||||
enrichedContent = messages[i].Content
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
initPendingMsgs = append(initPendingMsgs, providers.Message{
|
||||
Role: "user",
|
||||
Content: req.Message,
|
||||
Content: enrichedContent,
|
||||
MediaRefs: mediaRefs,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -335,6 +335,68 @@ func (l *Loop) enrichImageIDs(messages []providers.Message, refs []providers.Med
|
||||
messages[lastIdx].Content = content
|
||||
}
|
||||
|
||||
// enrichImagePaths updates ALL user messages to include persisted file paths
|
||||
// in <media:image> tags. This enables the LLM to call read_image(path=...)
|
||||
// to analyze images without inline base64 (saving context tokens).
|
||||
// Unlike enrichImageIDs (last user message only), this enriches ALL messages
|
||||
// so historical images from prior turns are also accessible via file path.
|
||||
func (l *Loop) enrichImagePaths(messages []providers.Message) {
|
||||
if l.mediaStore == nil {
|
||||
return
|
||||
}
|
||||
for i := range messages {
|
||||
if messages[i].Role != "user" || len(messages[i].MediaRefs) == 0 {
|
||||
continue
|
||||
}
|
||||
content := messages[i].Content
|
||||
changed := false
|
||||
for _, ref := range messages[i].MediaRefs {
|
||||
if ref.Kind != "image" {
|
||||
continue
|
||||
}
|
||||
p := ref.Path
|
||||
if p == "" {
|
||||
var err error
|
||||
p, err = l.mediaStore.LoadPath(ref.ID)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
}
|
||||
if p == "" {
|
||||
continue
|
||||
}
|
||||
pathAttr := fmt.Sprintf(" path=%q", p)
|
||||
|
||||
// Skip if path already present in this tag.
|
||||
tagWithID := fmt.Sprintf(`<media:image id=%q`, ref.ID)
|
||||
if idx := strings.Index(content, tagWithID); idx >= 0 {
|
||||
closeIdx := strings.Index(content[idx:], ">")
|
||||
if closeIdx >= 0 {
|
||||
tag := content[idx : idx+closeIdx]
|
||||
if strings.Contains(tag, "path=") {
|
||||
continue // already has path
|
||||
}
|
||||
// Inject path before closing >
|
||||
content = content[:idx+closeIdx] + pathAttr + content[idx+closeIdx:]
|
||||
changed = true
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// Bare <media:image> without id — replace LAST occurrence.
|
||||
bare := "<media:image>"
|
||||
if idx := strings.LastIndex(content, bare); idx >= 0 {
|
||||
replacement := fmt.Sprintf(`<media:image id=%q%s>`, ref.ID, pathAttr)
|
||||
content = content[:idx] + replacement + content[idx+len(bare):]
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
if changed {
|
||||
messages[i].Content = content
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// mediaKindFromMime returns the media kind ("image", "video", "audio", "document")
|
||||
// based on MIME type prefix.
|
||||
func mediaKindFromMime(mime string) string {
|
||||
|
||||
@@ -97,9 +97,9 @@ var coreToolSummaries = map[string]string{
|
||||
"session_status": "Show session status (model, tokens, compaction count)",
|
||||
"sessions_history": "Fetch message history for a session",
|
||||
"sessions_send": "Send a message into another session",
|
||||
"read_image": "Analyze images attached to the conversation. Call this when you see <media:image> tags",
|
||||
"read_audio": "Analyze audio files attached to the conversation. Call this when you see <media:audio> tags",
|
||||
"read_video": "Analyze video files attached to the conversation. Call this when you see <media:video> tags",
|
||||
"read_image": "REQUIRED when you see <media:image> tags — call this tool with the path attribute to analyze the image. You CAN see images through this tool. Never say you cannot see images",
|
||||
"read_audio": "REQUIRED when you see <media:audio> tags — call this tool to transcribe or analyze audio content",
|
||||
"read_video": "REQUIRED when you see <media:video> tags — call this tool to analyze video content",
|
||||
"create_video": "Generate videos from text descriptions using AI",
|
||||
"read_document": "Analyze documents (PDF, DOCX, etc.) attached to the conversation. Call this when you see <media:document> tags. If this tool fails, use a relevant skill instead (e.g. pdf skill with exec tool). The path attribute in <media:document path=\"...\"> is a directly accessible file in your workspace — use it directly, no need to copy",
|
||||
"create_image": "Generate images from text descriptions using AI",
|
||||
@@ -351,6 +351,27 @@ func buildToolingSection(toolNames []string, hasSandbox bool, shellDenyGroups ma
|
||||
"You can install packages at runtime with `pip3 install <pkg>` or `npm install -g <pkg>` — no sudo needed.",
|
||||
)
|
||||
}
|
||||
// Add media capabilities section when media tools are available.
|
||||
hasMediaTools := false
|
||||
for _, name := range toolNames {
|
||||
if name == "read_image" || name == "read_video" || name == "read_audio" || name == "read_document" {
|
||||
hasMediaTools = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if hasMediaTools {
|
||||
lines = append(lines,
|
||||
"",
|
||||
"### Media Files",
|
||||
"When users send images, videos, audio, or documents, you see tags like:",
|
||||
` <media:image id="..." path="...">`,
|
||||
` <media:video id="...">, <media:audio id="...">, <media:document path="...">`,
|
||||
"You MUST use the corresponding read_* tool (with the path or media_id) to analyze them.",
|
||||
"You have full vision/audio/video capabilities through these tools.",
|
||||
"NEVER say you cannot see images or files — always use the tools.",
|
||||
)
|
||||
}
|
||||
|
||||
lines = append(lines,
|
||||
"",
|
||||
"IMPORTANT: write_file content longer than ~12000 characters may be truncated by the API.",
|
||||
|
||||
@@ -31,6 +31,11 @@ Things like:
|
||||
- Default speaker: Kitchen HomePod
|
||||
```
|
||||
|
||||
## Media Files
|
||||
|
||||
When users send images, videos, audio, or documents, use the `read_*` tools to analyze them.
|
||||
The `path` attribute in media tags points to the file — pass it directly to the tool.
|
||||
|
||||
## Why Separate?
|
||||
|
||||
Skills are shared. Your setup is yours. Keeping them apart means you can update skills without losing your notes, and share skills without leaking your infrastructure.
|
||||
|
||||
@@ -60,14 +60,32 @@ func (c *Channel) handleMessageEvent(ctx context.Context, event *MessageEvent) {
|
||||
// 3. Resolve sender name (cached)
|
||||
senderName := c.resolveSenderName(ctx, mc.SenderID)
|
||||
|
||||
// 4. Group policy
|
||||
// 4. Resolve media BEFORE mention gate so non-mentioned messages
|
||||
// also have their files downloaded and stored in pending history.
|
||||
var earlyMedia []media.MediaInfo
|
||||
switch mc.ContentType {
|
||||
case "image", "file", "audio", "video", "sticker":
|
||||
earlyMedia = c.resolveMediaFromMessage(ctx, mc.MessageID, mc.ContentType, msg.Content)
|
||||
case "post":
|
||||
if imageKeys := extractPostImageKeys(msg.Content); len(imageKeys) > 0 {
|
||||
earlyMedia = c.resolvePostImages(ctx, mc.MessageID, imageKeys)
|
||||
}
|
||||
}
|
||||
var earlyMediaPaths []string
|
||||
for _, m := range earlyMedia {
|
||||
if m.FilePath != "" {
|
||||
earlyMediaPaths = append(earlyMediaPaths, m.FilePath)
|
||||
}
|
||||
}
|
||||
|
||||
// 5. Group policy
|
||||
if mc.ChatType == "group" {
|
||||
if !c.checkGroupPolicy(ctx, mc.SenderID, mc.ChatID) {
|
||||
slog.Debug("feishu group message rejected by policy", "sender_id", mc.SenderID, "chat_id", mc.ChatID)
|
||||
return
|
||||
}
|
||||
|
||||
// 5. RequireMention check — record to history if not mentioned
|
||||
// 6. RequireMention check — record to history if not mentioned
|
||||
requireMention := true
|
||||
if c.cfg.RequireMention != nil {
|
||||
requireMention = *c.cfg.RequireMention
|
||||
@@ -81,6 +99,7 @@ func (c *Channel) handleMessageEvent(ctx context.Context, event *MessageEvent) {
|
||||
Sender: senderName,
|
||||
SenderID: mc.SenderID,
|
||||
Body: mc.Content,
|
||||
Media: earlyMediaPaths,
|
||||
Timestamp: time.Now(),
|
||||
MessageID: messageID,
|
||||
}, c.historyLimit)
|
||||
@@ -173,23 +192,26 @@ func (c *Channel) handleMessageEvent(ctx context.Context, event *MessageEvent) {
|
||||
}
|
||||
}
|
||||
|
||||
// 10. Resolve inbound media (image, file, audio, video, sticker)
|
||||
// 10. Build media list from early-resolved media (step 4) + reply media.
|
||||
// Media was already downloaded before the mention gate — reuse results.
|
||||
var mediaList []media.MediaInfo
|
||||
// Reply media first (context), current media second.
|
||||
// Reply media first (context), current-message media second.
|
||||
if len(replyMediaList) > 0 {
|
||||
mediaList = append(mediaList, replyMediaList...)
|
||||
}
|
||||
switch mc.ContentType {
|
||||
case "image", "file", "audio", "video", "sticker":
|
||||
mediaList = append(mediaList, c.resolveMediaFromMessage(ctx, mc.MessageID, mc.ContentType, msg.Content)...)
|
||||
case "post":
|
||||
if imageKeys := extractPostImageKeys(msg.Content); len(imageKeys) > 0 {
|
||||
mediaList = append(mediaList, c.resolvePostImages(ctx, mc.MessageID, imageKeys)...)
|
||||
mediaList = append(mediaList, earlyMedia...)
|
||||
|
||||
// 10b. Collect media from pending history (files downloaded by earlier non-mentioned messages).
|
||||
var mediaFiles []bus.MediaFile
|
||||
if mc.ChatType == "group" && c.historyLimit > 0 {
|
||||
if histMediaPaths := c.groupHistory.CollectMedia(chatID); len(histMediaPaths) > 0 {
|
||||
for _, p := range histMediaPaths {
|
||||
mediaFiles = append(mediaFiles, bus.MediaFile{Path: p}) // cannot use append(slice, other...) — different types
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 11. Process media: STT transcription, document extraction, build tags
|
||||
var mediaFiles []bus.MediaFile
|
||||
if len(mediaList) > 0 {
|
||||
var extraContent string
|
||||
for i := range mediaList {
|
||||
|
||||
@@ -193,6 +193,7 @@ func (c *Channel) handleMessage(ev *slackevents.MessageEvent) {
|
||||
Sender: displayName,
|
||||
SenderID: senderID,
|
||||
Body: content,
|
||||
Media: mediaPaths,
|
||||
Timestamp: time.Now(),
|
||||
MessageID: ev.TimeStamp,
|
||||
}, c.historyLimit)
|
||||
@@ -238,6 +239,10 @@ func (c *Channel) handleMessage(ev *slackevents.MessageEvent) {
|
||||
if peerKind == "group" {
|
||||
annotated := fmt.Sprintf("[From: %s]\n%s", displayName, content)
|
||||
if c.historyLimit > 0 {
|
||||
// Collect media from pending history (files downloaded by earlier non-mentioned messages).
|
||||
if histMediaPaths := c.groupHistory.CollectMedia(localKey); len(histMediaPaths) > 0 {
|
||||
mediaPaths = append(mediaPaths, histMediaPaths...)
|
||||
}
|
||||
finalContent = c.groupHistory.BuildContext(localKey, annotated, c.historyLimit)
|
||||
} else {
|
||||
finalContent = annotated
|
||||
|
||||
Reference in new issue
Block a user