diff --git a/internal/agent/media.go b/internal/agent/media.go index 27828a5d..a0c68e71 100644 --- a/internal/agent/media.go +++ b/internal/agent/media.go @@ -151,7 +151,6 @@ func (l *Loop) enrichDocumentPaths(messages []providers.Message, refs []provider if len(messages) == 0 { return } - // Find last user message lastIdx := -1 for i := len(messages) - 1; i >= 0; i-- { if messages[i].Role == "user" { @@ -168,7 +167,6 @@ func (l *Loop) enrichDocumentPaths(messages []providers.Message, refs []provider if ref.Kind != "document" { continue } - // Use persisted workspace path; fall back to legacy .media/ lookup. p := ref.Path if p == "" && l.mediaStore != nil { var err error @@ -180,41 +178,20 @@ func (l *Loop) enrichDocumentPaths(messages []providers.Message, refs []provider if p == "" { continue } - // Replace or with version that includes path. - // The hint tells the agent the file is directly accessible (no copy needed). pathAttr := fmt.Sprintf(" path=%q", p) - old1 := "" - new1 := "" - // Replace the LAST bare tag (current message, not group history). - if idx := strings.LastIndex(content, old1); idx >= 0 { - content = content[:idx] + new1 + content[idx+len(old1):] - continue - } - // For named variant, inject path attribute (last occurrence) - if idx := strings.LastIndex(content, "") - if closeIdx >= 0 { - tag := content[idx : idx+closeIdx] - content = content[:idx] + tag + pathAttr + ">" + content[idx+closeIdx+1:] - } - } - // For Slack variant with file= attribute (last occurrence) - if idx := strings.LastIndex(content, "") - if closeIdx >= 0 { - tag := content[idx : idx+closeIdx] - content = content[:idx] + tag + pathAttr + ">" + content[idx+closeIdx+1:] - } - } + + // Match first without a path — covers bare, named, and file= variants. + content, _ = replaceFirstMediaTag(content, " and tags so the LLM can reference them. -// Without this, the LLM sees plain and cannot pass a valid media_id. -// Replaces the LAST bare tag (current message) rather than the first (which may be -// in group history context), so the current turn's media gets the correct ID. func (l *Loop) enrichAudioIDs(messages []providers.Message, refs []providers.MediaRef) { if len(messages) == 0 { return @@ -237,27 +214,27 @@ func (l *Loop) enrichAudioIDs(messages []providers.Message, refs []providers.Med } idAttr := fmt.Sprintf(" id=%q", ref.ID) - // Replace the LAST bare with - bare := "" - if idx := strings.LastIndex(content, bare); idx >= 0 { - content = content[:idx] + "" + content[idx+len(bare):] - continue - } - // Replace the LAST bare with - bareVoice := "" - if idx := strings.LastIndex(content, bareVoice); idx >= 0 { - content = content[:idx] + "" + content[idx+len(bareVoice):] + var replaced bool + content, replaced = replaceFirstMediaTag(content, " tags so the LLM can reference them via read_video tool. -// Without this, the LLM sees plain and hallucinates a media_id. -// Replaces the LAST bare tag (current message) rather than the first (which may be -// in group history context), so the current turn's media gets the correct ID. func (l *Loop) enrichVideoIDs(messages []providers.Message, refs []providers.MediaRef) { if len(messages) == 0 { return @@ -280,12 +257,11 @@ func (l *Loop) enrichVideoIDs(messages []providers.Message, refs []providers.Med } idAttr := fmt.Sprintf(" id=%q", ref.ID) - // Replace the LAST bare with - bare := "" - if idx := strings.LastIndex(content, bare); idx >= 0 { - content = content[:idx] + "" + content[idx+len(bare):] - continue - } + content, _ = replaceFirstMediaTag(content, "= 0; i-- { - ref := refs[i] + for _, ref := range refs { if ref.Kind != "image" { continue } @@ -325,8 +297,7 @@ func (l *Loop) enrichImageIDs(messages []providers.Message, refs []providers.Med pathAttr = fmt.Sprintf(" path=%q", ref.Path) } - var replaced bool - content, replaced = replaceLastMediaTag(content, " 0 { - idx := strings.LastIndex(content[:searchFrom], prefix) +// replaceFirstMediaTag finds the first tag in content starting with prefix +// whose full text satisfies match, and replaces it using replace. +// Forward scanning ensures natural positional pairing when iterating refs in order. +func replaceFirstMediaTag(content, prefix string, match func(tag string) bool, replace func(tag string) string) (string, bool) { + pos := 0 + for pos < len(content) { + idx := strings.Index(content[pos:], prefix) if idx < 0 { return content, false } + idx += pos endRel := strings.IndexByte(content[idx:], '>') if endRel < 0 { return content, false @@ -433,7 +405,7 @@ func replaceLastMediaTag(content, prefix string, match func(tag string) bool, re if match(tag) { return content[:idx] + replace(tag) + content[end:], true } - searchFrom = idx + pos = end } return content, false } diff --git a/internal/agent/media_test.go b/internal/agent/media_test.go index ce9a8b53..cd66173c 100644 --- a/internal/agent/media_test.go +++ b/internal/agent/media_test.go @@ -1,6 +1,8 @@ package agent import ( + "os" + "path/filepath" "strings" "testing" @@ -131,3 +133,190 @@ func TestEnrichImagePaths_AttributeOrderIndependence(t *testing.T) { t.Fatalf("expected id to be preserved, got %q", got) } } + +func TestEnrichImageIDs_MultipleRefs(t *testing.T) { + messages := []providers.Message{{ + Role: "user", + Content: "first \nsecond ", + }} + refs := []providers.MediaRef{ + {ID: "img-a", Kind: "image", Path: "/tmp/a.jpg"}, + {ID: "img-b", Kind: "image", Path: "/tmp/b.jpg"}, + } + + var loop Loop + loop.enrichImageIDs(messages, refs) + + want := `first ` + "\n" + `second ` + if messages[0].Content != want { + t.Fatalf("multi-ref alignment:\n got %q\nwant %q", messages[0].Content, want) + } +} + +func TestEnrichImagePaths_MultipleRefsKeepTagAlignment(t *testing.T) { + storeDir := t.TempDir() + mediaStore, err := media.NewStore(storeDir) + if err != nil { + t.Fatalf("NewStore() error = %v", err) + } + + const ( + sessionKey = "session-1" + pathA = "/persisted/a.jpg" + pathB = "/persisted/b.jpg" + ) + + srcA := filepath.Join(storeDir, "a.jpg") + if err := os.WriteFile(srcA, []byte("a"), 0644); err != nil { + t.Fatalf("WriteFile(a) error = %v", err) + } + idA, _, err := mediaStore.SaveFile(sessionKey, srcA, "image/jpeg") + if err != nil { + t.Fatalf("SaveFile(a) error = %v", err) + } + + srcB := filepath.Join(storeDir, "b.jpg") + if err := os.WriteFile(srcB, []byte("b"), 0644); err != nil { + t.Fatalf("WriteFile(b) error = %v", err) + } + idB, _, err := mediaStore.SaveFile(sessionKey, srcB, "image/jpeg") + if err != nil { + t.Fatalf("SaveFile(b) error = %v", err) + } + + messages := []providers.Message{{ + Role: "user", + Content: "first \nsecond ", + MediaRefs: []providers.MediaRef{ + {ID: idA, Kind: "image", Path: pathA}, + {ID: idB, Kind: "image", Path: pathB}, + }, + }} + + var loop Loop + loop.mediaStore = mediaStore + loop.enrichImagePaths(messages) + + want := `first ` + + "\n" + `second ` + if messages[0].Content != want { + t.Fatalf("enrichImagePaths() content = %q, want %q", messages[0].Content, want) + } +} + +func TestEnrichDocumentPaths_MultipleRefs(t *testing.T) { + messages := []providers.Message{{ + Role: "user", + Content: "first \nsecond ", + }} + refs := []providers.MediaRef{ + {ID: "doc-a", Kind: "document", Path: "/tmp/a.pdf"}, + {ID: "doc-b", Kind: "document", Path: "/tmp/b.pdf"}, + } + + var loop Loop + loop.enrichDocumentPaths(messages, refs) + + want := `first ` + "\n" + `second ` + if messages[0].Content != want { + t.Fatalf("multi-ref alignment:\n got %q\nwant %q", messages[0].Content, want) + } +} + +func TestEnrichAudioIDs_MultipleRefs(t *testing.T) { + messages := []providers.Message{{ + Role: "user", + Content: "first \nsecond ", + }} + refs := []providers.MediaRef{ + {ID: "aud-a", Kind: "audio"}, + {ID: "aud-b", Kind: "audio"}, + } + + var loop Loop + loop.enrichAudioIDs(messages, refs) + + want := `first ` + "\n" + `second ` + if messages[0].Content != want { + t.Fatalf("multi-ref alignment:\n got %q\nwant %q", messages[0].Content, want) + } +} + +func TestEnrichVideoIDs_MultipleRefs(t *testing.T) { + messages := []providers.Message{{ + Role: "user", + Content: "first \nsecond ", + }} + refs := []providers.MediaRef{ + {ID: "vid-a", Kind: "video"}, + {ID: "vid-b", Kind: "video"}, + } + + var loop Loop + loop.enrichVideoIDs(messages, refs) + + want := `first ` + "\n" + `second ` + if messages[0].Content != want { + t.Fatalf("multi-ref alignment:\n got %q\nwant %q", messages[0].Content, want) + } +} + +// TestEnrichImageIDs_MoreRefsThanTags verifies that extra refs are silently skipped. +func TestEnrichImageIDs_MoreRefsThanTags(t *testing.T) { + messages := []providers.Message{{ + Role: "user", + Content: "only one ", + }} + refs := []providers.MediaRef{ + {ID: "img-a", Kind: "image", Path: "/tmp/a.jpg"}, + {ID: "img-b", Kind: "image", Path: "/tmp/b.jpg"}, + } + + var loop Loop + loop.enrichImageIDs(messages, refs) + + want := `only one ` + if messages[0].Content != want { + t.Fatalf("more refs than tags:\n got %q\nwant %q", messages[0].Content, want) + } +} + +// TestEnrichImageIDs_MoreTagsThanRefs verifies that extra tags are left bare. +func TestEnrichImageIDs_MoreTagsThanRefs(t *testing.T) { + messages := []providers.Message{{ + Role: "user", + Content: "first \nsecond ", + }} + refs := []providers.MediaRef{ + {ID: "img-a", Kind: "image", Path: "/tmp/a.jpg"}, + } + + var loop Loop + loop.enrichImageIDs(messages, refs) + + want := `first ` + "\n" + `second ` + if messages[0].Content != want { + t.Fatalf("more tags than refs:\n got %q\nwant %q", messages[0].Content, want) + } +} + +// TestEnrichAudioIDs_MixedAudioAndVoice verifies correct pairing when both +// and tags appear in the same message. +func TestEnrichAudioIDs_MixedAudioAndVoice(t *testing.T) { + messages := []providers.Message{{ + Role: "user", + Content: "hear this \nand this ", + }} + refs := []providers.MediaRef{ + {ID: "aud-1", Kind: "audio"}, + {ID: "aud-2", Kind: "audio"}, + } + + var loop Loop + loop.enrichAudioIDs(messages, refs) + + want := `hear this ` + "\n" + `and this ` + if messages[0].Content != want { + t.Fatalf("mixed audio/voice:\n got %q\nwant %q", messages[0].Content, want) + } +}