feat: "Reproduce" action — replay a snapshot and diff metrics vs the original run - #162
feat: "Reproduce" action — replay a snapshot and diff metrics vs the original run#162lucastononro wants to merge 3 commits into
Conversation
#112) Snapshots were a passive record (dataset/code hashes + manifest). This adds an active reproduce path: - services/reproduce.py: load the snapshot manifest, verify current workspace files against the captured hashes (input drift), replay the captured .py scripts in a Modal sandbox (stage=None so the replay never writes into the original run's metric history), parse the metrics the replay emits, and diff final values vs the recorded run with a configurable tolerance — flagging drift/missing/new metrics. Report is also written to the volume as reproduce_report.json (best-effort). - POST /api/sessions/{id}/snapshot/reproduce with Pydantic request/response schemas (404 no snapshot, 409 manifest unreadable / no scripts). - Frontend: SnapshotReproduce component (button + status pill + metric diff table + input-drift warning) mounted in the experiment detail snapshot section; api.reproduceSnapshot + ReproduceReport types. - Tests: pure diff/verify/select logic + route-level tests with the sandbox call faked (drift, match, failed replay, 404). Closes #112 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
- P1: contain SystemExit per script in the replay runner — a clean sys.exit()/sys.exit(0) no longer silently aborts the remaining scripts (misreporting multi-script snapshots as match/drift on a partial metric set); nonzero exits still fail the whole replay. - verify_inputs now flags files *added* since the snapshot, not just mutated/deleted ones (expected_sha256=None marks additions). - build_replay_code: embed script paths as a plain JSON/Python list literal instead of double JSON-encoding + runtime json.loads. - SnapshotReproduce: named export per components/AGENTS.md; import updated in the experiment page. - Tests: replay-runner SystemExit regressions (clean exit continues, bare exit continues, nonzero aborts) + added-file verify_inputs case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
|
Addressed the Greptile review findings in c57f596 — all 5 were valid, none dismissed. Fixed
Tests (
Validation: backend 313 passed / 8 skipped (full suite, 17 in |
| class ChangedFile(BaseModel): | ||
| path: str | ||
| expected_sha256: str | ||
| actual_sha256: Optional[str] = None # None => file no longer exists |
There was a problem hiding this comment.
expected_sha256 is declared as str (not Optional[str]), but verify_inputs explicitly sets it to None for files that were added to the workspace after the snapshot was taken. Pydantic v2 will raise a ValidationError when FastAPI serialises the response through response_model=ReproduceReport, returning a 500 to the caller. The unit test test_verify_inputs_detects_added_files exercises this at the raw-dict level and passes, but any real request that hits this path (workspace has a new file) will fail at the HTTP boundary.
| class ChangedFile(BaseModel): | |
| path: str | |
| expected_sha256: str | |
| actual_sha256: Optional[str] = None # None => file no longer exists | |
| class ChangedFile(BaseModel): | |
| path: str | |
| expected_sha256: Optional[str] = None # None => file was added (not in snapshot) | |
| actual_sha256: Optional[str] = None # None => file no longer exists |
| inputs: { | ||
| dataset_verified: boolean; | ||
| code_verified: boolean; | ||
| changed_files: { path: string; expected_sha256: string; actual_sha256: string | null }[]; |
There was a problem hiding this comment.
The TypeScript type has
expected_sha256: string (non-nullable), which mirrors the Pydantic bug above. Once the schema is fixed to Optional[str], the backend will return null for added files and this type must accept it — otherwise TypeScript callers that read expected_sha256 won't handle the null case.
| changed_files: { path: string; expected_sha256: string; actual_sha256: string | null }[]; | |
| changed_files: { path: string; expected_sha256: string | null; actual_sha256: string | null }[]; |
…f metrics (closes #112) # Conflicts: # backend/schemas.py
Summary
Reproducibility snapshots (
backend/services/snapshot.py) captured dataset/code hashes + a manifest but were purely passive. This PR adds an active Reproduce action (#112): re-execute the snapshot's captured scripts in a sandbox against the hashed data, diff the resulting metrics against the original run, and flag any drift.Changes
Backend
services/reproduce.py(new): the reproduce pipeline —RunSnapshotrow + its JSON manifest from the volume;snapshot.py's hashing) and reports input drift (changed/deleted files) with per-file expected vs actual SHA-256;.pyscripts (notebooks and__init__.pystubs skipped) via the existing Modal sandbox path (services/sandbox.run_code— same volume, workdir, andtrainableSDK preamble as the original run). The replay runs withstage=Noneon purpose, so its metric lines are never persisted into the original run's metric history;services/metrics.parse_metric_lines), collapses to final value per metric, and diffs vs the final recorded values with a configurable tolerance (math.isclose, default1e-6) — per-metric statusmatch | drift | missing | new, overall statusmatch | drift | error;reproduce_report.jsonnext tosnapshot.jsonon the volume.POST /api/sessions/{session_id}/snapshot/reproduce(routers/snapshots.py) with typed request/response schemas inschemas.py(ReproduceRequestwith optionaltolerance,ReproduceReport). 404 when no snapshot, 409 when the manifest is unreadable or captured no scripts.Frontend
components/experiments/SnapshotReproduce.tsx(new, self-contained): Reproduce button + status pill (reproduced — metrics match/drift detected/replay failed), input-drift warning banner, stderr tail on failure, and the metric diff table (original / reproduced / Δ / status).lib/api.tsreproduceSnapshot,lib/types.tsReproduceReport/MetricDiffRow.app/experiments/[id]/page.tsx: tiny edit — one import + mounting<SnapshotReproduce/>inside the existing snapshot section (4 lines).Tests —
backend/tests/test_reproduce.py(13 tests): pure diff/tolerance/missing/new logic, final-value collapse, script selection, replay-code generation, input verification, plus route-level tests with a faked sandbox result asserting drift detection, clean match, failed-replay reporting,stage=Nonenon-contamination, and 404.Test plan
take_snapshot/GET|POST /sessions/{id}/snapshotuntouched; full backend suite green (309 passed, 8 skipped).pytest tests/green (Python 3.12 venv). Frontend:tsc --noEmit,next lint,next buildall green.run_codeseam unchanged.Closes #112
🤖 Generated with Claude Code
https://claude.ai/code/session_01Wp66uSq9saSpRq9Bbmjxj4
Greptile Summary
This PR activates the previously passive reproducibility snapshot by adding a full Reproduce action: re-execute captured scripts in a Modal sandbox, diff the resulting metrics against the original run, and flag drift. The backend adds
services/reproduce.pyand a newPOST /sessions/{id}/snapshot/reproduceendpoint; the frontend adds a self-containedSnapshotReproducecomponent with a status pill, input-drift warning, and metric diff table.reproduce_snapshotloads the manifest, verifies input hashes (detecting changed, deleted, and newly added files), replays.pyscripts via the existingrun_codesandbox path withstage=Noneto avoid contaminating the original run's metric history, then diffs final metric values with configurable tolerance.SnapshotReproduceis a named-export client component rendering the Reproduce button, status pill (match/drift/error), input-drift banner, stderr tail on failure, and a per-metric diff table.sys.exitcontainment, input verification (including newly added files), and route-level drift/match/failed-replay/404 scenarios.Confidence Score: 4/5
Safe to merge after fixing the ChangedFile schema — the rest of the reproduce pipeline is well-tested and the sandbox/metric-diff logic is sound.
The
ChangedFile.expected_sha256field is declaredstrin the Pydantic schema but is set toNonebyverify_inputsfor files newly added to the workspace after the snapshot. FastAPI response serialisation throughReproduceReportwill raise aValidationErrorand return a 500 whenever a new file is present — exactly the case the added-file detection code was written to handle. The fix is a one-line schema change plus the matching TypeScript type.Files Needing Attention: backend/schemas.py (ChangedFile.expected_sha256 type) and frontend/src/lib/types.ts (matching TS type for expected_sha256)
Important Files Changed
Reviews (3): Last reviewed commit: "style: prettier format frontend/src/comp..." | Re-trigger Greptile