Add the ability for a Testbed to have a Spec

[epompeii]

This changeset adds the ability for a Testbed to have an associated Spec.

[github-actions]

🤖 Claude Code Review

PR: #673
Base: devel
Head: u/ep/testbed-spec
Commit: 8bc3e405d7ebaa1ca71d856ed22a7be829e7b015

---

Now I have all the information needed for a comprehensive review.

---

PR Review: Testbed Spec Feature

Summary

This PR adds "spec" (hardware specification) support to testbeds, allowing benchmarks to be associated with specific hardware configurations. It spans migrations, schema models, API endpoints, CLI, frontend, and extensive tests across ~6,000 new lines.

Critical Issues

1. report.spec_id is never populated for new reports

lib/bencher_schema/src/model/project/report/mod.rs:133-140:

// TODO: Set spec_id from the job's spec at report creation time
let insert_report = InsertReport::from_json(
    ...
    None,  // spec_id is always None
    ...
);

This means every new report will have spec_id = NULL. The detector filtering at lib/bencher_schema/src/model/project/report/results/detector/data.rs:40-42 checks report.spec_id, but since it's always NULL for new reports, spec-based metric isolation in the boundary detector will never activate. The migration backfills existing reports from job.spec_id, but new reports going forward won't benefit.

This is the most significant functional gap in the PR.

2. Migration uses LIMIT 1 without ORDER BY

lib/bencher_schema/migrations/2026-02-21-120000_testbed_spec/up.sql:91:

(SELECT j.spec_id FROM job j WHERE j.report_id = r.id LIMIT 1) AS spec_id,

If a report has multiple jobs (with different spec_id values), the selected value is non-deterministic. SQLite doesn't guarantee ordering without an explicit ORDER BY. In practice this may be fine if reports have at most one job, but it should be documented or an ORDER BY clause added for correctness.

Medium Issues

3. Utc::now() in detector data query

lib/bencher_schema/src/model/project/report/results/detector/data.rs:46:

let now = Utc::now().timestamp();

Per CLAUDE.md, time-based logic should use bencher_json::Clock::Custom (behind test-clock) for deterministic testing. This uses wall-clock time directly, making it non-deterministic in tests. This appears to be pre-existing code, but it's worth noting as it's in the path of the new spec filtering.

Minor Issues

4. Unused spec field binding in Patch variant

lib/bencher_schema/src/model/project/testbed.rs:274-275:

#[cfg(feature = "plus")]
    spec: _,

The From impl discards the spec, and resolve_spec() re-reads it from the original JSON to do the DB lookup. This two-step dance is necessary since From can't do DB I/O, but it's worth a brief comment explaining why spec is intentionally discarded here and resolved separately.

Strengths

• Strong typing throughout: Uses SpecId, SpecUuid, TestbedUuid, etc. everywhere — no raw primitives for identifiers, consistent with project conventions.
• Proper #[expect(...)] usage: No #[allow(...)] attributes found. All lint suppressions use #[expect(..., reason = "...")].
• Feature gating: #[cfg(feature = "plus")] is applied consistently to all spec-related code paths, with proper non-Plus fallbacks.
• Comprehensive test coverage: ~3,900 lines of new tests across perf, testbeds, metrics, branches, and thresholds — covering CRUD, historical queries, edge cases (404s, wrong resources), and Plus/non-Plus builds.
• Test determinism: Tests use base_timestamp() and deterministic time helpers rather than wall-clock time.
• Three-way update semantics: The JsonUpdateTestbed untagged enum cleanly distinguishes "don't change spec" (spec absent), "set spec" (spec: "<uuid>"), and "clear spec" (spec: null) via the Patch/Null variant pattern.
• thiserror for errors: No anyhow usage in library crates. Error handling uses project-standard macros (resource_not_found_err!, resource_conflict_err!).
• CLI conventions: Parser definitions in separate parser module, handler logic in separate modules, no num_args usage.
• SolidJS patterns: Frontend correctly uses createMemo, createResource, createSignal, and Switch/Match for reactivity.

Checklist

Check	Status
Strong types (no raw primitives)	Pass
#[expect] not #[allow]	Pass
thiserror not anyhow in libs	Pass
Plus feature gating	Pass
cargo gen-types run	Pass (openapi.json + bencher.ts updated)
Test coverage	Pass
Deterministic time in tests	Pass
No shared global mutable state	Pass
Documentation updated (9 languages)	Pass

Recommendation

The PR is well-implemented with strong adherence to project standards. The main concern is issue #1 — report.spec_id never being populated for new reports, which renders the detector's spec-based filtering inoperative. That TODO should be resolved before merge if spec-aware boundary detection is an intended feature of this PR. If it's intentionally deferred, that should be documented explicitly (e.g., a tracking issue).

---

_{Model: claude-opus-4-6}

[github-actions]

Bencher Report

Branch	u/ep/testbed-spec
Testbed	ubuntu-22.04

Click to view all benchmark results

Benchmark	Latency	Benchmark Result microseconds (µs) (Result Δ%)	Upper Boundary microseconds (µs) (Limit %)
Adapter::Json	📈 view plot 🚷 view threshold	3.83 µs (+8.88%) Baseline: 3.51 µs	4.68 µs (81.70%)
Adapter::Magic (JSON)	📈 view plot 🚷 view threshold	3.75 µs (+6.93%) Baseline: 3.51 µs	4.61 µs (81.42%)
Adapter::Magic (Rust)	📈 view plot 🚷 view threshold	25.87 µs (+0.56%) Baseline: 25.73 µs	29.48 µs (87.74%)
Adapter::Rust	📈 view plot 🚷 view threshold	2.84 µs (+1.37%) Baseline: 2.80 µs	3.16 µs (89.89%)
Adapter::RustBench	📈 view plot 🚷 view threshold	2.85 µs (+1.56%) Baseline: 2.80 µs	3.17 µs (89.97%)
head_version_insert/batch/10	📈 view plot 🚷 view threshold	96.59 µs (+5.92%) Baseline: 91.19 µs	112.52 µs (85.85%)
head_version_insert/batch/100	📈 view plot 🚷 view threshold	232.80 µs (+2.08%) Baseline: 228.06 µs	248.15 µs (93.82%)
head_version_insert/batch/255	📈 view plot 🚷 view threshold	453.68 µs (+0.36%) Baseline: 452.07 µs	483.88 µs (93.76%)
head_version_insert/batch/50	📈 view plot 🚷 view threshold	157.42 µs (+3.84%) Baseline: 151.60 µs	173.29 µs (90.84%)
threshold_query/join/10	📈 view plot 🚷 view threshold	138.35 µs (+2.23%) Baseline: 135.33 µs	156.93 µs (88.16%)
threshold_query/join/20	📈 view plot 🚷 view threshold	153.06 µs (+2.01%) Baseline: 150.04 µs	170.73 µs (89.65%)
threshold_query/join/5	📈 view plot 🚷 view threshold	130.93 µs (+2.15%) Baseline: 128.17 µs	149.57 µs (87.54%)
threshold_query/join/50	📈 view plot 🚷 view threshold	196.87 µs (+2.98%) Baseline: 191.18 µs	213.54 µs (92.19%)

🐰 View full continuous benchmarking report in Bencher