[sachalmalick]

Summary

Extract production HTTP transport, lifecycle, disconnect monitoring, and metric primitives into dynamo-http-server, and separate chat inference behind ChatController. Services can reuse the server core and supply a different chat implementation while retaining the existing API schema, validation, metrics, cancellation, and JSON/SSE delivery.

Closes #17177. Part of #11320.

How This Was Implemented

• Rebase Ishan's server-core extraction from refactor(http): extract reusable server core #11281 onto the chat-controller branch, preserving its commit attribution.
• Move the current HTTP/2 accept loop, TLS/mTLS setup, socket write deadlines, request helpers, and graceful-drain lifecycle into lib/http-server.
• Move metrics and connection primitives while retaining current serving-load accounting; model readiness, model-card/LoRA adapters, protocol response conversion, and event publication remain in dynamo-llm.
• Keep ChatController and DynamoChatController in lib/llm, with .chat_controller(...) builder injection and the existing route/schema.
• Update HTTP/gRPC consumers, the gateway metrics import, workspace and bindings lockfiles, and generated ownership metadata.

Walkthrough

• lib/http-server/src/server.rs: HttpServer::prepare performs fallible setup before background tasks start; serve handles drain and shutdown.
• lib/llm/src/http/service/service_v2.rs: builds routes and runtime-backed state, then supplies the completed router to the reusable server.
• lib/llm/src/controller/chat.rs and controller/chat/dynamo.rs: the replaceable inference contract and its default implementation.

The server crate depends on runtime utility types but does not activate discovery or request-plane infrastructure, and has no dependency on dynamo-llm.

Validation

Validated on macOS; LLM commands use --no-default-features.

• cargo test -p dynamo-http-server --offline --lib -- --test-threads=1 — 56 passed.
• cargo test -p dynamo-llm --no-default-features --locked --offline --lib -- http::service:: frontend_load prompt_tokens --test-threads=1 — 389 passed.
• Nine integration targets (chat_controller, http-service, chat_template_render_errors, http_metrics, http_websocket, protocols, frontend_protocol_validation_http, anthropic_http_replay, responses_http_replay) — 125 passed.
• Embedding input-token accounting test and final injected-controller check passed.
• cargo check passed for both crates and dynamo-ext-proc.
• Server Clippy passed for all targets with -D warnings. LLM Clippy passed with -D warnings -A clippy::collapsible_match; the allowance covers two existing findings in unchanged protocols/codec.rs.
• Formatting, diff checks, locked bindings resolution, and strict CODEOWNERS coverage passed.

All existing HTTP tests were retained across the move. Performance was not benchmarked; the chat-controller boundary adds one boxed async future per request.

Summary by CodeRabbit

• New Features
  • Chat completion services can now use a configurable inference handler, allowing applications to customize how models are resolved and responses are generated.
  • Existing chat request validation, HTTP error responses, and streaming and non-streaming response formats are preserved.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: ai-dynamo/dynamo
• Branch: main
• File: .coderabbit.yaml
• Commit: 75d0686
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 12bd4e5d-5f46-4c58-9041-1fb4b77a935d

📥 Commits

Reviewing files that changed from the base of the PR and between 75d0686 and 062c61a.

📒 Files selected for processing: 11

• lib/llm/src/controller.rs
• lib/llm/src/controller/chat.rs
• lib/llm/src/controller/chat/dynamo.rs
• lib/llm/src/http/service.rs
• lib/llm/src/http/service/openai.rs
• lib/llm/src/http/service/service_v2.rs
• lib/llm/src/lib.rs
• lib/llm/src/preprocessor.rs
• lib/llm/src/preprocessor/request_options.rs
• lib/llm/src/preprocessor/structural_tag.rs
• lib/llm/tests/chat_controller.rs

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

Chat completions now use a typed controller for model resolution, readiness checks, and inference. The HTTP service can inject an alternate controller. The default Dynamo controller retains model and parser selection. Integration tests cover injected controllers across streaming and non-streaming requests.

Changes

Chat completions controller

Layer / File(s)	Summary
Controller contract and Dynamo implementation lib/llm/src/controller.rs, lib/llm/src/controller/..., lib/llm/src/preprocessor.rs, lib/llm/src/preprocessor/..., lib/llm/src/http/service.rs, lib/llm/src/lib.rs	Adds the ChatController contract and typed model, output, and error types. The Dynamo implementation resolves models, checks readiness, selects parsing options, and generates output. Request-specific parsing-option construction moves to the preprocessor.
HTTP controller injection lib/llm/src/http/service/openai.rs, lib/llm/src/http/service/service_v2.rs	The chat route delegates readiness checks, model resolution, and generation to the controller. HttpServiceConfig accepts an optional controller, and the default route continues to construct the Dynamo controller.
Controller integration tests lib/llm/tests/chat_controller.rs	Tests injected controller behavior, model templating, HTTP errors, OpenAPI consistency, and cancellation after a streaming response is dropped.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 062c6

No merge-blocking issue was identified in the chat-controller change. Merge after the normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 42.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 10 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		Issue #17177 coding requirements are met. lib/llm/src/controller/chat.rs defines ChatController, typed outputs, errors, and DynamoChatController. service_v2.rs injects a controller through the…
Out of Scope Changes check		The changes remain within lib/llm and focus on chat-completions controller separation. The preprocessor helper move supports the controller boundary. The added integration test verifies the required…
Title check		The title clearly and concisely describes the primary changes: extracting HTTP server concerns and separating chat inference behind a controller.
Description check		The description explains the implementation, affected components, reviewer focus, validation results, and linked issues. It uses different headings from the template but provides the required informat…

Full details: Linked Issues check

Explanation

Issue #17177 coding requirements are met. lib/llm/src/controller/chat.rs defines ChatController, typed outputs, errors, and DynamoChatController. service_v2.rs injects a controller through the builder. openai.rs keeps HTTP decoding, templates, validation, error mapping, metrics, and JSON/SSE delivery in the handler. The integration test covers injected unary and streaming requests without a registered model, schema parity, aliases, validation, error statuses, and disconnect cancellation. DynamoChatController retains model readiness, engine/parser pairing, and request parser options.

Full details: Out of Scope Changes check

Explanation

The changes remain within lib/llm and focus on chat-completions controller separation. The preprocessor helper move supports the controller boundary. The added integration test verifies the required HTTP contract. No package extraction or controllers for other APIs are introduced, and the summary states that other APIs and model listing continue to use ModelManager.

Full details: Description check

Explanation

The description explains the implementation, affected components, reviewer focus, validation results, and linked issues. It uses different headings from the template but provides the required information, including the required issue reference.

Full details: Docstring Coverage

Explanation

Docstring coverage is 42.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 10 files. (1 skipped: 1 too large.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[dynamo-review-agent]

This comment only narrates the immediately following ChatOutput destructuring and repeats the controller/HTTP ownership already documented on ChatController; it adds no non-obvious constraint.

🤖 AI Fix

Remove this comment.