Skip to content

docs(routing): audit and fix PD disaggregation, routing policies, gRPC pipeline - #1154

Merged
slin1237 merged 1 commit into
mainfrom
docs/audit-team-3-routing
Apr 15, 2026
Merged

slin1237 merged 1 commit into
mainfrom
docs/audit-team-3-routing

Conversation

@slin1237

@slin1237 slin1237 commented Apr 15, 2026 •

Copy link
Copy Markdown
Member

Description

Problem

Documentation for PD disaggregation, routing policies, and the gRPC
pipeline had drifted from the current model_gateway codebase. The
cache-aware routing description used the wrong threshold operator and
the wrong fallback branch; the gRPC pipeline "monitoring metrics" table
listed four metrics that are not registered anywhere in the binary;
the reasoning/tool-call parser docs referenced SMG_* environment
variables that clap does not actually read; and the service-discovery
examples for PD mode used a --prefill-selector "k1=v1,k2=v2" form
that is silently wrong because parse_selector splits on the first
= only.

Solution

Systematic audit of 6 documentation files in this area against the
source code, verifying every CLI flag, default, metric name, env var,
and routing-behavior claim. 6 fixes applied across 4 files. Every
change cites a file.rs:LINE reference in the commit body. A Tech
Lead pass independently re-verified each change against cited source
code before this PR was opened.

Changes

  • Verified ~90 claims across 6 doc files (cache-aware, prefix_hash,
    manual, consistent_hashing, bucket, power_of_two, round_robin,
    random policies; PD disaggregation flags; reasoning + tool-call
    parser tables; pipeline metrics).
  • Fixed 6 inaccuracies across 4 files:
    • docs/concepts/routing/cache-aware.md (1 edit): match-rate threshold
      uses strict > and fallback is least-loaded healthy worker
      (model_gateway/src/policies/cache_aware.rs:802, 811, 863, 872).
    • docs/getting-started/load-balancing.md (1 edit): same correction
      in the --cache-threshold table description.
    • docs/concepts/architecture/grpc-pipeline.md (3 edits):
      • Replaced four phantom metric rows
        (smg_pipeline_stage_duration_seconds,
        smg_reasoning_extractions_total, smg_tool_calls_total,
        smg_tool_execution_duration_seconds) with the real
        smg_router_stage_duration_seconds
        (observability/metrics.rs:190, 622). Kept the verified
        smg_mcp_tool_calls_total row.
      • Removed the Environment rows for SMG_REASONING_PARSER and
        SMG_TOOL_CALL_PARSER from the reasoning/tool-call parser
        configuration tables. The --reasoning-parser /
        --tool-call-parser clap args (main.rs:455-461) carry no
        env = ... attribute, and a repo-wide grep confirmed no code
        path reads those env vars.
    • docs/concepts/routing/pd-disaggregation.md (2 edits): rewrote
      both occurrences of --prefill-selector "app=sglang,role=prefill"
      to the space-separated form --prefill-selector app=sglang role=prefill.
      The parse_selector function (main.rs:793-803) splits each
      selector value on its first = only, and the flag declares
      num_args = 0.. (main.rs:262, 266), so the comma-joined form
      silently drops the second label. The corrected form matches the
      already-correct usage in
      docs/concepts/architecture/high-availability.md:455.
  • No source code modified.
  • Two files in scope (docs/getting-started/pd-disaggregation.md,
    docs/concepts/routing/load-balancing.md) needed no edits after
    full verification. Their claims about policy enums, PD CLI flags,
    headers, assignment modes, virtual nodes, and the HashRing
    signature (worker/hash_ring.rs:44-48) all match the current code.

Test Plan

  • Every fix cites a source code reference (file.rs:LINE) in the
    commit body and in the Tech Lead review notes.
  • mkdocs build --strict --clean --quiet exits 0 at HEAD on this
    branch (pre-existing unrelated anchor warnings in
    getting-started/tokenization-and-parsing.md remain, untouched by
    this PR).
  • Tech Lead phase independently re-verified each edit against the
    cited source code in model_gateway/src/policies/cache_aware.rs,
    model_gateway/src/observability/metrics.rs,
    model_gateway/src/main.rs (parse_selector, parser args), and
    repo-wide grep for the removed metric names and env vars.
  • No source code was modified.
Checklist
  • Documentation updated
  • (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

Summary by CodeRabbit

Documentation

  • Updated gRPC pipeline architecture documentation with revised configuration and monitoring guidance
  • Clarified cache-aware routing behavior, including threshold comparisons and worker selection logic
  • Updated Kubernetes label selector syntax examples for disaggregated prefill/decode setup
  • Refined cache-threshold parameter documentation for improved load-balancing clarity

…C pipeline

Systematic audit of PD disaggregation, routing policy, and gRPC
pipeline documentation against model_gateway/src/policies,
routers/http/pd_router, routers/grpc/common/stages, worker/hash_ring,
config/types RoutingPolicy, and the --policy / --pd-disaggregation /
--prefill / --decode CLI flags. Writer phase performed inventory,
verify, discover, and fix; Tech Lead phase independently re-verified
every change against cited source code before shipping.

- Verified ~90 claims across 6 doc files; 6 fixes applied across 4 files.
- cache-aware threshold: doc said "match ratio >= cache-threshold" and
  "most cache capacity" fallback; code at
  model_gateway/src/policies/cache_aware.rs:802,863 uses strict `>` and
  falls back to least-loaded healthy worker via
  `min_by_key(|&&idx| workers[idx].load())` at lines 811, 872. Corrected
  in docs/concepts/routing/cache-aware.md and
  docs/getting-started/load-balancing.md.
- grpc-pipeline monitoring metrics table listed four metrics that are
  not registered anywhere (`smg_pipeline_stage_duration_seconds`,
  `smg_reasoning_extractions_total`, `smg_tool_calls_total`,
  `smg_tool_execution_duration_seconds`). The real pipeline-stage metric
  is `smg_router_stage_duration_seconds`
  (model_gateway/src/observability/metrics.rs:190, 622). Replaced the
  phantom rows with the real metric; kept `smg_mcp_tool_calls_total`
  which is registered at metrics.rs:306, 1100.
- grpc-pipeline Reasoning/Tool Call parser "Configuration" tables
  claimed `SMG_REASONING_PARSER` / `SMG_TOOL_CALL_PARSER` environment
  variables. The `--reasoning-parser` and `--tool-call-parser` args in
  model_gateway/src/main.rs:455-461 carry no `env = ...` clap attribute
  and no other code path reads those env vars. Removed the "Environment"
  rows.
- pd-disaggregation service-discovery examples used
  `--prefill-selector "app=sglang,role=prefill"`. The
  `parse_selector` function in model_gateway/src/main.rs:793-803 splits
  each selector value on its first `=` only, and the flag declares
  `num_args = 0..` (main.rs:262). With the comma-joined form, the
  second label is silently dropped. Rewrote both occurrences in
  docs/concepts/routing/pd-disaggregation.md to use the space-separated
  form (matching the already-correct usage in
  docs/concepts/architecture/high-availability.md:455).

No source code was modified. The per-team audit worksheet is kept as
worktree-local scratch and is not committed to avoid polluting the
rendered mkdocs site.

Refs: .claude/plans/2026-04-15-docs-audit-workflow.md (Team 3)
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
@slin1237
slin1237 requested a review from CatherineSue as a code owner April 15, 2026 19:09
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Apr 15, 2026
@coderabbitai

coderabbitai Bot commented Apr 15, 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: 72c0ba08-53a4-418c-9d5c-ae2d1e47f635

📥 Commits

Reviewing files that changed from the base of the PR and between 5caa51c and 3455b6d.

📒 Files selected for processing (4)
  • docs/concepts/architecture/grpc-pipeline.md
  • docs/concepts/routing/cache-aware.md
  • docs/concepts/routing/pd-disaggregation.md
  • docs/getting-started/load-balancing.md

📝 Walkthrough

Walkthrough

Four documentation files updated: gRPC pipeline configuration and monitoring metrics documentation revised; cache-aware routing threshold logic and fallback behavior clarified; Kubernetes label selector CLI examples reformatted from comma-separated to space-separated; and cache-aware policy parameter description updated to reflect new least-loaded fallback behavior.

Changes

Cohort / File(s) Summary
Pipeline Architecture
docs/concepts/architecture/grpc-pipeline.md
Removes environment variable configuration entries for reasoning and tool call parsers; replaces multiple pipeline stage and parsing metrics with a single router-focused stage duration metric.
Routing & Load-balancing
docs/concepts/routing/cache-aware.md, docs/concepts/routing/pd-disaggregation.md, docs/getting-started/load-balancing.md
Updates cache-aware routing threshold comparisons (≥ to >, < to ≤) and changes fallback selection from most-cache-capacity to least-loaded-healthy-worker; reformats Kubernetes label selector CLI examples from comma-separated to space-separated format; clarifies cache-threshold parameter description.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

Suggested labels

documentation

Suggested reviewers

  • CatherineSue
  • key4ng

Poem

🐰 Hops through docs with glee,
Metrics refined, thresholds set free,
Labels arranged in space, not commas tight,
The routing runs swift, the pipeline runs 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 accurately summarizes the main changes: a documentation audit and fixes affecting routing policies, PD disaggregation, and gRPC pipeline documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/audit-team-3-routing

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 pull request updates the documentation to reflect changes in environment variables, metrics, and routing logic. Key changes include the removal of deprecated environment variables and metrics in the gRPC pipeline documentation, and an update to the cache-aware routing logic which now routes to the least-loaded healthy worker when the match ratio is at or below the threshold. Additionally, the CLI syntax for prefill and decode selectors has been updated to remove quotes and commas. I have no feedback to provide.

@slin1237
slin1237 merged commit 033df18 into main Apr 15, 2026
8 checks passed
@slin1237
slin1237 deleted the docs/audit-team-3-routing branch April 15, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant