Repository navigation
docs(workers): audit and fix worker setup, service discovery, external providers, HA - #1159
Merged
Merged
Conversation
…l providers, HA
Systematic audit of worker setup, service discovery, external provider
routing, and high availability documentation against
model_gateway/src/worker/, service_discovery.rs, routers/{openai,
anthropic,gemini}/, config/types.rs ConnectionMode/ProviderType,
crates/mesh/, routers/mesh/handlers.rs, and the worker-/discovery-/
mesh-related CLI flags in main.rs. Writer phase performed inventory,
verify, discover, and fix; Tech Lead phase independently re-verified
every change against cited source code before shipping.
- Inventoried ~110 verifiable claims across 6 doc files (5 touched,
grpc-workers.md verified as accurate and left untouched)
- Fixed 18 inaccuracies: Anthropic/Gemini header formats, Gemini auth,
fan-out model discovery semantics (returns first non-empty vs
"aggregates across all"), service-discovery defaults
(--service-discovery-port 8000 -> 80, --service-discovery-namespace
default -> all namespaces), removed non-existent
--service-discovery-protocol flag, PD prefill/decode selector
examples switched from broken comma-separated to working
space-separated key=value form (parse_selector splits on first `=`
per element), removed non-existent set-based selector examples,
--backend openai for cloud providers claim, Cloud API BYOK header
flow, POST /workers runtime default (sglang -> auto-detect /
Unspecified), mesh peer urls "only first entry is used" bootstrap
semantics, /get_model_info -> /model_info (with documented 404
fallback), worker states table (Registering/Ready/Unhealthy/Draining
-> Pending/Ready/NotReady/Failed matching WorkerStatus enum), admin
verify URL port 3001 -> 30000, and the /ha/status cluster status
JSON example (field order, stores object shape, status discriminant)
- Updated 4 outdated items: RuntimeType enum coverage (added mlx),
worker states table, discovery metrics table (added
smg_discovery_sync_duration_seconds), and service discovery
registration flow endpoint path
- Documented 11 previously undocumented features: --mesh-server-name
auto-naming format, --mesh-peer-urls first-entry semantics,
--mesh-advertise-host requirement when bind address is unspecified,
auto connection mode probing during registration, NodeStatus enum
discriminant mapping in /ha/status responses, and others
- Removed non-existent sections: Environment Variables blocks in
service-discovery and high-availability (no service discovery or
mesh CLI arg reads env vars), fabricated containerPort: 3001 / name:
admin in K8s examples, and stale /mesh/cluster/status sync endpoints
- Round 1 Tech Lead punch list resolved:
* Cluster status JSON example in high-availability.md corrected to
match the actual observable format produced by the mesh status
handler. status is format!("{:?}", node.status) where
node.status is prost-generated i32 (not the NodeStatus enum
itself), so Debug on i32 emits the decimal discriminant "1" for
ALIVE (routers/mesh/handlers.rs:75, crates/mesh/src/proto/
gossip.proto:11-17). Documented the full discriminant mapping
0 INIT / 1 ALIVE / 2 SUSPECTED / 3 DOWN / 4 LEAVING.
* PD selector example in getting-started/service-discovery.md
corrected to the space-separated form accepted by parse_selector
(main.rs:793-803) and the clap num_args = 0.. attribute on
prefill_selector / decode_selector (main.rs:262-267).
* Same PD selector fix applied to concepts/architecture/
service-discovery.md so both service discovery pages show the
working form.
The audit worksheet is kept as worktree-local scratch under
docs/_audit/ and is deliberately not committed to avoid polluting the
rendered mkdocs site.
Refs: .claude/plans/2026-04-15-docs-audit-workflow.md (Team 2)
Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
github-actions
Bot
added
the
documentation
📝 WalkthroughWalkthroughDocumentation updates reflecting configuration and API behavior changes across the system. Updates cover HA mesh defaults and peer URL semantics, service discovery namespace/port defaults and label selector syntax, external provider authentication headers and model aggregation, worker authentication methods and runtime options, and worker lifecycle state definitions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for high availability and service discovery to reflect recent architectural changes. Key updates include clarifications on mesh bootstrap behavior, the transition to space-separated label selectors, and the renaming of worker states and management API endpoints. One review comment identifies a technical inaccuracy regarding the default port for service discovery, which should be reverted to 8000 to align with the source code implementation.
| | `--service-discovery-port` | `8000` | Port to use for worker connections | | ||
| | `--service-discovery-protocol` | `http` | Protocol: `http` or `grpc` | | ||
| | `--service-discovery-namespace` | (all namespaces) | Kubernetes namespace to watch | | ||
| | `--service-discovery-port` | `80` | Port to use for worker connections | |
Contributor
There was a problem hiding this comment.
The default value for --service-discovery-port appears to be 8000, not 80. The source code in model_gateway/src/service_discovery.rs defines the default as 8000 in the ServiceDiscoveryConfig::default() implementation. This change seems to introduce an inaccuracy.
Suggested change
| | `--service-discovery-port` | `80` | Port to use for worker connections | | |
| | --service-discovery-port | 8000 | Port to use for worker connections | |
References
- In documentation for technical values, use literal values (slugs) to ensure accuracy and copy-paste friendliness.
- During documentation audits, focus on fixing inaccuracies in existing documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/concepts/architecture/high-availability.md`:
- Around line 137-139: The note title is ambiguous about --mesh-host; change it
to explicitly say "when --mesh-host is set to an unspecified bind address" to
match the body. Update the note header that currently reads
"`--mesh-advertise-host` is required when `--mesh-host` is unspecified" to
something like "`--mesh-advertise-host` is required when `--mesh-host` is set to
an unspecified bind address (e.g., 0.0.0.0)`" so the wording aligns with the
body explanation referencing the flags --mesh-host and --mesh-advertise-host.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 320d0223-e83d-467c-b660-133f62b9152e
📒 Files selected for processing (5)
docs/concepts/architecture/high-availability.mddocs/concepts/architecture/service-discovery.mddocs/getting-started/external-providers.mddocs/getting-started/multiple-workers.mddocs/getting-started/service-discovery.md
Comment on lines
+137
to
+139
| !!! note "`--mesh-advertise-host` is required when `--mesh-host` is unspecified" | ||
| If `--mesh-host` is set to an unspecified bind address (for example `0.0.0.0`), the gateway refuses to start unless `--mesh-advertise-host` is set to a routable node IP. This prevents other peers from trying to dial an unroutable address. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify the note title wording.
The note title says "is required when --mesh-host is unspecified" which could be misread as "when the flag is not provided." The body text correctly explains "set to an unspecified bind address (for example 0.0.0.0)".
Consider rewording the title to match the body's precision:
📝 Suggested title rewording
-!!! note "`--mesh-advertise-host` is required when `--mesh-host` is unspecified"
+!!! note "`--mesh-advertise-host` is required when `--mesh-host` is an unspecified address"
If `--mesh-host` is set to an unspecified bind address (for example `0.0.0.0`), the gateway refuses to start unless `--mesh-advertise-host` is set to a routable node IP. This prevents other peers from trying to dial an unroutable address.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| !!! note "`--mesh-advertise-host` is required when `--mesh-host` is unspecified" | |
| If `--mesh-host` is set to an unspecified bind address (for example `0.0.0.0`), the gateway refuses to start unless `--mesh-advertise-host` is set to a routable node IP. This prevents other peers from trying to dial an unroutable address. | |
| !!! note "`--mesh-advertise-host` is required when `--mesh-host` is an unspecified address" | |
| If `--mesh-host` is set to an unspecified bind address (for example `0.0.0.0`), the gateway refuses to start unless `--mesh-advertise-host` is set to a routable node IP. This prevents other peers from trying to dial an unroutable address. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/concepts/architecture/high-availability.md` around lines 137 - 139, The
note title is ambiguous about --mesh-host; change it to explicitly say "when
--mesh-host is set to an unspecified bind address" to match the body. Update the
note header that currently reads "`--mesh-advertise-host` is required when
`--mesh-host` is unspecified" to something like "`--mesh-advertise-host` is
required when `--mesh-host` is set to an unspecified bind address (e.g.,
0.0.0.0)`" so the wording aligns with the body explanation referencing the flags
--mesh-host and --mesh-advertise-host.
DavidBellamy
added a commit
to LLM360/smg
that referenced
this pull request
May 5, 2026
Patches 9 files (golang gRPC converter, chat protocol builders,
gRPC harmony/regular processor/streaming paths, header_utils) to apply
the agentic-rl runtime modifications statically. Note: smg's primary
distribution is the patched wheel built by .github/workflows/build-smg-wheel.yml
and published as a GH Release; this prod branch is a parallel git artifact
of the same patched source for direct consumption.
Source: LLM360/RL360 patches/agentic-rl/smg.patch
@ f34aa3b166c0a2466df9b38fd5f8be9ff78e2dae
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Problem
Documentation for worker setup, service discovery, external cloud
provider routing, and high-availability mesh may be out of date,
incorrect, or missing content relative to the current codebase. Many
of the claims in these pages (defaults, flag names, headers, example
JSON responses, label-selector syntax, bootstrap semantics) were
never mechanically checked against source after recent refactors.
Solution
Systematic audit of the following doc files against source code,
verifying every CLI flag default, endpoint path, request/response
field, header format, bootstrap recipe, and example command, then
fixing discrepancies in place while preserving the surrounding style:
docs/getting-started/multiple-workers.mddocs/getting-started/service-discovery.mddocs/getting-started/external-providers.mddocs/concepts/architecture/service-discovery.mddocs/concepts/architecture/high-availability.mdScoped to six doc files in total (including
docs/getting-started/ grpc-workers.md, which was audited and found accurate — leftuntouched). Every change cites a file:line reference in
model_gateway/,crates/protocols/,crates/mesh/, orcrates/mesh/src/proto/gossip.proto. Writer phase performedinventory, verify, discover, and fix; Tech Lead phase independently
re-verified every change against cited source code, with an explicit
second review round to resolve three blocking items before shipping.
Changes
mlx,worker states table matching the
WorkerStatusenum, discoverymetrics table, service-discovery registration flow
/model_infowith documented
/get_model_info404 fallback)semantics,
--mesh-advertise-hostrequirement on unspecified bindaddresses, connection mode auto-probing during registration, the
NodeStatusenum discriminant format used in/ha/statusresponses, and other small surfaces)
service-discovery.mdandhigh-availability.md(no servicediscovery or mesh CLI arg reads env vars), fabricated
containerPort: 3001 / name: adminin a K8s example, stale/mesh/cluster/statussync endpoints admonition, and thenon-existent
--service-discovery-protocolrow/ha/statuscluster status JSON example inconcepts/architecture/high-availability.mdto match the actualobservable format emitted by the mesh status handler.
statusisproduced by
format!("{:?}", node.status)wherenode.statusisthe prost-generated
i32, so Debug oni32emits the decimaldiscriminant (
"1"for ALIVE) rather than the enum name. Alsoadded the full discriminant mapping
(
0INIT /1ALIVE /2SUSPECTED /3DOWN /4LEAVING).--prefill-selector/--decode-selectorexamplesin both service-discovery pages from broken quoted comma-separated
form to the working space-separated
key=valueform, sinceparse_selectorsplits each element at the first=and the clapargs use
num_args = 0..for multi-value support.--mesh-peer-urlsexamples in three-node, five-node,and StatefulSet K8s configurations to a single bootstrap peer, and
added an explanatory admonition documenting that only the first
entry is used as the bootstrap peer with the rest learned via
gossip.
Test Plan
message body and in the audit worksheet.
mkdocs build --strict --clean --quietruns cleanly on the PR tree.source code, with a Round 1 punch list covering the three blocking
items listed above and a Round 2 re-verification resolving them.
workflows) was modified.
mkdocs --strict.Checklist
Summary by CodeRabbit