Skip to content

Commit e22cbb4

Browse files
Add minimal verifier extension hook (harbor-framework#1653)
* Add minimal verifier extension hook Add a small verifier factory hook that allows jobs to provide an optional custom verifier by import path while keeping the existing task verification flow as the default. This enables job-specific verification to supplement task-specific checks. For example, a job can attach generic trajectory evaluators, policy checks, or run-level scoring logic across many tasks without rebuilding, copying, or modifying those task definitions. The hook keeps task authorship and job evaluation concerns separate: tasks continue to define their normal verification, and jobs can opt into additional verifier behavior only when needed. Default behavior is unchanged when no custom verifier is configured. Signed-off-by: Anuradha Karuppiah <[email protected]> * Tighten verifier extension contract Introduce BaseVerifier and VerifierContext so custom verifiers receive a stable construction context while the built-in verifier keeps legacy kwargs compatibility. Require verifier outputs to be VerifierResult before assigning them to trial results, preserving Harbor aggregation semantics for built-in and imported verifiers. Keep legacy import-path constructors working through an adapter that enforces the return contract. Signed-off-by: Anuradha Karuppiah <[email protected]> * Reject unused verifier kwargs Fail fast when verifier kwargs are provided without a verifier import path, since the built-in verifier does not consume arbitrary extension kwargs. This makes CLI/config mistakes visible instead of silently dropping values like --verifier-kwarg foo=bar. Signed-off-by: Anuradha Karuppiah <[email protected]> * Fix verifier factory test patch Update Windows multi-step verifier tests to patch VerifierFactory.create_verifier_from_config after trial verification moved behind the factory hook. Signed-off-by: Anuradha Karuppiah <[email protected]> * Simplify verifier extension constructor * Simplify verifier factory contract * Fix skills merge example config paths --------- Signed-off-by: Anuradha Karuppiah <[email protected]> Co-authored-by: Alex Shaw <[email protected]>
1 parent 42b5a86 commit e22cbb4

11 files changed

Lines changed: 419 additions & 67 deletions

File tree

examples/jobs/skills-merge/config.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ environment:
55
agents:
66
- name: oracle
77
skills:
8-
- examples/jobs/skills
8+
- examples/jobs/skills-merge/skills
99
tasks:
10-
- path: examples/jobs/runtime-skill-merge
10+
- path: examples/jobs/skills-merge/runtime-skill-merge

src/harbor/__init__.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
from harbor.trial.hooks import TrialEvent, TrialHookEvent
7979
from harbor.trial.queue import TrialQueue
8080
from harbor.trial.trial import Trial
81+
from harbor.verifier.base import BaseVerifier
8182
from harbor.verifier.verifier import Verifier
8283

8384
__version__ = importlib.metadata.version("harbor")
@@ -92,6 +93,7 @@
9293
"BaseAgent": ("harbor.agents.base", "BaseAgent"),
9394
"BaseEnvironment": ("harbor.environments.base", "BaseEnvironment"),
9495
"ExecResult": ("harbor.environments.base", "ExecResult"),
96+
"BaseVerifier": ("harbor.verifier.base", "BaseVerifier"),
9597
"Verifier": ("harbor.verifier.verifier", "Verifier"),
9698
"TrialQueue": ("harbor.trial.queue", "TrialQueue"),
9799
# Job models
@@ -170,6 +172,7 @@ def __getattr__(name):
170172
"BaseAgent",
171173
"BaseEnvironment",
172174
"ExecResult",
175+
"BaseVerifier",
173176
"Verifier",
174177
"TrialQueue",
175178
# Job models

src/harbor/cli/jobs.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,6 +1015,24 @@ def start(
10151015
show_default=False,
10161016
),
10171017
] = None,
1018+
verifier_import_path: Annotated[
1019+
str | None,
1020+
Option(
1021+
"--verifier-import-path",
1022+
help="Import path for custom verifier (module.path:ClassName).",
1023+
rich_help_panel="Job Settings",
1024+
show_default=False,
1025+
),
1026+
] = None,
1027+
verifier_kwargs: Annotated[
1028+
list[str] | None,
1029+
Option(
1030+
"--verifier-kwarg",
1031+
help="Additional verifier kwarg in the format 'key=value'.",
1032+
rich_help_panel="Job Settings",
1033+
show_default=False,
1034+
),
1035+
] = None,
10181036
disable_verification: Annotated[
10191037
bool,
10201038
Option(
@@ -1212,6 +1230,10 @@ def start(
12121230

12131231
if verifier_env is not None:
12141232
config.verifier.env.update(parse_env_vars(verifier_env))
1233+
if verifier_import_path is not None:
1234+
config.verifier.import_path = verifier_import_path
1235+
if verifier_kwargs is not None:
1236+
config.verifier.kwargs.update(parse_kwargs(verifier_kwargs))
12151237
if disable_verification:
12161238
config.verifier.disable = disable_verification
12171239

src/harbor/cli/trials.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,24 @@ def start(
329329
show_default=False,
330330
),
331331
] = None,
332+
verifier_import_path: Annotated[
333+
str | None,
334+
Option(
335+
"--verifier-import-path",
336+
help="Import path for custom verifier (module.path:ClassName).",
337+
rich_help_panel="Verifier",
338+
show_default=False,
339+
),
340+
] = None,
341+
verifier_kwargs: Annotated[
342+
list[str] | None,
343+
Option(
344+
"--verifier-kwarg",
345+
help="Additional verifier kwarg in the format 'key=value'.",
346+
rich_help_panel="Verifier",
347+
show_default=False,
348+
),
349+
] = None,
332350
task_git_url: Annotated[
333351
str | None,
334352
Option(
@@ -439,6 +457,10 @@ def start(
439457
config.verifier.override_timeout_sec = verifier_timeout_sec
440458
if verifier_env is not None:
441459
config.verifier.env.update(parse_env_vars(verifier_env))
460+
if verifier_import_path is not None:
461+
config.verifier.import_path = verifier_import_path
462+
if verifier_kwargs is not None:
463+
config.verifier.kwargs.update(parse_kwargs(verifier_kwargs))
442464

443465
if task_git_url is not None:
444466
config.task = TaskConfig(

src/harbor/models/trial/config.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ class VerifierConfig(BaseModel):
155155
override_timeout_sec: float | None = None
156156
max_timeout_sec: float | None = None
157157
env: dict[str, str] = Field(default_factory=dict)
158+
import_path: str | None = Field(default=None, exclude_if=lambda v: v is None)
159+
kwargs: dict[str, Any] = Field(default_factory=dict, exclude_if=lambda v: not v)
158160
disable: bool = False
159161

160162
@field_serializer("env")

src/harbor/trial/trial.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
from harbor.trial.hooks import TrialEvent, TrialHookEvent
3939
from harbor.utils.logger import logger as global_logger
4040
from harbor.utils.scripts import quote_shell_arg
41-
from harbor.verifier.verifier import Verifier
41+
from harbor.verifier.factory import VerifierFactory
4242

4343
TrialHookCallback = Callable[[TrialHookEvent], Awaitable[None]]
4444

@@ -291,7 +291,8 @@ async def _run_shared_verifier(
291291
step_name: str | None = None,
292292
) -> VerifierResult:
293293
with self.agent_environment.with_default_user(user):
294-
verifier = Verifier(
294+
verifier = VerifierFactory.create_verifier_from_config(
295+
self.config.verifier,
295296
task=self.task,
296297
trial_paths=self.paths,
297298
environment=self.agent_environment,
@@ -343,7 +344,8 @@ async def _run_separate_verifier(
343344
artifacts=artifacts,
344345
)
345346

346-
verifier = Verifier(
347+
verifier = VerifierFactory.create_verifier_from_config(
348+
self.config.verifier,
347349
task=self.task,
348350
trial_paths=self.paths,
349351
environment=target_env,

src/harbor/verifier/base.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
from __future__ import annotations
2+
3+
import logging
4+
from abc import ABC, abstractmethod
5+
from typing import Any
6+
7+
from harbor.environments.base import BaseEnvironment
8+
from harbor.models.task.task import Task
9+
from harbor.models.trial.paths import TrialPaths
10+
from harbor.models.verifier.result import VerifierResult
11+
from harbor.utils.logger import logger as global_logger
12+
13+
14+
class BaseVerifier(ABC):
15+
"""Base class for Harbor verifiers."""
16+
17+
def __init__(
18+
self,
19+
*,
20+
task: Task,
21+
trial_paths: TrialPaths,
22+
environment: BaseEnvironment,
23+
override_env: dict[str, str] | None = None,
24+
logger: logging.Logger | None = None,
25+
verifier_env: dict[str, str] | None = None,
26+
step_name: str | None = None,
27+
**_: Any,
28+
) -> None:
29+
self.task = task
30+
self.trial_paths = trial_paths
31+
self.environment = environment
32+
self.override_env: dict[str, str] = dict(override_env) if override_env else {}
33+
self.logger: logging.Logger = (logger or global_logger).getChild(__name__)
34+
self.verifier_env = verifier_env
35+
self.step_name = step_name
36+
37+
@abstractmethod
38+
async def verify(self) -> VerifierResult:
39+
"""Run verification and return a Harbor verifier result."""

src/harbor/verifier/factory.py

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import importlib
2+
import logging
3+
from typing import Any
4+
5+
from harbor.environments.base import BaseEnvironment
6+
from harbor.models.task.task import Task
7+
from harbor.models.trial.config import VerifierConfig
8+
from harbor.models.trial.paths import TrialPaths
9+
from harbor.verifier.base import BaseVerifier
10+
from harbor.verifier.verifier import Verifier
11+
12+
13+
class VerifierFactory:
14+
@classmethod
15+
def create_verifier_from_import_path(
16+
cls,
17+
import_path: str,
18+
*,
19+
task: Task,
20+
trial_paths: TrialPaths,
21+
environment: BaseEnvironment,
22+
override_env: dict[str, str] | None = None,
23+
logger: logging.Logger | None = None,
24+
verifier_env: dict[str, str] | None = None,
25+
step_name: str | None = None,
26+
**kwargs: Any,
27+
) -> BaseVerifier:
28+
if ":" not in import_path:
29+
raise ValueError("Import path must be in format 'module.path:ClassName'")
30+
31+
module_path, class_name = import_path.split(":", 1)
32+
try:
33+
module = importlib.import_module(module_path)
34+
except ImportError as exc:
35+
raise ValueError(f"Failed to import module '{module_path}': {exc}") from exc
36+
37+
try:
38+
verifier_class = getattr(module, class_name)
39+
except AttributeError as exc:
40+
raise ValueError(
41+
f"Module '{module_path}' has no class '{class_name}'"
42+
) from exc
43+
44+
if not isinstance(verifier_class, type):
45+
raise TypeError(f"Imported verifier '{import_path}' must be a class")
46+
if not issubclass(verifier_class, BaseVerifier):
47+
raise TypeError(
48+
f"Imported verifier '{import_path}' must subclass BaseVerifier"
49+
)
50+
51+
verifier_args = {
52+
"task": task,
53+
"trial_paths": trial_paths,
54+
"environment": environment,
55+
"override_env": override_env,
56+
"logger": logger,
57+
"verifier_env": verifier_env,
58+
"step_name": step_name,
59+
}
60+
return verifier_class(
61+
**verifier_args,
62+
**kwargs,
63+
)
64+
65+
@classmethod
66+
def create_verifier_from_config(
67+
cls,
68+
config: VerifierConfig,
69+
*,
70+
task: Task,
71+
trial_paths: TrialPaths,
72+
environment: BaseEnvironment,
73+
override_env: dict[str, str] | None = None,
74+
logger: logging.Logger | None = None,
75+
verifier_env: dict[str, str] | None = None,
76+
step_name: str | None = None,
77+
skip_tests_upload: bool = False,
78+
**kwargs: Any,
79+
) -> BaseVerifier:
80+
if config.import_path is not None:
81+
return cls.create_verifier_from_import_path(
82+
config.import_path,
83+
task=task,
84+
trial_paths=trial_paths,
85+
environment=environment,
86+
override_env=override_env,
87+
logger=logger,
88+
verifier_env=verifier_env,
89+
step_name=step_name,
90+
**config.kwargs,
91+
**kwargs,
92+
)
93+
94+
unused_kwargs = {**config.kwargs, **kwargs}
95+
if unused_kwargs:
96+
kwarg_names = ", ".join(sorted(unused_kwargs))
97+
raise ValueError(
98+
"Verifier kwargs require verifier.import_path. Set "
99+
f"--verifier-import-path or remove verifier kwargs: {kwarg_names}"
100+
)
101+
102+
return Verifier(
103+
task=task,
104+
trial_paths=trial_paths,
105+
environment=environment,
106+
override_env=override_env,
107+
logger=logger,
108+
verifier_env=verifier_env,
109+
step_name=step_name,
110+
skip_tests_upload=skip_tests_upload,
111+
)

0 commit comments

Comments
 (0)