fix: address review — fail loud on non-video bodies, dedupe form plumbing, doc parity
- videoMIME no longer hard-falls-back to video/mp4: a 2xx body that is neither declared nor sniffable as video (JSON job envelope, HTML error page) is now an APIError instead of a 'successful' corrupt clip. - Resolution rides the wire as width/height AND the OpenAI-style size string, so either upstream convention honors an explicit request. - writeFormFields + mimeFromContentType shared helpers replace the copied multipart loop (audio.go/video.go) and Content-Type branch. - ADR-0019 indexed in docs/adr/README.md; README gains the videogen section + support-matrix mention (docs-parity rule). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AXQxVhXBw8PwFAtsVrXSmj
This commit is contained in:
@@ -266,6 +266,24 @@ tr, err := tm.Transcribe(ctx, audio.TranscriptionRequest{
|
|||||||
voices, err := ls.ListVoices(ctx, "kokoro") // []string of voice ids
|
voices, err := ls.ListVoices(ctx, "kokoro") // []string of voice ids
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## Video: text-to-video + image-to-video
|
||||||
|
|
||||||
|
Video generation lives in the `videogen` package (ADR-0019), mirroring
|
||||||
|
imagegen/audio: one small `Model` contract, zero values mean backend
|
||||||
|
defaults, bytes in/out. Text-to-video and image-to-video are one surface —
|
||||||
|
a nil `InitImage` is a pure text prompt; setting it conditions generation
|
||||||
|
on that frame (hybrid checkpoints like Wan 2.2 TI2V serve both). First
|
||||||
|
backend: llama-swap (blocking `/v1/videos/sync`, vLLM-Omni style — the
|
||||||
|
response body is the encoded clip, so `Result` carries a single `Video`).
|
||||||
|
Generation runs for minutes; bound the call with a context deadline.
|
||||||
|
|
||||||
|
```go
|
||||||
|
vm, _ := ls.VideoModel("videogen-wan22-5b")
|
||||||
|
res, err := vm.Generate(ctx, videogen.Request{Prompt: "a cat surfing"},
|
||||||
|
videogen.WithSize("1280x704"), videogen.WithNumFrames(81))
|
||||||
|
// res.Video.Data ([]byte) + res.Video.MIME ("video/mp4")
|
||||||
|
```
|
||||||
|
|
||||||
## Tool calls
|
## Tool calls
|
||||||
|
|
||||||
```go
|
```go
|
||||||
@@ -398,7 +416,8 @@ response as a single delta plus final event.
|
|||||||
capabilities are present at the client level; whether a given call succeeds
|
capabilities are present at the client level; whether a given call succeeds
|
||||||
depends on the llama.cpp model llama-swap loads. llama-swap also provides
|
depends on the llama.cpp model llama-swap loads. llama-swap also provides
|
||||||
**image generation + editing** (`imagegen`), **speech synthesis +
|
**image generation + editing** (`imagegen`), **speech synthesis +
|
||||||
transcription** (`audio`) — separate axes, not shown above — plus a `Health`
|
transcription** (`audio`), **video generation** (`videogen`) — separate
|
||||||
|
axes, not shown above — plus a `Health`
|
||||||
probe and management methods on `*llamaswap.Provider`.
|
probe and management methods on `*llamaswap.Provider`.
|
||||||
|
|
||||||
Notes: Ollama has no native tool_choice — `"none"` drops the tools;
|
Notes: Ollama has no native tool_choice — `"none"` drops the tools;
|
||||||
|
|||||||
@@ -22,3 +22,4 @@ One decision per file, append-only; supersede rather than rewrite.
|
|||||||
| [0016](0016-imagegen-interface.md) | imagegen — a canonical text-to-image interface | Accepted |
|
| [0016](0016-imagegen-interface.md) | imagegen — a canonical text-to-image interface | Accepted |
|
||||||
| [0017](0017-audio-interfaces.md) | audio — canonical speech synthesis + transcription interfaces | Accepted |
|
| [0017](0017-audio-interfaces.md) | audio — canonical speech synthesis + transcription interfaces | Accepted |
|
||||||
| [0018](0018-imagegen-editor.md) | imagegen.Editor — image-to-image as a separate optional interface | Accepted |
|
| [0018](0018-imagegen-editor.md) | imagegen.Editor — image-to-image as a separate optional interface | Accepted |
|
||||||
|
| [0019](0019-videogen-interface.md) | videogen — canonical video-generation surface | Accepted |
|
||||||
|
|||||||
@@ -77,7 +77,7 @@ func (m *speechModel) Speak(ctx context.Context, req audio.SpeechRequest, opts .
|
|||||||
// is a concrete audio type, else a mapping from the requested format, else
|
// is a concrete audio type, else a mapping from the requested format, else
|
||||||
// audio/mpeg (the OpenAI endpoint's default container is mp3).
|
// audio/mpeg (the OpenAI endpoint's default container is mp3).
|
||||||
func speechMIME(contentType, format string) string {
|
func speechMIME(contentType, format string) string {
|
||||||
if mt, _, err := mime.ParseMediaType(contentType); err == nil && strings.HasPrefix(mt, "audio/") {
|
if mt := mimeFromContentType(contentType, "audio/"); mt != "" {
|
||||||
return mt
|
return mt
|
||||||
}
|
}
|
||||||
switch strings.ToLower(strings.TrimSpace(format)) {
|
switch strings.ToLower(strings.TrimSpace(format)) {
|
||||||
@@ -128,24 +128,13 @@ func (m *transcriptionModel) Transcribe(ctx context.Context, req audio.Transcrip
|
|||||||
if _, err := fw.Write(req.Audio); err != nil {
|
if _, err := fw.Write(req.Audio); err != nil {
|
||||||
return nil, fmt.Errorf("llama-swap: build transcription form: %w", err)
|
return nil, fmt.Errorf("llama-swap: build transcription form: %w", err)
|
||||||
}
|
}
|
||||||
// Required fields always go on the wire (an empty model id should fail
|
if err := writeFormFields(w, "build transcription form", []formField{
|
||||||
// loudly upstream, not silently vanish); optional ones only when set.
|
|
||||||
fields := []struct {
|
|
||||||
key, value string
|
|
||||||
required bool
|
|
||||||
}{
|
|
||||||
{"model", m.id, true},
|
{"model", m.id, true},
|
||||||
{"response_format", "json", true},
|
{"response_format", "json", true},
|
||||||
{"language", req.Language, false},
|
{"language", req.Language, false},
|
||||||
{"prompt", req.Prompt, false},
|
{"prompt", req.Prompt, false},
|
||||||
}
|
}); err != nil {
|
||||||
for _, f := range fields {
|
return nil, err
|
||||||
if !f.required && f.value == "" {
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
if err := w.WriteField(f.key, f.value); err != nil {
|
|
||||||
return nil, fmt.Errorf("llama-swap: build transcription form: %w", err)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
if err := w.Close(); err != nil {
|
if err := w.Close(); err != nil {
|
||||||
return nil, fmt.Errorf("llama-swap: build transcription form: %w", err)
|
return nil, fmt.Errorf("llama-swap: build transcription form: %w", err)
|
||||||
|
|||||||
@@ -30,6 +30,8 @@ import (
|
|||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
|
"mime"
|
||||||
|
"mime/multipart"
|
||||||
"net/http"
|
"net/http"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
@@ -280,3 +282,34 @@ func (p *Provider) apiError(resp *http.Response, model string) error {
|
|||||||
e.Message = strings.TrimSpace(string(body))
|
e.Message = strings.TrimSpace(string(body))
|
||||||
return e
|
return e
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// formField is one multipart form entry for the media endpoints. Required
|
||||||
|
// fields always go on the wire (an empty model id should fail loudly
|
||||||
|
// upstream, not silently vanish); optional ones only when set.
|
||||||
|
type formField struct {
|
||||||
|
key, value string
|
||||||
|
required bool
|
||||||
|
}
|
||||||
|
|
||||||
|
// writeFormFields appends fields to a multipart writer, skipping unset
|
||||||
|
// optional entries. wrap labels errors ("build transcription form", ...).
|
||||||
|
func writeFormFields(w *multipart.Writer, wrap string, fields []formField) error {
|
||||||
|
for _, f := range fields {
|
||||||
|
if !f.required && f.value == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if err := w.WriteField(f.key, f.value); err != nil {
|
||||||
|
return fmt.Errorf("llama-swap: %s: %w", wrap, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// mimeFromContentType returns the parsed media type when it matches prefix
|
||||||
|
// ("audio/", "video/"), else "".
|
||||||
|
func mimeFromContentType(contentType, prefix string) string {
|
||||||
|
if mt, _, err := mime.ParseMediaType(contentType); err == nil && strings.HasPrefix(mt, prefix) {
|
||||||
|
return mt
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|||||||
+26
-19
@@ -64,30 +64,24 @@ func (m *videoModel) Generate(ctx context.Context, req videogen.Request, opts ..
|
|||||||
|
|
||||||
var buf bytes.Buffer
|
var buf bytes.Buffer
|
||||||
w := multipart.NewWriter(&buf)
|
w := multipart.NewWriter(&buf)
|
||||||
// Required fields always go on the wire (an empty model id should fail
|
if err := writeFormFields(w, "build video form", []formField{
|
||||||
// loudly upstream, not silently vanish); optional ones only when set.
|
|
||||||
fields := []struct {
|
|
||||||
key, value string
|
|
||||||
required bool
|
|
||||||
}{
|
|
||||||
{"model", m.id, true},
|
{"model", m.id, true},
|
||||||
{"prompt", req.Prompt, true},
|
{"prompt", req.Prompt, true},
|
||||||
{"negative_prompt", req.NegativePrompt, false},
|
{"negative_prompt", req.NegativePrompt, false},
|
||||||
|
// Resolution rides the wire twice: width/height (vLLM-Omni's
|
||||||
|
// names) AND the equivalent OpenAI-style size string, since
|
||||||
|
// upstreams silently ignore fields they don't understand and the
|
||||||
|
// values can never disagree.
|
||||||
{"width", formatInt(width), false},
|
{"width", formatInt(width), false},
|
||||||
{"height", formatInt(height), false},
|
{"height", formatInt(height), false},
|
||||||
|
{"size", strings.TrimSpace(req.Size), false},
|
||||||
{"num_frames", formatNonZero(req.NumFrames), false},
|
{"num_frames", formatNonZero(req.NumFrames), false},
|
||||||
{"fps", formatNonZero(req.FPS), false},
|
{"fps", formatNonZero(req.FPS), false},
|
||||||
{"num_inference_steps", formatInt(req.Steps), false},
|
{"num_inference_steps", formatInt(req.Steps), false},
|
||||||
{"guidance_scale", formatFloat(req.GuidanceScale), false},
|
{"guidance_scale", formatFloat(req.GuidanceScale), false},
|
||||||
{"seed", formatInt64(req.Seed), false},
|
{"seed", formatInt64(req.Seed), false},
|
||||||
}
|
}); err != nil {
|
||||||
for _, f := range fields {
|
return nil, err
|
||||||
if !f.required && f.value == "" {
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
if err := w.WriteField(f.key, f.value); err != nil {
|
|
||||||
return nil, fmt.Errorf("llama-swap: build video form: %w", err)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
if req.InitImage != nil {
|
if req.InitImage != nil {
|
||||||
fw, err := w.CreateFormFile("input_reference", initImageFilename(req.InitImage.MIME))
|
fw, err := w.CreateFormFile("input_reference", initImageFilename(req.InitImage.MIME))
|
||||||
@@ -109,20 +103,33 @@ func (m *videoModel) Generate(ctx context.Context, req videogen.Request, opts ..
|
|||||||
if len(videoBytes) == 0 {
|
if len(videoBytes) == 0 {
|
||||||
return nil, &llm.APIError{Provider: m.p.name, Model: m.id, Message: "video response contained no video"}
|
return nil, &llm.APIError{Provider: m.p.name, Model: m.id, Message: "video response contained no video"}
|
||||||
}
|
}
|
||||||
return &videogen.Result{Video: videogen.Video{Data: videoBytes, MIME: videoMIME(contentType, videoBytes)}}, nil
|
mimeType := videoMIME(contentType, videoBytes)
|
||||||
|
if mimeType == "" {
|
||||||
|
// A 2xx body that is neither declared nor sniffable as video is a
|
||||||
|
// misconfigured upstream (a JSON job envelope, an HTML error page
|
||||||
|
// behind a proxy) — fail loud rather than hand back garbage as a
|
||||||
|
// playable clip.
|
||||||
|
return nil, &llm.APIError{
|
||||||
|
Provider: m.p.name,
|
||||||
|
Model: m.id,
|
||||||
|
Message: fmt.Sprintf("video response is not a video (content-type %q)", contentType),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return &videogen.Result{Video: videogen.Video{Data: videoBytes, MIME: mimeType}}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// videoMIME resolves the result MIME type: the response Content-Type when it
|
// videoMIME resolves the result MIME type: the response Content-Type when it
|
||||||
// is a concrete video type, else content sniffing, else video/mp4 (the
|
// is a concrete video type, else content sniffing (mp4/webm magic bytes),
|
||||||
// endpoint's default container).
|
// else "" — the caller treats undetectable as an upstream error, unlike the
|
||||||
|
// audio path where the request's format param implies the container.
|
||||||
func videoMIME(contentType string, data []byte) string {
|
func videoMIME(contentType string, data []byte) string {
|
||||||
if mt, _, err := mime.ParseMediaType(contentType); err == nil && strings.HasPrefix(mt, "video/") {
|
if mt := mimeFromContentType(contentType, "video/"); mt != "" {
|
||||||
return mt
|
return mt
|
||||||
}
|
}
|
||||||
if mt := http.DetectContentType(data); strings.HasPrefix(mt, "video/") {
|
if mt := http.DetectContentType(data); strings.HasPrefix(mt, "video/") {
|
||||||
return mt
|
return mt
|
||||||
}
|
}
|
||||||
return "video/mp4"
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
// initImageFilename picks the multipart filename hint for the conditioning
|
// initImageFilename picks the multipart filename hint for the conditioning
|
||||||
|
|||||||
@@ -179,6 +179,9 @@ func TestVideoModelRequiresBaseURL(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestVideoMIME(t *testing.T) {
|
func TestVideoMIME(t *testing.T) {
|
||||||
|
// A minimal ISO-BMFF prefix http.DetectContentType sniffs as video/mp4
|
||||||
|
// (the "mp4" brand prefix must appear inside the declared ftyp box).
|
||||||
|
mp4Magic := append([]byte{0, 0, 0, 20}, []byte("ftypmp42\x00\x00\x00\x00mp42")...)
|
||||||
cases := []struct {
|
cases := []struct {
|
||||||
contentType string
|
contentType string
|
||||||
data []byte
|
data []byte
|
||||||
@@ -186,12 +189,36 @@ func TestVideoMIME(t *testing.T) {
|
|||||||
}{
|
}{
|
||||||
{"video/webm", []byte("x"), "video/webm"},
|
{"video/webm", []byte("x"), "video/webm"},
|
||||||
{"video/mp4; charset=binary", []byte("x"), "video/mp4"},
|
{"video/mp4; charset=binary", []byte("x"), "video/mp4"},
|
||||||
{"application/octet-stream", []byte("x"), "video/mp4"},
|
{"application/octet-stream", mp4Magic, "video/mp4"},
|
||||||
{"", []byte("x"), "video/mp4"},
|
// Neither declared nor sniffable as video → "" (Generate errors).
|
||||||
|
{"application/octet-stream", []byte(`{"id":"job-1"}`), ""},
|
||||||
|
{"", []byte("x"), ""},
|
||||||
}
|
}
|
||||||
for _, tc := range cases {
|
for _, tc := range cases {
|
||||||
if got := videoMIME(tc.contentType, tc.data); got != tc.want {
|
if got := videoMIME(tc.contentType, tc.data); got != tc.want {
|
||||||
t.Errorf("videoMIME(%q) = %q, want %q", tc.contentType, got, tc.want)
|
t.Errorf("videoMIME(%q, %q) = %q, want %q", tc.contentType, tc.data, got, tc.want)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestVideoGenerateNonVideoBodyErrors(t *testing.T) {
|
||||||
|
// A stock async /v1/videos handler mounted at the sync path (or an HTML
|
||||||
|
// error page behind a proxy) answers 200 with a non-video body — that
|
||||||
|
// must be an error, never a "successful" garbage clip.
|
||||||
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.Header().Set("Content-Type", "application/json")
|
||||||
|
_, _ = w.Write([]byte(`{"id":"job-1","status":"queued"}`))
|
||||||
|
}))
|
||||||
|
defer srv.Close()
|
||||||
|
|
||||||
|
p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
|
||||||
|
vm, _ := p.VideoModel("videogen-wan")
|
||||||
|
_, err := vm.Generate(context.Background(), videogen.Request{Prompt: "x"})
|
||||||
|
var apiErr *llm.APIError
|
||||||
|
if !errors.As(err, &apiErr) {
|
||||||
|
t.Fatalf("err = %v, want *llm.APIError for non-video 2xx body", err)
|
||||||
|
}
|
||||||
|
if !strings.Contains(apiErr.Message, "not a video") {
|
||||||
|
t.Errorf("message = %q, want mention of non-video body", apiErr.Message)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user