fix(benchmark): install the authorities the observation path needs - #723
Conversation
Test Linux fails on three claude_observation_benchmark tests. They drive the real observation pipeline, which host admission now refuses without the process background-CPU authority (background_cpu_unavailable) and, on the Codex provider path, without configured resident-memory preparation resources. Production installs both during daemon bootstrap, which a benchmark never runs. tracedecay-global-db's harness installs them in profile_with_session_capture_resources, but that helper is #[cfg(test)] pub(crate) and unreachable from here, so this reproduces its race-tolerant pattern locally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f245016b2c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A sibling benchmark can win the process-wide installation race at a | ||
| // different canonical width; reuse that authority rather than making | ||
| // success depend on execution order. | ||
| if install_process_background_cpu(NonZeroUsize::MIN).is_err() { |
There was a problem hiding this comment.
Use the production CPU width for benchmark measurements
On a fresh run of the documented benchmark command, no daemon authority exists, so this permanently installs width 1. Codex derives shared_jsonl_preparation_workers() from that width and acquires those permits around its Rayon frame-preparation work, whereas production installs the worker plan's effective_workers; the provider benchmark therefore serializes Codex preparation and records non-production latency and throughput. Initialize the canonical production worker plan rather than using the minimum width.
AGENTS.md reference: AGENTS.md:L7-L12
Useful? React with 👍 / 👎.
| let _ = tracedecay_sessions::runtime::codex::CodexDiscoveryHub::default() | ||
| .configure_preparation_resources(memory); |
There was a problem hiding this comment.
Use the canonical resident-memory authority
When a nonignored benchmark fixture runs before daemon bootstrap tests in the same cargo test --lib process, this benchmark-owned Arc permanently wins the shared JSONL OnceLock; bootstrap then configures its scheduler-owned memory and receives InvalidFrameState, making the suite execution-order dependent. If bootstrap wins first, discarding this result instead makes the benchmark silently use an unrelated authority. Compose the fixture with the canonical process authority rather than installing a shadow one.
AGENTS.md reference: AGENTS.md:L82-L84
Useful? React with 👍 / 👎.
…plan-reopened' into claude/fix-test-linux-benchmark
a10e46d
into
codex/tracedecay-total-redesign-plan-reopened
Fixes
Test Linuxon #707Three
claude_observation_benchmarktests fail, all on missing process-wide authorities:Why it happens
These benchmarks drive the real observation pipeline. Host admission now refuses every capture when the process background-CPU authority is absent, and the Codex provider path additionally refuses without configured resident-memory preparation resources. Production installs both during daemon bootstrap — which a benchmark never runs.
Why it wasn't caught by the existing harness
tracedecay-global-db's harness does install both, inprofile_with_session_capture_resources. But that helper is#[cfg(test)]andpub(crate), so it is reachable only from inside that crate. The benchmark calls plainHostAdmissionTestRuntimeV1::profile(), which installs neither.So this is not a test that drifted — it is a production prerequisite that grew, with the only fixture that satisfies it locked inside another crate.
The fix
One
ensure_background_cpu_authority()helper in the benchmark runner, wired into both constructors. It reproduces the harness's own race-tolerant pattern: install atNonZeroUsize::MIN, and if a sibling benchmark already won the process-wide race at a different canonical width, adopt that authority rather than making success depend on execution order.Test-side only. No production code changed, and no ceiling, width, or admission rule is relaxed — the tests now satisfy the contract instead of the contract being weakened to suit them.
Verification
Before: 18 passed, 1 failed (and 3 failed prior to the background-CPU half).
cargo check -p tracedecay --lib --lockedclean,cargo fmtclean.Not addressed here:
Build Windows tests,Test Windows(its cascade), andDashboard accessibilityare separate failures on the same run.🤖 Generated with Claude Code