Skip to content

fix(bench): handle Compaction variant in routing_allocation_bench - #1331

Merged
slin1237 merged 1 commit into
mainfrom
fix/bench-compaction-arm
Apr 22, 2026
Merged

slin1237 merged 1 commit into
mainfrom
fix/bench-compaction-arm

Conversation

@slin1237

@slin1237 slin1237 commented Apr 22, 2026 •

Copy link
Copy Markdown
Member

Summary

PR #1302 (I3 compaction) added the ResponseInputOutputItem::Compaction variant but did not extend the exhaustive match in routing_allocation_bench.rs, breaking cargo clippy --all-targets --all-features -- -D warnings on main. This is the forced-cascade arm that should have shipped with #1302.

Error on main

error[E0004]: non-exhaustive patterns: `&openai_protocol::responses::ResponseInputOutputItem::Compaction { .. }` not covered
   --> model_gateway/benches/routing_allocation_bench.rs:17:38
    |
 17 |             .filter_map(|item| match item {
    |                                      ^^^^ pattern `...::Compaction { .. }` not covered

All downstream PRs that run clippy are red because of this — e.g., PR #1330 unit-tests lane: https://github.com/lightseekorg/smg/actions/runs/24800464547/job/72581561317

Change

Add a single arm returning None — Compaction carries only encrypted_content, no routable text to extract. Matches the neutral shape of the sibling McpApprovalRequest / ImageGenerationCall arms.

                ResponseInputOutputItem::ImageGenerationCall { .. } => None,
+               ResponseInputOutputItem::Compaction { .. } => None,
            })

Test plan

  • Local: cargo clippy --all-targets --all-features -- -D warnings completes clean (Finished dev profile ... target(s) in 4m 56s).
  • No behavior change — bench was already returning None for all non-text-bearing variants; this adds the arm for the new Compaction variant following the same pattern.

Follow-up

None required — this is a one-line forced-cascade fix.

Summary by CodeRabbit

  • Bug Fixes
    • Refined routing allocation item filtering to exclude inappropriate item types during text extraction, resulting in more accurate routing decisions.

PR #1302 added `ResponseInputOutputItem::Compaction` per I3 but did
not extend the exhaustive match in the routing-allocation bench,
breaking `cargo clippy --all-targets --all-features -- -D warnings`
on main:

  error[E0004]: non-exhaustive patterns:
    `ResponseInputOutputItem::Compaction { .. }` not covered
    --> model_gateway/benches/routing_allocation_bench.rs:17:38

Compaction carries encrypted_content only (no routable text), so
the forced-cascade arm returns None, matching the neutral shape of
the sibling McpApprovalRequest / ImageGenerationCall arms.

Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Apr 22, 2026 •

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

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: 9afad4ef-c8ea-4147-960b-8b1fac57b6cd

📥 Commits

Reviewing files that changed from the base of the PR and between 03fdbd0 and 5eaaebf.

📒 Files selected for processing (1)
  • model_gateway/benches/routing_allocation_bench.rs

📝 Walkthrough

Walkthrough

The legacy routing text extraction function in the benchmark now explicitly filters out ResponseInputOutputItem::Compaction items, preventing their inclusion in routing allocation text. Only the legacy extraction path was modified; the new extraction path and benchmark setup remain unchanged.

Changes

Cohort / File(s) Summary
Legacy Routing Extraction Filter
model_gateway/benches/routing_allocation_bench.rs
Added explicit filtering to exclude ResponseInputOutputItem::Compaction items in extract_text_for_routing_old function, preventing compaction items from being included in the routing allocation concatenated string.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested labels

benchmarks, model-gateway

Suggested reviewers

  • CatherineSue
  • key4ng
  • zhaowenzi

Poem

🐰 A line of code, so clean and bright,
Filters compaction from the sight,
Legacy paths now exclude with care,
Routing allocations, stripped so fair,
One tiny tweak makes benchmarks right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: fixing a clippy error by handling the Compaction variant in the routing_allocation_bench exhaustive match pattern.
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.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/bench-compaction-arm

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

@github-actions github-actions Bot added benchmarks Benchmark changes model-gateway Model gateway crate changes labels Apr 22, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good — single match arm addition to handle the new Compaction variant in the benchmark. Returning None is correct since compaction items carry no user text for routing extraction.

@slin1237
slin1237 merged commit 18f63e4 into main Apr 22, 2026
33 of 35 checks passed
@slin1237
slin1237 deleted the fix/bench-compaction-arm branch April 22, 2026 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmarks Benchmark changes model-gateway Model gateway crate changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant