mgechev / mgechev/skillgrade

Feature: persist the agent's session and expose its path to graders

Open
#37 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
711
Forks
46
PR merge metrics
No merged PRs in 30d

Description

## Problem

Graders never see the agent's session. `DeterministicGrader.grade` is handed the session log and discards it (`_sessionLog`), so a grader that wants the final answer, the tool calls, the timing or the cost has to go and find them itself.

Ours reads Claude Code's transcript from `~/.claude/projects//.jsonl`. That means a Claude-only grader, plus a hook baked into the eval image to record which session id belongs to which trial — because "newest .jsonl" is a coin flip as soon as anything else runs.

That workaround has now bitten us twice in ways worth naming, because both are arguments about *where the artifact lives*, not about its shape:

- **The hook loses the race.** Across two runs of 11 tasks, 7 event logs were truncated at exactly the same point — the hook subprocesses hadn't finished when `claude -p` exited. Those trials answered correctly and in full; the trace just stopped. Graders scored them ~0 on everything.
- **The evidence dies with the trial.** `providers/docker.js` kills and force-removes the container per trial, and `checks[]` is flattened into `details` by `DeterministicGrader`, so `details` is the only channel out. A truncation bug you can't reproduce because the container is gone costs a day.

## Revised proposal: persist the session, hand graders the path

Two parts.

### 1. Pin where the agent writes its session

Every agent already has a convention and an env var to relocate it. Set it instead of hoping:

| agent | env | resulting path |
|---|---|---|
| `claude` | `CLAUDE_CONFIG_DIR=/agent` | `/agent/projects//.jsonl` |
| `codex` | `CODEX_HOME=/agent` | `/agent/sessions///

/rollout--.jsonl` |
| `gemini` | (equivalent) | |

This is Harbor's approach — [`agents/installed/claude_code.py:1840`](https://github.com/harbor-framework/harbor/blob/main/src/harbor/agents/installed/claude_code.py) and `codex.py:56`. It removes the race outright: one trial, one known path, no hook, no session-id bookkeeping, and identical on every provider because the path is chosen rather than discovered. Harbor also refuses to guess when it *is* ambiguous — `_get_session_dir` returns `None` and logs rather than picking a file.

Worth teeing agent stdout at the same time. Harbor runs:

```
claude --verbose --output-format=stream-json --print 2>&1 | tee /claude-code.txt
```

because the final `{"type":"result", ..., "total_cost_usd": N}` line is the only authoritative cost figure Claude Code emits. That single line makes a cost grader trivial; reconstructing cost from the transcript cannot get there (see below).

Then copy `/` out to the results dir alongside the report, so a trial stays debuggable after its container is gone.

### 2. Tell graders where it is

```ts
export interface GraderInput {
task: string;
trial: number;
expected?: unknown;
metadata?: Record;
session?: {
format: 'claude-code-jsonl' | 'codex-rollout' | 'gemini-stream-json';
path: string; // in the same filesystem the grader runs in
stdout?: string; // teed agent stdout, when the adapter captures it
};
}
```

- **Path, not contents, by default.** A session holds every tool result verbatim and runs to megabytes; `SKILLGRADE_INPUT` is an env var. Graders execute in the same filesystem as the run, so a path is sufficient. Inlining can be opt-in per grader if anyone wants it.
- **`format` is the discriminator.** A grader that only understands one format fails loudly instead of silently misreading another.
- **Optional**, so an agent with no session file (or a hand-run grader) is still valid — which the current `messages: Message[]` design could not express.
- Named `session`, not `messages`: it is the agent's session, in whatever shape that agent writes.

## Rejected alternative: normalised `messages` (the original proposal)

This issue originally proposed a ChatML `messages: Message[]` on `GraderInput`. Backing away from it, for two reasons.

**It is lossy exactly where graders need signal.** Claude Code's session carries, per event:

- `duration_ms` per tool call
- `message.usage` — `input_tokens`, `output_tokens`, `cache_read_input_tokens`, `cache_creation_input_tokens`, and inside `cache_creation` the split between `ephemeral_1h_input_tokens` and `ephemeral_5m_input_tokens` (1h cache writes cost 2× input, 5m 1.25× — ignoring the split understated one of our runs by 35%)
- `message.id` — **required for correctness**, not a nicety: parallel tool calls share one message id and usage accumulates per streamed chunk, so summing naively inflated our token counts 2.0–2.8×
- `isSidechain` and `subagents/*.jsonl` for delegated turns
- `toolUseResult` with `stdout`, `stderr`, `exitCode`, `interrupted`

ChatML has nowhere to put any of it. A grader that wants the last assistant message can get it from the native file in three lines; a grader that wants cost or timing can't get it from ChatML at all.

**A normalised format is a project, not a field.** Harbor did exactly this and it took an RFC — [ATIF, RFC-0001](https://github.com/harbor-framework/harbor/blob/main/rfcs/0001-trajectory-format.md), now v1.7 — plus a per-agent adapter of ~1.5–1.9k lines each. And they *still* keep the native file as the lossless artifact, offering ATIF only as the portable one; their own docs say the conversion "keeps the conversation (messages, tool calls, tool results) but drops agent-specific session details."

So: ship the lossless artifact first, and let a normaliser be built on top of it if anyone wants one. That ordering works; the reverse doesn't.

## Out of scope

Cross-agent graders still have to branch on `format`, or use a helper. That's deliberate — a normaliser is a separate proposal, and it needs the persisted session to exist before it can be written or tested.

`sessionLog` (skillgrade's own record of the commands it issued) stays as-is and is worth keeping — it's the only place `agent_result` lives when an adapter has no session file at all.

Happy to implement.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by tracing providers/docker.js and DeterministicGrader to understand how trial artifacts and grader inputs currently flow, then inspect the GraderInput interface and agent adapters. Done means sessions and teed stdout are persisted with each trial, the session path and format reach graders without breaking callers that have no session file, and the artifacts remain available after container removal.

Written by the indexing model from the issue text.

Assessment

Tech stack
docker, typescript
Domain
testing-qa, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.