Add ability to designate a fallback Spec

[epompeii]

This changeset adds the ability to designate a fallback Spec for the server.
In the future, if a Spec is not specified in the Job nor associated with the given Testbed, then the fallback Spec will be used for the job. Further, the provided Testbed will have the given fallback Spec assigned to it.

[github-actions]

🤖 Claude Code Review

PR: #677
Base: devel
Head: u/ep/spec-fallback
Commit: 220b917f23824a699e5b0b170f30ce3bc50d19fe

---

PR Review: Add fallback field to Specs

Commits: f6c261700 (spec_fallback), 220b917f2 (claude_1)

Summary

This PR adds a fallback column to the spec table, allowing exactly one spec to be designated as the fallback. The fallback is represented as a nullable timestamp (Option<DateTime>), consistent with the existing archived pattern. Changes span migration, schema, JSON types, API endpoints, CLI, and generated types (OpenAPI + TypeScript).

---

Positive

• Excellent test coverage — 16 new integration tests covering create, update, set/unset, replacement, archival interaction, idempotency, no-ops, and edge cases.
• Proper write lock scoping — clear_fallback + insert/update are held within the same write_conn! block, preventing race conditions.
• DB-level enforcement — SQLite triggers (enforce_single_fallback_insert, enforce_single_fallback_update) provide a defense-in-depth safety net.
• Clock discipline — InsertSpec::new and UpdateSpec::new now accept a DateTime parameter sourced from context.clock.now(), aligning with the project's deterministic clock guidance.
• Clean migration — Proper PRAGMA foreign_keys toggling, standard table-rebuild pattern, and correct down migration that drops triggers.

---

Issues

1. Unintended side effect with {fallback: true, archived: true} (Medium)

File: lib/bencher_schema/src/model/spec/plus.rs:143-148

When a PATCH sends both fallback: true and archived: true, the code:

1. Sees is_setting_fallback == true → calls clear_fallback() on all specs
2. Then UpdateSpec::new sees is_archiving == true → sets fallback = Some(None) (clears it)

Net result: the existing fallback spec loses its status, and the target spec is archived without becoming fallback. This means sending {fallback: true, archived: true} silently clears the other spec's fallback with no replacement. The test specs_patch_fallback_true_and_archived_true documents this behavior but it's surprising — a user probably expects either an error or for archived to simply win without affecting other specs.

Suggestion: Check is_setting_fallback after accounting for is_archiving:

let is_setting_fallback = json_spec.fallback == Some(true) && json_spec.archived != Some(true);

2. Unused get_fallback method (Low)

File: lib/bencher_schema/src/model/spec/plus.rs:66-75

QuerySpec::get_fallback() is defined but never called in production code in this PR. It's referenced in PLAN.md for future job creation, but shipping dead code adds maintenance burden.

Suggestion: Either remove it and add it when needed, or add a #[cfg(test)] annotation, or note explicitly that it's for the upcoming phase.

3. JsonSpec field reordering is a wire-format change (Low)

File: lib/bencher_json/src/spec/mod.rs:58-63

archived moved from between network and created to after modified. While JSON is unordered, some clients (especially code-generated ones) may serialize in field-declaration order, which could cause cosmetic diffs in snapshots or ordering-sensitive comparisons. The OpenAPI spec and TypeScript types were regenerated correctly — just flagging as a note.

4. Tests don't use Clock::Custom (Low)

File: plus/api_specs/tests/specs.rs

Per CLAUDE.md: "All time-based tests should be deterministic and use time manipulation not real wall-clock time." These tests check is_some()/is_none() so wall-clock time doesn't affect correctness here. But if future tests need to verify ordering or specific timestamps, switching to Clock::Custom would be needed.

---

Nits

• The PLAN.md cleanup in claude_1 removes the "What Already Works" section and Phase A checklist — this is fine as those items are now completed, but it's a non-code change bundled into a code commit.
• clear_fallback always runs even when the target spec is already the only fallback (the clear + set pattern is simple/correct but does an extra UPDATE when the spec is re-setting itself as fallback). Not a real issue — simplicity wins here.

---

Verdict

Solid, well-tested implementation. The main actionable item is Issue #1 — the {fallback: true, archived: true} edge case should either be guarded against or its behavior should be an intentional design choice rather than an emergent side effect.

---

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

[github-actions]

Bencher Report

Branch	u/ep/spec-fallback
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.50 µs (-1.89%) Baseline: 3.56 µs	4.66 µs (75.08%)
Adapter::Magic (JSON)	📈 view plot 🚷 view threshold	3.37 µs (-5.12%) Baseline: 3.55 µs	4.56 µs (73.84%)
Adapter::Magic (Rust)	📈 view plot 🚷 view threshold	22.12 µs (-13.90%) Baseline: 25.69 µs	29.55 µs (74.85%)
Adapter::Rust	📈 view plot 🚷 view threshold	2.62 µs (-6.89%) Baseline: 2.81 µs	3.17 µs (82.60%)
Adapter::RustBench	📈 view plot 🚷 view threshold	2.61 µs (-7.30%) Baseline: 2.82 µs	3.18 µs (82.01%)
head_version_insert/batch/10	📈 view plot 🚷 view threshold	98.41 µs (+6.10%) Baseline: 92.76 µs	119.37 µs (82.44%)
head_version_insert/batch/100	📈 view plot 🚷 view threshold	245.32 µs (+6.65%) Baseline: 230.02 µs	257.56 µs (95.25%)
head_version_insert/batch/255	📈 view plot 🚷 view threshold	471.39 µs (+3.74%) Baseline: 454.38 µs	491.80 µs (95.85%)
head_version_insert/batch/50	📈 view plot 🚷 view threshold	167.18 µs (+8.85%) Baseline: 153.59 µs	182.45 µs (91.63%)
threshold_query/join/10	📈 view plot 🚷 view threshold	150.04 µs (+9.26%) Baseline: 137.32 µs	166.40 µs (90.17%)
threshold_query/join/20	📈 view plot 🚷 view threshold	164.93 µs (+8.65%) Baseline: 151.81 µs	178.48 µs (92.41%)
threshold_query/join/5	📈 view plot 🚷 view threshold	144.43 µs (+11.11%) Baseline: 129.99 µs	157.56 µs (91.67%)
threshold_query/join/50	📈 view plot 🚷 view threshold	206.65 µs (+7.18%) Baseline: 192.81 µs	220.86 µs (93.57%)

🐰 View full continuous benchmarking report in Bencher