Skip to content

Harbor integration: serve Harbor task datasets through OpenEnv as trainable environments - #1036

Open
adithya-s-k wants to merge 48 commits into
huggingface:mainfrom
adithya-s-k:harbor-integration
Open

Harbor integration: serve Harbor task datasets through OpenEnv as trainable environments#1036
adithya-s-k wants to merge 48 commits into
huggingface:mainfrom
adithya-s-k:harbor-integration

Conversation

@adithya-s-k

@adithya-s-k adithya-s-k commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Closes #1035.

Serves Harbor's task datasets via OpenEnv for training compatibility.

Pick any Harbor dataset, pick any task in it, pick any supported agent harness, and pick a sandbox to run it on, and you get back a proper trainable rollout.

All you have to provide is the URL of a hosted vLLM started with token capture on (--return-tokens-as-token-ids --logprobs-mode processed_logprobs). Without those flags any OpenAI-spec endpoint still works, you just get evals rather than trainable rollouts.

The rollout comes back as the exact token ids and per-token logprobs of every model call the agent made, together with the task's own reward. Agent and sandbox are chosen per rollout rather than baked into the deployment, so one server covers the whole matrix.

The reason to consume Harbor rather than re-implement it: today each coding agent costs OpenEnv a whole environment package, and each package carries its own copy of an interception proxy that has to be correct about token ids. Harbor already decouples task, harness and sandbox behind one interface, with roughly 39 agents and 23 backends. One integration makes all of them trainable, and adding the next agent becomes a table entry rather than a package.

What a caller gets

Rewards alone do not train a policy. On-policy methods need, per turn, (prompt_token_ids, completion_token_ids, per_token_logps) plus the reward, and producing that tuple is what this PR is for. It cannot be reconstructed afterwards: re-rendering a prompt offline with apply_chat_template drifts from what the model actually saw, and a prompt off by a single token silently fragments one long conversation into several short ones. Capture has to happen on the wire, at rollout time.

The CLI

command what it does
openenv harbor info Reports what this machine can actually run: whether the LLM returns token ids, which sandbox backends have both working credentials and an importable SDK, which datasets resolve and how many tasks each holds, and which harnesses are validated. Read-only, boots nothing.
openenv harbor rollout Runs rollouts with no env server involved. Boots the capture proxy, publishes it so a remote sandbox can reach it, runs -n tasks and writes the full token-level JSON. Also the debugging path: if rollout works and serve does not, the fault is in the serving layer and nothing below it.
openenv harbor serve The env server: Task API for discovery, one long-running run_rollout MCP tool for execution, and a web UI. Refuses to start if the LLM cannot return token ids.
openenv harbor push Deploys the same server to a Hugging Face Space. Configuration travels as Space variables, provider credentials as Space secrets, and --dry-run prints exactly what would be sent first.
openenv harbor info    --llm-url $LLM --dataset org/train,org/eval
openenv harbor rollout --llm-url $LLM --dataset org/train --task-index 0 -n 5 --harness codex --sandbox modal
openenv harbor serve   --llm-url $LLM --dataset org/train,org/eval
openenv harbor push    --llm-url $LLM --dataset org/train,org/eval --repo-id you/harbor-env

--llm-url is required and has no default and no environment fallback, because an unset endpoint produces rollouts that look completely normal and carry no token ids.

How capture works

An OpenAI-spec proxy sits between the agent and the inference endpoint. Each agent is pointed at it by a per-agent seam, usually one environment variable, and the agent's API key is really a capture session id, which is how one proxy serves many concurrent rollouts without per-rollout ports.

flowchart LR
  A["agent, in a Harbor sandbox"] -->|"base URL = proxy<br/>API key = session id"| P["capture proxy"]
  P -->|"detect dialect, normalise to chat,<br/>force token ids and logprobs on"| E["vLLM"]
  E -->|"prompt_token_ids,<br/>sampled ids + logprobs"| P
  P -->|"replay in the agent's own dialect<br/>(SSE if it asked for SSE)"| A
  P --> G["rollout graph"]
Loading

Two properties make this general rather than per-agent. Nothing is tokenised locally: the engine tokenises each prompt in order to serve it and hands back prompt_token_ids, so turn k+1's prompt is by construction the canonical tokenisation of everything before it, tool results included. And four wire dialects are supported, because coding agents did not converge on one: chat-completions, OpenAI Responses, Anthropic Messages and Google generateContent.

The rollout graph

Turns are not appended to a list, they are linked by exact token prefix: a call whose prompt_token_ids begin with an existing node's full token sequence becomes that node's child. Nothing else is consulted, no request ids, no timestamps, no conversation headers, because those are per-agent and the prefix is not.

flowchart TD
  R["root: system + first user turn"] --> T1["turn 1"]
  T1 --> T2["turn 2"]
  T2 --> T3["turn 3"]
  T2 -.->|"same prefix, branch died"| T3b["turn 3', a retry"]
  S["second root: subagent,<br/>different system prompt"] --> S1["turn 1"]
Loading

That falls out into the structure a trainer needs. A root is a conversation that started fresh, so several roots mean the agent ran subagents or auxiliary calls rather than one long chain. A fork is a retry or resample. A path from root to leaf is one training sequence, on which every token is either a prompt token the model conditioned on or a sampled token with its logprob. Branches that led nowhere are marked discarded and excluded from paths while staying visible in the report.

Inspiration

Neither half of this is novel and we did not treat it as such. The dialect translation is adapted from the Polar gateway (Apache-2.0), which had already solved converting Anthropic, Responses and Google requests into chat-completions calls faithfully enough to replay in the original dialect; it is vendored into dialects/ rather than depended on because the package named polar on PyPI is unrelated, with provenance in dialects/README.md. Polar's engine and proxy layers are not vendored, since they target SGLang, which cannot return token ids at all (sgl-project/sglang#18378), so a ~160-line vLLM-only upstream.py replaces them.

verifiers solves the same problem from the other direction, and we referred to how its Dialect ABC handles two cases that are easy to get wrong: auxiliary routes, so a call like claude-code's count_tokens is answered without becoming a model turn, and per-dialect streaming detection, since Google signals streaming in the URL rather than the body.

Agents supported today

16 harnesses are validated end to end, grouped by the dialect they speak:

dialect agents
chat-completions opencode, goose, qwen-coder, swe-agent, mini-swe-agent, openhands-sdk, openclaw, hermes, kimi-cli, pi, vibe, terminus-2
OpenAI Responses codex, trae-agent
Anthropic Messages claude-code
Google generateContent gemini-cli

Supporting all four dialects rather than chat-completions alone is what buys the last four rows. terminus-2 runs host-side in the server process, so it needs no public URL. Anything else Harbor supports can be reached with --harness module:Class, and adding it properly means one entry in the seam table.

Validation against ATIF

Capture is checked against Harbor's own trace format, ATIF, which the harness writes independently of anything here. Reconciliation compares the two call by call: turn count, per-call completion token counts, and which calls the harness considers real agent steps rather than auxiliary. A rollout comes back as atif="match", "MISMATCH" or "none" when the harness emits no trajectory.

This matters because it is the only check that is not self-referential. The proxy could be internally consistent and still wrong, and a mismatch has already caught a real bug: one harness sending an empty tools array got a 400 from vLLM, which truncated its trajectory while leaving a graph that looked perfectly well-formed. Calls that ATIF marks auxiliary are also demoted so they cannot be credited with the reward earned by solving the task.

Validation runs on ingest rather than export, because a turn whose logprobs are misaligned has to be caught while we still know which turn it was.

Sandboxing

All of it is Harbor's. This PR adds no sandbox code, no provider SDK imports and no image building: a TrialConfig names an environment type and Harbor does the rest, which is what makes 23 backends available instead of the two someone would have hand-written.

Worth stating because the words collide: every OpenEnv provider (local_docker, hf_sandbox, modal, aca, daytona, uv) is a ContainerProvider that hosts the env server and has no exec. Harbor's backends are the agent-exec sandboxes, and --sandbox refers to those. Availability is asked for rather than assumed: a backend counts as usable only if its class imports and Harbor's preflight() passes, since a provider with valid credentials but no SDK installed otherwise reports available and fails at rollout time.

Reward

Harbor's verifier produces a dict[str, float] and OpenEnv wants a scalar. The dict travels verbatim and the scalar is chosen by an explicit rule: one key, or one named reward, otherwise fail and require --reward-key. Combining keys automatically would be inventing reward semantics, and shaping belongs to the trainer.

reward=None is not zero. It means the verifier never ran, and conflating the two makes a dead sandbox look like a wrong answer.

Real-time updates

A rollout takes minutes, so it can be watched while it runs. The capture proxy exposes GET /sessions with per-session turn count, root count and seconds since the last model call, updated as calls land, and the UI streams the same numbers before rendering the finished graph and the per-turn token ids and logprobs behind an accordion. It also separates the two ways a rollout can look stuck: no session yet means the sandbox is still booting, while a session with zero turns means the agent is installed but has not called the model.

The failure model

A failed rollout returns a result, never an exception: HarborRolloutResult(ok=False, reward=None, error=...). This is the architectural reason the layer exists. In the in-process predecessor a rollout exception reached the trainer and hung every rank at the NCCL barrier forever, which is why trl.experimental.harbor runs to ~400 lines with nearly every environment call individually wrapped in try/except. Behind an HTTP boundary that failure class cannot occur, and eval and training collapse onto one code path so hardening applies to both.

Structure

path
src/openenv/core/harness/capture/ Dialect-agnostic capture: proxy, rollout graph, ingest validation, LLM certification, port forwarding. No Harbor knowledge.
src/openenv/harbor/ Harbor specifics: seams, task discovery, rollout, capabilities, serving, UI, client.
envs/harbor_env/ Deployment packaging only: manifest, Dockerfile, ASGI entry point.
src/openenv/cli/commands/harbor.py info / rollout / serve / push.

Capture lives in core because it is the piece every future agent environment would otherwise duplicate, and openenv.harbor sits alongside core/cli/auto rather than inside envs/ so it can be shared. Keeping it all in envs/harbor_env would be a smaller diff and would forfeit exactly the reuse this is for.

Two ports locally: the env server faces trainers and browsers, the capture proxy faces the sandbox and is the only one published. A single port would expose the env server as soon as the proxy became reachable. Hosted, that inverts. A Space has one port and one URL, so the proxy is mounted on the env server's own app at /capture and reached at <space-url>/capture, with nothing forwarded. The Space has to be public for that to work, since a private one requires an auth header the agent inside the sandbox does not send. Public is safe here because the proxy rejects any caller without a registered session id, so the mount is not an open relay.


Note

High Risk
Large new surface area spanning LLM proxying, multi-dialect API translation, sandbox rollouts, and HF Space deployment—mistakes in capture level or logprob handling could silently produce non-trainable or wrong training data.

Overview
Adds Harbor as a first-class OpenEnv path: run Harbor tasks with a chosen agent harness and sandbox, and get verifier rewards plus full traces—and, when the LLM supports it, per-turn prompt_token_ids, completion_token_ids, and per_token_logprobs for on-policy training.

The new openenv harbor subcommands are info (capabilities probe), rollout (local runs without a server), serve (Task API + run_rollout MCP + /web UI), and push (HF Space deploy with dataset bucket/volume wiring and bundle pruning). Root pyproject.toml gains an optional harbor extra (Python ≥3.12, explicit Harbor sandbox backends, avoiding unsatisfiable harbor[cloud] deps).

src/openenv/core/harness/capture/ is new shared infrastructure: an intercept proxy (session id as API key), rollout graph stitched by token prefix, LLM startup validation (train vs eval, processed logprobs, tool-calling probes), provider 400 auto-fixes, and vendored four-dialect transformers (chat, Responses, Anthropic, Google). envs/harbor_env/ packages the FastAPI/Space deployment; docs and .gitignore add Harbor/Gradio artifacts. scripts/logprob_parity.py checks captured logprobs against engine rescoring.

Reviewed by Cursor Bugbot for commit 5986b79. Bugbot is set up for automated code reviews on this repo. Configure here.

An OpenAI-spec proxy that sits between a coding agent and an inference
endpoint and records the exact token ids and per-token logprobs of every
model call, so a rollout is trainable.

Nothing is tokenised locally: the engine returns prompt_token_ids, so turn
k+1's prompt is the canonical tokenisation of everything before it and turns
link by exact token prefix. Re-rendering a prompt offline drifts from what
the model saw, and a drifted prompt silently fragments one conversation into
several.

Four wire dialects (chat-completions, OpenAI Responses, Anthropic Messages,
Google generateContent), adapted from the Polar gateway (Apache-2.0);
provenance in dialects/README.md. Vendored because the package named polar
on PyPI is unrelated. Two ideas are borrowed from verifiers: aux routes, so
a count_tokens call is answered without becoming a model turn, and
per-dialect streaming detection, since Google signals streaming in the URL.

Includes engine certification, which refuses an endpoint that cannot return
token ids, and port forwarding for sandboxes that cannot reach localhost.
Serves Harbor's task datasets over the Task API and runs a rollout through
one long-running MCP tool, with the agent and the sandbox chosen per call
rather than baked into the deployment.

A failed rollout returns a result, never an exception. That is the reason
this layer exists: in the in-process predecessor a rollout exception reached
the trainer and hung every rank at the NCCL barrier, which is why
trl.experimental.harbor wraps nearly every environment call individually.
Behind an HTTP boundary that failure class cannot occur.

Rewards are forwarded, never recomputed. Harbor's dict travels verbatim and
the scalar is chosen by an explicit rule, refusing rather than guessing when
several keys exist. reward=None is not zero: it means the verifier never ran,
and conflating them makes a dead sandbox look like a wrong answer.

Sandbox availability is asked for rather than assumed. A backend counts as
usable only if its class imports, its SDK is present, and Harbor's own
preflight passes; checking credentials alone reports a backend available and
then fails at rollout time.

Hosted deployments mount the capture proxy on the env server's own app, since
a Space has one port and one public URL and nothing needs forwarding there.
info reports what this machine can actually run. rollout runs one end to end
with no server involved, which halves the search space when something breaks:
if rollout works and serve does not, the fault is in the serving layer.
serve is the env server; push deploys the same thing to a Space.

--llm-url is required with no default and no environment fallback, because an
unset endpoint produces rollouts that look completely normal and carry no
token ids.

push attaches the task suites as a bucket volume mounted at /data instead of
downloading them: a Harbor suite is thousands of small files and Space disk is
ephemeral, so a download is re-paid on every restart. Copies are server side,
by xet hash. The mount is verified before the server is pointed at it, and it
falls back to downloading rather than reading paths that may not exist.

The harbor extra installs every sandbox backend. Not harbor[cloud], which is
unsatisfiable: it pulls langsmith[sandbox] and tensorlake, which demand
incompatible websockets ranges.
Manifest, Dockerfile and ASGI entry point only; the logic lives in
openenv.harbor so the capture layer can be shared rather than duplicated per
agent environment.

The Dockerfile pins UV_PYTHON_INSTALL_DIR and copies it across the stage
boundary. Harbor needs Python >= 3.12 while openenv-base ships 3.11, so uv
downloads its own interpreter and the venv's bin/python is a symlink into it;
copying only .venv leaves a dangling link and the container dies with
'not found'. A build-time assertion now catches that at build rather than at
startup.

The entry point resolves and validates the served model the way harbor serve
does. Without it the proxy has no served model id and forwards whatever name
the harness used straight to the engine.
Each of these pins a failure that was silent in production and cheap to
reintroduce. No credentials or network needed.

Port ownership: a capture server used to report healthy on a port another
process owned, because the liveness probe connected to the incumbent while its
own bind error died unobserved on a background thread. Sessions were then
minted in one registry and rejected by another, producing a 401 and a rollout
with zero model calls.

Request normalisation: kimi-cli sends tools: [] once its loop has no tools
left, and vLLM rejects an empty array outright, truncating the trajectory
while leaving a well-formed graph behind.

Hosted serving: a Space must mount the capture proxy rather than forward it.
One test monkeypatches make_forwarder to raise, so a hosted deployment that
ever tries to forward fails the suite.
Copilot AI lite review requested due to automatic review settings August 2, 2026 19:24
@bot-ci-comment

bot-ci-comment Bot commented Aug 2, 2026

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

check-env-docs generates a docs stub per environment README and fails when one
is missing, which it was.

The README line about the capture proxy being the only thing forwarded
publicly predated the hosted path and was wrong for a Space, where there is one
port and one public URL and the proxy is mounted rather than forwarded. Fixed
in the README so the generated stub follows.

_toctree.yml is maintained by hand, so the generated page needs an entry there
or it exists without being reachable from the sidebar.
Comment thread src/openenv/core/harness/capture/forwarding.py Outdated
Comment thread src/openenv/harbor/models.py Outdated
Comment thread src/openenv/core/harness/capture/server.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

There are correctness issues in the capture/training contract plumbing (async asyncio.run usage, incomplete per-turn prompt IDs, and dropping additional agent roots) that would cause silent data loss or runtime failures in valid usage paths.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Adds a Harbor-backed environment integration that can serve Harbor task datasets through OpenEnv and produce trainable rollouts by capturing engine-native token IDs + per-token logprobs (via a multi-dialect capture proxy), plus Harbor verifier rewards.

Changes:

  • Introduces openenv.core.harness.capture: a capture proxy + rollout-graph + validation/export utilities (incl. dialect adapters and port-forwarding).
  • Adds openenv.harbor package: dataset discovery, capabilities/preflight reporting, rollout runner, serving layer (including hosted “single-port” mounting behavior), typed client, and ATIF reconciliation.
  • Adds deployable envs/harbor_env packaging (Space/FastAPI entrypoint + Dockerfile) and wires a new openenv harbor CLI group + openenv[harbor] extra.
File summaries
File Description
tests/envs/test_harbor_hosted_serving.py Pins single-port hosted/Space behavior (mount capture; never forward).
tests/envs/test_harbor_capture_server.py Pins capture server port-ownership + instance-identity invariants.
tests/envs/test_harbor_capture_normalise.py Asserts request normalization that avoids vLLM 400s that silently truncate rollouts.
src/openenv/harbor/tasks.py Implements dataset spec resolution (HF repo/local/registry) with caching + prefetch.
src/openenv/harbor/startup.py Startup gating/preflight (LLM capture capability, sandboxes, datasets) with report rendering.
src/openenv/harbor/serving.py Serving layer that chooses between forwarding (local) vs mount-at-/capture (hosted).
src/openenv/harbor/runner.py CLI rollout runner: boot capture + forwarder, run tasks, print batch reports.
src/openenv/harbor/rollout.py Core rollout execution and “never raise” result shaping, plus ATIF reconciliation integration.
src/openenv/harbor/models.py Wire models (HarborRolloutResult, HarborTurn, etc.) and document-to-wire transformations.
src/openenv/harbor/environment.py MCPEnvironment wrapper exposing run_rollout + discovery tools and Task API duck-typing.
src/openenv/harbor/client.py Typed client for Task API + MCP tool execution with long timeouts.
src/openenv/harbor/capabilities.py Capability discovery for harnesses/sandboxes/datasets, using Harbor preflight.
src/openenv/harbor/atif.py ATIF ingest + reconciliation, and optional merge of captured tokens/logprobs into ATIF.
src/openenv/harbor/init.py Package overview and dependency/layering notes.
src/openenv/core/harness/capture/validate.py Validation logic for per-turn/per-sequence/per-rollout invariants.
src/openenv/core/harness/capture/validate_llm.py Live probe to certify LLM returns token IDs + logprobs required for capture.
src/openenv/core/harness/capture/upstream.py vLLM-only upstream client + request/response normalization.
src/openenv/core/harness/capture/sse.py Synthetic SSE replay: capture non-streaming, respond streaming for harness compatibility.
src/openenv/core/harness/capture/sessions.py Session multiplexing/routing (API key == session id) and session summaries.
src/openenv/core/harness/capture/graph.py Prefix-linked rollout graph + training-sequence flattening.
src/openenv/core/harness/capture/forwarding.py Port forwarder strategies (direct/gradio/cloudflare) with preflight + reliability constraints.
src/openenv/core/harness/capture/export.py Export graph to validated JSON training document + role assignment.
src/openenv/core/harness/capture/dialects/reasoning.py Reasoning/thinking block round-trip helpers used by dialect transformers.
src/openenv/core/harness/capture/dialects/README.md Provenance + transformer scope/notes for vendored dialect code.
src/openenv/core/harness/capture/dialects/openai_chat.py Chat-completions transformer shim.
src/openenv/core/harness/capture/dialects/images.py Multimodal/image block conversions across dialects.
src/openenv/core/harness/capture/dialects/base.py Base transformer + request normalization helpers (developer role merge, per-model fixes).
src/openenv/core/harness/capture/dialects/init.py Transformer dispatch manager by detected API dialect.
src/openenv/core/harness/capture/detection.py Dialect detection logic (path/header/body heuristics).
src/openenv/core/harness/capture/contract.py Adapter layer exporting capture to downstream consumer “contracts” (TRL, per-turn records).
src/openenv/core/harness/capture/init.py Public exports for the capture subsystem.
src/openenv/cli/main.py Adds openenv harbor Typer subcommand group.
pyproject.toml Adds openenv[harbor] optional extra with Python>=3.12 marker.
envs/harbor_env/server/Dockerfile Space/deployment image build (uv + Python 3.12 carry-through) and runtime entrypoint.
envs/harbor_env/server/app.py ASGI app that validates LLM, starts/mounts capture, and builds the env server app.
envs/harbor_env/server/init.py Package marker for deployed server module.
envs/harbor_env/README.md Environment-level usage + config docs for Space deployment.
envs/harbor_env/pyproject.toml Environment packaging deps (openenv + harbor extras + server deps).
envs/harbor_env/openenv.yaml OpenEnv deployment manifest for the harbor_env Space runtime.
envs/harbor_env/models.py Re-export wire types for harbor_env.* parity with other env packages.
envs/harbor_env/client.py Re-export typed client for environment package ergonomics.
envs/harbor_env/init.py Env package overview + re-exports.
.gitignore Ignores Gradio UI build artifacts.
Review details
  • Files reviewed: 51/53 changed files
  • Comments generated: 4
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread src/openenv/harbor/tasks.py
Comment thread src/openenv/harbor/models.py Outdated
Comment thread src/openenv/core/harness/capture/contract.py Outdated
Comment thread src/openenv/harbor/capabilities.py
Copilot AI review requested due to automatic review settings August 2, 2026 19:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

The current per-turn export populates prompt_token_ids only for the first turn (breaking the stated training contract), and there are a couple of concrete operational/error-message issues that should be corrected before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (4)

src/openenv/harbor/capabilities.py:168

  • The missing-SDK hint tells users to install harbor[cloud], but this PR explicitly documents that harbor[cloud] is unsatisfiable (see envs/harbor_env/pyproject.toml and root extra rationale). This message will send operators down a dead-end and hide the real remediation.
    src/openenv/harbor/models.py:242
  • turns_from_document only includes prompt_token_ids for the very first emitted turn (if index == 0 else []). This contradicts the stated training contract (“per turn (prompt_token_ids, completion_token_ids, per_token_logps)”) and causes every later turn in contract.json to have an empty prompt, making exact prompt-token fidelity impossible for multi-turn rollouts.
    src/openenv/harbor/tasks.py:165
  • This download path is meant to avoid HF's symlink-based snapshot layout (so Harbor's tar uploads don't preserve dangling symlinks), but snapshot_download can still create symlinks depending on huggingface_hub settings/version. Setting local_dir_use_symlinks=False makes the “real files” guarantee explicit and future-proof.
    envs/harbor_env/server/app.py:77
  • _service.start() can create background resources (capture server thread and/or external forwarder subprocess) when this module is run outside Spaces. Because startup happens at import time and there is no shutdown hook, those resources may leak until process exit (and named forwards can persist even longer). Register a shutdown handler so the service is always torn down cleanly.
# Resolve capture before the app is built. A Space gives no separate boot hook, the UI needs the
# proxy's public URL to exist by the time anyone presses Run, and `build_app` has to see the service
# in order to mount it.
if _LLM_URL:
    _service = HarborService(
  • Files reviewed: 51/53 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Copilot AI review requested due to automatic review settings August 2, 2026 19:35
Takes harbor coverage from 19 tests to 112, no credentials or network needed.
The areas chosen are the ones that fail silently rather than loudly: a bug in
any of them produces plausible training data instead of an error.

  graph        prefix linking, roots, forks, discarded branches, loss masking
  rewards      the explicit selection rule, including 0.0 vs None
  seams        model-name normalisation, session threading, dialect coverage
  discovery    ordering stability, the symlink regression, spec classification
  validation   ingest checks and sandbox SDK detection
  rendering    result models, verdict states, contract.json

Two real bugs surfaced while writing them.

Google streaming requests were misclassified. `detect` tested
`"generateContent" in path`, but the streaming variant capitalises the G, so
every `:streamGenerateContent` call fell through to chat-completions and would
have been parsed by the wrong transformer. `wants_stream` already lowercased
the path; `detect` did not. gemini-cli passed the sweep because it used the
non-streaming route.

Anthropic tool calls were absent from results. `models._tool_calls` read only
the chat-completions `tool_calls` key, so claude-code's `tool_use` content
blocks never reached `HarborTurn`, leaving `contract.json` and the rendered
conversation showing an agent that produced text and took no actions.

Two expectations of mine were wrong rather than the code, and are now pinned
as behaviour: an empty served model raises instead of returning an empty
string, and a turn that sampled nothing warns rather than invalidating the
rollout.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

The current implementation has a few concrete contract/API mismatches (notably capture contract node selection and HarborEnv export/docs consistency) plus a misleading install hint that should be corrected before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (5)

src/openenv/core/harness/capture/contract.py:43

  • _agent_nodes() only keeps nodes from the first role == "agent" sequence/root. This contradicts export._assign_roles()’s documented behavior that multiple agent roots are normal (e.g. harnesses that rewrite prompts mid-run) and will silently drop valid agent turns from to_turn_records() / to_trace_entries() output.
def _agent_nodes(graph: RolloutGraph, document: dict[str, Any]) -> list[TurnNode]:
    """Nodes on the agent's conversation, in arrival order, excluding discarded retries."""
    agent_rows = [r for r in document["sequences"] if r["role"] == "agent"]
    if not agent_rows:
        return []
    root = agent_rows[0]["root_id"]
    keep = set(agent_rows[0]["node_ids"])
    return [
        n
        for n in graph.nodes()
        if graph.root_of(n.node_id) == root and n.node_id in keep
    ]

src/openenv/harbor/capabilities.py:168

  • The missing-SDK hint recommends pip install 'harbor[cloud]', but the repo’s own dependency comments state harbor[cloud] is unsatisfiable (see root pyproject.toml harbor extra). This message will send users toward an install path that can’t work.
    src/openenv/harbor/models.py:257
  • turns_from_document() only populates prompt_token_ids for index == 0 and leaves it empty for later turns. That conflicts with the stated training contract (“per turn -> (prompt_token_ids, completion_token_ids, per_token_logps)”) and the HarborTurn docstring implying prompt_token_ids is defined for each turn.
    envs/harbor_env/init.py:9
  • The package docs/examples use from harbor_env import HarborEnv, but harbor_env/__init__.py doesn’t export HarborEnv (it only exports models). Either the docs are wrong or this module should re-export the client like other env packages (e.g. opencode_env).
from openenv.harbor.models import HarborRolloutResult, HarborTaskRef, HarborTurn

__all__ = ["HarborRolloutResult", "HarborTaskRef", "HarborTurn"]

src/openenv/harbor/environment.py:28

  • SUPPORTS_CONCURRENT_SESSIONS = True makes this environment explicitly support multiplexed concurrent trajectories on one server instance. This appears to conflict with the documented design principle “One env = one trajectory” (PRINCIPLES.md), so it would be good to confirm this is an intentional exception for harbor_env (and that downstream trainers/collectors won’t assume 1:1 env↔trajectory).
  • Files reviewed: 56/58 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

`_PROC_ENV_LOCK` guards `os.environ` while an agent is constructed, because
Harbor's wrappers read credentials there rather than from the config. It was an
`asyncio.Lock`, which binds to the first event loop that uses it and then raises
"is bound to a different event loop" for every other one.

Rollouts arrive on several loops. The env server answers each request on its
own, and any caller using `asyncio.run` per rollout creates another. So the
first concurrent rollout succeeded and the rest failed instantly, with zero
model calls and no useful error.

It passed every test and every sequential run, and only appeared under real
concurrency: 96 of 98 rollouts failed within seconds of the first parallel
sweep.

`threading.Lock` is the right primitive: the resource is global to the process,
not to a loop. It is a blocking acquire inside an async function, which is
acceptable only because construction does no I/O worth speaking of, the sandbox
is booted later by `trial.run()` outside the lock.

The regression test drives the lock from eight event loops at once, which is
the shape that failed.
Copilot AI review requested due to automatic review settings August 2, 2026 19:46
Comment thread src/openenv/core/harness/capture/contract.py Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

The current rollout contract output drops required per-turn prompt token ids and also truncates multi-root agent sequences, which can silently break training data correctness.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (5)

src/openenv/harbor/capabilities.py:168

  • The missing-SDK hint recommends installing harbor[cloud], but this PR explicitly documents harbor[cloud] as unsatisfiable due to dependency conflicts. Point users to openenv[harbor] (or backend-specific extras) to avoid sending them to an installation dead end.
    src/openenv/core/harness/capture/contract.py:38
  • _agent_nodes only keeps the first agent sequence/root. This drops additional agent roots (e.g. harnesses that rewrite system prompts mid-run), contradicting the capture layer’s own stance that multiple agent roots can be legitimate agent work and should remain trainable.
    agent_rows = [r for r in document["sequences"] if r["role"] == "agent"]
    if not agent_rows:
        return []
    root = agent_rows[0]["root_id"]
    keep = set(agent_rows[0]["node_ids"])

src/openenv/harbor/models.py:252

  • turns_from_document only includes prompt_token_ids for the first turn; later turns get []. Since _write_contract() serializes prompt_token_ids per turn, this produces contract files with missing prompt token ids for multi-turn rollouts, violating the stated training tuple contract.
    envs/harbor_env/README.md:28
  • This doc claims the server refuses to start when the LLM cannot return token ids, but the Space ASGI entry point (envs/harbor_env/server/app.py) intentionally boots even when LLM validation fails (to surface the error in the UI/capabilities). The docs should reflect this hosted vs CLI behavior difference.
| `MODAL_TOKEN_ID`, `MODAL_TOKEN_SECRET` | offer the `modal` sandbox |

docs/source/environments/harbor.md:28

  • This environment doc says the server refuses to start if the LLM lacks token-id capture, but the Space entry point is designed to boot and report llm.ok=false so the UI can show the fault. Align the docs with the hosted behavior (or change the Space entry point to hard-fail).
Without them it answers every request normally and returns no token ids, so captured rollouts are
empty and nothing reports an error. The server refuses to start rather than let that happen.
  • Files reviewed: 56/58 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

`ok` is what a trainer filters on, so it has to mean "this row is usable". It
did not. Only trace reconciliation could clear it, while the capture document's
own validation findings were recorded in `findings` and otherwise ignored.

A 98-rollout parallel sweep surfaced the consequence: four rollouts came back
`ok=True` with zero model calls and zero trainable tokens, because
reconciliation agreed with the capture when both sides were empty. One of them
carried reward=1.0, which is the worst available shape, a row with nothing in it
and a positive reward attached.

Any FATAL from document validation now clears `ok` and becomes the error, so
"the intercept saw no model calls" is reported as a failed rollout rather than a
successful empty one.
Copilot AI review requested due to automatic review settings August 2, 2026 20:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

Several concrete correctness/operability issues were found in the changed code paths (per-turn prompt ids missing in turn rows, unsafe asyncio.run usage, brittle top_logprobs handling, misleading install guidance, and a capture-server lifecycle leak on forwarder failures).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (3)

src/openenv/harbor/capabilities.py:168

  • The missing-SDK hint currently recommends pip install 'harbor[cloud]', but this PR’s own pyproject.toml notes that harbor[cloud] is unsatisfiable due to conflicting websockets constraints. This message will send users toward an install that cannot succeed.
    src/openenv/harbor/serving.py:104
  • If make_forwarder(...) or forwarder.start(...) fails, the capture server has already been started and will be left running. That leaks a listener/port and can make subsequent starts fail with “already in use”.
    envs/harbor_env/init.py:9
  • Docs/examples import HarborEnv via from harbor_env import HarborEnv, but harbor_env/__init__.py doesn’t export it (only models). This makes the quickstart import fail for users.
from openenv.harbor.models import HarborRolloutResult, HarborTaskRef, HarborTurn

__all__ = ["HarborRolloutResult", "HarborTaskRef", "HarborTurn"]
  • Files reviewed: 56/58 changed files
  • Comments generated: 3
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread src/openenv/core/harness/capture/upstream.py Outdated
Comment thread src/openenv/harbor/environment.py Outdated
Comment thread src/openenv/harbor/models.py
Copilot AI review requested due to automatic review settings August 3, 2026 07:15
--api-key and --auth-header on info, rollout, serve and push, defaulting to
$OPENENV_LLM_API_KEY, threaded down to the proxy's upstream client and into the
Space entry point. push sends it as a Space secret, never a variable, since
variables are readable from the repo page.

Every result now says which kind of rollout it is: rollout_type, capture_level,
and the list of parameter workarounds that were applied to produce it. A
rewritten request is a changed experiment — dropping temperature alters the
sampling distribution, forcing reasoning_effort off changes the model — so it
travels with the data rather than living in a log. The startup report prints
TRAIN or EVAL ONLY in the same column as a failure, because that line decides
whether anything the server produces can be trained on.

startup no longer refuses an endpoint that cannot return token ids; only an
unreachable one is fatal. ATIF reconciliation compares call counts rather than
per-call token counts on the eval path, since the token counts do not exist —
still enough to catch a truncated harness trajectory, which is the one real bug
it has ever found.

The seam's process env now spans trial.run(), not just Trial.create. Harbor's
goose wrapper reads os.environ["OPENAI_API_KEY"] inside its own run, so
restoring the env in between handed it the operator's real provider key and our
own proxy answered 401 on every call — identically against all five upstreams,
which is what identified it as the key rather than any endpoint. os.environ is
process-global, so rollouts of a proc-env seam now serialise; that is a property
of an agent reading a global at run time, not a shortcut. Acquired via
to_thread, or the first two overlapping goose rollouts would deadlock the loop.
Ran all 16 validated harnesses against five upstreams, three tasks each. Four
harnesses failed on every upstream or on one, and every cause was on our side.

gemini-cli could not install at all: InterceptGeminiCli called
ensure_system_dependencies, which does not exist in Harbor 0.20.0, so the
install died with an AttributeError and zero model calls. Harbor's own wrapper
now apt-installs curl itself, so the subclass mirrors that invocation and adds
the bash the nvm snippet still needs.

kimi-cli was never broken. Every rollout captured turns with atif=match, up to
1320 trainable tokens, and then died in Harbor's result collection. The guard
for kimi's `kill 0` teardown only matched RemoteProtocolError/StreamReset;
0.20.0 raises the same event as ConnectError: Error reading content. The guard
lists the spellings now, and the safety net is downstream: a genuinely
unreachable sandbox makes no model calls and check_rollout fails it anyway.

codex's failure against OpenAI was the orphaned parallel_tool_calls, fixed in
the capture layer. What remains is codex preferring a Responses WebSocket
transport and burning five retries before falling back (openai/codex#19821).
Not worked around here: supports_websockets=false needs a custom provider, since
codex refuses to override a built-in one, and a custom provider needs its
base_url threaded through six chained -c overrides. Noted on the seam so nobody
repeats the attempt. It is survivable — codex hits the same 403 twelve times
per rollout against gpt-4o-mini and still finishes the task.

Result: 11 of 16 harnesses clean on all five upstreams. The rest are not
compatibility problems — swe-agent and trae-agent exceed the 900s agent deadline
against a 4B while passing on every hosted model, and gemini-cli is flaky one
run in three on vLLM.
A password field for the upstream key, and three validation outcomes instead of
two: ready to train, ready for eval only, or not usable. Eval enables Run and
relabels it, since an eval rollout is a perfectly good thing to press.

The result card names the rollout type where it used to print trainable
tokens=0, which read as a capture failure on a healthy eval run, and the
contract slot explains its absence rather than offering an empty file.

Warnings that change what you are measuring are shown at Validate, not after a
rollout: reasoning turned off, a dropped temperature, an endpoint that will not
emit tool calls. A rollout costs a sandbox and several minutes to learn the same
thing.

Also warns when the URL in the box is not the URL the server is serving. The
capture proxy is built at boot and cannot be repointed from a browser, so
validating one endpoint and running against another was previously silent.

Info icons: Gradio's own info= for the one-liners, hover bubbles for the two
long ones — what the API key is and is not, and what separates a train rollout
from an eval one.
… are real

96 new tests. Every provider error payload in them is a verbatim copy of one a
live endpoint returned, because the same rejection of the same parameter is
phrased two ways by two models from the same vendor and one of them leaves
error.param null — a matcher written against a single shape passes its tests and
fails in production.

Notable cases: a parameter name is never invented from prose (an earlier regex
extracted "time" from "at the same time is not supported"); an unknown
conflicting pair is refused rather than half-dropped; truncation during the tool
probe is inconclusive; message-prefix linking survives tool-call arguments being
re-serialised; an eval rollout still rebuilds its conversations; and a full
train rollout is pinned end to end through the proxy — input_ids, loss_mask,
logprobs, turn_lengths and the per-turn contract — since CI has no GPU.

scripts/logprob_parity.py is the one check that tests the numbers rather than
the shape. It captures a real multi-turn tool conversation and re-scores the
same token ids with the same serving engine, which is equivalent to asserting
GRPO's importance ratio is 1.0 on fresh on-policy data. It calibrates against
its own negative control, because the absolute residual depends on the model and
how much prefix cache the turn reused: aligned 0.10 nats, prompt short by one
token 15.7, completion rotated by one 33.3 — so a misalignment shows up ~150x
larger than the honest residual on both vLLM and SGLang.

Worth knowing from building it: the residual is the engine's, not ours. Scoring
the same sequence three times is bitwise identical, but a model's sampling path
and its scoring path disagree in bf16 by up to 0.026 nats measured directly. So
even with perfect capture the step-0 importance ratio is ~1.03, not 1.0.
Rewrites the prerequisites around the train/eval split rather than presenting
token capture as the only mode: what you get either way, how to point the server
at OpenAI, Anthropic or HF Inference Providers, and why --api-key is not the key
the agent receives.

Adds the two things that are invisible until they bite. Why
--logprobs-mode processed_logprobs is not optional, with the measured numbers,
since token ids arrive without it and the endpoint grades as trainable while the
logprobs are pre-temperature. And what startup checks about agents rather than
capture — including that gpt-5.6 takes function tools only with reasoning off,
after which agentic loops make one call and stop.

Also notes Anthropic's asymmetry: /v1/chat/completions takes Bearer while
/v1/models wants x-api-key plus anthropic-version, so the model list comes back
empty and --model is required. That is why an unusable model list is not treated
as unreachable.
Copilot AI review requested due to automatic review settings August 5, 2026 16:22
@adithya-s-k

Copy link
Copy Markdown
Collaborator Author

Pushed a set of commits that make this serve both kinds of rollout, and validated them end to end.

The split is now explicit: a rollout is train or eval, decided by probing the endpoint rather
than by a flag. train needs token ids plus the sampling distribution's logprobs — vLLM with
--return-tokens-as-token-ids --logprobs-mode processed_logprobs, or SGLang from main. Everything
else is eval: reward and the full trace, no token fields, no contract.json. Same server, same
agents, same code path, and the level is stamped on every result so an eval rollout can't be mistaken
for a trainable one.

That required real work on the upstream leg, because "without those flags any OpenAI-spec endpoint
still works" was not true — the proxy sent no Authorization header at all, and return_token_ids is
a hard 400 on OpenAI, so every call failed rather than merely losing its ids. There's now an
--api-key/--auth-header, and a compat layer that reads a provider's 400 and applies the one edit
it names. Current OpenAI models reject four things harnesses routinely send, in two different phrasings
of the same rejection; Anthropic names the field in backticks under a generic code.

Validation

Live rollouts, all five upstreams. vLLM+flags and SGLang-from-main both produce trainable
rollouts with exact token-prefix chains (580 and 1100 trainable tokens, atif=match). OpenAI,
Anthropic and HF Inference Providers all produce eval rollouts with reward 1.00, multi-turn traces
and atif=match.

Compatibility matrix: 16 harnesses × 5 upstreams × 3 tasks = 240 rollouts. 11 of 16 harnesses
clean on all five. It found four harness bugs, all on our side, all fixed: gemini-cli couldn't
install (dead Harbor API), kimi-cli was capturing fine and dying in teardown, goose was sending
the operator's real provider key so our own proxy 401'd it, and codex was tripping on a
parallel_tool_calls orphan. The remainder aren't compatibility failures — swe-agent/trae-agent
exceed the 900s agent deadline against a 4B while passing on every hosted model, and gemini-cli is
flaky one run in three on vLLM.

Contract fidelity. scripts/logprob_parity.py re-scores a captured conversation with the same
serving engine, which is equivalent to asserting GRPO's importance ratio is 1.0 on fresh on-policy
data. It calibrates against its own negative control: aligned 0.10 nats, prompt short by one token
15.7, completion rotated by one 33.3 — misalignment shows up ~150× larger than the honest residual, on
both engines.

1779 tests (+96), lint clean.

Two findings worth flagging

A silent training-correctness hole, now closed. token_ids comes from the request parameter,
not from either serving flag — so a vLLM started with neither still returns aligned, negative,
correctly-counted logprobs and grades as fully trainable. But logprobs_mode defaults to
raw_logprobs, i.e. pre-temperature, which is the wrong number for an importance ratio. Measured on
the same token at T=0.7: -1.3292 with the flags, -1.2546 without. Startup now measures it directly
(the top-two logprob gap scales by 1/T if processed, and cannot move if raw) and demotes such an
endpoint to eval.

gpt-5.6 breaks agentic loops, and validate now says so. It takes function tools on
/v1/chat/completions only if reasoning_effort is "none", and with reasoning off it emits one
valid tool call and then the loop dies: goose and codex each managed a single model call and 0/3
tasks, while both scored 3/3 against gpt-4o-mini on the same endpoint. The capture probe sent no
tools, so this was invisible until minutes into a rollout; a second non-fatal probe now sends a
manifest the way a harness does and warns at validate time.

That last one is the argument for a Responses upstream: it's the only route that keeps reasoning on.
Worth noting it buys reachability rather than trainability — vLLM's /v1/responses returns no
prompt_token_ids at all, so it can never be a training path, and Anthropic doesn't serve that route.

Also, incidentally: even with perfect capture the step-0 importance ratio isn't exactly 1.0. Scoring
the same sequence three times is bitwise identical, but a model's sampling path and its scoring path
disagree in bf16 by up to 0.026 nats measured directly — so ~1.03 typical, 1.15 on a long
cache-reusing turn. Worth knowing when setting ratio clipping.

Comment thread scripts/logprob_parity.py Outdated
Comment thread envs/harbor_env/server/app.py
Comment thread src/openenv/core/harness/capture/export.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Human review recommended

It introduces a large, security- and correctness-sensitive capture/forwarding surface area, and the review found concrete blockers (e.g., upstream auth header handling) that must be addressed before merge.

Review details
  • Files reviewed: 65/67 changed files
  • Comments generated: 2
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread src/openenv/core/harness/capture/upstream.py
Comment thread src/openenv/core/harness/capture/forwarding.py Outdated
Two shape bugs in the graph, both of which trained work that never counted.

`_find_parent` only looked backwards and skipped any candidate whose end was
longer than the new prompt, so a turn arriving BEFORE its own ancestor stayed
orphaned forever: ingest [1,2,3,4]->[5] then [1,2]->[3] and one genuine two-turn
trajectory became two roots. That contradicts what this class documents —
arrival order is not meaningful, because harnesses issue concurrent requests —
and the existing test only covered the ancestor-first direction. Linking now
looks forwards too, adopting roots that the new turn turns out to precede.

`discarded_nodes` only scanned forks, i.e. parents with more than one child.
Retrying the FIRST call gives two roots with parent_id=None, which is not a fork
at all, so the abandoned attempt was exported as a full agent sequence and
trained with the rollout's reward. Roots are now grouped by their exact prompt,
which is precise rather than heuristic: a subagent or an aux call starts from a
different prompt and never groups with the real first turn.
A turn whose logprobs are rejected on ingest keeps its sampled ids, because they
are real context for later turns, and `sequence_for` masks it out. Everything
downstream then treated it as ordinary training data.

`HarborTurn` carries a per-turn `trainable` flag, read from the loss mask so it
cannot drift from the flattener's decision. Without it, a masked turn shipped
zero-filled logprobs that a per-turn trainer reads as genuine p=1.0 tokens.

`to_turn_records` and `to_trace_entries` apply the same usable-check
`sequence_for` does. They walk the graph directly, so they emitted every sampled
id with an empty logprob list — an unequal pair, unasserted, which a consumer
zipping the two misattributes across the whole turn.

Subagent trajectories are FATAL rather than a warning. The old text said
subagent turns must not carry the parent's reward and then did nothing to stop
it: no ids collected, no role changed. ATIF does not say which captured calls
belong to the subagent, so the rollout cannot be attributed and is refused
rather than guessed at.

Role assignment no longer leans on trainable-token counts at eval level, where
they are 0 for every sequence: a harness that sends no tool manifest —
terminus-2 parses tool calls out of raw text — had every path marked auxiliary,
which emptied result.turns on a rollout that captured perfectly well.

Also flags a turn cut off by the output cap. Its tokens are genuine; its ENDING
is an artefact, and a trajectory of truncated turns teaches stopping early.
The reward coercion sat between the trial's try/except and `_pick_reward`'s own,
so a verifier returning None, "N/A" or a nested dict raised straight out of
`run_rollout`, through the MCP tool, and into the trainer — the every-rank-hangs
-on-the-NCCL-barrier failure this HTTP boundary exists to make impossible.
Verifiers are task-supplied code, so the value is now coerced inside a guard and
checked for finiteness: inf and nan coerce fine and are still wrong, inf reading
as solved and nan poisoning any average over a batch.

Sequence-level findings now reach the result. `check_sequence` FATALs — a
positive logprob, a length mismatch, nothing trainable — live on the row rather
than in the document's validation list, so a rollout carrying one came back with
a clean findings list and ok=True. A train rollout with no surviving trainable
sequence now fails, which is checked only on the train path since having nothing
trainable is the defining property of an eval rollout.

Auxiliary demotion was per sequence while detection is per node, so a sequence
mixing an aux node with real agent turns stayed agent in full and shipped the aux
call as a training turn credited with the task's reward. That held only under the
unstated assumption that aux calls always form their own single-node root. The
aux node's tokens are now masked at token level: still context, no longer
targets, which is how `sequence_for` already treats a turn it cannot trust.
The registered-session check guarded the catch-all proxy route and nothing else,
which is fine on a private port and wrong once the app is mounted at /capture on
a public Space: GET /sessions enumerated every live rollout,
GET /sessions/{id}/rollout returned its full token-level training data, DELETE
ended it, and POST /sessions minted a key the proxy would then honour. Those
routes are the trainer's control plane, not the agent's data plane, so they are
gated on an admin key — unset by default, so a local run is exactly as
convenient as before, and minted by `serve` whenever the proxy is reachable from
outside. $OPENENV_CAPTURE_ADMIN_KEY overrides it. Compared with
`secrets.compare_digest`: a timing oracle on a public endpoint is free to close.

Two upstream replies used to break ingest. A 200 with no choices was recorded as
a node with no prompt and no completion, inflating n_turns and n_roots and able
to push a worthless rollout past the degenerate_rollout FATAL that exists to
catch it. And a null inside the logprobs list passed the length check and then
crashed export on `None > 0.0`, turning GET /sessions/{id}/rollout into a 500.

Aux routes are now per dialect. Only Anthropic's count_tokens was matched, so
gemini-cli's :countTokens fell through to the catch-all, became a real chat
completion, landed in the graph as a bogus root, and returned a
{"candidates": [...]} envelope where the caller wanted {"totalTokens": N}.
Verify the ids/logprobs pairing per position instead of trusting equal length.
The two arrive on separate vLLM channels joined by index, and an equal-length but
SHIFTED pairing passes that guard — a stop or EOS token present in one channel
and not the other is enough — after which every logprob is attributed to its
neighbour's token and training proceeds on the misattribution. With
--return-tokens-as-token-ids the check is free and exact: each entry's token
field reads `token_id:{id}`, so it is compared at every position rather than
inspected once for a warning. A disagreement drops the logprobs, which is what
every other unusable-logprob path already does.

Only replay a tool call as local_shell_call when the REQUEST declared a
local_shell tool. The name alone is not evidence: a user-defined function called
`execute` or `run_command` was rewritten to a shell call, so turn k recorded
name="execute" and turn k+1 replayed name="shell", the exact token prefix no
longer matched and the turn was orphaned into a new root — and codex would run
the caller's own function as a shell command.

`.get("id", fallback)` returns None when the key is present and null, which
defeated the uuid fallback in two places and emitted items with no call id.

Non-text system content no longer vanishes when system messages merge: an image
part reduced to "" and was skipped by the falsy check, so the merged prompt
silently lost it and no longer matched what the harness sent.

The cloudflare forwarder waits with select before readline. readline blocks, so a
cloudflared that started and went quiet held the loop inside that call forever
and startup_timeout_s was advisory. Also drops a dead `_streaming` body flag
nothing set, on a path where normalise_for_capture forces stream=False anyway.
The Space entry point defaulted its capture level to "tokens" and corrected it
only when a model resolved AND the probe succeeded. An ambiguous model list, an
unset model or a probe that raised left a Space building its proxy at token level
and stamping every rollout it produced as trainable — the one failure the capture
level exists to prevent. Unknown now means "text", corrected upward only by
evidence, and a Space that resolved no model says so in its findings.

Also makes the logprob parity check runnable. It carried an absolute path from
the machine it was written on, so it could not import openenv on any other
checkout — and a check nobody can run is not a check. Resolved from the file's
own location now, and only when openenv is not already importable, so an
installed package still wins over the working tree.
Direct coverage for `reconcile`, which was the largest gap: it is billed as the
only non-self-referential correctness check and its decision logic — the
turn-count FATAL, the coverage floor, the aux-subsequence inference, the subagent
refusal, the eval count comparison — had no tests at all, only its helpers did.
Twelve cases, with the documents built by `export_session` rather than hand-rolled
dicts; three KeyErrors while writing them showed how fast a hand-written document
drifts from the contract the code actually reads.

Plus the behavioural fixes: a toolless harness stays the agent on an eval
endpoint while the train path still uses trainable-token counts as its tiebreak,
and a Space that cannot probe defaults to the weaker tier.
Every subclass here is fitted to a specific Harbor wrapper, so this module
unavoidably reaches into Harbor's internals. What was avoidable is the blast
radius: as plain module-top imports, a single upstream rename raised ImportError
for the whole module and took out every seam routed through `import_path` — pi,
gemini-cli, openhands, hermes, cline, kimi, openclaw and swe-agent at once, none
of them related to the rename. That is not hypothetical; a dead
`ensure_system_dependencies` call is what broke gemini-cli on Harbor 0.20.0.

Each base is imported individually now, and a failure yields a placeholder that
is still subclassable, so the module imports and the other seven agents keep
working. Instantiating the affected one raises and names what moved.
A processed logprob is taken over the distribution AFTER every logit processor
runs, so a harness sampling with top_p<1, top_k or a repetition penalty produces
logprobs over a truncated and renormalised distribution — while a trainer
recomputing over the full vocabulary gets different numbers for the same tokens.
Neither is wrong; they answer different questions, and the mismatch is invisible
unless the parameters travel with the turn. The startup probe uses temperature
1.0 and no truncation, so it never exercises this.

Recorded rather than stripped: these parameters ARE the policy that produced the
tokens, and removing them would change what was sampled. Carried on the turn, in
the exported document and on HarborTurn, so a recompute can apply the same
processors and a rollout that used them stays identifiable afterwards.

This does not by itself prove a recompute matches — measuring that needs a
trainer-side pass, which scripts/logprob_parity.py does only at temperature 1.0
with no truncation. It makes the question answerable instead of silent.
Copilot AI review requested due to automatic review settings August 6, 2026 13:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

There are 3 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 31d0939. Configure here.

Comment thread src/openenv/harbor/rollout.py Outdated
Comment thread src/openenv/core/harness/capture/server.py
The running offset was advanced as if each turn contributed only prompt-plus-sampled tokens, so from
the second turn on it pointed at interstitial context: the aux call's real completion tokens stayed at
mask 1 and the function silently did nothing. `n_prompt` is already the sequence-coordinate start of a
turn's sampled span, so use it directly.

Tests parametrise the aux node's position, because the first-turn case passes under either arithmetic.
The estimator read `messages` and `system` only, so a `:countTokens` from gemini-cli hit the
`max(1, ...)` floor and answered 1 no matter how long the conversation was. An agent uses that figure
to decide when to compact, so a constant 1 means it never does and overruns its real context window.
Reads `contents`/`parts` and `systemInstruction` in both spellings the REST API and the SDK emit.
`run_batch` bound the capture port in a background thread and only then built the forwarder, outside
any guard. A cloudflared that would not start left that thread up, so the next invocation died on a
port conflict naming nothing about the real cause. Teardown had the mirror gap: a forwarder whose
`stop()` raised took the port with it. Both now follow `HarborService.start`.
vLLM masks the top-p/top-k tail to -inf and takes the log-softmax after
(v1/sample/ops/topk_topp_sampler.py:135 then :139), so under processed_logprobs a captured logprob is
renormalised over the surviving set: log p_full(t) - log(kept_mass). A trainer recomputing over the
full vocabulary gets log p_full(t), which puts GRPO's step-0 importance ratio at kept_mass instead of
1 — bounded by top_p, unbounded for top_k, and reordering rather than shifting for the penalties.

An on-policy rollout has to be drawn from the distribution being trained, so at the `tokens` tier
these knobs go to their no-op values; the eval tiers keep whatever the harness asked for. What was
requested is still recorded per turn and now also stated as a finding, so the override is never silent.
Copilot AI review requested due to automatic review settings August 6, 2026 14:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

`--top-p` samples at a given top_p while the rescore stays full-vocab, which is exactly the
capture-versus-trainer comparison. Reports the MEAN SIGNED residual alongside the existing max: a
truncation bias is systematic and one-directional, so the mean is what reveals it, while the max is
what reveals a misalignment. Measured on Qwen3.5-4B, vLLM DP=1 with processed_logprobs:

  top_p=1.0   +0.000358 nats  ratio 0.9996   (noise floor)
  top_p=0.95  +0.002682 nats  ratio 0.9973
  top_p=0.9   +0.005786 nats  ratio 0.9942
  top_p=0.8   +0.015446 nats  ratio 0.9847

Monotonic, positive as predicted, and the max residual per level tracks the -log(kept_mass) bound.
Through the proxy, where prepare_request neutralises it, the same top_p=0.8 conversation returns
-0.000209 nats / ratio 1.0002 — back at the noise floor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenEnv × Harbor: make multi-harness agentic training the easy path

3 participants