fix: ensure request ID is set in context before PreHooks

[TejasGhatte]

Summary

Ensure request ID is consistently set in context before PreHooks are executed, fixing an issue where request IDs might be missing or inconsistently generated.

Changes

• Added request ID generation in handleRequest and handleStreamRequest functions to ensure it's set before PreHooks are executed
• Removed redundant request ID generation in the Maxim plugin's PreHook function, now relying on the guaranteed ID from the context
• Removed BifrostContextKeyStreamEndIndicator from the reserved keys list as it was no longer needed

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins
• UI (Next.js)
• Docs

How to test

Verify that request IDs are consistently present in logs and responses:

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

# Test with Maxim plugin enabled
curl -X POST http://localhost:8000/v1/chat/completions \
  -H "Content-Type: application/json" \
  -d '{"model":"gpt-3.5-turbo", "messages":[{"role":"user", "content":"Hello"}]}'

Breaking changes

• Yes
• No

Related issues

Fixes inconsistent request ID generation across the application.

Security considerations

This change improves traceability by ensuring request IDs are consistently generated and available throughout the request lifecycle.

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

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Request ID is now always established before request processing, ensuring consistent tracking and improved observability and debugging.

• Improvements

  • Context key restrictions loosened to allow previously blocked writes, increasing flexibility in storing request-related data and enabling additional behaviors.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Walkthrough

Ensure a RequestID is injected into the request context in core/bifrost.go before PreHooks run; remove BifrostContextKeyStreamEndIndicator from the reserved keys in core/schemas/context.go; modify plugins/maxim/main.go to locally generate and log a fallback UUID when the context lacks a string RequestID (without writing it back). Changelogs updated.

Changes

Cohort / File(s)	Summary
Request ID initialization core/bifrost.go	handleRequest and handleStreamRequest now ensure a string RequestID exists in context: if missing, a new UUID is generated and inserted into the context before PreHooks execute.
Context reserved keys core/schemas/context.go	Removed BifrostContextKeyStreamEndIndicator from reservedKeys, allowing that key to be written via SetValue.
Plugin PreHook fallback behavior plugins/maxim/main.go	PreHook now retrieves RequestID via short type assertion; if missing or not a string, it generates a new UUID and logs a warning but does not call ctx.SetValue to write it back.
Documentation / changelogs core/changelog.md, transports/changelog.md	Added changelog entries: "fix: ensure request ID is consistently set in context before PreHooks are executed" (and retained existing entries).

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Bifrost
  participant PreHook
  participant Plugin as "Maxim Plugin"
  rect rgb(230, 245, 255)
    Client->>Bifrost: Send request
    Bifrost->>Bifrost: Ensure RequestID in context (generate UUID if missing)
    Bifrost->>PreHook: Invoke PreHooks with context (contains RequestID)
  end
  rect rgb(245, 255, 230)
    PreHook->>Plugin: Call plugin PreHook (reads RequestID from ctx)
    alt Plugin finds non-string/missing RequestID
      Plugin->>Plugin: Generate local UUID, log warning (does not set ctx)
    else Plugin reads RequestID string
      Plugin-->>PreHook: Continue
    end
    PreHook-->>Bifrost: Return control
    Bifrost->>Client: Process request and respond
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I hopped in, nose to the breeze,
Found each request traced with ease,
A UUID tucked in the flow,
PreHooks hum and onward go,
Logs whisper where the fallback grew. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main change: ensuring request ID is set in context before PreHooks, which is the core fix across handleRequest, handleStreamRequest, and the Maxim plugin.
Description check	✅ Passed	The description follows the template structure with all key sections completed: Summary, Changes, Type of change, Affected areas, How to test, Breaking changes, Related issues, Security considerations, and Checklist. All required information is present and well-documented.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch 01-03-fix_set_request_id_before_prehooks

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 23755a3 and 69ea61d.

📒 Files selected for processing (5)

• core/bifrost.go
• core/changelog.md
• core/schemas/context.go
• plugins/maxim/main.go
• transports/changelog.md

💤 Files with no reviewable changes (1)

• core/schemas/context.go

✅ Files skipped from review due to trivial changes (2)

• core/changelog.md
• transports/changelog.md

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

• core/bifrost.go

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)

Files:

• plugins/maxim/main.go

🧠 Learnings (2)

📚 Learning: 2025-12-09T17:07:42.007Z

Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.

Applied to files:

• plugins/maxim/main.go

📚 Learning: 2025-12-29T11:54:55.836Z

Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 1153
File: framework/configstore/rdb.go:2221-2246
Timestamp: 2025-12-29T11:54:55.836Z
Learning: In Go reviews, do not flag range-over-int patterns like for i := range n as compile-time errors, assuming Go 1.22+ semantics. Only flag actual range-capable values (slices, arrays, maps, channels, strings) and other compile-time issues. This applies to all Go files across the repository.

Applied to files:

• plugins/maxim/main.go

🧬 Code graph analysis (1)

plugins/maxim/main.go (2)

core/schemas/bifrost.go (1)

• BifrostContextKeyRequestID (124-124)

plugins/semanticcache/main.go (1)

• PluginLoggerPrefix (150-150)

🔇 Additional comments (1)

plugins/maxim/main.go (1)

401-406: Defensive check correctly implemented.

The safe type assertion with fallback UUID generation and warning log successfully addresses the previous review concern about potential panics. The implementation is appropriate given the core guarantee that request IDs are set before PreHooks execute.

Verification confirms consistent defensive programming across plugins: Other plugins (logging, semantic cache, otel, and others) all consistently use safe type assertions with defensive checks for request ID and context value retrieval. The logging plugin has similar checks at lines 227-228 and 354-355, and the semantic cache plugin has multiple defensive checks throughout its context value retrieval (lines 362-363, 472-473, 478-479, etc.). No unsafe direct type assertions were found across any plugin, confirming a consistent defensive programming pattern.

Minor observation: In the edge case where the fallback is triggered (lines 402-406), PreHook creates a stream accumulator with the fallback requestID (line 410), but PostHook will return early if the context still lacks the request ID (lines 443-446), leaving the accumulator uncleaned. Given the "should never happen" guarantee and the warning log for observability, this edge case handling seems acceptable.

---

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[TejasGhatte]

• fix: ensure request ID is set in context before PreHooks #1227 👈 (View in Graphite)
• main

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

[github-actions]

🧪 Test Suite Available

This PR can be tested by a repository admin.

Run tests for PR #1227

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 187190b and 23755a3.

📒 Files selected for processing (3)

• core/bifrost.go
• core/schemas/context.go
• plugins/maxim/main.go

💤 Files with no reviewable changes (1)

• core/schemas/context.go

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs (if there is a stack, if there is no stack you can continue to make localized suggestions/reviews)

Files:

• core/bifrost.go
• plugins/maxim/main.go

🧠 Learnings (2)

📚 Learning: 2025-12-09T17:07:42.007Z

Learnt from: qwerty-dvorak
Repo: maximhq/bifrost PR: 1006
File: core/schemas/account.go:9-18
Timestamp: 2025-12-09T17:07:42.007Z
Learning: In core/schemas/account.go, the HuggingFaceKeyConfig field within the Key struct is currently unused and reserved for future Hugging Face inference endpoint deployments. Do not flag this field as missing from OpenAPI documentation or require its presence in the API spec until the feature is actively implemented and used. When the feature is added, update the OpenAPI docs accordingly; otherwise, treat this field as non-breaking and not part of the current API surface.

Applied to files:

• core/bifrost.go
• plugins/maxim/main.go

📚 Learning: 2025-12-29T11:54:55.836Z

Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 1153
File: framework/configstore/rdb.go:2221-2246
Timestamp: 2025-12-29T11:54:55.836Z
Learning: In Go reviews, do not flag range-over-int patterns like for i := range n as compile-time errors, assuming Go 1.22+ semantics. Only flag actual range-capable values (slices, arrays, maps, channels, strings) and other compile-time issues. This applies to all Go files across the repository.

Applied to files:

• core/bifrost.go
• plugins/maxim/main.go

🧬 Code graph analysis (2)

core/bifrost.go (1)

core/schemas/bifrost.go (1)

• BifrostContextKeyRequestID (124-124)

plugins/maxim/main.go (1)

core/schemas/bifrost.go (1)

• BifrostContextKeyRequestID (124-124)

🔇 Additional comments (2)

core/bifrost.go (2)

2217-2221: LGTM! Request ID initialization ensures consistent tracing.

The implementation correctly ensures a request ID exists in the context before PreHooks execute. The type assertion check handles both missing keys and incorrect types, guaranteeing downstream plugins can rely on this value.

---

2315-2319: LGTM! Consistent request ID initialization for streaming requests.

The streaming path correctly mirrors the non-streaming implementation to ensure request ID presence before PreHooks. While this duplicates the logic from handleRequest, the duplication is acceptable given these are two distinct request paths.