Add direct assertions for chutoro-benches discovery/probe and profiling-source return values

[leynos]

Context

Scheduled mutation-testing run 29561667528 (2026-07-17, main) is the first green run since the baseline flakiness fix in #169 landed. It scoped to chutoro-benches/, chutoro-core/ and related files changed in the preceding window and produced a clean baseline, then tested mutants against that baseline:

219 mutants tested in 20m: 10 missed, 36 caught, 173 unviable

All 10 survivors sit in the chutoro-benches benchmark-support/profiling harness. No survivors were reported outside chutoro-benches in this run.

Survivors (representative sample, all 10 listed)

• chutoro-benches/src/criterion_support.rs:37 — replace is_benchmark_discovery -> bool with true
• chutoro-benches/src/criterion_support.rs:101 — replace is_exact_benchmark_probe -> bool with true
• chutoro-benches/src/criterion_support.rs:101 — replace is_exact_benchmark_probe -> bool with false
• chutoro-benches/src/neighbour_scoring/benchmark_support.rs:86 — replace ^ with | in make_values
• chutoro-benches/src/neighbour_scoring/benchmark_support.rs:86 — replace ^ with & in make_values
• chutoro-benches/src/neighbour_scoring/build_profile.rs:95 — replace should_collect_build_profile -> bool with false
• chutoro-benches/src/neighbour_scoring/build_profile.rs:205 — replace build_profile_report_target -> Option<ReportTarget> with None
• chutoro-benches/src/neighbour_scoring/profiling.rs:87 — replace <impl DataSource for ProfilingSource<S>>::name -> &str with ""
• chutoro-benches/src/neighbour_scoring/profiling.rs:87 — replace <impl DataSource for ProfilingSource<S>>::name -> &str with "xyzzy"
• chutoro-benches/src/neighbour_scoring/profiling.rs:91 — replace <impl DataSource for ProfilingSource<S>>::metric_descriptor -> MetricDescriptor with Default::default()

Analysis

The survivors cluster into two related groups within the benchmark harness:

1. Benchmark discovery/probe classification (criterion_support.rs, benchmark_support.rs): the predicates that decide whether a Criterion invocation is a discovery pass or an exact probe, and the make_values bit-flag construction, have no test asserting their return values directly — only end-to-end smoke coverage that happens not to distinguish true/false or ^/|/& outcomes.
2. Profiling data source (build_profile.rs, profiling.rs): ProfilingSource::name, metric_descriptor, should_collect_build_profile, and build_profile_report_target are exercised by the harness but their concrete return values are never asserted, so mutating them to defaults, empty strings, or placeholder values goes undetected.

This is benchmark-support tooling rather than the core clustering algorithm, so risk is lower, but the harness is what gates performance regression detection, and an untested probe/discovery classifier can silently misclassify benchmark runs.

Proposed next step

Add direct unit assertions for:

• is_benchmark_discovery and is_exact_benchmark_probe against representative Criterion argument vectors (both true and false cases).
• make_values' use of XOR in bit construction (assert a case that would differ under OR/AND).
• ProfilingSource::name and metric_descriptor return values.
• should_collect_build_profile and build_profile_report_target against both enabled and disabled configuration.

No infrastructure changes are needed; this is pure test-coverage debt in chutoro-benches.

Rescope (2026-08-13)

The pure argument-taking variants have since gained real rstest coverage (criterion_support.rs:218-286: args_contain_flag, is_exact_benchmark_probe_args across true/false/embedded-substring cases, point_count_for_exact_probe_args, is_nextest_exact_benchmark_probe_args, should_short_circuit_exact_label_probe_args). The remaining unasserted survivors this issue now tracks:

• is_benchmark_discovery — criterion_support.rs:36 (thin env-reading wrapper)
• is_exact_benchmark_probe — criterion_support.rs:100-101 (thin env-reading wrapper)
• should_collect_build_profile — build_profile.rs:95 (wraps the doctested _value variant with std::env::var)
• build_profile_report_target — build_profile.rs:207 (same pattern)
• make_values XOR logic — benchmark_support.rs:79-86 (no direct assertion; benchmark_support_tests.rs covers fixtures and report writing only)
• ProfilingSource::name / metric_descriptor — profiling.rs:87,91 (profiling_source_tests.rs checks counters, batch stats, and snapshot reset only)

Note the four env-reading wrappers intersect with the injected-seam convention (#177): asserting them directly may be easiest via the same seam approach rather than mutating the process environment in tests.

[coderabbitai]

🔗 Related PRs

leynos/whitaker#235 - Implement rstest helper call-site collection (8.2.2) [merged]
#143 - HNSW packed-index neighbour evaluation (2.3.1) [merged]
#157 - Adopt the Whitaker Dylint suite in the lint gate and CI [merged]
leynos/shared-actions#355 - Kill mutation-testing runner, graphql retry, and matrix survivors (#336, #340, #341, #342) [open]
leynos/wireframe#612 - Kill mutation-testing survivors and widen scaffolding excludes (#566, #567, #569, #570, #593, #595, #598) [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[leynos]

Rescoped (see the dated section appended to the description): the pure argument-taking variants gained rstest coverage since filing, so the issue now tracks the six remaining unasserted survivors — four thin env-reading wrappers, the make_values XOR, and the ProfilingSource metadata accessors — with a note that the env wrappers should be asserted via an injected seam per #177's convention, not process-env mutation.

[leynos]

@CodeRabbit plan

[coderabbitai]

Coding Plan

Summary

Kill the deterministic survivors directly: reconstruct the XOR-seeded RNG stream for make_values, and assert the stub name/metric_descriptor pass-through values.

Apply the injected-seam convention (#177) to the four env/args wrappers: route command-line classification through the tested *_args helpers, and give the build-profile decisions an env-lookup closure parameter.

Drive both branches of every seam function with controlled inputs, so each mutant returns a value the tests reject.

Change no mutation-testing configuration and mutate no process environment; production behaviour is preserved by supplying the real args iterator and std::env::var lookup at the call sites.

Design Choices

Design Choice 1: Make the four zero-argument env/args wrappers killable without mutating the process environment

Options Considered:

1. Direct zero-argument assertions only. Assert each wrapper against the default test environment. This kills only the mutant whose constant differs from the default value; it leaves should_collect_build_profile -> false, build_profile_report_target -> None, and is_exact_benchmark_probe -> false alive.
2. In-process environment mutation guarded by a mutex. Forbidden by AGENTS.md and directly opposed by issue #177.
3. Subprocess invocation with controlled argv/env. No precedent in the workspace; heavyweight; no assert_cmd dependency.
4. Injected-seam refactor. Make the mutated decision function accept its input source (an args iterator or an env-lookup closure) as a parameter. Route production call sites through the real source. Drive both branches in tests. This matches issue #177 and the workspace load_with_lookup convention.

Chosen Option: 4

Rationale: A mutant that replaces a zero-argument wrapper with its default-environment value (for example should_collect_build_profile -> false when the variable is unset) cannot be caught by any test that reads the real environment, because the mutant equals the observed value. The survivor only dies if the mutated function returns a different value under a controlled input. Option 4 kills both mutant variants of every wrapper, removes the untestable global-state read that cargo-mutants cannot otherwise defeat, and follows the established _args/_value/load_with_lookup seam convention.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Assert deterministic return values directly

Cover the survivors whose correct values are already observable in tests. make_values is reachable through the #[path]-included test module. ProfilingSource delegates name and metric_descriptor unchanged to a stub that returns distinguishable values. No refactor is required.

Task 1: Assert the XOR seed derivation in `make_values`

Prove that make_values derives its RNG seed with BENCH_SEED ^ dimension, so the ^→| and ^→& mutants change the generated stream.

• Add a #[test] to chutoro-benches/src/neighbour_scoring/benchmark_support_tests.rs.
• Reach super::make_values and super::BENCH_SEED directly; the module is already #[path]-included, so no visibility change is needed.
• Reconstruct the expected stream in the test with an independently seeded SmallRng::seed_from_u64(BENCH_SEED ^ dimension) and the same gen_range(-1.0_f32..1.0_f32) calls.
• Choose a dimension whose bits partly overlap BENCH_SEED (for example 3) so that ^, |, and & produce three distinct seeds. This guarantees both mutants change the first generated value.
• Assert that make_values(row_count, dimension) equals the reconstructed stream.

Task 2: Assert `ProfilingSource::name` and `metric_descriptor`

Prove the pass-through delegation to the wrapped DataSource, so the "", "xyzzy", and Default::default() mutants die.

• Add assertions to chutoro-benches/src/neighbour_scoring/profiling_source_tests.rs using the existing source() helper.
• Assert source.name() equals "stub".
• Import MetricDescriptor from chutoro_core and assert source.metric_descriptor() equals MetricDescriptor::new("stub:absolute").

🤖 Prompt for AI agents

Implement Phase 1: add direct assertions for the deterministic survivors,
without refactoring any production code.

For `make_values`:
- Add a `#[test]` in
`chutoro-benches/src/neighbour_scoring/benchmark_support_tests.rs`.
- Access `super::make_values` and `super::BENCH_SEED` directly, since the module
is `#[path]`-included.
- In the test, reconstruct the expected value stream by independently seeding
`SmallRng::seed_from_u64(BENCH_SEED ^ dimension)` and calling
`gen_range(-1.0_f32..1.0_f32)` the same way `make_values` does.
- Pick a dimension (for example `3`) whose bits partly overlap `BENCH_SEED`, so
`^`, `|`, and `&` mutants each produce a distinct seed and a distinct first
generated value.
- Assert `make_values(row_count, dimension)` equals the reconstructed stream.

For `ProfilingSource::name` and `metric_descriptor`:
- Add assertions in
`chutoro-benches/src/neighbour_scoring/profiling_source_tests.rs`, using the
existing `source()` helper.
- Assert `source.name()` equals `"stub"`.
- Import `MetricDescriptor` from `chutoro_core`.
- Assert `source.metric_descriptor()` equals
`MetricDescriptor::new("stub:absolute")`.

Phase 2: Inject the input source into the discovery/probe and build-profile wrappers

Apply the injected-seam convention to the four env/args wrappers. Move each decision into a function that takes its source as a parameter. Route production call sites through the real source. Drive both branches in tests. This removes the global-state-reading wrappers that cargo-mutants cannot defeat.

Task 1: Route command-line classification through the pure `*_args` helpers

Eliminate the zero-argument criterion_support.rs wrappers so the mutated decision lives in the already-tested *_args helpers.

• In chutoro-benches/src/criterion_support.rs, add is_benchmark_discovery_args that delegates to args_contain_flag(args, "--list"), mirroring is_exact_benchmark_probe_args.
• Replace call sites of is_benchmark_discovery() and is_exact_benchmark_probe() (in the crate benches/ and any harness code) with is_benchmark_discovery_args(std::env::args()) and is_exact_benchmark_probe_args(std::env::args()).
• Remove the zero-argument is_benchmark_discovery and is_exact_benchmark_probe wrappers, together with their non-asserting doctests. The std::env::args() read now sits at the call site closure and is not a constant-foldable return.
• Add #[rstest] cases in the existing mod tests for is_benchmark_discovery_args: a --list-present case (true) and an absent case (false), following the #[case::name(...)] style already used for is_exact_benchmark_probe_args.

Task 2: Inject an env-lookup closure into the build-profile decisions

Convert the two build_profile.rs wrappers to accept a lookup closure, following the workspace load_with_lookup pattern, so both branches are testable.

• In chutoro-benches/src/neighbour_scoring/build_profile.rs, change should_collect_build_profile and build_profile_report_target to take lookup: impl Fn(&str) -> Option<String>.
• Delegate should_collect_build_profile to the existing should_collect_build_profile_value using lookup(BUILD_PROFILE_ENV).
• Delegate build_profile_report_target using lookup(CARGO_TARGET_DIR_ENV) for the parent directory and lookup(BUILD_PROFILE_ENV) for the enable flag, reusing report_parent_dir_value and build_profile_report_target_value.
• Update the doctests on both functions to pass a lookup closure instead of calling them with no arguments.
• Update the production call sites (in neighbour_scoring/benchmark_support.rs, benchmark_runner.rs, or the benches/) to pass |key| std::env::var(key).ok(), preserving current behaviour.
• Keep the #[doc(hidden)] pub use re-exports in neighbour_scoring/mod.rs; the exported names are unchanged.

Task 3: Assert both build-profile branches through the injected lookup

Drive the enabled and disabled paths so the should_collect_build_profile -> false and build_profile_report_target -> None mutants die.

• Add #[rstest] tests in chutoro-benches/tests/neighbour_scoring_support.rs, adding imports for the seam forms should_collect_build_profile and build_profile_report_target.
• Build in-memory fake lookups (a HashMap<&str, String>-backed closure) so no test touches the real environment.
• Assert should_collect_build_profile returns true for an enabled lookup (for example CHUTORO_BENCH_NEIGHBOUR_PROFILE = "1") and false for an empty lookup.
• Assert build_profile_report_target returns Some(_) for an enabled lookup and None for a disabled lookup, checking the resolved parent directory when CARGO_TARGET_DIR is supplied.

🤖 Prompt for AI agents

Implement Phase 2: apply the injected-seam pattern to the four env/args
wrappers, so each decision function takes its input source as a parameter and
both branches become testable without process-environment mutation.

Command-line classification (`criterion_support.rs`):
- Add `is_benchmark_discovery_args`, delegating to `args_contain_flag(args,
"--list")`, mirroring the existing `is_exact_benchmark_probe_args`.
- Replace call sites of `is_benchmark_discovery()` and
`is_exact_benchmark_probe()` in the crate `benches/` and any harness code with
`is_benchmark_discovery_args(std::env::args())` and
`is_exact_benchmark_probe_args(std::env::args())`.
- Remove the zero-argument `is_benchmark_discovery` and
`is_exact_benchmark_probe` wrappers and their non-asserting doctests.
- Add `#[rstest]` cases in the existing `mod tests` for
`is_benchmark_discovery_args`: one case with `--list` present (expect `true`),
one case with it absent (expect `false`), following the `#[case::name(...)]`
style already used for `is_exact_benchmark_probe_args`.

Build-profile decisions (`neighbour_scoring/build_profile.rs`):
- Change `should_collect_build_profile` and `build_profile_report_target` to
accept `lookup: impl Fn(&str) -> Option<String>`.
- Delegate `should_collect_build_profile` to
`should_collect_build_profile_value` using `lookup(BUILD_PROFILE_ENV)`.
- Delegate `build_profile_report_target` using `lookup(CARGO_TARGET_DIR_ENV)`
for the parent directory and `lookup(BUILD_PROFILE_ENV)` for the enable flag,
reusing `report_parent_dir_value` and `build_profile_report_target_value`.
- Update the doctests on both functions to pass a lookup closure instead of
calling with no arguments.
- Update production call sites (in `neighbour_scoring/benchmark_support.rs`,
`benchmark_runner.rs`, or `benches/`) to pass `|key| std::env::var(key).ok()`,
preserving current behaviour.
- Keep the `#[doc(hidden)] pub use` re-exports in `neighbour_scoring/mod.rs`
unchanged.

Branch tests for build-profile decisions:
- Add `#[rstest]` tests in `chutoro-benches/tests/neighbour_scoring_support.rs`,
importing the seam forms `should_collect_build_profile` and
`build_profile_report_target`.
- Build in-memory fake lookups (a `HashMap<&str, String>`-backed closure); do
not touch the real environment in any test.
- Assert `should_collect_build_profile` returns `true` for an enabled lookup
(for example `CHUTORO_BENCH_NEIGHBOUR_PROFILE` = `"1"`) and `false` for an empty
lookup.
- Assert `build_profile_report_target` returns `Some(_)` for an enabled lookup
and `None` for a disabled lookup, and check the resolved parent directory when
`CARGO_TARGET_DIR` is supplied.

File-level changes to make across this phase:
- `chutoro-benches/src/criterion_support.rs`: add `is_benchmark_discovery_args`,
remove the two zero-argument wrappers, add rstest cases.
- `chutoro-benches/src/neighbour_scoring/build_profile.rs`: convert the two
wrappers to take a lookup closure and update their doctests.
- `chutoro-benches/src/neighbour_scoring/mod.rs`: confirm re-exports remain
valid; no signature-name changes needed there.
- `chutoro-benches/benches/` and
`neighbour_scoring/benchmark_support.rs`/`benchmark_runner.rs`: update call
sites to pass the real args iterator or env-lookup closure.
- `chutoro-benches/tests/neighbour_scoring_support.rs`: add the branch tests
with fake lookups.

Research

chutoro-benches holds the Criterion benchmark harness.

criterion_support.rs classifies command-line invocations. Its pure *_args helpers already have rstest coverage; only the thin zero-argument std::env::args() wrappers remain untested.

neighbour_scoring/build_profile.rs decides whether and where to write a build-profile report. Its pure *_value helpers are doctested and rstest-covered; only the zero-argument std::env::var wrappers remain untested.

neighbour_scoring/benchmark_support.rs holds the private make_values fixture generator and the private BENCH_SEED constant; its test file is #[path]-included, so it can reach both directly.

neighbour_scoring/profiling.rs wraps a DataSource with counters; its test file already defines a StubSource returning distinguishable "stub"/"stub:absolute" values.

The workspace forbids process-environment mutation in tests and prefers dependency injection (docs/reliable-testing-in-rust-via-dependency-injection.md), applied as a "value function"/"lookup closure" seam (see chutoro-test-support's load_with_lookup).

No mutants.toml or skip-annotation mechanism exists; chutoro-benches is in mutation-testing scope.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement Phase 1: add direct assertions for the deterministic survivors,
without refactoring any production code.

For `make_values`:
- Add a `#[test]` in
`chutoro-benches/src/neighbour_scoring/benchmark_support_tests.rs`.
- Access `super::make_values` and `super::BENCH_SEED` directly, since the module
is `#[path]`-included.
- In the test, reconstruct the expected value stream by independently seeding
`SmallRng::seed_from_u64(BENCH_SEED ^ dimension)` and calling
`gen_range(-1.0_f32..1.0_f32)` the same way `make_values` does.
- Pick a dimension (for example `3`) whose bits partly overlap `BENCH_SEED`, so
`^`, `|`, and `&` mutants each produce a distinct seed and a distinct first
generated value.
- Assert `make_values(row_count, dimension)` equals the reconstructed stream.

For `ProfilingSource::name` and `metric_descriptor`:
- Add assertions in
`chutoro-benches/src/neighbour_scoring/profiling_source_tests.rs`, using the
existing `source()` helper.
- Assert `source.name()` equals `"stub"`.
- Import `MetricDescriptor` from `chutoro_core`.
- Assert `source.metric_descriptor()` equals
`MetricDescriptor::new("stub:absolute")`.
===============================================================================

Task: 2

Implement Phase 2: apply the injected-seam pattern to the four env/args
wrappers, so each decision function takes its input source as a parameter and
both branches become testable without process-environment mutation.

Command-line classification (`criterion_support.rs`):
- Add `is_benchmark_discovery_args`, delegating to `args_contain_flag(args,
"--list")`, mirroring the existing `is_exact_benchmark_probe_args`.
- Replace call sites of `is_benchmark_discovery()` and
`is_exact_benchmark_probe()` in the crate `benches/` and any harness code with
`is_benchmark_discovery_args(std::env::args())` and
`is_exact_benchmark_probe_args(std::env::args())`.
- Remove the zero-argument `is_benchmark_discovery` and
`is_exact_benchmark_probe` wrappers and their non-asserting doctests.
- Add `#[rstest]` cases in the existing `mod tests` for
`is_benchmark_discovery_args`: one case with `--list` present (expect `true`),
one case with it absent (expect `false`), following the `#[case::name(...)]`
style already used for `is_exact_benchmark_probe_args`.

Build-profile decisions (`neighbour_scoring/build_profile.rs`):
- Change `should_collect_build_profile` and `build_profile_report_target` to
accept `lookup: impl Fn(&str) -> Option<String>`.
- Delegate `should_collect_build_profile` to
`should_collect_build_profile_value` using `lookup(BUILD_PROFILE_ENV)`.
- Delegate `build_profile_report_target` using `lookup(CARGO_TARGET_DIR_ENV)`
for the parent directory and `lookup(BUILD_PROFILE_ENV)` for the enable flag,
reusing `report_parent_dir_value` and `build_profile_report_target_value`.
- Update the doctests on both functions to pass a lookup closure instead of
calling with no arguments.
- Update production call sites (in `neighbour_scoring/benchmark_support.rs`,
`benchmark_runner.rs`, or `benches/`) to pass `|key| std::env::var(key).ok()`,
preserving current behaviour.
- Keep the `#[doc(hidden)] pub use` re-exports in `neighbour_scoring/mod.rs`
unchanged.

Branch tests for build-profile decisions:
- Add `#[rstest]` tests in `chutoro-benches/tests/neighbour_scoring_support.rs`,
importing the seam forms `should_collect_build_profile` and
`build_profile_report_target`.
- Build in-memory fake lookups (a `HashMap<&str, String>`-backed closure); do
not touch the real environment in any test.
- Assert `should_collect_build_profile` returns `true` for an enabled lookup
(for example `CHUTORO_BENCH_NEIGHBOUR_PROFILE` = `"1"`) and `false` for an empty
lookup.
- Assert `build_profile_report_target` returns `Some(_)` for an enabled lookup
and `None` for a disabled lookup, and check the resolved parent directory when
`CARGO_TARGET_DIR` is supplied.

File-level changes to make across this phase:
- `chutoro-benches/src/criterion_support.rs`: add `is_benchmark_discovery_args`,
remove the two zero-argument wrappers, add rstest cases.
- `chutoro-benches/src/neighbour_scoring/build_profile.rs`: convert the two
wrappers to take a lookup closure and update their doctests.
- `chutoro-benches/src/neighbour_scoring/mod.rs`: confirm re-exports remain
valid; no signature-name changes needed there.
- `chutoro-benches/benches/` and
`neighbour_scoring/benchmark_support.rs`/`benchmark_runner.rs`: update call
sites to pass the real args iterator or env-lookup closure.
- `chutoro-benches/tests/neighbour_scoring_support.rs`: add the branch tests
with fake lookups.

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---