Unify reproducible benchmark result storage#306
Conversation
…out kwargs Update all run_llm_benchmark_cli() calls in the integration test file from the retired output_path/checkpoint_path/artifacts_dir kwargs to output_dir/run_id, and read persisted artifacts back from the RunLayout-derived paths (run_dir/summary.json, predictions_dir, cases.jsonl) instead of the old single output_path file. Covers 5 tests flagged by review plus 4 more in the same file with the same root cause that were missed because they don't match "llm" in their test names. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tifacts-dir with --output-dir/--run-id/--overwrite
…ifests terms.jsonl/cases.jsonl are only computed once at completion, so a partial-run manifest's counts.terms/counts.cases previously always read 0 while a checkpoint was still in progress.
Run Artifacts and LLM Full-Text Benchmark sections still described the pre-unification results/llm/<timestamp>.json layout and a stale output JSON key list.
… matches _build_benchmark_payload never wrote capture_phase1_debug into the checkpoint, so _checkpoint_matches_run compared the stored default (None) against the CLI's real default (False) and always rejected the checkpoint on any real --run-id/--overwrite resume. Discovered live while resuming an interrupted OpenRouter run.
…/base_url current_run compared the raw --llm-provider/--llm-base-url CLI inputs, but run_llm_benchmark() persists the provider resolver's resolved values (e.g. the default provider when --llm-provider is omitted, or the model-prefix-inferred provider). Resolve once in the CLI so a resume with the same omitted flags matches instead of always rejecting the checkpoint.
Checkpoints written before capture_phase1_debug started being persisted still lack the key entirely, so a resume against one of those legacy files failed the same match check the previous fix addressed for newly-written checkpoints. Discovered resuming a real interrupted run whose checkpoint predated that fix.
|
Independent review findings addressed in ec70549: dataset-content and accounting fingerprints now gate checkpoint reuse; deprecated output paths remain supported with warnings; retrieval cases without gold IDs are recorded as not_evaluable. Full suite: 2023 passed, 48 skipped; check and typecheck pass. |
Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 8.1.3 to 8.1.4. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/main/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v8.1.4/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-version: 8.1.4 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…tory with 8 updates Bumps the frontend-minor-patch group with 6 updates in the /frontend directory: | Package | From | To | | --- | --- | --- | | [libphonenumber-js](https://gitlab.com/catamphetamine/libphonenumber-js) | `1.13.7` | `1.13.8` | | [@vitest/coverage-v8](https://github.com/vitest-dev/vitest/tree/HEAD/packages/coverage-v8) | `4.1.9` | `4.1.10` | | [eslint](https://github.com/eslint/eslint) | `10.6.0` | `10.7.0` | | [postcss](https://github.com/postcss/postcss) | `8.5.16` | `8.5.18` | | [prettier](https://github.com/prettier/prettier) | `3.9.4` | `3.9.5` | | [terser](https://github.com/terser/terser) | `5.48.0` | `5.49.0` | Updates `libphonenumber-js` from 1.13.7 to 1.13.8 - [Changelog](https://gitlab.com/catamphetamine/libphonenumber-js/blob/master/CHANGELOG.md) - [Commits](https://gitlab.com/catamphetamine/libphonenumber-js/commits/master) Updates `@vitest/coverage-v8` from 4.1.9 to 4.1.10 - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Changelog](https://github.com/vitest-dev/vitest/blob/main/docs/releases.md) - [Commits](https://github.com/vitest-dev/vitest/commits/v4.1.10/packages/coverage-v8) Updates `@vitest/ui` from 4.1.9 to 4.1.10 - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Changelog](https://github.com/vitest-dev/vitest/blob/main/docs/releases.md) - [Commits](https://github.com/vitest-dev/vitest/commits/v4.1.10/packages/ui) Updates `eslint` from 10.6.0 to 10.7.0 - [Release notes](https://github.com/eslint/eslint/releases) - [Commits](eslint/eslint@v10.6.0...v10.7.0) Updates `postcss` from 8.5.16 to 8.5.18 - [Release notes](https://github.com/postcss/postcss/releases) - [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md) - [Commits](postcss/postcss@8.5.16...8.5.18) Updates `prettier` from 3.9.4 to 3.9.5 - [Release notes](https://github.com/prettier/prettier/releases) - [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md) - [Commits](prettier/prettier@3.9.4...3.9.5) Updates `terser` from 5.48.0 to 5.49.0 - [Changelog](https://github.com/terser/terser/blob/master/CHANGELOG.md) - [Commits](terser/terser@v5.48.0...v5.49.0) Updates `vitest` from 4.1.9 to 4.1.10 - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Changelog](https://github.com/vitest-dev/vitest/blob/main/docs/releases.md) - [Commits](https://github.com/vitest-dev/vitest/commits/v4.1.10/packages/vitest) --- updated-dependencies: - dependency-name: libphonenumber-js dependency-version: 1.13.8 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: frontend-minor-patch - dependency-name: "@vitest/coverage-v8" dependency-version: 4.1.10 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: frontend-minor-patch - dependency-name: "@vitest/ui" dependency-version: 4.1.10 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: frontend-minor-patch - dependency-name: eslint dependency-version: 10.7.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: postcss dependency-version: 8.5.18 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: frontend-minor-patch - dependency-name: prettier dependency-version: 3.9.5 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: frontend-minor-patch - dependency-name: terser dependency-version: 5.49.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: vitest dependency-version: 4.1.10 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: frontend-minor-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…produce Two defects in the shared result store, both of which undermine the reproducibility guarantee the store exists to provide. `create_run_layout(overwrite=True)` reused a run directory without clearing it. `write_manifest` decides what to advertise purely from `Path.exists()`, so a rerun under the same `--run-id` inherited the previous run's files and published them as its own: re-running an extraction without diagnostics still advertised the earlier `diagnostics/chunks.jsonl`, and a narrower `--doc-ids` LLM rerun left the earlier documents inside the advertised `predictions/` directory. Clear the run directory on overwrite, preserving only `checkpoint.json` — the LLM benchmark reuses a run directory precisely to resume from it, and that checkpoint's identity is validated separately before any of it is trusted. The checkpoint filename is now owned by `RunLayout` instead of being duplicated as a literal. `discover_artifacts` was also type-blind. All three benchmark types write a `summary` role into the shared results root, so the recursive discovery handed extraction and LLM summaries to the two loaders that render dense-retrieval tables. Those summaries carry no `mrr_dense`, so `phentrieve benchmark compare` listed them as models scoring `MRR (Dense) = 0`, and every new extraction or LLM run added another junk row. Add an optional `benchmark_type` filter and set it in both retrieval loaders; the legacy fallback stays unfiltered because the old flat layout only ever held retrieval summaries. `load_summary_files` also swapped `glob.glob` (str) for `discover_artifacts` (Path) without updating the `file_path` it injects into each summary, which broke JSON serialization of loaded summaries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GfjxrHsHYH2TqjwgEcq6oS
`cases.jsonl` mixed two identifier schemes. Evaluable and failed cases were keyed on a 0-based `case_id` — the same key `terms.jsonl` joins on — while cases skipped for having no gold terms were keyed on a 1-based `case_index` and carried no `case_id` at all. A downstream join over the artifact therefore silently dropped exactly the rows the manifest counts under `not_evaluable`, and the two index bases sat in one file. Emit `case_id` (and the case text) for non-evaluable rows too, so every row in `cases.jsonl` is joinable on one stable key. Also return `run_dir` as a str: it is part of a public result dict, and the LLM payload already returns a str, so a caller serializing the retrieval result hit `TypeError: Object of type PosixPath is not JSON serializable`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GfjxrHsHYH2TqjwgEcq6oS
…tatus The checkpoint identity recorded `prompt_templates_dir` but not the templates themselves. The prompt is an experimental variable, so editing a template in place between two runs of the same `--run-id` left the identity unchanged and let the resume merge old-prompt and new-prompt document outputs into one set of metrics. Hash the templates a run will actually load — both the packaged set and the effective user set, which is `~/.phentrieve/prompts` even when the flag is omitted — and make that hash part of the identity. Checkpoints written before the key existed carry no evidence about the templates they ran under, so they stay resumable under the behaviour they were written with rather than being invalidated on upgrade; checkpoints written from now on always carry the hash and are compared strictly. The manifest also always claimed `status: complete`. `run_llm_benchmark` reports `completed` for any run that finishes its document loop, including one in which every document failed, and the CLI mapped anything not equal to `failed` onto `complete`. Derive the run status from the per-case outcomes instead — as the retrieval runner already does — and record failed/complete counts so the status is auditable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GfjxrHsHYH2TqjwgEcq6oS
Both new artifact writers open-coded `1.0 - distance` when the retriever returned distances without similarities, while the retriever itself converts through `calculate_similarity`, which clamps to [0, 1]. An unclamped score also feeds `passes_threshold` in the extraction chunk diagnostics. Use the shared helper so the artifacts agree with the scores the pipeline actually acted on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GfjxrHsHYH2TqjwgEcq6oS
The guide told users to pass `--summaries-dir results/retrieval` to work around comparisons picking up non-retrieval runs; the loaders now filter by benchmark type, so the plain results root is safe. It also described `traces/` as written only with `--capture-phase1-debug`. The prediction record always carries a trace, so traces are always written; the flag adds phase-1 retrieval detail to them rather than enabling them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GfjxrHsHYH2TqjwgEcq6oS
…/vite-8.1.4' into chore/benchmark-result-store
Merges the two open Dependabot frontend PRs into this branch. Both edit the same dependency block, so package.json is resolved as the union of the two — vite ^8.1.4 from #307 alongside the six bumps from #308 (libphonenumber-js, eslint, postcss, prettier, terser, @vitest/coverage-v8) — and package-lock.json is regenerated from the resolved manifest rather than hand-merged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GfjxrHsHYH2TqjwgEcq6oS
CLI takes a minor bump for the benchmark result store and its new flags; the frontend takes a patch bump for the folded-in dependency updates. The API is untouched by this change and keeps 0.17.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GfjxrHsHYH2TqjwgEcq6oS
Deep review — full-stack + RAG/LLM lens, plus an independent Codex (gpt-5.5, xhigh) passStrong PR: the run-directory + manifest + checksum model is the right shape, I found six defects and fixed them on the branch. Both reviews converged on the same top-two independently, which is a good signal they're real. Each is covered by a regression test. Fixed1. Overwritten runs inherited artifacts they never produced (high — this defeats the PR's core promise) Now the run directory is cleared on overwrite, preserving only 2.
3. Checkpoint identity didn't cover the prompt templates (high — RAG-specific) 4. LLM manifests always claimed 5. 6. Also: the distance→similarity fallback open-coded Checked, no findingRetrieval scoring semantics are sound: the store is ChromaDB cosine, Also folded inDependabot #307 (vite 8.1.4) and #308 (8 frontend deps). Both edit the same block, so Versions
Verified locally: 🤖 Generated with Claude Code |
pip-audit went red on this branch without a dependency change of ours: the click and pillow advisories were published after the branch's last green run, and main carries the identical versions. Both have patched releases, so they are upgraded rather than added to the ignore list (which is reserved for advisories with no fix available). - click 8.3.2 -> 8.4.2 (PYSEC-2026-2132) - pillow 12.2.0 -> 12.3.0 (PYSEC-2026-2253 through -2257) typer stays on 0.25.1; the lazy CLI subgroups were verified to still populate after the click minor bump. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GfjxrHsHYH2TqjwgEcq6oS
Summary
Motivation
Benchmark outputs previously used overlapping flat paths and inconsistent artifact formats. This made repeated runs, checkpoint resume, cross-model comparison, and downstream manuscript analysis difficult to reproduce reliably.
This PR contains only reusable Phentrieve benchmark infrastructure, tests, and technical user documentation. Manuscript-specific analyses, run data, logs, figures, and planning documents are intentionally excluded.
Validation
make checkmake typecheck-fastuv sync --all-extras --frozenmake test(2023 passed, 48 skipped)