fix: video pricing fixes

[TejasGhatte]

Summary

Adds support for the seconds parameter in video generation responses across all providers (Gemini, Replicate, Runway, and Vertex). This enhancement allows clients to receive the requested video duration in the response, improving API consistency and user experience.

Changes

• Modified video generation response conversion functions to accept and include a seconds parameter
• Added default video duration constant (8 seconds) for Gemini provider
• Updated all video generation endpoints to extract seconds from request parameters and pass to response converters
• Fixed indentation issues in Replicate provider's streaming response handling
• Added provider normalization for "runwayml" to map to Runway provider in model catalog utilities

Type of change

• Feature
• Bug fix

Affected areas

• Core (Go)
• Providers/Integrations

How to test

Test video generation endpoints across all providers to verify the seconds parameter is properly included in responses:

# Test video generation with seconds parameter
curl -X POST /v1/video/generations \
  -H "Content-Type: application/json" \
  -d '{
    "model": "gemini-2.0-flash-exp",
    "prompt": "A cat playing with a ball",
    "params": {
      "seconds": "10"
    }
  }'

# Verify response includes seconds field
# Expected: response.seconds should equal "10"

# Test without seconds parameter (should use default for Gemini/Vertex)
curl -X POST /v1/video/generations \
  -H "Content-Type: application/json" \
  -d '{
    "model": "gemini-2.0-flash-exp", 
    "prompt": "A dog running in a park"
  }'

# Expected: response.seconds should equal "8" (default)

# Run tests
go test ./core/providers/gemini/...
go test ./core/providers/replicate/...
go test ./core/providers/runway/...
go test ./core/providers/vertex/...

Breaking changes

• No

This is a backward-compatible addition that enhances existing video generation responses without breaking current functionality.

Security considerations

No security implications - this change only adds a duration parameter to video generation responses without affecting authentication, authorization, or data handling.

Checklist

• I read docs/contributing/README.md and followed the guidelines
• I added/updated tests where appropriate
• I updated documentation where needed
• I verified builds succeed (Go and UI)
• I verified the CI pipeline passes locally if applicable

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[TejasGhatte]

• fix: video pricing fixes #2044 👈 (View in Graphite)
• fix: remove resolution parameter from image generation and add MCP client config fields #2041
• feat: refactor model catalog pricing engine with unified cost calculation #1800 : 2 other dependent PRs (#1825 , #2037 )
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 004c4cc3-fd0c-41b2-8077-6559218e0c9b

📥 Commits

Reviewing files that changed from the base of the PR and between 3a6fa90 and 9037928.

📒 Files selected for processing (6)

• core/bifrost.go
• core/providers/gemini/gemini.go
• core/providers/replicate/replicate.go
• core/providers/vertex/vertex.go
• core/schemas/videos.go
• framework/modelcatalog/utils.go

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Added RunwayML provider recognition for video workflows.

• Improvements

  • Streaming responses reworked into a structured lifecycle (Created → InProgress → Outputs → Completed) with richer timing metadata, clearer stage events, and unified error handling.
  • Video responses now backfill and apply a default duration for Gemini/Vertex when not specified, ensuring generated videos include duration where applicable.

Walkthrough

Reworks Replicate streaming to a staged lifecycle with structured stage events and enriched error/raw-response handling; adds DefaultVideoDuration and BackfillParams to populate video Seconds for Gemini/Vertex and Remix; maps "runwayml" to Runway schema; removes stray comments/empty lines in Gemini/Vertex files.

Changes

Cohort / File(s)	Summary
Replicate streaming lifecycle core/providers/replicate/replicate.go	Replaces per-event delta emissions with stage-based events (Created, InProgress, OutputItemAdded, ContentPartAdded, OutputTextDelta, Completed-like); centralizes RawResponse accumulation and unified error propagation; updates streaming payload shapes and timing/latency metadata.
Video defaults & backfill core/schemas/videos.go, core/bifrost.go	Adds DefaultVideoDuration, getSecondsFromVideoRequest, and BifrostVideoGenerationResponse.BackfillParams; invokes BackfillParams after non-streaming VideoGeneration responses to populate Seconds for Gemini/Vertex and Remix flows.
Provider schema mapping framework/modelcatalog/utils.go	Adds conditional mapping so provider identifiers containing runwayml return schemas.Runway.
Provider comment/whitespace cleanup core/providers/gemini/gemini.go, core/providers/vertex/vertex.go	Removed stray comment lines and an empty line; no behavioral or signature changes.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Bifrost
  participant ReplicateProvider
  participant Storage

  Client->>Bifrost: Start VideoGeneration (stream=true)
  Bifrost->>ReplicateProvider: Forward streaming request
  ReplicateProvider->>Bifrost: Emit Created (initial Response + latency)
  ReplicateProvider->>Storage: Persist initial output item (itemID)
  ReplicateProvider->>Bifrost: Emit InProgress (stage update)
  ReplicateProvider->>Bifrost: Emit OutputItemAdded (new item metadata)
  ReplicateProvider->>Bifrost: Emit ContentPartAdded (partID, raw data if enabled)
  ReplicateProvider->>Bifrost: Emit Completed (timing, final RawResponse)
  Bifrost->>Client: Relay staged events and final completion

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• fix: video pricing fixes #2044 — Strong overlap: modifies video backfill behavior and schemas (DefaultVideoDuration / BackfillParams) and related provider mapping.

Suggested reviewers

• Pratham-Mishra04

Poem

🐰 I hopped through streams and stitched each part,
I gave seconds to videos and tidy charts,
Mapped runway names, trimmed stray lines away,
Emitted stages bright as dawn's first ray,
A rabbit cheers this tidy code bouquet! 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'fix: video pricing fixes' is vague and does not clearly convey the specific changes made; it lacks actionable detail about what was fixed.	Revise the title to be more specific, e.g., 'feat: add seconds parameter to video generation responses' to better reflect the primary feature addition.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The pull request description is comprehensive, follows the template structure, includes all required sections (Summary, Changes, Type of change, Affected areas, How to test, Breaking changes, Security considerations, and Checklist), and provides clear testing instructions.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 03-12-fix_video_pricing_fixes

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

core/providers/replicate/videos.go (1)

83-104: ⚠️ Potential issue | 🟠 Major

Extract duration from prediction input as fallback in retrieve/status responses.

The converter receives nil for the seconds parameter on retrieve/status calls (line 2799), which causes response.Seconds to remain unset. Since the ReplicatePredictionResponse.Input field carries the original request parameters (including the duration sent during creation), the converter should attempt to extract and set Seconds from prediction.Input["duration"] when the parameter is nil to prevent data loss in async flows.

func ToBifrostVideoGenerationResponse(prediction *ReplicatePredictionResponse, seconds *string) (*schemas.BifrostVideoGenerationResponse, *schemas.BifrostError) {
	// ... nil check and basic fields ...
	
	response := &schemas.BifrostVideoGenerationResponse{
		ID:        prediction.ID,
		CreatedAt: ParseReplicateTimestamp(prediction.CreatedAt),
		Model:     prediction.Model,
		Object:    "video",
	}
	
	// Use provided seconds, or extract from prediction input as fallback
	if seconds != nil {
		response.Seconds = seconds
	} else if prediction.Input != nil {
		if duration, ok := prediction.Input["duration"]; ok {
			// Extract and format duration for response
		}
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/providers/replicate/videos.go` around lines 83 - 104, In
ToBifrostVideoGenerationResponse, when the seconds parameter is nil, read the
original duration from the ReplicatePredictionResponse.Input map
(prediction.Input["duration"]) and set response.Seconds as a string pointer
fallback; ensure you check prediction.Input != nil, type-assert the duration
safely (handle numeric and string representations), format it into the expected
string form, create a *string and assign it to response.Seconds, and avoid
panics on missing/invalid types.

core/providers/replicate/replicate.go (1)

1459-1729: ⚠️ Potential issue | 🟠 Major

Don’t map every done event to a successful completion.

This branch never parses ReplicateDoneEvent, so a terminal reason:"error" / reason:"canceled" will still emit response.completed. It also never accumulates the streamed text, so the synthesized response.output_text.done / response.content_part.done / response.output_item.done payloads are empty. Please mirror the existing done handling used by the other Replicate stream handlers here and build the terminal lifecycle events from an accumulated text buffer before sending response.completed.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/providers/replicate/replicate.go` around lines 1459 - 1729, The done
branch currently treats every "done" event as a successful completion and emits
empty final payloads because it never parses ReplicateDoneEvent nor uses an
accumulated text buffer; update the "done" case to parse currentEvent into a
ReplicateDoneEvent (check the terminal Reason field for "error" or "canceled"),
and if non-successful emit the appropriate terminal lifecycle/error response
instead of a normal ResponsesStreamResponseTypeCompleted; also maintain and use
an accumulated text buffer (e.g., accumulate currentEvent.Data into a buffer
when handling "output" deltas—separate from rawResponseChunks) so that when
emitting textDoneResp, partDoneResp and itemDoneResp you populate their
Text/ContentBlocks with the accumulated text before calling
providerUtils.ProcessAndSendResponse (use the existing sequenceNumber,
hasReceivedContent, rawResponseChunks, and the same
providerUtils.GetBifrostResponseForStreamResponse flow to send the final
events).

🧹 Nitpick comments (2)

framework/modelcatalog/utils.go (1)

21-22: Lowercase once before alias matching.

strings.Contains is case-sensitive, so RunwayML-style inputs still bypass this new mapping and keep pricing keys fragmented by casing. Normalizing p with strings.ToLower at the top of normalizeProvider would make the new alias robust.

Suggested refactor

 func normalizeProvider(p string) string {
+	p = strings.ToLower(p)
 	if strings.Contains(p, "vertex_ai") || p == "google-vertex" {
 		return string(schemas.Vertex)
 	} else if strings.Contains(p, "bedrock") {
 		return string(schemas.Bedrock)
 	} else if strings.Contains(p, "cohere") {
 		return string(schemas.Cohere)
 	} else if strings.Contains(p, "runwayml") {
 		return string(schemas.Runway)
 	} else {
 		return p
 	}
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@framework/modelcatalog/utils.go` around lines 21 - 22, The provider alias
check in normalizeProvider is case-sensitive (it uses strings.Contains(p,
"runwayml")), so normalizeProvider should lowercase the input once: at the start
of normalizeProvider set a normalized variable (e.g., p = strings.ToLower(p) or
use a new local like lower := strings.ToLower(p)) and then perform alias checks
(including the strings.Contains for "runwayml") against that lowercased value so
mappings like schemas.Runway match regardless of input casing.

core/providers/gemini/videos.go (1)

217-229: Copy seconds into the response instead of reusing the caller’s pointer.

Line 228 stores caller-owned state on the returned response. That makes the response mutable through later changes to the original variable and is inconsistent with the Runway path in this stack, which already copies the value. Prefer creating a fresh pointer here.

Suggested fix

 	if seconds != nil {
-		response.Seconds = seconds
+		response.Seconds = schemas.Ptr(*seconds)
 	}

Based on learnings, prefer using `bifrost.Ptr()` to create pointers consistently across the codebase.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/providers/gemini/videos.go` around lines 217 - 229, The function
ToBifrostVideoGenerationResponse is assigning the caller-owned seconds pointer
directly into response.Seconds, which preserves a mutable alias; change this to
copy the string into a new pointer before assigning (e.g., create a fresh string
pointer using the project's helper, bifrost.Ptr) so response.Seconds owns its
own memory; update the assignment in ToBifrostVideoGenerationResponse to use
that new pointer rather than the incoming seconds pointer.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@core/providers/gemini/gemini.go`:
- Around line 2484-2485: The polling response drops duration because
ToBifrostVideoGenerationResponse is called with nil for the duration parameter;
extract the seconds value from the operation (e.g., operation.Payload /
operation.Result / operation.Metadata where the VideoGeneration payload stores
duration) or from the persisted request metadata used by VideoGeneration and
pass that seconds value into ToBifrostVideoGenerationResponse instead of nil so
VideoRetrieve preserves response.seconds; update the call site around the
operation variable and ensure the same key used by VideoGeneration is read and
converted to the expected type before passing it to
ToBifrostVideoGenerationResponse.

In `@core/providers/gemini/types.go`:
- Around line 2231-2233: DefaultVideoDuration is a Gemini-specific exported
constant placed in types.go; move it into the Gemini provider's utils.go as an
unexported camelCase constant (e.g., defaultVideoDuration) so provider-specific
defaults remain internal, update any references to use the new name, and remove
the exported declaration from types.go to keep the package surface minimal and
follow the provider utils convention.

In `@core/providers/replicate/replicate.go`:
- Line 2799: To fix the dropped seconds, update the conversion path so
ToBifrostVideoGenerationResponse will populate response.Seconds even when the
explicit seconds arg is nil: inside ToBifrostVideoGenerationResponse, add a
fallback that extracts duration from the prediction object (e.g., check
prediction.Output/metadata/metrics or any duration field present on
ReplicatePredictionResponse) and set response.Seconds from that before
returning; alternatively, ensure VideoRetrieve passes the persisted duration
into ToBifrostVideoGenerationResponse instead of nil. Modify the code at the
ToBifrostVideoGenerationResponse call site and the converter function
(referencing ToBifrostVideoGenerationResponse, VideoRetrieve,
ReplicatePredictionResponse, VideoGeneration, and response.Seconds) so seconds
is never left nil.

---

Outside diff comments:
In `@core/providers/replicate/replicate.go`:
- Around line 1459-1729: The done branch currently treats every "done" event as
a successful completion and emits empty final payloads because it never parses
ReplicateDoneEvent nor uses an accumulated text buffer; update the "done" case
to parse currentEvent into a ReplicateDoneEvent (check the terminal Reason field
for "error" or "canceled"), and if non-successful emit the appropriate terminal
lifecycle/error response instead of a normal
ResponsesStreamResponseTypeCompleted; also maintain and use an accumulated text
buffer (e.g., accumulate currentEvent.Data into a buffer when handling "output"
deltas—separate from rawResponseChunks) so that when emitting textDoneResp,
partDoneResp and itemDoneResp you populate their Text/ContentBlocks with the
accumulated text before calling providerUtils.ProcessAndSendResponse (use the
existing sequenceNumber, hasReceivedContent, rawResponseChunks, and the same
providerUtils.GetBifrostResponseForStreamResponse flow to send the final
events).

In `@core/providers/replicate/videos.go`:
- Around line 83-104: In ToBifrostVideoGenerationResponse, when the seconds
parameter is nil, read the original duration from the
ReplicatePredictionResponse.Input map (prediction.Input["duration"]) and set
response.Seconds as a string pointer fallback; ensure you check prediction.Input
!= nil, type-assert the duration safely (handle numeric and string
representations), format it into the expected string form, create a *string and
assign it to response.Seconds, and avoid panics on missing/invalid types.

---

Nitpick comments:
In `@core/providers/gemini/videos.go`:
- Around line 217-229: The function ToBifrostVideoGenerationResponse is
assigning the caller-owned seconds pointer directly into response.Seconds, which
preserves a mutable alias; change this to copy the string into a new pointer
before assigning (e.g., create a fresh string pointer using the project's
helper, bifrost.Ptr) so response.Seconds owns its own memory; update the
assignment in ToBifrostVideoGenerationResponse to use that new pointer rather
than the incoming seconds pointer.

In `@framework/modelcatalog/utils.go`:
- Around line 21-22: The provider alias check in normalizeProvider is
case-sensitive (it uses strings.Contains(p, "runwayml")), so normalizeProvider
should lowercase the input once: at the start of normalizeProvider set a
normalized variable (e.g., p = strings.ToLower(p) or use a new local like lower
:= strings.ToLower(p)) and then perform alias checks (including the
strings.Contains for "runwayml") against that lowercased value so mappings like
schemas.Runway match regardless of input casing.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5afe555f-c08d-41df-9737-a92c079f4534

📥 Commits

Reviewing files that changed from the base of the PR and between c017629 and 228b24a.

📒 Files selected for processing (8)

• core/providers/gemini/gemini.go
• core/providers/gemini/types.go
• core/providers/gemini/videos.go
• core/providers/replicate/replicate.go
• core/providers/replicate/videos.go
• core/providers/runway/runway.go
• core/providers/vertex/vertex.go
• framework/modelcatalog/utils.go

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@core/schemas/videos.go`:
- Around line 124-158: The helper currently infers whether to apply the
DefaultVideoDuration from the original request's provider, which fails for
fallback/alias routing; update getSecondsFromVideoRequest and
BifrostVideoGenerationResponse.BackfillParams to accept an explicit concrete
provider parameter (e.g., provider ModelProvider) and key the defaulting logic
on that concrete provider instead of req.VideoGenerationRequest.Provider or
req.VideoRemixRequest.Provider; then modify all provider call sites that call
BackfillParams to pass the actual concrete provider constant (e.g.,
schemas.Gemini, schemas.Vertex, schemas.Runway) so that
getSecondsFromVideoRequest uses that provider to decide when to return
Ptr(DefaultVideoDuration) for r.Seconds (preserving existing nil checks and use
of Ptr and DefaultVideoDuration).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e3f5c08a-4c82-4646-abad-dac6002d3c1d

📥 Commits

Reviewing files that changed from the base of the PR and between 228b24a and 3a6fa90.

📒 Files selected for processing (7)

• core/providers/gemini/gemini.go
• core/providers/openai/openai.go
• core/providers/replicate/replicate.go
• core/providers/runway/runway.go
• core/providers/vertex/vertex.go
• core/schemas/videos.go
• framework/modelcatalog/utils.go

🚧 Files skipped from review as they are similar to previous changes (1)

• core/providers/vertex/vertex.go

[Pratham-Mishra04]

Merge activity

• Mar 13, 4:57 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Mar 13, 5:04 AM UTC: Graphite rebased this pull request as part of a merge.
• Mar 13, 5:04 AM UTC: @Pratham-Mishra04 merged this pull request with Graphite.