Phase 26: Twelve Labs Async Embedding

[tazarov]

Summary

Phase 26: Twelve Labs Async Embedding
Goal: Twelve Labs provider handles async task responses for long-running audio and video embeddings
Status: Verified on 2026-04-14 (26-VERIFICATION.md status: passed)

This phase adds an explicit async path for Twelve Labs long-running media embeddings. Audio and video callers can now opt in with WithAsyncPolling(maxWait) to create a task, poll until it reaches a terminal state, and return the final embedding or a sanitized failure. Text/image callers and all non-opt-in traffic remain on the existing synchronous path.

Changes

Plan 26-01: Async HTTP foundation

Added dedicated async request/response types for the tasks API, decoded the _id alias correctly, preserved raw task failure bodies for later sanitization, and introduced internal polling configuration fields/defaults plus task POST/GET helpers.

Key files:

• pkg/embeddings/twelvelabs/twelvelabs.go

Plan 26-02: Polling loop and modality routing

Implemented the async polling loop with capped backoff, distinct cancellation/deadline/maxWait behavior, failure-body sanitization, fused-audio rejection on the async path, and audio/video routing into the tasks flow while leaving text/image unchanged.

Key files:

• pkg/embeddings/twelvelabs/twelvelabs_async.go
• pkg/embeddings/twelvelabs/content.go

Plan 26-03: Public option and config round-trip

Added the public WithAsyncPolling(maxWait) option and made async config persist/rebuild through async_polling and async_max_wait_ms without changing default behavior for callers who do not opt in.

Key files:

• pkg/embeddings/twelvelabs/option.go
• pkg/embeddings/twelvelabs/twelvelabs.go

Plan 26-04: Async test coverage

Added 12 async-focused tests covering task creation, ready/failed/unexpected task states, context cancellation, SDK maxWait behavior, sync-path preservation for text/image, config round-trip, raw failure sanitization, blocked HTTP maxWait enforcement, and fused-mode rejection.

Key files:

• pkg/embeddings/twelvelabs/twelvelabs_test.go

Requirements Addressed

• TLA-01: Twelve Labs provider detects async task responses and enters a polling loop
• TLA-02: Async polling respects caller context for cancellation and timeout
• TLA-03: Async polling handles terminal states with appropriate result delivery or error messages
• TLA-04: Tests cover async task creation, polling, completion, failure, and context cancellation

Verification

• Automated verification passed (26-VERIFICATION.md, 4/4 must-haves verified on 2026-04-14)
• go test -tags=ef -count=1 ./pkg/embeddings/twelvelabs/...
• make lint
• Human verification not required for this library change

Key Decisions

• Async stays opt-in through WithAsyncPolling(maxWait); polling internals remain hidden.
• Audio/video route through the tasks API; text/image always stay on sync /embed-v2.
• Async audio request payloads use embedding_option as []string, and task responses decode Twelve Labs’ _id field.
• Failed-task errors sanitize the authentic raw response body instead of a re-marshaled subset.
• fused audio is rejected on the async path instead of being silently remapped.

[claude]

Code Review: Phase 26 — Twelve Labs Async Embedding

Bug: context.Canceled handling discards original error and risks nil return

In both pollTask (twelvelabs_async.go:88) and createTaskAndPoll (twelvelabs_async.go:173), the context.Canceled branch wraps ctx.Err() instead of the original transport error err:

if errors.Is(err, context.Canceled) {
    return nil, errors.Wrap(ctx.Err(), "Twelve Labs async polling canceled")
}

This is inconsistent with the context.DeadlineExceeded branch directly above, which correctly preserves the original error:

return nil, errors.Wrapf(err, "Twelve Labs async polling deadline exceeded")

Two problems:

1. Lost debug context: The original err contains the full transport error chain (e.g., "failed to send task retrieve request to .../tasks/abc: context canceled"). Wrapping only ctx.Err() discards this, making cancel-related failures harder to diagnose.

2. Potential nil error / nil-pointer panic: pkg/errors.Wrap(nil, msg) returns nil. If errors.Is(err, context.Canceled) is true but ctx.Err() is nil (possible with custom transports or derived sub-contexts), this returns (nil, nil). The caller then dereferences the nil *TaskResponse at resp.Status (pollTask) or nil *TaskCreateResponse (createTaskAndPoll), causing a panic — violating the library's no-panic contract from CLAUDE.md.

Fix: Use errors.Wrap(err, ...) instead of errors.Wrap(ctx.Err(), ...) in both locations, matching the DeadlineExceeded branch pattern.

---

Minor: Config round-trip truncation on 32-bit platforms

GetConfig() stores asyncMaxWait.Milliseconds() as int64, but NewTwelveLabsEmbeddingFunctionFromConfig reads it via embeddings.ConfigInt() which casts int64 → int. On 32-bit systems, maxWait values exceeding ~24.8 days would silently truncate. Not a practical concern for the 30-minute default, but worth noting if the API evolves.

---

Note: PR description outdated

The PR description lists a known issue: "Initial doTaskPost is not yet bounded by asyncMaxWait." However, the implementation in createTaskAndPoll (lines 149–156) already bounds it with a derived deadline context, and TestTwelveLabsAsyncMaxWaitHardBound verifies this. The description should be updated to reflect that this was resolved.

---

Positive observations

• Consistent use of url.PathEscape(taskID) prevents path injection from server-controlled task IDs.
• All response bodies read through chttp.ReadLimitedBody — no unbounded reads.
• Error bodies consistently sanitized via chttp.SanitizeErrorBody.
• Backoff timer correctly stopped on context cancellation (no timer leak).
• FailureDetail correctly copies response bytes to avoid buffer reuse issues.
• Good test coverage including edge cases (blocked HTTP, fused rejection, empty ID, early-ready).

[claude]

PR Review: Phase 26 — Twelve Labs Async Embedding

Overall this is well-structured with thorough test coverage and careful deadline/cancellation handling. A few issues found:

Bug: createTaskAndPoll does not handle status=failed from task creation

In twelvelabs_async.go:181, only taskStatusReady is handled as an early return from the create response:

if created.Status == taskStatusReady && len(created.Data) > 0 {
    return buildEmbeddingFromData(created.Data)
}

If the server returns a task with status: "failed" and a non-empty _id in the create response, the code falls through to pollTask, which fires an unnecessary GET request. Worse, the failure details from the create response body are lost — doTaskPost does not capture FailureDetail like doTaskGet does, so the error message from the poll GET may differ from the original failure reason.

Suggested fix: Add a taskStatusFailed check after the ready check:

if created.Status == taskStatusFailed {
    return nil, errors.Errorf("Twelve Labs task [%s] failed at creation", created.ID)
}

Security (defense-in-depth): No URL scheme allowlist in buildMediaSource

content.go:128 validates that the URL has a scheme and host, which correctly rejects file://, data:, and javascript: URLs (empty host). However, schemes like ftp://, gopher://, or dict:// with a valid host would pass validation and be sent to the Twelve Labs API. If the API fetches these URLs server-side, this could enable SSRF via the client library.

Suggested fix: Add a scheme allowlist:

if !strings.EqualFold(parsed.Scheme, "http") && !strings.EqualFold(parsed.Scheme, "https") {
    return MediaSource{}, errors.Errorf("URL source scheme %q not supported; use http or https", parsed.Scheme)
}

Minor: Early-ready path silently falls through to polling when data is missing

twelvelabs_async.go:181 requires len(created.Data) > 0 alongside taskStatusReady. If the server returns status: "ready" with an empty data array, the code falls through to polling instead of returning an immediate error. This masks a malformed server response behind an extra network request that will likely produce the same result.

---

Test coverage is solid — 25+ async tests with good edge case coverage (blocked HTTP, parent deadline, fused rejection, config round-trip). The per-HTTP-call deadline wrapping in pollTask is particularly well done, with correct discrimination between SDK maxWait, parent deadline, and client timeout errors.

🤖 Generated with Claude Code

[claude]

Code Review: Phase 26 — Twelve Labs Async Embedding

Overall this is a well-structured implementation. The polling loop, deadline propagation, and error classification are thorough. A few findings below.

Bugs / Issues

1. No minimum validation on asyncMaxWait (option.go:112)

WithAsyncPolling(1 * time.Nanosecond) is accepted but will immediately time out before any HTTP request completes. Consider enforcing a minimum (e.g., 1 second) to prevent confusing instant-timeout behavior.

2. SDK maxWait timeout errors are not programmatically distinguishable (twelvelabs_async.go:79)

errors.Errorf with %v intentionally breaks the error chain so errors.Is(err, context.DeadlineExceeded) returns false (D-20). But this means callers have no way to programmatically detect an SDK maxWait timeout other than string matching. Consider defining a sentinel error (e.g., var ErrAsyncMaxWaitExceeded) that callers can check with errors.Is, while still keeping it distinct from context.DeadlineExceeded.

3. Stale PR review note

The PR body states: "Initial doTaskPost is not yet bounded by asyncMaxWait". But the code does bound it -- createTaskAndPoll wraps the create call with context.WithDeadline derived from sdkMaxWaitDeadline (lines 149-158 of twelvelabs_async.go). This note should be removed to avoid confusing future reviewers.

Minor / Informational

4. Empty failure detail produces slightly odd error message

If a server returns {"status":"failed"} with no error/message/reason fields, sanitizeTaskFailureDetail falls through to dumping the raw JSON body. The error becomes Twelve Labs task [abc] terminal status=failed: {"_id":"abc","status":"failed"}. Consider returning a generic fallback like "(no failure detail provided)" when extractTaskFailureDetail finds no structured reason and body is non-nil.

5. Config round-trip test does not cover JSON serialization

TestTwelveLabsAsyncConfigRoundTrip passes the config map directly in-memory. If the config is persisted via JSON (e.g., a registry), int64 values decode as float64. ConfigInt handles this, but the test does not exercise that path. Adding a JSON marshal/unmarshal step to the round-trip test would lock in this guarantee.

What looks good

• Deadline propagation through createTaskAndPoll -> pollTask correctly shares a single maxWait budget (D-09 hard bound)
• Per-HTTP-call deadline (callCtx) prevents a hung GET from outliving maxWait
• Error classification (D-20) correctly distinguishes parent context deadline, SDK maxWait, HTTP client timeout, and cancellation -- with good test coverage
• url.PathEscape on task IDs prevents path traversal
• FailureDetail uses json:"-" with manual population to preserve the authentic server response
• SanitizeErrorBody on all user-visible error paths prevents log flooding
• Fused-audio rejection on the async path is deterministic and tested

No critical bugs or security issues found.

[claude]

PR Review — Phase 26: Twelve Labs Async Embedding

Overall this is a well-structured PR with solid timeout/cancellation handling and good test coverage. Two issues found:

Bug: Task-creation failure discards server diagnostic detail

twelvelabs_async.go:196 — When doTaskPost returns a response with status=failed, the error message includes only the task ID:

case taskStatusFailed:
    return nil, errors.Errorf("Twelve Labs async task [%s] terminal status=failed at creation", created.ID)

Compare with the polling path (twelvelabs_async.go:107) which preserves and sanitizes the server's failure reason:

return nil, errors.Errorf("Twelve Labs task [%s] terminal status=failed: %s", taskID, sanitizeTaskFailureDetail(resp.FailureDetail))

The TaskCreateResponse struct has no FailureDetail field and doTaskPost does not capture the raw response body for failed-status responses (unlike doTaskGet at twelvelabs.go:354-356). If a task creation ever returns status=failed (acknowledged as rare in the comment at line 188-193), the caller gets no server-side context about why it failed, making debugging harder.

Suggested fix: Either add raw-body capture to doTaskPost (mirroring doTaskGet), or pass the structured TaskCreateResponse fields through the error so the caller gets whatever the server returned.

---

Latent bug: WithAudioEmbeddingOption("fused") + WithAsyncPolling(...) accepted at construction, fails at embed time

option.go:89-98 accepts "fused" as a valid audio option. twelvelabs_async.go:31-34 rejects it on the async path. This means a client constructed with both options will successfully create without error, then fail at the first async audio embed call:

// Succeeds at construction
ef, _ := NewTwelveLabsEmbeddingFunction(
    WithAPIKey("..."),
    WithAudioEmbeddingOption("fused"),
    WithAsyncPolling(10 * time.Minute),
)

// Fails here at runtime, potentially minutes into a pipeline
emb, err := ef.EmbedContent(ctx, audioContent)

"fused" is legitimately valid on the sync path, so the option itself isn't wrong — but the combination with async is a misconfiguration that could be caught at NewTwelveLabsClient validation time (in validate()), surfacing it before any work is done.

---

Everything else looks solid — the per-call deadline derivation with distinct error classification (SDK maxWait vs. parent context), url.PathEscape on the task ID, bounded response reads via ReadLimitedBody, and the SanitizeErrorBody usage on all error paths are all good patterns.

[claude]

Code Review: Phase 26 — Twelve Labs Async Embedding

Overall this is a well-structured addition with good test coverage (24+ async tests), proper context propagation, and careful error differentiation. A few issues worth addressing:

Bug: Inconsistent HTTP status code acceptance between sync and async paths

doPost (twelvelabs.go:264) only accepts 200 OK:

if resp.StatusCode != http.StatusOK {

While doTaskPost (line 307) and doTaskGet (line 353) accept any 2xx:

if resp.StatusCode < 200 || resp.StatusCode >= 300 {

If the Twelve Labs API ever returns 201 Created for task creation (common REST convention), doPost would reject it but the async endpoints would accept it. This inconsistency should be unified — either all should accept 2xx or all should accept only specific status codes.

Bug: Default HTTP client has no timeout

applyDefaults (twelvelabs.go:60) creates a bare &http.Client{} with no timeout. While the async path mitigates this with per-call context deadlines derived from maxWait, the sync doPost path has no such protection — a hung Twelve Labs server would block the caller indefinitely. Consider adding a default timeout (e.g., 30s or 60s) to the fallback client:

c.Client = &http.Client{Timeout: 30 * time.Second}

Potential issue: nextBackoff overflow for extreme multiplier values

nextBackoff (twelvelabs_async.go:140) converts time.Duration to float64 and back:

next := time.Duration(float64(cur) * mul)

While the multiplier is validated to be >= 1.0, there's no upper bound validation. An extremely large multiplier (e.g., math.MaxFloat64) could overflow time.Duration (int64) producing a negative value, which would bypass the cap check and create a negative sleep. The cap comparison saves this in practice since the cap is validated, but adding if next <= 0 { next = backoffCap } would be more defensive.

Minor: sanitizeTaskFailureDetail — JSON-object guard logic

In twelvelabs.go:578:

var probe map[string]any
if len(body) > 0 && json.Unmarshal(body, &probe) != nil {
    // non-JSON bodies — surface them
}

This only surfaces non-JSON bodies. If the body is valid JSON but has no known diagnostic fields (e.g., {"_id":"x","status":"failed","some_unknown_field":"useful_info"}), the raw body is discarded and the generic "(no failure detail provided)" is returned. This is documented as intentional (avoiding leaking housekeeping fields), but genuinely useful server-side fields not in the known-key list (message, failure_reason, reason, detail, error) will be silently lost.

Positive observations

• URL validation in buildMediaSource correctly blocks file://, javascript:, and other non-HTTP schemes — good SSRF defense-in-depth
• url.PathEscape(taskID) in doTaskGet prevents path traversal
• chttp.SanitizeErrorBody is used consistently to prevent credential/token leakage in error messages
• FailureDetail using json:"-" + manual byte copy avoids both re-marshaling data loss and buffer aliasing bugs
• The dual-deadline design (parent ctx deadline vs SDK maxWait) with distinct error messages is well-implemented
• ErrAsyncMaxWaitExceeded is correctly kept separate from context.DeadlineExceeded for caller introspection
• Test coverage is thorough — including edge cases like empty task IDs, ready-on-create, blocked HTTP calls, and config round-trips through JSON