Skip to content

[fix]: accept string-valued Responses image actions - #5835

Open
citrocat wants to merge 103 commits into
maximhq:devfrom
citrocat:thoxvi/fix-responses-image-generation-action
Open

[fix]: accept string-valued Responses image actions#5835
citrocat wants to merge 103 commits into
maximhq:devfrom
citrocat:thoxvi/fix-responses-image-generation-action

Conversation

@citrocat

@citrocat citrocat commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix OpenAI Responses image-generation calls whose action is a JSON string such as "generate". Valid response.output_item.* and response.completed events now decode and round-trip instead of being skipped.

Root cause

ResponsesToolMessageActionStruct.UnmarshalJSON assumed every action was an object with a type discriminator. OpenAI image-generation actions are strings, so decoding the terminal frame failed before it could be forwarded.

Changes

  • Accept and preserve string-valued image-generation actions without changing existing object action variants.
  • Retain the optional image-generation tool action on requests.
  • Preserve the action through Framework streaming accumulator copies while remaining compatible with the Framework module's older pinned Core release.

Tests

  • GOMAXPROCS=4 go test -p 2 ./core/schemas ./core/providers/openai ./framework/streaming -count=1
  • (cd framework && GOMAXPROCS=4 GOWORK=off go test -p 2 ./streaming -count=1)
  • GOMAXPROCS=4 go build -p 2 ./core/schemas ./core/providers/openai ./framework/streaming
  • git diff --check origin/dev

Closes #5834

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b4074094-2d9a-4677-a1eb-937b7c6aab27

📥 Commits

Reviewing files that changed from the base of the PR and between 12d0e85 and 8a9033e.

📒 Files selected for processing (5)
  • core/providers/openai/responses_test.go
  • core/schemas/responses.go
  • core/schemas/responses_test.go
  • framework/streaming/responses.go
  • framework/streaming/responses_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
  • framework/streaming/responses_test.go
  • framework/streaming/responses.go
  • core/schemas/responses_test.go
  • core/schemas/responses.go
  • core/providers/openai/responses_test.go

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added support for preserving string-based actions associated with image-generation requests in responses and tool messages.
    • Image-generation tool data now exposes an optional action value.
  • Bug Fixes

    • Fixed response processing and streaming copies so image-generation action values are retained correctly.
    • Improved validation for invalid image-generation action data.
  • Tests

    • Added coverage for encoding, decoding, copying, and invalid input handling.

Walkthrough

Responses schemas now decode and preserve string-valued image-generation actions. Framework streaming deep-copy logic copies the new action field. Tests cover round trips, invalid input, and copy preservation.

Changes

Image-generation action support

Layer / File(s) Summary
Schema union and action decoding
core/schemas/responses.go, core/schemas/responses_test.go, core/providers/openai/responses_test.go
The action union accepts string-valued image-generation actions. Unmarshalling clears prior union state and rejects invalid JSON values. Image-generation tools now include an optional Action field. Tests cover "generate" and "edit" round trips and non-object inputs.
Framework streaming deep-copy preservation
framework/streaming/responses.go, framework/streaming/responses_test.go
Deep-copy logic preserves ResponsesImageGenerationCallAction. The optional string-field helper accepts generic source and destination values. A regression test verifies the copied action value.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: akshaydeo

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main fix for string-valued Responses image actions.
Description check ✅ Passed The description explains the root cause, changes, tests, affected behavior, and linked issue; optional template sections are not critical.
Linked Issues check ✅ Passed The implementation accepts and preserves string actions, retains object variants, preserves request and streaming-copy state, and adds regression tests for issue #5834.
Out of Scope Changes check ✅ Passed All production and test changes directly support the linked issue objectives for Core schemas, OpenAI Responses, and Framework streaming.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@citrocat
citrocat marked this pull request as ready for review August 4, 2026 13:44
@coderabbitai
coderabbitai Bot requested a review from akshaydeo August 4, 2026 13:45
coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 4, 2026
@akshaydeo
akshaydeo dismissed coderabbitai[bot]’s stale review August 5, 2026 07:18

The merge-base changed after approval.

@akshaydeo
akshaydeo requested a review from a team as a code owner August 5, 2026 07:18
@citrocat
citrocat force-pushed the thoxvi/fix-responses-image-generation-action branch from 887a17e to 4357425 Compare August 5, 2026 08:22
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 5, 2026
@citrocat
citrocat force-pushed the thoxvi/fix-responses-image-generation-action branch from 4357425 to 2b54969 Compare August 6, 2026 03:08
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 6, 2026
@citrocat
citrocat force-pushed the thoxvi/fix-responses-image-generation-action branch from 2b54969 to eac70e6 Compare August 6, 2026 08:58
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 6, 2026
@citrocat
citrocat force-pushed the thoxvi/fix-responses-image-generation-action branch from eac70e6 to b58e261 Compare August 7, 2026 03:48
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

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

Inline comments:
In `@core/schemas/responses.go`:
- Around line 1813-1829: Update ResponsesToolMessageActionStruct.UnmarshalJSON
to reject any non-object token after the string branch, including null, before
the object-dispatch Unmarshal into the type peek structure; return a clear error
and preserve string/object handling. Extend
TestResponsesToolMessageActionStruct_EdgeCases with a null input case asserting
rejection.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2ab78407-0c28-4c15-9728-65fac680e0df

📥 Commits

Reviewing files that changed from the base of the PR and between 7b1149e and b58e261.

📒 Files selected for processing (4)
  • core/schemas/responses.go
  • core/schemas/responses_test.go
  • framework/streaming/responses.go
  • framework/streaming/responses_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • framework/streaming/responses_test.go

Comment thread core/schemas/responses.go
@citrocat
citrocat force-pushed the thoxvi/fix-responses-image-generation-action branch from b58e261 to 195f35a Compare August 7, 2026 07:24
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 7, 2026
@citrocat

citrocat commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@akshaydeo — flagging for your review. This is an upstream schema incompatibility, not a config or timeout issue, and it is currently live in our production deployment.

The bug: OpenAI Responses image_generation_call.action is a JSON string ("generate"), but ResponsesToolMessageActionStruct.UnmarshalJSON modeled every action as an object union with a type discriminator. When a valid image-generation event arrives, decoding fails and the streaming handler logs Failed to parse stream response and skips the entire SSE frame — including the terminal response.completed. Clients then hang until idle/client timeout or hit an abnormal EOF; they never receive a terminal event.

Severity: we observe this in production telemetry on transports/v1.6.6 — ~16 occurrences on Aug 4, 2–4/day since, all Mismatch type ... value "generate". Every occurrence is a lost terminal event for that request. The same object-only schema exists on the latest stable release, so anyone using image generation through the Responses API is affected. This is silent data loss: the request returns HTTP 200 with partial events and no terminal state.

The fix (195f35a): accept string-valued actions, keep the object variants unchanged, reject non-string/non-object tokens, preserve the action through the Framework accumulator, and retain the optional request-side action. Minimal 5-file diff (+100/−6) with regression tests; go test -race on affected packages plus standalone module builds are green. CodeRabbit has approved.

Would appreciate your review when you have a chance.

jeremym-tanium and others added 4 commits August 10, 2026 15:10
…iew maintenance (maximhq#5693)

* feat: support matview_refresh_interval "off" to disable logstore matview maintenance

The materialized views back only the dashboard UI. Deployments that run
Bifrost headless behind their own observability stack pay the REFRESH
MATERIALIZED VIEW CONCURRENTLY cost for views nothing reads, and the 5s
floor means the interval alone cannot turn maintenance off.

With "off" (or a non-positive duration) the logs store skips view
creation, the initial refresh, and the periodic refresher entirely.
matViewsReady stays false, so dashboard queries fall back to the raw
tables, and the runtime self-heal path cannot re-arm maintenance since
it only triggers from matview-path queries.

* fix: guard matview self-heal when maintenance is disabled

Review follow-up: carry the resolved disabled state onto the store so
triggerMatViewSelfHeal cannot recreate views the configuration says must
not exist, and make the schema/docs explicit that a zero duration also
disables (positive sub-5s values still clamp up).
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

* **Bug Fixes**
  * Improved token parameter compatibility handling to preserve alternative formats when the primary option is unsupported.

* **Chores**
  * Version updated to 2.0.0.
  * Enhanced load testing configuration for more reliable builds.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Briefly explain the purpose of this PR and the problem it solves.

- What was changed and why
- Any notable design decisions or trade-offs

- [ ] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

Describe the steps to validate this change. Include commands and expected outcomes.

```sh
go version
go test ./...

cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

If adding new configs or environment variables, document them here.

If UI changes, add before/after screenshots or short clips.

- [ ] Yes
- [ ] No

If yes, describe impact and migration instructions.

Link related issues and discussions. Example: Closes maximhq#123

Note any security implications (auth, secrets, PII, sandboxing, etc.).

- [ ] 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

Adds a `THIRD_PARTY_NOTICES.md` file to formally document third-party components used in Bifrost that carry license terms requiring explicit attribution — specifically MPL-2.0 licensed dependencies and embedded source code derived from external projects.

## Changes

- Introduces `THIRD_PARTY_NOTICES.md` to attribute:
  - Embedded source code in `framework/migrator/migrator.go` derived from `go-gormigrate/gormigrate` (MIT)
  - Go binary dependencies carrying MPL-2.0 terms: `github.com/cyphar/filepath-securejoin` and `github.com/hashicorp/go-version`
  - npm build-time devDependencies carrying MPL-2.0 terms: `lightningcss` (never shipped to end users) and `dompurify` (Apache-2.0 option elected)
- All MPL-2.0 components are used unmodified and combined as a "Larger Work" per MPL-2.0 Section 3.3; no Bifrost source files are themselves MPL-licensed.

## Type of change

- [ ] Bug fix
- [ ] Feature
- [ ] Refactor
- [x] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [x] Docs

## How to test

No functional changes — review the file contents to confirm accuracy of license attributions against the listed upstream repositories.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

This change has no security implications. It is a legal/compliance attribution document only.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
roroghost17 and others added 7 commits August 10, 2026 15:18
…aximhq#5941)

Adds `traces_enabled`, `trace_headers`, and `metrics_headers` fields to the OTEL plugin configuration (both single-profile and multi-profile shapes), enabling metrics-only OTEL profiles and per-signal header overrides.

- Added `traces_enabled` boolean to OTEL config. When set to `false`, trace export is skipped and `collector_url` / `trace_type` are no longer required, allowing a metrics-only profile to be configured without a trace collector.
- Added `trace_headers` and `metrics_headers` maps to OTEL config. The existing `headers` field continues to apply to both endpoints; `trace_headers` and `metrics_headers` are overlaid on top per-signal, with the more specific key winning on conflict. This supports cases where a collector requires a signal-specific header (e.g. a Databricks table name on the metrics endpoint only).
- Updated validation logic in `_helpers.tpl` so that `collector_url` and `trace_type` are only required when `traces_enabled` is `true`, and `protocol` is only required when at least one of traces or metrics is enabled.
- Updated `values.schema.json` conditional validation (`allOf`/`if`/`then`) to reflect the same rules: `collector_url`, `trace_type`, and `protocol` are gated on both `enabled` and `traces_enabled` not being `false`; `metrics_endpoint` and `protocol` are required together when `metrics_enabled` is `true`.
- Updated `values.yaml` comments and `README.md` changelog to document the new fields.

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

Deploy the Helm chart with a metrics-only OTEL profile and verify that no trace collector URL is required:

```yaml
bifrost:
  plugins:
    otel:
      enabled: true
      config:
        traces_enabled: false
        metrics_enabled: true
        metrics_endpoint: "http://otel-collector:4318/v1/metrics"
        protocol: "http"
        metrics_headers:
          x-databricks-table: "my_table"
```

```sh
helm template . -f values.yaml | grep -A 30 "otel"

helm lint .
```

Verify that omitting `collector_url` with `traces_enabled: false` passes linting, and that omitting it with `traces_enabled: true` (default) still fails with the appropriate error message.

N/A

- [ ] Yes
- [x] No

N/A

`trace_headers` and `metrics_headers` support the `env.VAR_NAME` prefix for injecting secrets from environment variables, consistent with the existing `headers` field. No new secret handling mechanisms are introduced.

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
…cs list (maximhq#5942)

## Summary

Documents two new OTel plugin capabilities: per-signal headers (`trace_headers` and `metrics_headers`) and a `traces_enabled` flag that enables a metrics-only mode where `collector_url` is not required.

## Changes

- Added `traces_enabled` field documentation — when set to `false`, the trace client is never built and `collector_url`/`trace_type` become optional, enabling metrics-only profiles
- Added `trace_headers` and `metrics_headers` fields — these are overlaid on top of the shared `headers` field for their respective endpoints, with per-signal values winning on key collision
- Clarified that `headers` is sent to both trace and metrics endpoints, and that `protocol` is shared between both signals
- Added a "Per-signal headers" section with a worked example showing `Authorization` shared via `headers` and `X-Databricks-Table` scoped to the metrics endpoint via `metrics_headers`
- Added a "Metrics-only mode" section with a full JSON configuration example
- Expanded the pushed metrics table to include `bifrost_cache_read_input_tokens_total`, `bifrost_cache_write_input_tokens_total`, `bifrost_cache_write_input_tokens_5m_total`, `bifrost_cache_write_input_tokens_1h_total`, `bifrost_request_retries`, and `mcp.client.operation.duration`
- Added a note clarifying that an unreachable metrics endpoint never blocks Bifrost startup
- Updated env-var substitution docs to include `trace_headers` and `metrics_headers`
- Applied the same changes to both the `config-json` and Helm plugin reference pages

## Type of change

- [ ] Bug fix
- [ ] Feature
- [ ] Refactor
- [x] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [x] Docs

## How to test

Review the rendered documentation for the OTel plugin pages:

- `docs/features/observability/otel.mdx`
- `docs/deployment-guides/config-json/plugins.mdx`
- `docs/deployment-guides/helm/plugins.mdx`

Verify that:
1. The `traces_enabled: false` example produces a valid metrics-only config with no `collector_url`
2. The per-signal headers example correctly shows `Authorization` on both endpoints and `X-Databricks-Table` only on the metrics endpoint
3. All new metrics in the pushed metrics table are accurately described

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

`trace_headers` and `metrics_headers` support the `env.` prefix for environment variable substitution, consistent with the existing `headers` field. No new secrets are stored in configuration.

## Checklist

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

Extends video logging and the log detail UI to fully support delete, list, download, and generation/remix/retrieve response types. Previously, delete responses were not routed to any log column, and the video detail view lacked support for delete output, base64-encoded video, and several generation metadata fields.

## Changes

- In `applyNonStreamingOutputToEntry`, added routing for `VideoGenerationResponse`, `VideoDownloadResponse`, `VideoListResponse`, and `VideoDeleteResponse` into their respective log entry fields. `VideoGenerationResponse` is shared by generation, remix, and retrieve operations, so the request type is used as the discriminator to separate retrieve into its own column.
- Added `video_delete_output` to the `videoOutput` expression in `logDetailView.tsx` so delete responses trigger the video detail panel.
- Updated `VideoView` to handle `BifrostVideoDeleteOutput` as a distinct output type, rendering the video ID and deleted status.
- Replaced the ad-hoc `requestType.toLowerCase().includes(...)` label logic with a lookup against `RequestTypeLabels`.
- Added `getVideoSrc` to resolve a video source from either a URL or a base64 payload, and updated the video rendering loop to support multiple videos and base64-encoded content.
- Added display of additional generation metadata fields: duration (`seconds`), size, and `remixed_from_video_id`.
- Added `CopyableId` to video ID fields in the download and generation output sections.
- Added the `ContentFilterInfo` type and `content_filter` field to `BifrostVideoGenerationOutput`.
- Changed `seconds` from `number` to `string` on both `VideoObject` and `BifrostVideoGenerationOutput` to match the API shape.
- Added tests covering all video response types (generation, remix, retrieve, download, list, delete) and verifying that content logging disabled suppresses video output.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

```sh
# Core/Transports
go test ./plugins/logging/...

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```

Trigger video generation, remix, retrieve, download, list, and delete requests and verify each response appears in the correct log column in the UI. Confirm that with content logging disabled, no video output fields are populated.

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

No new auth, secrets, or PII surface area introduced. Video content is explicitly noted as not stored in logs for download responses.

## Checklist

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

Bedrock was rejecting documents with "The PDF specified was not valid" because the document format was always resolved to `"pdf"` regardless of the actual file type. Standard OpenAI clients encode the MIME type inside the data URL (e.g. `data:application/vnd.openxmlformats-officedocument.spreadsheetml.sheet;base64,...`) rather than in the `file_type` field, which was the only source previously consulted. This PR fixes format resolution for both the Chat and Responses paths, unifies the mapping logic, and corrects several related data URL parsing defects.  
  
Fixes maximhq#5472

## Changes

- Extracted a shared `bedrockDocumentFormat` helper in `utils.go` that maps MIME types and bare file extensions to Bedrock Converse document format strings, replacing two duplicated inline switch blocks that were missing most MIME types.
- Format resolution now follows a priority chain: `file_type` → data URL media type → filename extension → `"pdf"` default. Previously only `file_type` was consulted.
- `ParseDataURL` in `schemas/utils.go` is now a public function that correctly handles media type parameters (e.g. `;charset=utf-8`), uppercase media types, and payloads containing newlines. The old regex silently dropped any data URL whose header contained a parameter, causing the entire `"data:..."` string to be forwarded to Bedrock as the document payload.
- Non-base64 data URLs (e.g. `data:text/plain,Hello%20World`) are now percent-decoded and their text content is populated in both `source.text` and `source.bytes` instead of being forwarded verbatim.
- The Responses path (`responses.go`) previously ignored `file_url` entirely, emitting a document block with an empty source. It now fetches and inlines the bytes the same way the Chat path does, and propagates fetch errors rather than swallowing them.
- `convertBifrostMessageToBedrockMessage` now returns an error instead of silently returning `nil` on conversion failure, so a missing turn is never silently dropped from the request.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./core/providers/bedrock/... ./core/schemas/...
```

Key test cases added:

- `TestDocumentFormatFromDataURL` — verifies that each supported MIME type embedded in a data URL resolves to the correct Bedrock format string and that the `data:...` prefix is stripped from `source.bytes`.
- `TestDocumentFormatResolutionPrecedence` — verifies the `file_type` → data URL → filename extension → default priority chain.
- `TestDocumentInlineTextDataURL` — verifies that non-base64 data URLs are percent-decoded and stored in both `source.text` and `source.bytes`.
- `TestToBedrockResponsesRequest_DocumentFormatFromDataURL` — same format fix verified on the Responses path.
- `TestToBedrockResponsesRequest_DocumentFileURLIsFetched` — verifies that an unreachable `file_url` surfaces as an error rather than producing an empty document block.
- `TestParseDataURL` — unit tests for the new public `ParseDataURL` function covering parameters, uppercase, newlines in payload, and invalid inputs.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

`file_url` values are now fetched over the network on the Responses path (matching existing Chat path behaviour). The fetch is performed with the existing `providerUtils.FetchAndEncodeURL` helper, which is subject to the same controls already in place for image URL fetching.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
Adds regression coverage for maximhq#5472, where Bedrock's document format converter defaulted every uploaded document to `format:"pdf"` regardless of the actual file type, causing AWS to reject non-PDF documents with `ValidationException`. This PR adds 14 end-to-end test cases to the provider harness collection covering the fixed behavior across both `/v1/chat/completions` and `/v1/responses`.

- Added folder **42. Bedrock Document Uploads via OpenAI type:"file" (maximhq#5472)** to the provider harness collection with 14 test cases:
  - Cases 1–11 exercise `/v1/chat/completions` with XLSX, DOCX, CSV, PDF, TXT, and `file_url` inputs, covering format resolution by data URL media type, filename extension, explicit `file_type`, charset-parameterized data URLs, non-base64 percent-encoded data URLs, opaque media types, and streaming
  - Cases 12–14 pin the same invariants on `/v1/responses` `input_file` blocks (XLSX data URL, CSV data URL, `file_url`)
  - Every fixture embeds the token `BIFROST7788` so assertions confirm the document was actually parsed by Claude, not merely accepted
- Updated `HARNESS_COVERAGE_BACKLOG.md` to mark the **Document input** item as partially covered (`[~]`), noting that the OpenAI `type:"file"` / Responses `input_file` path is now covered by folder 42, while a native Converse-shaped `document` block posted directly at `/bedrock/model/{id}/converse` remains uncovered

- [ ] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [x] Chore/CI

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

Import `tests/e2e/api/collections/provider-harness.json` into Postman and run folder **42. Bedrock Document Uploads via OpenAI type:"file" (maximhq#5472)** against a running Bifrost instance with Bedrock credentials configured.

Each test asserts:
- The response does not contain `"The PDF specified was not valid"`, `"could not be parsed as the specified format"`, or `"The document source bytes"` (the AWS rejection messages from the bug)
- The response status is below 400
- For document-content cases, the model's reply includes `BIFROST7788`, confirming the document was read

Before the fix, cases 1–3, 5–8, and 12–14 all returned a 400 `ValidationException`.

- [x] No

Closes maximhq#5472

None. Test fixtures contain only synthetic document content with no real credentials or PII.

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

xAI's `grok-imagine` image generation API returns a `cost_in_usd_ticks` field in its usage object instead of token counts. Without this field on `ImageUsage`, the value was silently dropped during unmarshalling, causing the response to surface an empty `"usage":{}`.  
  
Fixes maximhq#5498

## Changes

- Added `CostInUsdTicks *int64` to `ImageUsage` with `omitempty` so it is only serialized when present, leaving existing provider responses (OpenAI, Gemini, etc.) unaffected.
- Extended `DeepCopy` to allocate a new pointer for `CostInUsdTicks`, preserving the no-shared-pointers contract relied on by cost calculation logic.
- Added tests covering round-trip marshal/unmarshal of `cost_in_usd_ticks`, omission of the field when absent, and pointer independence after `DeepCopy`.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./core/schemas/...
```

Expected: all three new tests pass — `TestImageUsage_CostInUsdTicksRoundTrip`, `TestImageUsage_CostInUsdTicksOmittedWhenAbsent`, and `TestImageUsage_DeepCopyCostInUsdTicks`.

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

No security implications. The new field is a cost/billing value returned by xAI and is passed through as-is.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
…ximhq#5960)

## Summary

Anthropic returns a 400 error with no error code when it rejects a `redacted_thinking` block containing a foreign or invalid payload. The existing `isEncryptedReasoningRejection` detection did not match this error format, causing the retry logic to miss these rejections and fail to strip the offending encrypted content before retrying.

## Changes

- Extended `isEncryptedReasoningRejection` to also match Anthropic's `redacted_thinking`-specific rejection message: `"Invalid \`data\` in \`redacted_thinking\` block"`.
- Added a comment explaining why this additional check is needed (Anthropic omits an error code and names the offending block in the message text instead).
- Added three new test cases:
  - Confirms the `redacted_thinking` rejection is correctly detected.
  - Confirms that a `thinking` block signature rejection is intentionally **not** matched (since stripping encrypted content would not fix it and would cause an infinite retry loop).
  - Confirms that an unrelated Anthropic 400 mentioning `thinking` (e.g., invalid `budget_tokens`) is not incorrectly matched.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./core/...
```

The three new test cases in `TestIsEncryptedReasoningRejection` cover the added detection logic and the intentional non-matches.

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

No auth, secrets, or PII implications. The change only affects error message pattern matching used to decide whether to strip encrypted reasoning content before retrying a request.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
@akshaydeo
akshaydeo dismissed coderabbitai[bot]’s stale review August 10, 2026 22:20

The merge-base changed after approval.

akshaydeo and others added 11 commits August 10, 2026 19:23
## Summary

Bumps a set of Go and JavaScript/TypeScript dependencies to their latest patch/minor releases across all modules in the repository. Also fixes a misindented test block in the governance utility tests and removes the Node.js engine constraint from the UI `package.json`.

## Changes

- **Go dependencies upgraded:**
  - `golang.org/x/text`: `v0.37.0` → `v0.39.0`
  - `golang.org/x/crypto`: `v0.52.0` → `v0.53.0`
  - `golang.org/x/net`: `v0.55.0` → `v0.56.0`
  - `golang.org/x/sys`: `v0.45.0` → `v0.46.0`
  - `golang.org/x/sync`: `v0.20.0` → `v0.21.0`
  - `github.com/go-jose/go-jose/v4`: `v4.1.3` → `v4.1.4` (token-exchange-demo-server)
  - `github.com/buger/jsonparser`: `v1.1.1` → `v1.1.2` (token-exchange-demo-server)
  - `github.com/go-git/go-git/v5`: `v5.19.1` → `v5.19.2` (transports)
  - Added `github.com/google/uuid v1.6.0` as a direct dependency in `plugins/logging`

- **JavaScript/TypeScript dependencies upgraded:**
  - `axios`: `1.16.1` → `1.19.0`
  - `dompurify`: `3.4.12` → `3.4.13`
  - `nanoid`: `3.3.16` → `3.3.18`
  - `hono`: `4.12.27` → `4.12.34` (MCP example servers)
  - `fast-uri`: `3.1.4` → `3.1.5` (MCP example servers, added as an explicit override)
  - `zod`, `express`, and `hono` marked as `peer` dependencies in lock files

- **Test fix:** Corrected indentation of the `"anchors calendar-aligned validity to the current period boundary"` `it` block in `governance.test.ts`, which was previously placed outside its enclosing `describe` block due to a missing closing brace.

- **UI:** Removed the `engines.node` constraint (`>=22.12.0`) from `ui/package.json`.

## Type of change

- [ ] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [x] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

```sh
# Core/Transports
go version
go test ./...

# UI
cd ui
npm i
npm test
npm run build
```

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

`golang.org/x/crypto` and `golang.org/x/net` are security-sensitive packages; upgrading them to the latest patch versions ensures any upstream CVE fixes are included. `axios` `1.19.0` and `dompurify` `3.4.13` similarly incorporate upstream security patches.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
…il (maximhq#5955)

## Summary

Clarifies that `policy_id` is a **required** field for the Gray Swan Cygnal Monitor guardrail integration. Previously, the docs presented it as optional and showed an empty string as the default, which caused Gray Swan to reject monitor requests silently. This update corrects the field's required status across all documentation surfaces and adds an explicit note explaining that Gray Swan does not apply a default policy and that the Bifrost profile name is not a substitute for a Gray Swan policy ID.

## Changes

- Marked `policy_id` as required (was `No`, now `Yes`) in the Gray Swan parameter tables across the config-json, Helm, and integration docs
- Replaced the empty `policy_id: ""` placeholder with `"YOUR_GRAYSWAN_POLICY_ID"` in all example snippets to make the requirement immediately visible
- Added `policy_id` to the enterprise guardrails example configurations (both JSON and Helm) where it was previously missing
- Added an explicit callout in the integration reference doc clarifying that `policy_id` is mandatory even when custom `rules` are defined, and that the Bifrost configuration name is not a Gray Swan policy ID

## Type of change

- [ ] Bug fix
- [ ] Feature
- [ ] Refactor
- [x] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [x] Docs

## How to test

Review the rendered documentation pages for the following:

- `docs/integrations/guardrails/grayswan.mdx` — confirm `policy_id` row shows `Yes` in the Required column and the new callout paragraph is present
- `docs/deployment-guides/config-json/guardrails.mdx` — confirm the example snippet shows `"YOUR_GRAYSWAN_POLICY_ID"` and the table reflects `Yes` / `Required Gray Swan policy ID string`
- `docs/deployment-guides/helm/guardrails.mdx` — same as above for the Helm variant
- `docs/enterprise/guardrails.mdx` — confirm `policy_id` appears in both the JSON and Helm example blocks

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

No secrets or auth changes. The update ensures users do not accidentally omit a required policy identifier, which could result in unguarded requests reaching Gray Swan without any policy enforcement applied.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
…aximhq#5522)

## Summary

Adds a "Group" toggle to the logs table that collapses fallback chains under their root request. When enabled, the table fetches only root-level log entries (`roots_only=true`) and lazily loads each chain's children via the sessions endpoint when a row is expanded. This makes it easier to understand multi-step fallback sequences without being overwhelmed by individual attempt rows.

## Changes

- Added a `grouped` URL state parameter (`parseAsBoolean`) that is automatically disabled when a `parent_request_id` session filter is active, since that view is already scoped to a single chain.
- Introduced a `rootsOnly` parameter to the `getLogs` API query, which appends `roots_only=true` to the request when grouped view is active.
- Added `child_count`, `children_cost`, and `children_tokens` fields to `LogEntry` for aggregate data returned by the `roots_only` endpoint.
- Introduced a `DisplayLogEntry` type that extends `LogEntry` with a `__chainChild` flag, used to mark lazily-loaded child rows injected below their expanded parent in the table.
- Added an `expand` column to the logs table in grouped mode. Root rows with children show a chevron + child count button; child rows show a corner connector icon to indicate hierarchy.
- Chain expansion state (`expandedChainIds`, `chainChildren`, `loadingChainIds`) is managed locally on the page and reset whenever filters, pagination, or the grouped toggle changes.
- Added a `tableMeta` prop to `LogsDataTable` so the expand column can access toggle callbacks without threading props through column factories.
- Child rows are visually distinguished with a left border and a muted background.
- Added a "Group" button to `LogsHeaderView` with a tooltip explaining the behavior and a performance caveat for large tables.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

1. Navigate to the Logs page.
2. Click the **Group** button in the header toolbar.
3. Verify the table switches to showing only root requests, with a chevron and child count on rows that have fallback children.
4. Click a chevron to expand a chain — child rows should appear indented below the root with a left border.
5. Click the chevron again to collapse.
6. Apply a session/parent filter and confirm the Group toggle is automatically disabled.
7. Change the page or filters and confirm expanded state resets.

```sh
cd ui
pnpm i
pnpm build
```

## Screenshots/Recordings

_Add before/after screenshots of the grouped vs. flat log table view._

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

No new auth surfaces. The sessions endpoint used for lazy-loading children is already gated by the same RBAC policies as the main logs endpoint.

## 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

Adds the ability to cancel a running cost recalculation job from the logs header. Previously, once a recalculation was started it could not be stopped from the UI. This PR wires up a new `POST /logs/recalculate-cost/cancel` API endpoint, adds a `cancelled` terminal status to the job lifecycle, and updates the progress toast and "More actions" menu to expose a cancel control.

## Changes

- Added `cancelRecalculateCostJob` mutation to `logsApi` targeting `POST /logs/recalculate-cost/cancel`, which resolves with the job's post-cancel status.
- Added `"cancelled"` as a terminal `RecalcJobStatus` status alongside `completed` and `failed`, with counters reflecting work committed before stopping.
- Introduced `isTerminalRecalcStatus` helper and a shared `RECALC_TOAST_ID` constant to consolidate all recalculation lifecycle toast updates onto a single toast.
- Added `recalcCancelRequested` state to track the window between the cancel request being sent and the job settling, preventing duplicate cancel clicks and avoiding progress toast overwrites during that window.
- The in-progress toast now includes a **Cancel** action button; `event.preventDefault()` keeps the toast mounted so it can report the cancellation result rather than dismissing immediately.
- The "More actions" menu item transforms into a **Cancel recalculation** control while a job is running, providing a fallback cancel path if the toast was dismissed. It is disabled (not hidden) while cancellation is in flight.
- On a `cancelled` terminal status, an informational (non-error) toast reports the partial result, and logs/stats are refreshed since the job may have committed partial cost updates.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

1. Navigate to the Logs view and open **More actions**.
2. Click **Recalculate costs** to start a recalculation over a large log window.
3. While the progress toast is visible, click the **Cancel** button on the toast. Verify:
   - The toast switches to "Cancelling cost recalculation…" with a description about finishing the current batch.
   - The "More actions" menu item shows "Cancelling…" and is disabled.
   - Once the job settles, an info toast appears reporting the partial result (rows updated/skipped before stopping).
   - The logs and stats views refresh.
4. Repeat and cancel via the **More actions** menu item instead of the toast button.
5. Dismiss the progress toast mid-run, then open **More actions** and verify **Cancel recalculation** is still available and functional.
6. Verify that a completed or failed job still reports correctly and is unaffected by this change.

```sh
cd ui
pnpm i || npm i
pnpm build || npm run build
```

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

No new auth surfaces. The cancel endpoint follows the same authentication pattern as the existing recalculate-cost endpoints.

## 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

Adds a `cancelled` terminal status to the Sidekiq job system and wires it end-to-end: from the database layer through the runner to a new `POST /api/logs/recalculate-cost/cancel` HTTP endpoint. A cancelled job stops immediately on the owning node (via context cancellation), never gets re-claimed by the dispatcher, and is never reaped as stale. Partial progress committed before the stop is preserved in the job's metadata so the UI can report what was accomplished.

## Changes

- **`tables.SidekiqStatusCancelled`** added as a distinct terminal status alongside `completed` and `failed`. A `SidekiqTerminalStatuses` slice and `IsSidekiqTerminalStatus` helper centralize terminal-status checks across the codebase.

- **`CancelSidekiqJob`** (store): atomically flips a pending or running job to `cancelled` and stamps `completed_at`. Deliberately not fenced on `runner_id` — the cancel request can arrive on any node. Returns `true` only when this call performed the transition.

- **`FinalizeCancelledSidekiqJob`** (store): writes the handler's last metadata snapshot onto a cancelled job without touching its status. Fenced on `runner_id` and `status = cancelled` so a stale runner cannot overwrite a re-claimed row.

- **`Runner.Cancel`** (sidekiq runner): cancels the durable row first, then immediately signals the handler's context via a new `jobCancels` map if the job is running on the same node. A race window between claim and registration is closed by re-checking the row status after registration.

- **`execute` error path** (sidekiq runner): distinguishes a cancelled unwind from a genuine failure. When the handler returns an error with a cancelled context and the row status is `cancelled`, `FinalizeCancelledSidekiqJob` is called instead of `FailSidekiqJob`. The same check applies when `CompleteSidekiqJob` is rejected because a cancel landed during the handler's final batch.

- **`RunCostRecalcJob`** (cost recalc plugin): the early-exit path (both the `ctx.Err()` guard at the top of the loop and the checkpoint-rejection path mid-loop) now sets a human-readable `Message` summarising how far the job got before stopping, so the UI can display it for both shutdown-interrupted and user-cancelled runs.

- **`POST /api/logs/recalculate-cost/cancel`**: new HTTP endpoint. Accepts an optional `?id=` query parameter; without it, cancels the current in-flight recalculation job. Guards against cancelling jobs of other kinds. Returns the job's status after cancellation (including partial progress counters) so the caller can settle its UI from the same shape it was polling. Cancelling an already-terminal job is a no-op that returns the job as-is.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./framework/configstore/... ./framework/sidekiq/... ./transports/bifrost-http/... ./plugins/logging/...
```

**Manual flow:**

1. Start a cost recalculation against a large log window so it runs for several seconds.
2. `POST /api/logs/recalculate-cost/cancel` — expect `200` with `status: "cancelled"` and non-zero `processed`/`updated` counters.
3. `GET /api/logs/recalculate-cost/status` — confirm the job remains `cancelled` and is not re-claimed or reaped.
4. Start another recalculation — confirm it is accepted (the cancelled job is not treated as in-flight).
5. Let a job complete, then `POST /api/logs/recalculate-cost/cancel?id=<completed-id>` — expect `200` with `status: "completed"` and no state change.

## Breaking changes

- [ ] Yes
- [x] No

Any store implementation of `ConfigStore` or `sidekiq.Store` must now implement `CancelSidekiqJob` and `FinalizeCancelledSidekiqJob`. The `MockConfigStore` and all fake stores in tests have been updated.

## Related issues

## Security considerations

The cancel endpoint is guarded by the same middleware chain as the existing recalculate-cost endpoints. The `?id=` path validates that the resolved job's `Kind` matches `CostRecalcJobKind`, preventing the endpoint from being used to cancel arbitrary background jobs.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
…aximhq#5806)

## Summary

Adds white-label branding support to the UI, allowing enterprise deployments to replace the Bifrost logo and icon across the sidebar and login screen. On OSS builds, the feature is gated behind an enterprise check and redirects to client settings, while still rendering a "Contact Us" upgrade prompt via the OSS fallback stub.

## Changes

- Added a `useBranding` hook that queries a new `/branding` API endpoint and resolves which logo and icon to render. Each slot (logo, icon) falls back independently to the bundled Bifrost defaults, so a deployment that only uploaded a logo keeps the default icon. Custom assets are theme-agnostic; `isDark` only selects between the two bundled defaults.
- Added `brandingApi` with `getBranding`, `updateBranding`, and `resetBranding` endpoints. The read endpoint is public so the login screen can fetch branding before a session exists.
- Replaced all hardcoded `/bifrost-logo*.webp` and `/bifrost-icon*.webp` references in the sidebar and login view with `useBranding`, including correct `alt` text (empty string on white-labelled deployments, "Bifrost" otherwise).
- Fixed the login route's pending/loading screen so it wraps `PendingCard` in `ReduxProvider`, allowing it to call `useBranding` and show the customer's logo instead of flashing the Bifrost default.
- Added a `/workspace/config/branding` route (enterprise-only). On OSS it immediately redirects to client settings. On enterprise it renders `BrandingView`.
- Added an OSS fallback stub for `BrandingView` that renders a "Contact Us" upgrade prompt with a link to the enterprise docs.
- Added a "Branding" entry to the sidebar config nav (enterprise-only) with a `Palette` icon.
- Added a `resolveBrandingAssetUrl` utility to correctly prefix API-relative asset paths in development, where the app and API are served from different origins.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

1. **OSS build**: Navigate to `/workspace/config/branding` — it should redirect to `/workspace/config/client-settings`. The "Branding" sidebar entry should not appear.
2. **Enterprise build**: Navigate to `/workspace/config/branding` — the `BrandingView` should render. The "Branding" sidebar entry should appear under config.
3. Upload a custom logo via the enterprise branding API (`PUT /branding`). Verify the sidebar and login screen reflect the uploaded logo in both light and dark mode without flashing the Bifrost default on navigation.
4. Reset branding (`DELETE /branding`) and confirm the Bifrost defaults are restored.
5. Verify the login screen's "Checking authentication..." pending state shows the correct logo rather than the hardcoded Bifrost one.

```sh
cd ui
pnpm i || npm i
pnpm build || npm run build
```

## Screenshots/Recordings

_Add before/after screenshots of the sidebar and login screen with a custom logo applied._

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

_Link related issues here._

## Security considerations

The `/branding` read endpoint is intentionally public (no session required) so the login screen can fetch branding assets before authentication. Write and delete endpoints are enterprise-only and should be protected by the existing RBAC settings access check on the backend.

## 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
…ewriting (maximhq#5807)

## Summary

Introduces a `ShellRewriter` hook that allows enterprise builds to rewrite the pre-hydration HTML shell before it is served, eliminating the flash of the default Bifrost logo before React hydrates on branded deployments. OSS deployments are unaffected — the rewriter is `nil` by default and the embedded document is served exactly as bundled.

## Changes

- Added the `ShellRewriter` function type to `UIHandler`, providing a seam for the enterprise build to swap logo references in the static HTML skeleton before it reaches the client. The rewriter is only invoked for `.html` files and is skipped entirely when `nil`.
- Exported `ShellRewriter` on `BifrostHTTPServer` so the enterprise build can assign an implementation before `RegisterUIRoutes` is called, which constructs the `UIHandler` from it.
- Updated `NewUIHandler` to accept a `ShellRewriter` parameter (nil-safe) and `RegisterUIRoutes` to forward `s.ShellRewriter` into it.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## Breaking changes

- [x] Yes — `NewUIHandler` now requires a second `ShellRewriter` argument (pass `nil` for OSS/default behaviour)
- [ ] No

## Security considerations

- The rewriter runs on the request path for every HTML document served; implementations must be cheap and must return data unchanged when there is nothing to do.
- OSS builds never execute any rewrite logic, as the `nil` check short-circuits before the function is called.

## Checklist

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

Adds the ability to assign a virtual key directly to a user from the virtual key sheet. Previously, user assignment was read-only (displayed as a static label) and blocked all other entity-type changes. Now users can select "Assign to User" from the entity type picker, choose a user via the enterprise user picker, and save — with the assignment applied via the dedicated `/virtual-keys/{id}/users` endpoint rather than the VK payload itself.

## Changes

- Added `useAttachVirtualKeyUsersMutation` and `useDetachVirtualKeyUserMutation` OSS fallback stubs so the virtual key sheet type-checks in non-enterprise builds. The "Assign to User" option remains hidden in OSS because the user picker registry is never populated.
- Added `"user"` as a valid `entityType` in the form schema, along with a `userId` field and a corresponding validation refinement.
- Replaced the static "Assigned To" read-only display with a `UserPicker` form field that appears when `entityType === "user"` and the enterprise picker is registered.
- User assignment is applied after the VK payload update so that a key moving from a team to a user has its `team_id` cleared before the attach request lands. On create, assignment failure is surfaced as a separate toast so the user knows the key was created but is unassigned.
- Removed the restriction that locked the entity type selector when a user was already attached. The assignment can now be changed freely, including detaching a user by switching to "none".
- Added a `useRef` guard to seed the `userId` form field from the separately-fetched user association only once and only if the user hasn't already touched the assignment field.
- The `team_id`/`customer_id` clearing logic in the update payload was simplified using a `clearsEntity` flag, removing the previous `assignedUsers.length > 0` guard that was preventing those fields from being nulled out.
- A helper note is shown below the user picker explaining the one-user limit and access profile adoption behavior.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

1. On an enterprise build, open the virtual key sheet for an existing key with no entity assignment.
2. Set "Assignment Type" to "Assign to User", select a user, and save. Confirm the key reloads with the user shown.
3. Re-open the sheet, switch to "Assign to Team", select a team, and save. Confirm the user is detached and the team is set.
4. Create a new virtual key with "Assign to User" set. Confirm the key is created and the user association is applied.
5. On an OSS build, confirm "Assign to User" does not appear in the entity type picker.

```sh
cd ui
pnpm i
pnpm build
```

## Screenshots/Recordings

_Add before/after screenshots of the entity assignment section showing the new "Assign to User" option and the user picker._

## Breaking changes

- [x] No

## Related issues

_Link related issues here._

## Security considerations

User assignment is gated behind the enterprise user picker registry, which is never registered in OSS builds. The attach/detach mutations require the same RBAC permissions as other virtual key mutations.

## 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
…aximhq#6041)

## Summary

The encrypted-reasoning fail-soft that strips `encrypted_content` from reasoning items before retrying a rejected request only covered `ResponsesRequest`. Bifrost models `/v1/responses/compact` as a separate top-level request shape (`CompactionRequest` with its own `Input` and `RawRequestBody`), so the strip returned `false` for compaction requests, the retry never fired, and the raw `400 invalid_encrypted_content` was handed straight to the client. Codex surfaced this as "Error running remote compact task" and retried the identical body indefinitely.

## Changes

- Introduced `encryptedReasoningCarriers`, which resolves the `Input` and `RawRequestBody` pointers for whichever of the three Responses-shaped request types (`ResponsesRequest`, `CountTokensRequest`, `CompactionRequest`) is present on a given `BifrostRequest`. This lets a single code path in `stripResponsesEncryptedContent` and `stripRawResponsesEncryptedContent` cover all three shapes without duplicating logic.
- `stripResponsesEncryptedContent` now keys off the resolved pointers rather than a hard check for `ResponsesRequest != nil`, so compaction and token-counting requests are rewritten on the same retry path.
- `stripRawResponsesEncryptedContent` now accepts a `*[]byte` instead of a `*schemas.BifrostResponsesRequest`, removing the coupling to one specific request type.
- Added unit tests covering the compaction strip path for both the structured and raw-body (passthrough) cases, and for the drop-on-empty-summary behaviour on compaction input.
- Added two provider-harness cases under entry 45 that pin the end-to-end behaviour: one where the stripped reasoning item survives (it has a summary), and one where it is dropped entirely (empty summary) rather than forwarded as a bare id the upstream never issued.
- Updated `AGENTS.md` to make explicit that any wire-visible change under `core/` must ship with a provider-harness case, not only bug fixes, and documents the structural validation workflow and the narrow exemptions.

## Type of change

- [x] Bug fix

## Affected areas

- [x] Core (Go)

## How to test

```sh
# Unit tests — fast red/green loop
go test ./core/... -run TestStripResponsesEncryptedContent

# Structural harness validation — no live API key needed
node tests/e2e/api/runners/augment-provider-harness.mjs \
  --source tests/e2e/api/collections/provider-harness.json \
  --out tmp/harness-augmented.json

node tests/e2e/api/runners/filter-collection.mjs \
  --source tmp/harness-augmented.json \
  --out tmp/filtered.json \
  --feature "Encrypted Reasoning Fail-Soft on Compaction"
```

The two new harness cases (`openai/gpt-5-mini /openai/v1/responses/compact unverifiable encrypted reasoning degrades` and the summary-less drop variant) should pass structurally. Against a live endpoint they verify that the compaction response returns `object: "response.compaction"` with a non-empty `output` array and no `invalid_encrypted_content` in the body.

## Screenshots/Recordings

N/A

## Breaking changes

- [x] No

## Related issues

N/A

## Security considerations

No auth, secrets, or PII implications. The strip removes `encrypted_content` blobs that the upstream has already refused; no key material is logged or forwarded.

## Checklist

- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
…maximhq#6046)

* fix: preserve minimal reasoning effort for GPT-5-family OpenAI models

normalizeOpenAIReasoningEffort unconditionally downgraded
reasoning.effort "minimal" to "low" for every OpenAI/Azure model,
even GPT-5-family models that natively support "minimal". This
mirrors the same missing-capability-check bug already fixed for
xhigh (maximhq#3122) and DeepSeek max (maximhq#4320).

Fixes maximhq#6044

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Signed-off-by: jitokim <pigberger70@gmail.com>

* test: cover o1/o4/gpt-oss unsupported-model fallback for minimal reasoning effort

Extend the reasoning-effort normalization tests so every affected
site verifies that non-GPT-5 reasoning models (o1, o4, gpt-oss),
not just o3, still downgrade "minimal" to "low". Mirrors the
existing o3 fallback case across the chat, responses, and marshal
test tables.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: jitokim <pigberger70@gmail.com>

* fix: narrow minimal reasoning effort support to bare gpt-5 only

OpenAI's official per-model docs (developers.openai.com/api/docs/guides/
latest-model) confirm "minimal" is only accepted by the original "gpt-5"
model. gpt-5.1, gpt-5.2, gpt-5.3-codex, gpt-5.4, gpt-5.5, and the gpt-5.6
family all dropped "minimal" in favor of none/xhigh/max. Narrow the
prefix match to an exact match on "gpt-5" to avoid sending newer
dot-revisions a reasoning_effort value they don't accept (which the
OpenAI API rejects with a 400).

Signed-off-by: jitokim <pigberger70@gmail.com>

* fix: extend minimal reasoning effort support to gpt-5-mini and gpt-5-nano

OpenAI's official docs group gpt-5, gpt-5-mini, and gpt-5-nano as "the
GPT-5 family" sharing the same reasoning.effort enum (minimal, low,
medium, high) — consistent with how every later dot-revision's docs
page confirms its mini/nano/pro variants share one enum with the base
model. Widen supportsOpenAIMinimalReasoningEffort from an exact match
on "gpt-5" alone to the full three-model family.

Signed-off-by: jitokim <pigberger70@gmail.com>

---------

Signed-off-by: jitokim <pigberger70@gmail.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
OpenAI Responses image_generation_call.action is a JSON string such as
"generate", but ResponsesToolMessageActionStruct modeled every action as
an object union with a type discriminator. Valid response.output_item.*
and response.completed frames failed to decode and were silently skipped
by the streaming handler, so clients never received a terminal event.

- Accept string-valued actions in the response-side union and reject
  null and other non-string non-object tokens with a clear error.
- Retain the optional image-generation tool action on requests.
- Preserve the action through Framework streaming accumulator copies
  while staying compatible with the Framework module's pinned Core.
@citrocat
citrocat force-pushed the thoxvi/fix-responses-image-generation-action branch from 195f35a to 8a9033e Compare August 11, 2026 06:46
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 11, 2026
@citrocat

Copy link
Copy Markdown
Contributor Author

@TejasGhatte @akshaydeo — gentle follow-up on this one, no rush.

A quick data point that may help with prioritization: we have been running this exact patch in our production deployment for 6 days now (cherry-picked onto the v1.6.6 baseline we run). Results so far:

  • Before the patch: the Failed to parse stream response ... Mismatch type ... value "generate" decode error occurred daily (2–16 occurrences/day over the preceding two weeks), each one dropping the terminal response.completed frame for an image-generation stream.
  • After the patch: zero occurrences over the full 6 days, with normal image-generation traffic continuing throughout. No regressions observed on any other Responses traffic.

The PR itself is in a quiet state: single commit, rebased cleanly on current dev after last week's branch rework, CodeRabbit approved (its earlier null-token concern is addressed with a guard + edge-case tests), and all visible checks green. The remaining steps — human review and the approval-gated workflows — are on the maintainer side.

Happy to adjust anything if the review turns up concerns. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Responses streaming drops image generation calls with string actions