From 9561476347fd1bde4f6c2d01f6cf078701c53d2d Mon Sep 17 00:00:00 2001 From: Guoguo Date: Wed, 29 Apr 2026 03:22:12 -0700 Subject: [PATCH] fix(media): match all channel image placeholders in injectPathTags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Different channels emit different placeholder formats — Telegram/Feishu use [image: photo], WeCom/WeChat/Line use bare [image], QQ/Discord use [image: ]. The previous string-match code only handled [image: photo], so for the other channels the path tag was appended as a duplicate, producing content like "[image] [image:/path]". Switch to per-type regex that matches all generic placeholder shapes while leaving path tags ([image:/path]) untouched. Also fixes the same issue for [audio], [video], [file] tags. Added test coverage for the various placeholder shapes. Co-Authored-By: Claude Opus 4.6 --- pkg/agent/agent_media.go | 41 +++++++++++++++++++++++++--------- pkg/agent/agent_test.go | 48 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/pkg/agent/agent_media.go b/pkg/agent/agent_media.go index 3bbeca352..db9d5072b 100644 --- a/pkg/agent/agent_media.go +++ b/pkg/agent/agent_media.go @@ -11,6 +11,7 @@ import ( "encoding/base64" "io" "os" + "regexp" "strings" "github.com/h2non/filetype" @@ -20,12 +21,23 @@ import ( "github.com/sipeed/picoclaw/pkg/providers" ) +// genericPlaceholderRegex matches generic media placeholders emitted by various +// channels: [image], [image: photo], [image: filename.jpg] — but NOT path tags +// like [image:/path/to/file] (path tags have no space after the colon). +var ( + imagePlaceholderRegex = regexp.MustCompile(`\[image(:\s+[^\]]*)?\]`) + audioPlaceholderRegex = regexp.MustCompile(`\[audio(:\s+[^\]]*)?\]`) + videoPlaceholderRegex = regexp.MustCompile(`\[video(:\s+[^\]]*)?\]`) + filePlaceholderRegex = regexp.MustCompile(`\[file(:\s+[^\]]*)?\]`) +) + // resolveMediaRefs resolves media:// refs in messages. // For user messages: images get path tags only ([image:/path]) so the LLM // can decide whether to view them via load_image or operate on the file. // For tool messages: images are base64-encoded and appended as a synthetic -// user message after the contiguous tool-message block ends, preserving -// the required assistant→tool ordering for LLM APIs. +// user message only after the contiguous tool-message block ends, so we don't +// break the tool-results-must-immediately-follow-assistant constraint that +// LLM APIs enforce. // Non-image files always get path tags regardless of role. // Returns a new slice; original messages are not mutated. func resolveMediaRefs(messages []providers.Message, store media.MediaStore, maxSize int) []providers.Message { @@ -227,24 +239,31 @@ func buildPathTag(mime, localPath string) string { } // injectPathTags replaces generic media tags in content with path-bearing versions, -// or appends if no matching generic tag is found. +// or appends if no matching generic tag is found. Channels emit a few different +// placeholder formats — [image], [image: photo], [image: filename.jpg] — so we +// match all of them via regex while leaving path tags ([image:/path]) untouched. func injectPathTags(content string, tags []string) string { for _, tag := range tags { - var generic string + var pattern *regexp.Regexp switch { case strings.HasPrefix(tag, "[image:"): - generic = "[image: photo]" + pattern = imagePlaceholderRegex case strings.HasPrefix(tag, "[audio:"): - generic = "[audio]" + pattern = audioPlaceholderRegex case strings.HasPrefix(tag, "[video:"): - generic = "[video]" + pattern = videoPlaceholderRegex case strings.HasPrefix(tag, "[file:"): - generic = "[file]" + pattern = filePlaceholderRegex } - if generic != "" && strings.Contains(content, generic) { - content = strings.Replace(content, generic, tag, 1) - } else if content == "" { + if pattern != nil { + if loc := pattern.FindStringIndex(content); loc != nil { + content = content[:loc[0]] + tag + content[loc[1]:] + continue + } + } + + if content == "" { content = tag } else { content += " " + tag diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index afc709508..26da280e1 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -4992,6 +4992,54 @@ func TestResolveMediaRefs_NoGenericTagAppendsPath(t *testing.T) { } } +func TestInjectPathTags_HandlesVariousChannelPlaceholders(t *testing.T) { + cases := []struct { + name string + content string + tag string + want string + }{ + // Telegram / Feishu format + {"image_photo", "[image: photo]", "[image:/tmp/p.png]", "[image:/tmp/p.png]"}, + // WeCom / WeChat / Line format + {"bare_image", "[image]", "[image:/tmp/p.png]", "[image:/tmp/p.png]"}, + // QQ / Discord format with filename + {"image_filename", "[image: pic.jpg]", "[image:/tmp/p.png]", "[image:/tmp/p.png]"}, + {"audio_with_filename", "[audio: voice.m4a]", "[audio:/tmp/a.m4a]", "[audio:/tmp/a.m4a]"}, + {"bare_audio", "[audio]", "[audio:/tmp/a.m4a]", "[audio:/tmp/a.m4a]"}, + {"bare_video", "[video]", "[video:/tmp/v.mp4]", "[video:/tmp/v.mp4]"}, + {"bare_file", "[file]", "[file:/tmp/f.pdf]", "[file:/tmp/f.pdf]"}, + // Mixed surrounding text + { + "with_text", + "hello [image] world", + "[image:/tmp/p.png]", + "hello [image:/tmp/p.png] world", + }, + // No placeholder — append + {"no_placeholder", "hello world", "[image:/tmp/p.png]", "hello world [image:/tmp/p.png]"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := injectPathTags(tc.content, []string{tc.tag}) + if got != tc.want { + t.Errorf("expected %q, got %q", tc.want, got) + } + }) + } +} + +func TestInjectPathTags_DoesNotReplacePathTag(t *testing.T) { + // If content already contains a path tag, we must not touch it. + content := "see [image:/already/placed.png] thanks" + got := injectPathTags(content, []string{"[image:/new/path.png]"}) + want := "see [image:/already/placed.png] thanks [image:/new/path.png]" + if got != want { + t.Fatalf("expected %q, got %q", want, got) + } +} + func TestResolveMediaRefs_EmptyContentGetsPathTag(t *testing.T) { store := media.NewFileMediaStore() dir := t.TempDir()