fix: include fallbacks when preparing transcription requests

[mmelvin0]

Summary

This fixes an issue where the fallbacks parameter ignored by transcription endpoints.

This affects transcription requests and governance routing rules fallback behavior.

It seems reading the fallbacks field was missed when internally preparing a transcription request. It explicitly added by governance/routing rules here and here and implicitly allowed as a form field here.

Additionally, there's an impedance mismatch between JSON requests map[string]any and multipart/form-data map[string][]string. Because multiple fallbacks get serialized as a JSON array by WriteMultipartField(), parseFallbacks() doesn't work as intended.

Changes

• ParseMultipartFormFields() now extracts multiple fields values with the same name to []string instead of only the last one as string.
• WriteMultipartField() now encodes []string as multiple form values under the same name instead of as JSON.
• prepareTranscriptionRequest() no longer ignores the fallbacks.

Type of change

• Bug fix

Affected areas

• Core
• Transports (HTTP)

How to test

I tested this manually with both request-driven (e.g. curl -F fallback=provider/model and governance-drive and every combination I could think of for my use cases.

Open to guidance as to how to add good tests for this. Current Go unit tests have some failures on clean dev branch for me.

Breaking changes

• No

If you happened to have a transcription endpoint using the fallback feature, it wasn't working before and will start working now.

Related issues

Closes #4005

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

Summary by CodeRabbit

• Bug Fixes

  • Repeated multipart form text fields are now preserved and aggregated instead of being overwritten.
  • Multipart reconstruction avoids writing duplicate form fields when payload overrides are applied.
  • Multi-value fields are serialized as multiple form entries rather than a single encoded value.

• New Features

  • Transcription requests submitted via multipart forms can include parsed fallback options.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
8 out of 11 committers have signed the CLA.

✅ akshaydeo
✅ BearTS
✅ roroghost17
✅ impoiler
✅ mmelvin0
✅ axelray-dev
✅ yanhao98
✅ alexef
❌ TejasGhatte
❌ Pratham-Mishra04
❌ stepsecurity-app[bot]
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Preserve repeated multipart text fields as strings or aggregated []string; emit each []string element as its own multipart field; avoid duplicate writes during reconstruction; parse multipart fallbacks and include parsed Fallbacks and Provider in transcription requests.

Changes

Fallback Handling in Transcription

Layer / File(s)	Summary
Multipart field aggregation and serialization core/network/multipart.go	ParseMultipartFormFields aggregates repeated non-file form fields into []string on second occurrence; WriteMultipartField emits one multipart field per []string element.
Reconstruction skip duplicate fields core/network/multipart.go	ReconstructMultipartBody closes current part and skips writing a payload-derived replacement when the same field name was already written earlier.
Tests: expect []string for repeated fields core/network/multipart_test.go	TestWriteMultipartField updated to assert parsed["tags"] is a []string when multiple tags values are written.
Fallback parsing and transcription request wiring transports/bifrost-http/handlers/inference.go	prepareTranscriptionRequest reads form.Value["fallbacks"], calls parseFallbacks (returns an error on parse failure), and includes parsed Fallbacks and the resolved Provider in schemas.BifrostTranscriptionRequest.

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• #3408: Multipart fallbacks parsing and propagation relate to routing-rule fallback leakage and ensuring fallbacks reach transcription requests.

Possibly Related PRs

• maximhq/bifrost#3924: Also touches multipart fallbacks handling and router-side filtering of fallbacks via available providers.

Suggested Reviewers

• akshaydeo

Poem

🐰 In fields where values used to hide,
I gather repeats and keep them side by side.
I write each tag as its own small song,
skip doubled echoes that don't belong.
Fallbacks read and wired along. 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: include fallbacks when preparing transcription requests' accurately and concisely describes the main change in the PR: fixing the bug where fallbacks were being ignored in transcription request preparation.
Description check	✅ Passed	The PR description covers the key required sections: Summary explaining the problem, Changes detailing what was modified, Type of change marked as Bug fix, Affected areas identified, Testing approach described, and Related issues linked. Minor gaps exist (no screenshots applicable, tests not added but acknowledged), but overall compliance is strong.
Linked Issues check	✅ Passed	The PR implementation fully addresses the linked issue #4005: code changes preserve fallback information through the request pipeline (ParseMultipartFormFields aggregates repeated fields, prepareTranscriptionRequest now reads fallbacks, WriteMultipartField serializes arrays correctly), ensuring fallbacks are forwarded and attempted when primary providers fail.
Out of Scope Changes check	✅ Passed	All changes directly support fixing the fallbacks bug: multipart parsing/writing enhancements enable proper handling of multiple fallback values, and prepareTranscriptionRequest integration ensures fallbacks flow through the request pipeline. No extraneous modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

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.}

[greptile-apps]

Confidence Score: 5/5

The changes are narrowly scoped to transcription multipart handling and do not touch inference routing, pooled objects, or provider converters; the logic is straightforward and correct across all traced governance + multipart round-trip paths.

All three files change in a consistent, complementary way: the serialization layer, reconstruction layer, and handler all now agree on how multi-value form fields are represented. No new shared state, no concurrency surface, and no provider interface changes.

core/network/multipart_test.go — the new ParseMultipartFormFields aggregation path and ReconstructMultipartBody []string dedup path are not directly exercised by any test case.

Important Files Changed

Filename	Overview
core/network/multipart.go	Three connected fixes: ParseMultipartFormFields aggregates repeated field names into []string, WriteMultipartField writes []string as individual form fields instead of JSON, and ReconstructMultipartBody adds a writtenFields dedup guard to prevent []string values from being written once per original occurrence.
core/network/multipart_test.go	Updates the WriteMultipartField test to assert []string return instead of JSON-encoded string; the new multi-value parsing behavior and ReconstructMultipartBody []string dedup path have no dedicated test coverage.
transports/bifrost-http/handlers/inference.go	prepareTranscriptionRequest now reads form.Value["fallbacks"] and passes them through parseFallbacks into BifrostTranscriptionRequest.Fallbacks; also removes redundant schemas.ModelProvider() cast since provider is already that type.

_{Reviews (4): Last reviewed commit: "fix: include fallbacks when preparing tr..." | Re-trigger Greptile}

[mmelvin0]

I got rid of the JSON encoding of []string in multi part requests entirely.

It looks like nothing else was using this other than fallbacks and none of the inference handlers that read it were decoding the JSON anyway.

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

core/network/multipart_test.go (1)

223-241: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add a regression test for repeated-field reconstruction (dedupe + order preservation).

This test covers write/parse, but the new ReconstructMultipartBody duplicate-skip logic is still untested for repeated keys (the critical fallback path). Please add a case that starts with repeated fallbacks, reconstructs with payload overrides, and asserts no duplicated entries and stable order.

As per coding guidelines, “Apply standard Go review practices: … deterministic tests, and table-driven coverage for behavior changes.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/network/multipart_test.go` around lines 223 - 241, The test suite is
missing coverage for ReconstructMultipartBody's duplicate-skip fallback path for
repeated keys; add a deterministic test that writes an initial multipart body
containing repeated field "fallbacks" (e.g., ["f1","f2","f2","f3"]), then call
ReconstructMultipartBody with payload overrides that replace some entries, parse
the reconstructed body (using ParseMultipartFormFields) and assert that the
resulting "fallbacks" has no duplicated values and preserves the intended order
(stable ordering after dedupe), and include this as a new table-driven case
alongside the existing Test that exercises WriteMultipartField and
ParseMultipartFormFields so regressions in ReconstructMultipartBody are caught.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@core/network/multipart_test.go`:
- Around line 223-241: The test suite is missing coverage for
ReconstructMultipartBody's duplicate-skip fallback path for repeated keys; add a
deterministic test that writes an initial multipart body containing repeated
field "fallbacks" (e.g., ["f1","f2","f2","f3"]), then call
ReconstructMultipartBody with payload overrides that replace some entries, parse
the reconstructed body (using ParseMultipartFormFields) and assert that the
resulting "fallbacks" has no duplicated values and preserves the intended order
(stable ordering after dedupe), and include this as a new table-driven case
alongside the existing Test that exercises WriteMultipartField and
ParseMultipartFormFields so regressions in ReconstructMultipartBody are caught.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f0ffba43-c543-43ef-92c4-474d3f4672bd

📥 Commits

Reviewing files that changed from the base of the PR and between f459b4c and d0d91d4.

📒 Files selected for processing (3)

• core/network/multipart.go
• core/network/multipart_test.go
• transports/bifrost-http/handlers/inference.go

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

core/network/multipart_test.go (1)

223-240: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add a regression test for duplicate-field reconstruction behavior.

Please add a focused case covering ReconstructMultipartBody’s new duplicate-skip path (Line 97–Line 100 in core/network/multipart.go) to assert payload-backed fields are written once even when the original multipart contains repeated field names.

As per coding guidelines, behavior changes in Go should include deterministic test coverage (preferably table-driven).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/network/multipart_test.go` around lines 223 - 240, Add a deterministic
unit test (table-driven style) in multipart_test.go that exercises
ReconstructMultipartBody’s duplicate-skip path by creating an original multipart
with repeated field names where one occurrence is payload-backed (e.g., a
file/part with payload metadata) and asserting that after calling
ReconstructMultipartBody the reconstructed body contains only a single written
instance of that payload-backed field (and other duplicate non-payload fields
are preserved or handled as expected); locate helper functions like
WriteMultipartField and ParseMultipartFormFields in the test to build the
original multipart, call ReconstructMultipartBody, and validate the parsed
output contains exactly one entry for the payload-backed field name.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@core/network/multipart_test.go`:
- Around line 223-240: Add a deterministic unit test (table-driven style) in
multipart_test.go that exercises ReconstructMultipartBody’s duplicate-skip path
by creating an original multipart with repeated field names where one occurrence
is payload-backed (e.g., a file/part with payload metadata) and asserting that
after calling ReconstructMultipartBody the reconstructed body contains only a
single written instance of that payload-backed field (and other duplicate
non-payload fields are preserved or handled as expected); locate helper
functions like WriteMultipartField and ParseMultipartFormFields in the test to
build the original multipart, call ReconstructMultipartBody, and validate the
parsed output contains exactly one entry for the payload-backed field name.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: dc1b8bce-51e5-49a8-a8c1-cc15acd36d9f

📥 Commits

Reviewing files that changed from the base of the PR and between d0d91d4 and 37e910f.

📒 Files selected for processing (3)

• core/network/multipart.go
• core/network/multipart_test.go
• transports/bifrost-http/handlers/inference.go

💤 Files with no reviewable changes (1)

• transports/bifrost-http/handlers/inference.go

[akshaydeo]

Hi @mmelvin0 — thanks for the contribution! This PR is currently blocked because our CLA bot shows the Contributor License Agreement as not yet signed. Could you sign it here so we can move this forward: https://cla-assistant.io/maximhq/bifrost?pullRequest=4011

Let us know if you run into any issues signing.