Skip to content

Fix pier verify silently dropping trajectory data#3

Merged
jbragg merged 2 commits into
mainfrom
verify-infer-agent
Apr 10, 2026
Merged

Fix pier verify silently dropping trajectory data#3
jbragg merged 2 commits into
mainfrom
verify-infer-agent

Conversation

@jbragg

@jbragg jbragg commented Apr 9, 2026

Copy link
Copy Markdown
Collaborator

Problem

When pier verify couldn't determine which agent was used — because
none was registered, or --agent wasn't passed — it silently saved
the reward without the trajectory (conversation, tool use, cost data).
Users would see the score and assume everything was captured.

Changes

Fix trajectory loss: pier verify auto-infers the agent from the
session (set by pier start --agent) and prints which one it's using.
No agent → reward still saved, but with clear guidance on how to
capture the trajectory. Multiple agents → error asking to pick one.

Remove agent-specific session detection from pier: Container mode
now delegates to Harbor's populate_context_post_run directly
(extract_agent_context), removing find_container_agent_session_dir
and its per-agent detection code. extract_agent_logs calls
extract_agent_context after bridging.

Container --session-dir: Now refers to a path inside the
container (copied via docker cp). Supports agents not installed
through pier, e.g. baked into the image.

--session-dir requires -a: When --session-dir is explicitly
passed, -a/--agent is required (session inference is skipped to avoid
mismatching the agent with unrelated session files). Agent
auto-detection from file patterns is also removed.

Messaging: pier start without --agent hints about agent
installation and trajectory capture. --agent in host mode errors
with explanation. Extraction failure in container mode warns explicitly.

--no-mount doc fix: Clarified that Harbor's internal mounts
(agent logs, verifier output) still write to the host under .pier/.

Architecture note in README: pier stays agent-agnostic — agent
details belong in Harbor.

Breaking changes

  • pier capture/verify --session-dir without -a now errors
  • In container mode, --session-dir is now interpreted as a container
    path (previously a host path)

Copilot AI 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.

Pull request overview

Fixes pier verify dropping trajectory data when an agent isn’t explicitly selected by inferring the agent from the pier session (when unambiguous) and delegating container-mode log extraction to Harbor. Updates CLI behavior/messaging and documentation to make trajectory capture expectations explicit, and aligns the test suite with the new flow.

Changes:

  • Infer agent for pier verify/pier capture from the session when exactly one is registered; error on multiple; provide clearer messaging when none.
  • Remove pier-side agent/session-dir detection heuristics; introduce extract_agent_context() and route container-mode extraction through Harbor directly.
  • Update README and tests for new --session-dir semantics and required flags.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
README.md Documents new trajectory capture behavior, --session-dir semantics, and pier’s agent-agnostic architecture stance.
pier/cli.py Implements agent inference, container-path --session-dir copying via docker cp, and revised capture/verify assembly logic.
pier/harbor_bridge.py Removes agent-detection helpers and adds extract_agent_context() to delegate extraction to Harbor.
pier/tests/test_pier_cli.py Adds/updates tests for agent inference, multi-agent errors, container --session-dir copy, and -a requirements.
pier/tests/test_harbor_bridge.py Updates default agent name expectation and removes tests for deleted detection helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pier/cli.py
Comment thread pier/cli.py Outdated
Comment thread pier/cli.py
Comment thread pier/cli.py Outdated
Comment thread pier/cli.py
Comment thread pier/cli.py
@jbragg jbragg force-pushed the verify-infer-agent branch from 3970350 to a6e24f7 Compare April 9, 2026 23:58
@jbragg jbragg requested a review from Copilot April 9, 2026 23:59

Copilot AI 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.

Pull request overview

This PR fixes pier verify silently dropping trajectory data when the agent can’t be determined, and moves agent/session detection responsibilities toward Harbor to keep pier agent-agnostic.

Changes:

  • Infer the agent for pier verify/pier capture from the saved session (when unambiguous), emit clearer messaging, and error on multi-agent ambiguity.
  • In container mode, delegate trajectory extraction to Harbor via extract_agent_context, and reinterpret --session-dir as a container path copied out via docker cp.
  • Remove pier-side agent auto-detection heuristics and update docs/tests accordingly.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
README.md Updates CLI docs/messaging around --no-mount, agent inference, and --session-dir semantics; adds architecture note.
pier/cli.py Implements agent inference, new container session copy helper, revised verify/capture flow, and improved user-facing guidance.
pier/harbor_bridge.py Removes agent/session-dir heuristics and introduces extract_agent_context to delegate parsing to Harbor.
pier/tests/test_pier_cli.py Adds/updates coverage for agent inference, ambiguity errors, container --session-dir copy behavior, and -a requirement.
pier/tests/test_harbor_bridge.py Updates trial-result default agent name expectation; removes tests for deleted heuristics.
Comments suppressed due to low confidence (1)

pier/harbor_bridge.py:909

  • extract_agent_context() calls AgentName(agent_name) / AgentFactory.create_agent_from_name(...) outside the try block. If the agent name is invalid (user typo) or Harbor rejects the logs dir, this will raise and can crash pier verify/pier capture instead of returning None as the docstring promises. Wrap agent creation and name parsing in the same try/except (logging a warning and returning None) so extraction failures remain non-fatal and consistent.
    from harbor.agents.factory import AgentFactory
    from harbor.models.agent.context import AgentContext
    from harbor.models.agent.name import AgentName

    agent = AgentFactory.create_agent_from_name(
        AgentName(agent_name), logs_dir=logs_dir
    )
    context = AgentContext()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pier/cli.py
@jbragg jbragg force-pushed the verify-infer-agent branch from a6e24f7 to 2bcddd8 Compare April 10, 2026 00:05
@jbragg jbragg requested a review from mikerjacobi April 10, 2026 00:10
@mikerjacobi

Copy link
Copy Markdown

Bug: _tar_copy_from_container swallows stderr on failure

Carried in from the merge of #2. In _tar_copy_from_container the docker exec process is started without stderr=subprocess.PIPE, so when the tar fails the error message is just:

tar copy from container failed (create=1, extract=0)

No docker error text, no indication of what actually went wrong (container stopped, path doesn't exist, permissions, etc.).

_tar_copy_to_container right next to it handles this correctly — it captures stderr and surfaces it in the ClickException. Same pattern should be applied here:

with subprocess.Popen(
    ["docker", "exec", container, "tar", "-cf", "-", ...],
    stdout=subprocess.PIPE,
    stderr=subprocess.PIPE,   # add this
) as tar_create:
    ...
tar_create_stderr = tar_create.stderr.read()
# then reference it in the error message like _tar_copy_to_container does

@jbragg jbragg merged commit 02ec929 into main Apr 10, 2026
9 checks passed
@jbragg jbragg deleted the verify-infer-agent branch April 10, 2026 04:55
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.

3 participants