Skip to content

reflect: analysts see no trajectories for task ids that #309 maps, so LiveMathematicianBench training makes no edits #332

Open

Description

What happens. Since #309 (confine benchmark task output paths), rollouts write each task's outputs under predictions/<task_output_segment(id)>, so ids containing characters such as : are mapped to a safe directory name. skillopt/gradient/reflect.py still reads those outputs from the raw id, os.path.join(prediction_dir, tid, ...), in five places on main (84b36cd): conversation.json (line 141), target_system_prompt.txt (170), target_user_prompt.txt (182), codex_trace_summary.txt (195) and claude_trace_steps.txt (218).

LiveMathematicianBench ids have the form <month>:<number>, so every conversation.json lookup misses and fmt_minibatch_trajectories skips every item. Each analyst then returns no edits, and training runs all its epochs without changing the skill.

To reproduce. Train with configs/livemathematicianbench/default.yaml at 1a44050 (we used the claude_chat backend, Opus 4.6 as target and optimizer, on Linux with Python 3.12). Every step's analysts return 0 edits, and the skill stays at initial.md.

The fix we used. Read the outputs through the same mapping the rollouts use, falling back to the raw id:

from skillopt.envs.task_output import task_output_segment

def _task_dir(prediction_dir: str, tid: str) -> str:
    """Directory holding a task's outputs: the mapped segment rollouts write (#309), else the raw id."""
    mapped = os.path.join(prediction_dir, task_output_segment(tid, map_unsafe=True))
    if os.path.isdir(mapped):
        return mapped
    return os.path.join(prediction_dir, tid)

and use _task_dir(prediction_dir, tid) at each of the five reads. With it, the analysts get their trajectories again and training makes edits. In one LiveMathematicianBench run, test accuracy went from 46.4% for the starting skill to 74.4% for the best skill.

We're happy to open a PR with this change and a focused test, if that would help.

Activity

  1. SU-AN-coder commented on Oct 30, 2026

    @SU-AN-coder
    Contributor

    Thanks for the write-up — the diagnosis reproduces exactly on bc0a392: with an id-shaped task directory the formatted minibatch is empty (0 characters), while the same rollout data read through the mapped segment yields 190.

    I have this working and opened #341. Two notes on top of your sketch:

    • _task_output_dir() resolves the mapped segment first and falls back to the raw-id directory, so runs recorded before fix(envs): confine benchmark task output paths #309 still read.
    • There is a sixth read that isn't in your list: spreadsheet_preview.txt (reflect.py:227) has the same os.path.join(prediction_dir, tid, ...), and it is covered too. The fallback also refuses an id such as ../../escape, which on the base commit does read outside predictions/.

    You offered to open a PR — if you already have it in flight, say so and I will close #341 rather than split the review; otherwise it is up and the tests are next to the slow_update ones in tests/test_benchmark_task_output_paths.py.

  2. colehurwitz commented on Oct 30, 2026

    @colehurwitz
    Author

    Thanks for the fast response! I think you can go ahead and fix it. I haven't started my PR yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions