docs: Update structural docs for LiteLLM upstream routing [doc-updater] - #2776
Conversation
Four structural docs were missing coverage of the gateway upstream router and LiteLLM deployment introduced in #2773 (slice-1/2 of #2769): - sdlc-pipeline.md: Add agent_models to PipelineConfig field table - credential-injection.md: Add LITELLM_MASTER_KEY auth type; add upstream_registry.py to the files table; update intro sentence - resource-sizing.md: Add litellm pod row to allocations table - kubernetes-migration.md: Add litellm Deployment to namespace diagram The dedicated feature docs (docs/guides/per-agent-models.md and docs/architecture/upstream-routing.md) were added in the same commit and are already linked from docs/index.md.
There was a problem hiding this comment.
No agent-mode design concerns. This is a doc-only PR updating structural/reference docs for the LiteLLM upstream routing feature — no prompts, workflows, or agent-invocation paths are touched. The agent_models documentation correctly uses model aliases (opus, sonnet, haiku); the non-Claude example (qwen3-coder-30b) is a genuine LiteLLM upstream model name, not a pinned Claude identifier, so no EGG201 concern.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review: PR #2776 — docs update for LiteLLM upstream routing
Doc-only PR backfilling four structural docs after #2773. I verified every documented claim against the source. Most are accurate, but the PR ships one internal contradiction it directly introduced.
Blocking
1. docs/deploy/resource-sizing.md — stale "three pod types" count (two occurrences)
The PR adds a 4th row (litellm) to the allocations table but leaves the surrounding prose saying "three":
- Line 3:
Resource requests and limits for the **three** pod types in the egg stack... - Line 45:
All **three** pod types are **Burstable**...
After this PR the table lists four pod types — gateway, orchestrator, litellm, egg-sandbox-* — and all four rows are Burstable. Both prose references must become "four". As shipped, the doc internally contradicts itself: prose says three, table shows four. For a PR whose stated purpose is doc accuracy ("the canonical reference operators consult"), an internal contradiction in the section being modified is a correctness defect, not a nit.
While fixing this, also note the line-3 clause "...with the observed telemetry they were tuned against": the litellm row has no entry in the "Observed usage" snapshots (those predate the deployment), and the row was sized from k8s/base/litellm-deployment.yaml, not telemetry. The "tuned against telemetry" claim no longer holds for all four rows — consider rewording so it doesn't over-claim for litellm.
Non-blocking
2. docs/architecture/kubernetes-migration.md — # optional annotation is imprecise
The added line reads:
└── Deployment: litellm (+ Service :4000) # optional; non-Claude model proxy (#2769)
litellm-deployment.yaml, litellm-service.yaml, and litellm-configmap.yaml are all unconditionally listed in k8s/base/kustomization.yaml resources:, so the Deployment and pod always exist when the base manifests are applied — it is not optional. What is inert-by-default is the routing (no agent reaches LiteLLM until agent_models is set / the ConfigMap model_list is populated). This also mildly contradicts resource-sizing.md, which (correctly) lists litellm as a standard always-present allocation. Suggest rewording to match the kustomization comment, e.g. # non-Claude model proxy; no-op until model_list populated (#2769).
3. docs/architecture/kubernetes-migration.md — second egg-system diagram not updated
The same file has a second diagram (Network Isolation → "Kubernetes Model (After)", ~line 138) that depicts the egg-system namespace with only orchestrator and gateway. The PR updated only the first ("After (Kubernetes)") diagram. This is defensible — the second diagram shows network flow (agent isolation boundary), not a Deployment inventory — so omitting litellm there is reasonable. Flagging only for completeness; no change required unless you want both egg-system diagrams to agree.
Verified accurate (no action needed)
credential-injection.mdintro + auth-table row:LITELLM_MASTER_KEYfromsecrets.env, injected asx-api-key, LiteLLM-upstream-only — matchesgateway/anthropic_credentials.py(LiteLLMCredentialsManager).credential-injection.mdfiles table:upstream_registry.pydescription ((httpx.Client, credential_resolver)pairs, lazy init,upstreamvalidation on session-create) matchesgateway/upstream_registry.pyand theis_known()check atgateway/session_manager.py:622. Theanthropic_credentials.pyrow update is correct.kubernetes-migration.md:litellmDeployment +Service :4000matcheslitellm-service.yaml(single port 4000).resource-sizing.mdlitellmrow values100m / 1 / 256Mi / 1Gi / Burstablematchlitellm-deployment.yaml(requests cpu 100m/mem 256Mi, limits cpu 1/mem 1Gi →request < limit→ Burstable).sdlc-pipeline.mdagent_modelsrow: typedict[str, str], default{}, role-key restriction, Claude-alias-vs-LiteLLM routing, and construction-time rejection of utility roles all matchorchestrator/models.py:757-806(PipelineConfig.agent_models+_validate_agent_models_roles). Linked docsper-agent-models.mdandupstream-routing.mdboth exist.
Verdict
One blocking fix required: update the two "three pod types" references in resource-sizing.md to "four" (and adjust the telemetry clause). Items 2–3 are non-blocking suggestions.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Address review feedback on #2776: resource-sizing.md said 'three pod types' in two places after the table grew to four rows; both now say 'four'. The intro telemetry clause no longer over-claims for the litellm row, which was sized from the deployment manifest rather than tuned against the observed snapshots. The kubernetes-migration.md litellm annotation drops the inaccurate 'optional' wording (the Deployment is unconditionally listed in kustomization.yaml) in favour of the no-op-until-model_list framing used in the kustomization comment.
Review feedback addressedThanks for the careful verification. Disposition per item: 1. 2. 3. Verified-accurate section — no action needed; thanks for the source cross-checks. — Authored by egg |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
No agent-mode design concerns.
Re-review of the delta since my last review (00066e93b..ea98a8872): two doc-prose corrections only — resource-sizing.md updates "three pod types" → "four" and reworks the telemetry clause; kubernetes-migration.md rewords the litellm annotation from # optional to # non-Claude model proxy; no-op until model_list populated. No prompts, workflows, or agent-invocation paths touched. Model aliases still used correctly. Approving.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review: PR #2776 — docs update for LiteLLM upstream routing
Re-reviewed the delta since commit 00066e93 (one new commit, ea98a88). The change is 3 lines across 2 docs files, all addressing my prior review feedback. Verified every change against source.
Previous blocking item — resolved
docs/deploy/resource-sizing.md — stale "three pod types" count
Both occurrences now correctly say "four":
- Line 3 intro — matches the 4-row allocations table (
gateway,orchestrator,litellm,egg-sandbox-*). - Line 45 QoS rationale — all four table rows are
Burstable, so "All four pod types are Burstable" is accurate.
The telemetry over-claim I flagged is also fixed. The reworded line-3 clause attributes the observed telemetry only to gateway/orchestrator/egg-sandbox-* — confirmed: the "Observed usage" tables contain exactly those three pods and no litellm row. The litellm row is correctly described as sized from k8s/base/litellm-deployment.yaml; that file exists and its resources: block (requests 100m/256Mi, limits 1/1Gi) matches the allocations-table row, and request < limit confirms Burstable.
Previous non-blocking item — resolved
docs/architecture/kubernetes-migration.md — # optional annotation
The annotation now reads # non-Claude model proxy; no-op until model_list populated (#2769). Verified k8s/base/kustomization.yaml unconditionally lists litellm-configmap.yaml, litellm-deployment.yaml, and litellm-service.yaml under resources:, so the Deployment is not optional — the new wording is accurate and matches the kustomization comment (no-op until operators populate the ConfigMap's model_list). The prior contradiction with resource-sizing.md (which lists litellm as a standard always-present allocation) is removed.
Previous non-blocking item — author declined, acceptable
The second egg-system diagram (Network Isolation) was left unchanged. As I noted in my prior review, that diagram depicts network flow / the agent isolation boundary rather than a Deployment inventory, so omitting litellm there is correct. No change needed.
Verdict
All blocking feedback resolved; non-blocking items appropriately handled. No new issues introduced. Approving.
— Authored by egg
|
egg review completed. View run logs 5 previous review(s) hidden. |
Update documentation to reflect changes from 8f0c980 (#2773):
docs/guides/sdlc-pipeline.md: Addagent_modelsfield to the PipelineConfig table; it was missing despite the field being user-facing and the table being the canonical reference operators consult when submitting pipelinesdocs/architecture/credential-injection.md: AddLITELLM_MASTER_KEYrow to the Authentication Types table; addupstream_registry.pyto the Files table; update intro sentence to mention LiteLLM as a covered credential classdocs/deploy/resource-sizing.md: Addlitellmpod row (100m/1 CPU, 256Mi/1Gi mem) matchingk8s/base/litellm-deployment.yamldocs/architecture/kubernetes-migration.md: AddlitellmDeployment to theegg-systemnamespace diagramThe dedicated feature docs (
docs/guides/per-agent-models.mdanddocs/architecture/upstream-routing.md) were added in the same merged commit and are already linked fromdocs/index.md; this PR only patches the structural/reference docs that pre-date the feature and were not updated inline.Triggered by: #2773
Test Plan
orchestrator/models.pyforagent_models,k8s/base/litellm-deployment.yamlfor resource sizing,gateway/upstream_registry.pyfor credential-injection files table)Authored-by: egg