From b7107eddc97a6280a79418abeeed287c087020bb Mon Sep 17 00:00:00 2001 From: Hung Nguyen Date: Mon, 10 Aug 2026 19:51:16 +0700 Subject: [PATCH 1/3] feat(audio): provider-neutral speech-to-text seam Transcribe ahead of the loop so every provider can drive an audio-fed agent, and a long recording uploads once instead of re-sending each turn. --- README.md | 3 + pkg/audio/audio.go | 170 +++++++++++++++++++++++++++++ pkg/audio/audio_test.go | 77 +++++++++++++ pkg/llm/gemini/transcriber.go | 152 ++++++++++++++++++++++++++ pkg/llm/gemini/transcriber_test.go | 132 ++++++++++++++++++++++ pkg/llm/openai/transcriber.go | 120 ++++++++++++++++++++ pkg/llm/openai/transcriber_test.go | 161 +++++++++++++++++++++++++++ 7 files changed, 815 insertions(+) create mode 100644 pkg/audio/audio.go create mode 100644 pkg/audio/audio_test.go create mode 100644 pkg/llm/gemini/transcriber.go create mode 100644 pkg/llm/gemini/transcriber_test.go create mode 100644 pkg/llm/openai/transcriber.go create mode 100644 pkg/llm/openai/transcriber_test.go diff --git a/README.md b/README.md index 8070511..4087df5 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,9 @@ go get github.com/hung12ct/gopheragent chain for logging, timing, rate limiting, and tracing. - **Multi-provider** — OpenAI, Anthropic, Gemini, Vertex, OpenRouter, and OpenAI-compatible backends, each in its own subpackage; multi-model routing; sampling controls. +- **Speech to text** — `pkg/audio` transcribes with Whisper or Gemini behind one + interface, ahead of the loop, so any provider can drive an audio-fed agent and + a long recording is uploaded once rather than re-sent every turn. - **Sub-agents & async** — sub-agent streaming, conversation forking, background workers, first-class task tracking. - **Cross-session memory** — a post-session consolidator distills transcripts diff --git a/pkg/audio/audio.go b/pkg/audio/audio.go new file mode 100644 index 0000000..bb7ff9d --- /dev/null +++ b/pkg/audio/audio.go @@ -0,0 +1,170 @@ +// Package audio defines provider-neutral types for speech-to-text. +// +// Transcription is deliberately kept outside the agent loop. Of the three +// LLM providers this framework ships, only Gemini accepts audio in a chat +// message today; Anthropic has no audio content block at all, and the OpenAI +// Chat Completions client cannot express one. Routing audio through a +// Transcriber first turns it into ordinary text, so every provider can drive +// an audio-fed agent — and the transcript, not the waveform, is what lands in +// session history. +// +// That last point matters for cost as much as for portability. History is +// re-sent on every LLM call in a session, so inline audio in a message would +// be re-uploaded on every subsequent turn. A one-hour meeting transcribed once +// costs one upload; the same meeting carried as message parts would be billed +// again on each turn of the conversation about it. +package audio + +import ( + "context" + "errors" + "fmt" + "strings" + "time" +) + +// Sentinel errors returned by Transcriber implementations. They are separate +// types because they demand opposite responses: an oversized clip should be +// split and retried, an unsupported format should be re-encoded, and an empty +// clip is a caller bug. Route on these with errors.Is rather than matching on +// message text. +var ( + // ErrNoAudio is returned when a clip carries no sample data. + ErrNoAudio = errors.New("audio: clip has no data") + + // ErrUnsupportedFormat is returned when the clip's MIME type is not one + // the backend accepts. Re-encode and retry; retrying as-is always fails. + ErrUnsupportedFormat = errors.New("audio: unsupported format") + + // ErrTooLarge is returned when a clip exceeds the backend's per-request + // size limit. Callers feeding a live stream should respond by cutting + // shorter chunks, not by retrying the same clip. + ErrTooLarge = errors.New("audio: clip exceeds provider size limit") +) + +// Clip is a single piece of audio to transcribe. +// +// Data holds the whole clip in memory. That is deliberate rather than a +// missing optimization: both backends buffer the full payload anyway — the +// OpenAI client assembles its multipart body into a bytes.Buffer, and Gemini +// inline data is a []byte field — so an io.Reader here would add a streaming +// API that does not stream. Callers transcribing long recordings should cut +// them into chunks, which is what a live-capture pipeline does regardless. +type Clip struct { + // MIME is the IANA media type, e.g. "audio/wav" or "audio/webm". + // Parameters are allowed and ignored: browsers' MediaRecorder reports + // "audio/webm;codecs=opus", and that value is accepted verbatim. + MIME string + + // Data is the raw encoded clip — the bytes of a .wav or .webm file, not + // decoded PCM samples. + Data []byte +} + +// Options tunes a single transcription request. The zero value is valid and +// asks the backend to auto-detect everything. +type Options struct { + // Language is an ISO-639-1 hint such as "en" or "vi". Empty means + // auto-detect. Setting it cuts latency and materially improves accuracy + // on short clips, where there is little signal to detect from. + Language string + + // Prompt biases decoding toward expected vocabulary — proper nouns, + // product names, jargon, acronyms. For a chunked live stream, passing the + // tail of the previous chunk's text carries context across the seam and + // reduces mid-word splits. + Prompt string +} + +// Segment is a timed span of transcribed speech. +type Segment struct { + Start time.Duration // offset from the start of the clip + End time.Duration + Text string +} + +// Transcript is the result of transcribing one clip. +type Transcript struct { + // Text is the full transcription. Always populated on success. + Text string + + // Language is the detected or configured language, best-effort, and may + // be empty when the backend reports none. + // + // The spelling is the backend's, not a normalized code: Whisper returns + // an English name ("english"), while a backend given Options.Language + // echoes that ISO-639-1 code back. Compare it against a fixed set at your + // own risk; it is for display and logging. + Language string + + // Duration is the length of the source audio, best-effort. Zero when the + // backend does not report it. + Duration time.Duration + + // Segments carries per-span timings when the backend provides them. + // Nil is a normal result, not an error: some backends transcribe without + // emitting any timing at all. Callers that need timestamps must check for + // nil rather than assuming a populated slice. + Segments []Segment +} + +// Transcriber converts audio into text. Implementations live in the provider +// subpackages under pkg/llm. +// +// Implementations must be safe for concurrent use: a live-capture pipeline +// transcribes overlapping chunks from several goroutines to keep up with +// real time, which is the primary use for this seam. +type Transcriber interface { + Transcribe(ctx context.Context, clip Clip, opts Options) (Transcript, error) +} + +// extByMIME maps accepted media types to the file extension backends use to +// infer the container format. Both the audio/* and video/* spellings of the +// shared containers are listed: MediaRecorder emits "video/webm" for an +// audio-only recording on some browsers, and rejecting that would fail a clip +// the backend decodes fine. +var extByMIME = map[string]string{ + "audio/wav": "wav", + "audio/x-wav": "wav", + "audio/wave": "wav", + "audio/vnd.wave": "wav", + "audio/mpeg": "mp3", + "audio/mp3": "mp3", + "audio/mp4": "m4a", + "audio/x-m4a": "m4a", + "audio/webm": "webm", + "video/webm": "webm", + "video/mp4": "mp4", + "audio/ogg": "ogg", + "application/ogg": "ogg", + "audio/opus": "ogg", + "audio/flac": "flac", + "audio/x-flac": "flac", +} + +// Ext returns the file extension for a media type, or "" when the type is not +// a recognized audio container. Parameters after ";" are stripped and the type +// is matched case-insensitively, so "AUDIO/WEBM;codecs=opus" resolves to +// "webm". +func Ext(mime string) string { + base, _, _ := strings.Cut(mime, ";") + return extByMIME[strings.ToLower(strings.TrimSpace(base))] +} + +// Validate reports whether the clip is well-formed enough to send. It does not +// enforce per-provider size limits — those belong to the implementation that +// knows them. +func (c Clip) Validate() error { + if len(c.Data) == 0 { + return ErrNoAudio + } + // The sentinels already carry the package prefix, so wrapping adds only + // the detail — otherwise every message reads "audio: audio: ...". + if c.MIME == "" { + return fmt.Errorf("%w: MIME is required", ErrUnsupportedFormat) + } + if Ext(c.MIME) == "" { + return fmt.Errorf("%w: %q", ErrUnsupportedFormat, c.MIME) + } + return nil +} diff --git a/pkg/audio/audio_test.go b/pkg/audio/audio_test.go new file mode 100644 index 0000000..e820b46 --- /dev/null +++ b/pkg/audio/audio_test.go @@ -0,0 +1,77 @@ +package audio + +import ( + "errors" + "strings" + "testing" +) + +func TestExtStripsParametersAndCase(t *testing.T) { + // MediaRecorder reports the codec as a parameter, and some browsers use + // the video/* spelling for an audio-only recording. Both must resolve. + for _, tc := range []struct{ mime, want string }{ + {"audio/webm", "webm"}, + {"audio/webm;codecs=opus", "webm"}, + {"audio/webm; codecs=opus", "webm"}, + {"video/webm;codecs=opus", "webm"}, + {"AUDIO/WEBM;CODECS=OPUS", "webm"}, + {" audio/wav ", "wav"}, + {"audio/mpeg", "mp3"}, + {"audio/x-m4a", "m4a"}, + {"audio/flac", "flac"}, + {"audio/opus", "ogg"}, + {"text/plain", ""}, + {"", ""}, + } { + if got := Ext(tc.mime); got != tc.want { + t.Fatalf("Ext(%q) = %q, want %q", tc.mime, got, tc.want) + } + } +} + +func TestClipValidate(t *testing.T) { + data := []byte{0x1, 0x2} + for _, tc := range []struct { + name string + clip Clip + want error + }{ + {"ok", Clip{MIME: "audio/wav", Data: data}, nil}, + {"ok with codec parameter", Clip{MIME: "audio/webm;codecs=opus", Data: data}, nil}, + {"no data", Clip{MIME: "audio/wav"}, ErrNoAudio}, + {"no mime", Clip{Data: data}, ErrUnsupportedFormat}, + {"unknown mime", Clip{MIME: "application/pdf", Data: data}, ErrUnsupportedFormat}, + } { + t.Run(tc.name, func(t *testing.T) { + err := tc.clip.Validate() + if tc.want == nil { + if err != nil { + t.Fatalf("Validate() = %v, want nil", err) + } + return + } + if !errors.Is(err, tc.want) { + t.Fatalf("Validate() = %v, want %v", err, tc.want) + } + // The sentinels already carry the package prefix; wrapping must + // add only detail, not a second "audio: ". + if strings.Contains(err.Error(), "audio: audio:") { + t.Fatalf("Validate() = %q, want a single package prefix", err) + } + }) + } +} + +// The sentinels exist so a live-capture pipeline can route on them: a +// too-large clip should be re-cut, an unsupported one re-encoded. Distinct +// identities are the contract, so guard against a careless collapse into one. +func TestSentinelsAreDistinct(t *testing.T) { + all := []error{ErrNoAudio, ErrUnsupportedFormat, ErrTooLarge} + for i, a := range all { + for j, b := range all { + if i != j && errors.Is(a, b) { + t.Fatalf("errors.Is(%v, %v) = true, want distinct sentinels", a, b) + } + } + } +} diff --git a/pkg/llm/gemini/transcriber.go b/pkg/llm/gemini/transcriber.go new file mode 100644 index 0000000..036345e --- /dev/null +++ b/pkg/llm/gemini/transcriber.go @@ -0,0 +1,152 @@ +package gemini + +import ( + "context" + "fmt" + "os" + "strings" + + "github.com/hung12ct/gopheragent/pkg/audio" + "google.golang.org/genai" +) + +// maxInlineBytes caps what this transcriber will send as inline data. +// Gemini requires requests above roughly 20 MB to go through the Files API +// instead, which has upload-and-poll semantics that do not fit a +// single-request seam. Callers streaming a long recording should cut shorter +// chunks; ErrTooLarge tells them to. +const maxInlineBytes = 20 << 20 + +// transcribeInstruction constrains a generative model to behave like a +// transcription endpoint. Gemini will otherwise open with a preamble +// ("Here is the transcript:") or summarize instead of transcribing, and both +// corrupt a transcript that downstream code appends verbatim. +const transcribeInstruction = `You are a speech transcription engine. +Transcribe the supplied audio verbatim. +Output only the transcribed words. Do not add a preamble, commentary, translation, headings, quotation marks, or speaker labels that are not spoken aloud. +Preserve the original language; do not translate. +If the audio contains no intelligible speech, output nothing at all.` + +// Transcriber implements audio.Transcriber using Gemini's multimodal API. +// +// Gemini has no dedicated transcription endpoint, so this drives the ordinary +// generation API with an inline audio blob and a constraining instruction. +// Two consequences follow, and both are visible in the returned Transcript: +// Segments is always nil because generation emits no timing, and the result +// is a model sample rather than a decoder output, so it can paraphrase under +// adversarial audio in a way a dedicated ASR model does not. +// +// It is a good fit when Gemini is already the deployment's provider and +// timestamps are not needed. Prefer the OpenAI Transcriber when segment +// timings matter. +type Transcriber struct { + client *genai.Client + model string +} + +var _ audio.Transcriber = (*Transcriber)(nil) + +// NewTranscriber builds a transcriber. apiKey defaults to GEMINI_API_KEY; +// model defaults to "gemini-2.5-flash". +func NewTranscriber(apiKey, model string) (*Transcriber, error) { + if apiKey == "" { + apiKey = os.Getenv("GEMINI_API_KEY") + } + if apiKey == "" { + return nil, fmt.Errorf("gemini: NewTranscriber: GEMINI_API_KEY not set") + } + if model == "" { + model = "gemini-2.5-flash" + } + client, err := genai.NewClient(context.Background(), &genai.ClientConfig{APIKey: apiKey}) + if err != nil { + return nil, fmt.Errorf("gemini: NewTranscriber: %w", err) + } + return &Transcriber{client: client, model: model}, nil +} + +// Transcribe converts one clip to text. The returned Transcript never carries +// Segments; see the type doc. +func (t *Transcriber) Transcribe(ctx context.Context, clip audio.Clip, opts audio.Options) (audio.Transcript, error) { + if err := clip.Validate(); err != nil { + return audio.Transcript{}, fmt.Errorf("gemini: Transcriber: %w", err) + } + if len(clip.Data) > maxInlineBytes { + return audio.Transcript{}, fmt.Errorf("gemini: Transcriber: %w: %d bytes exceeds %d", + audio.ErrTooLarge, len(clip.Data), maxInlineBytes) + } + + contents := []*genai.Content{{ + Role: genai.RoleUser, + Parts: []*genai.Part{{InlineData: &genai.Blob{MIMEType: clip.MIME, Data: clip.Data}}}, + }} + + // Temperature 0: transcription wants the single most likely token, not a + // sample from the distribution. + var temperature float32 + config := &genai.GenerateContentConfig{ + Temperature: &temperature, + SystemInstruction: &genai.Content{ + Parts: []*genai.Part{{Text: buildInstruction(opts)}}, + }, + } + + resp, err := t.client.Models.GenerateContent(ctx, t.model, contents, config) + if err != nil { + return audio.Transcript{}, fmt.Errorf("gemini: Transcriber: transcribe: %w", classifyErr(err)) + } + return transcriptFromResponse(resp, opts) +} + +// transcriptFromResponse turns a generation response into a Transcript. +// Split out from Transcribe so the ordering below is testable without a +// client: the checks are order-sensitive in a way that is easy to regress. +func transcriptFromResponse(resp *genai.GenerateContentResponse, opts audio.Options) (audio.Transcript, error) { + // Zero candidates means the request produced nothing — normally a + // prompt-level block. That is not the same as audio containing no speech, + // and reporting it as an empty transcript would let a blocked meeting + // look like a silent one. + if resp == nil || len(resp.Candidates) == 0 { + return audio.Transcript{}, fmt.Errorf("gemini: Transcriber: no candidate returned") + } + // Checked before Content, not after: a candidate blocked for safety + // arrives with a non-STOP reason and nil Content, so testing Content + // first would report a content block as silence. A non-STOP reason also + // means any parts below are a partial answer, not the whole one — the + // same silent-prefix trap the streaming path guards. + if reasonErr := finishReasonErr(resp.Candidates[0].FinishReason); reasonErr != nil { + return audio.Transcript{}, fmt.Errorf("gemini: Transcriber: %w", reasonErr) + } + if resp.Candidates[0].Content == nil { + // Stopped cleanly with nothing to say: audio that held no + // intelligible speech. An empty transcript is the honest answer. + return audio.Transcript{Language: opts.Language}, nil + } + + var sb strings.Builder + for _, p := range resp.Candidates[0].Content.Parts { + sb.WriteString(p.Text) + } + return audio.Transcript{ + Text: strings.TrimSpace(sb.String()), + Language: opts.Language, + }, nil +} + +// buildInstruction folds the caller's hints into the system instruction. +// Gemini has no language or prompt parameter equivalent to a dedicated ASR +// endpoint, so both have to travel as text. +func buildInstruction(opts audio.Options) string { + if opts.Language == "" && opts.Prompt == "" { + return transcribeInstruction + } + var sb strings.Builder + sb.WriteString(transcribeInstruction) + if opts.Language != "" { + fmt.Fprintf(&sb, "\nThe audio is expected to be in this language (ISO 639-1): %s", opts.Language) + } + if opts.Prompt != "" { + fmt.Fprintf(&sb, "\nExpect these terms, spelled exactly as given: %s", opts.Prompt) + } + return sb.String() +} diff --git a/pkg/llm/gemini/transcriber_test.go b/pkg/llm/gemini/transcriber_test.go new file mode 100644 index 0000000..8571474 --- /dev/null +++ b/pkg/llm/gemini/transcriber_test.go @@ -0,0 +1,132 @@ +package gemini + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/hung12ct/gopheragent/pkg/audio" + "google.golang.org/genai" +) + +// Gemini has no language or vocabulary parameter, so both hints have to reach +// the model as text. Dropping them silently would degrade accuracy with +// nothing in the request to show why. +func TestBuildInstructionCarriesHints(t *testing.T) { + base := buildInstruction(audio.Options{}) + if base != transcribeInstruction { + t.Fatal("empty Options must not alter the instruction") + } + + withHints := buildInstruction(audio.Options{Language: "vi", Prompt: "GopherAgent, Parakeet"}) + if !strings.HasPrefix(withHints, transcribeInstruction) { + t.Fatal("hints must extend the base instruction, not replace it") + } + for _, want := range []string{"vi", "GopherAgent, Parakeet"} { + if !strings.Contains(withHints, want) { + t.Fatalf("instruction missing %q:\n%s", want, withHints) + } + } + + // A language hint alone must not smuggle in an empty vocabulary line. + langOnly := buildInstruction(audio.Options{Language: "en"}) + if strings.Contains(langOnly, "Expect these terms") { + t.Fatalf("empty Prompt produced a vocabulary line:\n%s", langOnly) + } +} + +// Validation runs before the client is touched, so these fail fast without a +// network call or an API key. +func TestTranscribeRejectsInvalidClipBeforeCallingAPI(t *testing.T) { + tr := &Transcriber{} + for _, tc := range []struct { + name string + clip audio.Clip + want error + }{ + {"empty", audio.Clip{MIME: "audio/wav"}, audio.ErrNoAudio}, + {"unsupported", audio.Clip{MIME: "application/pdf", Data: []byte("x")}, audio.ErrUnsupportedFormat}, + {"oversized", audio.Clip{MIME: "audio/wav", Data: make([]byte, maxInlineBytes+1)}, audio.ErrTooLarge}, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := tr.Transcribe(context.Background(), tc.clip, audio.Options{}); !errors.Is(err, tc.want) { + t.Fatalf("Transcribe error = %v, want %v", err, tc.want) + } + }) + } +} + +func TestTranscriptFromResponse(t *testing.T) { + content := func(text string) *genai.Content { + return &genai.Content{Parts: []*genai.Part{{Text: text}}} + } + candidate := func(c *genai.Candidate) *genai.GenerateContentResponse { + return &genai.GenerateContentResponse{Candidates: []*genai.Candidate{c}} + } + + t.Run("joins and trims parts", func(t *testing.T) { + resp := &genai.GenerateContentResponse{Candidates: []*genai.Candidate{{ + FinishReason: genai.FinishReasonStop, + Content: &genai.Content{Parts: []*genai.Part{{Text: " hello "}, {Text: "world "}}}, + }}} + out, err := transcriptFromResponse(resp, audio.Options{Language: "en"}) + if err != nil { + t.Fatalf("transcriptFromResponse: %v", err) + } + if out.Text != "hello world" { + t.Fatalf("Text = %q, want %q", out.Text, "hello world") + } + if out.Language != "en" { + t.Fatalf("Language = %q, want en", out.Language) + } + if out.Segments != nil { + t.Fatal("generation emits no timing; Segments must stay nil") + } + }) + + // A clean stop with nothing to say is audio that held no speech. + t.Run("clean stop with no content is empty, not an error", func(t *testing.T) { + out, err := transcriptFromResponse(candidate(&genai.Candidate{ + FinishReason: genai.FinishReasonStop, + }), audio.Options{}) + if err != nil { + t.Fatalf("transcriptFromResponse: %v", err) + } + if out.Text != "" { + t.Fatalf("Text = %q, want empty", out.Text) + } + }) + + // Regression: a safety block arrives as a non-STOP reason with nil + // Content. Testing Content before FinishReason reported that as silence, + // so a blocked recording was indistinguishable from a quiet one. + t.Run("safety block with nil content errors", func(t *testing.T) { + _, err := transcriptFromResponse(candidate(&genai.Candidate{ + FinishReason: genai.FinishReasonSafety, + }), audio.Options{}) + if err == nil { + t.Fatal("a safety block with nil Content must not read as an empty transcript") + } + }) + + // A truncated answer is a partial transcript; returning its prefix as if + // whole is the silent-prefix trap. + t.Run("truncation errors rather than returning a prefix", func(t *testing.T) { + _, err := transcriptFromResponse(candidate(&genai.Candidate{ + FinishReason: genai.FinishReasonMaxTokens, + Content: content("first half of the meeting"), + }), audio.Options{}) + if err == nil { + t.Fatal("truncated response must error, not return the prefix") + } + }) + + t.Run("no candidates errors", func(t *testing.T) { + for _, resp := range []*genai.GenerateContentResponse{nil, {}} { + if _, err := transcriptFromResponse(resp, audio.Options{}); err == nil { + t.Fatalf("resp %+v: want error for a response with no candidate", resp) + } + } + }) +} diff --git a/pkg/llm/openai/transcriber.go b/pkg/llm/openai/transcriber.go new file mode 100644 index 0000000..466bfbe --- /dev/null +++ b/pkg/llm/openai/transcriber.go @@ -0,0 +1,120 @@ +package openai + +import ( + "bytes" + "context" + "fmt" + "strings" + "time" + + "github.com/hung12ct/gopheragent/pkg/audio" + "github.com/sashabaranov/go-openai" +) + +// maxClipBytes is the OpenAI audio endpoint's documented per-request limit +// (25 MB). Checked before upload so an oversized clip fails immediately +// instead of after transferring the payload. +const maxClipBytes = 25 << 20 + +// Transcriber implements audio.Transcriber against OpenAI's audio +// transcription endpoint. +// +// Model choice changes the shape of the result. whisper-1 supports the +// verbose_json response format and so populates Transcript.Segments, +// Language, and Duration. The gpt-4o-transcribe family accepts only json and +// text, so it returns text alone with Segments nil — correct, but useless to +// a caller that needs timestamps. whisper-1 is the default for that reason. +// +// Pass WithBaseURL to transcribe against an OpenAI-compatible endpoint (a +// self-hosted Whisper server, Groq) instead of api.openai.com. +type Transcriber struct { + client *openai.Client + model string +} + +var _ audio.Transcriber = (*Transcriber)(nil) + +// NewTranscriber constructs a transcriber. apiKey falls back to +// OPENAI_API_KEY. model defaults to whisper-1. +func NewTranscriber(apiKey string, model string, opts ...ClientOption) (*Transcriber, error) { + client, err := newClientFor(apiKey, "NewTranscriber", opts) + if err != nil { + return nil, err + } + if model == "" { + model = openai.Whisper1 + } + return &Transcriber{client: client, model: model}, nil +} + +// Transcribe converts one clip to text. +func (t *Transcriber) Transcribe(ctx context.Context, clip audio.Clip, opts audio.Options) (audio.Transcript, error) { + if err := clip.Validate(); err != nil { + return audio.Transcript{}, fmt.Errorf("openai: Transcriber: %w", err) + } + if len(clip.Data) > maxClipBytes { + return audio.Transcript{}, fmt.Errorf("openai: Transcriber: %w: %d bytes exceeds %d", + audio.ErrTooLarge, len(clip.Data), maxClipBytes) + } + + format := openai.AudioResponseFormatJSON + var granularity []openai.TranscriptionTimestampGranularity + if t.supportsSegments() { + format = openai.AudioResponseFormatVerboseJSON + granularity = []openai.TranscriptionTimestampGranularity{ + openai.TranscriptionTimestampGranularitySegment, + } + } + + // With Reader set, FilePath is a filename hint for the multipart form + // rather than a path on disk. The endpoint infers the container from its + // extension, so a wrong extension rejects a clip that would decode fine. + resp, err := t.client.CreateTranscription(ctx, openai.AudioRequest{ + Model: t.model, + FilePath: "clip." + audio.Ext(clip.MIME), + Reader: bytes.NewReader(clip.Data), + Language: opts.Language, + Prompt: opts.Prompt, + Format: format, + TimestampGranularities: granularity, + }) + if err != nil { + return audio.Transcript{}, fmt.Errorf("openai: Transcriber: transcribe: %w", classifyErr(err)) + } + + out := audio.Transcript{ + Text: strings.TrimSpace(resp.Text), + Language: resp.Language, + Duration: secondsToDuration(resp.Duration), + } + if len(resp.Segments) > 0 { + out.Segments = make([]audio.Segment, 0, len(resp.Segments)) + for _, s := range resp.Segments { + out.Segments = append(out.Segments, audio.Segment{ + Start: secondsToDuration(s.Start), + End: secondsToDuration(s.End), + Text: strings.TrimSpace(s.Text), + }) + } + } + return out, nil +} + +// supportsSegments reports whether the configured model accepts the +// verbose_json response format. Only the whisper family does; asking for it +// elsewhere fails the request outright rather than degrading, so this must +// stay conservative in the other direction — a missed match costs timings, +// a false match costs the whole request. +// +// Matched as a substring rather than a prefix because compatible endpoints +// name the same weights differently: "Systran/faster-whisper-large-v3" and +// "ggml-whisper.cpp" both speak verbose_json. +func (t *Transcriber) supportsSegments() bool { + return strings.Contains(strings.ToLower(t.model), "whisper") +} + +// secondsToDuration converts the endpoint's fractional seconds to a Duration +// without losing sub-second precision to integer truncation. +func secondsToDuration(sec float64) time.Duration { + return time.Duration(sec * float64(time.Second)) +} diff --git a/pkg/llm/openai/transcriber_test.go b/pkg/llm/openai/transcriber_test.go new file mode 100644 index 0000000..d5c3d8b --- /dev/null +++ b/pkg/llm/openai/transcriber_test.go @@ -0,0 +1,161 @@ +package openai + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/hung12ct/gopheragent/pkg/audio" +) + +// transcriptionServer captures the multipart body the client sends and +// replies with a fixed verbose_json payload. +func transcriptionServer(t *testing.T, body string) (*httptest.Server, *string) { + t.Helper() + var got string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + raw, _ := io.ReadAll(r.Body) + got = string(raw) + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, body) + })) + t.Cleanup(srv.Close) + return srv, &got +} + +func newTestTranscriber(t *testing.T, srv *httptest.Server, model string) *Transcriber { + t.Helper() + tr, err := NewTranscriber("test-key", model, WithBaseURL(srv.URL+"/v1")) + if err != nil { + t.Fatalf("NewTranscriber: %v", err) + } + return tr +} + +func TestTranscribeMapsSegmentsAndDuration(t *testing.T) { + const resp = `{"task":"transcribe","language":"english","duration":3.5, + "segments":[{"id":0,"start":0.0,"end":1.25,"text":" Hello there"}, + {"id":1,"start":1.25,"end":3.5,"text":" second part "}], + "text":" Hello there second part "}` + srv, _ := transcriptionServer(t, resp) + + out, err := newTestTranscriber(t, srv, ""). + Transcribe(context.Background(), audio.Clip{MIME: "audio/wav", Data: []byte("RIFF")}, audio.Options{}) + if err != nil { + t.Fatalf("Transcribe: %v", err) + } + if out.Text != "Hello there second part" { + t.Fatalf("Text = %q, want trimmed transcript", out.Text) + } + if out.Language != "english" { + t.Fatalf("Language = %q, want english", out.Language) + } + // 3.5s must survive as sub-second precision, not truncate to 3s. + if out.Duration != 3500*time.Millisecond { + t.Fatalf("Duration = %v, want 3.5s", out.Duration) + } + if len(out.Segments) != 2 { + t.Fatalf("Segments = %d, want 2", len(out.Segments)) + } + if out.Segments[0].End != 1250*time.Millisecond || out.Segments[0].Text != "Hello there" { + t.Fatalf("Segments[0] = %+v, want end 1.25s and trimmed text", out.Segments[0]) + } +} + +// The filename extension is the only signal the endpoint has for the +// container format, so a wrong one rejects a clip that would decode. A codec +// parameter in the MIME type must not leak into it. +func TestTranscribeSendsExtensionDerivedFromMIME(t *testing.T) { + srv, body := transcriptionServer(t, `{"text":"ok"}`) + tr := newTestTranscriber(t, srv, "") + + clip := audio.Clip{MIME: "audio/webm;codecs=opus", Data: []byte("webm-bytes")} + if _, err := tr.Transcribe(context.Background(), clip, audio.Options{}); err != nil { + t.Fatalf("Transcribe: %v", err) + } + if !strings.Contains(*body, `filename="clip.webm"`) { + t.Fatalf("multipart body missing clip.webm filename:\n%s", *body) + } +} + +func TestTranscribeForwardsLanguageAndPrompt(t *testing.T) { + srv, body := transcriptionServer(t, `{"text":"ok"}`) + tr := newTestTranscriber(t, srv, "") + + opts := audio.Options{Language: "vi", Prompt: "GopherAgent, Parakeet"} + clip := audio.Clip{MIME: "audio/wav", Data: []byte("RIFF")} + if _, err := tr.Transcribe(context.Background(), clip, opts); err != nil { + t.Fatalf("Transcribe: %v", err) + } + for _, want := range []string{"vi", "GopherAgent, Parakeet"} { + if !strings.Contains(*body, want) { + t.Fatalf("multipart body missing %q:\n%s", want, *body) + } + } +} + +// gpt-4o-transcribe rejects verbose_json outright, so asking for it would +// fail every request rather than degrading to a transcript without timings. +func TestTranscribeOmitsVerboseFormatForNonWhisperModels(t *testing.T) { + for _, tc := range []struct{ model, wantFormat string }{ + {"whisper-1", "verbose_json"}, + {"gpt-4o-transcribe", "json"}, + } { + t.Run(tc.model, func(t *testing.T) { + srv, body := transcriptionServer(t, `{"text":"ok"}`) + tr := newTestTranscriber(t, srv, tc.model) + clip := audio.Clip{MIME: "audio/wav", Data: []byte("RIFF")} + if _, err := tr.Transcribe(context.Background(), clip, audio.Options{}); err != nil { + t.Fatalf("Transcribe: %v", err) + } + if !strings.Contains(*body, tc.wantFormat) { + t.Fatalf("model %s: body missing response_format %s:\n%s", tc.model, tc.wantFormat, *body) + } + if tc.wantFormat == "json" && strings.Contains(*body, "verbose_json") { + t.Fatalf("model %s: body requested verbose_json:\n%s", tc.model, *body) + } + }) + } +} + +// An oversized clip must fail before the upload, and as ErrTooLarge rather +// than a generic error, so a capture pipeline knows to cut shorter chunks. +func TestTranscribeRejectsOversizedClipWithoutCallingAPI(t *testing.T) { + called := false + srv := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { called = true })) + defer srv.Close() + + clip := audio.Clip{MIME: "audio/wav", Data: make([]byte, maxClipBytes+1)} + _, err := newTestTranscriber(t, srv, "").Transcribe(context.Background(), clip, audio.Options{}) + if !errors.Is(err, audio.ErrTooLarge) { + t.Fatalf("Transcribe error = %v, want ErrTooLarge", err) + } + if called { + t.Fatal("oversized clip was uploaded; want a local rejection") + } +} + +func TestTranscribeRejectsInvalidClip(t *testing.T) { + srv, _ := transcriptionServer(t, `{"text":"ok"}`) + tr := newTestTranscriber(t, srv, "") + + for _, tc := range []struct { + name string + clip audio.Clip + want error + }{ + {"empty", audio.Clip{MIME: "audio/wav"}, audio.ErrNoAudio}, + {"unsupported", audio.Clip{MIME: "application/pdf", Data: []byte("x")}, audio.ErrUnsupportedFormat}, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := tr.Transcribe(context.Background(), tc.clip, audio.Options{}); !errors.Is(err, tc.want) { + t.Fatalf("Transcribe error = %v, want %v", err, tc.want) + } + }) + } +} From 3b74f884dd8d279afd7ef598b2d74a908cfedb36 Mon Sep 17 00:00:00 2001 From: Hung Nguyen Date: Mon, 10 Aug 2026 19:51:25 +0700 Subject: [PATCH 2/3] fix(llm): error on unrenderable media parts instead of dropping them A dropped part silently changes the question: the model answers from the caption alone, indistinguishable from success in the response and the logs. --- pkg/agent/errors.go | 16 +++++++ pkg/agent/retry.go | 8 ++-- pkg/history/types.go | 18 +++++-- pkg/llm/anthropic/anthropic.go | 42 ++++++++++++---- pkg/llm/anthropic/multimodal_test.go | 72 ++++++++++++++++++++++++++-- pkg/llm/gemini/gemini.go | 37 +++++++++++--- pkg/llm/gemini/multimodal_test.go | 70 +++++++++++++++++++++++++-- pkg/llm/openai/multimodal_test.go | 61 +++++++++++++++++++---- pkg/llm/openai/openai.go | 35 +++++++++++--- 9 files changed, 317 insertions(+), 42 deletions(-) diff --git a/pkg/agent/errors.go b/pkg/agent/errors.go index 7188e2d..901c553 100644 --- a/pkg/agent/errors.go +++ b/pkg/agent/errors.go @@ -93,6 +93,22 @@ var ( // caller must change the request or surface the block to the user. ErrLLMContentBlocked = errors.New("agent: LLM stopped generating for a content policy") + // ErrUnrenderablePart is returned by a provider adapter when a message + // carries a history.MediaPart the adapter cannot put on the wire — an + // unsupported part type, or a part whose payload is missing. + // + // Failing is the point. The alternative, dropping the part and sending + // the rest, produces a fluent well-formed answer from a model that never + // received the media, and nothing distinguishes that from success: not + // the response, not the logs, not a schema check. A judge that cannot see + // the image it is judging is not a degraded judge, it is a random one. + // + // Deterministic like ErrLLMAuth, not transient like ErrLLMFailure — the + // same message fails identically on every retry. isRetryable treats it as + // terminal; the caller must re-encode the part or choose a provider that + // supports it. + ErrUnrenderablePart = errors.New("agent: message carries a media part this provider cannot render") + // ErrContextCancelled is returned when the request context is cancelled mid-loop. ErrContextCancelled = errors.New("agent: operation cancelled") diff --git a/pkg/agent/retry.go b/pkg/agent/retry.go index 69cb99c..f26affb 100644 --- a/pkg/agent/retry.go +++ b/pkg/agent/retry.go @@ -57,13 +57,15 @@ func (r *RetryConfig) delay(attempt int) time.Duration { } // isRetryable returns false for errors that fail identically on every -// attempt: context-level cancellation, and a provider content-policy stop -// (deterministic for a given prompt — retrying only burns the budget). +// attempt: context-level cancellation, a provider content-policy stop +// (deterministic for a given prompt — retrying only burns the budget), and a +// message the adapter cannot render (the same bytes fail the same way). func isRetryable(err error) bool { if err == nil { return false } return !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) && - !errors.Is(err, ErrLLMContentBlocked) + !errors.Is(err, ErrLLMContentBlocked) && + !errors.Is(err, ErrUnrenderablePart) } diff --git a/pkg/history/types.go b/pkg/history/types.go index 97aea2b..046c671 100644 --- a/pkg/history/types.go +++ b/pkg/history/types.go @@ -72,10 +72,20 @@ type Message struct { } // PartType enumerates the kinds of content a MediaPart can hold. -// Video and audio are intentionally omitted for now — Anthropic does not -// accept them in messages today, and Gemini video requires Files-API upload -// semantics that do not fit the simple inline model. Video remains on the -// tool path via media_analyze. +// +// Video and audio are intentionally omitted. Anthropic accepts neither in a +// message, and Gemini video requires Files-API upload semantics that do not +// fit the simple inline model. Video remains on the tool path via +// media_analyze; audio goes through pkg/audio, whose Transcriber converts it +// to text before it reaches a message. +// +// Transcribing rather than embedding is also the cheaper shape for anything +// long. History is re-sent on every LLM call in a session, so audio carried +// as a message part is re-uploaded on every subsequent turn — a one-hour +// recording would be billed again on each turn of the conversation about it. +// +// A part whose type an adapter cannot render fails the request with +// agent.ErrUnrenderablePart rather than being dropped. type PartType string const ( diff --git a/pkg/llm/anthropic/anthropic.go b/pkg/llm/anthropic/anthropic.go index 819c3e4..11289fe 100644 --- a/pkg/llm/anthropic/anthropic.go +++ b/pkg/llm/anthropic/anthropic.go @@ -105,6 +105,14 @@ func (p *Provider) GenerateStream(ctx context.Context, memory []history.Message, memory = agent.PatchDanglingToolCalls(memory) for _, m := range memory { + // Only user messages render media. Parts on any other role are + // rejected rather than dropped: the branches below read Content + // alone, so passing them through would answer from media the model + // never received. + if len(m.Parts) > 0 && m.Role != "user" { + return agent.LLMResult{}, fmt.Errorf("anthropic: %w: %s message carries %d media parts, which this API accepts only on user messages", + agent.ErrUnrenderablePart, m.Role, len(m.Parts)) + } switch m.Role { case "system": block := anthropic.TextBlockParam{Text: m.Content} @@ -115,7 +123,11 @@ func (p *Provider) GenerateStream(ctx context.Context, memory []history.Message, case "user": var blocks []anthropic.ContentBlockParamUnion if len(m.Parts) > 0 { - blocks = blocksFromMediaParts(m.Content, m.Parts) + rendered, err := blocksFromMediaParts(m.Content, m.Parts) + if err != nil { + return agent.LLMResult{}, err + } + blocks = rendered } else { blocks = []anthropic.ContentBlockParamUnion{anthropic.NewTextBlock(m.Content)} } @@ -470,15 +482,19 @@ func stampCacheControl(block *anthropic.ContentBlockParamUnion) { // the image. // // Raw bytes are base64-encoded; URLs pass through as URLImageSourceParam. -// Parts with no usable payload are silently skipped (the alternative — -// surfacing an error — would force every caller to pre-validate, which is -// more trouble than it's worth for a format issue). -func blocksFromMediaParts(caption string, parts []history.MediaPart) []anthropic.ContentBlockParamUnion { +// +// A part this adapter cannot render fails the whole message with +// agent.ErrUnrenderablePart. Skipping it was the earlier behavior and was +// wrong: Anthropic accepts no audio or video block at all, so a caller who +// sent one got back a confident answer from a model that received only the +// caption, indistinguishable from success. See that error's doc. Empty text +// parts remain skipped — they carry nothing to lose. +func blocksFromMediaParts(caption string, parts []history.MediaPart) ([]anthropic.ContentBlockParamUnion, error) { blocks := make([]anthropic.ContentBlockParamUnion, 0, len(parts)+1) if caption != "" { blocks = append(blocks, anthropic.NewTextBlock(caption)) } - for _, p := range parts { + for i, p := range parts { switch p.Type { case history.PartText: if p.Text == "" { @@ -486,16 +502,24 @@ func blocksFromMediaParts(caption string, parts []history.MediaPart) []anthropic } blocks = append(blocks, anthropic.NewTextBlock(p.Text)) case history.PartImage: - if len(p.Data) > 0 { + switch { + case len(p.Data) > 0: mime := p.MIME if mime == "" { mime = "image/png" } blocks = append(blocks, anthropic.NewImageBlockBase64(mime, base64.StdEncoding.EncodeToString(p.Data))) - } else if p.URL != "" { + case p.URL != "": blocks = append(blocks, anthropic.NewImageBlock(anthropic.URLImageSourceParam{URL: p.URL})) + default: + return nil, fmt.Errorf("anthropic: %w: part %d is an image with neither URL nor Data", agent.ErrUnrenderablePart, i) } + default: + return nil, fmt.Errorf("anthropic: %w: part %d has unsupported type %q", agent.ErrUnrenderablePart, i, p.Type) } } - return blocks + if len(blocks) == 0 { + return nil, fmt.Errorf("anthropic: %w: %d parts produced no renderable content", agent.ErrUnrenderablePart, len(parts)) + } + return blocks, nil } diff --git a/pkg/llm/anthropic/multimodal_test.go b/pkg/llm/anthropic/multimodal_test.go index 85dde0c..4e968a9 100644 --- a/pkg/llm/anthropic/multimodal_test.go +++ b/pkg/llm/anthropic/multimodal_test.go @@ -1,10 +1,13 @@ package anthropic import ( + "context" "encoding/base64" + "errors" "strings" "testing" + "github.com/hung12ct/gopheragent/pkg/agent" "github.com/hung12ct/gopheragent/pkg/history" ) @@ -16,9 +19,12 @@ import ( var pngBytes = []byte{0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a} func TestAnthropic_CaptionPlusImageURL(t *testing.T) { - blocks := blocksFromMediaParts("describe", []history.MediaPart{ + blocks, err := blocksFromMediaParts("describe", []history.MediaPart{ history.NewImagePartURL("image/jpeg", "https://example.com/a.jpg"), }) + if err != nil { + t.Fatalf("blocksFromMediaParts: %v", err) + } if len(blocks) != 2 { t.Fatalf("expected 2 blocks, got %d", len(blocks)) } @@ -34,9 +40,12 @@ func TestAnthropic_CaptionPlusImageURL(t *testing.T) { } func TestAnthropic_BytesUseBase64Source(t *testing.T) { - blocks := blocksFromMediaParts("", []history.MediaPart{ + blocks, err := blocksFromMediaParts("", []history.MediaPart{ history.NewImagePartBytes("image/png", pngBytes), }) + if err != nil { + t.Fatalf("blocksFromMediaParts: %v", err) + } if len(blocks) != 1 { t.Fatalf("expected 1 block, got %d", len(blocks)) } @@ -55,9 +64,12 @@ func TestAnthropic_BytesUseBase64Source(t *testing.T) { func TestAnthropic_DefaultMIME(t *testing.T) { // No MIME on a bytes part — adapter must default to image/png. - blocks := blocksFromMediaParts("", []history.MediaPart{ + blocks, err := blocksFromMediaParts("", []history.MediaPart{ {Type: history.PartImage, Data: pngBytes}, }) + if err != nil { + t.Fatalf("blocksFromMediaParts: %v", err) + } if len(blocks) != 1 { t.Fatalf("expected 1 block") } @@ -65,3 +77,57 @@ func TestAnthropic_DefaultMIME(t *testing.T) { t.Fatalf("expected image/* default, got %q", blocks[0].OfImage.Source.OfBase64.MediaType) } } + +// An empty text part carries nothing to lose, so it is dropped. Everything +// else this adapter cannot render fails the message: Anthropic has no audio +// or video block at all, and dropping one would answer confidently from media +// the model never received. +func TestAnthropic_DropsEmptyTextButRejectsUnrenderable(t *testing.T) { + blocks, err := blocksFromMediaParts("", []history.MediaPart{ + {Type: history.PartText, Text: ""}, + history.NewImagePartURL("image/png", "https://x/a"), + }) + if err != nil { + t.Fatalf("empty text part must be dropped, not rejected: %v", err) + } + if len(blocks) != 1 { + t.Fatalf("expected 1 block after dropping empty text, got %d", len(blocks)) + } + + for _, tc := range []struct { + name string + part history.MediaPart + }{ + {"image without payload", history.MediaPart{Type: history.PartImage, MIME: "image/png"}}, + {"audio part", history.MediaPart{Type: history.PartType("audio"), MIME: "audio/wav", Data: pngBytes}}, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := blocksFromMediaParts("caption", []history.MediaPart{tc.part}); !errors.Is(err, agent.ErrUnrenderablePart) { + t.Fatalf("error = %v, want ErrUnrenderablePart", err) + } + }) + } +} + +// Parts on a non-user role are rejected at the message level. The branches +// for system/assistant/tool read Content alone, so letting them through would +// answer from media the model never received — the same silent drop this +// adapter used to do inside blocksFromMediaParts. +func TestAnthropic_RejectsPartsOnNonUserRole(t *testing.T) { + p, err := New("test-key", "claude-sonnet-4-20250514") + if err != nil { + t.Fatalf("New: %v", err) + } + for _, role := range []string{"system", "assistant", "tool"} { + t.Run(role, func(t *testing.T) { + memory := []history.Message{{ + Role: role, + Parts: []history.MediaPart{history.NewImagePartURL("image/png", "https://x/a")}, + }} + _, err := p.GenerateStream(context.Background(), memory, nil, make(chan agent.StreamEvent, 1)) + if !errors.Is(err, agent.ErrUnrenderablePart) { + t.Fatalf("GenerateStream error = %v, want ErrUnrenderablePart", err) + } + }) + } +} diff --git a/pkg/llm/gemini/gemini.go b/pkg/llm/gemini/gemini.go index afc5e34..45335f8 100644 --- a/pkg/llm/gemini/gemini.go +++ b/pkg/llm/gemini/gemini.go @@ -90,6 +90,14 @@ func (p *Provider) GenerateStream(ctx context.Context, memory []history.Message, var systemInstruction *genai.Content for _, m := range memory { + // Only user messages render media. Parts on any other role are + // rejected rather than dropped: the branches below read Content + // alone, so passing them through would answer from media the model + // never received. + if len(m.Parts) > 0 && m.Role != "user" { + return agent.LLMResult{}, fmt.Errorf("gemini: %w: %s message carries %d media parts, which this API accepts only on user messages", + agent.ErrUnrenderablePart, m.Role, len(m.Parts)) + } if m.Role == "system" { systemInstruction = &genai.Content{ Parts: []*genai.Part{{Text: m.Content}}, @@ -104,7 +112,11 @@ func (p *Provider) GenerateStream(ctx context.Context, memory []history.Message, case "user": role = "user" if len(m.Parts) > 0 { - parts = append(parts, partsFromMediaParts(m.Content, m.Parts)...) + rendered, err := partsFromMediaParts(m.Content, m.Parts) + if err != nil { + return agent.LLMResult{}, err + } + parts = append(parts, rendered...) } else { parts = append(parts, &genai.Part{Text: m.Content}) } @@ -265,12 +277,17 @@ func (p *Provider) applySampling(config *genai.GenerateContentConfig) { // // A non-empty caption is prepended as a text part so prompts travel // alongside the media. MIME defaults to image/png when unspecified. -func partsFromMediaParts(caption string, parts []history.MediaPart) []*genai.Part { +// +// A part this adapter cannot render fails the whole message with +// agent.ErrUnrenderablePart rather than being skipped; see that error's doc +// for why silence is the worse outcome. Empty text parts are the one +// exception — they carry nothing to lose. +func partsFromMediaParts(caption string, parts []history.MediaPart) ([]*genai.Part, error) { out := make([]*genai.Part, 0, len(parts)+1) if caption != "" { out = append(out, &genai.Part{Text: caption}) } - for _, p := range parts { + for i, p := range parts { switch p.Type { case history.PartText: if p.Text == "" { @@ -282,18 +299,26 @@ func partsFromMediaParts(caption string, parts []history.MediaPart) []*genai.Par if mime == "" { mime = "image/png" } - if len(p.Data) > 0 { + switch { + case len(p.Data) > 0: out = append(out, &genai.Part{ InlineData: &genai.Blob{MIMEType: mime, Data: p.Data}, }) - } else if p.URL != "" { + case p.URL != "": out = append(out, &genai.Part{ FileData: &genai.FileData{FileURI: p.URL, MIMEType: mime}, }) + default: + return nil, fmt.Errorf("gemini: %w: part %d is an image with neither URL nor Data", agent.ErrUnrenderablePart, i) } + default: + return nil, fmt.Errorf("gemini: %w: part %d has unsupported type %q", agent.ErrUnrenderablePart, i, p.Type) } } - return out + if len(out) == 0 { + return nil, fmt.Errorf("gemini: %w: %d parts produced no renderable content", agent.ErrUnrenderablePart, len(parts)) + } + return out, nil } // applyStructuredOutput translates the ctx-carried StructuredOutput diff --git a/pkg/llm/gemini/multimodal_test.go b/pkg/llm/gemini/multimodal_test.go index 2cc05ef..174261e 100644 --- a/pkg/llm/gemini/multimodal_test.go +++ b/pkg/llm/gemini/multimodal_test.go @@ -1,8 +1,11 @@ package gemini import ( + "context" + "errors" "testing" + "github.com/hung12ct/gopheragent/pkg/agent" "github.com/hung12ct/gopheragent/pkg/history" ) @@ -14,9 +17,12 @@ import ( var pngBytes = []byte{0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a} func TestGemini_BytesBecomeInlineData(t *testing.T) { - parts := partsFromMediaParts("look", []history.MediaPart{ + parts, err := partsFromMediaParts("look", []history.MediaPart{ history.NewImagePartBytes("image/png", pngBytes), }) + if err != nil { + t.Fatalf("partsFromMediaParts: %v", err) + } if len(parts) != 2 { t.Fatalf("expected 2 parts, got %d", len(parts)) } @@ -32,9 +38,12 @@ func TestGemini_BytesBecomeInlineData(t *testing.T) { } func TestGemini_URLBecomesFileData(t *testing.T) { - parts := partsFromMediaParts("", []history.MediaPart{ + parts, err := partsFromMediaParts("", []history.MediaPart{ history.NewImagePartURL("image/jpeg", "gs://bucket/obj.jpg"), }) + if err != nil { + t.Fatalf("partsFromMediaParts: %v", err) + } if len(parts) != 1 { t.Fatalf("expected 1 part, got %d", len(parts)) } @@ -47,12 +56,67 @@ func TestGemini_URLBecomesFileData(t *testing.T) { } func TestGemini_InterleavedTextAndImage(t *testing.T) { - parts := partsFromMediaParts("", []history.MediaPart{ + parts, err := partsFromMediaParts("", []history.MediaPart{ history.NewTextPart("A"), history.NewImagePartBytes("image/png", pngBytes), history.NewTextPart("B"), }) + if err != nil { + t.Fatalf("partsFromMediaParts: %v", err) + } if len(parts) != 3 || parts[0].Text != "A" || parts[2].Text != "B" { t.Fatalf("expected interleaved text parts preserved, got %+v", parts) } } + +// An empty text part carries nothing to lose, so it is dropped. Everything +// else this adapter cannot render fails the message rather than answering +// from media the model never received. +func TestGemini_DropsEmptyTextButRejectsUnrenderable(t *testing.T) { + parts, err := partsFromMediaParts("", []history.MediaPart{ + {Type: history.PartText, Text: ""}, + history.NewImagePartURL("image/png", "https://x/a"), + }) + if err != nil { + t.Fatalf("empty text part must be dropped, not rejected: %v", err) + } + if len(parts) != 1 { + t.Fatalf("expected 1 part after dropping empty text, got %d", len(parts)) + } + + for _, tc := range []struct { + name string + part history.MediaPart + }{ + {"image without payload", history.MediaPart{Type: history.PartImage, MIME: "image/png"}}, + {"unknown part type", history.MediaPart{Type: history.PartType("audio"), MIME: "audio/wav", Data: pngBytes}}, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := partsFromMediaParts("caption", []history.MediaPart{tc.part}); !errors.Is(err, agent.ErrUnrenderablePart) { + t.Fatalf("error = %v, want ErrUnrenderablePart", err) + } + }) + } +} + +// Parts on a non-user role are rejected at the message level. The branches +// for system/assistant/tool read Content alone, so letting them through would +// answer from media the model never received. +func TestGemini_RejectsPartsOnNonUserRole(t *testing.T) { + p, err := New("test-key", "gemini-2.5-flash") + if err != nil { + t.Fatalf("New: %v", err) + } + for _, role := range []string{"system", "assistant", "tool"} { + t.Run(role, func(t *testing.T) { + memory := []history.Message{{ + Role: role, + Parts: []history.MediaPart{history.NewImagePartURL("image/png", "https://x/a")}, + }} + _, err := p.GenerateStream(context.Background(), memory, nil, make(chan agent.StreamEvent, 1)) + if !errors.Is(err, agent.ErrUnrenderablePart) { + t.Fatalf("GenerateStream error = %v, want ErrUnrenderablePart", err) + } + }) + } +} diff --git a/pkg/llm/openai/multimodal_test.go b/pkg/llm/openai/multimodal_test.go index 5746a53..61058d6 100644 --- a/pkg/llm/openai/multimodal_test.go +++ b/pkg/llm/openai/multimodal_test.go @@ -1,9 +1,12 @@ package openai import ( + "context" "encoding/base64" + "errors" "testing" + "github.com/hung12ct/gopheragent/pkg/agent" "github.com/hung12ct/gopheragent/pkg/history" ) @@ -15,9 +18,12 @@ import ( var pngBytes = []byte{0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a} func TestOpenAI_CaptionPlusImageURL(t *testing.T) { - parts := partsFromMediaParts("what's in this?", []history.MediaPart{ + parts, err := partsFromMediaParts("what's in this?", []history.MediaPart{ history.NewImagePartURL("image/jpeg", "https://example.com/a.jpg"), }) + if err != nil { + t.Fatalf("partsFromMediaParts: %v", err) + } if len(parts) != 2 { t.Fatalf("expected 2 parts, got %d", len(parts)) } @@ -30,9 +36,12 @@ func TestOpenAI_CaptionPlusImageURL(t *testing.T) { } func TestOpenAI_BytesBecomeDataURI(t *testing.T) { - parts := partsFromMediaParts("", []history.MediaPart{ + parts, err := partsFromMediaParts("", []history.MediaPart{ history.NewImagePartBytes("image/png", pngBytes), }) + if err != nil { + t.Fatalf("partsFromMediaParts: %v", err) + } if len(parts) != 1 { t.Fatalf("expected 1 part, got %d", len(parts)) } @@ -42,13 +51,49 @@ func TestOpenAI_BytesBecomeDataURI(t *testing.T) { } } -func TestOpenAI_SkipsEmpty(t *testing.T) { - parts := partsFromMediaParts("", []history.MediaPart{ - {Type: history.PartText, Text: ""}, // empty text — drop - {Type: history.PartImage, MIME: "image/png"}, // no URL, no Data — drop - history.NewImagePartURL("image/png", "https://x/a"), // keep +// An empty text part carries nothing to lose, so it is dropped. Everything +// else this adapter cannot render fails the message: dropping it would send a +// prompt about media the model never received and return a confident answer. +func TestOpenAI_DropsEmptyTextButRejectsUnrenderable(t *testing.T) { + parts, err := partsFromMediaParts("", []history.MediaPart{ + {Type: history.PartText, Text: ""}, + history.NewImagePartURL("image/png", "https://x/a"), }) + if err != nil { + t.Fatalf("empty text part must be dropped, not rejected: %v", err) + } if len(parts) != 1 { - t.Fatalf("expected 1 part after filtering, got %d", len(parts)) + t.Fatalf("expected 1 part after dropping empty text, got %d", len(parts)) + } + + for _, tc := range []struct { + name string + part history.MediaPart + }{ + {"image without payload", history.MediaPart{Type: history.PartImage, MIME: "image/png"}}, + {"unknown part type", history.MediaPart{Type: history.PartType("audio"), MIME: "audio/wav", Data: pngBytes}}, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := partsFromMediaParts("caption", []history.MediaPart{tc.part}); !errors.Is(err, agent.ErrUnrenderablePart) { + t.Fatalf("error = %v, want ErrUnrenderablePart", err) + } + }) + } +} + +// Parts on a non-user role are rejected at the message level: OpenAI has no +// multimodal assistant or tool content to render them into. +func TestOpenAI_RejectsPartsOnNonUserRole(t *testing.T) { + p, err := New("test-key", "gpt-4o") + if err != nil { + t.Fatalf("New: %v", err) + } + memory := []history.Message{{ + Role: "assistant", + Parts: []history.MediaPart{history.NewImagePartURL("image/png", "https://x/a")}, + }} + _, err = p.GenerateStream(context.Background(), memory, nil, make(chan agent.StreamEvent, 1)) + if !errors.Is(err, agent.ErrUnrenderablePart) { + t.Fatalf("GenerateStream error = %v, want ErrUnrenderablePart", err) } } diff --git a/pkg/llm/openai/openai.go b/pkg/llm/openai/openai.go index 8b16fe3..d512b02 100644 --- a/pkg/llm/openai/openai.go +++ b/pkg/llm/openai/openai.go @@ -114,9 +114,22 @@ func (p *Provider) GenerateStream(ctx context.Context, memory []history.Message, // Multimodal: when Parts is set we must populate MultiContent // instead of Content (OpenAI rejects both being set on the same // message — see ErrContentFieldsMisused in the SDK). - if len(m.Parts) > 0 && m.Role == "user" { + // + // Only user messages may carry media. Parts on any other role are + // rejected rather than dropped: OpenAI has no multimodal assistant or + // tool content, so silently sending the text alone would answer from + // media the model never received. + if len(m.Parts) > 0 { + if m.Role != "user" { + return agent.LLMResult{}, fmt.Errorf("openai: %w: %s message carries %d media parts, which this API accepts only on user messages", + agent.ErrUnrenderablePart, m.Role, len(m.Parts)) + } + multi, err := partsFromMediaParts(m.Content, m.Parts) + if err != nil { + return agent.LLMResult{}, err + } msg.Content = "" - msg.MultiContent = partsFromMediaParts(m.Content, m.Parts) + msg.MultiContent = multi } // tool result: needs ToolCallID if m.Role == "tool" && m.ToolCallID != "" { @@ -373,7 +386,12 @@ func reasoningEffortFor(model string, budget int) string { // // For image parts, raw Data is folded into a data: URI — OpenAI accepts both // https URLs and data: URIs interchangeably for image_url. -func partsFromMediaParts(caption string, parts []history.MediaPart) []openai.ChatMessagePart { +// +// Any part this adapter cannot render fails the whole message with +// agent.ErrUnrenderablePart rather than being skipped; see that error's doc +// for why silence is the worse outcome. Empty text parts are the one +// exception — they carry nothing to lose. +func partsFromMediaParts(caption string, parts []history.MediaPart) ([]openai.ChatMessagePart, error) { out := make([]openai.ChatMessagePart, 0, len(parts)+1) if caption != "" { out = append(out, openai.ChatMessagePart{ @@ -381,7 +399,7 @@ func partsFromMediaParts(caption string, parts []history.MediaPart) []openai.Cha Text: caption, }) } - for _, p := range parts { + for i, p := range parts { switch p.Type { case history.PartText: if p.Text == "" { @@ -401,13 +419,18 @@ func partsFromMediaParts(caption string, parts []history.MediaPart) []openai.Cha url = "data:" + mime + ";base64," + base64.StdEncoding.EncodeToString(p.Data) } if url == "" { - continue + return nil, fmt.Errorf("openai: %w: part %d is an image with neither URL nor Data", agent.ErrUnrenderablePart, i) } out = append(out, openai.ChatMessagePart{ Type: openai.ChatMessagePartTypeImageURL, ImageURL: &openai.ChatMessageImageURL{URL: url}, }) + default: + return nil, fmt.Errorf("openai: %w: part %d has unsupported type %q", agent.ErrUnrenderablePart, i, p.Type) } } - return out + if len(out) == 0 { + return nil, fmt.Errorf("openai: %w: %d parts produced no renderable content", agent.ErrUnrenderablePart, len(parts)) + } + return out, nil } From d2642bd65fb0b585fa410137d2789e11f438e536 Mon Sep 17 00:00:00 2001 From: Hung Nguyen Date: Mon, 10 Aug 2026 19:51:25 +0700 Subject: [PATCH 3/3] docs(changelog): v0.41.0 --- CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 024f72e..d5e7286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ All notable changes to GopherAgent are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/); versions follow [Semantic Versioning](https://semver.org/) — pre-1.0, breaking API changes only require a minor bump. +## [v0.41.0] — 2026-08-10 + +### Added + +- **`pkg/audio` — a provider-neutral speech-to-text seam, so an audio-fed agent is not limited to the one vendor that accepts audio in a message.** Of the three providers here, only Gemini takes audio in a chat message: Anthropic has no audio content block at all, and the OpenAI Chat Completions client cannot express one. Transcribing *before* the message rather than inside it makes the capability portable — every provider can drive an audio-fed agent, because what reaches the model is ordinary text. It is also the cheaper shape for anything long. History is re-sent on every LLM call in a session, so audio carried as a message part is re-uploaded on every subsequent turn: a one-hour recording transcribed once costs one upload, while the same recording as message parts is billed again on each turn of the conversation about it. The package is a stdlib-only leaf holding the `Transcriber` interface plus `Clip`, `Transcript`, `Segment`, and `Options`. Three sentinels — `ErrNoAudio`, `ErrUnsupportedFormat`, `ErrTooLarge` — are separate rather than one error because a live-capture pipeline responds to each differently: an oversized clip must be re-cut into shorter chunks, an unsupported one re-encoded, and an empty one is a caller bug; routing on `errors.Is` beats matching message text. `Transcript.Segments` is nil when the backend emits no timing, which is a normal result rather than a failure, so callers needing timestamps must check rather than assume. `Ext` strips MIME parameters and matches case-insensitively, because browsers' `MediaRecorder` reports `audio/webm;codecs=opus` and some emit the `video/webm` spelling for an audio-only recording — rejecting either would fail a clip the backend decodes fine. (`pkg/audio`) +- **`openai.NewTranscriber` and `gemini.NewTranscriber`.** The OpenAI implementation drives the audio transcription endpoint and populates `Segments`, `Language`, and `Duration`. It selects the `verbose_json` response format only for whisper models: the `gpt-4o-transcribe` family rejects that format outright rather than degrading, so asking for it everywhere would fail every request instead of merely losing timings. The match is a substring rather than a prefix, since compatible endpoints name the same weights differently. Oversized clips are rejected against the endpoint's 25 MB limit before the upload rather than after transferring the payload. `WithBaseURL` works here as on every other client in the package, so a self-hosted transcription server is a supported target. The Gemini implementation has no dedicated transcription endpoint to call, so it constrains the generation API with a system instruction — without one the model opens with a preamble or summarizes instead of transcribing, and both corrupt a transcript appended verbatim. Its `Segments` is always nil, stated on the type rather than discovered at run time, and its language and vocabulary hints travel as instruction text because the API has no parameter for either. (`pkg/llm/openai/transcriber.go`, `pkg/llm/gemini/transcriber.go`) + +### Changed (breaking) + +- **A message carrying a media part the adapter cannot render now fails with `agent.ErrUnrenderablePart` instead of being silently dropped.** All three adapters converted `history.MediaPart` with a `switch` that fell through for anything unexpected, and one of them documented the omission as deliberate on the grounds that erroring would force callers to pre-validate. That trade was wrong in the direction that matters. Dropping the part does not degrade the call, it silently changes what the question was: the model receives the caption alone and answers it fluently, and nothing distinguishes that from success — not the response, not the logs, not a schema check, because a well-formed answer is exactly what success looks like. A judge that cannot see the image it is judging is not a degraded judge, it is a random one. Four shapes now fail: an unknown part type, an image with neither `URL` nor `Data`, a parts slice that yields no content at all, and media parts on a non-`user` role, which every adapter previously ignored wholesale — OpenAI behind an explicit role guard, Anthropic and Gemini by rendering media only under their `user` branch. Empty text parts are still skipped, as they carry nothing to lose. `blocksFromMediaParts` and `partsFromMediaParts` grew an `error` return; both are unexported and every call site is inside `GenerateStream`, which already returned one. `isRetryable` treats the sentinel as terminal — the same bytes fail identically on every attempt, so retrying only burns the budget. Callers that relied on a malformed part being ignored will now see the call fail; that is the point. (`pkg/agent/errors.go`, `pkg/agent/retry.go`, `pkg/llm/anthropic`, `pkg/llm/openai`, `pkg/llm/gemini`) + +### Fixed + +- **The Gemini transcriber reads `FinishReason` before testing for nil content.** A candidate stopped by a content filter arrives with a non-`STOP` reason *and* nil content, so checking content first reported a blocked recording as an empty transcript — a filtered meeting became indistinguishable from a silent one, with no error to act on. The same ordering guards truncation, where returning the accumulated prefix would present half a transcript as the whole. A response with no candidate at all is now an error rather than an empty transcript, since it signals a prompt-level block rather than audio without speech. (`pkg/llm/gemini/transcriber.go`) + ## [v0.40.0] — 2026-08-09 ### Added @@ -620,6 +635,7 @@ Multi-user, long-running, audit-friendly chat surface — the foundation for sid - README section on the permission flow — documents `RequiresConfirmation` × `ConfirmHITL` × `Permissions` interaction. - Enum struct tag support in `tools.SchemaFor[T]()` — emit values into JSON-Schema's `enum` array so providers reject invalid values upstream. +[v0.41.0]: https://github.com/hung12ct/gopheragent/releases/tag/v0.41.0 [v0.40.0]: https://github.com/hung12ct/gopheragent/releases/tag/v0.40.0 [v0.39.0]: https://github.com/hung12ct/gopheragent/releases/tag/v0.39.0 [v0.38.0]: https://github.com/hung12ct/gopheragent/releases/tag/v0.38.0