From 7b7ea138e38095f890ad7d466b89b9c7861c7db5 Mon Sep 17 00:00:00 2001 From: Shehab Yasser Date: Sat, 1 Aug 2026 19:08:49 +0300 Subject: [PATCH] fix: say where a run reports to W&B, loudly when nothing pins it `wandb.entity` is optional and omitting it is silent. The client falls back to the launching user's personal entity, so a grid launched by more than one person scatters across their accounts. The expensive part is not the scattering, it is that a misplaced run is indistinguishable from a missing one: checking the shared org shows nothing, which reads as "the run failed to start". That happened on 2026-08-01. An atlas cell reported into a personal entity for nearly three hours and was twice diagnosed as having produced no W&B run at all, while it was in fact running normally and had already scored 0.32 on development. Exporting WANDB_ENTITY is the natural workaround and does not work, which is why the warning names it. The sidecar owns the run and receives only WANDB_API_KEY and WANDB_BASE_URL through its passthrough list, so the variable never reaches the process that calls `wandb.init`. Only the build config's `entity:` decides the destination. This repo's own shell profile carries a comment saying exactly that, and the trap still caught us. Printed on every run rather than only the bad case: knowing where a cell reports is worth one line, and a warning the reader has no baseline for is easy to skim past. Placed beside `_preflight_models`, both ahead of `compile_harbor_task`, so it lands before a sandbox exists and before a case is scored. Reads through getattr because it is a diagnostic: a config shape carrying no W&B settings degrades to silence rather than an exception, since a report that raises would block the run it exists to describe. Three existing tests pass exactly such stubs, and attribute access broke all three. Deliberately a warning and not an error. Reporting to a personal entity is legitimate for a local one-off, and this fixes the silence, not the default. Test plan: 497 passed, 16 skipped (8 new). The ordering test was A/B'd against the guard: with the `_report_wandb_destination` call removed it fails, restored it passes. Co-Authored-By: Claude Opus 5 (1M context) --- vero/src/vero/harbor/cli.py | 45 ++++++++ vero/tests/test_v05_wandb_destination.py | 137 +++++++++++++++++++++++ 2 files changed, 182 insertions(+) create mode 100644 vero/tests/test_v05_wandb_destination.py diff --git a/vero/src/vero/harbor/cli.py b/vero/src/vero/harbor/cli.py index 90562c56..b969f226 100644 --- a/vero/src/vero/harbor/cli.py +++ b/vero/src/vero/harbor/cli.py @@ -678,6 +678,50 @@ def _probe_model(base_url: str, api_key: str, model: str) -> tuple[int | None, s return last +def _report_wandb_destination(config) -> None: + """Name where results will land, before the run starts spending. + + `wandb.entity` is optional, and omitting it is silent: the client falls back + to the launching user's personal default, so a grid launched by more than one + person scatters across their accounts and a cross-cell comparison has to be + reassembled by hand. Worse, the omission looks like a missing run rather than + a misplaced one, and "I cannot find it in W&B" is then diagnosed as a failed + run. That happened on 2026-08-01: an atlas cell reported into a personal + entity for nearly three hours while being read as having produced nothing. + + Exporting WANDB_ENTITY does not fix it and is the natural thing to try. The + sidecar owns the run and receives only WANDB_API_KEY and WANDB_BASE_URL + through its passthrough list, so the variable never reaches the process that + calls `wandb.init`. The build config's `entity:` is the only thing that + decides the destination. + + Printed on every run, not only when unset: knowing where a cell reports is + worth one line, and a warning that appears only in the bad case is one the + reader has no baseline for. + """ + + # getattr, not attribute access: this is a diagnostic, and one that raises + # would block the run it exists to describe. Reporting nothing is the correct + # degradation for a config shape that does not carry W&B settings at all. + wandb = getattr(config, "wandb", None) + if wandb is None: + return + name = getattr(wandb, "name", None) or "" + project = getattr(wandb, "project", None) or "" + if getattr(wandb, "entity", None): + click.echo(f"W&B: {wandb.entity}/{project} run={name}") + return + click.echo( + f"W&B: /{project} run={name}\n" + " WARNING: wandb.entity is unset, so this run reports to the launching\n" + " user's personal entity, not a shared org. Exporting WANDB_ENTITY will\n" + " NOT change this: the sidecar passes through only WANDB_API_KEY and\n" + " WANDB_BASE_URL. Set `entity:` under `wandb:` in the build config to\n" + " pin it.", + err=True, + ) + + def _preflight_models(config) -> None: """Refuse to launch when a configured model is not deployed upstream. @@ -784,6 +828,7 @@ def run_command(config_path, agent, model, environment, params, env_file, extra) resolved.setdefault("optimizer_model", model) config = load_harbor_build_config(config_path, params=resolved) _preflight_models(config) + _report_wandb_destination(config) with tempfile.TemporaryDirectory(prefix="vero-harbor-") as temporary: task = compile_harbor_task( config, diff --git a/vero/tests/test_v05_wandb_destination.py b/vero/tests/test_v05_wandb_destination.py new file mode 100644 index 00000000..6b54d7b7 --- /dev/null +++ b/vero/tests/test_v05_wandb_destination.py @@ -0,0 +1,137 @@ +"""The launch-time report naming where a run's W&B results will land. + +`wandb.entity` is optional and omitting it is silent, so a cell reports into the +launching user's personal entity while looking, to anyone checking the shared +org, like a run that never started. These pin that the destination is always +stated and that the unset case says so loudly. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +import click +import pytest + +from vero.harbor import cli as harbor_cli + + +def _config(entity=None, project="harness-engineering-bench", name="cell-1"): + return SimpleNamespace( + wandb=SimpleNamespace(entity=entity, project=project, name=name) + ) + + +def test_pinned_entity_is_reported(capsys) -> None: + harbor_cli._report_wandb_destination(_config(entity="egp")) + out = capsys.readouterr() + assert "W&B: egp/harness-engineering-bench run=cell-1" in out.out + assert "WARNING" not in out.out + out.err + + +def test_unset_entity_warns_and_names_the_fallback(capsys) -> None: + harbor_cli._report_wandb_destination(_config(entity=None)) + err = capsys.readouterr().err + assert "personal default" in err + assert "WARNING" in err + # The natural workaround has to be named, or the reader tries it and the run + # still scatters: the sidecar owns wandb.init and never sees the variable. + assert "WANDB_ENTITY will" in err and "NOT change this" in err + assert "entity:" in err + + +def test_empty_string_entity_is_treated_as_unset(capsys) -> None: + """`entity: ""` resolves from an unset ${param} and must not read as pinned.""" + + harbor_cli._report_wandb_destination(_config(entity="")) + assert "WARNING" in capsys.readouterr().err + + +def test_silent_when_wandb_is_not_configured(capsys) -> None: + harbor_cli._report_wandb_destination(SimpleNamespace(wandb=None)) + out = capsys.readouterr() + assert out.out == "" and out.err == "" + + +def test_missing_run_name_does_not_crash_the_launch(capsys) -> None: + """A diagnostic that raises would block the run it is meant to describe.""" + + harbor_cli._report_wandb_destination(_config(entity="egp", name=None)) + assert "run=" in capsys.readouterr().out + + +def test_harbor_run_warns_before_it_compiles_the_task(tmp_path, monkeypatch) -> None: + """Ordering is the point: the line has to land before anything is spent. + + Proven by making compilation fail. If the report already ran, its warning is + in the output even though the command died at compile, which is exactly the + guarantee wanted: a misdirected destination is visible before a sandbox + exists and before a single case is scored. + """ + + from click.testing import CliRunner + + from vero.cli import main + from vero.harbor import build as harbor_build + + config_path = tmp_path / "build.yaml" + config_path.write_text("name: org/task\n", encoding="utf-8") + + class _Config: + harbor_requirement = "harbor[modal]==0.20.0" + agent_env: dict[str, str] = {} + optimizer_harbor_args: list[str] = [] + extra_harbor_args: list[str] = [] + name = "vero/stub-benchmark" + wandb = SimpleNamespace(entity=None, project="heb", name="cell-1") + + def _explode(config, output): + raise RuntimeError("compilation reached") + + monkeypatch.setattr( + harbor_build, "load_harbor_build_config", lambda *a, **k: _Config() + ) + monkeypatch.setattr(harbor_build, "compile_harbor_task", _explode) + monkeypatch.setattr(harbor_cli.shutil, "which", lambda name: "/usr/bin/uvx") + monkeypatch.setattr(harbor_cli, "_preflight_models", lambda config: None) + + result = CliRunner().invoke( + main, + [ + "harbor", + "run", + "--config", + str(config_path), + "--agent", + "codex", + "--model", + "gpt-5.3-codex", + "--yes", + ], + ) + + # click >=8.2 splits the streams; the warning is deliberately on stderr. + emitted = result.output + (result.stderr or "") + assert "compilation reached" in str(result.exception) + assert "WARNING" in emitted + assert "personal" in emitted + + +def test_config_argument_is_required() -> None: + """A diagnostic called with no config should fail loudly, not pass silently.""" + + with pytest.raises(TypeError): + harbor_cli._report_wandb_destination() + + +def test_config_without_a_wandb_attribute_is_a_no_op(capsys) -> None: + """Degrade to silence, never to an exception. + + Several call sites and tests pass config shapes that carry no W&B settings at + all. A diagnostic that raised on those would block the launch it exists to + describe, which is a strictly worse failure than the one it prevents. + """ + + harbor_cli._report_wandb_destination(SimpleNamespace(name="org/task")) + out = capsys.readouterr() + assert out.out == "" and out.err == ""