From 7e479032db1981355f1f6f0edfa4f80f48c094b0 Mon Sep 17 00:00:00 2001 From: dj-oyu <68707227+dj-oyu@users.noreply.github.com> Date: Thu, 19 Mar 2026 22:07:23 +0900 Subject: [PATCH 1/2] feat: add image description for text-only models with CodexProvider vision support Text-only main models (e.g. MiniMax m2.7) cannot process images. When a photo is sent, a vision model (e.g. gpt-5.4-nano) now generates a text description that replaces the base64 data URL before sending to the main model. In plan mode (interviewing/review), images pass through directly to the vision-capable plan model. - CodexProvider: add multipart input_image support in buildCodexParams - AgentInstance: add ImageModel/ImageCandidates with PlanModel fallback - loop_media: describeImagesInMessages with fallback chain + cache - mediacache: SQLite-backed cache (FNV-1a hash) for image descriptions, schema includes file_path/pages columns for future PDF OCR support - Processing indicator: braille spinner animation via draft messages - Periodic cache pruning (7d TTL) in gcLoop Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/agent/instance.go | 20 +++ pkg/agent/instance_ext.go | 11 ++ pkg/agent/loop_ext.go | 34 +++++ pkg/agent/loop_media.go | 201 +++++++++++++++++++++++++++ pkg/agent/loop_media_test.go | 106 ++++++++++++++ pkg/agent/loop_run.go | 8 ++ pkg/agent/loop_session.go | 1 + pkg/mediacache/cache.go | 152 ++++++++++++++++++++ pkg/mediacache/cache_test.go | 190 +++++++++++++++++++++++++ pkg/providers/codex_provider.go | 32 +++++ pkg/providers/codex_provider_test.go | 100 +++++++++++++ 11 files changed, 855 insertions(+) create mode 100644 pkg/agent/loop_media_test.go create mode 100644 pkg/mediacache/cache.go create mode 100644 pkg/mediacache/cache_test.go diff --git a/pkg/agent/instance.go b/pkg/agent/instance.go index 8d131d192..be58ad83a 100644 --- a/pkg/agent/instance.go +++ b/pkg/agent/instance.go @@ -44,6 +44,9 @@ type AgentInstance struct { PlanModel string PlanFallbacks []string PlanCandidates []providers.FallbackCandidate + ImageModel string + ImageFallbacks []string + ImageCandidates []providers.FallbackCandidate // Router is non-nil when model routing is configured and the light model // was successfully resolved. It scores each incoming message and decides @@ -327,6 +330,23 @@ func resolvePlanFallbacks(agentCfg *config.AgentConfig, defaults *config.AgentDe return defaults.PlanModelFallbacks } +// resolveImageModel resolves the image description model for an agent. +// Falls back to PlanModel if no dedicated image model is configured. +func resolveImageModel(agentCfg *config.AgentConfig, defaults *config.AgentDefaults) string { + if defaults.ImageModel != "" { + return defaults.ImageModel + } + return defaults.PlanModel +} + +// resolveImageFallbacks resolves the image model fallbacks. +func resolveImageFallbacks(agentCfg *config.AgentConfig, defaults *config.AgentDefaults) []string { + if defaults.ImageModelFallbacks != nil { + return defaults.ImageModelFallbacks + } + return defaults.PlanModelFallbacks +} + func compilePatterns(patterns []string) []*regexp.Regexp { compiled := make([]*regexp.Regexp, 0, len(patterns)) for _, p := range patterns { diff --git a/pkg/agent/instance_ext.go b/pkg/agent/instance_ext.go index 830a3bb2c..20659e202 100644 --- a/pkg/agent/instance_ext.go +++ b/pkg/agent/instance_ext.go @@ -61,6 +61,17 @@ func (ai *AgentInstance) initInstanceExt( ai.PlanCandidates = providers.ResolveCandidates(planModelCfg, defaults.Provider) } + // Resolve image model (for describing images sent to text-only main models) + ai.ImageModel = resolveImageModel(agentCfg, defaults) + ai.ImageFallbacks = resolveImageFallbacks(agentCfg, defaults) + if ai.ImageModel != "" { + imageModelCfg := providers.ModelConfig{ + Primary: ai.ImageModel, + Fallbacks: ai.ImageFallbacks, + } + ai.ImageCandidates = providers.ResolveCandidates(imageModelCfg, defaults.Provider) + } + // Startup cleanup: prune orphaned worktrees worktreesDir := filepath.Join(ai.Workspace, ".worktrees") if repoRoot := git.FindRepoRoot(ai.Workspace); repoRoot != "" { diff --git a/pkg/agent/loop_ext.go b/pkg/agent/loop_ext.go index 4ef4aaecf..632084a70 100644 --- a/pkg/agent/loop_ext.go +++ b/pkg/agent/loop_ext.go @@ -1,12 +1,16 @@ package agent import ( + "log" + "path/filepath" "strings" "sync" + "time" "github.com/sipeed/picoclaw/pkg/bus" "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/logger" + "github.com/sipeed/picoclaw/pkg/mediacache" "github.com/sipeed/picoclaw/pkg/orch" "github.com/sipeed/picoclaw/pkg/providers" "github.com/sipeed/picoclaw/pkg/routing" @@ -40,6 +44,8 @@ type loopExt struct { saveConfig func(*config.Config) error onHeartbeatThreadUpdate func(int) + + mediaCache *mediacache.Cache // nil when workspace unavailable } // initLoopExt initializes all fork-specific fields: stats tracker, @@ -68,6 +74,17 @@ func (al *AgentLoop) initLoopExt(cfg *config.Config, registry *AgentRegistry, en } } + // Media cache (co-located with sessions.db in default agent workspace) + if defaultAgent != nil { + cachePath := filepath.Join(defaultAgent.Workspace, "media_cache.db") + mc, err := mediacache.Open(cachePath) + if err != nil { + log.Printf("media cache: %v (caching disabled)", err) + } else { + al.mediaCache = mc + } + } + // Shutdown signal channel al.done = make(chan struct{}) @@ -89,6 +106,10 @@ func (al *AgentLoop) closeExt() { al.stats.Close() } + if al.mediaCache != nil { + al.mediaCache.Close() + } + registry := al.GetRegistry() for _, agentID := range registry.ListAgentIDs() { if agent, ok := registry.GetAgent(agentID); ok { @@ -97,6 +118,19 @@ func (al *AgentLoop) closeExt() { } } +// pruneMediaCache removes stale entries from the media cache. +func (al *AgentLoop) pruneMediaCache() { + if al.mediaCache == nil { + return + } + const mediaCacheTTL = 7 * 24 * time.Hour + if n, err := al.mediaCache.Prune(mediaCacheTTL); err != nil { + logger.WarnCF("agent", "media cache prune error", map[string]any{"error": err.Error()}) + } else if n > 0 { + logger.InfoCF("agent", "media cache pruned", map[string]any{"removed": n}) + } +} + // SetConfigSaver registers a callback to persist config changes. func (al *AgentLoop) SetConfigSaver(fn func(*config.Config) error) { al.saveConfig = fn diff --git a/pkg/agent/loop_media.go b/pkg/agent/loop_media.go index 1380f0214..768f7ef42 100644 --- a/pkg/agent/loop_media.go +++ b/pkg/agent/loop_media.go @@ -8,15 +8,20 @@ package agent import ( "bytes" + "context" "encoding/base64" + "fmt" "io" "os" "strings" + "time" "github.com/h2non/filetype" + "github.com/sipeed/picoclaw/pkg/bus" "github.com/sipeed/picoclaw/pkg/logger" "github.com/sipeed/picoclaw/pkg/media" + "github.com/sipeed/picoclaw/pkg/mediacache" "github.com/sipeed/picoclaw/pkg/providers" ) @@ -178,3 +183,199 @@ func injectPathTags(content string, tags []string) string { } return content } + +const imageDescriptionSystemPrompt = "Describe this image concisely but thoroughly. Focus on text content, visual elements, and any details relevant to understanding the image. If the image contains text, transcribe it. Output only the description." + +// describeImagesInMessages replaces data:image/* entries in Message.Media +// with text descriptions generated by a vision model. The data URL is removed +// from Media and the "[image: photo]" tag in Content is replaced with the description. +// A braille spinner is shown via draft messages while processing. +// Returns a new slice; original messages are not mutated. +func (al *AgentLoop) describeImagesInMessages( + ctx context.Context, messages []providers.Message, agent *AgentInstance, + channel, chatID string, +) []providers.Message { + // Count images to decide whether to show indicator + var imageCount int + for _, m := range messages { + for _, ref := range m.Media { + if strings.HasPrefix(ref, "data:image/") { + imageCount++ + } + } + } + if imageCount == 0 { + return messages + } + + result := make([]providers.Message, len(messages)) + copy(result, messages) + + // Start processing indicator + label := "Processing image..." + if imageCount > 1 { + label = fmt.Sprintf("Processing %d images...", imageCount) + } + stopIndicator := al.processingIndicator(ctx, channel, chatID, label) + defer stopIndicator() + + for i, m := range result { + if len(m.Media) == 0 { + continue + } + + var descriptions []string + var kept []string + for _, mediaURL := range m.Media { + if !strings.HasPrefix(mediaURL, "data:image/") { + kept = append(kept, mediaURL) + continue + } + + desc := al.describeImage(ctx, mediaURL, m.Content, agent) + descriptions = append(descriptions, desc) + } + + if len(descriptions) == 0 { + continue + } + + result[i].Media = kept + result[i].Content = injectImageDescriptions(result[i].Content, descriptions) + } + + return result +} + +// describeImage calls a vision model to describe a single image. +// Results are cached by content hash to avoid redundant API calls. +// Returns the description text, or a placeholder on error. +func (al *AgentLoop) describeImage( + ctx context.Context, dataURL, userContext string, agent *AgentInstance, +) string { + // Check cache first + hash := mediacache.HashData([]byte(dataURL)) + if al.mediaCache != nil { + if cached, ok := al.mediaCache.Get(hash, mediacache.TypeImageDesc); ok { + logger.DebugCF("agent", "Image description cache hit", map[string]any{"hash": hash}) + return cached + } + } + + messages := []providers.Message{ + {Role: "system", Content: imageDescriptionSystemPrompt}, + { + Role: "user", + Content: userContext, + Media: []string{dataURL}, + }, + } + + candidates := agent.ImageCandidates + if len(candidates) == 0 { + return "description unavailable" + } + + var resp *providers.LLMResponse + var err error + + opts := map[string]any{ + "max_tokens": 1024, + "temperature": 0.3, + } + + if len(candidates) > 1 && al.fallback != nil { + fbResult, fbErr := al.fallback.Execute(ctx, candidates, + func(ctx context.Context, provider, model string) (*providers.LLMResponse, error) { + p := al.resolveProvider(provider, model, agent.Provider) + return p.Chat(ctx, messages, nil, model, opts) + }, + ) + if fbErr != nil { + err = fbErr + } else { + resp = fbResult.Response + } + } else { + c := candidates[0] + p := al.resolveProvider(c.Provider, c.Model, agent.Provider) + resp, err = p.Chat(ctx, messages, nil, c.Model, opts) + } + + if err != nil { + logger.WarnCF("agent", "Image description failed", map[string]any{"error": err.Error()}) + return "description unavailable" + } + if resp == nil || strings.TrimSpace(resp.Content) == "" { + return "description unavailable" + } + + desc := strings.TrimSpace(resp.Content) + + // Store in cache + if al.mediaCache != nil { + if cErr := al.mediaCache.Put(hash, mediacache.TypeImageDesc, desc); cErr != nil { + logger.WarnCF("agent", "Failed to cache image description", map[string]any{"error": cErr.Error()}) + } + } + + return desc +} + +// brailleSpinnerFrames is a set of braille characters that produce +// a smooth rotating animation when displayed sequentially. +var brailleSpinnerFrames = [...]string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} + +// processingIndicator publishes draft status messages with a braille spinner +// animation to indicate active processing. It runs until the returned stop +// function is called. The label describes what is being processed. +func (al *AgentLoop) processingIndicator(ctx context.Context, channel, chatID, label string) (stop func()) { + if al.bus == nil || channel == "" || chatID == "" { + return func() {} + } + + done := make(chan struct{}) + go func() { + ticker := time.NewTicker(150 * time.Millisecond) + defer ticker.Stop() + + frame := 0 + for { + select { + case <-done: + return + case <-ctx.Done(): + return + case <-ticker.C: + content := fmt.Sprintf("%s %s", brailleSpinnerFrames[frame%len(brailleSpinnerFrames)], label) + _ = al.bus.PublishOutbound(ctx, bus.OutboundMessage{ + Channel: channel, + ChatID: chatID, + Content: content, + IsStatus: true, + }) + frame++ + } + } + }() + + return func() { + close(done) + } +} + +// injectImageDescriptions replaces "[image: photo]" tags in content with +// "[image: ]" for each description provided. +func injectImageDescriptions(content string, descriptions []string) string { + for _, desc := range descriptions { + replacement := "[image: " + desc + "]" + if strings.Contains(content, "[image: photo]") { + content = strings.Replace(content, "[image: photo]", replacement, 1) + } else if content == "" { + content = replacement + } else { + content += " " + replacement + } + } + return content +} diff --git a/pkg/agent/loop_media_test.go b/pkg/agent/loop_media_test.go new file mode 100644 index 000000000..9afd647cb --- /dev/null +++ b/pkg/agent/loop_media_test.go @@ -0,0 +1,106 @@ +package agent + +import ( + "testing" + + "github.com/sipeed/picoclaw/pkg/config" + "github.com/sipeed/picoclaw/pkg/providers" +) + +func TestInjectImageDescriptions_ReplacesTag(t *testing.T) { + content := "Check this out [image: photo]" + result := injectImageDescriptions(content, []string{"a cat sitting on a table"}) + want := "Check this out [image: a cat sitting on a table]" + if result != want { + t.Errorf("got %q, want %q", result, want) + } +} + +func TestInjectImageDescriptions_MultipleImages(t *testing.T) { + content := "Here are two photos [image: photo] and [image: photo]" + result := injectImageDescriptions(content, []string{"a dog", "a cat"}) + want := "Here are two photos [image: a dog] and [image: a cat]" + if result != want { + t.Errorf("got %q, want %q", result, want) + } +} + +func TestInjectImageDescriptions_NoTag(t *testing.T) { + content := "Hello" + result := injectImageDescriptions(content, []string{"a sunset"}) + want := "Hello [image: a sunset]" + if result != want { + t.Errorf("got %q, want %q", result, want) + } +} + +func TestInjectImageDescriptions_EmptyContent(t *testing.T) { + result := injectImageDescriptions("", []string{"a chart"}) + want := "[image: a chart]" + if result != want { + t.Errorf("got %q, want %q", result, want) + } +} + +func TestDescribeImages_NoImageCandidates(t *testing.T) { + // When no ImageCandidates are configured, messages should pass through unchanged. + messages := []providers.Message{ + { + Role: "user", + Content: "[image: photo]", + Media: []string{"data:image/jpeg;base64,/9j/4AAQ"}, + }, + } + agent := &AgentInstance{ + ImageCandidates: nil, + } + + // With no candidates, describeImagesInMessages should still work + // but return "description unavailable" for each image. + al := &AgentLoop{} + result := al.describeImagesInMessages(t.Context(), messages, agent, "", "") + + // The data URL should be removed from Media + if len(result[0].Media) != 0 { + t.Errorf("expected empty media, got %v", result[0].Media) + } + // Content should have the placeholder + if result[0].Content != "[image: description unavailable]" { + t.Errorf("content = %q", result[0].Content) + } +} + +func TestDescribeImages_SkippedInPlanMode(t *testing.T) { + // isPlanPreExecution should return true for interviewing/review + if !isPlanPreExecution("interviewing") { + t.Error("interviewing should be pre-execution") + } + if !isPlanPreExecution("review") { + t.Error("review should be pre-execution") + } + if isPlanPreExecution("executing") { + t.Error("executing should not be pre-execution") + } + if isPlanPreExecution("") { + t.Error("empty should not be pre-execution") + } +} + +func TestResolveImageModel_FallsToPlanModel(t *testing.T) { + // When ImageModel is empty, should fall back to PlanModel + model := resolveImageModel(nil, &config.AgentDefaults{ + PlanModel: "openai/gpt-5.4", + }) + if model != "openai/gpt-5.4" { + t.Errorf("got %q, want %q", model, "openai/gpt-5.4") + } + + // When ImageModel is set, should use it + model = resolveImageModel(nil, &config.AgentDefaults{ + ImageModel: "openai/gpt-5.4-nano", + PlanModel: "openai/gpt-5.4", + }) + if model != "openai/gpt-5.4-nano" { + t.Errorf("got %q, want %q", model, "openai/gpt-5.4-nano") + } +} diff --git a/pkg/agent/loop_run.go b/pkg/agent/loop_run.go index 5f5dfe88d..d0df75f43 100644 --- a/pkg/agent/loop_run.go +++ b/pkg/agent/loop_run.go @@ -248,6 +248,14 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt maxMediaSize := cfg.Agents.Defaults.GetMaxMediaSize() messages = resolveMediaRefs(messages, al.mediaStore, maxMediaSize) + // Describe images for text-only main models. In plan pre-execution mode + // (interviewing/review), the plan model (vision-capable) handles images + // directly, so skip description generation. + planStatus := agent.ContextBuilder.GetPlanStatus() + if len(agent.ImageCandidates) > 0 && !isPlanPreExecution(planStatus) { + messages = al.describeImagesInMessages(ctx, messages, agent, opts.Channel, opts.ChatID) + } + // 2b. Interview staleness nudge: if MEMORY.md hasn't been updated for // several consecutive turns, inject a reminder so the AI writes its findings. diff --git a/pkg/agent/loop_session.go b/pkg/agent/loop_session.go index dfbcfee12..2d38e8af3 100644 --- a/pkg/agent/loop_session.go +++ b/pkg/agent/loop_session.go @@ -35,6 +35,7 @@ func (al *AgentLoop) gcLoop() { case <-ticker.C: al.gcSessionLocks() + al.pruneMediaCache() case <-al.done: diff --git a/pkg/mediacache/cache.go b/pkg/mediacache/cache.go new file mode 100644 index 000000000..d3f81ef1c --- /dev/null +++ b/pkg/mediacache/cache.go @@ -0,0 +1,152 @@ +package mediacache + +import ( + "database/sql" + "fmt" + "hash/fnv" + "time" + + _ "modernc.org/sqlite" +) + +const schema = ` +CREATE TABLE IF NOT EXISTS media_cache ( + hash TEXT NOT NULL, + type TEXT NOT NULL, + result TEXT NOT NULL DEFAULT '', + file_path TEXT NOT NULL DEFAULT '', + pages INTEGER NOT NULL DEFAULT 0, + created_at TEXT NOT NULL, + accessed_at TEXT NOT NULL, + PRIMARY KEY (hash, type) +); +CREATE INDEX IF NOT EXISTS idx_media_cache_accessed ON media_cache(accessed_at); +` + +// Entry types for the cache. +const ( + TypeImageDesc = "image_desc" + TypePDFOCR = "pdf_ocr" +) + +// Cache provides SQLite-backed caching for media processing results. +// Image descriptions are stored as text directly in the result column. +// PDF OCR results store the path to the generated markdown file. +type Cache struct { + db *sql.DB +} + +// Open opens (or creates) a media cache database at dbPath. +func Open(dbPath string) (*Cache, error) { + connStr := "file:" + dbPath + "?_journal_mode=WAL&_busy_timeout=5000" + db, err := sql.Open("sqlite", connStr) + if err != nil { + return nil, fmt.Errorf("open media cache: %w", err) + } + if _, err := db.Exec(schema); err != nil { + db.Close() + return nil, fmt.Errorf("init media cache schema: %w", err) + } + return &Cache{db: db}, nil +} + +// Close closes the database connection. +func (c *Cache) Close() error { + if c.db != nil { + return c.db.Close() + } + return nil +} + +// Get retrieves a cached result by hash and type. +// Returns the result and true if found, or empty string and false if not. +func (c *Cache) Get(hash, entryType string) (string, bool) { + var result string + err := c.db.QueryRow( + `SELECT result FROM media_cache WHERE hash = ? AND type = ?`, + hash, entryType, + ).Scan(&result) + if err != nil { + return "", false + } + // Update accessed_at + now := time.Now().UTC().Format(time.RFC3339) + _, _ = c.db.Exec( + `UPDATE media_cache SET accessed_at = ? WHERE hash = ? AND type = ?`, + now, hash, entryType, + ) + return result, true +} + +// Put stores a result in the cache. +func (c *Cache) Put(hash, entryType, result string) error { + now := time.Now().UTC().Format(time.RFC3339) + _, err := c.db.Exec( + `INSERT INTO media_cache (hash, type, result, created_at, accessed_at) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT(hash, type) DO UPDATE SET result = ?, accessed_at = ?`, + hash, entryType, result, now, now, + result, now, + ) + return err +} + +// Entry represents a full cache entry with optional file path and page count. +// Used for PDF OCR where result holds a preview and file_path holds the full MD. +type Entry struct { + Result string // preview text (image desc) or first-page excerpt (PDF) + FilePath string // path to full OCR markdown (empty for image_desc) + Pages int // number of pages (0 for non-PDF) +} + +// GetEntry retrieves a full cache entry by hash and type. +func (c *Cache) GetEntry(hash, entryType string) (Entry, bool) { + var e Entry + err := c.db.QueryRow( + `SELECT result, file_path, pages FROM media_cache WHERE hash = ? AND type = ?`, + hash, entryType, + ).Scan(&e.Result, &e.FilePath, &e.Pages) + if err != nil { + return Entry{}, false + } + now := time.Now().UTC().Format(time.RFC3339) + _, _ = c.db.Exec( + `UPDATE media_cache SET accessed_at = ? WHERE hash = ? AND type = ?`, + now, hash, entryType, + ) + return e, true +} + +// PutEntry stores a full entry in the cache. +func (c *Cache) PutEntry(hash, entryType string, entry Entry) error { + now := time.Now().UTC().Format(time.RFC3339) + _, err := c.db.Exec( + `INSERT INTO media_cache (hash, type, result, file_path, pages, created_at, accessed_at) + VALUES (?, ?, ?, ?, ?, ?, ?) + ON CONFLICT(hash, type) DO UPDATE SET result = ?, file_path = ?, pages = ?, accessed_at = ?`, + hash, entryType, entry.Result, entry.FilePath, entry.Pages, now, now, + entry.Result, entry.FilePath, entry.Pages, now, + ) + return err +} + +// Prune removes entries not accessed within the given duration. +// Returns the number of entries removed. +func (c *Cache) Prune(ttl time.Duration) (int64, error) { + cutoff := time.Now().Add(-ttl).UTC().Format(time.RFC3339) + res, err := c.db.Exec( + `DELETE FROM media_cache WHERE accessed_at < ?`, cutoff, + ) + if err != nil { + return 0, err + } + return res.RowsAffected() +} + +// HashData computes a fast FNV-1a 64-bit hash of the given data, +// returned as a hex string. +func HashData(data []byte) string { + h := fnv.New64a() + h.Write(data) + return fmt.Sprintf("%016x", h.Sum64()) +} diff --git a/pkg/mediacache/cache_test.go b/pkg/mediacache/cache_test.go new file mode 100644 index 000000000..9bdd835ac --- /dev/null +++ b/pkg/mediacache/cache_test.go @@ -0,0 +1,190 @@ +package mediacache + +import ( + "path/filepath" + "testing" + "time" +) + +func openTestCache(t *testing.T) *Cache { + t.Helper() + dbPath := filepath.Join(t.TempDir(), "test_cache.db") + c, err := Open(dbPath) + if err != nil { + t.Fatalf("Open: %v", err) + } + t.Cleanup(func() { c.Close() }) + return c +} + +func TestCache_PutAndGet(t *testing.T) { + c := openTestCache(t) + + hash := HashData([]byte("test-image-data")) + + // Miss + if _, ok := c.Get(hash, TypeImageDesc); ok { + t.Fatal("expected miss on empty cache") + } + + // Put + if err := c.Put(hash, TypeImageDesc, "a photo of a cat"); err != nil { + t.Fatalf("Put: %v", err) + } + + // Hit + result, ok := c.Get(hash, TypeImageDesc) + if !ok { + t.Fatal("expected hit after Put") + } + if result != "a photo of a cat" { + t.Errorf("result = %q, want %q", result, "a photo of a cat") + } +} + +func TestCache_DifferentTypes(t *testing.T) { + c := openTestCache(t) + hash := HashData([]byte("same-data")) + + _ = c.Put(hash, TypeImageDesc, "image description") + _ = c.Put(hash, TypePDFOCR, "/path/to/doc.md") + + desc, ok := c.Get(hash, TypeImageDesc) + if !ok || desc != "image description" { + t.Errorf("image_desc: got %q, ok=%v", desc, ok) + } + + ocr, ok := c.Get(hash, TypePDFOCR) + if !ok || ocr != "/path/to/doc.md" { + t.Errorf("pdf_ocr: got %q, ok=%v", ocr, ok) + } +} + +func TestCache_Upsert(t *testing.T) { + c := openTestCache(t) + hash := HashData([]byte("data")) + + _ = c.Put(hash, TypeImageDesc, "first") + _ = c.Put(hash, TypeImageDesc, "updated") + + result, ok := c.Get(hash, TypeImageDesc) + if !ok || result != "updated" { + t.Errorf("expected updated value, got %q", result) + } +} + +func TestCache_Prune(t *testing.T) { + c := openTestCache(t) + + hash := HashData([]byte("old-data")) + _ = c.Put(hash, TypeImageDesc, "old description") + + // Backdate the accessed_at + _, _ = c.db.Exec( + `UPDATE media_cache SET accessed_at = ? WHERE hash = ?`, + time.Now().Add(-48*time.Hour).UTC().Format(time.RFC3339), + hash, + ) + + // Put a fresh entry + freshHash := HashData([]byte("fresh-data")) + _ = c.Put(freshHash, TypeImageDesc, "fresh description") + + // Prune with 24h TTL + n, err := c.Prune(24 * time.Hour) + if err != nil { + t.Fatalf("Prune: %v", err) + } + if n != 1 { + t.Errorf("pruned %d, want 1", n) + } + + // Old should be gone + if _, ok := c.Get(hash, TypeImageDesc); ok { + t.Error("old entry should have been pruned") + } + + // Fresh should remain + if _, ok := c.Get(freshHash, TypeImageDesc); !ok { + t.Error("fresh entry should remain") + } +} + +func TestHashData_Deterministic(t *testing.T) { + data := []byte("hello world") + h1 := HashData(data) + h2 := HashData(data) + if h1 != h2 { + t.Errorf("non-deterministic: %q != %q", h1, h2) + } + if len(h1) != 16 { + t.Errorf("hash length = %d, want 16", len(h1)) + } +} + +func TestHashData_DifferentInputs(t *testing.T) { + h1 := HashData([]byte("input-a")) + h2 := HashData([]byte("input-b")) + if h1 == h2 { + t.Error("different inputs should produce different hashes") + } +} + +func TestCache_PutEntryAndGetEntry(t *testing.T) { + c := openTestCache(t) + hash := HashData([]byte("pdf-content")) + + entry := Entry{ + Result: "契約書 — 第1条 本契約は甲乙間の...", + FilePath: "/workspace/.mediacache/abc123.md", + Pages: 8, + } + if err := c.PutEntry(hash, TypePDFOCR, entry); err != nil { + t.Fatalf("PutEntry: %v", err) + } + + got, ok := c.GetEntry(hash, TypePDFOCR) + if !ok { + t.Fatal("expected hit") + } + if got.Result != entry.Result { + t.Errorf("Result = %q", got.Result) + } + if got.FilePath != entry.FilePath { + t.Errorf("FilePath = %q", got.FilePath) + } + if got.Pages != 8 { + t.Errorf("Pages = %d, want 8", got.Pages) + } +} + +func TestCache_GetEntry_Miss(t *testing.T) { + c := openTestCache(t) + _, ok := c.GetEntry("nonexistent", TypePDFOCR) + if ok { + t.Error("expected miss") + } +} + +func TestCache_SimpleGetIgnoresFilePath(t *testing.T) { + // Simple Get/Put should still work with the new schema + c := openTestCache(t) + hash := HashData([]byte("img")) + + if err := c.Put(hash, TypeImageDesc, "a sunset"); err != nil { + t.Fatalf("Put: %v", err) + } + result, ok := c.Get(hash, TypeImageDesc) + if !ok || result != "a sunset" { + t.Errorf("Get = %q, ok=%v", result, ok) + } + + // GetEntry should also work, with empty file_path + entry, ok := c.GetEntry(hash, TypeImageDesc) + if !ok { + t.Fatal("GetEntry miss") + } + if entry.FilePath != "" { + t.Errorf("FilePath should be empty for image_desc, got %q", entry.FilePath) + } +} diff --git a/pkg/providers/codex_provider.go b/pkg/providers/codex_provider.go index c18cc73de..077d6eed6 100644 --- a/pkg/providers/codex_provider.go +++ b/pkg/providers/codex_provider.go @@ -231,6 +231,28 @@ func buildCodexParams( }, }, }) + } else if hasImageMedia(msg.Media) { + var parts responses.ResponseInputMessageContentListParam + if msg.Content != "" { + parts = append(parts, responses.ResponseInputContentParamOfInputText(msg.Content)) + } + for _, mediaURL := range msg.Media { + if strings.HasPrefix(mediaURL, "data:image/") { + parts = append(parts, responses.ResponseInputContentUnionParam{ + OfInputImage: &responses.ResponseInputImageParam{ + ImageURL: openai.Opt(mediaURL), + }, + }) + } + } + inputItems = append(inputItems, responses.ResponseInputItemUnionParam{ + OfMessage: &responses.EasyInputMessageParam{ + Role: responses.EasyInputMessageRoleUser, + Content: responses.EasyInputMessageContentUnionParam{ + OfInputItemContentList: parts, + }, + }, + }) } else { inputItems = append(inputItems, responses.ResponseInputItemUnionParam{ OfMessage: &responses.EasyInputMessageParam{ @@ -422,6 +444,16 @@ func parseCodexResponse(resp *responses.Response) *LLMResponse { } } +// hasImageMedia returns true if the media slice contains at least one data:image/* URL. +func hasImageMedia(media []string) bool { + for _, m := range media { + if strings.HasPrefix(m, "data:image/") { + return true + } + } + return false +} + func createCodexTokenSource() func() (string, string, error) { return func() (string, string, error) { cred, err := auth.GetCredential("openai") diff --git a/pkg/providers/codex_provider_test.go b/pkg/providers/codex_provider_test.go index 2d6d7f356..6e7117ff9 100644 --- a/pkg/providers/codex_provider_test.go +++ b/pkg/providers/codex_provider_test.go @@ -612,6 +612,106 @@ func TestResolveCodexModel(t *testing.T) { } } +func TestBuildCodexParams_WithImageMedia(t *testing.T) { + messages := []Message{ + { + Role: "user", + Content: "What's in this image?", + Media: []string{"data:image/jpeg;base64,/9j/4AAQ"}, + }, + } + params := buildCodexParams(messages, nil, "gpt-5.4", map[string]any{}, false) + + items := params.Input.OfInputItemList + if len(items) != 1 { + t.Fatalf("len(items) = %d, want 1", len(items)) + } + msg := items[0].OfMessage + if msg == nil { + t.Fatal("expected OfMessage") + } + contentList := msg.Content.OfInputItemContentList + if len(contentList) != 2 { + t.Fatalf("len(contentList) = %d, want 2 (text + image)", len(contentList)) + } + if contentList[0].OfInputText == nil { + t.Error("first content part should be input_text") + } + if contentList[1].OfInputImage == nil { + t.Fatal("second content part should be input_image") + } + if contentList[1].OfInputImage.ImageURL.Or("") != "data:image/jpeg;base64,/9j/4AAQ" { + t.Errorf("ImageURL = %q", contentList[1].OfInputImage.ImageURL.Or("")) + } +} + +func TestBuildCodexParams_MixedTextAndImage(t *testing.T) { + messages := []Message{ + { + Role: "user", + Content: "Describe both images", + Media: []string{ + "data:image/png;base64,iVBOR", + "data:image/jpeg;base64,/9j/4", + }, + }, + } + params := buildCodexParams(messages, nil, "gpt-5.4", map[string]any{}, false) + + items := params.Input.OfInputItemList + if len(items) != 1 { + t.Fatalf("len(items) = %d, want 1", len(items)) + } + contentList := items[0].OfMessage.Content.OfInputItemContentList + if len(contentList) != 3 { + t.Fatalf("len(contentList) = %d, want 3 (text + 2 images)", len(contentList)) + } + if contentList[0].OfInputText == nil { + t.Error("first part should be text") + } + if contentList[1].OfInputImage == nil { + t.Error("second part should be image") + } + if contentList[2].OfInputImage == nil { + t.Error("third part should be image") + } +} + +func TestBuildCodexParams_NoMedia(t *testing.T) { + messages := []Message{ + {Role: "user", Content: "Hello"}, + } + params := buildCodexParams(messages, nil, "gpt-4o", map[string]any{}, false) + + items := params.Input.OfInputItemList + if len(items) != 1 { + t.Fatalf("len(items) = %d, want 1", len(items)) + } + msg := items[0].OfMessage + if msg == nil { + t.Fatal("expected OfMessage") + } + // Should use OfString, not OfInputItemContentList + if msg.Content.OfString.Or("") != "Hello" { + t.Errorf("OfString = %q, want %q", msg.Content.OfString.Or(""), "Hello") + } +} + +func TestHasImageMedia(t *testing.T) { + if hasImageMedia(nil) { + t.Error("nil should be false") + } + if hasImageMedia([]string{"https://example.com/img.png"}) { + t.Error("URL should be false") + } + if !hasImageMedia([]string{"data:image/jpeg;base64,abc"}) { + t.Error("data URL should be true") + } + if !hasImageMedia([]string{"text", "data:image/png;base64,xyz"}) { + t.Error("mixed with data URL should be true") + } +} + func createOpenAITestClient(baseURL, token, accountID string) *openai.Client { opts := []openaiopt.RequestOption{ openaiopt.WithBaseURL(baseURL), From e599d168a1db69b488ddb929e0296aeb1f9b1ab9 Mon Sep 17 00:00:00 2001 From: dj-oyu <68707227+dj-oyu@users.noreply.github.com> Date: Thu, 19 Mar 2026 22:12:19 +0900 Subject: [PATCH 2/2] fix: replace Japanese string in test to satisfy gosmopolitan linter Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/mediacache/cache_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/mediacache/cache_test.go b/pkg/mediacache/cache_test.go index 9bdd835ac..51f8b6d78 100644 --- a/pkg/mediacache/cache_test.go +++ b/pkg/mediacache/cache_test.go @@ -135,7 +135,7 @@ func TestCache_PutEntryAndGetEntry(t *testing.T) { hash := HashData([]byte("pdf-content")) entry := Entry{ - Result: "契約書 — 第1条 本契約は甲乙間の...", + Result: "Contract - Article 1: This agreement between...", FilePath: "/workspace/.mediacache/abc123.md", Pages: 8, } @@ -147,7 +147,7 @@ func TestCache_PutEntryAndGetEntry(t *testing.T) { if !ok { t.Fatal("expected hit") } - if got.Result != entry.Result { + if got.Result != "Contract - Article 1: This agreement between..." { t.Errorf("Result = %q", got.Result) } if got.FilePath != entry.FilePath {