Skip to content

refactor(gateway): simplify openai router internals - #737

Merged
slin1237 merged 1 commit into
mainfrom
slin/oai-refactor-7
Mar 12, 2026
Merged

slin1237 merged 1 commit into
mainfrom
slin/oai-refactor-7

Conversation

@slin1237

@slin1237 slin1237 commented Mar 12, 2026 •

Copy link
Copy Markdown
Member

Summary

Cleanup pass on the OpenAI router internals following the recent extraction refactors (#732, #735). Eliminates redundant state, simplifies function signatures, and fixes an O(n*m) performance issue in MCP metadata injection.

Refs: #732, #735

What changed

  • context.rs: ResponsesComponents.shared changed from owned SharedComponents to Arc<SharedComponents>, sharing the same allocation with OpenAIRouter.shared_components
  • router.rs: ResponsesComponents now clones the existing shared_components Arc instead of constructing a second SharedComponents; removed redundant client field from ResponsesRouterContext construction
  • route.rs: Removed derivable client field from ResponsesRouterContext struct; extracted conversation as Option<&str> before passing to load_input_history instead of passing two ResponsesRequest references
  • history.rs: Replaced two-ResponsesRequest-refs signature with explicit conversation: Option<&str> parameter; consolidated redundant .clone().filter() + .take().filter() of previous_response_id into a single .take(); replaced raw "function_call" string with ItemType::FUNCTION_CALL constant
  • tool_loop.rs: Replaced O(n*m) Vec::insert(0, ...) loops in inject_mcp_metadata_streaming and build_incomplete_response with Vec::splice(0..0, prefix) using a pre-built prefix vector

Why

The recent extraction refactors (#732, #735) faithfully moved code but left some structural redundancies:

  • reqwest::Client was stored in two separate SharedComponents instances
  • ResponsesRouterContext carried a client field always derivable from responses_components
  • load_input_history took two references to ResponsesRequest (original + mutable clone) just to read the conversation field from the original
  • previous_response_id was processed twice with the same filter predicate (clone+filter, then take+filter)
  • MCP metadata injection used repeated Vec::insert(0, ...) causing O(n) shifts per insert

How

  • Changed ResponsesComponents.shared to Arc<SharedComponents> so both the chat and responses paths share the same allocation (transparent via Deref)
  • Removed the client field and inlined the access path at the single call site
  • Extracted conversation in route_responses before calling load_input_history, passing it as Option<&str> to eliminate the need for the original body reference
  • Used a single .take().filter() and borrowed the result for the chain-loading branch
  • Built prefix vectors and used splice(0..0, ...) for O(1) amortized prepend

Test plan

  • cargo check -p smg passes
  • cargo clippy -p smg --all-targets --all-features -- -D warnings passes clean
  • All changes are internal refactors with no behavioral change — existing integration tests cover the affected paths

Summary by CodeRabbit

  • Refactor
    • Optimized shared component initialization for improved memory efficiency.
    • Streamlined MCP tool list handling through bulk operation processing.
    • Simplified conversation history parameter handling and data flow.
    • Removed redundant internal field references and consolidated component access patterns.

- Share SharedComponents via Arc between OpenAIRouter and
  ResponsesComponents, eliminating a redundant reqwest::Client
  clone in context.rs and router.rs

- Remove derivable `client` field from ResponsesRouterContext;
  access it through responses_components.shared.client instead
  (route.rs, router.rs)

- Simplify load_input_history signature: replace the confusing
  two-ResponsesRequest-refs pattern (body + request_body) with
  an explicit `conversation: Option<&str>` parameter (history.rs,
  route.rs)

- Consolidate redundant clone()+take() of previous_response_id
  into a single take() in history.rs

- Use ItemType::FUNCTION_CALL constant instead of raw
  "function_call" string literal in history.rs

- Replace O(n*m) Vec::insert(0, ...) loops with Vec::splice in
  inject_mcp_metadata_streaming and build_incomplete_response
  (tool_loop.rs)

Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
@github-actions github-actions Bot added model-gateway Model gateway crate changes openai OpenAI router changes labels Mar 12, 2026
@coderabbitai

coderabbitai Bot commented Mar 12, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6f3f8e5c-dbf4-48f9-bd02-59eadd1033dc

📥 Commits

Reviewing files that changed from the base of the PR and between 1c7da91 and e4ccb79.

📒 Files selected for processing (5)
  • model_gateway/src/routers/openai/context.rs
  • model_gateway/src/routers/openai/mcp/tool_loop.rs
  • model_gateway/src/routers/openai/responses/history.rs
  • model_gateway/src/routers/openai/responses/route.rs
  • model_gateway/src/routers/openai/router.rs

📝 Walkthrough

Walkthrough

This PR refactors shared component management by wrapping SharedComponents in an Arc for safer multi-reference usage, optimizes MCP tool list prepending by consolidating multiple insert operations into a single splice, and updates the history loading interface to accept an optional conversation ID instead of a full request object.

Changes

Cohort / File(s) Summary
Shared Components Arc Wrapping
model_gateway/src/routers/openai/context.rs, model_gateway/src/routers/openai/router.rs
Changed ResponsesComponents.shared field type from SharedComponents to Arc<SharedComponents> and updated ComponentRefs::Shared variant to hold the Arc-wrapped type; Arc initialization updated in router construction.
History Loading Refactoring
model_gateway/src/routers/openai/responses/history.rs, model_gateway/src/routers/openai/responses/route.rs
Updated load_input_history function signature to accept optional conversation ID instead of ResponsesRequest reference; changed item-type matching from string literal to ItemType::FUNCTION_CALL enum variant; conversation variable handling refactored in route handler.
MCP Tool Loop Optimization
model_gateway/src/routers/openai/mcp/tool_loop.rs
Replaced per-item prepend logic with bulk prefix construction; consolidated multiple front-insert operations into a single splice for both streaming and incomplete response handling, reducing algorithmic complexity.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

  • #735: Modifies the same ResponsesComponents.shared and ResponsesRouterContext Arc-wrapping refactoring across router context code paths.
  • #730: Updates the same model_gateway/src/routers/openai/mcp/tool_loop.rs MCP list-tools insertion logic.
  • #732: Changes the load_input_history function signature and history-loading implementation in model_gateway/src/routers/openai/responses/history.rs.

Suggested labels

mcp

Suggested reviewers

  • CatherineSue
  • key4ng

Poem

🐰 Wrapping shared in Arc so bright,
Splicing tools with all our might,
Conversation IDs refined,
Efficient patterns now aligned!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor(gateway): simplify openai router internals' accurately captures the main objective of the pull request, which is to clean up and simplify the OpenAI router's internal structure through refactoring.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch slin/oai-refactor-7

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

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the internal architecture of the OpenAI router within the model gateway, focusing on improving efficiency and reducing structural redundancies. It addresses issues such as duplicate reqwest::Client instances, unnecessary fields in context structs, and a significant performance bottleneck in MCP metadata injection. The changes streamline component sharing, simplify function interfaces, and optimize critical data manipulation, resulting in a cleaner, more performant, and maintainable codebase without altering any external behavior.

Highlights

  • Shared Component Management: Updated ResponsesComponents.shared to use Arc<SharedComponents>, ensuring shared allocation and reducing redundancy across components.
  • Performance Optimization: Replaced inefficient O(n*m) Vec::insert(0, ...) loops with Vec::splice(0..0, prefix) for MCP metadata injection, significantly improving performance.
  • Simplified Function Signatures: Refactored load_input_history to accept conversation directly as Option<&str>, removing the need for passing two ResponsesRequest references.
  • Redundant Field Removal: Eliminated the derivable client field from ResponsesRouterContext and its construction, streamlining the context structure.
  • Code Clarity: Consolidated previous_response_id processing and replaced a raw string literal with ItemType::FUNCTION_CALL for improved readability and maintainability.
Changelog
  • model_gateway/src/routers/openai/context.rs
    • Updated ResponsesComponents.shared field type from SharedComponents to Arc<SharedComponents>.
  • model_gateway/src/routers/openai/mcp/tool_loop.rs
    • Replaced Vec::insert(0, ...) loops with Vec::splice(0..0, prefix) in inject_mcp_metadata_streaming for optimized metadata injection.
    • Applied Vec::splice optimization in build_incomplete_response for prepending items.
  • model_gateway/src/routers/openai/responses/history.rs
    • Imported ItemType from openai_protocol::event_types.
    • Modified load_input_history function signature to accept conversation: Option<&str>.
    • Consolidated previous_response_id handling to a single .take().filter() operation.
    • Replaced the string literal "function_call" with ItemType::FUNCTION_CALL.
    • Updated append_current_input call to use conv_id_str directly.
    • Changed the return value of load_input_history to previous_response_id.
  • model_gateway/src/routers/openai/responses/route.rs
    • Removed the client field from the ResponsesRouterContext struct.
    • Updated WorkerSelector::new to use deps.responses_components.shared.client directly.
    • Refactored conversation variable initialization to use filter and is_some() for validation.
    • Modified the call to super::history::load_input_history to pass conversation.map(String::as_str).
  • model_gateway/src/routers/openai/router.rs
    • Updated the initialization of ResponsesComponents to use Arc::clone(&shared_components) for the shared field.
    • Removed the client field from the responses_route::ResponsesRouterContext construction.
Activity
  • The author confirmed that cargo check -p smg passes.
  • The author confirmed that cargo clippy -p smg --all-targets --all-features -- -D warnings passes clean.
  • The author verified that all changes are internal refactors with no behavioral change, and existing integration tests cover the affected paths.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request refactors the OpenAI router internals to simplify state management and improve performance. Key changes include sharing SharedComponents via an Arc to eliminate redundant allocations, removing the derivable client field from ResponsesRouterContext, and simplifying the load_input_history function signature. Additionally, an O(n*m) performance issue in metadata injection has been resolved by replacing repeated Vec::insert calls with a more efficient splice operation. These changes improve code clarity and efficiency.

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

Labels

model-gateway Model gateway crate changes openai OpenAI router changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant