diff --git a/.config/nextest.toml b/.config/nextest.toml index a81942af..319690a1 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -88,7 +88,7 @@ retries = 1 [[profile.default.overrides]] # Trybuild compile checks build fixture crates in isolated target directories # and can exceed the generic 60s timeout on shared developer machines. -filter = "test(/portable_simd_gating_compile_checks|session_api_compiles_when_cpu_feature_is_enabled|dataset_recipe_phase_order|arrow_parquet_types_share_one_family/)" +filter = "test(/portable_simd_without_feature_is_rejected|portable_simd_with_feature_compiles|session_api_compiles_when_cpu_feature_is_enabled|dataset_recipe_phase_order|arrow_parquet_types_share_one_family/)" threads-required = 4 slow-timeout = { period = "300s", terminate-after = 1, grace-period = "5s" } @@ -171,7 +171,7 @@ retries = 1 [[profile.ci.overrides]] # Trybuild compile checks build fixture crates in isolated target directories # and can exceed the generic 60s timeout on shared runners. -filter = "test(/portable_simd_gating_compile_checks|session_api_compiles_when_cpu_feature_is_enabled|dataset_recipe_phase_order|arrow_parquet_types_share_one_family/)" +filter = "test(/portable_simd_without_feature_is_rejected|portable_simd_with_feature_compiles|session_api_compiles_when_cpu_feature_is_enabled|dataset_recipe_phase_order|arrow_parquet_types_share_one_family/)" threads-required = 4 slow-timeout = { period = "300s", terminate-after = 1, grace-period = "5s" } diff --git a/.github/dependabot.yml b/.github/dependabot.yml index be71fcac..fa94c8d6 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -38,3 +38,13 @@ updates: cooldown: default-days: 7 semver-major-days: 14 + + # Keep the pinned Rust toolchain current alongside Cargo dependencies. + - package-ecosystem: "rust-toolchain" + directory: "/" + open-pull-requests-limit: 5 + labels: + - "dependencies" + - "rust-toolchain" + schedule: + interval: "weekly" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 04c6f848..24b07d39 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,6 +70,11 @@ jobs: # out is what makes the local backend's indifference to those # variables count. use-sccache: 'false' + # `make lint` selects the dated Cranelift maintenance toolchain and + # checksum-verified mold binary. The direct stable SIMD and coverage + # commands below continue to use the repository toolchain. + - name: Install development build prerequisites + run: make install-dev-fast # Pinned, checksum-verified release archive; nothing is built from # source in CI. - name: Install sccache diff --git a/.github/workflows/coverage-main.yml b/.github/workflows/coverage-main.yml index e46da4d7..738fc456 100644 --- a/.github/workflows/coverage-main.yml +++ b/.github/workflows/coverage-main.yml @@ -54,6 +54,10 @@ jobs: # the rest of the job. Keeping the step out avoids that. See # ci.yml. use-sccache: 'false' + # Keep the lint build shape identical to ci.yml's cache reader. Coverage + # itself invokes Cargo directly and remains on the repository toolchain. + - name: Install development build prerequisites + run: make install-dev-fast # Pinned, checksum-verified release archive; nothing is built from # source in CI. - name: Install sccache diff --git a/.github/workflows/nightly-portable-simd.yml b/.github/workflows/nightly-portable-simd.yml index 15001705..26305ef6 100644 --- a/.github/workflows/nightly-portable-simd.yml +++ b/.github/workflows/nightly-portable-simd.yml @@ -73,7 +73,7 @@ jobs: run: | cargo +nightly test -p chutoro-providers-dense \ --features nightly_portable_simd \ - -- --skip portable_simd_gating_compile_checks + -- --skip portable_simd_without_feature_is_rejected - name: Lint dense portable SIMD backend if: ${{ steps.gate.outputs.should_run == 'true' }} run: cargo +nightly clippy -p chutoro-providers-dense --all-targets --features nightly_portable_simd -- -D warnings diff --git a/.markdownlint-cli2.jsonc b/.markdownlint-cli2.jsonc index 9b9f99aa..ce5bc75b 100644 --- a/.markdownlint-cli2.jsonc +++ b/.markdownlint-cli2.jsonc @@ -12,6 +12,8 @@ }, "ignores": [ "**/.venv/**", + // Generated third-party Verus installation Markdown is outside repository ownership and must not be formatted or linted. + "**/.verus/**", ".node_modules/**", "**/node_modules/**", "**/target/**", diff --git a/AGENTS.md b/AGENTS.md index 17d6828e..8f1cb348 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -133,9 +133,8 @@ This repository is written in Rust and uses Cargo for building and dependency management. Contributors should follow these best practices when working on the project: -- Run `make check-fmt`, `make lint`, and `make test` before committing. These - targets wrap the following commands, so contributors understand the exact - behaviour and policy enforced: +- Run `make check-fmt`, `make lint`, and `make test` before committing. Their + routing is part of the repository build policy: - `make check-fmt` executes: ```sh @@ -143,16 +142,20 @@ project: ``` validating formatting across the entire workspace without modifying files. - - `make lint` executes rustdoc and Clippy followed by the Whitaker Dylint - suite: + - `make lint` runs `lint-clippy` and then `lint-whitaker`. The first target + runs rustdoc with `RUSTDOCFLAGS="--cfg docsrs -D warnings"` and Clippy + with all targets and features plus `-D warnings`, using the toolchain and + fragment recorded by `tools/dev-fast/TOOLCHAIN` and + `tools/dev-fast/config.toml`. That fragment selects Cranelift and uses GCC + with `mold` on Linux. Install its toolchain components and `mold` with + `make install-dev-fast`; provide GCC on Linux. The installer verifies the + pinned `mold` archive. See the [toolchain and build routing + guide](docs/developers-guide.md#rust-toolchain-and-build-routing). Linux CI + provisions the prerequisites and runs `lint-clippy` through the fragment; + `lint-whitaker` invokes the wrapper + with `RUSTFLAGS="-D warnings"` and uses its own pinned toolchain without + the development fragment. - ```sh - cargo doc --workspace --no-deps - cargo clippy --all-targets --all-features -- -D warnings - RUSTFLAGS="-D warnings" whitaker --all -- --all-targets --all-features - ``` - - linting every target with all features enabled and denying all warnings. The `whitaker` wrapper must already be on `PATH`; never install, upgrade, or downgrade it from this repository — repository tooling and agents must not modify the user's Whitaker installation. If the wrapper @@ -160,15 +163,36 @@ project: `no_std_fs_operations` exclusions, each with a rationale comment) lives in the root `dylint.toml`. Use `make lint-clippy` for a Clippy-only pass where Whitaker is unavailable. - - `make test` executes: - - ```sh - cargo test --workspace - ``` - - running the full workspace test suite. Use `make fmt` - (`cargo fmt --workspace`) to apply formatting fixes reported by the - formatter check. + - `make test` runs `cargo-nextest` with the selected development toolchain + and configuration for ordinary tests. Nextest starts a separate Cargo + invocation to compile tests and does not inherit the outer Cargo + configuration, so the Makefile passes the fragment before `nextest` and + again with `nextest run` for that inner invocation. On Linux, ordinary + tests use nightly Cranelift with GCC and `mold`. The Makefile sets + `RUSTFLAGS` explicitly because environment `RUSTFLAGS` replaces configured + target rustflags; it repeats the documentation-denial flags and, on Linux, + the `mold` linker flag. + The byte-exact core diagnostic suites (`result_api_surface` and + `session_api_surface`) and the dense negative `portable_simd` check are + excluded from nextest and run in separate Cargo commands under the + repository-pinned stable toolchain. Those commands retain warning and + documentation denials. The positive dense portable-SIMD coverage remains + on the accelerated nextest path. + The HNSW regression test + `graph_helpers_report_poisoning_without_reconfiguring` is a fourth binding + stable test leaf: it deliberately panics while holding the graph's + `RwLock` to verify poisoned-lock handling, and the selected nightly + Cranelift backend aborts on that path instead of unwinding. Make excludes + only this exact case from nextest and runs it separately with stable LLVM + and the warning and documentation denials. + `make build`, `make typecheck`, and the explicit `make dev-build` and + `make dev-test` targets also select this configuration. `make release`, + formatting, coverage, Kani, Verus, benchmarks, and Whitaker do not. Direct + Cargo invocations use stable Rust 1.93.1 and `.cargo/config.toml`, above the + workspace MSRV of 1.89. Non-Linux Make backend support is not established by + CI; the direct stable Cargo path remains available there. Use `make fmt` + (`cargo fmt --all` plus Markdown formatting) to apply formatting fixes + reported by the formatter checks. - Clippy warnings MUST be disallowed. - Fix any warnings emitted during tests in the code itself rather than silencing them. @@ -286,8 +310,10 @@ project: - Use `anyhow`/`eyre` with `.context(...)` to **preserve backtraces** and provide clear, typed failure paths. - Update helpers (e.g., `set_dir`) to **return errors** rather than panicking. -- Consume fallible fixtures in `rstest` by **making the test return `Result`** - and applying `?` to the fixture. +- Consume fallible fixtures with a reasoned `.expect(...)` in recognized + `#[test]` and `#[rstest]` bodies when `Result` would only carry setup errors + or make assertions trip `panic_in_result_fn`. Tests with genuinely fallible + work may return `Result` and use `?` when the lint permits it. ### Observability diff --git a/Makefile b/Makefile index 90b6940e..e9ce7dcb 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,24 @@ -.PHONY: help all clean test build release typecheck lint lint-clippy lint-whitaker fmt check-fmt markdownlint nixie spelling kani kani-full verus bench test-workflow-contracts +.PHONY: help all clean test build release dev-build dev-test install-dev-fast typecheck lint lint-clippy lint-whitaker fmt check-fmt markdownlint nixie spelling kani kani-full verus bench test-workflow-contracts +.NOTPARALLEL: lint -export PATH := $(HOME)/.cargo/bin:$(HOME)/.bun/bin:$(PATH) +export PATH := $(HOME)/.cargo/bin:$(HOME)/.local/bin:$(HOME)/.bun/bin:$(PATH) APP ?= chutoro-cli CARGO ?= cargo +DEV_FAST_TOOLCHAIN := $(strip $(file bool { - if let Some(value) = env.string("CHUTORO_BENCH_HNSW_MEMORY_PROFILE") { - let normalized = value.trim().to_ascii_lowercase(); - if matches!(normalized.as_str(), "0" | "false" | "off") { - return false; - } - if matches!(normalized.as_str(), "1" | "true" | "on") { - return true; - } - } - !is_benchmark_discovery() && !is_exact_benchmark_probe() -} - -/// Resolve the memory-report path through an injected environment reader. -fn memory_report_path_with_env(env: &dyn Env) -> PathBuf { - env.os_string("CHUTORO_BENCH_HNSW_MEMORY_REPORT_PATH") - .map_or_else(|| PathBuf::from(MEMORY_REPORT_PATH), PathBuf::from) -} -/// Collect and write optional HNSW memory measurements before benchmark setup. -fn profile_hnsw_memory_impl() -> Result, BenchSetupError> { - profile_hnsw_memory_impl_with_env(&DefaultEnv) -} - -/// Collect memory measurements through an injected environment reader. -fn profile_hnsw_memory_impl_with_env(env: &dyn Env) -> Result, BenchSetupError> { - if !should_collect_memory_profile_with_env(env) { - return Ok(None); - } - - let report_path = memory_report_path_with_env(env); - let mut records = Vec::new(); - - for &point_count in POINT_COUNTS { - let source = make_bench_source(point_count)?; - - for &m in MAX_CONNECTIONS { - let params = make_hnsw_params(m)?; - let ef_construction = params.ef_construction(); - let (build_result, measurement) = - match measure_peak_resident_set_size(MEMORY_SAMPLE_INTERVAL, || { - CpuHnsw::build_with_edges(&source, params.clone()) - }) { - Ok(measurement) => measurement, - Err(ProfilingError::UnsupportedPlatform { .. }) => return Ok(None), - Err(err) => return Err(err.into()), - }; - let (_index, harvest) = build_result?; - records.push(HnswMemoryRecord::new( - HnswMemoryInput { - point_count, - max_connections: m, - ef_construction, - measurement, - edge_count: harvest.len(), - }, - EDGE_SCALING_BOUNDS, - )?); - } - } - - write_hnsw_memory_report(&report_path, &records) - .map(Some) - .map_err(BenchSetupError::from) -} /// Register edge-harvesting HNSW measurements and optional memory reporting. fn hnsw_build_with_edges_impl(c: &mut Criterion) -> Result<(), BenchSetupError> { - let _maybe_report_path = profile_hnsw_memory_impl()?; + let _maybe_report_path = memory_profile::profile_hnsw_memory_impl()?; bench_hnsw_build_generic(c, "hnsw_build_with_edges", |source, params| { CpuHnsw::build_with_edges(source, params).map(|_| ()) }) @@ -404,7 +327,7 @@ fn hnsw_build_diverse_sources_impl_with_env( ¶ms, ); - if should_include_mnist_with_env(env) { + if environment::should_include_mnist_with_env(env) { let mnist = SyntheticSource::load_mnist(&MnistConfig::default_with_env(env))?; bench_build_source( &mut group, @@ -422,10 +345,6 @@ fn hnsw_build_diverse_sources_impl_with_env( Ok(()) } -/// Determine whether the diverse-source benchmark should include MNIST. -fn should_include_mnist_with_env(env: &dyn Env) -> bool { - env.string("CHUTORO_BENCH_ENABLE_MNIST").as_deref() == Some("1") -} /// Register the public Criterion diverse-source HNSW entrypoint. fn hnsw_build_diverse_sources(c: &mut Criterion) { if let Err(err) = hnsw_build_diverse_sources_impl(c) { @@ -433,115 +352,6 @@ fn hnsw_build_diverse_sources(c: &mut Criterion) { } } -#[cfg(test)] -mod tests { - //! Tests for HNSW memory-profile environment configuration. - - #[rstest::fixture] - fn env_with_memory_settings_unset() -> mockable::MockEnv { - let mut env = mockable::MockEnv::new(); - - env.expect_string().returning(|key| { - assert_eq!(key, "CHUTORO_BENCH_HNSW_MEMORY_PROFILE"); - None - }); - env.expect_os_string().returning(|key| { - assert_eq!(key, "CHUTORO_BENCH_HNSW_MEMORY_REPORT_PATH"); - None - }); - - env - } - - #[test] - fn memory_profile_explicitly_disabled_by_environment() { - let mut env = mockable::MockEnv::new(); - env.expect_string().returning(|key| { - assert_eq!(key, "CHUTORO_BENCH_HNSW_MEMORY_PROFILE"); - Some("false".to_owned()) - }); - - assert!(!super::should_collect_memory_profile_with_env(&env)); - } - - #[test] - fn memory_profile_explicitly_enabled_by_environment() { - let mut env = mockable::MockEnv::new(); - env.expect_string().returning(|key| { - assert_eq!(key, "CHUTORO_BENCH_HNSW_MEMORY_PROFILE"); - Some("true".to_owned()) - }); - - assert!(super::should_collect_memory_profile_with_env(&env)); - } - - #[test] - fn memory_profile_uses_benchmark_mode_when_environment_is_unset() { - let env = env_with_memory_settings_unset(); - - assert_eq!( - super::should_collect_memory_profile_with_env(&env), - !super::is_benchmark_discovery() && !super::is_exact_benchmark_probe() - ); - } - - #[test] - fn memory_report_path_defaults_when_environment_is_unset() { - let env = env_with_memory_settings_unset(); - - assert_eq!( - super::memory_report_path_with_env(&env), - std::path::PathBuf::from(super::MEMORY_REPORT_PATH) - ); - } - - #[test] - fn memory_report_path_uses_configured_os_string() { - let configured_path = std::path::PathBuf::from("reports/hnsw-memory.csv"); - let expected_path = configured_path.clone(); - let mut env = mockable::MockEnv::new(); - env.expect_os_string().returning(move |key| { - assert_eq!(key, "CHUTORO_BENCH_HNSW_MEMORY_REPORT_PATH"); - Some(std::ffi::OsString::from(configured_path.clone())) - }); - - assert_eq!(super::memory_report_path_with_env(&env), expected_path); - } - - #[test] - fn mnist_is_excluded_when_environment_is_unset() { - let mut env = mockable::MockEnv::new(); - env.expect_string().returning(|key| { - assert_eq!(key, "CHUTORO_BENCH_ENABLE_MNIST"); - None - }); - - assert!(!super::should_include_mnist_with_env(&env)); - } - - #[test] - fn mnist_is_excluded_when_environment_is_disabled() { - let mut env = mockable::MockEnv::new(); - env.expect_string().returning(|key| { - assert_eq!(key, "CHUTORO_BENCH_ENABLE_MNIST"); - Some("0".to_owned()) - }); - - assert!(!super::should_include_mnist_with_env(&env)); - } - - #[test] - fn mnist_is_included_when_environment_is_enabled() { - let mut env = mockable::MockEnv::new(); - env.expect_string().returning(|key| { - assert_eq!(key, "CHUTORO_BENCH_ENABLE_MNIST"); - Some("1".to_owned()) - }); - - assert!(super::should_include_mnist_with_env(&env)); - } -} - mod bench_harness { //! Criterion entrypoint for the HNSW benchmark groups. diff --git a/chutoro-benches/benches/hnsw_ef_sweep.rs b/chutoro-benches/benches/hnsw_ef_sweep.rs index d0d92a5d..43b31cde 100644 --- a/chutoro-benches/benches/hnsw_ef_sweep.rs +++ b/chutoro-benches/benches/hnsw_ef_sweep.rs @@ -6,7 +6,7 @@ use std::{num::NonZeroUsize, path::PathBuf, time::Duration, time::Instant}; use criterion::{BatchSize, BenchmarkId, Criterion, criterion_main}; -use mockable::{DefaultEnv, Env}; +use mockable::DefaultEnv; use chutoro_benches::{ ef_sweep::{ @@ -20,6 +20,8 @@ use chutoro_benches::{ }; use chutoro_core::{CpuHnsw, DataSource}; +#[path = "internal/hnsw_ef_sweep_environment.rs"] +mod environment; #[path = "internal/quality_pass.rs"] mod quality_pass; use quality_pass::measure_clustering_quality_vs_ef_impl; @@ -36,12 +38,6 @@ const RECALL_K: usize = 10; /// Search beam width when querying the HNSW index for recall. const RECALL_EF_SEARCH: usize = 64; -/// Report destination for recall-versus-ef_construction metrics. -const RECALL_REPORT_PATH: &str = concat!( - env!("CARGO_MANIFEST_DIR"), - "/../target/benchmarks/hnsw_recall_vs_ef.csv" -); - /// Dataset size for clustering-quality measurement. const CLUSTERING_QUALITY_POINT_COUNT: usize = 1_000; @@ -51,11 +47,6 @@ const CLUSTERING_QUALITY_CLUSTER_COUNT: usize = 8; /// Minimum cluster size used for hierarchy extraction in quality measurement. const CLUSTERING_QUALITY_MIN_CLUSTER_SIZE: usize = 5; -/// Report destination for ARI/NMI-versus-ef_construction metrics. -const CLUSTERING_QUALITY_REPORT_PATH: &str = concat!( - env!("CARGO_MANIFEST_DIR"), - "/../target/benchmarks/hnsw_cluster_quality_vs_ef.csv" -); /// Reduced sweep sizes used while Criterion discovers benchmark names. const DISCOVERY_EF_SWEEP_POINT_COUNTS: &[usize] = &[500]; @@ -72,37 +63,6 @@ fn unwrap_build(result: Result, context: &str) // -- Recall measurement ------------------------------------------------ -/// Emits a warning to stderr for an unrecognized env var value. -#[expect( - clippy::print_stderr, - reason = "Benchmark-only diagnostic for invalid env var; no structured logging available." -)] -fn warn_unrecognised_bool_env(env_var_name: &str, value: &str) { - eprintln!( - "warning: unrecognized value {value:?} for \ - {env_var_name}; expected 0/1/true/false/on/off" - ); -} - -/// Parse one optional benchmark boolean environment variable. -fn parse_bool_env_var(env: &dyn Env, env_var_name: &str) -> Option { - let value = env.string(env_var_name)?; - let normalized = value.trim().to_ascii_lowercase(); - if matches!(normalized.as_str(), "0" | "false" | "off") { - return Some(false); - } - if matches!(normalized.as_str(), "1" | "true" | "on") { - return Some(true); - } - warn_unrecognised_bool_env(env_var_name, &value); - None -} - -/// Determine whether Criterion is enumerating benchmark names. -fn is_discovery_mode() -> bool { - std::env::args().any(|arg| arg == "--list" || arg == "--exact") -} - /// Determine whether nextest is executing an exact Criterion probe. fn is_exact_benchmark_probe() -> bool { std::env::args().any(|arg| arg == "--exact") @@ -121,7 +81,7 @@ fn configure_ef_sweep_group( /// Select full or reduced source sizes for this Criterion invocation. fn ef_sweep_point_counts() -> &'static [usize] { - if is_discovery_mode() { + if environment::is_discovery_mode() { DISCOVERY_EF_SWEEP_POINT_COUNTS } else { EF_SWEEP_POINT_COUNTS @@ -130,46 +90,19 @@ fn ef_sweep_point_counts() -> &'static [usize] { /// Determine whether this invocation should write the recall report. fn should_collect_recall_report() -> bool { - should_collect_recall_report_with_env(&DefaultEnv) -} - -/// Read recall-report configuration through an injected environment reader. -fn should_collect_recall_report_with_env(env: &dyn Env) -> bool { - parse_bool_env_var(env, "CHUTORO_BENCH_HNSW_RECALL_REPORT") - .unwrap_or_else(|| !is_discovery_mode()) + environment::should_collect_recall_report_with_env(&DefaultEnv) } /// Resolve the configured destination for the recall report. fn recall_report_path() -> PathBuf { - recall_report_path_with_env(&DefaultEnv) -} - -/// Resolve the recall-report path through an injected environment reader. -fn recall_report_path_with_env(env: &dyn Env) -> PathBuf { - env.os_string("CHUTORO_BENCH_HNSW_RECALL_REPORT_PATH") - .map_or_else(|| PathBuf::from(RECALL_REPORT_PATH), PathBuf::from) + environment::recall_report_path_with_env(&DefaultEnv) } /// Determine whether this invocation should write the clustering-quality report. fn should_collect_cluster_quality_report() -> bool { - should_collect_cluster_quality_report_with_env(&DefaultEnv) -} - -/// Read clustering-quality configuration through an injected reader. -fn should_collect_cluster_quality_report_with_env(env: &dyn Env) -> bool { - parse_bool_env_var(env, "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT") - .unwrap_or_else(|| !is_discovery_mode()) + environment::should_collect_cluster_quality_report_with_env(&DefaultEnv) } /// Resolve the configured destination for the clustering-quality report. fn cluster_quality_report_path() -> PathBuf { - cluster_quality_report_path_with_env(&DefaultEnv) -} - -/// Resolve the clustering-quality path through an injected reader. -fn cluster_quality_report_path_with_env(env: &dyn Env) -> PathBuf { - env.os_string("CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT_PATH") - .map_or_else( - || PathBuf::from(CLUSTERING_QUALITY_REPORT_PATH), - PathBuf::from, - ) + environment::cluster_quality_report_path_with_env(&DefaultEnv) } /// Returns an evenly-spaced query index for deterministic recall sampling. /// @@ -309,159 +242,6 @@ fn hnsw_build_ef_sweep(c: &mut Criterion) { } } -#[cfg(test)] -mod tests { - //! Tests for HNSW ef-sweep environment configuration. - - #[rstest::rstest] - #[case::true_value(Some("true"), Some(true))] - #[case::false_value(Some("false"), Some(false))] - #[case::invalid_value(Some("unknown"), None)] - #[case::absent_value(None, None)] - fn parses_boolean_environment_values( - #[case] value: Option<&str>, - #[case] expected: Option, - ) { - let configured_value = value.map(str::to_owned); - let mut env = mockable::MockEnv::new(); - env.expect_string().returning(move |key| { - assert_eq!(key, "CHUTORO_BENCH_HNSW_RECALL_REPORT"); - configured_value.clone() - }); - - assert_eq!( - super::parse_bool_env_var(&env, "CHUTORO_BENCH_HNSW_RECALL_REPORT"), - expected - ); - } - - #[rstest::rstest] - #[case::recall_enabled( - crate::should_collect_recall_report_with_env, - "CHUTORO_BENCH_HNSW_RECALL_REPORT", - "true" - )] - #[case::cluster_quality_enabled( - crate::should_collect_cluster_quality_report_with_env, - "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT", - "true" - )] - fn explicit_report_enablement_overrides_benchmark_arguments( - #[case] should_collect: fn(&dyn mockable::Env) -> bool, - #[case] environment_key: &'static str, - #[case] value: &'static str, - ) { - let mut env = mockable::MockEnv::new(); - env.expect_string().returning(move |key| { - assert_eq!(key, environment_key); - Some(value.to_owned()) - }); - - assert!(should_collect(&env)); - } - - #[rstest::rstest] - #[case::recall_disabled( - crate::should_collect_recall_report_with_env, - "CHUTORO_BENCH_HNSW_RECALL_REPORT" - )] - #[case::cluster_quality_disabled( - crate::should_collect_cluster_quality_report_with_env, - "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT" - )] - fn explicit_report_disablement_overrides_benchmark_arguments( - #[case] should_collect: fn(&dyn mockable::Env) -> bool, - #[case] environment_key: &'static str, - ) { - let mut env = mockable::MockEnv::new(); - env.expect_string().returning(move |key| { - assert_eq!(key, environment_key); - Some("false".to_owned()) - }); - - assert!(!should_collect(&env)); - } - - #[rstest::rstest] - #[case::recall( - crate::should_collect_recall_report_with_env, - "CHUTORO_BENCH_HNSW_RECALL_REPORT" - )] - #[case::cluster_quality( - crate::should_collect_cluster_quality_report_with_env, - "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT" - )] - fn absent_report_configuration_uses_discovery_mode( - #[case] should_collect: fn(&dyn mockable::Env) -> bool, - #[case] environment_key: &'static str, - ) { - let mut env = mockable::MockEnv::new(); - env.expect_string().returning(move |key| { - assert_eq!(key, environment_key); - None - }); - - assert_eq!(should_collect(&env), !super::is_discovery_mode()); - } - - #[test] - fn recall_report_path_defaults_when_environment_is_unset() { - let mut env = mockable::MockEnv::new(); - env.expect_os_string().returning(|key| { - assert_eq!(key, "CHUTORO_BENCH_HNSW_RECALL_REPORT_PATH"); - None - }); - - assert_eq!( - super::recall_report_path_with_env(&env), - std::path::PathBuf::from(super::RECALL_REPORT_PATH) - ); - } - - #[test] - fn recall_report_path_uses_configured_os_string() { - let configured_path = std::path::PathBuf::from("reports/recall.csv"); - let expected_path = configured_path.clone(); - let mut env = mockable::MockEnv::new(); - env.expect_os_string().returning(move |key| { - assert_eq!(key, "CHUTORO_BENCH_HNSW_RECALL_REPORT_PATH"); - Some(std::ffi::OsString::from(configured_path.clone())) - }); - - assert_eq!(super::recall_report_path_with_env(&env), expected_path); - } - - #[test] - fn cluster_quality_report_path_defaults_when_environment_is_unset() { - let mut env = mockable::MockEnv::new(); - env.expect_os_string().returning(|key| { - assert_eq!(key, "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT_PATH"); - None - }); - - assert_eq!( - super::cluster_quality_report_path_with_env(&env), - std::path::PathBuf::from(super::CLUSTERING_QUALITY_REPORT_PATH) - ); - } - - #[test] - fn cluster_quality_report_path_uses_configured_os_string() { - let configured_path = std::path::PathBuf::from("reports/cluster-quality.csv"); - let expected_path = configured_path.clone(); - let mut env = mockable::MockEnv::new(); - env.expect_os_string().returning(move |key| { - assert_eq!(key, "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT_PATH"); - Some(std::ffi::OsString::from(configured_path.clone())) - }); - - assert_eq!( - super::cluster_quality_report_path_with_env(&env), - expected_path - ); - } -} - mod bench_harness { //! Criterion harness for the HNSW ef sweep benchmark. diff --git a/chutoro-benches/benches/internal/hnsw_ef_sweep_environment.rs b/chutoro-benches/benches/internal/hnsw_ef_sweep_environment.rs new file mode 100644 index 00000000..0b2192ec --- /dev/null +++ b/chutoro-benches/benches/internal/hnsw_ef_sweep_environment.rs @@ -0,0 +1,75 @@ +//! Environment policy shared by the HNSW ef-sweep benchmark and its tests. + +use std::path::PathBuf; + +use mockable::Env; + +/// Report destination for recall-versus-ef_construction metrics. +pub(super) const RECALL_REPORT_PATH: &str = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../target/benchmarks/hnsw_recall_vs_ef.csv" +); + +/// Report destination for ARI/NMI-versus-ef_construction metrics. +pub(super) const CLUSTERING_QUALITY_REPORT_PATH: &str = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../target/benchmarks/hnsw_cluster_quality_vs_ef.csv" +); + +/// Emits a warning to stderr for an unrecognized env var value. +#[expect( + clippy::print_stderr, + reason = "Benchmark-only diagnostic for invalid env var; no structured logging available." +)] +fn warn_unrecognized_bool_env(env_var_name: &str, value: &str) { + eprintln!( + "warning: unrecognized value {value:?} for \ + {env_var_name}; expected 0/1/true/false/on/off" + ); +} + +/// Parse one optional benchmark boolean environment variable. +pub(super) fn parse_bool_env_var(env: &dyn Env, env_var_name: &str) -> Option { + let value = env.string(env_var_name)?; + let normalized = value.trim().to_ascii_lowercase(); + if matches!(normalized.as_str(), "0" | "false" | "off") { + return Some(false); + } + if matches!(normalized.as_str(), "1" | "true" | "on") { + return Some(true); + } + warn_unrecognized_bool_env(env_var_name, &value); + None +} + +/// Determine whether Criterion is enumerating benchmark names. +pub(super) fn is_discovery_mode() -> bool { + std::env::args().any(|arg| arg == "--list" || arg == "--exact") +} + +/// Read recall-report configuration through an injected environment reader. +pub(super) fn should_collect_recall_report_with_env(env: &dyn Env) -> bool { + parse_bool_env_var(env, "CHUTORO_BENCH_HNSW_RECALL_REPORT") + .unwrap_or_else(|| !is_discovery_mode()) +} + +/// Resolve the recall-report path through an injected environment reader. +pub(super) fn recall_report_path_with_env(env: &dyn Env) -> PathBuf { + env.os_string("CHUTORO_BENCH_HNSW_RECALL_REPORT_PATH") + .map_or_else(|| PathBuf::from(RECALL_REPORT_PATH), PathBuf::from) +} + +/// Read clustering-quality configuration through an injected reader. +pub(super) fn should_collect_cluster_quality_report_with_env(env: &dyn Env) -> bool { + parse_bool_env_var(env, "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT") + .unwrap_or_else(|| !is_discovery_mode()) +} + +/// Resolve the clustering-quality path through an injected reader. +pub(super) fn cluster_quality_report_path_with_env(env: &dyn Env) -> PathBuf { + env.os_string("CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT_PATH") + .map_or_else( + || PathBuf::from(CLUSTERING_QUALITY_REPORT_PATH), + PathBuf::from, + ) +} diff --git a/chutoro-benches/benches/internal/hnsw_ef_sweep_tests.rs b/chutoro-benches/benches/internal/hnsw_ef_sweep_tests.rs new file mode 100644 index 00000000..149aa942 --- /dev/null +++ b/chutoro-benches/benches/internal/hnsw_ef_sweep_tests.rs @@ -0,0 +1,152 @@ +//! Tests for HNSW ef-sweep environment configuration. + +#[rstest::rstest] +#[case::true_value(Some("true"), Some(true))] +#[case::false_value(Some("false"), Some(false))] +#[case::invalid_value(Some("unknown"), None)] +#[case::absent_value(None, None)] +fn parses_boolean_environment_values(#[case] value: Option<&str>, #[case] expected: Option) { + let configured_value = value.map(str::to_owned); + let mut env = mockable::MockEnv::new(); + env.expect_string().returning(move |key| { + assert_eq!(key, "CHUTORO_BENCH_HNSW_RECALL_REPORT"); + configured_value.clone() + }); + + assert_eq!( + super::environment::parse_bool_env_var(&env, "CHUTORO_BENCH_HNSW_RECALL_REPORT"), + expected + ); +} + +#[rstest::rstest] +#[case::recall_enabled( + super::super::environment::should_collect_recall_report_with_env, + "CHUTORO_BENCH_HNSW_RECALL_REPORT", + "true" +)] +#[case::cluster_quality_enabled( + super::super::environment::should_collect_cluster_quality_report_with_env, + "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT", + "true" +)] +fn explicit_report_enablement_overrides_benchmark_arguments( + #[case] should_collect: fn(&dyn mockable::Env) -> bool, + #[case] environment_key: &'static str, + #[case] value: &'static str, +) { + let mut env = mockable::MockEnv::new(); + env.expect_string().returning(move |key| { + assert_eq!(key, environment_key); + Some(value.to_owned()) + }); + + assert!(should_collect(&env)); +} + +#[rstest::rstest] +#[case::recall_disabled( + super::super::environment::should_collect_recall_report_with_env, + "CHUTORO_BENCH_HNSW_RECALL_REPORT" +)] +#[case::cluster_quality_disabled( + super::super::environment::should_collect_cluster_quality_report_with_env, + "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT" +)] +fn explicit_report_disablement_overrides_benchmark_arguments( + #[case] should_collect: fn(&dyn mockable::Env) -> bool, + #[case] environment_key: &'static str, +) { + let mut env = mockable::MockEnv::new(); + env.expect_string().returning(move |key| { + assert_eq!(key, environment_key); + Some("false".to_owned()) + }); + + assert!(!should_collect(&env)); +} + +#[rstest::rstest] +#[case::recall( + super::super::environment::should_collect_recall_report_with_env, + "CHUTORO_BENCH_HNSW_RECALL_REPORT" +)] +#[case::cluster_quality( + super::super::environment::should_collect_cluster_quality_report_with_env, + "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT" +)] +fn absent_report_configuration_uses_discovery_mode( + #[case] should_collect: fn(&dyn mockable::Env) -> bool, + #[case] environment_key: &'static str, +) { + let mut env = mockable::MockEnv::new(); + env.expect_string().returning(move |key| { + assert_eq!(key, environment_key); + None + }); + + assert_eq!( + should_collect(&env), + !super::environment::is_discovery_mode() + ); +} + +#[test] +fn recall_report_path_defaults_when_environment_is_unset() { + let mut env = mockable::MockEnv::new(); + env.expect_os_string().returning(|key| { + assert_eq!(key, "CHUTORO_BENCH_HNSW_RECALL_REPORT_PATH"); + None + }); + + assert_eq!( + super::environment::recall_report_path_with_env(&env), + std::path::PathBuf::from(super::environment::RECALL_REPORT_PATH) + ); +} + +#[test] +fn recall_report_path_uses_configured_os_string() { + let configured_path = std::path::PathBuf::from("reports/recall.csv"); + let expected_path = configured_path.clone(); + let mut env = mockable::MockEnv::new(); + env.expect_os_string().returning(move |key| { + assert_eq!(key, "CHUTORO_BENCH_HNSW_RECALL_REPORT_PATH"); + Some(std::ffi::OsString::from(configured_path.clone())) + }); + + assert_eq!( + super::environment::recall_report_path_with_env(&env), + expected_path + ); +} + +#[test] +fn cluster_quality_report_path_defaults_when_environment_is_unset() { + let mut env = mockable::MockEnv::new(); + env.expect_os_string().returning(|key| { + assert_eq!(key, "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT_PATH"); + None + }); + + assert_eq!( + super::environment::cluster_quality_report_path_with_env(&env), + std::path::PathBuf::from(super::environment::CLUSTERING_QUALITY_REPORT_PATH) + ); +} + +#[test] +fn cluster_quality_report_path_uses_configured_os_string() { + let configured_path = std::path::PathBuf::from("reports/cluster-quality.csv"); + let expected_path = configured_path.clone(); + let mut env = mockable::MockEnv::new(); + env.expect_os_string().returning(move |key| { + assert_eq!(key, "CHUTORO_BENCH_HNSW_CLUSTER_QUALITY_REPORT_PATH"); + Some(std::ffi::OsString::from(configured_path.clone())) + }); + + assert_eq!( + super::environment::cluster_quality_report_path_with_env(&env), + expected_path + ); +} diff --git a/chutoro-benches/benches/internal/hnsw_environment.rs b/chutoro-benches/benches/internal/hnsw_environment.rs new file mode 100644 index 00000000..a5024ac8 --- /dev/null +++ b/chutoro-benches/benches/internal/hnsw_environment.rs @@ -0,0 +1,37 @@ +//! Shared environment policy for the HNSW benchmark and its focused tests. + +use std::path::PathBuf; + +use chutoro_benches::criterion_support::{is_benchmark_discovery, is_exact_benchmark_probe}; +use mockable::Env; + +/// Report destination for derived memory metrics. +pub(super) const MEMORY_REPORT_PATH: &str = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../target/benchmarks/hnsw_memory_profile.csv" +); + +/// Determine whether this invocation should collect HNSW memory measurements. +pub(super) fn should_collect_memory_profile_with_env(env: &dyn Env) -> bool { + if let Some(value) = env.string("CHUTORO_BENCH_HNSW_MEMORY_PROFILE") { + let normalized = value.trim().to_ascii_lowercase(); + if matches!(normalized.as_str(), "0" | "false" | "off") { + return false; + } + if matches!(normalized.as_str(), "1" | "true" | "on") { + return true; + } + } + !is_benchmark_discovery() && !is_exact_benchmark_probe() +} + +/// Resolve the memory-report path through an injected environment reader. +pub(super) fn memory_report_path_with_env(env: &dyn Env) -> PathBuf { + env.os_string("CHUTORO_BENCH_HNSW_MEMORY_REPORT_PATH") + .map_or_else(|| PathBuf::from(MEMORY_REPORT_PATH), PathBuf::from) +} + +/// Determine whether the diverse-source benchmark should include MNIST. +pub(super) fn should_include_mnist_with_env(env: &dyn Env) -> bool { + env.string("CHUTORO_BENCH_ENABLE_MNIST").as_deref() == Some("1") +} diff --git a/chutoro-benches/benches/internal/hnsw_memory_profile.rs b/chutoro-benches/benches/internal/hnsw_memory_profile.rs new file mode 100644 index 00000000..16ca11d6 --- /dev/null +++ b/chutoro-benches/benches/internal/hnsw_memory_profile.rs @@ -0,0 +1,71 @@ +//! Optional memory profiling for HNSW Criterion benchmarks. + +use std::{path::PathBuf, time::Duration}; + +use mockable::{DefaultEnv, Env}; + +use chutoro_benches::{ + ef_sweep::make_bench_source, + error::BenchSetupError, + profiling::{ + EdgeScalingBounds, HnswMemoryInput, HnswMemoryRecord, ProfilingError, + measure_peak_resident_set_size, write_hnsw_memory_report, + }, +}; +use chutoro_core::CpuHnsw; + +use super::environment::{memory_report_path_with_env, should_collect_memory_profile_with_env}; +use super::{MAX_CONNECTIONS, POINT_COUNTS, make_hnsw_params}; + +/// Sampling cadence for peak resident-set-size profiling. +const MEMORY_SAMPLE_INTERVAL: Duration = Duration::from_millis(2); + +/// Multiplicative edge-scaling tolerance around `expected = n * M`. +const EDGE_SCALING_BOUNDS: EdgeScalingBounds = EdgeScalingBounds::new(8, 8); + +/// Collect and write optional HNSW memory measurements before benchmark setup. +pub(super) fn profile_hnsw_memory_impl() -> Result, BenchSetupError> { + profile_hnsw_memory_impl_with_env(&DefaultEnv) +} + +/// Collect memory measurements through an injected environment reader. +fn profile_hnsw_memory_impl_with_env(env: &dyn Env) -> Result, BenchSetupError> { + if !should_collect_memory_profile_with_env(env) { + return Ok(None); + } + + let report_path = memory_report_path_with_env(env); + let mut records = Vec::new(); + + for &point_count in POINT_COUNTS { + let source = make_bench_source(point_count)?; + + for &m in MAX_CONNECTIONS { + let params = make_hnsw_params(m)?; + let ef_construction = params.ef_construction(); + let (build_result, measurement) = + match measure_peak_resident_set_size(MEMORY_SAMPLE_INTERVAL, || { + CpuHnsw::build_with_edges(&source, params.clone()) + }) { + Ok(measurement) => measurement, + Err(ProfilingError::UnsupportedPlatform { .. }) => return Ok(None), + Err(err) => return Err(err.into()), + }; + let (_index, harvest) = build_result?; + records.push(HnswMemoryRecord::new( + HnswMemoryInput { + point_count, + max_connections: m, + ef_construction, + measurement, + edge_count: harvest.len(), + }, + EDGE_SCALING_BOUNDS, + )?); + } + } + + write_hnsw_memory_report(&report_path, &records) + .map(Some) + .map_err(BenchSetupError::from) +} diff --git a/chutoro-benches/benches/internal/hnsw_tests.rs b/chutoro-benches/benches/internal/hnsw_tests.rs new file mode 100644 index 00000000..d21bbb42 --- /dev/null +++ b/chutoro-benches/benches/internal/hnsw_tests.rs @@ -0,0 +1,108 @@ +//! Tests for HNSW memory-profile environment configuration. + +use super::environment::*; +use chutoro_benches::criterion_support::{is_benchmark_discovery, is_exact_benchmark_probe}; + +#[rstest::fixture] +fn env_with_memory_settings_unset() -> mockable::MockEnv { + let mut env = mockable::MockEnv::new(); + + env.expect_string().returning(|key| { + assert_eq!(key, "CHUTORO_BENCH_HNSW_MEMORY_PROFILE"); + None + }); + env.expect_os_string().returning(|key| { + assert_eq!(key, "CHUTORO_BENCH_HNSW_MEMORY_REPORT_PATH"); + None + }); + + env +} + +#[test] +fn memory_profile_explicitly_disabled_by_environment() { + let mut env = mockable::MockEnv::new(); + env.expect_string().returning(|key| { + assert_eq!(key, "CHUTORO_BENCH_HNSW_MEMORY_PROFILE"); + Some("false".to_owned()) + }); + + assert!(!should_collect_memory_profile_with_env(&env)); +} + +#[test] +fn memory_profile_explicitly_enabled_by_environment() { + let mut env = mockable::MockEnv::new(); + env.expect_string().returning(|key| { + assert_eq!(key, "CHUTORO_BENCH_HNSW_MEMORY_PROFILE"); + Some("true".to_owned()) + }); + + assert!(should_collect_memory_profile_with_env(&env)); +} + +#[test] +fn memory_profile_uses_benchmark_mode_when_environment_is_unset() { + let env = env_with_memory_settings_unset(); + + assert_eq!( + should_collect_memory_profile_with_env(&env), + !is_benchmark_discovery() && !is_exact_benchmark_probe() + ); +} + +#[test] +fn memory_report_path_defaults_when_environment_is_unset() { + let env = env_with_memory_settings_unset(); + + assert_eq!( + memory_report_path_with_env(&env), + std::path::PathBuf::from(MEMORY_REPORT_PATH) + ); +} + +#[test] +fn memory_report_path_uses_configured_os_string() { + let configured_path = std::path::PathBuf::from("reports/hnsw-memory.csv"); + let expected_path = configured_path.clone(); + let mut env = mockable::MockEnv::new(); + env.expect_os_string().returning(move |key| { + assert_eq!(key, "CHUTORO_BENCH_HNSW_MEMORY_REPORT_PATH"); + Some(std::ffi::OsString::from(configured_path.clone())) + }); + + assert_eq!(memory_report_path_with_env(&env), expected_path); +} + +#[test] +fn mnist_is_excluded_when_environment_is_unset() { + let mut env = mockable::MockEnv::new(); + env.expect_string().returning(|key| { + assert_eq!(key, "CHUTORO_BENCH_ENABLE_MNIST"); + None + }); + + assert!(!should_include_mnist_with_env(&env)); +} + +#[test] +fn mnist_is_excluded_when_environment_is_disabled() { + let mut env = mockable::MockEnv::new(); + env.expect_string().returning(|key| { + assert_eq!(key, "CHUTORO_BENCH_ENABLE_MNIST"); + Some("0".to_owned()) + }); + + assert!(!should_include_mnist_with_env(&env)); +} + +#[test] +fn mnist_is_included_when_environment_is_enabled() { + let mut env = mockable::MockEnv::new(); + env.expect_string().returning(|key| { + assert_eq!(key, "CHUTORO_BENCH_ENABLE_MNIST"); + Some("1".to_owned()) + }); + + assert!(should_include_mnist_with_env(&env)); +} diff --git a/chutoro-benches/benches/internal/quality_pass.rs b/chutoro-benches/benches/internal/quality_pass.rs index 517a1218..e7b9f3d5 100644 --- a/chutoro-benches/benches/internal/quality_pass.rs +++ b/chutoro-benches/benches/internal/quality_pass.rs @@ -18,7 +18,7 @@ use chutoro_core::{ extract_labels_from_mst, parallel_kruskal, }; -use crate::{ +use super::{ CLUSTERING_QUALITY_CLUSTER_COUNT, CLUSTERING_QUALITY_MIN_CLUSTER_SIZE, CLUSTERING_QUALITY_POINT_COUNT, cluster_quality_report_path, should_collect_cluster_quality_report, diff --git a/chutoro-benches/src/neighbour_scoring/benchmark_runner.rs b/chutoro-benches/src/neighbour_scoring/benchmark_runner.rs index aff30504..0ccfd306 100644 --- a/chutoro-benches/src/neighbour_scoring/benchmark_runner.rs +++ b/chutoro-benches/src/neighbour_scoring/benchmark_runner.rs @@ -20,7 +20,7 @@ use mockable::{DefaultEnv, Env}; use super::{ CandidateBucket, ScoringFixture, benchmark_support::BenchError, benchmark_support::BenchResult, build_profile::report_parent_dir_with_env, make_fixture, scoring_plan, - write_build_profile_report, write_lane_utilisation_report, + write_build_profile_report, write_lane_utilization_report, }; /// Query row used for every neighbour-scoring benchmark iteration. @@ -109,7 +109,7 @@ fn neighbour_scoring_impl(c: &mut Criterion) -> BenchResult<()> { c, &DefaultEnv, ( - |report_parent_dir| write_lane_utilisation_report(report_parent_dir).map(drop), + |report_parent_dir| write_lane_utilization_report(report_parent_dir).map(drop), |report_parent_dir| write_build_profile_report(report_parent_dir).map(drop), ), bench_case, @@ -153,7 +153,7 @@ fn neighbour_scoring_impl_with( /// Registers the neighbour-scoring benchmark group and its diagnostic reports. /// /// This creates the `neighbour_scoring` Criterion group and writes its -/// lane-utilisation and optional build-profile CSV diagnostics before +/// lane-utilization and optional build-profile CSV diagnostics before /// registering benchmark cases. /// /// # Examples diff --git a/chutoro-benches/src/neighbour_scoring/benchmark_support.rs b/chutoro-benches/src/neighbour_scoring/benchmark_support.rs index d57493e7..ba23ecc4 100644 --- a/chutoro-benches/src/neighbour_scoring/benchmark_support.rs +++ b/chutoro-benches/src/neighbour_scoring/benchmark_support.rs @@ -6,9 +6,9 @@ use std::{io, num::TryFromIntError, sync::Arc, time::Instant}; use super::{ - BUILD_PROFILE_REPORT, BuildProfileReportRow, LANE_REPORT, LaneUtilisationReportRow, + BUILD_PROFILE_REPORT, BuildProfileReportRow, LANE_REPORT, LaneUtilizationReportRow, REPORT_DIR_NAME, ReportTarget, report_path_value, sorted_median, - write_build_profile_report_csv, write_lane_utilisation_report_csv, + write_build_profile_report_csv, write_lane_utilization_report_csv, }; use crate::source::{SyntheticConfig, SyntheticError, SyntheticSource}; use arrow_array::{ArrayRef, FixedSizeListArray, Float32Array}; @@ -155,16 +155,16 @@ pub(super) fn make_fixture( }) } -/// Write the lane-utilisation report for all candidate buckets. -pub(super) fn write_lane_utilisation_report( +/// Write the lane-utilization report for all candidate buckets. +pub(super) fn write_lane_utilization_report( report_parent_dir: &Utf8Path, ) -> BenchResult { let report_dir = open_report_dir(report_parent_dir)?; let target = report_path_value(report_parent_dir, LANE_REPORT); let mut file = report_dir.create(LANE_REPORT)?; - write_lane_utilisation_report_csv( + write_lane_utilization_report_csv( &mut file, - all_buckets().map(|bucket| LaneUtilisationReportRow { + all_buckets().map(|bucket| LaneUtilizationReportRow { bucket_kind: bucket.kind_name(), candidate_count: bucket.size(), }), diff --git a/chutoro-benches/src/neighbour_scoring/benchmark_support_tests.rs b/chutoro-benches/src/neighbour_scoring/benchmark_support_tests.rs index 10bc0e32..da942c0d 100644 --- a/chutoro-benches/src/neighbour_scoring/benchmark_support_tests.rs +++ b/chutoro-benches/src/neighbour_scoring/benchmark_support_tests.rs @@ -57,18 +57,20 @@ fn fixture_contains_provider_rows_and_one_based_candidates() { } #[rstest] -fn lane_utilisation_report_writes_expected_file( +fn lane_utilization_report_writes_expected_file( #[from(utf8_temp_dir)] temp_dir_result: Result, ) { use crate::neighbour_scoring::REPORT_DIR_NAME; - use super::{LANE_REPORT, write_lane_utilisation_report}; + use super::{LANE_REPORT, write_lane_utilization_report}; + + assert_eq!(LANE_REPORT, "neighbour_scoring_lane_utilisation.csv"); let temp_dir = temp_dir_result.expect("temp dir must be created"); let report_parent_dir = temp_dir.path(); - let report_path = write_lane_utilisation_report(report_parent_dir) - .expect("lane utilisation report must be written"); + let report_path = write_lane_utilization_report(report_parent_dir) + .expect("lane utilization report must be written"); assert_eq!( report_path, diff --git a/chutoro-benches/src/neighbour_scoring/mod.rs b/chutoro-benches/src/neighbour_scoring/mod.rs index 407822a1..fdd13562 100644 --- a/chutoro-benches/src/neighbour_scoring/mod.rs +++ b/chutoro-benches/src/neighbour_scoring/mod.rs @@ -11,13 +11,13 @@ mod report; use benchmark_plan::{CandidateBucket, all_buckets, scoring_plan}; use benchmark_support::{ - ScoringFixture, make_fixture, write_build_profile_report, write_lane_utilisation_report, + ScoringFixture, make_fixture, write_build_profile_report, write_lane_utilization_report, }; use profiling::{ProfilingError, ProfilingSource}; /// Register the neighbour-scoring Criterion group and its diagnostic reports. /// -/// Call this from a Criterion benchmark entrypoint. It writes lane-utilisation +/// Call this from a Criterion benchmark entrypoint. It writes lane-utilization /// and optional build-profile CSV reports before registering the benchmark /// cases. /// @@ -43,17 +43,17 @@ pub use build_profile::{ #[doc(hidden)] pub use profiling::{duration_nanos, saturating_add_u64, saturating_add_usize}; pub use report::{ - BuildProfileReportRow, LaneUtilisationReportRow, write_build_profile_report_csv, - write_lane_utilisation_report_csv, + BuildProfileReportRow, LaneUtilizationReportRow, write_build_profile_report_csv, + write_lane_utilization_report_csv, }; -/// Lane-utilisation report filename. +/// Lane-utilization report filename. pub const LANE_REPORT: &str = "neighbour_scoring_lane_utilisation.csv"; /// Number of candidate lanes represented by one dense-provider `SoA` block. const SIMD_LANES: usize = 16; -/// Calculate active-lane utilisation in basis points for one candidate bucket. +/// Calculate active-lane utilization in basis points for one candidate bucket. /// /// Returns `0` for an empty bucket. Non-empty buckets are rounded down to the /// integer basis-point value implied by padding the bucket to a full @@ -62,21 +62,21 @@ const SIMD_LANES: usize = 16; /// # Examples /// /// ``` -/// use chutoro_benches::neighbour_scoring::lane_utilisation_basis_points; +/// use chutoro_benches::neighbour_scoring::lane_utilization_basis_points; /// -/// assert_eq!(lane_utilisation_basis_points(8), 5_000); -/// assert_eq!(lane_utilisation_basis_points(16), 10_000); +/// assert_eq!(lane_utilization_basis_points(8), 5_000); +/// assert_eq!(lane_utilization_basis_points(16), 10_000); /// ``` #[expect( clippy::integer_division, - reason = "basis-point utilisation is an integer diagnostic report" + reason = "basis-point utilization is an integer diagnostic report" )] #[expect( clippy::integer_division_remainder_used, - reason = "basis-point utilisation is an integer diagnostic report" + reason = "basis-point utilization is an integer diagnostic report" )] #[must_use] -pub fn lane_utilisation_basis_points(candidate_count: usize) -> usize { +pub fn lane_utilization_basis_points(candidate_count: usize) -> usize { if candidate_count == 0 { return 0; } @@ -184,7 +184,7 @@ mod tests { use proptest::prelude::*; use rstest::rstest; - use super::{SIMD_LANES, duration_basis_points, lane_utilisation_basis_points, sorted_median}; + use super::{SIMD_LANES, duration_basis_points, lane_utilization_basis_points, sorted_median}; #[rstest] #[case(0, 0)] @@ -192,11 +192,11 @@ mod tests { #[case(16, 10_000)] #[case(24, 7_500)] #[case(usize::MAX, 9_999)] - fn lane_utilisation_handles_candidate_counts( + fn lane_utilization_handles_candidate_counts( #[case] candidate_count: usize, #[case] expected: usize, ) { - assert_eq!(lane_utilisation_basis_points(candidate_count), expected); + assert_eq!(lane_utilization_basis_points(candidate_count), expected); } #[rstest] @@ -253,17 +253,17 @@ mod tests { proptest! { #[test] - fn lane_utilisation_is_bounded(candidate_count in 1_usize..=1_000_000) { - let utilisation = lane_utilisation_basis_points(candidate_count); + fn lane_utilization_is_bounded(candidate_count in 1_usize..=1_000_000) { + let utilization = lane_utilization_basis_points(candidate_count); - prop_assert!((1..=10_000).contains(&utilisation)); + prop_assert!((1..=10_000).contains(&utilization)); if candidate_count.next_multiple_of(SIMD_LANES) == candidate_count { - prop_assert_eq!(utilisation, 10_000); + prop_assert_eq!(utilization, 10_000); } } #[test] - fn lane_utilisation_increases_within_one_padding_block( + fn lane_utilization_increases_within_one_padding_block( bucket in 0_usize..=1_024, lower_offset in 1_usize..=SIMD_LANES, offset_delta in 0_usize..SIMD_LANES, @@ -274,8 +274,8 @@ mod tests { let upper_candidate_count = block_start.saturating_add(upper_offset); prop_assert!( - lane_utilisation_basis_points(lower_candidate_count) - <= lane_utilisation_basis_points(upper_candidate_count) + lane_utilization_basis_points(lower_candidate_count) + <= lane_utilization_basis_points(upper_candidate_count) ); } diff --git a/chutoro-benches/src/neighbour_scoring/report.rs b/chutoro-benches/src/neighbour_scoring/report.rs index 490fccb4..cbeb659a 100644 --- a/chutoro-benches/src/neighbour_scoring/report.rs +++ b/chutoro-benches/src/neighbour_scoring/report.rs @@ -6,11 +6,11 @@ use std::{ time::Duration, }; -use super::{duration_basis_points, lane_utilisation_basis_points, padded_lane_count}; +use super::{duration_basis_points, lane_utilization_basis_points, padded_lane_count}; -/// One row in the lane-utilisation diagnostic CSV report. +/// One row in the lane-utilization diagnostic CSV report. #[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct LaneUtilisationReportRow<'a> { +pub struct LaneUtilizationReportRow<'a> { /// Bucket label, for example `realistic` or `diagnostic`. pub bucket_kind: &'a str, /// Number of active candidates scored in the bucket. @@ -59,19 +59,19 @@ fn csv_escape(field: &str) -> Cow<'_, str> { } } -/// Write the lane-utilisation report schema and rows. +/// Write the lane-utilization report schema and rows. /// /// # Examples /// /// ``` /// use chutoro_benches::neighbour_scoring::{ -/// LaneUtilisationReportRow, write_lane_utilisation_report_csv, +/// LaneUtilizationReportRow, write_lane_utilization_report_csv, /// }; /// /// let mut output = Vec::new(); -/// let result = write_lane_utilisation_report_csv( +/// let result = write_lane_utilization_report_csv( /// &mut output, -/// [LaneUtilisationReportRow { +/// [LaneUtilizationReportRow { /// bucket_kind: "realistic", /// candidate_count: 8, /// }], @@ -91,10 +91,10 @@ fn csv_escape(field: &str) -> Cow<'_, str> { /// # Errors /// /// Returns any error reported by the supplied writer. -pub fn write_lane_utilisation_report_csv<'a, W, I>(mut writer: W, rows: I) -> io::Result<()> +pub fn write_lane_utilization_report_csv<'a, W, I>(mut writer: W, rows: I) -> io::Result<()> where W: Write, - I: IntoIterator>, + I: IntoIterator>, { writeln!( writer, @@ -103,11 +103,11 @@ where for row in rows { let padded = padded_lane_count(row.candidate_count); let wasted = padded.saturating_sub(row.candidate_count as u128); - let utilisation = lane_utilisation_basis_points(row.candidate_count); + let utilization = lane_utilization_basis_points(row.candidate_count); let bucket_kind = csv_escape(row.bucket_kind); writeln!( writer, - "{bucket_kind},{},{padded},{wasted},{utilisation}", + "{bucket_kind},{},{padded},{wasted},{utilization}", row.candidate_count )?; } diff --git a/chutoro-benches/src/neighbour_scoring/report_tests.rs b/chutoro-benches/src/neighbour_scoring/report_tests.rs index ba68da6f..3dc9419a 100644 --- a/chutoro-benches/src/neighbour_scoring/report_tests.rs +++ b/chutoro-benches/src/neighbour_scoring/report_tests.rs @@ -1,9 +1,9 @@ //! Tests for neighbour-scoring CSV report rendering. use super::{ - BuildProfileReportRow, LaneUtilisationReportRow, duration_basis_points, - lane_utilisation_basis_points, padded_lane_count, write_build_profile_report_csv, - write_lane_utilisation_report_csv, + BuildProfileReportRow, LaneUtilizationReportRow, duration_basis_points, + lane_utilization_basis_points, padded_lane_count, write_build_profile_report_csv, + write_lane_utilization_report_csv, }; use proptest::prelude::*; use rstest::rstest; @@ -12,11 +12,11 @@ use std::time::Duration; #[rstest] #[case::multiple_rows( vec![ - LaneUtilisationReportRow { + LaneUtilizationReportRow { bucket_kind: "realistic", candidate_count: 8, }, - LaneUtilisationReportRow { + LaneUtilizationReportRow { bucket_kind: "diagnostic", candidate_count: 24, }, @@ -29,7 +29,7 @@ use std::time::Duration; ) )] #[case::escaped_bucket_kind( - vec![LaneUtilisationReportRow { + vec![LaneUtilizationReportRow { bucket_kind: "diagnostic,\n\"quoted\"\r", candidate_count: 16, }], @@ -39,14 +39,14 @@ use std::time::Duration; "\"diagnostic,\n\"\"quoted\"\"\r\",16,16,0,10000\n", ) )] -fn write_lane_utilisation_report_csv_renders_schema_and_rows( - #[case] rows: Vec>, +fn write_lane_utilization_report_csv_renders_schema_and_rows( + #[case] rows: Vec>, #[case] expected: &str, ) { let mut csv_buffer = Vec::new(); - let result = write_lane_utilisation_report_csv(&mut csv_buffer, rows); + let result = write_lane_utilization_report_csv(&mut csv_buffer, rows); - result.expect("render lane-utilisation CSV report"); + result.expect("render lane-utilization CSV report"); assert_eq!(String::from_utf8_lossy(&csv_buffer), expected); } @@ -138,15 +138,15 @@ fn parse_csv_records(csv: &str) -> Vec> { proptest! { #[test] - fn lane_utilisation_report_round_trips_generated_rows( + fn lane_utilization_report_round_trips_generated_rows( bucket_kind in csv_label(), candidate_count in any::(), ) { - let row = LaneUtilisationReportRow { bucket_kind: &bucket_kind, candidate_count }; + let row = LaneUtilizationReportRow { bucket_kind: &bucket_kind, candidate_count }; let mut csv_buffer = Vec::new(); - write_lane_utilisation_report_csv(&mut csv_buffer, [row]) - .expect("render generated lane-utilisation CSV row"); - let csv = String::from_utf8(csv_buffer).expect("lane-utilisation CSV is UTF-8"); + write_lane_utilization_report_csv(&mut csv_buffer, [row]) + .expect("render generated lane-utilization CSV row"); + let csv = String::from_utf8(csv_buffer).expect("lane-utilization CSV is UTF-8"); let records = parse_csv_records(&csv); prop_assert_eq!(records.len(), 2); let data_record = records.get(1).expect("CSV data row exists"); @@ -163,14 +163,14 @@ proptest! { .parse().expect("parse padded lane count"); let wasted_lanes: u128 = data_record.get(3).expect("CSV wasted-lanes field exists") .parse().expect("parse wasted lane count"); - let utilisation: usize = data_record.get(4).expect("CSV lane-utilisation field exists") - .parse().expect("parse lane-utilisation basis points"); + let utilization: usize = data_record.get(4).expect("CSV lane-utilization field exists") + .parse().expect("parse lane-utilization basis points"); let expected_padded = padded_lane_count(candidate_count); prop_assert!(padded_lanes >= candidate_count as u128); prop_assert_eq!(padded_lanes, expected_padded); prop_assert_eq!(wasted_lanes, padded_lanes - candidate_count as u128); - prop_assert_eq!(utilisation, lane_utilisation_basis_points(candidate_count)); - prop_assert!(utilisation <= 10_000); + prop_assert_eq!(utilization, lane_utilization_basis_points(candidate_count)); + prop_assert!(utilization <= 10_000); } #[test] diff --git a/chutoro-benches/src/params.rs b/chutoro-benches/src/params.rs index d56243fb..5f44fb99 100644 --- a/chutoro-benches/src/params.rs +++ b/chutoro-benches/src/params.rs @@ -50,7 +50,7 @@ pub struct ExtractionBenchParams { impl fmt::Display for ExtractionBenchParams { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "n={},min={}", self.point_count, self.min_cluster_size,) + write!(f, "n={},min={}", self.point_count, self.min_cluster_size) } } diff --git a/chutoro-benches/tests/hnsw_ef_sweep_environment.rs b/chutoro-benches/tests/hnsw_ef_sweep_environment.rs new file mode 100644 index 00000000..3b20c85b --- /dev/null +++ b/chutoro-benches/tests/hnsw_ef_sweep_environment.rs @@ -0,0 +1,6 @@ +//! Exercises the ef-sweep environment tests as a Cargo test target. + +#[path = "../benches/internal/hnsw_ef_sweep_environment.rs"] +mod environment; +#[path = "../benches/internal/hnsw_ef_sweep_tests.rs"] +mod tests; diff --git a/chutoro-benches/tests/hnsw_environment.rs b/chutoro-benches/tests/hnsw_environment.rs new file mode 100644 index 00000000..fa1e91dd --- /dev/null +++ b/chutoro-benches/tests/hnsw_environment.rs @@ -0,0 +1,7 @@ +//! Exercises the HNSW benchmark environment tests as a Cargo test target. + +#[path = "../benches/internal/hnsw_environment.rs"] +mod environment; + +#[path = "../benches/internal/hnsw_tests.rs"] +mod tests; diff --git a/chutoro-benches/tests/neighbour_scoring_support.rs b/chutoro-benches/tests/neighbour_scoring_support.rs index 56070f9f..44f83739 100644 --- a/chutoro-benches/tests/neighbour_scoring_support.rs +++ b/chutoro-benches/tests/neighbour_scoring_support.rs @@ -3,10 +3,10 @@ use camino::Utf8Path; use cap_std::{ambient_authority, fs_utf8::Dir}; use chutoro_benches::neighbour_scoring::{ - BUILD_PROFILE_REPORT, BuildProfileReportRow, LaneUtilisationReportRow, REPORT_DIR_NAME, + BUILD_PROFILE_REPORT, BuildProfileReportRow, LaneUtilizationReportRow, REPORT_DIR_NAME, ReportTarget, build_profile_report_target_value, report_parent_dir_value, should_collect_build_profile_value, truthy_env_value, write_build_profile_report_csv, - write_lane_utilisation_report_csv, + write_lane_utilization_report_csv, }; use rstest::{fixture, rstest}; use std::{io::Write, process::Command, time::Duration}; @@ -100,7 +100,7 @@ fn neighbour_scoring_script_wires_expected_benchmark_binary() { } #[rstest] -fn lane_utilisation_report_file_generation_writes_schema_and_rows( +fn lane_utilization_report_file_generation_writes_schema_and_rows( #[from(report_directory)] report_directory_result: Result, ) { let report_directory = report_directory_result.expect("report directory must be created"); @@ -110,14 +110,14 @@ fn lane_utilisation_report_file_generation_writes_schema_and_rows( .and_then(|dir| dir.create("lane.csv")) .expect("report file must be created"); - write_lane_utilisation_report_csv( + write_lane_utilization_report_csv( &mut file, - [LaneUtilisationReportRow { + [LaneUtilizationReportRow { bucket_kind: "realistic", candidate_count: 8, }], ) - .expect("lane utilisation report must be written"); + .expect("lane utilization report must be written"); let contents = report_directory .root diff --git a/chutoro-core/src/chutoro.rs b/chutoro-core/src/chutoro/mod.rs similarity index 99% rename from chutoro-core/src/chutoro.rs rename to chutoro-core/src/chutoro/mod.rs index 4addf93c..475c1d70 100644 --- a/chutoro-core/src/chutoro.rs +++ b/chutoro-core/src/chutoro/mod.rs @@ -358,9 +358,9 @@ impl Chutoro { } #[cfg(test)] -#[path = "chutoro_tests.rs"] +#[path = "../chutoro_tests.rs"] mod tests; #[cfg(all(test, feature = "cpu"))] -#[path = "chutoro/properties.rs"] +#[path = "properties.rs"] mod properties; diff --git a/chutoro-core/src/hnsw/cpu/construction.rs b/chutoro-core/src/hnsw/cpu/construction.rs index 18c5e710..629a0948 100644 --- a/chutoro-core/src/hnsw/cpu/construction.rs +++ b/chutoro-core/src/hnsw/cpu/construction.rs @@ -57,7 +57,7 @@ impl CpuHnsw { /// /// # Errors /// - /// Returns [`HnswError`] when initialisation or a source-backed insertion + /// Returns [`HnswError`] when initialization or a source-backed insertion /// fails. pub fn build(source: &D, params: HnswParams) -> Result { let index = Self::build_initial(source, params)?; @@ -110,7 +110,7 @@ impl CpuHnsw { /// /// # Errors /// - /// Returns [`HnswError`] when initialisation, insertion, or edge harvesting + /// Returns [`HnswError`] when initialization, insertion, or edge harvesting /// fails. pub fn build_with_edges( source: &D, diff --git a/chutoro-core/src/hnsw/cpu/test_helpers.rs b/chutoro-core/src/hnsw/cpu/test_helpers.rs index 74942e45..7ccfdfc1 100644 --- a/chutoro-core/src/hnsw/cpu/test_helpers.rs +++ b/chutoro-core/src/hnsw/cpu/test_helpers.rs @@ -37,11 +37,8 @@ impl CpuHnsw { } } - pub(crate) fn inspect_graph(&self, f: impl FnOnce(&Graph) -> R) -> R { - match self.read_graph(|graph| Ok(f(graph))) { - Ok(result) => result, - Err(err) => panic!("graph lock during inspect_graph: {err}"), - } + pub(crate) fn inspect_graph(&self, f: impl FnOnce(&Graph) -> R) -> Result { + self.read_graph(|graph| Ok(f(graph))) } pub(crate) fn current_thread_holds_write_graph_for_test() -> bool { @@ -61,19 +58,17 @@ impl CpuHnsw { Ok(deleted) } - pub(crate) fn reconfigure_for_test(&mut self, params: HnswParams) { + pub(crate) fn reconfigure_for_test(&mut self, params: HnswParams) -> Result<(), HnswError> { + self.write_graph(|graph| { + graph.set_params(¶ms); + Ok(()) + })?; let base_seed = params.rng_seed(); self.rng = Mutex::new(SmallRng::seed_from_u64(base_seed)); self.worker_rngs = build_worker_rngs(base_seed); self.distance_cache = DistanceCache::new(*params.distance_cache_config()); self.params = params; - let reconfigured = self.write_graph(|graph| { - graph.set_params(&self.params); - Ok(()) - }); - if let Err(err) = reconfigured { - panic!("graph lock during reconfigure_for_test: {err}"); - } + Ok(()) } } @@ -84,3 +79,49 @@ impl Drop for WriteGraphMarkerGuard { internal::disable_write_graph_marker(); } } + +#[cfg(test)] +mod tests { + //! Regression coverage for fallible graph inspection and reconfiguration helpers. + + use std::panic::{AssertUnwindSafe, catch_unwind}; + + use super::*; + + #[test] + fn graph_helpers_report_poisoning_without_reconfiguring() { + let params = HnswParams::new(2, 4).expect("initial parameters must be valid"); + let mut index = CpuHnsw::with_capacity(params, 3).expect("index must initialize"); + + let poison_result = catch_unwind(AssertUnwindSafe(|| { + let _guard = index + .graph + .write() + .expect("fresh graph lock must be available"); + panic!("poison the graph lock for this test"); + })); + assert!(poison_result.is_err(), "graph lock must be poisoned"); + + assert!( + matches!( + index.inspect_graph(|_| ()), + Err(HnswError::LockPoisoned { resource: "graph" }) + ), + "inspection must report the poisoned graph lock", + ); + + let replacement = HnswParams::new(3, 6).expect("replacement parameters must be valid"); + assert!( + matches!( + index.reconfigure_for_test(replacement), + Err(HnswError::LockPoisoned { resource: "graph" }) + ), + "reconfiguration must report the poisoned graph lock", + ); + assert_eq!( + index.params.max_connections(), + 2, + "failed reconfiguration must retain the active parameters", + ); + } +} diff --git a/chutoro-core/src/hnsw/distance_cache/mod.rs b/chutoro-core/src/hnsw/distance_cache/mod.rs index 3f790473..b103cd1f 100644 --- a/chutoro-core/src/hnsw/distance_cache/mod.rs +++ b/chutoro-core/src/hnsw/distance_cache/mod.rs @@ -108,7 +108,7 @@ struct DistanceKey { } impl DistanceKey { - /// Construct a key by canonicalising the node-pair order. + /// Construct a key by canonicalizing the node-pair order. const fn new(metric: MetricDescriptor, a: usize, b: usize) -> Self { let (left, right) = if a <= b { (a, b) } else { (b, a) }; Self { diff --git a/chutoro-core/src/hnsw/graph/core.rs b/chutoro-core/src/hnsw/graph/core.rs index b00ca659..21039123 100644 --- a/chutoro-core/src/hnsw/graph/core.rs +++ b/chutoro-core/src/hnsw/graph/core.rs @@ -278,7 +278,7 @@ impl Graph { Ok(()) } - /// Initialise an unoccupied graph slot with its node context. + /// Initialize an unoccupied graph slot with its node context. pub(crate) fn attach_node(&mut self, ctx: NodeContext) -> Result<(), HnswError> { self.attach_node_inner(ctx).map_err(|reason| match reason { AttachNodeError::LevelExceedsMax => HnswError::InvalidParameters { @@ -296,7 +296,7 @@ impl Graph { }) } - /// Validates the context and initialises the node slot. + /// Validates the context and initializes the node slot. /// /// Shared by the production and Kani constructors; returns a static /// reason so the Kani path never constructs formatted errors. diff --git a/chutoro-core/src/hnsw/graph/test_helpers/tests.rs b/chutoro-core/src/hnsw/graph/test_helpers/tests.rs index 4011791e..5da8b982 100644 --- a/chutoro-core/src/hnsw/graph/test_helpers/tests.rs +++ b/chutoro-core/src/hnsw/graph/test_helpers/tests.rs @@ -6,30 +6,30 @@ use crate::hnsw::{HnswError, graph::NodeContext, graph::core::Graph, params::Hns /// Creates default parameters for most deletion tests (`max_connections=2`, `ef_construction=4`). #[fixture] -fn basic_params() -> HnswParams { - match HnswParams::new(2, 4) { - Ok(params) => params, - Err(err) => panic!("params must be valid: {err}"), - } +fn basic_params() -> Result { + HnswParams::new(2, 4) } /// Creates a graph with capacity 3 using basic params. #[fixture] -fn small_graph(basic_params: HnswParams) -> Graph { - Graph::with_capacity(basic_params, 3) +fn small_graph( + #[from(basic_params)] basic_params_res: Result, +) -> Result { + let basic_params = basic_params_res?; + Ok(Graph::with_capacity(basic_params, 3)) } /// Creates restricted parameters for disconnection tests (`max_connections=1`, `ef_construction=1`). #[fixture] -fn restricted_params() -> HnswParams { - match HnswParams::new(1, 1) { - Ok(params) => params, - Err(err) => panic!("params must be valid: {err}"), - } +fn restricted_params() -> Result { + HnswParams::new(1, 1) } #[rstest] -fn delete_node_reconnects_neighbours_and_preserves_reachability(mut small_graph: Graph) { +fn delete_node_reconnects_neighbours_and_preserves_reachability( + #[from(small_graph)] small_graph_res: Result, +) { + let mut small_graph = small_graph_res.expect("deletion graph fixture must be valid"); small_graph .insert_first(NodeContext { node: 0, @@ -69,7 +69,10 @@ fn delete_node_reconnects_neighbours_and_preserves_reachability(mut small_graph: } #[rstest] -fn delete_node_returns_ok_false_for_missing_node(mut small_graph: Graph) { +fn delete_node_returns_ok_false_for_missing_node( + #[from(small_graph)] small_graph_res: Result, +) { + let mut small_graph = small_graph_res.expect("deletion graph fixture must be valid"); small_graph .insert_first(NodeContext { node: 0, @@ -98,17 +101,23 @@ fn delete_node_returns_ok_false_for_missing_node(mut small_graph: Graph) { } #[rstest] -fn delete_node_returns_invalid_parameters_for_out_of_bounds_index(mut small_graph: Graph) { +fn delete_node_returns_invalid_parameters_for_out_of_bounds_index( + #[from(small_graph)] small_graph_res: Result, +) { + let mut small_graph = small_graph_res.expect("deletion graph fixture must be valid"); let result = small_graph.delete_node(5); match result { Err(HnswError::InvalidParameters { .. }) => {} - other => panic!("expected Err(HnswError::InvalidParameters {{ .. }}), got {other:?}",), + other => panic!("expected Err(HnswError::InvalidParameters {{ .. }}), got {other:?}"), } } #[rstest] -fn delete_node_reverts_when_it_would_disconnect_graph(restricted_params: HnswParams) { +fn delete_node_reverts_when_it_would_disconnect_graph( + #[from(restricted_params)] restricted_params_res: Result, +) { + let restricted_params = restricted_params_res.expect("restricted parameters must be valid"); let mut graph = Graph::with_capacity(restricted_params, 5); graph .insert_first(NodeContext { diff --git a/chutoro-core/src/hnsw/helpers.rs b/chutoro-core/src/hnsw/helpers.rs index dc5968c6..45ed939d 100644 --- a/chutoro-core/src/hnsw/helpers.rs +++ b/chutoro-core/src/hnsw/helpers.rs @@ -163,7 +163,7 @@ pub(crate) fn batch_distances_for_trim( }); } - for ((index, miss), distance) in miss_meta.into_iter().zip(miss_distances.into_iter()) { + for ((index, miss), distance) in miss_meta.into_iter().zip(miss_distances) { cache.complete_miss(miss, distance)?; *distances .get_mut(index) diff --git a/chutoro-core/src/hnsw/insert/commit/mod.rs b/chutoro-core/src/hnsw/insert/commit/mod.rs index 1f7653b6..ebaefa11 100644 --- a/chutoro-core/src/hnsw/insert/commit/mod.rs +++ b/chutoro-core/src/hnsw/insert/commit/mod.rs @@ -9,13 +9,13 @@ use crate::hnsw::{error::HnswError, graph::Graph}; use super::{ reconciliation::EdgeReconciler, - types::{FinalisedUpdate, NewNodeContext, UpdateContext}, + types::{FinalizedUpdate, NewNodeContext, UpdateContext}, }; /// Final new-node neighbours and existing node-level lists touched by commit. pub(super) type ApplyUpdatesOutcome = (Vec>, Vec<(usize, usize)>); -/// Applies finalised insertion updates to a mutable graph. +/// Applies finalized insertion updates to a mutable graph. #[derive(Debug)] pub(super) struct CommitApplicator<'graph> { /// Graph that receives the committed adjacency-list changes. @@ -67,7 +67,7 @@ impl<'graph> CommitApplicator<'graph> { /// nodes now that their adjacency lists have been trimmed. pub(super) fn apply_neighbour_updates( &mut self, - final_updates: Vec, + final_updates: Vec, max_connections: usize, new_node: NewNodeContext, ) -> Result { diff --git a/chutoro-core/src/hnsw/insert/commit/tests/mod.rs b/chutoro-core/src/hnsw/insert/commit/tests/mod.rs index e6141a86..790fce30 100644 --- a/chutoro-core/src/hnsw/insert/commit/tests/mod.rs +++ b/chutoro-core/src/hnsw/insert/commit/tests/mod.rs @@ -317,7 +317,7 @@ mod metrics; fn isolation_replacement_keeps_bidirectionality( #[from(params_two_connections)] params_res: Result, ) -> Result<(), HnswError> { - let params = params_res.expect("params should be valid for tests"); + let params = params_res?; let max_connections = params.max_connections(); let mut graph = Graph::with_capacity(params, 3); diff --git a/chutoro-core/src/hnsw/insert/executor/mod.rs b/chutoro-core/src/hnsw/insert/executor/mod.rs index bbde816d..35a0a3f1 100644 --- a/chutoro-core/src/hnsw/insert/executor/mod.rs +++ b/chutoro-core/src/hnsw/insert/executor/mod.rs @@ -16,7 +16,7 @@ use super::reciprocity::ReciprocityAuditor; use super::reciprocity::ReciprocityWorkspace; use super::staging::InsertionStager; use super::types::{ - FinalisedUpdate, HealingContext, LayerProcessingOutcome, LinkContext, NewNodeContext, + FinalizedUpdate, HealingContext, LayerProcessingOutcome, LinkContext, NewNodeContext, PreparedInsertion, StagedUpdate, TrimWork, }; @@ -62,7 +62,7 @@ impl<'graph> InsertionExecutor<'graph> { let LayerProcessingOutcome { mut new_node_neighbours, staged, - initialised: _initialised, + initialized: _initialized, needs_trim, } = stager.process_insertion_layers( NodeContext { @@ -176,13 +176,13 @@ impl<'graph> InsertionExecutor<'graph> { fn prepare_final_updates( updates: Vec, trims: Vec, - ) -> Vec { + ) -> Vec { let mut trim_lookup: HashMap<(usize, usize), Vec> = trims .into_iter() .map(|result| ((result.node, result.ctx.level), result.neighbours)) .collect(); - let mut final_updates: Vec = Vec::with_capacity(updates.len()); + let mut final_updates: Vec = Vec::with_capacity(updates.len()); for update in updates { let neighbours = trim_lookup .remove(&(update.node, update.ctx.level)) diff --git a/chutoro-core/src/hnsw/insert/reciprocity.rs b/chutoro-core/src/hnsw/insert/reciprocity.rs index 910376a5..7b81b3f6 100644 --- a/chutoro-core/src/hnsw/insert/reciprocity.rs +++ b/chutoro-core/src/hnsw/insert/reciprocity.rs @@ -10,7 +10,7 @@ use std::collections::HashSet; #[cfg(any(test, debug_assertions))] use crate::hnsw::graph::Graph; -use super::{limits::compute_connection_limit, types::FinalisedUpdate}; +use super::{limits::compute_connection_limit, types::FinalizedUpdate}; /// Verifies reciprocity and degree bounds for insertion-touched nodes. #[cfg(any(test, debug_assertions))] @@ -119,7 +119,7 @@ pub(super) struct ReciprocityWorkspace<'a> { /// Pre-filter candidates used when a level needs a fallback link. pub(super) original: &'a [Vec], /// Final node updates whose reverse links may be adjusted. - pub(super) final_updates: &'a mut [FinalisedUpdate], + pub(super) final_updates: &'a mut [FinalizedUpdate], /// Identifier of the node receiving reciprocal links. pub(super) new_node: usize, /// Base connection limit used for fallback selection. @@ -164,7 +164,7 @@ struct FallbackSelector<'a> { /// Original candidates by graph level. original: &'a [Vec], /// Committed updates that may receive a reverse edge. - final_updates: &'a mut [FinalisedUpdate], + final_updates: &'a mut [FinalizedUpdate], /// Newly inserted node requiring a reciprocal link. new_node: usize, /// Base connection limit for each candidate level. diff --git a/chutoro-core/src/hnsw/insert/staging.rs b/chutoro-core/src/hnsw/insert/staging.rs index 55a1ac73..5e7be092 100644 --- a/chutoro-core/src/hnsw/insert/staging.rs +++ b/chutoro-core/src/hnsw/insert/staging.rs @@ -55,7 +55,7 @@ impl<'graph> InsertionStager<'graph> { ) -> Result { let mut new_node_neighbours = vec![Vec::new(); ctx.level + 1]; let mut staged: HashMap<(usize, usize), Vec> = HashMap::new(); - let mut initialised = HashSet::new(); + let mut initialized = HashSet::new(); let mut needs_trim = HashSet::new(); for layer in plan @@ -74,7 +74,7 @@ impl<'graph> InsertionStager<'graph> { level_capacity, &mut new_node_neighbours, &mut staged, - &mut initialised, + &mut initialized, &mut needs_trim, )?; } @@ -83,7 +83,7 @@ impl<'graph> InsertionStager<'graph> { Ok(LayerProcessingOutcome { new_node_neighbours, staged, - initialised, + initialized, needs_trim, }) } @@ -113,7 +113,7 @@ impl<'graph> InsertionStager<'graph> { level: lvl, max_connections, }; - prioritise_new_node(new_node.node, &mut candidates); + prioritize_new_node(new_node.node, &mut candidates); if needs_trim.contains(&(other, lvl)) { let mut sequences = Vec::with_capacity(candidates.len()); for &candidate in &candidates { @@ -188,7 +188,7 @@ impl<'graph> InsertionStager<'graph> { connection_limit: usize, new_node_neighbours: &mut [Vec], staged: &mut HashMap<(usize, usize), Vec>, - initialised: &mut HashSet<(usize, usize)>, + initialized: &mut HashSet<(usize, usize)>, needs_trim: &mut HashSet<(usize, usize)>, ) -> Result<(), HnswError> { let new_node_level = new_node_neighbours.get_mut(level_index).ok_or_else(|| { @@ -201,7 +201,7 @@ impl<'graph> InsertionStager<'graph> { new_node_level.push(neighbour); let key = (neighbour, level_index); - if initialised.insert(key) { + if initialized.insert(key) { let graph_node = self.graph .node(neighbour) @@ -238,7 +238,7 @@ impl<'graph> InsertionStager<'graph> { /// Move the newly inserted node to the front of candidate order. #[inline] -pub(super) fn prioritise_new_node(new_node: usize, candidates: &mut [usize]) { +pub(super) fn prioritize_new_node(new_node: usize, candidates: &mut [usize]) { if let Some(pos) = candidates .iter() .position(|&candidate| candidate == new_node) diff --git a/chutoro-core/src/hnsw/insert/types.rs b/chutoro-core/src/hnsw/insert/types.rs index c6d19473..2a0352bb 100644 --- a/chutoro-core/src/hnsw/insert/types.rs +++ b/chutoro-core/src/hnsw/insert/types.rs @@ -80,7 +80,7 @@ pub(crate) struct TrimResult { } /// A staged update paired with its final trimmed neighbours. -pub(crate) type FinalisedUpdate = (StagedUpdate, Vec); +pub(crate) type FinalizedUpdate = (StagedUpdate, Vec); /// Outcome of staging the insertion layers prior to trimming. pub(super) struct LayerProcessingOutcome { @@ -88,8 +88,8 @@ pub(super) struct LayerProcessingOutcome { pub(super) new_node_neighbours: Vec>, /// Staged neighbour lists keyed by origin and layer. pub(super) staged: HashMap<(usize, usize), Vec>, - /// Node-layer pairs initialised during staging. - pub(super) initialised: HashSet<(usize, usize)>, + /// Node-layer pairs initialized during staging. + pub(super) initialized: HashSet<(usize, usize)>, /// Node-layer pairs that exceed their degree limit. pub(super) needs_trim: HashSet<(usize, usize)>, } diff --git a/chutoro-core/src/hnsw/search/mod.rs b/chutoro-core/src/hnsw/search/mod.rs index c1d6b602..df8dafde 100644 --- a/chutoro-core/src/hnsw/search/mod.rs +++ b/chutoro-core/src/hnsw/search/mod.rs @@ -186,7 +186,7 @@ impl<'graph> LayerSearcher<'graph> { } let distances = inputs.validate_batch(ctx.query(), &fresh)?; - for (neighbour_id, distance) in fresh.into_iter().zip(distances.into_iter()) { + for (neighbour_id, distance) in fresh.into_iter().zip(distances) { let sequence = self.sequence_for_node(neighbour_id, "layer expansion")?; state.try_enqueue( SearchNeighbour::new(neighbour_id, distance, sequence), @@ -194,6 +194,6 @@ impl<'graph> LayerSearcher<'graph> { ); } } - Ok(state.finalise()) + Ok(state.finalize()) } } diff --git a/chutoro-core/src/hnsw/search/state.rs b/chutoro-core/src/hnsw/search/state.rs index 3b461739..e8cb9fb3 100644 --- a/chutoro-core/src/hnsw/search/state.rs +++ b/chutoro-core/src/hnsw/search/state.rs @@ -21,13 +21,13 @@ pub(super) struct SearchState { } impl SearchState { - /// Initialise queues using the compatibility fallback capacity. + /// Initialize queues using the compatibility fallback capacity. pub(super) fn new(entry: SearchNeighbour) -> Self { // Fallback when `ef` is not available at the call-site. Self::with_capacity(entry, 64) } - /// Initialise queues sized for the requested search width. + /// Initialize queues sized for the requested search width. pub(super) fn with_capacity(entry: SearchNeighbour, ef: usize) -> Self { let queue_capacity = ef.max(1); let set_capacity = queue_capacity.saturating_mul(4); @@ -100,7 +100,7 @@ impl SearchState { } /// Convert retained queue entries into ascending public neighbours. - pub(super) fn finalise(self) -> Vec { + pub(super) fn finalize(self) -> Vec { let mut neighbours: Vec<_> = self.best.into_vec(); neighbours.sort_unstable(); neighbours diff --git a/chutoro-core/src/hnsw/tests/build.rs b/chutoro-core/src/hnsw/tests/build.rs index ade23070..6a54f2eb 100644 --- a/chutoro-core/src/hnsw/tests/build.rs +++ b/chutoro-core/src/hnsw/tests/build.rs @@ -277,23 +277,25 @@ fn duplicate_insert_is_rejected() { } #[rstest] -fn cpu_hnsw_initialises_graph_with_params() { +fn cpu_hnsw_initializes_graph_with_params() { let params = HnswParams::new(2, 4) .expect("parameters must be valid") .with_rng_seed(7); - let index = CpuHnsw::with_capacity(params.clone(), 8).expect("index must initialise"); - index.inspect_graph(|graph| { - let graph_params = graph.params(); - assert_eq!(graph_params.max_connections(), params.max_connections()); - assert_eq!(graph_params.ef_construction(), params.ef_construction()); - assert_eq!(graph_params.rng_seed(), params.rng_seed()); - }); + let index = CpuHnsw::with_capacity(params.clone(), 8).expect("index must initialize"); + index + .inspect_graph(|graph| { + let graph_params = graph.params(); + assert_eq!(graph_params.max_connections(), params.max_connections()); + assert_eq!(graph_params.ef_construction(), params.ef_construction()); + assert_eq!(graph_params.rng_seed(), params.rng_seed()); + }) + .expect("graph inspection must succeed"); } #[rstest] fn trimming_prefers_lower_id_on_distance_ties() { let params = HnswParams::new(1, 4).expect("parameters must be valid"); - let index = CpuHnsw::with_capacity(params.clone(), 3).expect("index must initialise"); + let index = CpuHnsw::with_capacity(params.clone(), 3).expect("index must initialize"); let ctx = EdgeContext { level: 0, max_connections: params.max_connections(), @@ -327,7 +329,7 @@ fn trimming_prefers_lower_id_on_distance_ties() { #[rstest] fn score_trim_jobs_limits_results_to_max_connections() { let params = HnswParams::new(1, 4).expect("parameters must be valid"); - let index = CpuHnsw::with_capacity(params.clone(), 6).expect("index must initialise"); + let index = CpuHnsw::with_capacity(params.clone(), 6).expect("index must initialize"); let ctx = EdgeContext { level: 0, max_connections: 2, diff --git a/chutoro-core/src/hnsw/tests/edge_harvest/coverage.rs b/chutoro-core/src/hnsw/tests/edge_harvest/coverage.rs index 1cc2d6ce..97ebe36c 100644 --- a/chutoro-core/src/hnsw/tests/edge_harvest/coverage.rs +++ b/chutoro-core/src/hnsw/tests/edge_harvest/coverage.rs @@ -84,7 +84,7 @@ fn build_with_edges_edges_sorted_by_sequence() { } #[rstest] -#[case::already_canonical(CanonicaliseCase { +#[case::already_canonical(CanonicalizeCase { source: 1, target: 3, distance: 0.3, @@ -92,7 +92,7 @@ fn build_with_edges_edges_sorted_by_sequence() { expected_source: 1, expected_target: 3, })] -#[case::reversed(CanonicaliseCase { +#[case::reversed(CanonicalizeCase { source: 5, target: 2, distance: 0.5, @@ -100,7 +100,7 @@ fn build_with_edges_edges_sorted_by_sequence() { expected_source: 2, expected_target: 5, })] -#[case::self_edge(CanonicaliseCase { +#[case::self_edge(CanonicalizeCase { source: 4, target: 4, distance: 0.7, @@ -108,9 +108,9 @@ fn build_with_edges_edges_sorted_by_sequence() { expected_source: 4, expected_target: 4, })] -fn canonicalise_preserves_fields(#[case] case: CanonicaliseCase) { +fn canonicalize_preserves_fields(#[case] case: CanonicalizeCase) { let edge = CandidateEdge::new(case.source, case.target, case.distance, case.sequence); - let canonical = edge.canonicalise(); + let canonical = edge.canonicalize(); assert_eq!(canonical.source(), case.expected_source); assert_eq!(canonical.target(), case.expected_target); @@ -118,6 +118,13 @@ fn canonicalise_preserves_fields(#[case] case: CanonicaliseCase) { assert_eq!(canonical.sequence(), case.sequence); } +#[rstest] +fn legacy_alias_matches_preferred_method() { + let edge = CandidateEdge::new(5, 2, 0.5, 10); + + assert_eq!(edge.canonicalise(), edge.canonicalize()); +} + #[rstest] fn candidate_edge_ordering_is_deterministic() { let e1 = CandidateEdge::new(0, 1, 0.5, 10); diff --git a/chutoro-core/src/hnsw/tests/edge_harvest/mod.rs b/chutoro-core/src/hnsw/tests/edge_harvest/mod.rs index 5a826a87..2f91700c 100644 --- a/chutoro-core/src/hnsw/tests/edge_harvest/mod.rs +++ b/chutoro-core/src/hnsw/tests/edge_harvest/mod.rs @@ -168,7 +168,7 @@ fn comparison_cpu_hnsw( } #[derive(Debug)] -struct CanonicaliseCase { +struct CanonicalizeCase { source: usize, target: usize, distance: f32, diff --git a/chutoro-core/src/hnsw/tests/property/edge_harvest_output/harvest.rs b/chutoro-core/src/hnsw/tests/property/edge_harvest_output/harvest.rs index d167cfe1..e80ea70b 100644 --- a/chutoro-core/src/hnsw/tests/property/edge_harvest_output/harvest.rs +++ b/chutoro-core/src/hnsw/tests/property/edge_harvest_output/harvest.rs @@ -33,7 +33,7 @@ fn canonical_pair(left: usize, right: usize) -> (usize, usize) { fn build_edge_lookup(edges: &[CandidateEdge]) -> BTreeMap<(usize, usize), CandidateEdge> { let mut lookup = BTreeMap::new(); for edge in edges { - let canonical = edge.canonicalise(); + let canonical = edge.canonicalize(); lookup .entry((canonical.source(), canonical.target())) .or_insert(canonical); diff --git a/chutoro-core/src/hnsw/tests/property/idempotency_property.rs b/chutoro-core/src/hnsw/tests/property/idempotency_property.rs index 9e1b0c61..81372bf5 100644 --- a/chutoro-core/src/hnsw/tests/property/idempotency_property.rs +++ b/chutoro-core/src/hnsw/tests/property/idempotency_property.rs @@ -50,7 +50,8 @@ pub(super) fn run_idempotency_property( .map_err(|err| TestCaseError::fail(format!("index build failed: {err}")))?; // Snapshot the graph state before duplicate attempts - let snapshot = snapshot_graph(&index); + let snapshot = snapshot_graph(&index) + .map_err(|err| TestCaseError::fail(format!("initial graph snapshot failed: {err}")))?; // Attempt duplicate insertions let duplicate_indices = duplicate_indices_for_job(plan, len, is_coverage_job); @@ -79,7 +80,9 @@ pub(super) fn run_idempotency_property( } // Verify graph state is unchanged - if !graph_matches_snapshot(&index, &snapshot) { + if !graph_matches_snapshot(&index, &snapshot) + .map_err(|err| TestCaseError::fail(format!("final graph snapshot failed: {err}")))? + { return Err(TestCaseError::fail( "graph state changed after duplicate insertion attempts", )); @@ -150,7 +153,7 @@ struct GraphSnapshot { } /// Captures a snapshot of the graph's structural state. -fn snapshot_graph(index: &CpuHnsw) -> GraphSnapshot { +fn snapshot_graph(index: &CpuHnsw) -> Result { index.inspect_graph(|graph| { let entry = graph.entry(); let nodes = (0..graph.capacity()) @@ -173,9 +176,9 @@ fn snapshot_node(node: &crate::hnsw::node::Node) -> NodeSnapshot { } /// Checks whether the current graph state matches a snapshot. -fn graph_matches_snapshot(index: &CpuHnsw, snapshot: &GraphSnapshot) -> bool { - let current = snapshot_graph(index); - current == *snapshot +fn graph_matches_snapshot(index: &CpuHnsw, snapshot: &GraphSnapshot) -> Result { + let current = snapshot_graph(index)?; + Ok(current == *snapshot) } #[cfg(test)] @@ -302,7 +305,7 @@ mod tests { let source = fixture.into_source().expect("source"); let index = CpuHnsw::build(&source, params).expect("index"); - let snapshot = snapshot_graph(&index); + let snapshot = snapshot_graph(&index).expect("graph snapshot must succeed"); // Verify snapshot captures entry point assert!( @@ -316,7 +319,7 @@ mod tests { // Verify snapshot matches itself assert!( - graph_matches_snapshot(&index, &snapshot), + graph_matches_snapshot(&index, &snapshot).expect("graph comparison must succeed"), "graph must match its own snapshot" ); } diff --git a/chutoro-core/src/hnsw/tests/property/mutation_property/mod.rs b/chutoro-core/src/hnsw/tests/property/mutation_property/mod.rs index 42b547ea..8aed5cf8 100644 --- a/chutoro-core/src/hnsw/tests/property/mutation_property/mod.rs +++ b/chutoro-core/src/hnsw/tests/property/mutation_property/mod.rs @@ -214,7 +214,9 @@ impl MutationRunner<'_> { params: &HnswParamsSeed, ) -> Result { let next = next_reconfigure_params(self.active_params, params)?; - self.index.reconfigure_for_test(next.clone()); + self.index + .reconfigure_for_test(next.clone()) + .map_err(|err| TestCaseError::fail(format!("reconfigure failed: {err}")))?; let summary = format!( "reconfigure -> M={} ef={}", next.max_connections(), diff --git a/chutoro-core/src/hnsw/tests/property/tests.rs b/chutoro-core/src/hnsw/tests/property/tests.rs index 6537a727..46514588 100644 --- a/chutoro-core/src/hnsw/tests/property/tests.rs +++ b/chutoro-core/src/hnsw/tests/property/tests.rs @@ -242,15 +242,17 @@ fn bootstrap_uniform_fixture_remains_reachable() { .expect("bootstrap insertion must succeed"); } - index.inspect_graph(|graph| { - for node in 0..initial_population { - let node_ref = graph.node(node).expect("seeded node should exist"); - assert!( - !node_ref.neighbours(0).is_empty(), - "seeded node {node} should expose base neighbours", - ); - } - }); + index + .inspect_graph(|graph| { + for node in 0..initial_population { + let node_ref = graph.node(node).expect("seeded node should exist"); + assert!( + !node_ref.neighbours(0).is_empty(), + "seeded node {node} should expose base neighbours", + ); + } + }) + .expect("graph inspection must succeed"); index .invariants() diff --git a/chutoro-core/src/hnsw/tests/search.rs b/chutoro-core/src/hnsw/tests/search.rs index 58694e61..32409d94 100644 --- a/chutoro-core/src/hnsw/tests/search.rs +++ b/chutoro-core/src/hnsw/tests/search.rs @@ -225,7 +225,9 @@ fn search_respects_minimum_ef() { ) .expect("search must succeed"); assert_eq!(neighbours.len(), 1); - let entry = index.inspect_graph(|graph| graph.entry().expect("entry exists").node); + let entry = index + .inspect_graph(|graph| graph.entry().expect("entry exists").node) + .expect("graph inspection must succeed"); let entry_distance = source .distance(0, entry) .expect("entry distance must be valid"); diff --git a/chutoro-core/src/hnsw/types.rs b/chutoro-core/src/hnsw/types.rs index 953d9c10..979fcbcd 100644 --- a/chutoro-core/src/hnsw/types.rs +++ b/chutoro-core/src/hnsw/types.rs @@ -149,9 +149,9 @@ impl PartialOrd for RankedNeighbour { /// assert_eq!(edge.target(), 1); /// assert!((edge.distance() - 0.5).abs() < f32::EPSILON); /// -/// // Canonicalise ensures source <= target for undirected graphs. +/// // Canonicalize ensures source <= target for undirected graphs. /// let reversed = CandidateEdge::new(5, 2, 0.3, 10); -/// let canonical = reversed.canonicalise(); +/// let canonical = reversed.canonicalize(); /// assert_eq!(canonical.source(), 2); /// assert_eq!(canonical.target(), 5); /// ``` @@ -204,7 +204,7 @@ impl CandidateEdge { /// Useful for undirected MST construction where edge direction is /// irrelevant. #[must_use] - pub const fn canonicalise(self) -> Self { + pub const fn canonicalize(self) -> Self { if self.source <= self.target { self } else { @@ -215,6 +215,23 @@ impl CandidateEdge { } } } + + /// Returns the edge with ordered endpoints under the legacy API name. + /// + /// The name is retained for callers that used `canonicalise` before the + /// preferred [`Self::canonicalize`] method was added. + /// + /// # Examples + /// ``` + /// use chutoro_core::CandidateEdge; + /// + /// let edge = CandidateEdge::new(5, 2, 0.3, 10); + /// assert_eq!(edge.canonicalise(), edge.canonicalize()); + /// ``` + #[must_use] + pub const fn canonicalise(self) -> Self { + self.canonicalize() + } } impl Eq for CandidateEdge {} diff --git a/chutoro-core/src/hnsw/validate.rs b/chutoro-core/src/hnsw/validate.rs index 4d41f53c..3422c6ea 100644 --- a/chutoro-core/src/hnsw/validate.rs +++ b/chutoro-core/src/hnsw/validate.rs @@ -168,7 +168,7 @@ impl<'a, D: DataSource + Sync> CacheBatch<'a, D> { }); } - for ((index, miss), computed_value) in pending.into_iter().zip(computed.into_iter()) { + for ((index, miss), computed_value) in pending.into_iter().zip(computed) { let cached_value = self.cache.complete_miss(miss, computed_value)?; let result = results @@ -202,7 +202,7 @@ fn ensure_all_resolved( } let mut resolved = Vec::with_capacity(results.len()); - for (candidate, result_slot) in candidates.iter().zip(results.into_iter()) { + for (candidate, result_slot) in candidates.iter().zip(results) { match result_slot { Some(distance) => resolved.push(distance), None => { diff --git a/chutoro-core/src/mst/kani_model.rs b/chutoro-core/src/mst/kani_model.rs index 603d9b78..d75cdd18 100644 --- a/chutoro-core/src/mst/kani_model.rs +++ b/chutoro-core/src/mst/kani_model.rs @@ -13,7 +13,7 @@ use crate::CandidateEdge; use super::MinimumSpanningForest; use super::{MstEdge, MstError, validate_and_canonicalize_edge}; -/// Placeholder edge used to initialise the bounded forest buffer. +/// Placeholder edge used to initialize the bounded forest buffer. const EMPTY_MST_EDGE: MstEdge = MstEdge { source: 0, target: 0, diff --git a/chutoro-core/src/mst/mod.rs b/chutoro-core/src/mst/mod.rs index b6209df9..90f1eb11 100644 --- a/chutoro-core/src/mst/mod.rs +++ b/chutoro-core/src/mst/mod.rs @@ -301,7 +301,7 @@ fn is_mst_complete( } #[cfg(not(kani))] -/// Validate, canonicalise, sort, and deduplicate candidate edges. +/// Validate, canonicalize, sort, and deduplicate candidate edges. fn prepare_edge_list<'a>( edges: impl IntoIterator, node_count: usize, diff --git a/chutoro-core/src/mst/property/tests.rs b/chutoro-core/src/mst/property/tests.rs index f3fbc0ba..98cb3e84 100644 --- a/chutoro-core/src/mst/property/tests.rs +++ b/chutoro-core/src/mst/property/tests.rs @@ -46,7 +46,7 @@ const TEST_CASES: &[(WeightDistribution, u64, &str)] = &[ /// - `$runner` — property runner function with signature /// `fn(&MstFixture) -> TestCaseResult`. /// - `$expectation` — panic message passed to `.expect()`. -macro_rules! parameterised_property_test { +macro_rules! parameterized_property_test { ($test_name:ident, $runner:path, $expectation:expr) => { #[rstest::rstest] #[case::unique_42(WeightDistribution::Unique, 42)] @@ -97,28 +97,28 @@ proptest! { } // ======================================================================== -// rstest Parameterised Cases +// rstest Parameterized Cases // ======================================================================== -parameterised_property_test!( +parameterized_property_test!( oracle_equivalence_rstest, run_oracle_equivalence_property, "oracle equivalence must hold" ); -parameterised_property_test!( +parameterized_property_test!( structural_invariants_rstest, run_structural_invariants_property, "structural invariants must hold" ); -parameterised_property_test!( +parameterized_property_test!( concurrency_safety_rstest, run_concurrency_safety_property, "concurrency safety must hold" ); -parameterised_property_test!( +parameterized_property_test!( thread_pool_determinism_rstest, run_thread_pool_determinism_property, "thread-pool determinism must hold" @@ -138,7 +138,7 @@ fn test_cases_count_matches_macro_expectations() { assert_eq!( TEST_CASES.len(), 11, - "TEST_CASES length changed — update parameterised_property_test! macro" + "TEST_CASES length changed — update parameterized_property_test! macro" ); } diff --git a/chutoro-core/src/mst/tests/mod.rs b/chutoro-core/src/mst/tests/mod.rs index 5920e98d..0f02c996 100644 --- a/chutoro-core/src/mst/tests/mod.rs +++ b/chutoro-core/src/mst/tests/mod.rs @@ -19,10 +19,7 @@ fn harvest(edges: &[(usize, usize, f32, u64)]) -> EdgeHarvest { fn union_find_root(parent: &mut [usize], node: usize) -> usize { let mut current = node; - loop { - let Some(¤t_parent) = parent.get(current) else { - break; - }; + while let Some(¤t_parent) = parent.get(current) { if current_parent == current { break; } diff --git a/chutoro-core/tests/nextest_config.rs b/chutoro-core/tests/nextest_config.rs index 63b170e4..a671f039 100644 --- a/chutoro-core/tests/nextest_config.rs +++ b/chutoro-core/tests/nextest_config.rs @@ -1,12 +1,14 @@ //! Regression checks for the repository's nextest profile configuration. +use std::path::Path; +use std::process::Command; + use rstest::rstest; const NEXTEST_CONFIG: &str = include_str!("../../.config/nextest.toml"); const PROPERTY_TESTS_WORKFLOW: &str = include_str!("../../.github/workflows/property-tests.yml"); const BENCHMARK_REGRESSIONS_WORKFLOW: &str = include_str!("../../.github/workflows/benchmark-regressions.yml"); -const MAKEFILE: &str = include_str!("../../Makefile"); const BENCH_SLOW_TIMEOUT: &str = "slow-timeout = { period = \"600s\", terminate-after = 1, grace-period = \"5s\" }"; @@ -82,15 +84,6 @@ fn benchmark_workflow_job_block(job: &str) -> Result<&'static str, String> { ) } -fn make_target_block(target: &str) -> Result<&'static str, String> { - extract_block( - MAKEFILE, - &format!("\n{target}:"), - "\n\n", - &format!("Makefile target '{target}'"), - ) -} - #[test] fn nextest_default_profile_keeps_global_timeout_guard() { assert!(NEXTEST_CONFIG.contains("global-timeout = \"40m\"")); @@ -152,9 +145,9 @@ fn nextest_profiles_keep_trybuild_timeout_guards(#[case] profile_name: &str) { profile_name, [ "arrow_parquet_types_share_one_family", - "portable_simd_gating_compile_checks", + "portable_simd_without_feature_is_rejected", + "portable_simd_with_feature_compiles", "session_api_compiles_when_cpu_feature_is_enabled", - "arrow_parquet_types_share_one_family", "threads-required = 4", TRYBUILD_SLOW_TIMEOUT, ] @@ -206,11 +199,45 @@ fn profiles_preserve_write_lock_proptest_timeout(#[case] profile_name: &str) { #[test] fn makefile_exposes_typecheck_gate() { - let typecheck_block = make_target_block("typecheck").expect("typecheck target must exist"); - assert!(MAKEFILE.contains(" typecheck ")); - assert!(typecheck_block.contains("cargo") || typecheck_block.contains("$(CARGO)")); - assert!(typecheck_block.contains("check --workspace --all-targets --all-features")); - assert!(typecheck_block.contains("$(BUILD_JOBS)")); + let workspace_root = Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .expect("chutoro-core must be a workspace member"); + let make_output = Command::new("make") + .args([ + "--dry-run", + "typecheck", + "CARGO=probe-cargo", + "BUILD_JOBS=--jobs=3", + ]) + .current_dir(workspace_root) + .output() + .expect("Make dry-run must start"); + + assert!( + make_output.status.success(), + "Make dry-run should succeed: {}", + String::from_utf8_lossy(&make_output.stderr) + ); + let stdout = String::from_utf8(make_output.stdout).expect("Make output must be UTF-8"); + let cargo_commands = stdout + .lines() + .filter(|line| line.trim_start().starts_with("probe-cargo ")) + .collect::>(); + + assert_eq!( + cargo_commands.len(), + 1, + "typecheck must evaluate to exactly one injected Cargo command: {stdout}" + ); + let typecheck_command = cargo_commands + .first() + .expect("exactly one Cargo command must be present"); + let pinned_toolchain = include_str!("../../tools/dev-fast/TOOLCHAIN").trim(); + + assert!(typecheck_command.contains(&format!("+{pinned_toolchain}"))); + assert!(typecheck_command.contains("--config tools/dev-fast/config.toml")); + assert!(typecheck_command.contains(" check --workspace --all-targets --all-features ")); + assert!(typecheck_command.ends_with("--jobs=3")); } #[test] diff --git a/chutoro-core/tests/result_api_surface.rs b/chutoro-core/tests/result_api_surface.rs index 9c15b6b5..52c5d0da 100644 --- a/chutoro-core/tests/result_api_surface.rs +++ b/chutoro-core/tests/result_api_surface.rs @@ -27,7 +27,8 @@ fn clustering_result_api_is_checked_without_cpu() { .join("target") .join("result-api-surface-no-cpu"); - let output = Command::new(env!("CARGO")) + // Keep the nested trybuild check on the repository-pinned stable toolchain. + let output = Command::new("cargo") .current_dir(workspace_root) .args([ "test", @@ -41,6 +42,7 @@ fn clustering_result_api_is_checked_without_cpu() { "clustering_result_panicking_constructor_is_unavailable_without_cpu", ]) .env("CARGO_TARGET_DIR", target_dir) + .env_remove("RUSTUP_TOOLCHAIN") .env("RUSTFLAGS", "-D warnings") .output() .expect("cpu-disabled result API check should run"); diff --git a/chutoro-providers/dense/src/simd/kernels/mod.rs b/chutoro-providers/dense/src/simd/kernels/mod.rs index 3c5f8fea..cac7c64c 100644 --- a/chutoro-providers/dense/src/simd/kernels/mod.rs +++ b/chutoro-providers/dense/src/simd/kernels/mod.rs @@ -201,6 +201,8 @@ pub(super) fn euclidean_distance_neon_entry(left: &[f32], right: &[f32]) -> f32 unsafe { neon_simd::euclidean_distance_neon(left, right) } } +/// Invoke the nightly portable-SIMD pairwise kernel when compiled in. +/// Identical rows, for example, return a zero distance. #[cfg(all(feature = "nightly_portable_simd", nightly))] pub(super) fn euclidean_distance_portable_simd_entry(left: &[f32], right: &[f32]) -> f32 { portable_simd::euclidean_distance_portable_simd_entry(left, right) @@ -253,6 +255,8 @@ pub(super) fn euclidean_distance_query_points_scalar( } } +/// Invoke the nightly portable-SIMD packed query kernel when compiled in. +/// A query matching one packed point, for example, writes zero for that point. #[cfg(all(feature = "nightly_portable_simd", nightly))] pub(super) fn euclidean_distance_query_points_portable_simd_entry( query: &[f32], diff --git a/chutoro-providers/dense/src/simd/kernels/portable_simd.rs b/chutoro-providers/dense/src/simd/kernels/portable_simd.rs index b7e9ac25..020dec68 100644 --- a/chutoro-providers/dense/src/simd/kernels/portable_simd.rs +++ b/chutoro-providers/dense/src/simd/kernels/portable_simd.rs @@ -2,12 +2,15 @@ use std::simd::Simd; -use super::super::lane_output_count; use super::{DensePointView, finalize_distance, squared_l2_tail}; +/// Match the packed point view's 16-value alignment and padding unit. const PORTABLE_SIMD_LANES: usize = 16; +/// Hold one complete packed point-view block; for example, 16 distances. type PortableF32x16 = Simd; +/// Check matching row dimensions before entering the portable kernel. +/// Identical rows, for example, produce a zero distance. pub(super) fn euclidean_distance_portable_simd_entry(left: &[f32], right: &[f32]) -> f32 { assert_eq!( left.len(), @@ -17,10 +20,14 @@ pub(super) fn euclidean_distance_portable_simd_entry(left: &[f32], right: &[f32] euclidean_distance_portable_simd(left, right) } +/// Compute the Euclidean distance of two rows using portable SIMD. +/// Two identical rows, for example, have a distance of zero. pub(super) fn euclidean_distance_portable_simd(left: &[f32], right: &[f32]) -> f32 { finalize_distance(squared_l2_portable_simd(left, right).sqrt()) } +/// Compute distances from a query to packed points in 16-lane blocks. +/// A packed point equal to the query, for example, yields zero in `out`. pub(super) fn euclidean_distance_query_points_portable_simd_entry( query: &[f32], points: &DensePointView<'_>, @@ -29,32 +36,46 @@ pub(super) fn euclidean_distance_query_points_portable_simd_entry( debug_assert_eq!(query.len(), points.dimension().get()); debug_assert_eq!(out.len(), points.point_count()); - let padded_count = points.padded_point_count(); - for offset in (0..padded_count).step_by(PORTABLE_SIMD_LANES) { + for (chunk_index, outputs) in out.chunks_mut(PORTABLE_SIMD_LANES).enumerate() { + let offset = chunk_index * PORTABLE_SIMD_LANES; let mut acc = PortableF32x16::splat(0.0); for (dimension_index, query_value) in query.iter().copied().enumerate() { let query_lane = PortableF32x16::splat(query_value); let values = points.coordinate_block(dimension_index); - let point_lane = - PortableF32x16::from_slice(&values[offset..offset + PORTABLE_SIMD_LANES]); + let Some(values_lane) = values.get(offset..offset + PORTABLE_SIMD_LANES) else { + debug_assert!( + false, + "packed point block must contain every padded SIMD lane" + ); + return; + }; + let point_lane = PortableF32x16::from_slice(values_lane); let delta = query_lane - point_lane; acc += delta * delta; } - let remaining = lane_output_count(out.len(), offset, PORTABLE_SIMD_LANES); - for (lane_index, value) in acc.to_array().into_iter().take(remaining).enumerate() { - out[offset + lane_index] = finalize_distance(value.sqrt()); + for (output, value) in outputs.iter_mut().zip(acc.to_array()) { + *output = finalize_distance(value.sqrt()); } } } +/// Sum squared differences for complete SIMD blocks and their scalar tail. +/// For a 17-value row, for example, the final value uses the scalar tail. +#[expect( + clippy::float_arithmetic, + reason = "squared-L2 accumulation combines genuine floating-point SIMD and scalar sums" +)] fn squared_l2_portable_simd(left: &[f32], right: &[f32]) -> f32 { let mut index = 0_usize; let mut acc = PortableF32x16::splat(0.0); - while index + PORTABLE_SIMD_LANES <= left.len() { - let left_lane = PortableF32x16::from_slice(&left[index..index + PORTABLE_SIMD_LANES]); - let right_lane = PortableF32x16::from_slice(&right[index..index + PORTABLE_SIMD_LANES]); + for (left_chunk, right_chunk) in left + .chunks_exact(PORTABLE_SIMD_LANES) + .zip(right.chunks_exact(PORTABLE_SIMD_LANES)) + { + let left_lane = PortableF32x16::from_slice(left_chunk); + let right_lane = PortableF32x16::from_slice(right_chunk); let delta = left_lane - right_lane; acc += delta * delta; index += PORTABLE_SIMD_LANES; diff --git a/chutoro-providers/dense/src/simd/semantics.rs b/chutoro-providers/dense/src/simd/semantics.rs index 8bb58b38..80989ad6 100644 --- a/chutoro-providers/dense/src/simd/semantics.rs +++ b/chutoro-providers/dense/src/simd/semantics.rs @@ -2,7 +2,7 @@ //! //! The semantics contract is a small value object that records the tolerance //! and policy choices every backend must follow: `1.0e-5` epsilon, -//! `CanonicaliseToNan` for non-finite distances, `ReturnZero` for identical +//! `CanonicalizeToNan` for non-finite distances, `ReturnZero` for identical //! zero vectors, and `LowestRowIndexFirst` for future equal-distance ordering. //! Keeping those decisions in one object makes the parity assertions describe //! behaviour rather than re-encoding scattered constants in each property. @@ -37,7 +37,7 @@ use super::{DensePointView, kernels}; #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum NonFinitePolicy { /// Convert every non-finite final distance to `f32::NAN`. - CanonicaliseToNan, + CanonicalizeToNan, } /// Policy for vectors whose Euclidean norm difference is zero. @@ -73,7 +73,7 @@ impl DistanceSemantics { pub(crate) const fn default_euclidean() -> Self { Self { epsilon: 1.0e-5_f32, - non_finite_policy: NonFinitePolicy::CanonicaliseToNan, + non_finite_policy: NonFinitePolicy::CanonicalizeToNan, zero_vector_policy: ZeroVectorPolicy::ReturnZero, tie_breaking: TieBreakingPolicy::LowestRowIndexFirst, } @@ -166,10 +166,10 @@ impl DistanceSemantics { /// Returns whether a non-finite result satisfies the configured policy. /// - /// When the policy is `CanonicaliseToNan`, matching `NaN` values are + /// When the policy is `CanonicalizeToNan`, matching `NaN` values are /// accepted directly so callers can skip the finite-distance epsilon check. fn should_accept_non_finite(self, actual: f32, expected: f32) -> bool { - matches!(self.non_finite_policy, NonFinitePolicy::CanonicaliseToNan) + matches!(self.non_finite_policy, NonFinitePolicy::CanonicalizeToNan) && actual.is_nan() && expected.is_nan() } diff --git a/chutoro-providers/dense/src/simd/tests/backend_expectations.rs b/chutoro-providers/dense/src/simd/tests/backend_expectations.rs index 58f3504f..433dc33d 100644 --- a/chutoro-providers/dense/src/simd/tests/backend_expectations.rs +++ b/chutoro-providers/dense/src/simd/tests/backend_expectations.rs @@ -12,7 +12,7 @@ fn distance_semantics_contract_snapshot() { format!("{:?}", semantics::DistanceSemantics::default_euclidean()), concat!( "DistanceSemantics { epsilon: 1e-5, ", - "non_finite_policy: CanonicaliseToNan, ", + "non_finite_policy: CanonicalizeToNan, ", "zero_vector_policy: ReturnZero, ", "tie_breaking: LowestRowIndexFirst }", ), diff --git a/chutoro-providers/dense/src/simd/tests/parity/non_finite.rs b/chutoro-providers/dense/src/simd/tests/parity/non_finite.rs index 72b3c9e2..552621eb 100644 --- a/chutoro-providers/dense/src/simd/tests/parity/non_finite.rs +++ b/chutoro-providers/dense/src/simd/tests/parity/non_finite.rs @@ -22,7 +22,7 @@ proptest! { #![proptest_config(super::proptest_config(64))] #[test] - fn pairwise_backends_canonicalise_non_finite_to_nan( + fn pairwise_backends_canonicalize_non_finite_to_nan( (left, right) in strategies::non_finite_vector_pair(), ) { let semantics = DistanceSemantics::default_euclidean(); @@ -42,7 +42,7 @@ proptest! { } #[test] - fn query_point_backends_canonicalise_non_finite_to_nan( + fn query_point_backends_canonicalize_non_finite_to_nan( fixture in strategies::non_finite_query_points_fixture(), ) { let semantics = DistanceSemantics::default_euclidean(); diff --git a/chutoro-providers/dense/src/tests/provider.rs b/chutoro-providers/dense/src/tests/provider.rs index e76b49de..59cc0a3d 100644 --- a/chutoro-providers/dense/src/tests/provider.rs +++ b/chutoro-providers/dense/src/tests/provider.rs @@ -57,17 +57,17 @@ fn matrix_provider_distance_batch() { 0, 1, )] -#[case::non_finite_canonicalises_to_nan( +#[case::non_finite_canonicalizes_to_nan( vec![vec![1.0, f32::NAN, 3.0], vec![1.0, 2.0, 3.0]], 0, 1, )] -#[case::positive_infinity_canonicalises_to_nan( +#[case::positive_infinity_canonicalizes_to_nan( vec![vec![1.0, f32::INFINITY, 3.0], vec![1.0, 2.0, 3.0]], 0, 1, )] -#[case::negative_infinity_canonicalises_to_nan( +#[case::negative_infinity_canonicalizes_to_nan( vec![vec![1.0, f32::NEG_INFINITY, 3.0], vec![1.0, 2.0, 3.0]], 0, 1, diff --git a/chutoro-providers/dense/tests/portable_simd_gating.rs b/chutoro-providers/dense/tests/portable_simd_gating.rs index 3a4fe42a..851f2428 100644 --- a/chutoro-providers/dense/tests/portable_simd_gating.rs +++ b/chutoro-providers/dense/tests/portable_simd_gating.rs @@ -4,11 +4,16 @@ //! at compile time for the portable-SIMD backend. #[test] -fn portable_simd_gating_compile_checks() { +fn portable_simd_without_feature_is_rejected() { let t = trybuild::TestCases::new(); // When nightly_portable_simd feature is absent, portable-SIMD API should fail to compile t.compile_fail("tests/trybuild/portable_simd_without_feature.rs"); +} + +#[test] +fn portable_simd_with_feature_compiles() { + let t = trybuild::TestCases::new(); // When nightly_portable_simd feature is present, portable-SIMD API should compile t.pass("tests/trybuild/portable_simd_with_feature.rs"); diff --git a/chutoro-test-support/src/bin/benchmark_regression_gate.rs b/chutoro-test-support/src/bin/benchmark_regression_gate.rs index 26732e09..c04451d4 100644 --- a/chutoro-test-support/src/bin/benchmark_regression_gate.rs +++ b/chutoro-test-support/src/bin/benchmark_regression_gate.rs @@ -37,7 +37,7 @@ fn main() -> Result<(), Box> { Ok(()) } -/// Initialise non-failing stderr tracing for the gate process. +/// Initialize non-failing stderr tracing for the gate process. fn init_tracing() { let _subscriber_init_result = tracing_subscriber::fmt() .with_target(false) diff --git a/docs/chutoro-design.md b/docs/chutoro-design.md index c937a394..b56ee62c 100644 --- a/docs/chutoro-design.md +++ b/docs/chutoro-design.md @@ -1333,7 +1333,7 @@ manifests change. The nightly `kani-full` sweep is unchanged. _Implementation update (2026-02-02)._ Verus proofs now cover the edge harvest primitives described in `docs/property-testing-design.md` Appendix A. The proofs live in `verus/edge_harvest_proofs.rs` and model -`extract_candidate_edges`, `CandidateEdge::canonicalise`, and +`extract_candidate_edges`, `CandidateEdge::canonicalize`, and `EdgeHarvest::from_unsorted` with spec-only data types mirroring the helper signatures. Distances are represented as integers because the proofs only depend on equality and ordering, not floating-point semantics. Sorting uses diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 555e9ed8..01a4f2a3 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -4,6 +4,65 @@ This guide collects day-to-day practices for contributors working on the Chutoro codebase. It complements the more specialized documents in `docs/` and keeps operational guidance in one place. +## Rust toolchain and build routing + +The workspace pins stable Rust 1.93.1 in `rust-toolchain.toml` and declares +Rust 1.89 as its minimum supported Rust version (MSRV). Direct `cargo` commands +use that stable toolchain and `.cargo/config.toml`, which supplies the +documentation-warning flags; stable Rust uses LLVM for code generation. Cargo +selects the fragment's target rustflags in place of the root settings, so the +fragment repeats both documentation-denial flags. + +The standard debug Make targets explicitly select the fragment in +`tools/dev-fast/config.toml`, pinned to the dated nightly in +`tools/dev-fast/TOOLCHAIN`. It enables Cranelift for development builds. On +Linux it selects GCC as the linker driver and `mold` 2.41.0 as the linker. The +fragment is outside Cargo's automatic configuration paths, so installing the +nightly components alone does not activate it. Run `make install-dev-fast` to +install `rustfmt`, Clippy, `rust-analyzer`, and the Cranelift backend +component; provide GCC on Linux. The target installs the checksum-verified +`mold` 2.41.0 archive on Linux x86_64 and aarch64 hosts. + +The Makefile routes `build`, `test`, and `typecheck` through the fragment, +along with the explicit `dev-build` and `dev-test` targets. `lint-clippy` uses +it for rustdoc and Clippy. `test` uses `cargo-nextest`, which starts a separate +Cargo invocation to compile tests and does not inherit the outer Cargo +configuration. The Makefile passes the fragment before `nextest` and again to +`nextest run` so the inner Cargo invocation uses Cranelift and the selected +Linux linker. `test` also sets `RUSTFLAGS` for warning denial; because +environment `RUSTFLAGS` replaces configured target rustflags, it repeats the +fragment's documentation flags and Linux linker flag. The `CARGO` Make variable +remains injectable for these commands. + +Three byte-exact checks stay on the repository-pinned stable toolchain because +their expected compiler diagnostics depend on stable wording: the core +`result_api_surface` and `session_api_surface` integration suites, and the +dense provider's negative portable-SIMD check. `make test` excludes them from +nextest and runs them with separate stable Cargo commands that retain warning +and documentation denials. The positive dense portable-SIMD coverage remains in +the accelerated nextest run. + +`make test` also excludes the HNSW unit test +`graph_helpers_report_poisoning_without_reconfiguring` from nextest and runs +that exact case as a fourth binding test leaf under pinned stable LLVM, with +warning and documentation denials. The test deliberately panics while holding +the graph's `RwLock` so it can verify poisoned-lock handling. The selected +nightly Cranelift backend aborts on this path instead of unwinding; this +exception is limited to that regression test. The Make-route assertions belong +to `dev_fast_routing_test.py`; keep their helpers private to that contract and +do not reuse them in unrelated workflow contracts. + +Release builds, formatting, coverage, Kani, Verus, benchmarks, and Whitaker do +not select the development fragment. Whitaker uses its own pinned toolchain. The +`build-test` and `coverage-upload` CI jobs provision the development +prerequisites because both run `make lint`; their direct dense-SIMD and +coverage Cargo commands continue to use the repository's stable toolchain. +Linux CI provisions the selected toolchain and `mold`, then runs `lint-clippy` +through the fragment as part of `make lint`. It does not independently verify +which linker the compiled artefacts use; that evidence is currently local. The +fragment leaves the platform linker selected outside Linux, while direct stable +Cargo remains the documented route when the accelerated build is unavailable. + ## GitHub Actions runner profiles Every job on the developer feedback path runs on a paid Ubicloud runner, at @@ -1533,6 +1592,10 @@ entrypoint wiring and place testable profiling, candidate-planning, and argument-selection logic in `chutoro-benches/src/`, with unit tests beside the library modules. This gives those tests Cargo's normal test harness and avoids lint expectations for code that only appears unused in the harness-free build. +The HNSW and `ef_construction` sweep environment-policy modules are a narrow +exception: each is included only by its owning Criterion target and dedicated +integration test target, and remains private to those harnesses. Put helpers +reused across benchmarks in `chutoro-benches/src/`. The library seam is owned by `chutoro-benches`: benchmark binaries may call it, but production crates must not depend on it. Prefer pure helpers without @@ -1567,9 +1630,11 @@ By default, the reports are written to `target/benchmarks/neighbour_scoring_build_profile.csv` and `target/benchmarks/neighbour_scoring_lane_utilisation.csv`. When `CARGO_TARGET_DIR` is set, the same filenames are written below its -`benchmarks/` directory. Treat `hyperfine` as corroboration; cycle-count and -Criterion evidence remain the primary signal for keeping a structural -optimization. +`benchmarks/` directory. The lane report keeps its filename and the +`lane_utilisation_basis_points` CSV column for existing report consumers; Rust +identifiers and ordinary prose use Oxford `utilization` spelling. Treat +`hyperfine` as corroboration; cycle-count and Criterion evidence remain the +primary signal for keeping a structural optimization. ### Benchmark architecture diff --git a/docs/execplans/2-1-7-establish-a-ci-regression-detection-strategy.md b/docs/execplans/2-1-7-establish-a-ci-regression-detection-strategy.md index 23ad1c47..ed5136c0 100644 --- a/docs/execplans/2-1-7-establish-a-ci-regression-detection-strategy.md +++ b/docs/execplans/2-1-7-establish-a-ci-regression-detection-strategy.md @@ -202,7 +202,7 @@ Create `.github/workflows/benchmark-regressions.yml` with: - full scheduled job that runs baseline save + compare for benchmark suites. Use deterministic environment configuration (`CARGO_TERM_COLOR`, fixed seeds, -and existing benchmark env controls) and upload benchmark artifacts/logs. +and existing benchmark env controls) and upload benchmark artefacts/logs. Prefer commands that are already known to work in this repository's bench harness invocation path. @@ -300,7 +300,7 @@ Quality method: - Local gate commands listed in "Concrete steps". - CI workflow YAML validation through GitHub Actions execution. -- Bench logs/artifacts retained for scheduled runs. +- Bench logs/artefacts retained for scheduled runs. ## Idempotence and recovery diff --git a/docs/execplans/2-2-6-property-based-backend-parity-suite.md b/docs/execplans/2-2-6-property-based-backend-parity-suite.md index b1b1c4ab..f30b947a 100644 --- a/docs/execplans/2-2-6-property-based-backend-parity-suite.md +++ b/docs/execplans/2-2-6-property-based-backend-parity-suite.md @@ -540,7 +540,7 @@ tests-only scope; no source changes have been made yet. `tie_breaking: TieBreakingPolicy`. - Provide `DistanceSemantics::default_euclidean()` that returns the contract values used by the existing kernels (epsilon TBD by the - audit in stage C; non-finite policy `CanonicaliseToNan`; + audit in stage C; non-finite policy `CanonicalizeToNan`; zero-vector policy `ReturnZero`; tie-breaking `LowestRowIndexFirst`, even though Euclidean per-pair scoring does not currently break ties by index, the contract documents the choice for parity with future @@ -881,7 +881,7 @@ pub(crate) struct DistanceSemantics { #[derive(Clone, Copy, Debug)] pub(crate) enum NonFinitePolicy { - CanonicaliseToNan, + CanonicalizeToNan, } #[derive(Clone, Copy, Debug)] diff --git a/docs/execplans/2-3-1-hnsw-neighbour-evaluation-using-packed-indices.md b/docs/execplans/2-3-1-hnsw-neighbour-evaluation-using-packed-indices.md index 1b0e5f7f..09e4d658 100644 --- a/docs/execplans/2-3-1-hnsw-neighbour-evaluation-using-packed-indices.md +++ b/docs/execplans/2-3-1-hnsw-neighbour-evaluation-using-packed-indices.md @@ -575,7 +575,7 @@ Stop and escalate (do not work around) when: on disk; `chutoro-benches/src/lib.rs` resolves `pub mod neighbour_scoring` to `chutoro-benches/src/neighbour_scoring/mod.rs`, and `median_matches_sorted_middle_values` already lives beside `median`, - `lane_utilisation_basis_points`, and `duration_basis_points` there. + `lane_utilization_basis_points`, and `duration_basis_points` there. - [x] (2026-06-25) Captured full Criterion baseline, optional HNSW build profile CSV, Hyperfine corroboration, and `perf stat` counters. Criterion baseline `before` was saved for all 21 cases; the lane-utilization report @@ -768,7 +768,7 @@ Stop and escalate (do not work around) when: model, not repeat a narrower `usize` calculation. Evidence: the eleventh CodeRabbit review identified the only remaining `next_multiple_of` use in the report row writer. Impact: padded and wasted lane columns render from the - same checked/u128 basis used by `lane_utilisation_basis_points`. + same checked/u128 basis used by `lane_utilization_basis_points`. - Observation: report helpers should avoid avoidable work on the common path. Evidence: the twelfth CodeRabbit review noted that most benchmark bucket labels need no CSV escaping, but the previous helper still allocated a new @@ -783,7 +783,7 @@ Stop and escalate (do not work around) when: - Observation: CSV report invariants are compact enough to property-test directly. Evidence: the thirteenth CodeRabbit review asked for generated coverage over bucket labels and candidate counts, and the resulting - `lane_utilisation_report_round_trips_generated_rows` case checks CSV parsing, + `lane_utilization_report_round_trips_generated_rows` case checks CSV parsing, padding, wasted lanes, and utilization bounds without exceeding the repository's file-size limit. Impact: the report schema now has example and generated coverage. @@ -1331,7 +1331,7 @@ requiring re-benchmarking per target, and record the exact CPU model and 1. Re-run `neighbour_scoring` + `critcmp before after` + cycle counts + `hyperfine`; append a results table (or a null-result statement) to §6.3 and `Outcomes`. -2. Finalise ADR-003 and `developers-guide`; confirm `users-guide.md` needs +2. Finalize ADR-003 and `developers-guide`; confirm `users-guide.md` needs changes only if a user-visible feature flag was kept. 3. Confirm roadmap 2.3.1 marked done; ensure any deferred work is filed as a new item. diff --git a/docs/execplans/phase-1-hnsw-kani-eviction-deferred-scrub-scenario.md b/docs/execplans/phase-1-hnsw-kani-eviction-deferred-scrub-scenario.md index 7e1e2236..44d27643 100644 --- a/docs/execplans/phase-1-hnsw-kani-eviction-deferred-scrub-scenario.md +++ b/docs/execplans/phase-1-hnsw-kani-eviction-deferred-scrub-scenario.md @@ -70,7 +70,7 @@ completion, the relevant roadmap entry in `docs/roadmap.md` is marked as done. | `chutoro-core/src/hnsw/insert/commit.rs` | `CommitApplicator::apply_neighbour_updates` | | `chutoro-core/src/hnsw/insert/mod.rs` | Kani-only helpers (`apply_commit_updates_for_kani`, etc.) | | `chutoro-core/src/hnsw/insert/commit/tests.rs` | Existing commit-path unit tests | -| `chutoro-core/src/hnsw/insert/types.rs` | `DeferredScrub`, `UpdateContext`, `FinalisedUpdate` types | +| `chutoro-core/src/hnsw/insert/types.rs` | `DeferredScrub`, `UpdateContext`, `FinalizedUpdate` types | | `chutoro-core/src/hnsw/invariants/mod.rs` | `is_bidirectional` invariant checker | ### Existing Harnesses @@ -230,7 +230,7 @@ make markdownlint 2>&1 | tee /tmp/make-markdownlint.log ctx: update_ctx, candidates: vec![1], }; - let updates: Vec = vec![(staged, vec![1])]; + let updates: Vec = vec![(staged, vec![1])]; let new_node = NewNodeContext { id: 3, level: 1 }; apply_commit_updates_for_kani(&mut graph, max_connections, new_node, updates) diff --git a/docs/execplans/phase-1-verus-proofs-for-edge-harvest-primitives.md b/docs/execplans/phase-1-verus-proofs-for-edge-harvest-primitives.md index 230be559..1d2c06e0 100644 --- a/docs/execplans/phase-1-verus-proofs-for-edge-harvest-primitives.md +++ b/docs/execplans/phase-1-verus-proofs-for-edge-harvest-primitives.md @@ -72,7 +72,7 @@ Use the `rstest` patterns described in - `target != source_node` (no self edges). - `sequence == source_sequence` for all edges. - Edge count equals total neighbours across layers minus self neighbours. -2. `CandidateEdge::canonicalise`: +2. `CandidateEdge::canonicalize`: - `source <= target` in the result. - `distance` and `sequence` preserved. 3. `EdgeHarvest::from_unsorted`: @@ -83,7 +83,7 @@ Use the `rstest` patterns described in ### Step 1: Confirm helper stability and constraints -- Review `extract_candidate_edges`, `CandidateEdge::canonicalise`, and +- Review `extract_candidate_edges`, `CandidateEdge::canonicalize`, and `EdgeHarvest::from_unsorted` signatures to confirm they are stable. - Capture any implicit preconditions needed by proofs (for example, sorting comparator totality) and record them in the design document. @@ -108,7 +108,7 @@ Use the `rstest` patterns described in - Prove source, target, and sequence fields are preserved for each edge. - Prove the count formula for total neighbours minus self neighbours. -### Step 5: Prove `CandidateEdge::canonicalise` properties +### Step 5: Prove `CandidateEdge::canonicalize` properties - Prove the canonicalized edge preserves `distance` and `sequence`. - Prove `source <= target` and field swapping is the only change. @@ -130,7 +130,7 @@ Add or extend tests in `chutoro-core/src/hnsw/tests/edge_harvest.rs`: - Happy paths for multiple layers and neighbours. - Unhappy paths for self neighbours and empty layers. - Edge count assertions for mixed layers. -- `CandidateEdge::canonicalise`: +- `CandidateEdge::canonicalize`: - Already canonical edges and reversed edges. - Self edge behaviour. - `EdgeHarvest::from_unsorted`: diff --git a/docs/execplans/replace-3-node-bidirectionality-harness-with-commit-path-harness.md b/docs/execplans/replace-3-node-bidirectionality-harness-with-commit-path-harness.md index 24c9cb9c..b7ae8774 100644 --- a/docs/execplans/replace-3-node-bidirectionality-harness-with-commit-path-harness.md +++ b/docs/execplans/replace-3-node-bidirectionality-harness-with-commit-path-harness.md @@ -90,7 +90,7 @@ Start by designing a Kani-only helper in `chutoro-core/src/hnsw/insert/mod.rs` that wraps `CommitApplicator::apply_neighbour_updates`. This helper should take simple, public inputs (for example, a list of update specs that name the node, level, and neighbour list) and internally build the `StagedUpdate` and -`FinalisedUpdate` values required by the commit path. Add `kani::assume` guards +`FinalizedUpdate` values required by the commit path. Add `kani::assume` guards for preconditions (nodes exist, levels are valid, neighbour lists are deduplicated) to keep the state space bounded and consistent with production assumptions. diff --git a/docs/property-testing-design.md b/docs/property-testing-design.md index 41dbe1b3..ee9c229a 100644 --- a/docs/property-testing-design.md +++ b/docs/property-testing-design.md @@ -591,7 +591,7 @@ Prove that for any `InsertionPlan`: This helper is pure, deterministic, and bounded by the plan input, making it a strong Verus target. -### A2: `CandidateEdge::canonicalise` properties[^canonicalise] +### A2: `CandidateEdge::canonicalize` properties[^1] Location: `chutoro-core/src/hnsw/types.rs` @@ -612,8 +612,8 @@ Prove that: This is feasible if a trusted `sort_unstable_by` specification is accepted or the sort is replaced with a verified routine. -[^canonicalise]: The implementation uses "canonicalise" for API compatibility -even though Oxford spelling prefers the "-ize" suffix for Greek-origin words. +[^1]: The preferred method uses Oxford spelling. The legacy + `CandidateEdge::canonicalise` alias remains available for API compatibility. #### 4.3.1. Property 1: Equivalence with a sequential oracle @@ -976,7 +976,7 @@ jobs: if: failure() run: | # Add notification script here (e.g., curl to Slack webhook) - echo "Weekly deep test failed. See artifacts for failure seed." + echo "Weekly deep test failed. See artefacts for failure seed." ``` diff --git a/docs/repository-layout.md b/docs/repository-layout.md index 69588218..c615c3d6 100644 --- a/docs/repository-layout.md +++ b/docs/repository-layout.md @@ -78,7 +78,9 @@ _Table 3: Documentation and planning responsibilities._ | `.github/` | GitHub workflows, Dependabot configuration, and repository automation. | | `.codescene/` | CodeScene code health rule configuration. | | `scripts/` | Maintainer scripts for proof tooling and operational support. | -| `tools/` | Tool-specific support files that do not belong to a Rust crate. | +| `tools/dev-fast/` | Pinned nightly and explicit Cargo fragment for debug Make targets. | +| `tools/mold/` | Linux `mold` version and checksum pins for the installer. | +| `tools/` | Other tool-specific support files that do not belong to a Rust crate. | | `verus/` | Verus proof sources for formally verified properties. | _Table 4: Tooling and verification path responsibilities._ diff --git a/docs/roadmap.md b/docs/roadmap.md index ee144a0a..ef859136 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -273,7 +273,7 @@ ______________________________________________________________________ - [x] 1.6.4. Add Verus proofs for edge harvest primitives: - `extract_candidate_edges` invariants (source/target/sequence/count). - - `CandidateEdge::canonicalise` preserves order and fields. + - `CandidateEdge::canonicalize` preserves order and fields. - `EdgeHarvest::from_unsorted` permutation + ordering guarantees. - (See property-testing-design Appendix A) - Prerequisites: diff --git a/docs/users-guide.md b/docs/users-guide.md index 821a790a..9b55f5b0 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -248,7 +248,10 @@ Use `insert(node, source)` when only graph mutation is required. Use candidate edges considered during planning. The first insertion into an empty index returns an empty `Vec` because there are no prior nodes to connect to. Subsequent insertions return candidate edges that can be consumed -by incremental minimum spanning tree (MST) or auditing workflows. +by incremental minimum spanning tree (MST) or auditing workflows. Call +`CandidateEdge::canonicalize()` to order an undirected edge's endpoints; the +legacy `CandidateEdge::canonicalise()` name remains available for existing +callers. `insert_harvesting` follows the same insertion rules as `insert` and returns `Result, HnswError>`. `HnswError::DuplicateNode` is returned diff --git a/scripts/install-mold.sh b/scripts/install-mold.sh new file mode 100755 index 00000000..c1ae19ac --- /dev/null +++ b/scripts/install-mold.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash +# +# Install the pinned mold release binary for supported Linux architectures. +# +# CI uses the upstream binary-only archive so installing mold never builds +# it from source. The archive's SHA-256 is pinned in tools/mold/SHA256SUMS; +# unsupported platforms and checksum mismatches are hard errors. + +set -euo pipefail + +ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +# shellcheck source=scripts/lib/pinned-download.sh +source "${ROOT_DIR}/scripts/lib/pinned-download.sh" + +MOLD_VERSION="$(pinned_version "${ROOT_DIR}/tools/mold/VERSION")" +MOLD_OS="$(uname -s)" +[[ "${MOLD_OS}" == Linux ]] || + pinned_die "Unsupported mold operating system: ${MOLD_OS} (Linux required)" + +MOLD_HOST_ARCH="$(uname -m)" +case "${MOLD_HOST_ARCH}" in + x86_64) + MOLD_ARCHIVE_ARCH="x86_64" + ;; + aarch64) + MOLD_ARCHIVE_ARCH="aarch64" + ;; + *) + pinned_die "Unsupported Linux architecture for mold: ${MOLD_HOST_ARCH} (supported: x86_64, aarch64)" + ;; +esac + +INSTALL_PREFIX="${MOLD_INSTALL_PREFIX:-${HOME}/.local}" +BINARY="${INSTALL_PREFIX}/bin/mold" +ARCHIVE="mold-${MOLD_VERSION}-${MOLD_ARCHIVE_ARCH}-linux.tar.gz" +URL="https://github.com/rui314/mold/releases/download/v${MOLD_VERSION}/${ARCHIVE}" + +# A warm image can reuse a complete install. An incomplete or wrong-version +# install falls through and is replaced from the verified release archive. +if [[ -x "${BINARY}" ]] && + "${BINARY}" --version 2>/dev/null | grep -qE "^mold ${MOLD_VERSION//./\\.}([[:space:]]|$)" && + [[ -x "${INSTALL_PREFIX}/bin/ld.mold" ]] && + [[ -f "${INSTALL_PREFIX}/lib/mold/mold-wrapper.so" ]] && + [[ -e "${INSTALL_PREFIX}/libexec/mold/ld" ]]; then + echo "mold ${MOLD_VERSION} already installed at ${BINARY}" + exit 0 +fi + +EXPECTED_SHA="$(pinned_expected_sha "${ROOT_DIR}/tools/mold/SHA256SUMS" "${ARCHIVE}")" + +TMP_DIR="$(mktemp -d)" +cleanup() { + rm -rf -- "${TMP_DIR}" +} +trap cleanup EXIT + +pinned_fetch_verified "${URL}" "${TMP_DIR}/${ARCHIVE}" "${EXPECTED_SHA}" +tar -tzf "${TMP_DIR}/${ARCHIVE}" >/dev/null || + pinned_die "Invalid mold archive: ${ARCHIVE}" + +STAGE_DIR="${TMP_DIR}/mold" +mkdir -p "${STAGE_DIR}" +tar -xzf "${TMP_DIR}/${ARCHIVE}" -C "${STAGE_DIR}" --strip-components=1 +[[ -x "${STAGE_DIR}/bin/mold" ]] || + pinned_die "Mold archive does not contain an executable bin/mold" +[[ -f "${STAGE_DIR}/lib/mold/mold-wrapper.so" ]] || + pinned_die "Mold archive is missing lib/mold/mold-wrapper.so" +[[ -e "${STAGE_DIR}/libexec/mold/ld" ]] || + pinned_die "Mold archive is missing libexec/mold/ld" + +mkdir -p "${INSTALL_PREFIX}" +cp -a "${STAGE_DIR}/." "${INSTALL_PREFIX}/" + +"${BINARY}" --version | grep -qE "^mold ${MOLD_VERSION//./\\.}([[:space:]]|$)" || + pinned_die "Installed mold does not report ${MOLD_VERSION}" +"${INSTALL_PREFIX}/bin/ld.mold" --version | + grep -qE "^mold ${MOLD_VERSION//./\\.}([[:space:]]|$)" || + pinned_die "Installed ld.mold does not report ${MOLD_VERSION}" +echo "Installed mold ${MOLD_VERSION} at ${BINARY}" diff --git a/tests/workflow_contracts/dev_fast_ci_contract_test.py b/tests/workflow_contracts/dev_fast_ci_contract_test.py new file mode 100644 index 00000000..c2f20a85 --- /dev/null +++ b/tests/workflow_contracts/dev_fast_ci_contract_test.py @@ -0,0 +1,216 @@ +"""Contract tests for dev-fast CI workflow provisioning and lint sequencing.""" + +from __future__ import annotations + +import os +import re +import subprocess +import textwrap +from collections.abc import Iterator +from pathlib import Path +from typing import Any + +import pytest +import yaml + +from workflow_support import ROOT + +MAKEFILE_PATH = ROOT / "Makefile" +DEV_FAST_CONFIG = "tools/dev-fast/config.toml" +WORKFLOW_PATHS = { + "ci.yml": ROOT / ".github" / "workflows" / "ci.yml", + "coverage-main.yml": ROOT / ".github" / "workflows" / "coverage-main.yml", +} + + +def _workflow_job(workflow_name: str, job_name: str) -> dict[str, Any]: + workflow = yaml.safe_load(WORKFLOW_PATHS[workflow_name].read_text(encoding="utf-8")) + jobs = workflow.get("jobs") + assert isinstance(jobs, dict), f"{workflow_name} must define jobs" + job = jobs.get(job_name) + assert isinstance(job, dict), f"{workflow_name} must define job {job_name!r}" + return job + + +def _step_named(job: dict[str, Any], name: str) -> tuple[int, dict[str, Any]]: + steps = job.get("steps") + assert isinstance(steps, list), "workflow job must define a step list" + matches = [ + (index, step) for index, step in enumerate(steps) if step.get("name") == name + ] + assert len(matches) == 1, f"expected one {name!r} step, found {len(matches)}" + index, step = matches[0] + assert isinstance(step, dict), f"step {name!r} must be a mapping" + return index, step + + +@pytest.mark.parametrize( + ("workflow_name", "job_name"), + ( + pytest.param("ci.yml", "build-test", id="pull-request-ci"), + pytest.param("coverage-main.yml", "coverage-upload", id="main-coverage"), + ), +) +def test_ci_installs_development_prerequisites_before_lint( + workflow_name: str, job_name: str +) -> None: + job = _workflow_job(workflow_name, job_name) + install_index, install_step = _step_named( + job, "Install development build prerequisites" + ) + lint_index, lint_step = _step_named(job, "Lint") + assert install_index < lint_index, ( + f"{workflow_name} must install the development toolchain before lint" + ) + assert install_step.get("run", "").strip() == "make install-dev-fast" + assert lint_step.get("run", "").strip() == "make lint" + + +def _string_values(value: Any) -> Iterator[str]: + """Yield workflow scalar strings recursively for focused config checks.""" + if isinstance(value, str): + yield value + return + + if isinstance(value, dict): + nested_values = value.values() + elif isinstance(value, list): + nested_values = value + else: + return + + for nested in nested_values: + yield from _string_values(nested) + + +def test_string_values_preserves_container_order_and_omits_non_strings() -> None: + workflow_value = { + "first": "one", + "nested": [ + "two", + {"third": "three", "ignored": 7, "fourth": ["four", None]}, + False, + ], + "also_ignored": None, + } + + assert list(_string_values(workflow_value)) == ["one", "two", "three", "four"] + + +def _dense_simd_commands(step: dict[str, Any]) -> list[str]: + script = step.get("run") + assert isinstance(script, str), "Dense stable SIMD gating must use a run script" + joined = re.sub(r"\\\n[ \t]*", " ", script) + return [ + line.strip() + for line in joined.splitlines() + if line.lstrip().startswith("cargo ") + ] + + +@pytest.mark.parametrize( + ("workflow_name", "job_name"), + ( + pytest.param("ci.yml", "build-test", id="pull-request-ci"), + pytest.param("coverage-main.yml", "coverage-upload", id="main-coverage"), + ), +) +def test_ci_direct_simd_and_coverage_routes_stay_unfragmented( + workflow_name: str, job_name: str +) -> None: + job = _workflow_job(workflow_name, job_name) + _, dense_step = _step_named(job, "Dense stable SIMD gating") + commands = _dense_simd_commands(dense_step) + assert len(commands) == 2, ( + f"{workflow_name} must keep both direct dense SIMD Cargo commands: {commands!r}" + ) + for command in commands: + assert "--config" not in command and DEV_FAST_CONFIG not in command, ( + f"direct SIMD command must remain unfragmented: {command!r}" + ) + + _, coverage_step = _step_named(job, "Test and Measure Coverage") + coverage_values = "\n".join(_string_values(coverage_step)) + assert ( + DEV_FAST_CONFIG not in coverage_values and "--config" not in coverage_values + ), ( + f"coverage action inputs must not select the dev-fast fragment: " + f"{coverage_values!r}" + ) + + +def _make_probe(path: Path, role: str) -> None: + """Write a controlled fake gate that records overlap with its peer.""" + source = textwrap.dedent( + f"""\ + #!/usr/bin/env python3 + import os + from pathlib import Path + import time + + state = Path(os.environ["DEV_FAST_PROBE_STATE"]) + role = {role!r} + peer = "whitaker" if role == "cargo" else "cargo" + lock = state / "lock" + + def acquire_lock(): + deadline = time.monotonic() + 5 + while True: + try: + lock.mkdir() + return + except FileExistsError: + if time.monotonic() >= deadline: + raise SystemExit("timed out acquiring probe lock") + time.sleep(0.005) + + acquire_lock() + if (state / f"{{peer}}.running").exists(): + (state / "overlap").write_text(f"{{role}} overlapped {{peer}}\\n") + (state / f"{{role}}.running").write_text("running\\n") + lock.rmdir() + + time.sleep(0.15) + + acquire_lock() + (state / f"{{role}}.running").unlink() + lock.rmdir() + """ + ) + path.write_text(source, encoding="utf-8") + path.chmod(0o755) + + +def test_parallel_lint_serializes_clippy_and_whitaker(tmp_path: Path) -> None: + cargo = tmp_path / "probe-cargo" + whitaker = tmp_path / "probe-whitaker" + state = tmp_path / "state" + state.mkdir() + _make_probe(cargo, "cargo") + _make_probe(whitaker, "whitaker") + + environment = os.environ.copy() + environment["DEV_FAST_PROBE_STATE"] = str(state) + completed = subprocess.run( + [ + "make", + "--no-print-directory", + "-j2", + "--file", + str(MAKEFILE_PATH), + "lint", + f"CARGO={cargo}", + f"WHITAKER={whitaker}", + ], + cwd=ROOT, + check=False, + capture_output=True, + text=True, + env=environment, + ) + assert completed.returncode == 0, ( + f"controlled lint probe failed: {completed.stdout}\n{completed.stderr}" + ) + assert not (state / "overlap").exists(), ( + "lint-clippy and lint-whitaker ran concurrently under make -j2" + ) diff --git a/tests/workflow_contracts/dev_fast_routing_test.py b/tests/workflow_contracts/dev_fast_routing_test.py new file mode 100644 index 00000000..7c47b5cd --- /dev/null +++ b/tests/workflow_contracts/dev_fast_routing_test.py @@ -0,0 +1,380 @@ +"""Contract tests for explicit development Cargo routing.""" + +from __future__ import annotations + +import shlex +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +MAKEFILE_PATH = REPO_ROOT / "Makefile" +DEV_FAST_TOOLCHAIN_PATH = REPO_ROOT / "tools" / "dev-fast" / "TOOLCHAIN" +DEV_FAST_CONFIG = "tools/dev-fast/config.toml" +STABLE_VERIFY_FLAGS = "-D warnings -Dmissing_docs -Dmissing_crate_level_docs" +STABLE_POISONED_LOCK_TEST = ( + "hnsw::cpu::test_helpers::tests::graph_helpers_report_poisoning_without_reconfiguring" +) +EXPECTED_NEXTTEST_FILTER = ( + "not kind(bench) & not (package(chutoro-core) & binary(result_api_surface)) & " + "not (package(chutoro-core) & binary(session_api_surface)) & " + "not (package(chutoro-providers-dense) & " + "test(portable_simd_without_feature_is_rejected)) & " + "not (package(chutoro-core) & " + f"test(={STABLE_POISONED_LOCK_TEST}))" +) + + +def _make_output( + target: str, + *, + overrides: tuple[str, ...] = (), + makefile: Path = MAKEFILE_PATH, + jobs: int | None = None, +) -> str: + """Return evaluated Make recipes for one target without executing them.""" + command = ["make", "--no-print-directory", "--dry-run"] + if jobs is not None: + command.append(f"-j{jobs}") + command.extend(("--file", str(makefile), *overrides, target, "CARGO=probe-cargo")) + completed = subprocess.run( + command, + cwd=REPO_ROOT, + check=False, + capture_output=True, + text=True, + ) + assert completed.returncode == 0, ( + f"make dry-run for {target!r} failed with {completed.returncode}: " + f"{completed.stderr}" + ) + return completed.stdout + + +def _cargo_invocations(output: str) -> list[tuple[str, list[str]]]: + """Return each evaluated probe-cargo recipe with its shell tokens.""" + invocations: list[tuple[str, list[str]]] = [] + for line in output.splitlines(): + tokens = shlex.split(line) + if "probe-cargo" in tokens: + cargo_index = tokens.index("probe-cargo") + invocations.append((line, tokens[cargo_index:])) + return invocations + + +def _assert_routed_cargo(output: str, expected_verbs: tuple[str, ...]) -> None: + """Require every emitted Cargo command to select the pinned fragment.""" + toolchain = DEV_FAST_TOOLCHAIN_PATH.read_text(encoding="utf-8").strip() + assert toolchain, f"{DEV_FAST_TOOLCHAIN_PATH} must contain a toolchain pin" + assert (REPO_ROOT / DEV_FAST_CONFIG).is_file(), ( + f"the selected Cargo fragment is missing: {DEV_FAST_CONFIG}" + ) + + invocations = _cargo_invocations(output) + assert len(invocations) == len(expected_verbs), ( + f"expected Cargo verbs {expected_verbs!r}, found " + f"{[tokens for _, tokens in invocations]!r}" + ) + for (line, tokens), verb in zip(invocations, expected_verbs, strict=True): + expected_prefix = [f"+{toolchain}", "--config", DEV_FAST_CONFIG, verb] + assert tokens[1 : 1 + len(expected_prefix)] == expected_prefix, ( + f"every debug Cargo invocation must begin with " + f"'probe-cargo {' '.join(expected_prefix)}'; got {line!r}" + ) + + +def _assert_nextest_selects_fragment(output: str) -> None: + """Require nextest's Cargo subprocess to select the development fragment.""" + invocations = [ + (line, tokens) + for line, tokens in _cargo_invocations(output) + if "nextest" in tokens + ] + assert len(invocations) == 1, ( + f"expected one nextest Cargo invocation, found {invocations!r}" + ) + line, tokens = invocations[0] + nextest_args = tokens[tokens.index("nextest") + 1 :] + profile_index = nextest_args.index("--profile") + config_indices = [ + index for index, token in enumerate(nextest_args) if token == "--config" + ] + assert ( + len(config_indices) == 1 + and config_indices[0] < profile_index + and nextest_args[config_indices[0] + 1] == DEV_FAST_CONFIG + ), ( + f"nextest must pass exactly one {DEV_FAST_CONFIG} selection " + f"before --profile: {line!r}" + ) + + +def _assert_test_rustflags(output: str, host_os: str) -> None: + """Check development nextest retains warning policy and host linker flags.""" + invocations = _cargo_invocations(output) + assert invocations, "the Make recipe must emit an injected Cargo command" + _assert_nextest_selects_fragment(output) + line, tokens = next( + (line, tokens) + for line, tokens in invocations + if "nextest" in tokens + ) + toolchain = DEV_FAST_TOOLCHAIN_PATH.read_text(encoding="utf-8").strip() + assert tokens[1:5] == [ + f"+{toolchain}", + "--config", + DEV_FAST_CONFIG, + "nextest", + ], f"test must use the selected route: {line!r}" + assignments = [ + token for token in shlex.split(line) if token.startswith("RUSTFLAGS=") + ] + assert len(assignments) == 1, f"test must set RUSTFLAGS once: {line!r}" + rustflags = assignments[0].partition("=")[2] + required_flags = ("-D warnings", "-Dmissing_docs", "-Dmissing_crate_level_docs") + missing = [flag for flag in required_flags if flag not in rustflags] + assert not missing, f"test RUSTFLAGS lost {missing!r}: {line!r}" + has_mold = "-Clink-arg=-fuse-ld=mold" in rustflags + assert has_mold is (host_os == "Linux"), ( + f"test RUSTFLAGS must select mold only on Linux; " + f"DEV_FAST_HOST_OS={host_os!r}, got {rustflags!r}" + ) + + +def _assert_stable_test_commands( + stable_commands: list[tuple[str, list[str]]], toolchain: str +) -> None: + """Check the three stable test leaves; called only by `_assert_test_boundary`.""" + expected_stable = ( + ( + "test", + "-p", + "chutoro-core", + "--all-features", + "--test", + "result_api_surface", + "--test", + "session_api_surface", + ), + ( + "test", + "-p", + "chutoro-providers-dense", + "--all-features", + "--test", + "portable_simd_gating", + "--", + "--exact", + "portable_simd_without_feature_is_rejected", + ), + ( + "test", + "-p", + "chutoro-core", + "--all-features", + "--lib", + "--", + "--exact", + STABLE_POISONED_LOCK_TEST, + ), + ) + for (line, tokens), expected in zip(stable_commands, expected_stable, strict=True): + assert "env -u RUSTUP_TOOLCHAIN" in line, ( + f"stable diagnostic verification must clear the repository toolchain: {line!r}" + ) + assert DEV_FAST_CONFIG not in tokens and f"+{toolchain}" not in tokens, ( + f"stable verification must not select the dev-fast fragment: {line!r}" + ) + assert tokens[1 : 1 + len(expected)] == list(expected), ( + f"stable diagnostic verification targets changed: {line!r}" + ) + assignments = [ + token for token in shlex.split(line) if token.startswith("RUSTFLAGS=") + ] + assert len(assignments) == 1, f"stable test must set RUSTFLAGS once: {line!r}" + assert assignments[0] == f"RUSTFLAGS={STABLE_VERIFY_FLAGS}", ( + f"stable verification must keep its exact warning flags: {line!r}" + ) + assert "mold" not in line, ( + f"stable verification must not inherit the dev-fast linker: {line!r}" + ) + + +def _assert_test_boundary(output: str, host_os: str) -> None: + """Check nextest and the stable verification runs in their required order.""" + invocations = _cargo_invocations(output) + assert len(invocations) == 4, ( + f"expected four test Cargo invocations in order, got {invocations!r}" + ) + assert all(tokens[0] == "probe-cargo" for _, tokens in invocations), ( + f"CARGO=probe-cargo must reach every test command: {invocations!r}" + ) + + nextest_line, nextest_tokens = invocations[0] + toolchain = DEV_FAST_TOOLCHAIN_PATH.read_text(encoding="utf-8").strip() + assert nextest_tokens[1:5] == [ + f"+{toolchain}", + "--config", + DEV_FAST_CONFIG, + "nextest", + ], f"first test command must be accelerated nextest: {nextest_line!r}" + assert "-E" in nextest_tokens, f"nextest must receive its test filter: {nextest_line!r}" + filter_index = nextest_tokens.index("-E") + assert nextest_tokens[filter_index + 1] == EXPECTED_NEXTTEST_FILTER, ( + "nextest must exclude exactly the stable API, SIMD, and poisoned-lock tests " + f"while retaining the remaining suite: {nextest_line!r}" + ) + + _assert_stable_test_commands(invocations[1:], toolchain) + _assert_test_rustflags(output, host_os) + + +def _assert_unfragmented(output: str, target: str) -> None: + assert DEV_FAST_CONFIG not in output, ( + f"{target} must not select {DEV_FAST_CONFIG}; got:\n{output}" + ) + + +@pytest.mark.parametrize( + ("target", "verbs"), + ( + pytest.param("build", ("build",), id="standard-debug-build"), + pytest.param("test", ("nextest",), id="standard-debug-test"), + pytest.param("typecheck", ("check",), id="standard-debug-typecheck"), + pytest.param("lint-clippy", ("doc", "clippy"), id="lint-doc-and-clippy"), + pytest.param("dev-build", ("build",), id="explicit-dev-build"), + pytest.param("dev-test", ("nextest",), id="explicit-dev-test"), + ), +) +def test_debug_make_targets_select_the_pinned_fragment( + target: str, verbs: tuple[str, ...] +) -> None: + if target in {"test", "dev-test"}: + output = _make_output(target, overrides=("DEV_FAST_HOST_OS=Linux",)) + _assert_test_boundary(output, "Linux") + else: + _assert_routed_cargo(_make_output(target), verbs) + + +@pytest.mark.parametrize( + ("target", "host_os"), + ( + pytest.param("test", "Linux", id="standard-test-linux"), + pytest.param("test", "Darwin", id="standard-test-non-linux"), + pytest.param("dev-test", "Linux", id="dev-test-linux"), + pytest.param("dev-test", "Darwin", id="dev-test-non-linux"), + ), +) +def test_test_routes_preserve_warning_and_platform_flags( + target: str, host_os: str +) -> None: + output = _make_output(target, overrides=(f"DEV_FAST_HOST_OS={host_os}",)) + _assert_test_boundary(output, host_os) + + +@pytest.mark.parametrize( + ("target", "expected_command"), + ( + pytest.param("release", "probe-cargo", id="release-build"), + pytest.param("fmt", "probe-cargo", id="format"), + pytest.param("check-fmt", "probe-cargo", id="format-check"), + pytest.param("bench", "probe-cargo", id="bench"), + pytest.param("kani", "probe-cargo", id="kani"), + pytest.param("kani-full", "probe-cargo", id="full-kani"), + pytest.param("verus", "scripts/run-verus.sh", id="verus"), + pytest.param("lint-whitaker", "whitaker", id="whitaker"), + ), +) +def test_non_development_targets_do_not_select_the_fragment( + target: str, expected_command: str +) -> None: + output = _make_output(target, overrides=("WHITAKER=whitaker-probe",)) + _assert_unfragmented(output, target) + assert expected_command in output, ( + f"{target} did not emit its expected command {expected_command!r}: {output!r}" + ) + if expected_command == "probe-cargo": + invocations = _cargo_invocations(output) + assert invocations, f"{target} must use injected CARGO=probe-cargo" + assert all(tokens[0] == "probe-cargo" for _, tokens in invocations) + + +def _mutated_makefile(tmp_path: Path, old: str, new: str) -> Path: + """Write a private Makefile copy after one asserted text mutation.""" + source = MAKEFILE_PATH.read_text(encoding="utf-8") + assert source.count(old) == 1, f"mutation anchor must occur once: {old!r}" + mutated = tmp_path / "Makefile" + mutated.write_text(source.replace(old, new), encoding="utf-8") + return mutated + + +def test_route_assertion_rejects_a_debug_command_without_the_fragment( + tmp_path: Path, +) -> None: + """A missing explicit selection is caught using evaluated Make output.""" + makefile = _mutated_makefile( + tmp_path, + "DEV_CARGO = $(CARGO) +$(DEV_FAST_TOOLCHAIN) --config $(DEV_FAST_CONFIG)", + "DEV_CARGO = $(CARGO) +$(DEV_FAST_TOOLCHAIN)", + ) + output = _make_output("build", makefile=makefile) + with pytest.raises(AssertionError, match="must begin with"): + _assert_routed_cargo(output, ("build",)) + + +def test_unfragmented_assertion_rejects_a_contaminated_bench_command( + tmp_path: Path, +) -> None: + """A negative-route regression is caught without editing the live Makefile.""" + makefile = _mutated_makefile( + tmp_path, + "$(CARGO) bench -p chutoro-benches", + "$(CARGO) --config tools/dev-fast/config.toml bench -p chutoro-benches", + ) + output = _make_output("bench", makefile=makefile) + with pytest.raises(AssertionError, match="must not select"): + _assert_unfragmented(output, "bench") + + +def test_test_flag_assertion_rejects_a_missing_warning_flag(tmp_path: Path) -> None: + """The effective test environment must retain every required denial.""" + makefile = _mutated_makefile( + tmp_path, + "DEV_TEST_RUSTFLAGS = $(strip -D warnings -Dmissing_docs", + "DEV_TEST_RUSTFLAGS = $(strip -D warnings", + ) + output = _make_output( + "test", makefile=makefile, overrides=("DEV_FAST_HOST_OS=Linux",) + ) + with pytest.raises(AssertionError, match="lost"): + _assert_test_rustflags(output, "Linux") + + +@pytest.mark.parametrize( + "target", + ( + pytest.param("test", id="standard-test"), + pytest.param("dev-test", id="explicit-dev-test"), + ), +) +def test_nextest_assertion_rejects_a_missing_inner_fragment( + target: str, tmp_path: Path +) -> None: + """A missing nextest selection fails while outer Cargo routing remains.""" + makefile = _mutated_makefile( + tmp_path, + "nextest run --config $(DEV_FAST_CONFIG) --profile", + "nextest run --profile", + ) + output = _make_output( + target, + makefile=makefile, + overrides=("DEV_FAST_HOST_OS=Linux",), + ) + + with pytest.raises( + AssertionError, + match=r"exactly one tools/dev-fast/config\.toml selection", + ): + _assert_test_boundary(output, "Linux") diff --git a/tests/workflow_contracts/dev_fast_test_boundary_test.py b/tests/workflow_contracts/dev_fast_test_boundary_test.py new file mode 100644 index 00000000..23f5d666 --- /dev/null +++ b/tests/workflow_contracts/dev_fast_test_boundary_test.py @@ -0,0 +1,169 @@ +"""Mutation contracts for stable byte-exact Make test routing.""" + +from pathlib import Path +import subprocess +import sys + +import pytest + +from dev_fast_routing_test import ( + MAKEFILE_PATH, + STABLE_POISONED_LOCK_TEST, + _assert_test_boundary, + _make_output, + _mutated_makefile, +) + + +@pytest.mark.parametrize( + ("old", "new", "failure"), + ( + pytest.param( + '\tRUSTFLAGS="$(STABLE_VERIFY_RUSTFLAGS)" env -u RUSTUP_TOOLCHAIN ' + '$(CARGO) test -p chutoro-core --all-features --test result_api_surface ' + '--test session_api_surface $(BUILD_JOBS)', + "", + "expected four test Cargo invocations", + id="missing-core-api-stable-run", + ), + pytest.param( + '\tRUSTFLAGS="$(STABLE_VERIFY_RUSTFLAGS)" env -u RUSTUP_TOOLCHAIN ' + '$(CARGO) test -p chutoro-providers-dense --all-features ' + '--test portable_simd_gating $(BUILD_JOBS) -- --exact ' + 'portable_simd_without_feature_is_rejected', + "", + "expected four test Cargo invocations", + id="missing-dense-simd-stable-run", + ), + pytest.param( + '\tRUSTFLAGS="$(STABLE_VERIFY_RUSTFLAGS)" env -u RUSTUP_TOOLCHAIN ' + '$(CARGO) test -p chutoro-core --all-features --lib $(BUILD_JOBS) ' + '-- --exact $(STABLE_POISONED_LOCK_TEST)', + "", + "expected four test Cargo invocations", + id="missing-poisoned-lock-stable-run", + ), + pytest.param( + " & not (package(chutoro-core) & binary(result_api_surface))", + "", + "exclude exactly", + id="missing-result-api-exclusion", + ), + pytest.param( + " & not (package(chutoro-core) & binary(session_api_surface))", + "", + "exclude exactly", + id="missing-session-api-exclusion", + ), + pytest.param( + " & not (package(chutoro-providers-dense) & " + "test(portable_simd_without_feature_is_rejected))", + "", + "exclude exactly", + id="missing-simd-diagnostic-exclusion", + ), + pytest.param( + " & not (package(chutoro-core) & " + "test(=$(STABLE_POISONED_LOCK_TEST)))", + "", + "exclude exactly", + id="missing-poisoned-lock-exclusion", + ), + pytest.param( + "env -u RUSTUP_TOOLCHAIN $(CARGO) test -p chutoro-core " + "--all-features --test result_api_surface", + "env -u RUSTUP_TOOLCHAIN $(CARGO) --config $(DEV_FAST_CONFIG) " + "test -p chutoro-core --all-features --test result_api_surface", + "must not select the dev-fast fragment", + id="stable-api-run-gains-dev-fast-fragment", + ), + pytest.param( + "env -u RUSTUP_TOOLCHAIN $(CARGO) test -p chutoro-core " + "--all-features --lib", + "env -u RUSTUP_TOOLCHAIN $(CARGO) --config $(DEV_FAST_CONFIG) " + "test -p chutoro-core --all-features --lib", + "must not select the dev-fast fragment", + id="poisoned-lock-run-gains-dev-fast-fragment", + ), + ), +) +def test_test_boundary_assertion_rejects_routing_mutations( + old: str, new: str, failure: str, tmp_path: Path +) -> None: + """Each required stable run and exact nextest exclusion is binding.""" + makefile = _mutated_makefile(tmp_path, old, new) + output = _make_output( + "test", makefile=makefile, overrides=("DEV_FAST_HOST_OS=Linux",) + ) + with pytest.raises(AssertionError, match=failure): + _assert_test_boundary(output, "Linux") + + +@pytest.mark.parametrize( + ("failure_signature", "expected_invocations"), + ( + pytest.param("session_api_surface", 2, id="core-api-verification"), + pytest.param("chutoro-providers-dense", 3, id="dense-simd-verification"), + pytest.param( + STABLE_POISONED_LOCK_TEST, + 4, + id="poisoned-lock-verification", + ), + ), +) +def test_make_test_propagates_a_stable_cargo_failure( + failure_signature: str, expected_invocations: int, tmp_path: Path +) -> None: + """A failing stable verification leaf makes the Make gate fail.""" + invocations_path = tmp_path / "cargo-invocations.txt" + fake_cargo = tmp_path / "fake-cargo.py" + fake_cargo.write_text( + f"""import sys +from pathlib import Path + +with Path({str(invocations_path)!r}).open("a", encoding="utf-8") as output: + output.write(repr(sys.argv[1:]) + "\\n") +if "nextest" in sys.argv: + raise SystemExit(0) +if {failure_signature!r} in sys.argv: + raise SystemExit(37) +raise SystemExit(0) +""", + encoding="utf-8", + ) + command = [ + "make", + "--no-print-directory", + "--file", + str(MAKEFILE_PATH), + "test", + f"CARGO={sys.executable} {fake_cargo}", + "DEV_FAST_HOST_OS=Darwin", + ] + completed = subprocess.run( + command, + cwd=MAKEFILE_PATH.parent, + check=False, + capture_output=True, + text=True, + ) + + assert completed.returncode != 0, ( + "make test must fail when its stable Cargo verification command fails" + ) + invocations = invocations_path.read_text(encoding="utf-8").splitlines() + assert len(invocations) == expected_invocations, ( + f"expected ordered Cargo runs through {failure_signature}: {invocations!r}" + ) + expected_order = ( + "nextest", + "session_api_surface", + "chutoro-providers-dense", + STABLE_POISONED_LOCK_TEST, + ) + assert all( + signature in invocation + for signature, invocation in zip( + expected_order[:expected_invocations], invocations, strict=True + ) + ), f"Cargo leaf order changed before {failure_signature}: {invocations!r}" diff --git a/tests/workflow_contracts/toolchain_install_test.py b/tests/workflow_contracts/toolchain_install_test.py index d21a8f7e..9dd6c961 100644 --- a/tests/workflow_contracts/toolchain_install_test.py +++ b/tests/workflow_contracts/toolchain_install_test.py @@ -194,7 +194,7 @@ def test_the_reading_joins_only_what_the_shell_joins( #: feature. On nightly, rustc adds `help: add #![feature(portable_simd)]` to #: each of those errors because there it can be enabled, so the fixture #: mismatches for a reason about the toolchain rather than about the gating. -TOOLCHAIN_SPECIFIC_TRYBUILD_TEST = "portable_simd_gating_compile_checks" +TOOLCHAIN_SPECIFIC_TRYBUILD_TEST = "portable_simd_without_feature_is_rejected" #: The lane that runs the dense provider's tests on nightly. NIGHTLY_SIMD_WORKFLOW = "nightly-portable-simd.yml" diff --git a/tools/dev-fast/TOOLCHAIN b/tools/dev-fast/TOOLCHAIN new file mode 100644 index 00000000..acda8b31 --- /dev/null +++ b/tools/dev-fast/TOOLCHAIN @@ -0,0 +1 @@ +nightly-2026-05-28 diff --git a/tools/dev-fast/config.toml b/tools/dev-fast/config.toml new file mode 100644 index 00000000..e4a072af --- /dev/null +++ b/tools/dev-fast/config.toml @@ -0,0 +1,24 @@ +# Selected debug Cargo fragment, derived from leynos/netsuke at +# 924cb215d3841048dbf6649767a510d2a5dfb1b7 (Concordat issue #157). +# Cargo must see this only through an explicit --config argument. Direct Cargo, +# release, coverage, verification, and Whitaker retain their own build policy. + +[unstable] +codegen-backend = true + +# Cranelift shortens development builds at the expense of runtime speed. +[profile.dev] +codegen-backend = "cranelift" + +# The Linux linker is selected only for this development fragment. Cargo +# chooses target rustflags over [build].rustflags rather than merging them, +# so repeat Chutoro's two documentation-denial flags from .cargo/config.toml. +# GCC resolves `ld.mold` from PATH; Clang can instead select a system copy +# ahead of the version installed under ~/.local/bin. +[target.'cfg(target_os = "linux")'] +linker = "gcc" +rustflags = [ + "-Dmissing_docs", + "-Dmissing_crate_level_docs", + "-Clink-arg=-fuse-ld=mold", +] diff --git a/tools/mold/SHA256SUMS b/tools/mold/SHA256SUMS new file mode 100644 index 00000000..0e50c447 --- /dev/null +++ b/tools/mold/SHA256SUMS @@ -0,0 +1,2 @@ +a3696680d99e692970590a178bc3a33d78d60d1c6dc9db7a11b557b02b751f5d mold-2.41.0-x86_64-linux.tar.gz +946de2774b06a71346bd59b55fddba610b65b8d93c3a4a1559cc84e103472710 mold-2.41.0-aarch64-linux.tar.gz diff --git a/tools/mold/VERSION b/tools/mold/VERSION new file mode 100644 index 00000000..2d4c52ed --- /dev/null +++ b/tools/mold/VERSION @@ -0,0 +1 @@ +2.41.0 diff --git a/typos.local.toml b/typos.local.toml index e6387701..6265d766 100644 --- a/typos.local.toml +++ b/typos.local.toml @@ -11,29 +11,15 @@ accepted = [] [patterns] ignore = [ '\bCARGO_TERM_COLOR\b', - 'types: \[[^]]*\blabeled\b', # Inline code spans are masked; the shared dictionary does not mask them yet. '`[^`\n]+`', 'Header-only C\+\+/python library for fast approximate\s+nearest neighbors', - '"ises"', - '\b[A-Za-z0-9][A-Za-z0-9_]*_(?:canonicalis|finalis|initialis|parameteris|prioritis|unrecognis)\w*\b', - '\b(?:canonicalis|finalis|initialis|parameteris|prioritis|unrecognis)\w*_[A-Za-z0-9_]+\b', - '\b(?:[A-Za-z0-9]+(?:Canonicalis\w*|Finalis\w*|Initialis\w*|Parameteris\w*|Prioritis\w*|Unrecognis\w*)[A-Za-z0-9]*|(?:Canonicalis\w*|Finalis\w*|Initialis\w*|Parameteris\w*|Prioritis\w*|Unrecognis\w*)[A-Za-z0-9]+)\b', - '(?:::|\.)canonicalise\b', - '\bfn canonicalise\b', - '\[\^canonicalise\]', - '"canonicalise" for API compatibility', - '\bfn finalise\b', - '\.finalise\b', - '\binitialised\s*:\s*(?:_|&mut|HashSet)', - '\blet mut initialised\b', - '&mut initialised\b', - '\binitialised\.insert\b', - '\b_initialised\b', - '\n\s*initialised,\n', - '\blet ba =', - '\bba\.value\(\)', - '\bis_symmetric_result\(ab, ba\)', + # Preserve the public compatibility alias, not unrelated -ise prose. + '\bpub const fn canonicalise\b', + '\bedge\.canonicalise\b', + # Existing benchmark CSV consumers depend on these exact output names. + '\blane_utilisation_basis_points\b', + '\bneighbour_scoring_lane_utilisation\.csv\b', ] [files] diff --git a/typos.toml b/typos.toml index fb91906e..9037809b 100644 --- a/typos.toml +++ b/typos.toml @@ -30,33 +30,17 @@ extend-exclude = [ [default] locale = "en-gb" extend-ignore-re = [ - "\"canonicalise\" for API compatibility", - "\"ises\"", - "&mut initialised\\b", - "(?:::|\\.)canonicalise\\b", "(?s)```.*?```", "Azure\\s+Architecture\\s+Center\\s*\\|\\s*Microsoft\\s+Learn\\b", "GitHub\\s+Flavored\\s+Markdown\\b(?:\\s\\(GFM\\))?", "Header-only C\\+\\+/python library for fast approximate\\s+nearest neighbors", - "\\.finalise\\b", - "\\[\\^canonicalise\\]", - "\\b(?:[A-Za-z0-9]+(?:Canonicalis\\w*|Finalis\\w*|Initialis\\w*|Parameteris\\w*|Prioritis\\w*|Unrecognis\\w*)[A-Za-z0-9]*|(?:Canonicalis\\w*|Finalis\\w*|Initialis\\w*|Parameteris\\w*|Prioritis\\w*|Unrecognis\\w*)[A-Za-z0-9]+)\\b", - "\\b(?:canonicalis|finalis|initialis|parameteris|prioritis|unrecognis)\\w*_[A-Za-z0-9_]+\\b", "\\bCARGO_TERM_COLOR\\b", - "\\b[A-Za-z0-9][A-Za-z0-9_]*_(?:canonicalis|finalis|initialis|parameteris|prioritis|unrecognis)\\w*\\b", - "\\b_initialised\\b", - "\\bba\\.value\\(\\)", - "\\bfn canonicalise\\b", - "\\bfn finalise\\b", - "\\binitialised\\.insert\\b", - "\\binitialised\\s*:\\s*(?:_|&mut|HashSet)", - "\\bis_symmetric_result\\(ab, ba\\)", - "\\blet ba =", - "\\blet mut initialised\\b", + "\\bedge\\.canonicalise\\b", + "\\blane_utilisation_basis_points\\b", + "\\bneighbour_scoring_lane_utilisation\\.csv\\b", + "\\bpub const fn canonicalise\\b", "\\brust-analyzer\\b", - "\\n\\s*initialised,\\n", "`[^`\\n]+`", - "types: \\[[^]]*\\blabeled\\b", ] [default.extend-words] diff --git a/verus/edge_harvest_ordering.rs b/verus/edge_harvest_ordering.rs index ec02c95f..7ff8b9e4 100644 --- a/verus/edge_harvest_ordering.rs +++ b/verus/edge_harvest_ordering.rs @@ -282,25 +282,25 @@ pub(super) proof fn lemma_edge_leq_total_ordering() /// ```text /// Edges with source > target yield swapped endpoints. /// ``` -pub(super) proof fn lemma_canonicalise_preserves_fields(edge: CandidateEdgeSpec) +pub(super) proof fn lemma_canonicalize_preserves_fields(edge: CandidateEdgeSpec) ensures - edge.canonicalise().distance == edge.distance, - edge.canonicalise().sequence == edge.sequence, - edge.canonicalise().source <= edge.canonicalise().target, - edge.canonicalise().source + edge.canonicalize().distance == edge.distance, + edge.canonicalize().sequence == edge.sequence, + edge.canonicalize().source <= edge.canonicalize().target, + edge.canonicalize().source == if edge.source <= edge.target { edge.source } else { edge.target }, - edge.canonicalise().target + edge.canonicalize().target == if edge.source <= edge.target { edge.target } else { edge.source }, { - let canonical = edge.canonicalise(); + let canonical = edge.canonicalize(); if edge.source <= edge.target { assert(canonical == edge); diff --git a/verus/edge_harvest_proofs.rs b/verus/edge_harvest_proofs.rs index eabfe469..53930e1b 100644 --- a/verus/edge_harvest_proofs.rs +++ b/verus/edge_harvest_proofs.rs @@ -49,7 +49,7 @@ impl CandidateEdgeSpec { /// ```text /// (source=3, target=1) yields (source=1, target=3). /// ``` - pub open spec fn canonicalise(self) -> Self { + pub open spec fn canonicalize(self) -> Self { if self.source <= self.target { self } else {