Skip to content

fix: prevent request ID collisions and preserve session log grouping - #5348

Open
StrangeXin wants to merge 46 commits into
maximhq:devfrom
StrangeXin:codex/workbuddy-log-correlation
Open

StrangeXin wants to merge 46 commits into
maximhq:devfrom
StrangeXin:codex/workbuddy-log-correlation

Conversation

@StrangeXin

Copy link
Copy Markdown

What changed

  • Normalize missing or all-zero x-request-id values before tracing and context conversion.
  • Propagate the normalized request ID to downstream middleware and response headers.
  • Populate BifrostContextKeyParentRequestID from session correlation headers using this precedence:
    1. W3C baggage member session-id
    2. Bifrost x-bf-session-id
    3. Compatibility header x-conversation-id
  • Add regression coverage for request ID normalization and session-header precedence.

Why

Some OpenAI-compatible clients emit 00000000000000000000000000000000 as a placeholder x-request-id. Bifrost currently treats it as a valid ID, so unrelated requests can collide in the logging store and appear missing or overwritten.

Separately, Bifrost's logging plugin groups requests through BifrostContextKeyParentRequestID, while x-bf-session-id previously populated only the key-stickiness context value. Clients that provide a stable conversation/session header therefore produce complete individual records but no session grouping in LLM Logs.

Impact

  • Placeholder request IDs no longer collapse unrelated log records.
  • Bifrost's native session header now also drives LLM log grouping.
  • Clients using x-conversation-id retain conversation grouping without a separate proxy or transport plugin.
  • Existing valid caller-supplied request IDs remain unchanged.

Validation

  • go test ./bifrost-http/handlers
  • go test ./bifrost-http/handlers -run TestTracingMiddleware_SetsCorrelationHeaders -count=1 -timeout=90s
  • go test ./bifrost-http/lib -run 'TestNormalizeRequestID|TestConvertToBifrostContext_(BaggageSessionIDSetsGrouping|EmptyBaggageSessionIDIgnored|SessionHeaderPrecedence|ReplacesAllZeroRequestID)' -count=1 -timeout=90s

The full bifrost-http/lib package run was stopped after its unrelated integration-style tests continued for several minutes; all directly affected tests passed.

@CLAassistant

CLAassistant commented Jul 18, 2026 •

Copy link
Copy Markdown

CLA assistant check
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.
5 out of 7 committers have signed the CLA.

✅ akshaydeo
✅ roroghost17
✅ Madhuvod
✅ StrangeXin
✅ devonpmack
❌ TejasGhatte
❌ Pratham-Mishra04
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Jul 18, 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: 676c69d5-c262-4aac-a676-44ad7a876595

📥 Commits

Reviewing files that changed from the base of the PR and between cda044b and ec2ffcb.

📒 Files selected for processing (2)
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/lib/ctx_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/lib/ctx_test.go

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved request ID handling by normalizing whitespace-only and all-zero values to unique, valid identifiers.
    • Ensured normalized request IDs are consistently propagated via correlation headers in both request handling and responses.
    • Improved parent-request/session identification when multiple session-related headers are present, using a clear precedence order.
  • Tests

    • Added unit tests covering request ID normalization (including all-zero replacement).
    • Added tests validating session/parent-request precedence and downstream propagation behavior.

Walkthrough

Request ID normalization is centralized and applied by tracing middleware and Bifrost context conversion. Parent request ID selection now follows explicit session, baggage, and conversation-header precedence, with tests covering sentinel replacement and propagation.

Changes

Request ID and context handling

Layer / File(s) Summary
Request ID normalization utility
transports/bifrost-http/lib/requestid.go, transports/bifrost-http/lib/requestid_test.go
Adds NormalizeRequestID and tests preservation of valid IDs plus replacement of blank, whitespace-only, and all-zero values.
Bifrost context request and parent IDs
transports/bifrost-http/lib/ctx.go, transports/bifrost-http/lib/ctx_test.go
Normalizes context request IDs and applies parent request ID precedence across x-bf-session-id, baggage, and x-conversation-id, with corresponding tests.
Tracing middleware propagation
transports/bifrost-http/handlers/middlewares.go, transports/bifrost-http/handlers/middlewares_test.go
Normalizes incoming request IDs and propagates normalized values downstream and in responses, including all-zero inputs.

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

Suggested reviewers: pratham-mishra04, akshaydeo, bearts

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% 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 matches the main change: fixing request ID collisions and preserving session log grouping.
Description check ✅ Passed The description covers the summary, rationale, impact, and validation, but several template sections are omitted.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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

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

@StrangeXin
StrangeXin marked this pull request as ready for review July 18, 2026 14:25
@greptile-apps

greptile-apps Bot commented Jul 18, 2026 •

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
transports/bifrost-http/handlers/middlewares.go Normalizes request IDs before tracing and propagates the result through request and response headers.
transports/bifrost-http/handlers/middlewares_test.go Adds middleware coverage for replacing an all-zero request ID.
transports/bifrost-http/lib/ctx.go Normalizes context request IDs and adds session-header precedence for log grouping.
transports/bifrost-http/lib/ctx_test.go Adds coverage for session grouping precedence and all-zero request IDs.
transports/bifrost-http/lib/requestid.go Introduces the shared request-ID normalization helper.
transports/bifrost-http/lib/requestid_test.go Covers valid, empty, whitespace-only, and all-zero request IDs.

Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/dev..." | Re-trigger Greptile

@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 `@transports/bifrost-http/lib/ctx.go`:
- Around line 240-253: Update parentRequestID selection in
transports/bifrost-http/lib/ctx.go:240-253 to evaluate x-bf-session-id first,
then fall back to baggage and finally x-conversation-id, while preserving
trimming and length validation. Update the related assertions in
transports/bifrost-http/lib/ctx_test.go:300-328 to verify x-bf-session-id takes
precedence over baggage.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4f9d7813-179f-458d-8855-c93d76115173

📥 Commits

Reviewing files that changed from the base of the PR and between 493bff0 and cda044b.

📒 Files selected for processing (6)
  • transports/bifrost-http/handlers/middlewares.go
  • transports/bifrost-http/handlers/middlewares_test.go
  • transports/bifrost-http/lib/ctx.go
  • transports/bifrost-http/lib/ctx_test.go
  • transports/bifrost-http/lib/requestid.go
  • transports/bifrost-http/lib/requestid_test.go

Comment thread transports/bifrost-http/lib/ctx.go
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 18, 2026
akshaydeo and others added 18 commits July 21, 2026 22:27
…ebhook docs and OpenAPI spec (maximhq#5429)

## Summary

Clarifies that webhook delivery for async jobs is opt-in per request, not automatic. Previously, the docs implied that registering an endpoint was sufficient for delivery to occur. This PR corrects that by documenting the `x-bf-async-webhook` header as the explicit trigger, and refines the behavior around subscription validation timing.

## Changes

- Updated the async inference tip and webhook overview to state that the endpoint must be named via `x-bf-async-webhook` on the submit request for delivery to occur.
- Added a new "Webhook Notifications" section to `async-inference.mdx` detailing opt-in behavior, validation rules, and header scope.
- Added a new "Triggering a Delivery" section to `webhooks.mdx` with a curl example and clarifying bullet points.
- Corrected the OpenAPI description for `x-bf-async-webhook` to reflect that subscription validation happens at job completion time, not at submission — meaning a missing subscription no longer causes the submit to be rejected, but silently skips delivery instead.

## 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 to confirm:

1. The async inference page includes the "Webhook Notifications" section with accurate opt-in behavior.
2. The webhooks page includes the "Triggering a Delivery" section with a working curl example.
3. The OpenAPI spec correctly reflects that subscription absence at job completion skips delivery rather than rejecting the submit.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None. No changes to auth, secrets, or delivery signing behavior.

## 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
…ent-level overrides (maximhq#5455)

## Summary

Replaces the binary on/off switches for deployment-level boolean overrides (Replicate's "use deployments endpoint" and the "use Anthropic endpoints" toggle for SGLang, Deepseek, Fireworks, and vLLM) with a three-way select control. Previously, a plain switch could not distinguish between "explicitly off" and "inherit from the key-level setting," meaning turning the switch off was indistinguishable from leaving it unset. The new `TriStateOverrideRow` component expresses three states: `undefined` (inherit the key's setting), `true` (explicitly on), and `false` (explicitly off).

## Changes

- Added a `TriStateOverrideRow` component that renders a select with "Use key setting", "On", and "Off" options, mapping to `undefined`, `true`, and `false` respectively.
- Replaced the `Switch`-based inline rows in `ReplicateSection` and `UseAnthropicEndpointsToggleSection` with `TriStateOverrideRow`, preserving the `onChange` contract but now passing the value through directly rather than coercing `false` to `undefined`.
- Fixed inconsistent indentation (spaces vs. tabs) in `apiKeysFormFragment.tsx` and `deploymentsTable.tsx`.
- Reformatted a few long JSX attribute lists and inline strings for readability.

## 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. Open a provider that supports deployment-level overrides (e.g., Replicate, SGLang, Deepseek, Fireworks, vLLM).
2. Add or edit a deployment and locate the relevant override row.
3. Verify the control renders as a three-option select ("Use key setting", "On", "Off") rather than a toggle switch.
4. Set the value to "Off" and save. Confirm the deployment stores an explicit `false` rather than `undefined`.
5. Set the value to "Use key setting" and save. Confirm the field is stored as `undefined`/absent.

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

## Screenshots/Recordings

Before: A binary switch that could not express "explicitly off" — toggling it off was equivalent to leaving it unset.

After: A three-way select with "Use key setting" / "On" / "Off", allowing deployments to explicitly disable a toggle that is enabled at the key level.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

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

Briefly explain the purpose of this PR and the problem it solves.

## Changes

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

## Type of change

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

## Affected areas

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

## How to test

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

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

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

## Screenshots/Recordings

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

## Breaking changes

- [ ] Yes
- [ ] No

If yes, describe impact and migration instructions.

## Related issues

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

## Security considerations

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

## 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
…er background colors (maximhq#5451)

## Summary

Fixes two issues with the trial expiry banner: the background colours were using Tailwind opacity-modifier classes that didn't render correctly, and the trial expiry date parser rejected RFC3339 timestamps (e.g. `2024-06-01T00:00:00Z`) injected by the Docker build, causing the banner to silently disappear.

## Changes

- Replaced `bg-red-500/10` and `bg-amber-500/10` with explicit hex values (`#ffebea` and `#fff4e4`) to ensure the banner background renders as intended in both expired/critical and warning states.
- Updated `parseTrialExpiry` to accept both bare `YYYY-MM-DD` dates and full RFC3339 timestamps. Only the calendar date portion is used (interpreted at local midnight), so the time component is stripped before parsing.

## Type of change

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

## Affected areas

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

## How to test

1. Set `TRIAL_EXPIRY_DATE` to an RFC3339 value such as `2024-06-01T00:00:00Z` and confirm the banner appears with the correct background colour.
2. Set it to a bare date such as `2024-06-01` and confirm the banner still renders correctly.
3. Set it to a date within the warning window and confirm the amber (`#fff4e4`) background is shown.
4. Set it to an expired or critical date and confirm the red (`#ffebea`) background is shown.

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

## Screenshots/Recordings

Before: banner background was invisible due to unresolved Tailwind opacity-modifier classes.  
After: banner displays the correct solid tinted background in both warning and expired/critical states.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

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

`ToReplicateImageGenerationInput` previously ignored `InputImages` entirely and could not surface validation errors to callers. This PR adds input image support to the image generation path (mirroring what already existed for image edits) and propagates URL sanitization errors instead of silently dropping them.

## Changes

- Changed `ToReplicateImageGenerationInput` to return `(*ReplicatePredictionRequest, error)` so URL validation errors can be surfaced to callers.
- Added `InputImages` handling in the generation path: each image URL is sanitized via `schemas.SanitizeImageURL`, and the resulting slice is routed to the correct model-specific field using the new shared helper.
- Extracted the model-to-field dispatch logic (`image_prompt`, `input_image`, `image`, `input_images`) into a `setInputImageField` helper, eliminating the duplicated switch block that previously existed only in the edit path.
- Updated both `ImageGeneration` and `ImageGenerationStream` call sites to handle the new error return.
- Removed stale line-number references from the Replicate provider docs.

## Type of change

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

## Affected areas

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

## How to test

```sh
go test ./core/providers/replicate/...
```

New test cases cover:
- `InputImages_SingleImageField` — verifies that a kontext-pro model receives the first image in `input_image` and no other image fields are set.
- `InputImages_ArrayFieldWithBase64Normalization` — verifies that a generic model receives all images in `input_images` and that bare base64 strings are prefixed with the `data:image/png;base64,` URI scheme.
- `InputImages_InvalidURL` — verifies that a `file://` URI causes an error return and a `nil` result.

## Breaking changes

- [x] Yes
- [ ] No

`ToReplicateImageGenerationInput` now returns `(*ReplicatePredictionRequest, error)` instead of `*ReplicatePredictionRequest`. Any external callers must be updated to handle the additional return value.

## Related issues

## Security considerations

Input images are now validated through `schemas.SanitizeImageURL` before being forwarded to Replicate. This prevents schemes such as `file://` from being passed through to the provider.

## Checklist

- [ ] 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)
- [ ] I verified the CI pipeline passes locally if applicable
## Summary

Integer constraint fields in the Gemini `Schema` type were tagged with `,string` in their JSON struct tags, causing them to serialize as quoted strings (e.g., `"1"`) rather than JSON numbers (e.g., `1`). This broke strict JSON Schema validation upstream when these constraints were passed through `parametersJsonSchema` (issue maximhq#5433).

## Changes

- Removed the `,string` option from the JSON struct tags for `MinItems`, `MaxItems`, `MinLength`, `MaxLength`, `MinProperties`, and `MaxProperties` on the `Schema` type, so these fields now marshal as JSON numbers instead of quoted strings.
- Added a round-trip test (`TestGenAIToolSchemaConstraintsRoundTripAsNumbers`) that verifies integer constraints survive the genai → Bifrost → Gemini conversion as proper JSON numbers, covering both numeric and quoted input forms.

## Type of change

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

## Affected areas

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

## How to test

```sh
go test ./core/providers/gemini/... -run TestGenAIToolSchemaConstraintsRoundTripAsNumbers -v
go test ./core/providers/gemini/...
```

The new test sends a Gemini generation request with integer constraints specified both as raw numbers and as quoted strings, then asserts that after the full round-trip the output constraints are `float64` JSON numbers (not strings).

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

Closes maximhq#5433

## Security considerations

None.

## 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
… header forwarding (maximhq#5476)

## Summary

Documents how to inject dynamic, server-side headers into outgoing MCP requests from a `PreMCPHook` plugin, covering use cases that static configuration cannot address (e.g., per-user identity headers, short-lived service tokens, per-request correlation IDs).

## Changes

- Added a tip to the header forwarding section in `connecting-to-servers.mdx` clarifying that forwarded headers come from the caller and are untrusted, and pointing readers to the new plugin recipe for server-side injection.
- Added a new "Recipe: injecting dynamic headers server-side" section to `writing-go-plugin.mdx` (v1.5.x+) with a full `PreMCPHook` code example that merges plugin-injected headers into `BifrostContextKeyMCPExtraHeaders`, along with notes on the per-client allowlist, transport compatibility (HTTP/SSE only), and why Connect hooks are the wrong place for per-request identity.

## Type of change

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

## Affected areas

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

## How to test

Review the rendered documentation to confirm:
- The tip in `connecting-to-servers.mdx` links correctly to the new recipe anchor in `writing-go-plugin.mdx`.
- The code example in the recipe compiles without errors when dropped into a plugin project.
- The `<Note>` and key-points list render correctly in the docs site.

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

The added documentation explicitly calls out that caller-forwarded headers must be treated as untrusted input by upstream servers, and that plugin-injected identity headers (e.g., signed-in user email) are the appropriate mechanism when the caller must not control the value. The per-client `allowed_extra_headers` allowlist is noted as the enforcement boundary for both forwarded and plugin-injected headers.

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

Introduces a per-request upstream latency accumulator that tracks cumulative time Bifrost spends blocked on provider sockets across every attempt, retry, fallback, MCP tool call, and media fetch. Subtracting this from total wall time gives Bifrost's own processing overhead — a number that was previously impossible to derive accurately.

## Changes

- **New `upstreamlatency.go` schema**: Installs an `*atomic.Int64` accumulator on the `BifrostContext` once per request. Uses an atomic pointer so streaming goroutines can keep writing after the request handler returns without touching the context's value map.
- **`ResetUpstreamLatency` / `AddUpstreamLatency` / `GetUpstreamLatency`**: Core API for the accumulator. `Reset` is mandatory at request entry because Bifrost reuses a single process-global context for nil-ctx SDK callers; without it the counter would grow unboundedly.
- **`DoStreamingRequest` / `DoHTTPRequest` helpers**: Thin wrappers around `fasthttp.Client.Do` and `net/http.Client.Do` that record the call duration as upstream latency. All provider call sites are migrated to these helpers.
- **`idleTimeoutReader.Read` instrumentation**: Each blocking read in a streaming response is counted as upstream time, covering the token-generation window that `DoStreamingRequest` (which returns at first byte) cannot see.
- **MCP tool call instrumentation**: `executeToolInternal` wraps `CallTool` with the same accumulator, since waiting on an MCP server is upstream time, not Bifrost overhead.
- **`FetchAndEncodeURL` instrumentation**: Remote media fetches are counted as upstream, preventing multi-second fetches from appearing as Bifrost overhead.
- **`StampUpstreamLatency` / `PopulateUpstreamLatency`**: Write the accumulated total onto the root trace span (`bifrost.upstream.duration_ms`) and onto `BifrostResponseExtraFields.UpstreamLatency` respectively. Both are called via a named-return `defer` in `handleRequest` so they fire even on error paths.
- **`Trace.StampOverheadDuration`**: Computes `bifrost.overhead.duration_ms = root_span_duration - upstream_total` on the export snapshot, after the root span has ended. Clamped at zero to absorb clock skew.
- **OTel plugin**: Reads `AttrBifrostOverheadDurationMs` from the root span and records it as a new `bifrost_overhead_latency_seconds` histogram with fine-grained sub-millisecond buckets appropriate for processing overhead rather than network latency.
- **Prometheus plugin**: Records the same overhead histogram via the `HTTPTransportPreHook`/`HTTPTransportPostHook` window (widest available, matching the OTel root span). Falls back to the `PostLLMHook` window for SDK callers that bypass the transport layer.
- **HTTP transport**: Emits `x-bifrost-upstream-latency-ms` response header so proxy callers can derive overhead from their own elapsed time without parsing the response body.
- **New trace attributes**: `bifrost.upstream.duration_ms` and `bifrost.overhead.duration_ms` added to the attribute constant set.

## Type of change

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

## Affected areas

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

## How to test

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

- Make a request through the HTTP transport and verify the `x-bifrost-upstream-latency-ms` response header is present and less than the total elapsed time.
- Make a streaming request and confirm the header value grows to reflect the full generation window, not just time-to-first-byte.
- Make a request that triggers a fallback and confirm the upstream latency reflects the sum of both attempts.
- In OTel/Prometheus dashboards, verify `bifrost_overhead_latency_seconds` appears and that its values are in the sub-millisecond to low-tens-of-milliseconds range for healthy requests.
- Confirm `bifrost.upstream.duration_ms` and `bifrost.overhead.duration_ms` appear on root spans in exported traces.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None. The upstream latency value is derived from internal timing and contains no secrets or PII. The new response header exposes only a duration in milliseconds.

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

Fixes a bug where plain-text `SecretVar` objects (e.g. `{"value": "..."}` with no `ref`/`type` fields) were not being recognised as `SecretVar`-shaped during redaction restoration. This caused the UI to persist masked values instead of restoring the real stored secrets when saving Kafka SASL credentials or similar connectors that store secrets as plain strings but return them as value-only objects after a redacted GET.

## Changes

- `isSecretVarObject` previously required either `ref`+`type` or `env_var`+`from_env` alongside `value`, which excluded plain-text `SecretVar`s that marshal as `{"value": "..."}` alone (since `ref`/`type` are `omitempty`). The function now accepts any map whose keys are exclusively drawn from the known `SecretVar` field set (`value`, `ref`, `type`, `env_var`, `from_env`), with `value` required to be a string.
- This ensures that value-only objects round-tripped by the UI after a redacted GET are correctly identified and restored from the existing stored value, rather than being passed through with the masked content.
- Objects with a non-redacted value (e.g. username shown in clear) pass through unchanged, and intentional updates (new password, env reference) are not clobbered.
- Tests added for the Kafka SASL credential shape, the `FullyRedacted()` sentinel (`<REDACTED>`), and intentional secret rotation/env-ref switching.

## Type of change

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

## Affected areas

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

## How to test

```sh
go test ./transports/bifrost-http/handlers/...
```

The new tests cover:
- Kafka SASL `password` and `ca_cert` restored from stored plain strings when the UI sends back value-only masked objects.
- `FullyRedacted()` sentinel (`<REDACTED>`) correctly triggers restoration.
- Rotated passwords and env-ref switches pass through without being overwritten by the stored value.

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

This change affects how redacted secret values are handled during plugin configuration updates. The fix ensures masked values are never persisted in place of real secrets, and that intentional secret rotations or env-ref changes are not silently discarded. No new secret exposure surface is introduced.

## 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 a server-configured `batch_role_arn` field to the Bedrock key configuration, allowing operators to pin the IAM service role used for Bedrock batch jobs at the server level rather than relying on clients to supply it via `role_arn` in request extra params. When set, the server-side value takes priority over any client-provided `role_arn`.

## Changes

- Added `BatchRoleARN *SecretVar` to `BedrockKeyConfig` in `schemas/account.go`, stored under the JSON key `batch_role_arn` and kept separate from the STS AssumeRole identity (`bedrock_role_arn`).
- Updated `BatchCreate` in `bedrock.go` so that `key.BedrockKeyConfig.BatchRoleARN` is resolved first; the client-supplied `role_arn` in `ExtraParams` is only used as a fallback when the server value is absent.
- Added a database migration (`add_bedrock_batch_role_arn_column`) that adds the `bedrock_batch_role_arn` column to `config_keys`, with rollback support.
- Wired `BedrockBatchRoleARN` through all RDB read/write paths (`tableKeyFromSchemaKey`, `UpdateProvidersConfig`, `UpdateProvider`, `AddProvider`) and through `BeforeSave`/`AfterFind` hooks including encryption and decryption.
- Updated the `AfterFind` Bedrock config reconstruction condition to include `BedrockBatchRoleARN`.
- Added `BatchRoleARN` to the `mergeUpdatedKey` preserve logic in the HTTP handler so partial updates do not accidentally clear the field.
- Added `batch_role_arn` to the config JSON schema with a description noting its priority semantics and `env.` prefix support.
- Added `batch_role_arn` to the Zod schemas (`providerForm.ts`, `schemas.ts`) and rendered a **Batch Role ARN** input field in the UI form, visible only when the provider supports the batch API.
- Added redaction support for `BatchRoleARN` in `clientconfig.go`.

## Type of change

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

## Affected areas

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

## How to test

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

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

**Manual validation:**

1. Configure a Bedrock provider key with `batch_role_arn` set (either as a literal ARN or via `env.AWS_BATCH_ROLE_ARN`).
2. Submit a batch create request that also includes `role_arn` in `extra_params`.
3. Confirm that the server-configured `batch_role_arn` is used and the client-supplied value is ignored.
4. Remove `batch_role_arn` from the key config and resubmit; confirm the client-supplied `role_arn` is now used.
5. Verify the value is stored encrypted in the database and appears redacted in API responses.

**New config field:**

| Field | JSON key | Description |
|---|---|---|
| `BatchRoleARN` | `batch_role_arn` | Service role ARN Bedrock assumes for batch S3 access. Supports `env.` prefix. Takes priority over client-supplied `role_arn`. |

## Breaking changes

- [ ] Yes
- [x] No

## Security considerations

`BatchRoleARN` is treated as a secret: it is encrypted at rest via the existing `encryptSecretVarPtr`/`decryptSecretVarPtr` pipeline and redacted in API responses, consistent with other credential fields such as `RoleARN` and `ExternalID`.

## 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 additive override state to governance budgets, allowing a temporary or permanent spending limit increase to be layered on top of a budget's base `MaxLimit`. Overrides can be scoped to a finite number of reset cycles (`cycles` mode) or kept active indefinitely (`forever` mode).

## Changes

- Introduced `BudgetOverrideMode` type with `cycles` and `forever` constants in `tables/budget.go`.
- Added three new columns to `TableBudget`: `override_amount`, `override_mode`, and `override_cycles_remaining`.
- Added `validateOverride()` to enforce unambiguous override state (e.g., `cycles` mode requires both a positive amount and a positive cycle count; `forever` mode requires a positive amount and zero cycle count; non-finite amounts are rejected).
- Wired `validateOverride()` into the existing `BeforeSave` GORM hook so invalid override combinations are rejected at persistence time.
- Added the `add_budget_override_columns` database migration to introduce the three columns with safe defaults, including rollback support.

## Type of change

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

## Affected areas

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

## How to test

```sh
go test ./framework/configstore/...
go test ./framework/configstore/tables/...
```

- `TestMigrationAddBudgetOverrideColumns` verifies that legacy budget rows receive zero-value defaults after migration and that re-running the migration is idempotent.
- `TestCreateBudgetWithOverride` verifies that both `cycles` and `forever` override states round-trip correctly through the config store.
- `TestTableBudgetValidateOverride` covers all valid and invalid override field combinations, including NaN/infinite amounts, missing modes, and conflicting cycle counts.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None. Override amounts are validated to be finite and positive before persistence, preventing malformed data from reaching the budget enforcement layer.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] 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
* fix domains

* docs update

* [fix]: preserve tool_search_call argument wire shape

* [fix]: preserve tool_search execution flag and tool_search_output tools

Two further Responses-API wire-shape losses broke the codex tool_search
round-trip through bifrost:

- `execution: "client"` was dropped from tool_search_call items, so codex
  never recognized the call as client-dispatchable and silently ended the
  turn. Add Execution to ResponsesToolMessage.
- `tool_search_output` discovered tools are ResponsesTool-shaped
  (namespace/function, discriminated by `type`), not mcp_list_tools-shaped.
  The embedded ResponsesMCPListTools decode dropped `type` and nesting,
  causing OpenAI to 400 with "Missing required parameter input[].tools[].type".
  Round-trip the tools array verbatim as raw JSON.

Adds regression tests for both. Verified end-to-end with codex-acp against
a local bifrost build: the full tool_search -> discover -> call -> result
loop now completes instead of silently ending the turn.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore: trim verbose comments on tool_search wire-shape handling

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: preserve tool_search metadata in response copies

Keep tool_search_output raw tools, namespace, and execution metadata intact across the response deep-copy paths used by schema helpers and streaming accumulation. Also preserve object-shaped arguments on tool_search_output serialization and document the raw tools invariant for programmatic callers.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs: explain response copy compatibility shim

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Signed-off-by: Akshay Deo <akshay@akshaydeo.com>
Co-authored-by: akshaydeo <akshay@akshaydeo.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary

Adds a dedicated `UpdateBudgetOverride` operation that atomically replaces or clears only the override fields on a budget, ensuring concurrent usage changes and base configuration are never clobbered. Exposes this via two new HTTP endpoints (`PUT` and `DELETE`) scoped to virtual-key budgets.

## Changes

- Added `UpdateBudgetOverride` to `RDBConfigStore` and the `ConfigStore` interface. The method locks the row, validates the new override state via `SetOverride`, and updates only `override_amount`, `override_mode`, `override_cycles_remaining`, and `updated_at`, leaving `current_usage`, `max_limit`, and `reset_duration` untouched.
- Added `HasActiveOverride`, `EffectiveMaxLimit`, `SetOverride`, and `ClearOverride` helpers to `TableBudget`. `SetOverride` rolls back to the previous state if validation fails, making it non-mutating on error.
- Registered `PUT /api/governance/virtual-keys/{vk_id}/budgets/{budget_id}/override` and `DELETE /api/governance/virtual-keys/{vk_id}/budgets/{budget_id}/override` in the governance HTTP handler. Both routes resolve the budget only through model configs scoped to the virtual key, so AP-managed direct mirror budgets are unreachable (404) through this endpoint.
- After a successful override mutation the handler reloads the virtual key in the governance manager so in-memory state stays consistent.
- Added `BudgetOverrideRequest` and `BudgetOverrideResponse` HTTP types; the response includes the refreshed budget and its computed `effective_max_limit`.

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

## How to test

```sh
go test ./framework/configstore/... ./transports/bifrost-http/handlers/...
```

Key scenarios covered by the new tests:

- `TestUpdateBudgetOverridePreservesBudgetState` — confirms `max_limit`, `reset_duration`, and `current_usage` are unchanged after a partial override update and after clearing.
- `TestTableBudgetOverrideLifecycle` — verifies finite-cycle, forever, and cleared override transitions on the model directly.
- `TestTableBudgetSetOverrideRestoresPreviousState` — confirms an invalid `SetOverride` call leaves the budget unchanged.
- `TestVirtualKeyBudgetOverrideLifecycle` — end-to-end HTTP test covering finite override, replacement, clear, and virtual key reload (expects 3 reload calls).
- `TestVirtualKeyBudgetOverrideRejectsDirectMirrorBudget` — confirms AP-managed budgets attached directly to a virtual key return 404 and are not mutated.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

The override endpoints resolve budgets exclusively through model configs owned by the target virtual key. Budgets attached directly via `virtual_key_id` (AP mirror budgets) are not reachable, preventing unintended cross-scope mutations.

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

Budget overrides with a finite cycle count were not being decremented or expired as budget reset cycles completed. This meant a temporary spending override would remain active indefinitely rather than expiring after the configured number of resets. This PR wires the override lifecycle into the reset path so finite overrides count down and clear themselves automatically.

## Changes

- Added `ConsumeOverrideCycle()` to `TableBudget`, which decrements `OverrideCyclesRemaining` by one per reset cycle and calls `ClearOverride()` when the last cycle is consumed. Permanent (`BudgetOverrideModeForever`) overrides are left untouched.
- Called `ConsumeOverrideCycle()` in both the normal `ResetBudgetAt` path and the inline reset fallback inside `BumpBudgetUsage`, ensuring the cycle counter advances regardless of which reset code path fires.
- Updated `ResetExpiredBudgets` to persist the three override fields (`override_amount`, `override_mode`, `override_cycles_remaining`) alongside `current_usage` and `last_reset` so the decremented state is durably written to the database.
- Replaced direct `budget.MaxLimit` reads in `CheckBudget` and `GetBudgetAndRateLimitStatus` with `budget.EffectiveMaxLimit()` so enforcement and status reporting both reflect the active additive override rather than the bare base limit.
- The concurrent reset test was extended to assert that exactly one override cycle is consumed when multiple goroutines race to reset the same budget.

## Type of change

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

## Affected areas

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

## How to test

```sh
go test ./framework/configstore/tables/... -run TestTableBudgetConsumeOverrideCycle -v
go test ./plugins/governance/... -run TestGovernanceStoreCheckBudgetUsesOverride -v
go test ./plugins/governance/... -run TestGovernanceStoreResetBudgetAdvancesOverride -v
go test ./plugins/governance/... -run TestGovernanceStoreResetPersistsOverrideLifecycle -v
go test ./plugins/governance/... -run TestResetBudgetAt_ConcurrentResettersCollapse -v
go test ./plugins/governance/... -v
```

Expected: all tests pass; finite overrides expire after the configured number of reset cycles, permanent overrides survive resets, and the decremented cycle count is persisted to the database.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None. Override amounts are operator-configured values already validated on write; no new inputs or privilege boundaries are introduced.

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

Adds support for additive budget overrides on virtual key budgets, allowing operators to temporarily increase a virtual key's spending capacity beyond its base limit without modifying the base budget itself.

## Changes

- Added `override_amount`, `override_mode`, and `override_cycles_remaining` fields to the `Budget` type, along with `BudgetOverrideRequest` and `BudgetOverrideResponse` types.
- Introduced three utility functions in `governance.ts`: `hasActiveBudgetOverride`, `getEffectiveBudgetLimit`, and `validateBudgetOverride`. These handle override state detection, effective limit calculation, and input validation respectively.
- Replaced all direct references to `b.max_limit` in exhaustion checks, progress bars, and CSV exports with `getEffectiveBudgetLimit(b)` so overrides are reflected everywhere budgets are displayed or evaluated.
- Added `setVirtualKeyBudgetOverride` (PUT) and `removeVirtualKeyBudgetOverride` (DELETE) RTK Query mutations to `governanceApi`, targeting `/governance/virtual-keys/:vkId/budgets/:budgetId/override`.
- Created `BudgetOverrideDialog` component that lets operators add, edit, or remove an override on a single budget. The dialog supports two modes: a fixed number of reset cycles or indefinite ("until removed"). It is surfaced in the virtual key detail sheet for non-managed keys.
- The `BudgetDisplay` tooltip and inline label now show an "override" badge and a breakdown of base + override amounts when an active override is present.
- Added unit tests covering effective limit calculation, expired/incomplete override detection, and input validation.

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

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

1. Open a virtual key detail sheet for a key that is not managed by an access profile and has at least one budget with a persisted ID.
2. Click **Add override** next to a budget line.
3. Enter an additional amount, select a mode (cycles or forever), and save. Verify the usage bar and effective limit update immediately.
4. Re-open the dialog and click **Edit override** to confirm existing values are pre-populated.
5. Click **Remove override** and confirm the budget reverts to its base limit.
6. Verify that a key whose usage meets or exceeds the effective limit (base + override) is shown as exhausted.
7. Export virtual keys to CSV and confirm the Budget Limit column reflects the effective limit.

## Screenshots/Recordings

_Add before/after screenshots of the virtual key detail sheet showing the override badge, breakdown text, and dialog._

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

The override mutations respect the existing `RbacResource.VirtualKeys` / `RbacOperation.Update` permission check; the **Add/Edit override** button is disabled for users without that permission.

## 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
roroghost17 and others added 8 commits July 23, 2026 21:34
## Summary

Adds end-to-end observability test coverage for provider error paths — both non-streaming and streaming requests that fail before the first chunk. This pins a regression where spans were exported without `gen_ai.error.*` attributes when a stream failed before delivering any data, and ensures the `bifrost_error_requests_total` Prometheus counter is correctly labeled with `status_code` for both request types.

## Changes

- Introduced an `ERROR_TRIGGER` marker in the mock provider that returns a provider-style 404 error body (`model_not_found`) for any request containing it, covering both streaming and non-streaming failure paths.
- Added `chatError(id, stream)` to fire requests expected to fail with a 404, validating both the non-stream and pre-first-chunk stream error paths.
- Added `assertOtelErrorTrace(id, label)` to verify that exported spans carry `gen_ai.error.type`, `gen_ai.error.code`, and `http.response.status_code` attributes for error requests.
- Added `assertPrometheusErrorScrape()` to confirm `bifrost_error_requests_total{status_code="404"}` is present for both `chat_completion` and `chat_completion_stream` methods.
- Enabled `chat_completion_stream` in the mock provider's `allowed_requests` config so streaming error requests are routed correctly.
- Generalized `assertMockProviderRequest` to accept an expected request count rather than always asserting exactly one.

## Type of change

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

## Affected areas

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

## How to test

Run the local observability E2E test suite. The test will:
1. Fire a non-streaming request with the error trigger and assert the exported span contains `gen_ai.error.*` attributes.
2. Fire a streaming request with the error trigger and assert the same span attributes are present (pinning the deferred-span stamping regression).
3. Scrape `/metrics` and assert `bifrost_error_requests_total{status_code="404"}` is present for both `chat_completion` and `chat_completion_stream`.

```sh
node tests/e2e/api/runners/run-observability-local.mjs
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None. The error trigger is scoped entirely to the mock provider used in tests and does not affect production routing logic.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] 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

Replaces em-dash (`—`) separators used as clause connectors in UI copy with grammatically appropriate alternatives: semicolons, colons, commas, or periods. This improves readability and consistency across tooltips, descriptions, labels, alerts, and inline help text throughout the UI.

## Changes

- Replaced em-dashes used to join independent clauses with semicolons (e.g. "token is not revoked at the provider — it stays detached" → "...provider; it stays detached")
- Replaced em-dashes used to introduce elaborations or examples with colons or commas (e.g. "Large payload request — input content..." → "Large payload request: input content...")
- Replaced em-dashes used in parenthetical asides with parentheses or commas where appropriate
- Changed one em-dash team name separator in a combobox label to a standard hyphen (`-`) since it appears in a data label context rather than prose

## Type of change

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

## Affected areas

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

## How to test

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

Visually inspect affected UI surfaces (logging config, MCP config, routing rules, virtual keys, provider keys, pprof page, log detail view, sessions table, observability fragments) to confirm copy reads correctly with no regressions.

## Screenshots/Recordings

No visual layout changes expected; only text content is affected.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

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

Streaming responses in Bifrost write HTTP response headers before the first SSE chunk arrives, but routing identity (provider, model, key, fallback status) was only available on per-chunk `ExtraFields`. This meant routed-identity `x-bifrost-*` headers were missing entirely from streaming responses. This PR fixes that by stashing a `RoutingInfo` snapshot into the `BifrostContext` at stream setup time, then reading it in the transport layer to emit the correct headers before the first chunk is written.

## Changes

- Added `BifrostContextKeyRoutingInfo` as a new reserved context key. Core writes a `RoutingInfo` snapshot into the context at each stream attempt (overwritten on retry, so the winning attempt's snapshot survives). On successful fallback, the snapshot is updated to reflect `IsFallback`, `PrimaryProvider`, and `PrimaryModel`, mirroring the existing `SetFallbackRoutingInfo` logic.
- Added `RoutingInfo.ToExtraFields(requestType)` to build a `BifrostResponseExtraFields` from a finalized `RoutingInfo`, using the same deprecated-triplet sync rules as the non-streaming response path.
- Added `ApplyBifrostStreamResponseHeaders` in the transport lib, which reads the context snapshot and calls the existing `ApplyBifrostResponseHeaders` before any SSE write. When no snapshot is present (e.g. a plugin short-circuited the stream), only the request-type header is emitted.
- All streaming handler entry points (`handleStreamingTextCompletion`, `handleStreamingChatCompletion`, `handleStreamingResponses`, `handleStreamingSpeech`, `handleStreamingTranscriptionRequest`, `handleStreamingImageGeneration`, `handleStreamingImageEditRequest`) now pass their `RequestType` through to `handleStreamingResponse`, which calls `ApplyBifrostStreamResponseHeaders` after the stream channel is obtained but before any SSE headers are flushed.
- The generic router's `handleStreamingRequest` follows the same pattern, tracking `requestType` per branch and calling `ApplyBifrostStreamResponseHeaders` at the same point.
- Tests cover the normal identity case, the fallback-layered case (including deprecated header derivation), and the missing-snapshot fallback.

## Type of change

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

## Affected areas

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

## How to test

```sh
go test ./core/... ./transports/bifrost-http/...
```

To validate end-to-end: send a streaming chat completion request through the HTTP transport and inspect the response headers. Expect `x-bifrost-routing-info-provider`, `x-bifrost-routing-info-model`, `x-bifrost-routing-info-key`, and `x-bifrost-request-type` to be present on the response before any SSE data is received. For a fallback scenario, also expect `x-bifrost-routing-info-is-fallback: true`, `x-bifrost-routing-info-primary-provider`, and `x-bifrost-routing-info-primary-model`.

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

The `RoutingInfo` snapshot stored in context contains provider key identifiers (alias names, not secret values). This is consistent with what was already emitted on non-streaming responses via `ExtraFields`.

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

Adds streaming retrieval of stored OpenAI Responses API objects via `GET /v1/responses/{id}?stream=true`. Previously, retrieving a stored response only supported a unary (non-streaming) path. This change introduces a parallel SSE streaming path that replays the stored response as a stream of typed events, matching the behaviour of the create-stream path.

## Changes

- Added `ResponsesRetrieveStreamRequest` as a new `RequestType` constant (`"responses_retrieve_stream"`) and registered it across all routing, accumulator, logging, and permission checks that handle stream request types.
- Added a `Stream *bool` field to `BifrostResponsesRetrieveRequest` and an `IsStreamingRequested()` helper method. `buildResponsesRetrieveQuery` now encodes the `stream` query parameter when set.
- Added `ResponsesRetrieveStream` to the `ResponsesLifecycleProvider` interface and implemented it on `OpenAIProvider`. The implementation issues a streaming-aware `GET` with `Accept: text/event-stream`, runs the same SSE loop used by the create-stream path (idle timeout, gzip decompression, cancellation, error event handling, `response.completed`/`response.incomplete` terminal events), and is intentionally kept separate from the create-stream path.
- Added `ResponsesRetrieveStreamRequest` on `Bifrost` with the same nil/empty-field guards as other lifecycle methods, delegating to `handleStreamRequest`.
- Wired `ResponsesRetrieveStreamRequest` into `handleProviderStreamRequest` via the `ResponsesLifecycleProvider` type assertion, returning an unsupported-operation error for providers that do not implement the interface.
- Updated the HTTP transport (`responsesRetrieve` handler and `extractResponsesLifecycleFromPath`) to parse the `stream` query parameter and branch into `handleStreamingResponsesRetrieve` when `stream=true`, keeping the unary path unchanged.
- Added a `StreamConfig` with a `ResponsesStreamResponseConverter` to the OpenAI route config for the retrieve endpoint so SSE chunks are serialised consistently with the create-stream path.
- Updated the generic router's `handleStreamingRequest` to dispatch `ResponsesRetrieveRequest` payloads through `ResponsesRetrieveStreamRequest`.
- Added `"responses_retrieve_stream"` to the UI log constants, labels, and colour map (violet, matching other stream types).
- Added unit tests covering `buildResponsesRetrieveQuery` with `stream=true`/unset and `IsStreamingRequested` across nil/unset/false/true cases.

## Type of change

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

## Affected areas

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

## How to test

```sh
# Core/Transports
go test ./core/... ./framework/... ./plugins/... ./transports/...

# Create a response, then retrieve it as a stream
curl -N "http://localhost:8080/v1/responses/{id}?stream=true&provider=openai" \
  -H "x-bifrost-key: <key>" \
  -H "Accept: text/event-stream"
# Expect: a sequence of SSE events ending with event: response.completed

# Retrieve without stream param — must still return a unary JSON response
curl "http://localhost:8080/v1/responses/{id}?provider=openai" \
  -H "x-bifrost-key: <key>"

# UI
cd ui
pnpm i
pnpm build
```

## Breaking changes

- [x] No

`ResponsesLifecycleProvider` gains a new method (`ResponsesRetrieveStream`). Any external implementation of this interface must add the method. The built-in `OpenAIProvider` is the only implementation in this repository.

## Related issues

## Security considerations

No new auth mechanisms are introduced. The streaming retrieve path reuses the same bearer-token header injection and key-resolution logic as all other provider requests. No PII beyond what is already present in a stored response is exposed.

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

Briefly explain the purpose of this PR and the problem it solves.

## Changes

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

## Type of change

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

## Affected areas

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

## How to test

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

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

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

## Screenshots/Recordings

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

## Breaking changes

- [ ] Yes
- [ ] No

If yes, describe impact and migration instructions.

## Related issues

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

## Security considerations

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

## 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
…mhq#5505)

## Summary

Fixes the `customResponseHandler` path in `HandleOpenAIResponsesStreaming` where `sendBackRawRequest` and `sendBackRawResponse` were hardcoded to `false`, and where subsequent error/completion handling was incorrectly scoped inside the `else` block, causing it to be skipped entirely when a custom response handler was provided. Fixes maximhq#5504

## Changes

- Replaced hardcoded `false, false` arguments in the `customResponseHandler` call with the actual `sendBackRawRequest` and `sendBackRawResponse` values.
- Added `RawResponse` population for the custom handler path, mirroring the existing behavior in the non-custom handler path.
- Moved error type handling (`ResponsesStreamResponseTypeError`, `ResponsesStreamResponseTypeFailed`), completion handling (`ResponsesStreamResponseTypeCompleted`, `ResponsesStreamResponseTypeIncomplete`), and chunk dispatch outside of the `else` block so they execute regardless of whether a custom response handler is used.
- Removed the `// TODO fix this` comment that tracked this known issue.

## 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 ./...
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None.

## 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
… paginated log search (maximhq#5486)

## Summary

Fixes a pagination `total_count` bug where matview-eligible windows (≥24h) returned an inflated count because `mv_logs_hourly` predicates on `hour` rounded both time boundaries out to full hour buckets, counting logs that the exact-range row list could never page to. Closes maximhq#5329.

## Changes

- Replaced the single `mv_logs_hourly` sum in `getCountFromMatView` with a hybrid approach: only buckets fully contained within `[StartTime, EndTime]` are summed from the matview; the partial boundary buckets (at most one on each side) are counted directly from the raw `logs` table using a timestamp-indexed scan over ≤2 hours of rows.
- Introduced `countRawTerminal` to count raw log rows restricted to terminal statuses when no status filter is present, ensuring both halves of the hybrid count cover the same population as `mv_logs_hourly`.
- All bucket-grid arithmetic is performed in SQL via `date_trunc('hour', timestamptz)` rather than Go-side `time.Truncate`, so servers with fractional-hour timezone offsets (e.g. `Asia/Kolkata`, +05:30) produce correct bucket boundaries without misalignment.
- Added `TestSearchLogsMatViewCountMatchesRawRange` to assert that `total_count` matches the number of terminal logs strictly within `[StartTime, EndTime]` across mid-hour, hour-aligned-start, and hour-aligned-end boundary cases.

## Type of change

- [x] Bug fix

## Affected areas

- [x] Core (Go)

## How to test

```sh
go test ./framework/logstore/... -run TestSearchLogsMatViewCountMatchesRawRange -v
```

The test inserts logs at known timestamps straddling a 25h15m window, refreshes the matviews, forces the matview count path, and asserts that `total_count` equals exactly the number of terminal logs inside `[start, end]` for three boundary configurations (mid-hour boundaries, hour-aligned end, hour-aligned start).

## Breaking changes

- [x] No

## Related issues

Closes maximhq#5329

## Security considerations

None.

## 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
…hq#5507)

## Summary

This PR adds two improvements to the materialized view read path: cache-hit statistics are now served from the hybrid aggregate (interior buckets from `mv_logs_hourly`, boundary slivers classified raw) instead of a separate full-window raw scan, and a runtime self-heal mechanism automatically recovers from missing or stale-shaped materialized views without requiring a process restart.

## Changes

- **Cache hits in the hybrid aggregate**: Three new columns (`direct_cache_hits`, `semantic_cache_hits`, `cache_debug_count`) are added to `mv_logs_hourly` using the same `cacheDebugJSONGuard` and `cacheDebugHitTypeExpr` expressions shared across the matview DDL, boundary sliver queries, and `aggregateCacheHits`. `cache_debug_count` preserves the nil contract: when no row in the window carried valid `cache_debug` JSON the fields are omitted from the response, and when cache rows exist but none were direct/semantic explicit zeros are returned. The previous approach issued a separate full-window raw scan for cache hits after the hybrid aggregate completed; that scan is removed.

- **Runtime matview self-heal** (`matviewheal.go`): `isMatViewShapeError` classifies PostgreSQL error codes `42P01` (undefined table), `42703` (undefined column), and `55000` (object not in prerequisite state) as shape errors. `fallBackToRaw` is called at every matview dispatch site — on a shape error it disables the matview read path process-wide, logs a warning, and triggers a single-flight background repair via `triggerMatViewSelfHeal`. The repair runs `ensureMatViews` then `refreshMatViews` and re-enables the path on success. A 30-second cooldown (`matViewHealCooldown`) bounds repair frequency; while broken, every request continues succeeding via the raw fallback. Two new atomic fields (`matViewHealInFlight`, `matViewHealLastAttempt`) are added to `RDBLogStore`.

- **Shared SQL constants**: The inline regex strings for the cache debug guard and hit-type extractor are replaced with named constants `cacheDebugJSONGuard` and `cacheDebugHitTypeExpr`, used consistently across the matview DDL, `applyFilters`, `rawTerminalStatsAgg`, and `aggregateCacheHits`.

- **Tests**: `TestGetStatsMatViewCacheHitsHybrid` verifies the hybrid cache-hit path including nil and zero contracts and agreement with the raw path. `TestMatViewShapeErrorFallsBackAndSelfHeals`, `TestMatViewStaleShapeFallsBackAndSelfHeals`, and `TestFilterMatViewShapeErrorFallsBack` cover the 42P01, 42703, and filter-view drop scenarios end-to-end, including background self-heal convergence.

## Type of change

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

## Affected areas

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

## How to test

```sh
go test ./framework/logstore/... -run TestGetStatsMatViewCacheHitsHybrid
go test ./framework/logstore/... -run TestMatViewShapeErrorFallsBackAndSelfHeals
go test ./framework/logstore/... -run TestMatViewStaleShapeFallsBackAndSelfHeals
go test ./framework/logstore/... -run TestFilterMatViewShapeErrorFallsBack
go test ./framework/logstore/... -run TestIsMatViewShapeError
go test ./framework/logstore/...
```

The self-heal tests drop or replace `mv_logs_hourly` mid-run and assert that reads continue returning correct results from the raw table with no error, that `matViewsReady` is set to false immediately, and that the view is recreated with the correct shape within 90 seconds.

## Breaking changes

- [x] No

The new matview columns require a schema migration. On first deploy, `repairMatViewShapes` detects the missing columns, drops and recreates `mv_logs_hourly`, and the self-heal path handles any replica that reads before the rebuild completes.

## Related issues

Closes maximhq#5384

## Security considerations

No auth, secrets, PII, or sandboxing changes. The new SQL expressions are constants composed only of built-in PostgreSQL operators and are not user-controlled.

## 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 and others added 11 commits July 24, 2026 10:27
## Summary

Resolves a merge conflict in `rdb.go` by adopting the materialized view fallback pattern introduced in `5b4b7fe74`. The previous HEAD version returned immediately from matview queries and required bucket sizes to be exact multiples of 3600 seconds. The merged version removes the modulo constraint and falls back to raw queries when the matview query fails.

## Changes

- Removed the `bucketSizeSeconds%3600 == 0` constraint across all histogram matview query paths, allowing any bucket size ≥ 3600 seconds to use the matview.
- Replaced direct returns from matview queries with a fallback pattern: if the matview query returns an error that satisfies `fallBackToRaw`, the query is retried against the raw table instead of surfacing the error to the caller.
- Affected histogram methods: `GetTokenHistogram`, `GetThroughputHistogram`, `GetProviderThroughputHistogram`, `GetCostHistogram`, `GetModelHistogram`, `GetLatencyHistogram`, `GetProviderCostHistogram`, `GetProviderTokenHistogram`, `GetProviderLatencyHistogram`, `GetDimensionCostHistogram`, `GetDimensionTokenHistogram`, and `GetDimensionLatencyHistogram`.

## Type of change

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

## Affected areas

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

## How to test

```sh
go test ./framework/logstore/...
```

Verify that histogram queries with non-hour-aligned bucket sizes (e.g. 7200, 10800) correctly use the materialized view on PostgreSQL, and that queries fall back to raw table scans when the matview is unavailable or returns an error.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

Closes conflict introduced between HEAD and `5b4b7fe74 (add cached tokens to matview; allow matview refresh on the fly)`.

## Security considerations

None.

## 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
…nts (maximhq#5463)

## Summary

The Azure provider was unconditionally appending `api-version=preview` to all `/openai/v1/responses` and `/openai/v1/responses/compact` requests. Azure OpenAI v1 GA endpoints and Azure AI Foundry project endpoints reject requests on `/v1` paths that include an `api-version` query parameter, causing failures for users on those endpoint types. This PR makes `api-version` injection opt-in for the versionless v1 API — it is only attached when the caller explicitly configures an API version override.

## Changes

- Removed the automatic injection of `AzureAPIVersionPreview` for `Responses`, `ResponsesStream`, and `Compaction` endpoints; `api-version` is now only appended when a non-empty version is resolved from the caller's configuration.
- Updated `buildPassthroughURL` so that the `/openai/v1/responses` path branch no longer falls back to `AzureAPIVersionPreview` — it only sets `api-version` if the caller did not supply one and an explicit override is configured.
- Updated tests to reflect the new default behavior (no `api-version` injected when absent and no override is set) and added a test case for the `/openai/v1/responses/compact` route.
- Updated test descriptions and comments to accurately describe the opt-in semantics.

## Type of change

- [x] Bug fix

## Affected areas

- [x] Core (Go)
- [x] Providers/Integrations

## How to test

```sh
go test ./core/providers/azure/...
```

Validate that:

- Requests to `/openai/v1/responses` without a configured API version override do **not** include `api-version` in the query string.
- Requests with an explicit `AzureAliasCfg.APIVersion` override **do** include `api-version` in the query string.
- Caller-supplied `api-version` query parameters are always preserved as-is.

## Breaking changes

- [x] No

## Related issues

## Security considerations

None.

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

Azure Model Router bills a flat per-input-token infrastructure surcharge on top of the cost of whichever underlying model it actually routes to. Previously, cost calculation had no awareness of this two-part billing model, meaning only one pricing row was applied. This PR adds dedicated handling so that when a request is routed through `azure/model-router`, the surcharge and the underlying model's cost are both calculated and summed correctly.

## Changes

- Added a `calculateAzureModelRouterCost` method that computes the Model Router's own surcharge (from its catalog row) and then looks up and adds the cost of the model Azure actually served, read from the response body's `model` field rather than from `RoutingInfo.Model`.
- Added `azureModelRouterServedModel` helper to extract the real served model name across all supported response shapes (`ChatResponse`, `ResponsesResponse`, `ResponsesStreamResponse`, `TextCompletionResponse`).
- Text completion requests routed through Model Router are priced against `chat` mode rows, since Model Router has no native `/completions` support and always converts them internally.
- If the served model name is absent or echoes back `"model-router"` itself, only the surcharge is applied — no double-counting occurs.
- The two-part billing path is gated strictly on `provider == Azure` and `IsAzureModelRouter(model)`, so identically named models on other providers are unaffected.
- Added a dedicated test file covering chat completion, Responses API, streaming Responses, text completion (chat-mode fallback), missing underlying pricing, same-model guard, and non-Azure provider isolation.

## Type of change

- [ ] Bug fix
- [x] 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 ./framework/modelcatalog/datasheet/...
```

Expected: all tests pass, including the new `cost_azure_model_router_test.go` cases which validate surcharge-plus-underlying-model arithmetic for each response shape.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

None. This change is limited to cost calculation logic and does not touch auth, secrets, or PII.

## 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
…ngface (maximhq#5470)

# Fix duplicate inference-provider segment in HuggingFace model IDs

## Summary

Allowlist entries selected from a previous `ListModels` response already carry an inference-provider segment (e.g. `featherless-ai/org/model`). The backfill re-wrap was blindly prepending the current inference provider, producing compound IDs like `huggingface/cohere/featherless-ai/org/model` that duplicate the provider segment and break request routing.

This fix detects when a backfill entry's raw ID already begins with a known inference provider or the `auto` policy, and routes it accordingly instead of prepending another provider segment.

## Changes

- When a backfill entry's raw ID starts with a known inference provider, it is only emitted during that provider's pass and skipped in all others, preventing duplication across passes.
- When a backfill entry's raw ID starts with `auto`, it is emitted exactly once during the canonical first pass (the first entry in `INFERENCE_PROVIDERS`) and skipped in every other pass, since `auto` is not itself an inference provider in the listing loop.
- Entries without an inference-provider segment retain the existing behavior: the current inference provider is prepended as before.
- Added `isKnownInferenceProviderOrPolicy` helper to check whether a segment names a supported inference provider or the `auto` policy.
- Added regression tests covering all three cases.

## Type of change

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

## Affected areas

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

## How to test

```sh
go test ./core/providers/huggingface/...
```

Expected: all three regression test cases pass — matching provider emits a single correctly-prefixed entry, non-matching providers do not duplicate the entry, and the `auto`\-policy entry is emitted exactly once from the canonical first pass.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

Closes maximhq#4215

## Security considerations

None.

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

Briefly explain the purpose of this PR and the problem it solves.

## Changes

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

## Type of change

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

## Affected areas

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

## How to test

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

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

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

## Screenshots/Recordings

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

## Breaking changes

- [ ] Yes
- [ ] No

If yes, describe impact and migration instructions.

## Related issues

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

## Security considerations

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

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

Bumps several Go dependencies to their latest patch/minor versions and adds E2E test coverage for the OpenAI Responses API secondary-verb lifecycle, including the newly supported streaming retrieve (`GET /v1/responses/{id}?stream=true`).

## Changes

- Upgraded `github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp` from `v1.31.0` to `v1.32.0`
- Upgraded `go.opentelemetry.io/contrib/detectors/gcp` from `v1.42.0` to `v1.43.0`
- Upgraded `google.golang.org/genproto/googleapis/api` from `20260401024825-9d38bb4040a9` to `20260414002931-afd174a4e478`
- Upgraded `google.golang.org/grpc` from `v1.81.1` to `v1.82.1`
- Added E2E test collection entry **"29. OpenAI Responses Lifecycle + Streaming Retrieve"** covering:
  - Streamed background response creation (SSE, `store: true`, `background: true`)
  - Non-streaming retrieve, verifying the returned `id` matches the created response
  - Streaming retrieve (`GET ?stream=true&starting_after=0`), verifying SSE content-type and `response.*` events
  - `input_items` listing, verifying `object: list`
  - Delete/cleanup, verifying `deleted: true`
  - All of the above exercised against both the native `/v1/responses` routes and the `/openai/v1` drop-in routes
- Added `openai-responses-deleted` shape detection to the provider harness test script for `response.deleted` SSE events

## Type of change

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

## Affected areas

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

## How to test

```sh
# Verify Go dependency graph is consistent
go mod verify

# Run existing test suite
go test ./...

# Run the E2E Postman collection against a live environment
newman run tests/e2e/api/collections/provider-harness.json \
  --env-var baseUrl=<your-gateway-url> \
  --env-var openaiKey=<your-openai-key>
# Expect folder "29. OpenAI Responses Lifecycle + Streaming Retrieve" to pass all assertions
```

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

No new auth flows, secrets handling, or PII exposure introduced. The E2E tests use existing collection variables for API keys.

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

Bumps several Go dependencies to their latest patch/minor versions and adds two new UI components to improve the virtual key management experience for access-profile-managed keys.

## Changes

- **Dependency upgrades:**
  - `GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp`: `v1.31.0` → `v1.32.0`
  - `go.opentelemetry.io/contrib/detectors/gcp`: `v1.42.0` → `v1.43.0`
  - `google.golang.org/genproto/googleapis/api`: `20260401` → `20260414`
  - `google.golang.org/grpc`: `v1.81.1` → `v1.82.1`

- **`ManagedVirtualKeyActions` component:** Added a new enterprise component (with an OSS no-op fallback) rendered inside the access-profile alert banner in `virtualKeySheet.tsx`. Exposes the managing profile to allow enterprise-specific actions on managed virtual keys.

- **`ViewUserDetailsButton` component:** Added a new enterprise component (with an OSS no-op fallback) rendered in the Budget Information header of `virtualKeyDetailsSheet.tsx` when a key is managed by a profile. Links to the managing profile's user details using `managingProfile.user_id`.

- **`managingProfile` exposed from `useVirtualKeyUsage`:** The hook's `managingProfile` value is now destructured and passed down in `virtualKeySheet.tsx` so it can be forwarded to `ManagedVirtualKeyActions`.

- **Weight display fix:** Provider config weight in `virtualKeyDetailsSheet.tsx` now renders `"Not Set"` (muted italic) instead of a blank value when `weight` is `null` or `undefined`.

- **`BudgetOverrideDialog` button variant:** Changed the trigger button variant from conditionally `"outline"` (when active) to always `"ghost"` for visual consistency.

## Type of change

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

## Affected areas

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

## How to test

```sh
# UI
cd ui
pnpm i
pnpm build

# Verify Go modules resolve cleanly
cd framework && go mod verify
cd transports && go mod verify
```

- Open a virtual key managed by an access profile and confirm the `ManagedVirtualKeyActions` slot renders (enterprise) or is invisible (OSS).
- Open the details sheet for a managed key and confirm the `ViewUserDetailsButton` appears next to the Budget Information heading.
- Open a provider config with no weight set and confirm `"Not Set"` is displayed instead of a blank.
- Open the budget override dialog and confirm the trigger button always uses the ghost variant regardless of active state.

## Screenshots/Recordings

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

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

Link related issues and discussions.

## Security considerations

No new auth, secrets, or PII handling introduced. The `ViewUserDetailsButton` receives a `userId` prop but rendering is delegated to the enterprise implementation.

## 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
…arams (maximhq#5524)

## Summary

Adds `cache_hit_types` as a supported filter on the dashboard page, allowing users to filter requests by cache hit type via the URL state and filter panel.

## Changes

- Added `cache_hit_types` to the URL state parser with a default empty array
- Included `cache_hit_types` in the query parameters passed to the data fetching logic when the filter is non-empty
- Added `cache_hit_types` to the dependency array so the dashboard re-fetches when this filter changes
- Wired `cache_hit_types` into the filter update handler so it persists correctly when filters are applied

## 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 dashboard page
2. Apply a `cache_hit_types` filter
3. Verify the URL reflects the selected filter values
4. Verify the dashboard data updates to reflect the filter
5. Reload the page and confirm the filter is restored from the URL

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

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

No security implications. This change only adds a new client-side filter parameter to an existing dashboard query.

## 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
… chart and legend series order (maximhq#5526)

## Summary

Legend color indicators in dashboard charts were misaligned with their corresponding chart series when the number of series exceeded `TOP_SERIES_LIMIT`. The legend was deriving colors from the API's alphabetical label order, while the charts were rendering series sorted by volume. This PR fixes the desync by introducing `computeDisplaySeries`, a shared utility that produces the exact ordered series list (top-N by volume, with `OTHER_SERIES_KEY` appended for rolled-up tails) that both charts and legends consume.

## Changes

- Added `computeDisplaySeries` to `chartUtils.ts`, which wraps `pickTopSeries` and conditionally appends `OTHER_SERIES_KEY` when the tail is rolled up. An `includeOther` flag (defaulting to `true`) allows latency and throughput charts to opt out of the rollup, since averaging the long tail would produce misleading values.
- Replaced all direct `pickTopSeries` call sites in `costChart`, `modelUsageChart`, `providerCostChart`, `providerLatencyChart`, `providerThroughputChart`, and `providerTokenChart` with `computeDisplaySeries`.
- Updated `overviewTabView` and `providerUsageTabView` to derive legend provider/model lists using `computeDisplaySeries` instead of `sanitizeSeriesLabels` on the raw API label arrays, ensuring legend order and colors match what the charts draw.
- Fixed legend rendering in `overviewTab` and `providerUsageTab` to display `OTHER_SERIES_LABEL` and `OTHER_SERIES_COLOR` when the `OTHER_SERIES_KEY` sentinel appears in the series list.

## Type of change

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

## Affected areas

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

## How to test

Navigate to the dashboard with a workspace that has more providers or models than `TOP_SERIES_LIMIT`. Verify that the legend color dots next to each series name match the colors of the corresponding bars or areas in the chart, including the "Other" entry.

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

## Screenshots/Recordings

Verify before/after that legend color dots align with chart series colors when more than the top-N series are present, and that the "Other" legend entry appears with the correct gray color.
<img width="1512" height="862" alt="image" src="https://github.com/user-attachments/assets/d21db038-9528-467a-81a0-517a3407c5c0" />
<img width="1512" height="861" alt="image" src="https://github.com/user-attachments/assets/e80d1d17-19ca-4c03-933b-8589468b21b6" />


## Breaking changes

- [ ] Yes
- [x] No

## Related issues  
maximhq#5506

## Security considerations

None.

## 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 support for configuring a `batch_role_arn` directly in the Bedrock key config, so operators no longer need to pass the service role ARN on every batch request via `extra_params`. The error message for a missing role ARN is also updated to reflect both configuration paths. Additionally, fixes a nil-pointer issue in the OpenAI batch response converter by calling `WithDefaults()` before returning the response.

## Changes

- Added `batch_role_arn` as a first-class field in the Bedrock key config, with priority over any `role_arn` sent in the request. This allows the role to be set once at the key level (including via `env.` prefix) rather than requiring callers to supply it per-request.
- Updated the error message when `role_arn` is missing to mention both `extra_params` and the new `batch_role_arn` key config field.
- Updated the Helm chart `values.schema.json` and `values.yaml` to document and validate the new `batch_role_arn` field for both top-level and nested Bedrock key configs.
- Fixed the OpenAI batch response converter to call `resp.WithDefaults()` and guard against a `nil` return before passing the response downstream, preventing a potential nil-pointer dereference.

## Type of change

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

## Affected areas

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

## How to test

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

1. Configure a Bedrock key with `batch_role_arn` set (either as a literal ARN or via `env.MY_ROLE_ARN`).
2. Submit a batch request without passing `role_arn` in `extra_params` and confirm the job is created successfully using the configured ARN.
3. Submit a batch request with `role_arn` in `extra_params` alongside a `batch_role_arn` in the key config and confirm the key config value takes priority.
4. Submit a batch request with neither configured and confirm the error message reads: `role_arn is required for Bedrock batch API (send it in extra_params or set batch_role_arn in the key config)`.
5. Trigger an OpenAI batch response and confirm no nil-pointer panic occurs when `WithDefaults()` returns `nil`.

**New config field:**

| Field | Location | Description |
|---|---|---|
| `batch_role_arn` | Bedrock key config | Service role ARN Bedrock assumes for batch S3 access. Supports `env.` prefix. Takes priority over `role_arn` in `extra_params`. |

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

`batch_role_arn` supports the `env.` prefix, consistent with other sensitive fields in the Bedrock key config, so the ARN can be injected from environment variables rather than hardcoded in config files.

## 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
@akshaydeo
akshaydeo dismissed coderabbitai[bot]’s stale review July 24, 2026 22:43

The merge-base changed after approval.

@akshaydeo
akshaydeo requested a review from a team as a code owner July 24, 2026 22:43
@akshaydeo

Copy link
Copy Markdown
Contributor

Hi @StrangeXin — 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=5348

Let us know if you run into any issues signing.

This branch has not been deployed

No deployments
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.

10 participants