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

[shehabyasser-scale]

Supersedes #20 by construction. Mode A (vero-native per-sample scoring) and Mode B (nested harbor run) previously shared one flat BuildConfig/ServeConfig, so a field that only works in one mode silently no-opped in the other, and we kept papering over that with runtime warnings (#20's sample_timeout warning, and the Mode-A feedback-lever warning added in #30).

This splits both schemas into discriminated unions on mode:

• BuildConfigA (mode A: task, task_project, task_module, dataset, sample_timeout) and BuildConfigB (mode B: harbor, partition, inner_task, and the feat(harbor): feedback levers: failure transcripts, multi-fidelity screening, per-attempt detail #30 feedback levers), over a shared extra="forbid" base. Same for ServeConfigA/ServeConfigB.
• A wrong-mode field is now a load-time ValidationError instead of a silent no-op: sample_timeout in a Mode-B yaml fails to parse, feedback_transcripts in a Mode-A yaml fails to parse.
• from_file classmethods become load_build_config / load_serve_config (TypeAdapter loaders; a union is not a class). Missing mode still defaults to A.
• Deleted as dead code: _warn_mode_b_sample_timeout (all of fix(harbor): warn that sample_timeout is not enforced in Mode B #20) and _warn_mode_a_ignores_feedback_levers, plus their tests. The conditions they warned about are now structurally impossible.
• This also fixes a latent bug in fix(harbor): warn that sample_timeout is not enforced in Mode B #20's approach: the warning gated on model_fields_set, but the compiler bakes sample_timeout unconditionally into the serve JSON, so the "explicitly set" check would have fired on every compiled Mode-B task.

Acceptance tests cover both wrong-mode rejections and valid round-trips through compile_task. The three live Pawn lever configs parse and compile through the new loader (one of them ran a full trial through this code).

Stacked on #30 (harbor-3-feedback-levers).

🤖 Generated with Claude Code

Greptile Summary

Replaces the single flat BuildConfig/ServeConfig with a mode-discriminated union (BuildConfigA | BuildConfigB, ServeConfigA | ServeConfigB), so a wrong-mode field is now a load-time ValidationError rather than a silently-ignored no-op. The from_file classmethods become module-level load_build_config / load_serve_config TypeAdapter loaders, and the PR #20/#30 runtime warnings are deleted as structurally impossible.

• config.py / serve.py: _BuildConfigBase / _ServeConfigBase carry shared fields with extra="forbid"; Mode-A subclasses add task/dataset/sample_timeout, Mode-B subclasses add harbor/partition/feedback levers.
• compiler.py: _serve_config splits its return into mode-specific branches so no wrong-mode key ever reaches the serialised serve.json; compile_task replaces config.mode == "A" string checks with isinstance guards.
• Tests replace the warning-assertion classes with TestModeMismatchRejection, covering both wrong-mode directions, and add a built_b fixture for Mode-B compile round-trips.

Confidence Score: 4/5

Safe to merge — the refactor is structurally sound and well-tested; both wrong-mode directions are rejected at load time, and three live configs have been validated end-to-end.

The core split logic and _serve_config branching are correct. Two style inconsistencies were found: getattr in compiler.py for inner_task/instruct_multifidelity instead of an isinstance check (inconsistent with the rest of the function), and the is_mode_a boolean guard in serve.py that accesses Mode-A-only attributes via short-circuit evaluation rather than a narrowed block (safe at runtime, but static analysis tools will report errors on those lines).

The is_mode_a guard pattern in vero/src/vero/harbor/serve.py (lines 238–251) and the getattr calls in vero/src/vero/harbor/build/compiler.py are worth a second look if the project adopts strict mypy/pyright checking.

Important Files Changed

Filename	Overview
vero/src/vero/harbor/build/config.py	Core schema split: BuildConfig (flat class) → discriminated union of BuildConfigA / BuildConfigB on mode; from_file replaced by load_build_config with TypeAdapter. Path-resolution loop is safe (operates on the raw dict before validation). No issues.
vero/src/vero/harbor/serve.py	Mirror split for ServeConfig → ServeConfigA / ServeConfigB; build_components switches from config.harbor is not None guards to isinstance checks. Mode-A-only attributes (task_project, task_module, task, sample_timeout) are accessed via is_mode_a boolean short-circuit rather than type-narrowed blocks — safe at runtime but static analysis would flag it.
vero/src/vero/harbor/build/compiler.py	_serve_config split into mode-specific branches; compile_task uses isinstance checks throughout except for inner_task and instruct_multifidelity which use getattr with defaults — inconsistent with the rest of the function and bypasses type narrowing.
vero/tests/test_harbor_build.py	Tests updated to use concrete types; TestModeAIgnoresFeedbackLevers replaced by TestModeMismatchRejection covering both wrong-mode directions; new built_b fixture exercises Mode-B compile path. Coverage is thorough.
vero/tests/test_harbor_serve.py	Feedback-lever test rebuilt as a native ServeConfigB instance; test_mode_b_sample_timeout_warns replaced by test_mode_mismatch_fields_rejected verifying ValidationError in both directions. No issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["build.yaml / serve.json"] --> B{"mode present?"}
    B -- "no" --> C["inject mode='A'"]
    B -- "yes" --> D{"mode value"}
    C --> D
    D -- "A" --> E["BuildConfigA / ServeConfigA\n(task, task_project, task_module,\ndataset, sample_timeout)"]
    D -- "B" --> F["BuildConfigB / ServeConfigB\n(harbor, partition, inner_task,\nfeedback levers)"]
    D -- "wrong-mode field present" --> G["ValidationError\n(extra='forbid')"]

    E --> H["compile_task / build_components\n(Mode-A branch)"]
    F --> I["compile_task / build_components\n(Mode-B branch)"]

    H --> J["serve.json with mode='A'\n+ task/task_project/sample_timeout"]
    I --> K["serve.json with mode='B'\n+ harbor/feedback levers"]

    J --> L["load_serve_config → ServeConfigA"]
    K --> M["load_serve_config → ServeConfigB"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["build.yaml / serve.json"] --> B{"mode present?"}
    B -- "no" --> C["inject mode='A'"]
    B -- "yes" --> D{"mode value"}
    C --> D
    D -- "A" --> E["BuildConfigA / ServeConfigA\n(task, task_project, task_module,\ndataset, sample_timeout)"]
    D -- "B" --> F["BuildConfigB / ServeConfigB\n(harbor, partition, inner_task,\nfeedback levers)"]
    D -- "wrong-mode field present" --> G["ValidationError\n(extra='forbid')"]

    E --> H["compile_task / build_components\n(Mode-A branch)"]
    F --> I["compile_task / build_components\n(Mode-B branch)"]

    H --> J["serve.json with mode='A'\n+ task/task_project/sample_timeout"]
    I --> K["serve.json with mode='B'\n+ harbor/feedback levers"]

    J --> L["load_serve_config → ServeConfigA"]
    K --> M["load_serve_config → ServeConfigB"]

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
vero/src/vero/harbor/build/compiler.py:185-194
**`getattr` bypasses static type narrowing for Mode-B-only fields**

`getattr(config, "inner_task", None)` and `getattr(config, "instruct_multifidelity", False)` are safe at runtime (the fields exist on `BuildConfigB` and not on `BuildConfigA`), but they silently return the default for any misspelling (e.g. `"iner_task"` → `None`) instead of producing a NameError or type-checker error. Every other Mode-B check in this function uses `isinstance(config, BuildConfigB)`, so the two `getattr` calls are inconsistent and would be flagged by mypy/pyright.

### Issue 2 of 2
vero/src/vero/harbor/serve.py:232-253
**Implicit attribute access on union type bypasses type narrowing**

Lines 238–251 access `config.task_project`, `config.task_module`, `config.task`, and `config.sample_timeout` guarded only by the `is_mode_a` boolean. Python's short-circuit evaluation makes this safe at runtime, but the type of `config` is still `ServeConfigA | ServeConfigB` at those sites, so mypy/pyright reports `ServeConfigB has no attribute task_project` (etc.). Using a narrowed local or extracting the values inside an `if isinstance(config, ServeConfigA):` block would make the types check cleanly and avoid any future surprise if the guard is ever refactored.

```suggestion
    if isinstance(config, ServeConfigA):
        _task_project = Path(config.task_project) if config.task_project else None
        _task_module = config.task_module
        _default_task = config.task
        _sample_timeout = config.sample_timeout
    else:
        _task_project = None
        _task_module = None
        _default_task = None
        _sample_timeout = config.timeout
    evaluator = Evaluator(
        workspace,
        config.session_id,
        vero_home=vero_home,
        use_copy=config.use_copy,
        task_project=_task_project,
        task_module=_task_module,
        eval_strategy=eval_strategy,
    )

    db = ExperimentDatabase(id=config.session_id)  # shared by engine (writes) + verifier (reads)
    engine = EvaluationEngine(
        evaluator=evaluator,
        budget=budget,
        default_task=_default_task,
        db=db,
        run_constraints=BaseEvaluationParameters(
            timeout=config.timeout,
            sample_timeout=_sample_timeout,
            max_concurrency=config.max_concurrency,
        ),
```

_{Reviews (1): Last reviewed commit: "refactor(harbor): split Mode A / Mode B ..." | Re-trigger Greptile}