Skip to content

refactor: remove dead embedding/classify code from regular PreparationStage - #668

Merged
CatherineSue merged 2 commits into
mainfrom
chang/cleanup
Mar 7, 2026
Merged

CatherineSue merged 2 commits into
mainfrom
chang/cleanup

Conversation

@CatherineSue

@CatherineSue CatherineSue commented Mar 7, 2026 •

Copy link
Copy Markdown
Member

Description

Problem

EmbeddingPreparationStage was wired into the regular PreparationStage delegator for Embedding and Classify request types. However, these requests are routed to their own dedicated pipelines (new_embeddings / new_classify) by the gRPC router and never reach the regular pipeline — making this delegation dead code.

Additionally, EmbeddingPreparationStage had an inconsistent construction style compared to other stages: it used ::new() in some places and bare struct syntax in others.

Solution

  • Remove EmbeddingPreparationStage from the delegating PreparationStage and replace the Embedding/Classify match arms with an error (matching the existing Responses arm pattern)
  • Restore new() on EmbeddingPreparationStage for consistency with other stages constructed in pipeline.rs
  • Remove the unnecessary Default impl that was only needed to satisfy clippy for the now-removed new()

Changes

  • preparation.rs (delegator): Remove embedding_stage field and Embedding/Classify match arms; collapse into a single error arm for unexpected request types
  • embedding/preparation.rs: Restore new() constructor, remove Default impl
  • pipeline.rs: Use EmbeddingPreparationStage::new() consistently

Test Plan

  • Pre-commit hooks pass (rustfmt, clippy)
  • No behavioral change: Embedding/Classify requests already route to dedicated pipelines
Checklist
  • cargo +nightly fmt passes
  • cargo clippy --all-targets --all-features -- -D warnings passes
  • (Optional) Documentation updated

Summary by CodeRabbit

  • Refactor
    • Streamlined internal request handling pipeline and improved error messaging for enhanced maintainability.

…parationStage

Signed-off-by: Chang Su <chang.s.su@oracle.com>
@github-actions github-actions Bot added grpc gRPC client and router changes model-gateway Model gateway crate changes labels Mar 7, 2026
@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 EmbeddingPreparationStage by eliminating its new() and Default implementations, which were unnecessary for a unit struct. The change promotes consistency across preparation stages by directly instantiating EmbeddingPreparationStage where it is used, simplifying the codebase and removing boilerplate.

Highlights

  • Code Refactoring: Removed redundant new() and Default implementations from the EmbeddingPreparationStage unit struct, as they are unnecessary for types without fields.
  • Consistency: Updated all instantiation sites to directly use EmbeddingPreparationStage, aligning its usage with other similar preparation stages like ChatPreparationStage and GeneratePreparationStage.
Changelog
  • model_gateway/src/routers/grpc/pipeline.rs
    • Updated EmbeddingPreparationStage instantiation from EmbeddingPreparationStage::new() to EmbeddingPreparationStage in RequestPipeline::new_regular and RequestPipeline::new_streaming methods.
  • model_gateway/src/routers/grpc/regular/stages/embedding/preparation.rs
    • Removed the impl EmbeddingPreparationStage block containing the new() function.
    • Removed the impl Default for EmbeddingPreparationStage block.
  • model_gateway/src/routers/grpc/regular/stages/preparation.rs
    • Updated embedding_stage initialization in PreparationStage::new from EmbeddingPreparationStage::new() to EmbeddingPreparationStage.
Activity
  • Pre-commit hooks (rustfmt, clippy) passed.
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. ↩

@coderabbitai

coderabbitai Bot commented Mar 7, 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: a6f5c520-e8bc-4200-adab-985affa7033e

📥 Commits

Reviewing files that changed from the base of the PR and between b02a8e5 and 585c669.

📒 Files selected for processing (2)
  • model_gateway/src/routers/grpc/regular/stages/embedding/preparation.rs
  • model_gateway/src/routers/grpc/regular/stages/preparation.rs
💤 Files with no reviewable changes (1)
  • model_gateway/src/routers/grpc/regular/stages/embedding/preparation.rs

📝 Walkthrough

Walkthrough

The changes remove embedding preparation stage integration from the main preparation router. The EmbeddingPreparationStage's Default implementation is deleted, and the embedding_stage field is removed from PreparationStage struct, along with updates to request type handling logic.

Changes

Cohort / File(s) Summary
Embedding Preparation Removal
model_gateway/src/routers/grpc/regular/stages/embedding/preparation.rs, model_gateway/src/routers/grpc/regular/stages/preparation.rs
Removed Default trait implementation for EmbeddingPreparationStage; removed embedding_stage field from PreparationStage struct; refactored request type branching to use generic fallback and dynamic error messages instead of static embedding/classify handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 The embedding stage has hopped away,
Simpler flows now light the day,
Fewer branches, cleaner code so bright,
The router's refactored just right! ✨

🚥 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 describes the main refactoring: removing dead embedding/classify code from PreparationStage, which aligns with the core changes removing EmbeddingPreparationStage usage and related branching logic.
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 chang/cleanup

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

@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 is a good refactoring that improves code consistency by removing the unnecessary new() and Default implementations from the EmbeddingPreparationStage unit struct. The changes are correct and well-contained.

I've added a suggestion to apply a similar simplification to PreparationStage itself.

For further consistency, you might consider applying the same refactoring to other unit structs in a follow-up PR, such as:

  • EmbeddingRequestBuildingStage
  • EmbeddingResponseProcessingStage
  • ClassifyResponseProcessingStage

Overall, a solid cleanup.

…tage

The regular PreparationStage delegated to EmbeddingPreparationStage for
Embedding and Classify request types, but these requests are routed to
dedicated pipelines (new_embeddings/new_classify) and never reach the
regular pipeline. Remove the dead code and error on unexpected request
types instead.

Also restore new() on EmbeddingPreparationStage for consistency with
other stages constructed in pipeline.rs.

Signed-off-by: Chang Su <chang.s.su@oracle.com>
@CatherineSue CatherineSue changed the title refactor: remove unnecessary new() and Default from EmbeddingPreparationStage refactor: remove dead embedding/classify code from regular PreparationStage Mar 7, 2026
@CatherineSue
CatherineSue merged commit de7360e into main Mar 7, 2026
35 of 42 checks passed
@CatherineSue
CatherineSue deleted the chang/cleanup branch March 7, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

grpc gRPC client and router changes model-gateway Model gateway crate changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant