Skip to content

fix(deps): lock the hle extra, refresh stale test fixtures - #55

Merged
ethan-scitix merged 1 commit into
mainfrom
fix/hle-lock-and-resume-fixture-meta
Jul 30, 2026
Merged

fix(deps): lock the hle extra, refresh stale test fixtures#55
ethan-scitix merged 1 commit into
mainfrom
fix/hle-lock-and-resume-fixture-meta

Conversation

@ethan-scitix

Copy link
Copy Markdown
Collaborator

Type

  • fix — bug fix or alignment correction

Summary

Three blockers surfaced by the 0.7.0 release preflight. None were caught by CI, which runs only tests/unit (ci.yml:107).

Related Issues

Refs #43, #37, #7.

Test Plan

Automated

  • Lint/format clean (ruff check && ruff format --check) — only complaint is generated, gitignored sieval/_version.py
  • Type check clean (ty check)
  • Unit tests pass — tests/unit tests/integration tests/acceptance: 2724 passed, was 1 failed / 2723 passed
  • tests/performance: 66 passed, was 1 failed / 65 passed
  • check_preflight.py --level deep: zero [FAIL], was 1 [FAIL] on check_deps -G hle

Manual

  • Verified the pdm.lock diff per .claude/rules/deps.md: only the two groups lists changed. numpy was already locked via other groups and did not drift; no packages added or bumped.

Checklist

Required (all PRs)

  • PR title follows conventional format (type(scope): description)
  • No internal paths, credentials, or personal info in committed files
  • AI-generated code has AI-Generated Code - <model> (<provider>) in module docstring — n/a, no new modules
  • No new upper-layer dependencies added to core/
  • Deleted code verified — no remaining call sites depend on it (grepped .select( across tests/; no stale callers remain)

If: New Dependency

  • Added to correct PDM dependency group — no new dependency; locks an already-declared group

Note for reviewers

One unrelated flake is left untouched: test_io_overhead.py::test_dependency_loading_overhead asserts a wall-clock overhead ratio and fails under machine load, but passes 3/3 in isolation. It is pre-existing, outside CI and outside the release test command, so it is not folded in here.

🤖 Generated with Claude Code

Three pre-release blockers found by the 0.7.0 release preflight, none of
them caught by CI (which runs only tests/unit).

- pdm.lock was missing the `hle` group, so `pip/pdm install sieval[hle]`
  failed outright with "Requested groups not in lockfile: hle" — the HLE
  task (#43) declared the extra in pyproject.toml but never locked it.
  Locked via --update-reuse; only the group lists changed, numpy was
  already present via other groups and did not drift.

- write_completed_samples() fabricated a partial run without a meta.json,
  so every resume fixture built on it tripped the resume version gate
  (#37) and aborted fail-closed. It now calls TaskSaver.write_run_meta()
  first, exactly as a real run does at start, making the fabricated run
  indistinguishable from a genuinely interrupted one.

- test_dataset_select_and_shuffle still called Dataset.select(), renamed
  to slice() in #7. Renamed the call and its labels.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ethan-scitix
ethan-scitix merged commit 4288fc2 into main Jul 30, 2026
9 checks passed
ethan-scitix added a commit that referenced this pull request Jul 30, 2026
)

* ci: run integration + acceptance tests, stabilize a flaky perf gate

CI ran only `tests/unit`, so the whole integration and acceptance layer went
unverified on PRs. Two bugs shipped through that gap and were caught by hand
during the 0.7.0 release (#55): a resume fixture that predated the version
gate, and a perf test still calling the pre-#7 `Dataset.select()`.

Add `tests/integration` and `tests/acceptance` to the matrix job, deselecting
a new `benchmark` marker. That marker exists because `tests/acceptance` mixes
two kinds of test: nine deterministic checks of the regression-detection
helper, and one wall-clock throughput gate whose thresholds are calibrated on
a dedicated box (long_output holds only 1.10x headroom there, and coverage
tracing skews the latency it measures). CI gains the nine; the gate stays a
local tool, which a plain `pytest` still runs.

Also make `test_dependency_loading_overhead` robust. It compares two ~20ms
hydrations as a ratio, so timing each arm once fed any transient stall
straight into the metric: a loaded machine measured 876% against its 300%
bound, an idle one 27%. Each arm is now hydrated 5 times and compared on its
minimum, since contention only ever adds time. Measured under the same
concurrent-suite load that produced the 876%, the ratio now stays in an
18-37% band across six runs; the per-arm spread narrows from 7-107% to 2-16%.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* ci(release): enforce the benchmark gate as its own serial step

CI deselects `benchmark`, so the release becomes the only place the wall-clock
throughput gates are checked. Make that explicit rather than incidental.

The gates already ran here, but inside the batch this doc says to "run in
parallel" — a wall-clock assertion sharing a box with `ruff`, `ty`, and a
2723-test pytest run measures that load, not the code. That is the same
contention that made test_dependency_loading_overhead read 876%. Hold them
back with `-m "not benchmark"` and run them alone afterwards.

Also write benchmark_summary.json to gitignored outputs/ so the release has a
recorded performance baseline, and point at the summary table's actual
location in the output (it prints after the engine's log lines).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs: trim the benchmark-marker rationale to one home each

The same two facts — why CI cannot hold the thresholds, and why a ratio of
20ms timings needs repeats — were restated in the pyproject marker comment,
the ci.yml comment, the test docstring, tests/README.md, and the release doc.
Keep each where it is closest to the code and drop the echoes: the marker
description points at tests/README.md, which owns the CI rationale, and the
docstring owns the estimator rationale. Net 39 lines out, 20 in; no behaviour
change (same 2723 passed / 1 deselected, same 27.5% idle reading).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs: drop a paragraph the previous trim left duplicated

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

1 participant