Skip to content

refactor(harbor): split Mode A / Mode B into discriminated-union configs #31

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shehabyasser-scale wants to merge 1 commit into
base: harbor-3-feedback-levers
Choose a base branch
from
Open

refactor(harbor): split Mode A / Mode B into discriminated-union configs #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions vero/src/vero/harbor/build/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
"""The `vero harbor build` compiler: BuildConfig -> a runnable Harbor task dir."""

from vero.harbor.build.compiler import compile_task
from vero.harbor.build.config import BuildConfig
from vero.harbor.build.config import (
BuildConfig,
BuildConfigA,
BuildConfigB,
load_build_config,
)

__all__ = ["BuildConfig", "compile_task"]
__all__ = [
"BuildConfig",
"BuildConfigA",
"BuildConfigB",
"compile_task",
"load_build_config",
]
104 changes: 54 additions & 50 deletions vero/src/vero/harbor/build/compiler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from jinja2 import Environment, FileSystemLoader

from vero.evaluation.engine import EvalRequest
from vero.harbor.build.config import BuildConfig
from vero.harbor.build.config import BuildConfigA, BuildConfigB
from vero.harbor.protocol import StatusSummary

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -210,24 +210,14 @@ def _validate_partition_names(
)


def _serve_config(config: BuildConfig, dataset_id: str | None, base_commit: str) -> dict:
harbor = None
if config.harbor is not None:
# Local inner task -> baked sidecar-only path; registry ref -> pass through.
harbor = {**config.harbor}
if config.inner_task:
harbor["task_source"] = INNER_TASK
targets = [
{
"task": config.task,
"dataset_id": dataset_id,
"split": t.split,
"reward_key": t.reward_key,
"sample_ids": t.sample_ids,
}
for t in config.targets
]
return {
def _serve_config(
config: BuildConfigA | BuildConfigB, dataset_id: str | None, base_commit: str
) -> dict:
# Fields common to both ServeConfig variants. Mode-specific keys are added
# below so the wrong-mode key never reaches the (extra="forbid") ServeConfig.
task = config.task if isinstance(config, BuildConfigA) else None
common = {
"mode": config.mode,
"repo_path": AGENT_BASELINE,
"agent_repo_path": WORK_AGENT,
"session_id": SESSION_ID,
Expand All @@ -237,33 +227,58 @@ def _serve_config(config: BuildConfig, dataset_id: str | None, base_commit: str)
{"split": b.split, "dataset_id": dataset_id, **b.model_dump(exclude={"split"}, exclude_none=True)}
for b in config.budgets
],
"task": config.task,
"task_project": config.task_project,
"task_module": config.task_module,
"harbor": harbor,
"reward_mode": config.reward_mode,
"selection_split": config.selection_split,
"targets": targets,
"targets": [
{
"task": task,
"dataset_id": dataset_id,
"split": t.split,
"reward_key": t.reward_key,
"sample_ids": t.sample_ids,
}
for t in config.targets
],
"base_commit": base_commit,
"submit_enabled": config.submit_enabled,
"score_baseline": config.score_baseline,
"feedback_transcripts": config.feedback_transcripts,
"feedback_max_bytes": config.feedback_max_bytes,
"instruct_multifidelity": config.instruct_multifidelity,
"expose_attempt_detail": config.expose_attempt_detail,
"agent_volume": AGENT_VOLUME,
"admin_volume": ADMIN_VOLUME,
"admin_token_path": TOKEN_PATH,
"timeout": config.timeout,
"sample_timeout": config.sample_timeout,
"max_concurrency": config.max_concurrency,
"host": "0.0.0.0",
"port": 8000,
}
if isinstance(config, BuildConfigA):
return {
**common,
"task": config.task,
"task_project": config.task_project,
"task_module": config.task_module,
"sample_timeout": config.sample_timeout,
}
# Mode B: local inner task -> baked sidecar-only path; registry ref passes through.
harbor = None
if config.harbor is not None:
harbor = {**config.harbor}
if config.inner_task:
harbor["task_source"] = INNER_TASK
return {
**common,
"harbor": harbor,
"feedback_transcripts": config.feedback_transcripts,
"feedback_max_bytes": config.feedback_max_bytes,
"instruct_multifidelity": config.instruct_multifidelity,
"expose_attempt_detail": config.expose_attempt_detail,
}


def compile_task(
config: BuildConfig, out_dir: Path | str, *, vero_root: Path | None = None
config: BuildConfigA | BuildConfigB,
out_dir: Path | str,
*,
vero_root: Path | None = None,
) -> Path:
"""Compile ``config`` into a Harbor task directory at ``out_dir``."""
import json
Expand All @@ -272,23 +287,10 @@ def compile_task(

vero_root = vero_root or PACKAGE_DIR

# Mode A ignores the Mode-B-only feedback levers (they ride the nested
# `harbor run` collation, which Mode A never runs). Warn loudly at build time
# so a config that sets them in Mode A learns they will do nothing, rather
# than silently getting no feedback.
if config.mode == "A":
mode_b_only = [
n
for n in ("feedback_transcripts", "expose_attempt_detail")
if getattr(config, n)
]
if mode_b_only:
logger.warning(
"Mode A build sets Mode-B-only lever(s) %s; these have no effect "
"in Mode A (they ride the nested `harbor run` collation) and will "
"be ignored.",
", ".join(mode_b_only),
)
# The Mode-A "you set a Mode-B-only lever" warning (and its ServeConfig twin)
# is superseded by the Mode-A / Mode-B type split: a Mode-A config that sets
# feedback_transcripts / expose_attempt_detail is now a load-time
# ValidationError, so the condition is structurally impossible here.

out = Path(out_dir)
if out.exists():
Expand All @@ -315,7 +317,7 @@ def compile_task(
# the dataset into vh before the dir is torn down, so cleanup is safe.
with tempfile.TemporaryDirectory() as tmp_str:
tmp = Path(tmp_str)
if config.mode == "A":
if isinstance(config, BuildConfigA):
if not config.dataset:
raise ValueError("Mode A requires a dataset.")
dataset_id = _register(config.dataset, vh, tmp)
Expand Down Expand Up @@ -373,7 +375,9 @@ def compile_task(
selection_split=config.selection_split,
submit_enabled=config.submit_enabled,
eval_num_samples=None,
bake_inner_task=bool(config.inner_task),
# inner_task / instruct_multifidelity are Mode-B-only fields; on a
# Mode-A config they do not exist, so read them only when present.
bake_inner_task=bool(getattr(config, "inner_task", None)),
# The free-baseline bullet may only render when the sidecar shipping in
# this same tree actually grants the free eval; the feature lives on a
# different PR chain than the compiler, and an instruction that promises
Expand All @@ -392,7 +396,7 @@ def compile_task(
# sample's exact score, defeating the non_viewable contract. Only a
# viewable split is safe to screen with subsets. no_access splits are
# not agent-evaluable at all, so they never count either.
multifidelity=config.instruct_multifidelity
multifidelity=getattr(config, "instruct_multifidelity", False)
and {"sample_ids", "num_samples"}
<= {f.name for f in dataclasses.fields(EvalRequest)}
and any(s.access == "viewable" for s in config.splits),
Expand Down
136 changes: 88 additions & 48 deletions vero/src/vero/harbor/build/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,20 @@
Everything the compiler needs to emit a Harbor optimization task. Mode A (vero
runs inference + scoring) and Mode B (nested `harbor run`) share one topology;
the differences are which extras the sidecar bakes and which secrets it needs.

The two modes are DISTINCT types discriminated on `mode`: a Mode-A config that
sets a Mode-B-only field (or vice versa) is a load-time ValidationError, not a
silently-ignored no-op. `extra="forbid"` on the shared base plus the per-mode
subclasses means the wrong-mode key is simply unknown to the resolved variant.
"""

from __future__ import annotations

from pathlib import Path
from typing import Literal
from typing import Annotated, Literal

import yaml
from pydantic import BaseModel, ConfigDict, Field
from pydantic import BaseModel, ConfigDict, Field, TypeAdapter


class SplitAccessSpec(BaseModel):
Expand All @@ -33,12 +38,14 @@ class TargetSpec(BaseModel):
sample_ids: list[int] | None = None


class BuildConfig(BaseModel):
"""Inputs to `vero harbor build`."""
class _BuildConfigBase(BaseModel):
"""Fields shared by both modes. Not instantiated directly."""

# Reject unknown top-level keys so a mistyped lever fails loudly at load
# time instead of silently disabling the feature: pydantic's default is to
# ignore extras, which would turn `feeback_transcripts: true` into a no-op.
# Combined with the Mode-A / Mode-B split below, a wrong-mode field is also
# "unknown" to the resolved variant, so it fails the same way.
model_config = ConfigDict(extra="forbid")

# identity
Expand All @@ -48,22 +55,6 @@ class BuildConfig(BaseModel):
# the target repo the optimizer edits (baseline in main + sidecar)
agent_repo: str

# mode A (scoring in vero): task name + dataset (+ optional separate task project)
mode: Literal["A", "B"] = "A"
task: str | None = None
task_project: str | None = None
task_module: str | None = None
dataset: str | None = Field(
default=None, description="Path to a saved DatasetDict (Mode A)."
)

# mode B (scoring in nested harbor): HarborConfig kwargs (task_source filled by the
# compiler from inner_task), the {split: [task_names]} partition, and the inner
# Harbor task dir baked sidecar-only (the protected benchmark, mirrors Mode A's dataset).
harbor: dict | None = None
partition: dict[str, list[str]] | None = None
inner_task: str | None = None

# tiers / budget / reward
splits: list[SplitAccessSpec]
budgets: list[BudgetSpec] = Field(default_factory=list)
Expand All @@ -75,21 +66,6 @@ class BuildConfig(BaseModel):
# write it to <admin_volume>/baseline.json, so a candidate that generalizes
# WORSE than the untouched repo is visible as a regression.
score_baseline: bool = False
# Lever 1 (Mode B): each FAILED sample (reward 0) of an eval carries the
# tail of its trial transcript in the per-sample `feedback` field. Rides
# the per-sample result files the sidecar writes ONLY for viewable splits,
# so it can never surface for non_viewable / no_access tiers.
feedback_transcripts: bool = False
feedback_max_bytes: int = 3000
# Lever 2: the compiled instruction teaches multi-fidelity screening (triage
# rough ideas on subset evals via num_samples / sample_ids, confirm survivors
# on the full split). Renders only when the sidecar in the same tree actually
# accepts subset evals; see the compiler's ctx gate.
instruct_multifidelity: bool = False
# Lever 3 (Mode B): each sample's output carries an `attempts` list, one
# {reward, exception} entry per attempt. Same viewable-only exposure as
# feedback_transcripts.
expose_attempt_detail: bool = False

# write-access: paths in the target repo the optimizer may NOT edit
# (the scorer, by default). Applied as unix perms in main before the agent runs.
Expand All @@ -104,18 +80,82 @@ class BuildConfig(BaseModel):

# eval params baked into the ServeConfig
timeout: int = 1800
sample_timeout: int = 300
max_concurrency: int = 8

@classmethod
def from_file(cls, path: Path | str) -> BuildConfig:
path = Path(path).resolve()
data = yaml.safe_load(path.read_text())
# Resolve relative local-path fields against the build.yaml's directory, so a
# config is portable regardless of the working directory it's built from.
base = path.parent
for field in ("agent_repo", "dataset", "inner_task"):
val = data.get(field)
if isinstance(val, str) and not Path(val).is_absolute():
data[field] = str((base / val).resolve())
return cls.model_validate(data)

class BuildConfigA(_BuildConfigBase):
"""Mode A: vero runs inference + scoring against a saved dataset."""

mode: Literal["A"] = "A"

# task name + dataset (+ optional separate task project)
task: str | None = None
task_project: str | None = None
task_module: str | None = None
dataset: str | None = Field(
default=None, description="Path to a saved DatasetDict (Mode A)."
)
# Per-sample vero-scoring cap. Mode-A only: Mode B's nested `harbor run` uses
# each task's OWN harbor-configured timeouts, capped only by `timeout`.
sample_timeout: int = 300


class BuildConfigB(_BuildConfigBase):
"""Mode B: scoring runs in a nested `harbor run`."""

mode: Literal["B"]

# HarborConfig kwargs (task_source filled by the compiler from inner_task), the
# {split: [task_names]} partition, and the inner Harbor task dir baked
# sidecar-only (the protected benchmark, mirrors Mode A's dataset).
harbor: dict | None = None
partition: dict[str, list[str]] | None = None
inner_task: str | None = None
# Lever 1: each FAILED sample (reward 0) of an eval carries the tail of its
# trial transcript in the per-sample `feedback` field. Rides the per-sample
# result files the sidecar writes ONLY for viewable splits, so it can never
# surface for non_viewable / no_access tiers.
feedback_transcripts: bool = False
feedback_max_bytes: int = 3000
# Lever 2: the compiled instruction teaches multi-fidelity screening (triage
# rough ideas on subset evals via num_samples / sample_ids, confirm survivors
# on the full split). Renders only when the sidecar in the same tree actually
# accepts subset evals; see the compiler's ctx gate.
instruct_multifidelity: bool = False
# Lever 3: each sample's output carries an `attempts` list, one
# {reward, exception} entry per attempt. Same viewable-only exposure as
# feedback_transcripts.
expose_attempt_detail: bool = False


# Discriminated union on `mode`: `vero harbor build` resolves to exactly one
# variant, and a wrong-mode field (e.g. `feedback_transcripts` under mode A) is
# unknown to that variant and rejected by extra="forbid" at load time.
BuildConfig = Annotated[
BuildConfigA | BuildConfigB, Field(discriminator="mode")
]

# A discriminated union is not a class, so validation goes through a TypeAdapter.
_BuildConfigAdapter: TypeAdapter[BuildConfigA | BuildConfigB] = TypeAdapter(BuildConfig)


def load_build_config(path: Path | str) -> BuildConfigA | BuildConfigB:
"""Load and validate a build.yaml into the correct Mode-A / Mode-B variant.

Replaces the old ``BuildConfig.from_file`` classmethod: the discriminated
union is not a class, so the loader lives at module level. Relative
local-path fields are resolved against the build.yaml's directory, so a
config is portable regardless of the working directory it's built from.
"""
path = Path(path).resolve()
data = yaml.safe_load(path.read_text())
# `mode` defaults to "A" when omitted, preserving the pre-split behavior. The
# discriminated union needs the tag explicitly, so inject it before validating.
if isinstance(data, dict):
data.setdefault("mode", "A")
base = path.parent
for field in ("agent_repo", "dataset", "inner_task"):
val = data.get(field)
if isinstance(val, str) and not Path(val).is_absolute():
data[field] = str((base / val).resolve())
return _BuildConfigAdapter.validate_python(data)
8 changes: 4 additions & 4 deletions vero/src/vero/harbor/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ def serve_cmd(config_path):
@click.option("-o", "--out", required=True, help="Output task directory.")
def build_cmd(config_path, out):
"""Compile a build.yaml into a runnable Harbor optimization task directory."""
from vero.harbor.build import BuildConfig, compile_task
from vero.harbor.build import compile_task, load_build_config

task_dir = compile_task(BuildConfig.from_file(config_path), out)
task_dir = compile_task(load_build_config(config_path), out)
click.echo(f"Compiled task -> {task_dir}")


Expand All @@ -70,9 +70,9 @@ def run_cmd(config_path, agent, model, provider, extra):
import subprocess
import tempfile

from vero.harbor.build import BuildConfig, compile_task
from vero.harbor.build import compile_task, load_build_config

task_dir = compile_task(BuildConfig.from_file(config_path), Path(tempfile.mkdtemp()) / "task")
task_dir = compile_task(load_build_config(config_path), Path(tempfile.mkdtemp()) / "task")
cmd = ["uvx", "harbor", "run", "-p", str(task_dir), "-a", agent, "-e", provider]
if model:
cmd += ["-m", model]
Expand Down
Loading