Repository navigation
feat: add switchyard_model server for router-backed evals with condition provenance - #2141
Conversation
|
🌿 Preview your docs: https://nvidia-preview-feat-switchyard-model-server.docs.buildwithfern.com/nemo/gym Here are the markdown pages you've updated: |
github-actions
Bot
added
the
sla:triage-overdue
github-actions
Bot
added
sla:review-overdue
1621621 to
8cdac1e
Compare
|
Live end-to-end validation of the native-server revision (nemo-switchyard==0.2.0 wheel, hosted in-process): Setup: What was verified against live traffic:
One upstream observation from the run: Switchyard 0.2.0 forwards the 🤖 Generated with Claude Code |
Test results
What the suite covers, by layer:
What writing the real-wheel tests caught (both fixed in this PR):
The full-chain tests skip when Live end-to-end rollout results (real upstream, reward 1.0, live session correlation) are in the comment above. 🤖 Generated with Claude Code |
e9f7192 to
748acd9
Compare
linj-glitch
changed the title
| a copy with the session header merged in. The copy is a plain Pydantic model over Gym's | ||
| shared aiohttp client, so this costs an object allocation, not a connection. | ||
| """ | ||
| rollout_id = _ROLLOUT_ID.get() |
There was a problem hiding this comment.
This model server gets the rollout ID from the /ng-rollout/<id>/ URL prefix. BaseResponsesAPIAgent adds that prefix only when observability or token capture is enabled. In an ordinary eval with both features disabled, the request is unprefixed, _ROLLOUT_ID remains None, and no Switchyard session header is sent. As a result, the documented rollout correlation and session-affinity behavior depend on enabling an unrelated feature.
@Glorf IMO we should propagate rollout identity independently of capture: #2783
@linj-glitch for now, this requirement should be explicit and validated at startup
There was a problem hiding this comment.
Validated at startup in 5ac91a5: check_rollout_correlation reads the same global-config keys the agents consult. Under the default config it logs a warning naming the fix; when forward_session_id was set explicitly and no capture is enabled, the server refuses to start. Verified live in both directions — an explicit-forwarding run without observability refused with the documented message, and an observability run showed session_id 0-0 in Switchyard's request log, matching the rollout row's indexes. This surfaced a follow-on bug: the shipped yaml restated the default, which made every run look explicit; fixed in 66f877f with a regression test that reads the yaml. Agreed that propagating rollout identity independently of capture is the better long-term shape.
| deployment_sha256 = hashlib.sha256(deployment_bytes).hexdigest() | ||
| # The native schema references credentials by environment variable name, but an | ||
| # inline literal would be a secret -- drop the value, keep the line visible. | ||
| deployment_toml = re.sub( |
There was a problem hiding this comment.
The manifest archives the deployment TOML, but this redaction covers only lines named api_key.
Valid Switchyard deployments can store credentials in llm_clients.*.extra_headers, such as an Authorization header (upstream schema). Those values do not match this expression and are copied unchanged into switchyard-condition.json.
Please redact all extra_headers values after parsing the TOML, or avoid storing the raw deployment.
There was a problem hiding this comment.
Fixed in 5ac91a5. The archive is now redacted by parsing the TOML and blanking every extra_headers value wherever it appears (including inside arrays of tables) plus any api_key value; a deployment that does not parse cannot be checked, so it is not archived at all and only its hash is kept. Verified live: a deployment with a planted Authorization-style extra_headers value came out of the manifest as with the api_key_env reference intact. Confirmed while testing that the 0.2.0 client schema has no inline api_key field at all — extra_headers is exactly where a live secret would sit.
| return NeMoGymChatCompletion.model_validate(response_dict) | ||
|
|
||
|
|
||
| if __name__ == "__main__": |
There was a problem hiding this comment.
When num_workers > 1, Gym starts Uvicorn with an import string and expects this module to expose an app object (server startup). This module only starts the server under if __name__ == "__main__", so a multi-worker configuration fails during startup. Adding the normal import guard is sufficient for attached mode, where all workers can share one external proxy. In hosted mode, however, each worker would start a separate proxy, splitting session affinity and statistics and potentially colliding on a fixed port.
Please add the worker entrypoint for attached mode and reject hosted configurations with more than one worker unless the proxy is shared explicitly.
There was a problem hiding this comment.
Done in 5ac91a5: the module now exposes the worker entrypoint (elif is_nemo_gym_fastapi_entrypoint), so attach mode works under num_workers > 1 with all workers sharing the external proxy, and hosted configurations with more than one worker are rejected at config validation with guidance to attach instead. Verified live: a hosted num_workers=2 run failed fast with the validation message.
| if not self.config.condition_dir or root_url is None: | ||
| return | ||
| try: | ||
| with urllib.request.urlopen(f"{root_url}/v1/stats", timeout=5) as response: |
There was a problem hiding this comment.
The async lifespan calls synchronous urllib.request.urlopen() during shutdown. If the proxy is slow or unreachable, this blocks the event loop for up to five seconds, while Gym waits only one second before escalating to SIGKILL (shutdown window). This request also omits switchyard_api_key and default_headers, so an authenticated attached proxy can serve model calls successfully but return 401 for the stats snapshot. Please use Gym's shared async HTTP path, send the configured authentication headers, and keep the request within the shutdown budget.
There was a problem hiding this comment.
Fixed in 5ac91a5. The snapshot now goes through nemo_gym.server_utils.request on the shared aiohttp client, sends the configured default_headers plus the switchyard_api_key as the bearer token (same identity as the model path), and is hard-capped at 0.5s with asyncio.wait_for — the outer cap also matters because the shared client retries connection errors indefinitely. Any failure inside the cap logs a warning and shutdown proceeds.
| extra_body: Dict[str, Any] = Field(default_factory=dict) | ||
| default_headers: Dict[str, str] = Field(default_factory=dict) | ||
|
|
||
| max_concurrent_requests: Optional[int] = Field( |
There was a problem hiding this comment.
nit: please validate the value as strictly positive when set with Field(default=None, gt=0).
There was a problem hiding this comment.
Done in 5ac91a5, with a validation test.
| "route": self.config.switchyard_model, | ||
| "mode": "hosted" if self.config.launches_proxy else "attached", | ||
| "proxy_root_url": self.proxy_root_url(), | ||
| "nemo_switchyard_version": self._nemo_switchyard_version(), |
There was a problem hiding this comment.
When Gym hosts the proxy, the locally installed nemo-switchyard version identifies the code serving requests. In attached mode, requests go to an external proxy, but this field still reports the local wheel even if the external proxy runs a different version or commit. The attached manifest also has no deployment hash, so it cannot identify or reproduce the routing condition that actually served the run.
Could attached mode record remote or caller-supplied proxy provenance?
There was a problem hiding this comment.
Fixed in 5ac91a5: attached manifests now report nemo_switchyard_version as null (the local wheel never served a request) and take identity from a new proxy_provenance config mapping copied verbatim into the manifest, so the proxy's operator can record the build commit and deployment hash. 0.2.0 exposes no version or config-identity endpoint to query remotely, so caller-supplied is the honest option available. Verified live: an attached run against a standalone proxy produced a manifest with the null version and the supplied provenance.
| deployment_sha256: Optional[str] = None | ||
| deployment_toml: Optional[str] = None | ||
| if self.config.launches_proxy: | ||
| deployment_bytes = Path(str(self.config.deployment)).read_bytes() |
There was a problem hiding this comment.
this Path.read_bytes() call is outside the error handler. If the deployment becomes unreadable after the native server loads it, the read raises and stops server startup instead of logging a warning.
The manifest is also written before the lifespan enters the cleanup try/finally, so the already-started proxy is not explicitly closed.
All manifest preparation should be in the error handler, and the cleanup handling should take effect immediately after starting the proxy
There was a problem hiding this comment.
Restructured in 5ac91a5: the lifespan's cleanup scope now begins the moment the proxy is serving, so anything failing between proxy start and finished startup still closes it, and all manifest preparation including the read_bytes sits inside the best-effort handler — an unreadable or unparseable deployment costs a warning (and, for unparseable, the archive), never startup.
| import json | ||
|
|
||
| for condition in ("strong-only", "policy-model"): | ||
| rows = [json.loads(line) for line in open(f"results/{condition}/rollouts.jsonl")] |
There was a problem hiding this comment.
The example undercounts the cost of the llm_classifier route. response.usage.total_tokens covers the selected model call, while the classifier model's tokens are reported separately in switchyard-stats.json.
The script also computes each average independently, so failures can cause the two conditions to contain different tasks. Please include classifier tokens, align rows by task and rollout ID, and report failures before comparing reward and token cost.
There was a problem hiding this comment.
Rewritten in eb5e34f. The example now joins the two runs' rollout rows on _ng_task_index and _ng_rollout_index, reports rollouts that have no counterpart before comparing, and reads the classifier tokens from the stats snapshot alongside the selected-model usage — with surrounding prose explaining both pitfalls. Verified the classifier accounting live: an llm_classifier-routed call showed its classifier-side tokens only in the stats snapshot's classifier block, not in the response usage.
| ++policy_model.responses_api_models.switchyard_model.condition_dir=results/policy-model | ||
| ``` | ||
|
|
||
| Each results directory now identifies its condition. `switchyard-condition.json` is the provenance manifest — the route, the deployment's SHA-256 and archived contents (credential values redacted), and the `nemo-switchyard` version — written when the proxy starts. `switchyard-stats.json` is the proxy's `/v1/stats` at shutdown: per-target requests, tokens, retries, and latency, which for a hosted proxy would otherwise die with the process. Two runs are fairly comparable when their manifests differ only in `route`; a run is reproducible from the archived deployment alone. |
There was a problem hiding this comment.
I don't see retry count in /v1/stats - is that a mistake?
There was a problem hiding this comment.
You are right — I dumped /v1/stats from the real 0.2.0 wheel and it reports requests, errors, tokens, and latency plus a classifier block, with no retry counts anywhere. Removed the claim from the docs page, README, and docstrings in eb5e34f.
| [llm_clients.nvidia] | ||
| format = "openai_chat" | ||
| base_url = "https://integrate.api.nvidia.com/v1" | ||
| api_key_env = "NVIDIA_API_KEY" |
There was a problem hiding this comment.
Please add # pragma: allowlist secret to address the secrets-detector test failures in CI
|
Live end-to-end verification of this review round (commits 5ac91a5, eb5e34f, 66f877f), run against a real hosted proxy and a real inference gateway rather than the test stubs. Hosted leg: Attach leg: a standalone native server on its own port, pre-warmed with one Failure legs: setting The exercise also caught one real bug, fixed in 66f877f: the shipped yaml restated |
Serves Gym model calls through a Switchyard routing proxy, so any benchmark
Gym already supports can be evaluated against a router without touching
harness code:
gym eval run --benchmark <name> --model-type switchyard_model
Switchyard is integrated at the model layer rather than the agent layer. The
existing Switchyard work in Gym (#2026 token capture, #2080 OpenCode) wires
one harness at a time under responses_api_agents/swe_agents; putting the
router behind a Responses API model instead makes it available to every
benchmark at once, which is what evaluating the router itself requires.
Two modes. Attach (default) points at a proxy run outside Gym, which keeps
proxy lifecycle and image pinning with the eval harness so a run can be
compared against a scaled-evals run of the same Switchyard commit. Launch
starts the proxy from a routing profile and shuts it down at exit, for a
single-command run.
Gym's rollout-attempt id is forwarded as Switchyard's opaque session id, so
proxy-side routing decisions and costs join back to the rollout that produced
them. The correlation prefix is stripped by Gym's capture middleware before
handlers run, so a thin outer middleware republishes the id for the request.
The nemo-switchyard dependency is scoped to this package rather than Gym's
core dependencies: only runs that route through Switchyard pay for it, and
attach mode needs no dependency at all.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Lin Jia <linj@nvidia.com>
This server never imports Switchyard. It speaks OpenAI-compatible HTTP to the proxy, and launch_proxy mode shells out to the `switchyard` CLI, which only needs the executable on PATH. The dependency bought nothing at import time and made the package unresolvable, since the published nemo-switchyard 0.1.0 pins openai>=2.34.0 against Gym's openai<=2.7.2. Leaving the install to whoever wants launch mode also lets an eval pin the exact Switchyard build its numbers are reported against, rather than inheriting Gym's. start_proxy now checks PATH up front and explains how to fix a missing executable, instead of surfacing an opaque FileNotFoundError from Popen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Lin Jia <linj@nvidia.com>
The point of opting into switchyard_model is not having to run a proxy. If the user has to launch one anyway, openai_model pointed at it already works and this server barely earns its place. So launching is now the default rather than a flag: set routing_profiles and Gym starts, health-checks and tears down the proxy for you. switchyard_base_url stays as the escape hatch for attaching to a proxy you run yourself. That removes the launch_proxy boolean -- the mode is inferred from whichever of the two you configured, and configuring neither now fails naming both. The spawn also moves from model_post_init into setup_webserver, matching local_vllm_model. Launching on construction meant any path that merely built the model -- config resolution, validation, tests -- would start a proxy; the tests having to stub Popen just to construct an instance was the tell. Regression test added. proxy_host now defaults to the wildcard address, as local_vllm_model does for cross-node reachability, while this server addresses its own child over loopback, since a wildcard is a bind address and not a destination. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Lin Jia <linj@nvidia.com>
Covers the Gym-managed proxy launch, attaching to a proxy you run yourself, that --model names a route rather than a model, rollout correlation via proxy_x_session_id, and the config fields. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Lin Jia <linj@nvidia.com>
The proxy was launched as a child process and reaped through atexit, which never ran: Gym's orchestrator stops a server with SIGINT and escalates to SIGKILL after one second, and uvicorn's signal handling ends the process without running interpreter exit hooks. Every orchestrated shutdown therefore left a switchyard process alive, still holding its port. A single leak is invisible; an eval fleet cycling model servers accumulates one per run. Cleanup now has two layers, because no single one covers both shutdowns. A lifespan wrapper stops the proxy when the app shuts down, which handles the graceful SIGINT path. That alone is not enough -- a SIGKILLed parent runs no handler at all -- so the spawn additionally asks the kernel, via prctl(PR_SET_PDEATHSIG), to SIGTERM the child when its parent dies by any means. prctl is Linux-only and degrades to a no-op elsewhere, where the lifespan hook remains the only line of defence. atexit is kept as a backstop. Verified against a live proxy: SIGINT, SIGKILL, and a full `gym eval run` all now leave no surviving proxy, where the same eval previously leaked one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Lin Jia <linj@nvidia.com>
The published 0.1.0 cannot serve this server's /v1/responses endpoint at all. Its chat->responses translation emits a usage object without the input_tokens_details and output_tokens_details members that the Responses schema marks required, so every response fails NeMoGymResponse validation and the endpoint returns 500. A real eval against integrate.api.nvidia.com failed on every rollout; /v1/chat/completions was unaffected, which is why this only surfaced for Responses-API agents. NVIDIA-NeMo/Switchyard#144 makes both members unconditional. It is merged but unreleased -- PyPI still carries only 0.1.0 -- so this pins the merge commit directly and should move to `>=0.1.1,<0.2.0` as soon as a release ships. Installing from git builds Switchyard's Rust extension through maturin, so this now requires a cargo toolchain on the installing machine. Gym's CI does not provide one, so `gym env test` for this server will not run there until the pin moves to a released wheel. That cost is accepted deliberately: the alternative is shipping a server whose Responses endpoint cannot serve a single request. The uv override that neutralized 0.1.0's stale openai>=2.34.0 floor is dropped with it. Switchyard declares openai>=2.7,<3.0 as of #142, which co-installs with Gym's ceiling directly -- verified by rebuilding the server venv from scratch, which resolves openai 2.7.2 with no override present. Verified end to end from a clean venv: 25 unit tests pass, and a `gym eval run` against a live model returns reward 1.0 rollouts carrying well-formed usage, where the same run previously failed on every rollout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Lin Jia <linj@nvidia.com>
The pin's comment said Gym's CI has no cargo toolchain and that `gym env test` for this server therefore would not run there. Both halves are wrong. CI classifies this PR as server-only and runs `ng_test` against this server, which builds its venv from this pyproject -- git dependency included. The runners carry a cargo toolchain, so maturin builds the Rust extension and the suite runs: 25 passed on the latest run. The real cost is build time. Bypassing the published cp312-abi3 wheels adds about a minute to a cold environment build, which is worth stating plainly so nobody reads the pin as untested in CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Lin Jia <linj@nvidia.com>
The previous note claimed the pin needs a cargo toolchain. It does not. maturin resolves puccinialin as a build requirement and bootstraps rustup when it finds no toolchain, so a user who has never installed Rust still gets a working install with no action on their part. Verified by building in a venv with PATH stripped of cargo, HOME/CARGO_HOME/RUSTUP_HOME pointed at an empty directory and a cold uv cache: the extension compiles and `import switchyard_rust` succeeds. What the pin does cost is worth saying instead, because neither is guessable from a failure. The first startup spends an extra minute or two compiling rather than unpacking a wheel. And the build fetches from static.rust-lang.org and crates.io -- Switchyard vendors no crates -- which are two hosts a Gym user has no other reason to allow. On a cluster with restricted egress that is where this fails, and the error surfaces from deep inside a build log. README and the docs page both said "nothing to install" without either caveat. Both now carry it, scoped to the git pin and noting that the self-managed proxy mode installs nothing at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Lin Jia <linj@nvidia.com>
Review pass over the whole diff. Five things, two in code. setup_proxy_shutdown stopped the proxy in a trailing statement after the wrapped lifespan's `async with`, so it only ran when that context exited cleanly. The proxy is already serving by the time the app starts, so a startup that raises left one behind -- the same leak f73e006 set out to close, through the one door it did not cover. It is a `finally` now, with a test that a failing lifespan still terminates the child. _set_parent_death_signal carried `# pragma: no cover - runs post-fork, in the child`. True of production, wrong about the tests: four of them call it directly and cover every branch. The pragma was hiding real coverage, so app.py counts 150 statements now rather than 142, still with one miss -- the `__main__` guard. The README still described the dependency as pinned `==0.1.0` with a `[tool.uv] override-dependencies` entry relaxing a stale openai floor. Both were replaced by the git pin in 474defd; neither exists. It now describes the pin that is actually there and why. The docs claimed the Rust build does not apply when you attach to your own proxy. It does: the dependency is unconditional, so the environment is built identically in both modes and attaching leaves the CLI unused rather than uninstalled. Corrected in the README and the docs page. The config comment told users to `pip install 'nemo-switchyard[server]'` for launch mode, left over from when this server had no such dependency. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Lin Jia <linj@nvidia.com>
…g it Merging main brought in nemo_gym.rollout_correlation (#2114), which makes the rollout-id charset explicit: RolloutContextMiddleware matches [A-Za-z0-9][A-Za-z0-9._-]* and _validate_rollout_id enforces the same set on the capture path. This server predates that contract and matched [^/]+. That is too permissive here specifically. ASGI hands over a percent-decoded path, and this server does something the core middleware does not -- it puts the id into an outbound HTTP header. A crafted prefix such as /ng-rollout/x%0d%0ax-injected:%201/v1/responses parses as a rollout id carrying CRLF under the old pattern. The client library rejects such a value, so the practical outcome was a failed request rather than a forged header, but relying on a downstream library to catch what this regex let through is the wrong place to draw the line. Matching the core charset means an off-contract id is simply not published: the call goes out uncorrelated, exactly as it does for a request that never carried a prefix. The unused `rest` capture group goes too; the path is forwarded untouched, so it was never read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Lin Jia <linj@nvidia.com>
…0 pin Switchyard 0.2.0 deprecates the Python `switchyard serve` stack this server launched as a subprocess, and upstream main has already removed it. Re-target the launch mode to the native Rust server hosted in-process (switchyard_rust.server.Server) from an explicit TOML deployment: - routing_profiles (YAML) becomes deployment (native TOML), the artifact that makes routed runs comparable; env override moves to SWITCHYARD_DEPLOYMENT - the subprocess machinery (prctl death signal, atexit, health polling, kill escalation, PATH checks) is deleted -- the in-process server binds at construction and cannot outlive its owner - nemo-switchyard moves from a git commit to an exact released pin (==0.2.0, wheels only, no extras), following the local_vllm_model pinning practice - attach mode (switchyard_base_url) is unchanged; the session-id header and /v1 endpoint contract are identical on the native server - tests: proxy lifecycle reworked against a fake native server, plus two integration tests that host the real 0.2.0 wheel from a real TOML deployment Signed-off-by: Lin Jia <linj@nvidia.com>
Adds a full-chain integration suite: Gym's FastAPI app -> real hosted native Switchyard server -> local stub upstream, asserting the responses<->chat translation (usage detail objects, the reason the pin starts at 0.2.0), route override, credential wiring, /v1/stats, and the session id landing in Switchyard's routing metadata. Writing those tests against the real wheel surfaced two fixes: - The session header contract changed: the native server parses x-switchyard-session-id into routing metadata; proxy_x_session_id is only read by switchyard-server's durable routing log (--routing-log-file, attach mode) and is otherwise forwarded verbatim to the upstream provider. The config knob becomes session_id_headers (a list, default [x-switchyard-session-id]) and the docs explain when to add the log name. - The PyO3 Server binding is thread-affine (unsendable): the thread that constructs it must close it. Hosting moves from setup_webserver into the lifespan wrapper, so startup and shutdown share the app's loop thread on any host, and the proxy exists exactly as long as the app serves. Signed-off-by: Lin Jia <linj@nvidia.com>
…s source position: 8 collided with anthropic-messages.mdx -- a documented cause of preview 404s in auto-discovered folders -- and the attach-mode example referenced the switchyard-server binary without saying it must be built from the Switchyard repo (it is not part of the nemo-switchyard wheel). Signed-off-by: Lin Jia <linj@nvidia.com>
Two gaps against epic #2088's docs criteria: the model-server index had no Switchyard card, and the page never showed what a deployment TOML looks like, so a reader could not write one from Gym docs alone. Adds the index card and a validated example deployment (fixed baseline route + strong/weak classifier route, loaded against the pinned 0.2.0 wheel), framed as the two routing conditions a comparison runs under. Signed-off-by: Lin Jia <linj@nvidia.com>
Two routed runs were previously indistinguishable from their outputs -- no deployment identity, no switchyard version, and a hosted proxy's stats died with the process. Set condition_dir (one directory per run) and the server now writes the run's routing-condition record: - switchyard-condition.json at startup: route, mode, proxy root URL, nemo-switchyard version, deployment SHA-256 and archived contents with credential values redacted. Comparability is manifests differing only in route or deployment; reproducibility is the archive. - switchyard-stats.json at shutdown: the proxy's /v1/stats (per-target requests, tokens, retries, latency), captured before the in-process server goes away. Works in attach mode too. Both writes are best-effort: provenance failures log a warning rather than taking down serving. The docs page gains a documented two-condition comparison workflow built on the record, closing two acceptance criteria of #2088. Signed-off-by: Lin Jia <linj@nvidia.com>
An unwritable condition_dir must not stop serving, a missing dist still gets a manifest with version null, and a stats snapshot with no proxy to ask is a no-op. app.py back to 100% coverage. Signed-off-by: Lin Jia <linj@nvidia.com>
Redact every credential a deployment can carry (extra_headers values as well as api_key), or skip the archive when the TOML cannot be parsed. Fetch the stats snapshot through Gym's shared async client, as the configured caller, hard-capped inside the shutdown window, and label whether it covers this run or a shared proxy's lifetime. Take identity for an attached proxy from caller-supplied proxy_provenance instead of the local wheel. Validate at startup that rollout correlation can work, enter the proxy's cleanup scope the moment it serves, expose the worker entrypoint for attach mode, reject hosted multi-worker configs, and require max_concurrent_requests to be positive. Signed-off-by: Lin Jia <linj@nvidia.com>
/v1/stats reports requests, errors, tokens, and latency plus classifier usage; it has no retry counts. The comparison example now aligns rollout rows by task and rollout index, reports rollouts missing a counterpart, and adds the classifier tokens the selected model's usage omits. State the observability requirement for session correlation, the single-worker constraint on hosted mode, the aggregate scope of attached-proxy stats, and the proxy_provenance knob, and appease the secrets detector on the example's api_key_env line. Signed-off-by: Lin Jia <linj@nvidia.com>
A live end-to-end run surfaced this: the yaml set forward_session_id to its default, so every config built from it read as an explicit request for forwarding, and the startup check refused to serve any eval that ran without observability. The intended split -- warn on the default, refuse only a real user override the server cannot honor -- needs the yaml to stay silent. Verified live in both directions and pinned with a test that reads the shipped yaml. Signed-off-by: Lin Jia <linj@nvidia.com>
Both are inert: the inline api_key is the fake secret the redaction test plants to prove it never reaches the manifest, and the api_key_env lines name an environment variable rather than holding a value. Marked on the lines themselves so baseline line drift cannot re-flag them. Signed-off-by: Lin Jia <linj@nvidia.com>
315de1c to
0288db7
Compare
|
/ok to test 0288db7 |
Part of #2088. This PR now also absorbs #2600 (routing-condition provenance) so the whole feature reviews in one place.
Adds
responses_api_models/switchyard_model/, a model server that routes Gym model calls through a Switchyard proxy, so any benchmark Gym supports can run against a router without harness changes — unlike the per-harness integrations in #2026/#2080, which are right for RL trace capture but not for evaluating the router itself. Gym either hosts the router in-process from a native Switchyard TOML deployment (deployment=routes.toml; a bad deployment fails at startup, nothing outlives the process, and multi-worker configs are rejected since each worker would host its own proxy) or attaches to an externally run proxy viaswitchyard_base_url, which does support multiple workers. A caller-suppliedmodelis always overridden with the configured route so an agent can't bypass the router, and Gym forwards its rollout-attempt id as the Switchyard session id (x-switchyard-session-id) so routing decisions and costs join back to the rollout that produced them. That id only arrives when model-call capture or token capture is enabled, so the server validates the requirement at startup: a warning under the default config, a refusal to start when forwarding was requested explicitly but no capture is on.From #2600: set
condition_dirand the server records the routing condition the run served under —switchyard-condition.jsonat startup (route, mode, proxy URL, deployment SHA-256 plus an archive with inlineapi_keyand allextra_headersvalues redacted, and the proxy's identity: the local wheel version when hosting, a caller-suppliedproxy_provenancemapping when attaching) andswitchyard-stats.jsonat shutdown (per-target requests, errors, tokens, and latency plus classifier-side usage from/v1/stats, fetched through Gym's shared async client as the configured caller and hard-capped inside the shutdown window). The snapshot labels its scope, since a hosted proxy's counters cover exactly the run while an attached proxy's aggregate everything it has served. Both writes are best-effort and cannot take down serving. This makes two routed runs comparable and a hosted run reproducible from the archived deployment; the docs page gains a "Compare routing conditions" workflow that aligns rollout rows by task and rollout index, reports rollouts missing a counterpart, and includes the classifier tokens the selected model's usage omits.The only dependency is
nemo-switchyard==0.2.0, pinned exactly in this server's own pyproject.toml (Gym core never imports Switchyard); 0.2.0 is the floor because it is the first release whose Responses translation passesNeMoGymResponsevalidation (NVIDIA-NeMo/Switchyard#144), and the ceiling because upstream has already removed the Python serve stack this PR originally drove. 47 tests pass with 100% coverage on app.py, including an integration suite that drives the real 0.2.0 wheel with no mocks (chat/responses round trips, route override, stats, provenance files written across a real shutdown), and a live routed rollout ran against this revision with reward 1.0 (results in this comment).Two reviewer notes.
ModelCallRecord.model_refnames the Gym model server, not the routed target, so per-call attribution must be read from captured payloads — promoting it to a first-class record field is the follow-up flagged for #2088 and needs a core-schema design pass. Andharbor_agentoverwritesresponse["model"]with the policy model name, so Harbor-run benchmarks report the route name for every call; flagging in case that overwrite is unintentional.