From 643aeefe6d538b842828f5f7208f380ac19d8962 Mon Sep 17 00:00:00 2001 From: Simo Lin Date: Wed, 24 Dec 2025 13:54:48 -0500 Subject: [PATCH] [model-gateway] Fix logging module name, parse endpoint context, and tokenizer factory (#15782) --- sgl-model-gateway/src/middleware.rs | 2 +- sgl-model-gateway/src/routers/http/router.rs | 15 +-------------- sgl-model-gateway/src/routers/mod.rs | 11 ----------- .../src/routers/parse/handlers.rs | 15 ++------------- sgl-model-gateway/src/server.rs | 6 +++--- sgl-model-gateway/src/tokenizer/factory.rs | 19 ++++++++++++++++--- 6 files changed, 23 insertions(+), 45 deletions(-) diff --git a/sgl-model-gateway/src/middleware.rs b/sgl-model-gateway/src/middleware.rs index bae4c790e..ed464c596 100644 --- a/sgl-model-gateway/src/middleware.rs +++ b/sgl-model-gateway/src/middleware.rs @@ -289,7 +289,7 @@ impl MakeSpan for RequestSpan { status_code = Empty, latency = Empty, error = Empty, - module = "sglang::router_rs" + module = "sgl_model_gateway" ) } } diff --git a/sgl-model-gateway/src/routers/http/router.rs b/sgl-model-gateway/src/routers/http/router.rs index 9e601eed7..281a54706 100644 --- a/sgl-model-gateway/src/routers/http/router.rs +++ b/sgl-model-gateway/src/routers/http/router.rs @@ -35,14 +35,13 @@ use crate::{ completion::CompletionRequest, embedding::EmbeddingRequest, generate::GenerateRequest, - parser::{ParseFunctionCallRequest, SeparateReasoningRequest}, rerank::{RerankRequest, RerankResponse, RerankResult}, responses::{ResponsesGetParams, ResponsesRequest}, }, routers::{ error::{self, extract_error_code_from_response}, grpc::utils::{error_type_from_status, route_to_endpoint}, - header_utils, parse, RouterTrait, + header_utils, RouterTrait, }, }; @@ -54,7 +53,6 @@ pub struct Router { dp_aware: bool, enable_igw: bool, retry_config: RetryConfig, - context: Option>, } impl std::fmt::Debug for Router { @@ -66,7 +64,6 @@ impl std::fmt::Debug for Router { .field("dp_aware", &self.dp_aware) .field("enable_igw", &self.enable_igw) .field("retry_config", &self.retry_config) - .field("context", &"") .finish() } } @@ -81,7 +78,6 @@ impl Router { dp_aware: ctx.router_config.dp_aware, enable_igw: ctx.router_config.enable_igw, retry_config: ctx.router_config.effective_retry_config(), - context: Some(ctx.clone()), }) } @@ -817,14 +813,6 @@ impl RouterTrait for Router { } } - async fn parse_function_call(&self, req: &ParseFunctionCallRequest) -> Response { - parse::parse_function_call(self.context.as_ref(), req).await - } - - async fn parse_reasoning(&self, req: &SeparateReasoningRequest) -> Response { - parse::parse_reasoning(self.context.as_ref(), req).await - } - fn router_type(&self) -> &'static str { "regular" } @@ -859,7 +847,6 @@ mod tests { client: Client::new(), retry_config: RetryConfig::default(), enable_igw: false, - context: None, } } diff --git a/sgl-model-gateway/src/routers/mod.rs b/sgl-model-gateway/src/routers/mod.rs index 5143719ef..725107258 100644 --- a/sgl-model-gateway/src/routers/mod.rs +++ b/sgl-model-gateway/src/routers/mod.rs @@ -16,7 +16,6 @@ use crate::protocols::{ completion::CompletionRequest, embedding::EmbeddingRequest, generate::GenerateRequest, - parser::{ParseFunctionCallRequest, SeparateReasoningRequest}, rerank::RerankRequest, responses::{ResponsesGetParams, ResponsesRequest}, }; @@ -194,16 +193,6 @@ pub trait RouterTrait: Send + Sync + Debug { (StatusCode::NOT_IMPLEMENTED, "Rerank not implemented").into_response() } - /// Parse function calls from text - async fn parse_function_call(&self, req: &ParseFunctionCallRequest) -> Response { - parse::parse_function_call(None, req).await - } - - /// Separate reasoning from normal text - async fn parse_reasoning(&self, req: &SeparateReasoningRequest) -> Response { - parse::parse_reasoning(None, req).await - } - /// Get router type name fn router_type(&self) -> &'static str; diff --git a/sgl-model-gateway/src/routers/parse/handlers.rs b/sgl-model-gateway/src/routers/parse/handlers.rs index 7c944b670..6297a449c 100644 --- a/sgl-model-gateway/src/routers/parse/handlers.rs +++ b/sgl-model-gateway/src/routers/parse/handlers.rs @@ -28,13 +28,9 @@ fn error_response(status: StatusCode, message: &str) -> Response { /// Parse function calls from model output text pub async fn parse_function_call( - context: Option<&Arc>, + ctx: &Arc, req: &ParseFunctionCallRequest, ) -> Response { - let Some(ctx) = context else { - return error_response(StatusCode::SERVICE_UNAVAILABLE, "Context not initialized"); - }; - let Some(factory) = &ctx.tool_parser_factory else { return error_response( StatusCode::SERVICE_UNAVAILABLE, @@ -71,14 +67,7 @@ pub async fn parse_function_call( } /// Parse and separate reasoning from normal text -pub async fn parse_reasoning( - context: Option<&Arc>, - req: &SeparateReasoningRequest, -) -> Response { - let Some(ctx) = context else { - return error_response(StatusCode::SERVICE_UNAVAILABLE, "Context not initialized"); - }; - +pub async fn parse_reasoning(ctx: &Arc, req: &SeparateReasoningRequest) -> Response { let Some(factory) = &ctx.reasoning_parser_factory else { return error_response( StatusCode::SERVICE_UNAVAILABLE, diff --git a/sgl-model-gateway/src/server.rs b/sgl-model-gateway/src/server.rs index 4c53ffc35..82149f9b7 100644 --- a/sgl-model-gateway/src/server.rs +++ b/sgl-model-gateway/src/server.rs @@ -50,7 +50,7 @@ use crate::{ validated::ValidatedJson, worker_spec::{WorkerConfigRequest, WorkerUpdateRequest}, }, - routers::{conversations, router_manager::RouterManager, tokenize, RouterTrait}, + routers::{conversations, parse, router_manager::RouterManager, tokenize, RouterTrait}, service_discovery::{start_service_discovery, ServiceDiscoveryConfig}, wasm::route::{add_wasm_module, list_wasm_modules, remove_wasm_module}, workflow::{LoggingSubscriber, WorkflowEngine}, @@ -67,14 +67,14 @@ async fn parse_function_call( State(state): State>, Json(req): Json, ) -> Response { - state.router.parse_function_call(&req).await + parse::parse_function_call(&state.context, &req).await } async fn parse_reasoning( State(state): State>, Json(req): Json, ) -> Response { - state.router.parse_reasoning(&req).await + parse::parse_reasoning(&state.context, &req).await } async fn sink_handler() -> Response { diff --git a/sgl-model-gateway/src/tokenizer/factory.rs b/sgl-model-gateway/src/tokenizer/factory.rs index d8b66c012..569a81a48 100644 --- a/sgl-model-gateway/src/tokenizer/factory.rs +++ b/sgl-model-gateway/src/tokenizer/factory.rs @@ -254,14 +254,27 @@ pub async fn create_tokenizer_async_with_chat_template( } // Check if it's a GPT model name that should use Tiktoken - if model_name_or_path.contains("gpt-") + // Only match specific OpenAI model patterns to avoid catching HuggingFace models like "openai/gpt-oss-20b" + if model_name_or_path.contains("gpt-4") + || model_name_or_path.contains("gpt-3.5") + || model_name_or_path.contains("gpt-3") + || model_name_or_path.contains("turbo") || model_name_or_path.contains("davinci") || model_name_or_path.contains("curie") || model_name_or_path.contains("babbage") || model_name_or_path.contains("ada") + || model_name_or_path.contains("codex") { - let tokenizer = TiktokenTokenizer::from_model_name(model_name_or_path)?; - return Ok(Arc::new(tokenizer)); + // Try tiktoken first, but fall back to HuggingFace if it fails + match TiktokenTokenizer::from_model_name(model_name_or_path) { + Ok(tokenizer) => return Ok(Arc::new(tokenizer)), + Err(e) => { + debug!( + "Tiktoken failed for '{}': {}, falling back to HuggingFace", + model_name_or_path, e + ); + } + } } // Try to download tokenizer files from HuggingFace