fix(reasoning): use fresh parser on non-streaming path to avoid shared mutex

[slin1237]

Description

Problem

Non-streaming reasoning extraction served all requests for a given model from one pooled parser instance (Arc<Mutex<Box<dyn ReasoningParser>>>). Every non-streaming request would lock().await that single mutex, reset() the parser, run detect_and_parse_reasoning, then drop the state — so concurrent requests for the same model serialized on one mutex despite needing no shared state. The streaming path already avoids this by creating a fresh parser per request/index for state isolation.

From the codebase audit: crates/reasoning_parser/src/factory.rs:64 — Non-streaming requests serialize on one shared parser mutex per model.

Solution

Switch the non-streaming paths to a fresh, owned per-request parser via create_parser / create_reasoning_parser (the same approach the streaming path already uses). A fresh parser is inherently clean, so the explicit reset() and the lock().await are no longer needed, removing the per-model contention entirely. The now-unused pooled helper get_reasoning_parser is dropped.

Changes

• model_gateway/src/routers/grpc/regular/processor.rs: both non-streaming sites (chat completions + Anthropic messages) now take a fresh parser via utils::create_reasoning_parser(...) instead of the pooled parser + lock().await + reset().
• model_gateway/src/routers/parse/handlers.rs: the /separate_reasoning endpoint uses registry().create_parser(...) instead of get_pooled_parser(...).lock().await.
• model_gateway/src/routers/grpc/utils/parsers.rs: removed the now-unused get_reasoning_parser pooled helper (and its unused PooledParser import); refreshed the create_reasoning_parser doc to note it serves both streaming and non-streaming. Added unit tests.
• model_gateway/src/routers/grpc/utils/mod.rs: dropped the get_reasoning_parser re-export.

The tool-parser pooled path is intentionally left unchanged (separate concern/finding).

Test Plan

Added focused unit tests in parsers.rs:

• create_reasoning_parser_returns_independent_instances — two parsers created for the same model are independent: mark_reasoning_started() on one does not flip the other's is_in_reasoning(), and the untouched instance still parses a full <think>…</think> document correctly. This is exactly the isolation the shared pooled parser violated.
• create_reasoning_parser_honors_configured_parser — the configured-parser path resolves to the requested parser.

Gate (sccache disabled, scoped to smg):

$ cargo +nightly fmt --all
(clean, exit 0)

$ RUSTC_WRAPPER="" cargo clippy -p smg --all-targets -- -D warnings
    Checking smg v1.4.1
    Finished `dev` profile [unoptimized + debuginfo] target(s)
(no warnings, exit 0)

$ RUSTC_WRAPPER="" cargo test -p smg
running 2 tests
test routers::grpc::utils::parsers::tests::create_reasoning_parser_honors_configured_parser ... ok
test routers::grpc::utils::parsers::tests::create_reasoning_parser_returns_independent_instances ... ok
...
test result: ok. 1005 passed; 0 failed; 4 ignored; 0 measured; 0 filtered out
     Running tests/api_tests.rs
test result: ok. 105 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
     Running tests/routing_tests.rs
test result: ok. 92 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
     Running tests/spec_test.rs
test result: ok. 95 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out
     Running tests/security_tests.rs
test result: ok. 48 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
     (all remaining integration test binaries: ok, 0 failed)

Checklist

• cargo +nightly fmt passes
• cargo clippy --all-targets --all-features -- -D warnings passes
• (Optional) Documentation updated
• (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

Summary by CodeRabbit

Refactor

• Improved reasoning parser handling by creating independent parser instances per request instead of using shared pooled parsers, enhancing reliability and reducing contention.

Tests

• Added tests to verify parser isolation and correct parser selection based on configuration.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refactors reasoning parser management from a pooled, shared, mutex-locked pattern to per-request instantiation. The pooled get_reasoning_parser accessor is removed, gRPC processors and REST parse handlers create fresh parsers per request, and new tests verify state isolation.

Changes

Per-request reasoning parser refactor

Layer / File(s)	Summary
Parser factory and utility updates model_gateway/src/routers/grpc/utils/parsers.rs, model_gateway/src/routers/grpc/utils/mod.rs	Remove the pooled (non-streaming) reasoning parser accessor get_reasoning_parser, update imports to only include required types, and remove its re-export from the utilities module.
gRPC response processor refactors model_gateway/src/routers/grpc/regular/processor.rs	Update process_single_choice and process_non_streaming_messages_response to create fresh parser instances per request via create_reasoning_parser instead of acquiring and resetting a pooled parser; reasoning state marking and error handling remain unchanged.
REST parse handler update model_gateway/src/routers/parse/handlers.rs	Update parse_reasoning to create a fresh parser instance per request via factory.registry().create_parser(...) instead of obtaining and locking a pooled parser; error response shape for unknown parsers remains 400 Bad Request.
Per-request parser instantiation tests model_gateway/src/routers/grpc/utils/parsers.rs	Add unit tests for create_reasoning_parser that verify each call returns an independent parser instance with isolated reasoning state and that the configured parser name is honored.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• lightseekorg/smg#1505: The main PR refactors gRPC/parse code to create fresh reasoning parsers per request using the factory, and the retrieved PR adds/registers a new "none" reasoning parser in that same factory—the new parser option plugs directly into the per-request parser creation path.

• lightseekorg/smg#1031: The main PR's refactor to create fresh per-request reasoning parsers in process_single_choice/process_non_streaming_messages_response builds on the retrieved PR's reasoning-state hooks and template/user gating logic to keep reasoning-mode initialization consistent.

• lightseekorg/smg#747: The main PR's refactor of reasoning parsing in process_non_streaming_messages_response (switching from pooled locking to per-request creation) directly overlaps with the retrieved PR's introduction of that non-streaming messages response processing method.

Suggested labels

model-gateway, grpc, reasoning-parser

Suggested reviewers

• CatherineSue
• key4ng

Poem

🐰 Fresh parsers bloom with every request,
No locks, no pools, just isolation blessed,
Each thread thinks freely, state stays clean,
The cleanest reasoning flow we've ever seen!
Per-request, per-thought, logic takes flight! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: switching non-streaming reasoning paths from pooled parsers with shared mutexes to fresh per-request parsers, which directly addresses the concurrency/serialization problem identified in the PR objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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/reasoning-factory-nonstream-mutex

---

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

[gemini-code-assist]

Code Review

This pull request replaces the use of shared pooled reasoning parsers with fresh, independent parser instances per request for non-streaming extraction. This change avoids serialization bottlenecks on the shared pooled mutex and ensures proper state isolation. The unused get_reasoning_parser helper has been removed, and unit tests have been added to verify that independent parser instances are correctly created and configured. There are no review comments to address.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[claude]

Clean, correct refactoring. The switch from pooled parser + mutex to fresh per-request parser eliminates unnecessary serialization and the subtle state-leak risk (especially in handlers.rs where reset() was missing). All call sites are properly guarded by reasoning_parser_available, so the Option return from create_reasoning_parser/create_for_model won't cause silent skipping. Tests cover the key invariant (instance independence). No issues found.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

model_gateway/src/routers/grpc/utils/parsers.rs (1)

76-100: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make the availability check match this fallback behavior.

create_reasoning_parser() now warns and falls back to create_for_model(model), but the gRPC non-streaming callers only reach this code after check_reasoning_parser_availability(...) succeeds. That helper still returns false as soon as configured_parser is unknown, so a typo in configured_reasoning_parser skips reasoning extraction entirely and this new fallback never runs.

Suggested fix

 pub(crate) fn check_reasoning_parser_availability(
     reasoning_parser_factory: &ReasoningParserFactory,
     configured_parser: Option<&str>,
     model: &str,
 ) -> bool {
     if let Some(parser_name) = configured_parser {
-        reasoning_parser_factory.registry().has_parser(parser_name)
+        reasoning_parser_factory.registry().has_parser(parser_name)
+            || reasoning_parser_factory.registry().has_parser_for_model(model)
     } else {
         reasoning_parser_factory
             .registry()
             .has_parser_for_model(model)
     }
 }

🤖 Prompt for 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.

In `@model_gateway/src/routers/grpc/utils/parsers.rs` around lines 76 - 100, The
availability check in check_reasoning_parser_availability must match
create_reasoning_parser's fallback logic: when a configured_parser is provided
but registry().create_parser(parser_name) returns None, the helper should not
immediately return false — instead attempt registry().create_for_model(model)
and return true if that yields a parser. Update
check_reasoning_parser_availability to call
ReasoningParserFactory.registry().create_parser(parser_name) and, on None, call
registry().create_for_model(model) to determine availability (mirroring
create_reasoning_parser), so a typo in configured_reasoning_parser will fall
back to model-based selection.

🤖 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.

Outside diff comments:
In `@model_gateway/src/routers/grpc/utils/parsers.rs`:
- Around line 76-100: The availability check in
check_reasoning_parser_availability must match create_reasoning_parser's
fallback logic: when a configured_parser is provided but
registry().create_parser(parser_name) returns None, the helper should not
immediately return false — instead attempt registry().create_for_model(model)
and return true if that yields a parser. Update
check_reasoning_parser_availability to call
ReasoningParserFactory.registry().create_parser(parser_name) and, on None, call
registry().create_for_model(model) to determine availability (mirroring
create_reasoning_parser), so a typo in configured_reasoning_parser will fall
back to model-based selection.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 37f92b1d-a4e0-478a-a4bf-60eab3d0b067

📥 Commits

Reviewing files that changed from the base of the PR and between 27b528f and 0fac584.

📒 Files selected for processing (4)

• model_gateway/src/routers/grpc/regular/processor.rs
• model_gateway/src/routers/grpc/utils/mod.rs
• model_gateway/src/routers/grpc/utils/parsers.rs
• model_gateway/src/routers/parse/handlers.rs

[CatherineSue]

Maybe we can remove the shared mutex after this PR? Right now I don't see the usage of get_parser.