Repository navigation
feat(v1): grade in an isolated box, with Harbor-native artifacts #2144
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.
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
c3eab90
feat(v1): grade in an isolated box, with Harbor-native artifacts
rasdani a657bd4
refactor(v1): slim artifacts.py, and narrow the symlink rule
rasdani a8c7896
refactor(v1): drop archive vetting, accept the tampered-tar risk
rasdani ab941d8
refactor(v1): rename capture_patch's `publish` to `write_path`
rasdani 6a18ba9
refactor(v1): drop CollectedArtifact, return a dict of root -> archive
rasdani af508ac
refactor(v1): move fire-and-forget teardown onto Runtime.stop_nowait
rasdani 85a3af6
refactor(v1): drop stop_nowait, tear the solver box down normally
rasdani 91fa03b
refactor(v1): drop typo guards on artifact sources
rasdani e787bc9
fix(v1): let an unpinned judge runtime inherit the solver's
rasdani cce0921
Merge origin/main into feat/isolated-grading-artifacts
rasdani 4492b14
fix(v1): resolve relative artifact sources against the runtime workdir
rasdani 8355eaf
refactor(v1): inline the workdir resolution
rasdani 9fe09e0
fix(v1): attribute patch-capture failures, and skip grading on infra …
rasdani 245f806
docs(v1): say why the failed-solver path returns instead of raising
rasdani a6b82e3
fix(v1): keep `shared` as the agentic-judge default
rasdani e7ecfaf
fix(v1): capture the agent's edits only, not the image's untracked files
rasdani 7236d4b
feat(v1): `snapshot_untracked`, the exact form of the same exclusion
rasdani 5564d3f
refactor(v1): drop the mtime cutoff, keep only the recorded set
rasdani 9ac6daf
refactor(v1): address isolated grading review feedback (#2160)
rasdani 615efbb
Merge origin/main into feat/isolated-grading-artifacts
rasdani 960d45b
refactor(v1): one mode-neutral workspace note
rasdani 132d4b3
Apply suggestion from @macroscopeapp[bot]
rasdani 4228c44
fix(v1): address isolated grading review feedback
hallerite 3f8917c
Merge remote-tracking branch 'origin/main' into codex/pr-2144-takeover
hallerite fef21cd
fix(v1): validate isolated judge runtime early
hallerite 96f2b59
fix(v1): disambiguate judge text sources
hallerite 858ac72
test(v1): give coding harnesses token headroom
hallerite File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,198 @@ | ||
| """Carry a task's declared files out of the agent's box and into a grading box. | ||
|
rasdani marked this conversation as resolved.
Resolved
|
||
|
|
||
| Grading in the box the agent worked in leaves a seam a policy under RL pressure will | ||
|
rasdani marked this conversation as resolved.
Resolved
|
||
| find. Grading in a second box removes it — only what the task declares crosses over. | ||
|
|
||
| Two channels, non-overlapping: `Trace.info` is the durable record (`capture_patch` puts | ||
| the diff there, a judge puts its verdict there) and never travels; `/logs/artifacts/` is | ||
| transport, Harbor's in-sandbox convention, collected with no declaration and restored at | ||
| the same path in the grading box ("no translation", as in Harbor). | ||
|
|
||
| `collect` runs while the agent's box is alive, right after `Task.finalize` produced the | ||
| files. It is the barrier: once it returns the box can be torn down in the background. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import asyncio | ||
| import logging | ||
| import shlex | ||
| import uuid | ||
| from dataclasses import dataclass | ||
| from pathlib import PurePosixPath | ||
| from typing import TYPE_CHECKING | ||
|
|
||
| from verifiers.v1.errors import ArtifactError | ||
| from verifiers.v1.types import StrictBaseModel | ||
|
|
||
| if TYPE_CHECKING: | ||
| from verifiers.v1.runtimes import Runtime | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| CONVENTION_DIR = "/logs/artifacts" | ||
|
rasdani marked this conversation as resolved.
Resolved
|
||
| """Harbor's in-sandbox publish directory, swept implicitly so a task that writes here | ||
| needs no declaration.""" | ||
|
|
||
| MAX_ARTIFACT_BYTES = 32 * 1024 * 1024 | ||
| """Ceiling per collection. Sized for a delta, not a tree: the grading box boots from the | ||
| agent's image, so the repo is already there and only its output has to travel.""" | ||
|
|
||
|
|
||
| class Artifact(StrictBaseModel): | ||
| """One path to carry into the grading box, where it lands at this same path. | ||
|
|
||
| Harbor's `ArtifactConfig` minus `destination` (host trial-directory placement, which | ||
| verifiers has no equivalent for) and `service` (compose sidecars, unsupported). | ||
| """ | ||
|
|
||
| source: str | ||
| exclude: list[str] = [] | ||
| """`tar --exclude` patterns, applied when `source` is a directory.""" | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class CollectedArtifact: | ||
| """One source, tarred out of the agent's box and held on the host in between. | ||
| Transient: only `Trace.info` is durable.""" | ||
|
|
||
| root: str | ||
| archive: bytes | ||
|
|
||
|
|
||
| async def collect( | ||
| runtime: Runtime, artifacts: list[Artifact] | None = None | ||
| ) -> list[CollectedArtifact]: | ||
| """Tar the convention dir and every declared path out of `runtime`. | ||
|
|
||
| A declared source that is missing raises: it was declared because grading needs it, | ||
| and grading a partial state scores the rollout wrong rather than failing it. The | ||
| implicit convention sweep is exempt — most tasks never write there. | ||
|
|
||
| One archive per source: BusyBox `tar` (every alpine-based image) implements only | ||
|
rasdani marked this conversation as resolved.
Resolved
|
||
| `c`/`x`/`t` with no `-r` to append, and each source carries its own excludes anyway. | ||
| """ | ||
| declared = [_checked(a) for a in artifacts or []] | ||
| convention = PurePosixPath(CONVENTION_DIR) | ||
| sweep = not any( | ||
| (p := PurePosixPath(a.source)) == convention | ||
| or p.is_relative_to(convention) | ||
| or convention.is_relative_to(p) | ||
| for a in declared | ||
| ) | ||
| entries = ([Artifact(source=CONVENTION_DIR)] if sweep else []) + declared | ||
|
cursor[bot] marked this conversation as resolved.
Resolved
|
||
|
|
||
| collected: list[CollectedArtifact] = [] | ||
| budget = MAX_ARTIFACT_BYTES | ||
| for artifact in entries: | ||
| source = artifact.source | ||
| if (await runtime.run(["test", "-e", source], {})).exit_code != 0: | ||
|
rasdani marked this conversation as resolved.
Resolved
|
||
| if sweep and source == CONVENTION_DIR: | ||
| continue | ||
| raise ArtifactError( | ||
| f"declared artifact {source!r} does not exist in the box; the task must " | ||
| "produce it in finalize() (or a [[verifier.collect]] hook)" | ||
|
rasdani marked this conversation as resolved.
Resolved
|
||
| ) | ||
|
cursor[bot] marked this conversation as resolved.
Resolved
|
||
| archive = await _tar_out(runtime, artifact, budget) | ||
|
macroscopeapp[bot] marked this conversation as resolved.
Resolved
|
||
| budget -= len(archive) | ||
| collected.append(CollectedArtifact(root=source, archive=archive)) | ||
|
|
||
| logger.debug("collected artifact roots: %s", [c.root for c in collected]) | ||
| return collected | ||
|
|
||
|
|
||
| async def restore(runtime: Runtime, collected: list[CollectedArtifact]) -> None: | ||
| """Extract `collected` in `runtime` at the original absolute paths.""" | ||
| if not collected: | ||
| return | ||
| # Extraction writes to absolute paths. In a container that is the point; under the | ||
|
hallerite marked this conversation as resolved.
Resolved
|
||
| # subprocess runtime it is the developer's own filesystem. | ||
| if getattr(runtime.config, "type", None) == "subprocess": | ||
| raise ArtifactError( | ||
| "refusing to restore artifacts into the subprocess runtime: extraction " | ||
| "writes to absolute paths on the host. Grade in a container." | ||
| ) | ||
| # Clear every root up front, not per entry: a later nested root would otherwise | ||
| # delete content an earlier one just restored. Clearing also drops any file or | ||
| # symlink the image left at the target. | ||
| roots = " ".join(shlex.quote(entry.root) for entry in collected) | ||
| await _run(runtime, f"rm -rf -- {roots}", "clear artifact roots") | ||
| for entry in collected: | ||
| path = f"/tmp/vf-artifact-{uuid.uuid4().hex}.tar" | ||
| await runtime.write(path, entry.archive) | ||
| await _run( | ||
| runtime, | ||
| f"tar -xf {shlex.quote(path)} -C / && rm -f {shlex.quote(path)}", | ||
| f"restore artifact {entry.root!r}", | ||
| ) | ||
|
|
||
|
|
||
| def release(runtime: Runtime) -> None: | ||
| """Begin `runtime`'s teardown and return without waiting for it. | ||
|
|
||
| Once `collect` has returned nothing needs the agent's box, and on a remote runtime | ||
| its teardown is an API round trip the grading box should not wait behind. Safe | ||
| inside a `provision()` block only if the caller detached it (`pop_all()`); otherwise | ||
| the context manager awaits a teardown anyway and nothing is gained. | ||
| """ | ||
| if runtime.stopped: | ||
| return | ||
| # Mark it synchronously, as `Runtime.stop` does: `create_task` only schedules, so | ||
| # otherwise a second call would start a second teardown. | ||
| runtime.stopped = True | ||
| task = asyncio.create_task(runtime.stop()) | ||
| # asyncio holds only a weak reference to a running task, so a fire-and-forget | ||
| # teardown can be collected mid-flight. | ||
| _PENDING.add(task) | ||
|
macroscopeapp[bot] marked this conversation as resolved.
Resolved
|
||
| task.add_done_callback(_PENDING.discard) | ||
|
|
||
|
|
||
| _PENDING: set[asyncio.Task] = set() | ||
|
|
||
|
|
||
| def _checked(artifact: Artifact) -> Artifact: | ||
| path = PurePosixPath(artifact.source) | ||
| if not path.is_absolute() or ".." in path.parts: | ||
| raise ArtifactError( | ||
| f"artifact source {artifact.source!r} must be an absolute path with no '..'" | ||
| ) | ||
| resolved = path.as_posix().rstrip("/") | ||
| if not resolved: | ||
| raise ArtifactError("artifact source '/' would sweep the whole image") | ||
| return artifact.model_copy(update={"source": resolved}) | ||
|
|
||
|
|
||
| async def _tar_out(runtime: Runtime, artifact: Artifact, budget: int) -> bytes: | ||
| path = f"/tmp/vf-artifact-{uuid.uuid4().hex}.tar" | ||
| excludes = " ".join(f"--exclude={shlex.quote(p)}" for p in artifact.exclude) | ||
| try: | ||
| await _run( | ||
| runtime, | ||
| f"tar -cf {shlex.quote(path)} -C / {excludes} -- " | ||
| f"{shlex.quote(artifact.source.lstrip('/'))}", | ||
| f"collect artifact {artifact.source!r}", | ||
|
cursor[bot] marked this conversation as resolved.
Resolved
|
||
| ) | ||
| # Size it in the box: an oversized collection is refused before it reaches host | ||
| # memory, not after. | ||
| sized = await runtime.run(["sh", "-c", f"wc -c < {shlex.quote(path)}"], {}) | ||
| if (raw := sized.stdout.strip()).isdigit() and int(raw) > budget: | ||
| raise ArtifactError( | ||
| f"artifact {artifact.source!r} takes the collection over the " | ||
| f"{MAX_ARTIFACT_BYTES} byte limit. The grading box boots from the " | ||
| "agent's image, so only the delta needs to travel — narrow the source " | ||
| "or add `exclude` patterns." | ||
| ) | ||
| return await runtime.read(path) | ||
|
cursor[bot] marked this conversation as resolved.
Resolved
|
||
| finally: | ||
| # Best-effort: the box is about to be destroyed and the name is unique per call. | ||
| try: | ||
| await runtime.run(["rm", "-f", path], {}) | ||
| except Exception: # noqa: BLE001 - cleanup must not mask a collection error. | ||
| logger.debug("failed to remove %s", path, exc_info=True) | ||
|
|
||
|
|
||
| async def _run(runtime: Runtime, command: str, action: str) -> None: | ||
| result = await runtime.run(["sh", "-c", command], {}) | ||
| if result.exit_code: | ||
| detail = (result.stderr or result.stdout).strip()[-500:] | ||
| raise ArtifactError(f"failed to {action}: {detail}") | ||
|
rasdani marked this conversation as resolved.
Resolved
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.