Skip to content

feat(first-eval): offer local trajectory saving before run settings - #153

Open
jessbuhl wants to merge 4 commits into
mainfrom
jbuhl/ase-1265-ask-whether-to-log-evaluation-trajectories-for-future-audits
Open

jessbuhl wants to merge 4 commits into
mainfrom
jbuhl/ase-1265-ask-whether-to-log-evaluation-trajectories-for-future-audits

Conversation

@jessbuhl

@jessbuhl jessbuhl commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Eval bootstrapping now asks whether to save local trajectories in a separate check-in after suite approval, before other execution settings. The prompt explains that Eval Author needs these records to measure coverage against ETHOS.md. Scores alone are insufficient.

  • Remember the suite's choice and destination across interrupted sessions and authorized reruns, with a fresh choice for new suites. Reopen the choice when the destination, retained content, or storage/cost implications change.
  • Carry capture settings into generated native configurations, rerun instructions, and the custom-adapter engineering prompt. Integration work can start before suite approval, with capture disabled and pending until the saving choice is resolved.
  • Bind capture preferences to settings display and execution authorization separately from retained run identities. Adding or changing capture settings preserves completed suites and reviewable baseline runs.
  • Reuse automatic capture where available. Unsupported capture allows baseline setup to continue with saving disabled and an audit limitation in the normal settings summary.
  • Verify existing evidence through the audit parser or supported offline conversion. Preserve baseline results when trajectories are missing or unusable. Declining capture does not create optional converted copies.
  • Keep capture consent separate from execution and audit authorization. Show concise summaries, omit unused destinations, and avoid repeated location offers.
  • Merge current main and preserve its shared first-eval task tables.

Validation:

  • Full repository suite: 3,131 passed, 14 skipped, and 1 failed. test_discover_exits_early_without_harbor_or_gym also fails with the same phase mismatch on untouched PR commit 0d3fb03.
  • Regression cases cover legacy completed suites, capture choice and directory changes, reopened choices, new-launch permission, early adapter handoffs, literal tildes in paths, and capture disclosures.
  • Ruff lint and formatting, ty for Python 3.12 and 3.13, copyright headers, license validation, whitespace, and DCO checks passed.
  • Skips cover optional runtimes, live-model authorization, and platform-specific checks. No live model runs were started.

Fixes ASE-1265.

Signed-off-by: Jessica Buhl <jbuhl@nvidia.com>
@jessbuhl
jessbuhl requested a review from a team as a code owner October 28, 2026 14:49
Signed-off-by: Jessica Buhl <jbuhl@nvidia.com>
@rrhyne

rrhyne commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Review of commit c3f176e2366743449c809aa3849330acb367c5fe.

This PR adds a choice about saving detailed evaluation records, including the agent's inputs, responses, and tool activity. Later audits use those records to understand what happened. The workflow remembers the choice when work resumes or evaluations run again.

flowchart TD
    A[Approve evaluation suite] --> B{Can run records be saved?}
    B -->|Optional| C[Ask whether and where to save]
    B -->|Already automatic| D[Use existing recording]
    B -->|Unsupported| E[Explain limits on later audits]
    C --> F[Remember the choice]
    D --> G[Confirm settings and permission to run]
    E --> G
    F --> G
    G --> H[Run evaluations]
    H --> I[Review results and available records]
    I --> J[Offer an audit with separate permission]
Loading

[P2] Unsupported recording can block required setup. At skills/eval-author/scripts/conversation_flows/first_eval.py:175, capture_ready excludes "unsupported", although trajectory_capture_resolved() treats that choice as resolved. When a connection dependency remains, such as a missing configured Gym–Harbor bridge, the workflow hides the engineering handoff and asks the user to resolve recording again. There's no unanswered choice to resolve. The guard at line 122 also blocks a direct handoff.

This can strand a baseline evaluation that should run without saved records. Let the handoff accept resolved, unsupported recording as disabled and carry its limitation forward.

Overall assessment: The change makes saving permission explicit and preserves it across resumed work. I'd request changes because this setup blocker affects an otherwise supported workflow.

The review covered the pinned comparison across all 13 files, totaling 991 additions and 61 deletions. Tests were inspected but weren't executed.

Keep saving disabled and carry capture limitations into the adapter handoff.

Signed-off-by: Rob Rhyne <rrhyne@nvidia.com>

@aleckhoury aleckhoury left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review of 0d3fb03.

Request changes. The core design is sound. The saving check-in is a separate stage, the state helper enforces an answer → settings-display ordering across turns, the choice never authorizes execution, changing the answer invalidates authorization without forcing a redisplay, and a new suite resets the choice. Two issues contradict the PR's stated guarantees:

  1. Saving capture settings on a completed suite (legacy, or after changing choice/directory) flips it back to incomplete and orphans prior runs (inline on first_eval_state.py:633).
  2. The integration handoff is now deferred until after suite approval, removing parallel engineering work during authoring (inline on first_eval.py:119-120).

The prior P2 from @rrhyne (unsupported capture blocking the handoff) is resolved in 0d3fb03; capture_ready now uses trajectory_capture_resolved(), covered by test_unsupported_capture_allows_adapter_handoff_with_saving_disabled.

Verification

  • tests/test_first_eval_rendering.py, tests/test_first_eval_state.py, tests/test_skill_contract.py at 0d3fb03: 808 passed, 4 skipped. Full suite not run.
  • git merge-tree against current main (54674ec): clean.
  • New doc anchors, skills/gym-to-atif/SKILL.md, and the Harbor Trajectory.model_validate_json reference (audit_spec/measure.py) all resolve.
  • Throwaway probes:
    • Legacy suite completed, then mode: automatic saved → complete True→False, next_stage results→execution_settings.
    • Suite completed with choice: true, then choice: false → complete True→False, baseline no longer reviewable.
    • Suite completed, then only directory changed → complete True→False.
    • Rendered trajectory_settings for automatic/unsupported and execution_settings for declined; copy issues noted inline.

The docs are consistent across first-eval.md, the provider references, and suite-readme.md, and the Gym converter is correctly gated to enabled capture. The renderer copy needs a pass (inline P3s).

Comment thread tests/test_first_eval_rendering.py Resolved
Keep capture preferences out of retained run identities and bind them to
execution authorization and settings display instead. Restore the adapter
handoff before suite approval with saving disabled while its choice is pending.

Clarify capture disclosures and summaries, accept literal tildes in paths,
and cover completed suites, new launch permissions, and early handoffs.
Merge current main and retain its shared first-eval task tables.

Signed-off-by: Rob Rhyne <rrhyne@nvidia.com>

This branch has not been deployed

No deployments
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