fix: repair a scheme-less WANDB_BASE_URL and stop losing W&B sink failures - #57
Conversation
Confirmed live from inside a Modal sandboxThe open question on this PR was whether Modal egress to scaleai.wandb.io works, since the $0 repro only proved it from a laptop. It does. A swe-atlas-qna run started at 08:58:16 on a branch carrying this fix. One minute later: That entity listed zero projects before today. The project was created by the eval-sidecar running inside the Modal sandbox, on its And it is not just an empty project — it is streaming: Compare the failing run's So the entire Worth noting what this buys beyond the ticket: those Ready to merge from my side. |
f396e3f to
8d0ea6d
Compare
ccc7d07 to
ec708e9
Compare
8d0ea6d to
6091efd
Compare
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def normalize_wandb_base_url(environment: dict[str, str] | None = None) -> str | None: |
There was a problem hiding this comment.
why not just fix the url input?
Every harbor benchmark run has reported nothing to the self-hosted W&B:
`shehab-yasser` listed zero projects, and each run's exported
`artifacts/wandb/state.json` showed a run_id minted with
`evaluation_ids: []`, `next_step: 0`, `request_log_files: {}` — even for a
session that completed 11 evaluations.
That fingerprint is exactly what `SidecarWandbSink.__init__` leaves when
`wandb.init()` raises: state is written (wandb.py:225) before the run is
opened (wandb.py:230), and `deployment.py` catches anything from the
constructor and continues without W&B.
Reproduced locally against scaleai.wandb.io, $0, deterministic:
WANDB_BASE_URL=scaleai.wandb.io
pydantic_core.ValidationError: 1 validation error for Settings
base_url
Input should be a valid URL, relative URL without a base
[type=url_parsing, input_value='scaleai.wandb.io']
W&B parses `base_url` as a URL, so the natural way to write a self-hosted
host silently costs the run all of its reporting. With `https://` prepended,
the same script logs metrics, uploads an artifact and auto-creates the
project. Egress was never the problem.
Two changes:
- `normalize_wandb_base_url()` prepends `https://` to a scheme-less
`WANDB_BASE_URL` (and warns), called before both `wandb.init()` sites.
Already-qualified values, including plain `http://localhost`, pass through.
- The swallowed init failure now also lands in
`session/artifacts/wandb/init-error.json`. The existing `logger.warning`
goes to the sidecar container's stderr, which no run artifact captures, so
a disabled sink was indistinguishable from a healthy run that logged
nothing. Observability still never takes the eval path down.
Refs #52.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ac3aee3 to
a4e956a
Compare
6091efd to
934cc4f
Compare
16fbc2a
into
feat/swe-bench-pro-baseline-scaffold
Both branches had accumulated real vero/src work the other lacked -- 12 commits here (wandb URL repair, optimizer-trial harbor flags, model preflight, transient infra classification) against 5 on pr2 (opencode step limit, litellm base-url alias, outer Modal app naming, terminal budget status, backend in the eval plan). The stacked-PR assumption that vero/ lives only on pr2 had quietly stopped holding, and the cost was concrete: benchmark runs launch from this worktree, so three fixes committed on pr2 were simply absent at run time. It cost a wasted mini-swe-agent attempt and a gaia run launched without the step limit it needed. Merging rather than rebasing: PR #57 is already merged into this branch, so replaying commits over it would rewrite merged history for no benefit, and a merge resolves the overlap once instead of per-commit. Three conflicts, all where both sides edited the same lines: - The run command gained both sets of appended flags. The derived Modal app name now defers to an explicit app_name from *either* source, since a build can declare one in optimizer_harbor_args just as a caller can pass one on the command line, and appending ours after the build's would have silently won on harbor's last-value-per-key rule. - Three config stubs needed both `optimizer_harbor_args` and `name`; they were written before the other side's field existed. - Three assertions expecting `_opencode_gateway_args` to return nothing are now wrong by design: the step limit is unconditional, so a bare model name and a gateway-less task both still get it. Dropped those and kept every other test from both sides. 453 passed, 5 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #52 (pending the live confirmation noted at the bottom).
The symptom
No harbor benchmark run has ever reported anything to the self-hosted W&B. Entity
shehab-yasserlisted zero projects. Every run's exportedsession/artifacts/wandb/state.jsonlooked like this:{"evaluation_ids": [], "next_step": 0, "run_id": "vero-a25e1cd8b7064d7e", "request_log_files": {}}…from the 2026-07-25 swe-atlas-qna run whose session contains 11 completed evaluations. So "nothing was logged" was never "nothing happened".
The mechanism
That exact fingerprint is what
SidecarWandbSink.__init__leaves behind whenwandb.init()throws._save_state()runs atruntime/wandb.py:225,_open_wandb_run()at:230.harbor/deployment.py:265-287catches anything out of the constructor and continues withwandb_sink = None, by design — observability must not take the eval path down.The cause, reproduced locally at $0
W&B parses
base_urlas a URL. The natural way to write a self-hosted host is rejected, and the rejection is indistinguishable from "W&B is unavailable".Same script with
https://prepended:The project auto-created server-side;
query_wandb_entity_projects(entity="shehab-yasser")went from[]to[vero-egress-probe]. Metrics logged,wandb.Artifactuploaded. Egress was never blocked, credentials were never missing, and the code was never unplumbed.Note the repo's own
swe-bench-pro/baseline/secrets.env.example:23already gets it right (WANDB_BASE_URL=https://scaleai.wandb.io); the live gitignoredsecrets.envhad drifted to the scheme-less form. Since the deployed secrets file is not in the repo, fixing only the file would leave the trap armed for the next operator.Also worth correcting
Issue #52 is titled "Weave traces". There is no Weave in this codebase —
weaveis not a dependency and is not importable in the built env.wandb.log_tracesdrivesSidecarWandbSink._log_trace, which uploads awandb.Artifactof typeevaluation_trace. Socount_weave_traceswould return nothing even on a perfectly healthy run; the right server-side check is whether the project exists and carries runs and artifacts.Changes
normalize_wandb_base_url()prependshttps://to a scheme-lessWANDB_BASE_URLand warns; called before bothwandb.init()sites. Values that already carry a scheme (includinghttp://localhost:8080) pass through untouched; unset stays unset.session/artifacts/wandb/init-error.jsonwith the exception type and message. The existinglogger.warningonly reaches the sidecar container's stderr, which no run artifact captures — grepping every artifact of the failing run for "wandb" returns nothing. That invisibility is what made a one-line config bug survive multiple runs.Tests:
test_scheme_less_wandb_base_url_is_repaired_before_init(repair, passthrough, unset, and that a sink repairs before opening its run) andtest_wandb_init_failure_is_recorded_in_the_session_artifacts(eval path survives, reason is durable).Still to confirm
The local probe proves the URL fix and that scaleai.wandb.io is reachable from this laptop. Modal-sandbox egress to it is not yet proven. A live benchmark run on this branch is queued; I will post the resulting project URL and artifact count here before this merges.
Greptile Summary
This PR fixes two independent failure modes that caused every harbor benchmark run to report nothing to the self-hosted W&B instance: a scheme-less
WANDB_BASE_URLthat W&B's pydantic model rejected, and a swallowed init failure that left no durable evidence in run artifacts.normalize_wandb_base_url()prependshttps://to any scheme-less value and mutatesos.environin-place so W&B's settings model receives the corrected URL; it is called before both independentwandb.init()sites (_open_wandb_runforSidecarWandbSink, and directly inWandbEventSink.__init__).deployment.pynow writeswandb/init-error.jsoninto the session artifact directory (in addition to the existing stderr warning), making the failure discoverable via the exported run record rather than only through transient container logs.SidecarWandbSink, eval-path survival on init failure, and durability of the recorded error.Confidence Score: 5/5
Safe to merge once the live benchmark run confirms Modal-sandbox egress to scaleai.wandb.io (as noted in the PR description).
Both changes are narrow and defensive: URL normalization is idempotent and only fires on misconfigured input, and the error-recording path is wrapped in its own OSError guard so it can never affect the eval path. The existing sink-failure handling is preserved exactly; only the durability of the diagnosis is improved. Tests cover the new code paths directly.
Files Needing Attention: No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["WANDB_BASE_URL set in env"] --> B{normalize_wandb_base_url} B -->|"has '://' or empty"| C["pass through unchanged"] B -->|"scheme-less e.g. scaleai.wandb.io"| D["prepend https://\nlog warning\nmutate os.environ"] C --> E[wandb.init] D --> E subgraph SidecarWandbSink path F["SidecarWandbSink.__init__"] --> G["_save_state (run_id persisted)"] G --> H["_open_wandb_run()"] H --> B end subgraph WandbEventSink path I["WandbEventSink.__init__"] --> B end E -->|"success"| J["sink attached to engine.listeners"] E -->|"exception"| K["except Exception as error"] K --> L["logger.warning (stderr only)"] L --> M["ArtifactStore.write_json\nwandb/init-error.json"] M -->|"OSError"| N["logger.warning, continue"] M -->|"success"| O["wandb_sink = None\neval path continues"] N --> OReviews (4): Last reviewed commit: "fix: repair a scheme-less WANDB_BASE_URL..." | Re-trigger Greptile