HNSW packed-index neighbour evaluation (2.3.1)

[lodyai]

Summary

Adds the execution plan for roadmap item 2.3.1 — Restructure HNSW
neighbour evaluation to use packed indices and an SoA layout, prefetching
blocks and scoring outside the write lock (see docs/chutoro-design.md §6.3).

The plan document is
docs/execplans/2-3-1-hnsw-neighbour-evaluation-using-packed-indices.md.
This PR now contains the approved implementation and review follow-up changes for this plan.

How the plan was produced

• Reconnaissance + prior-art research (multi-agent workflow): mapped the
  HNSW insertion write-lock path, the dense provider's SoA infrastructure, and
  the benchmark/test/docs surface; researched prior art (instant-distance,
  hnswlib), stable-Rust _mm_prefetch, and hyperfine/Criterion methodology.
• Community-of-experts review (Logisphere design-review panel — six lenses,
  pre-mortem, alternatives, chair synthesis): verdict REVISE. All twelve
  required revisions were folded into the committed draft.

Key findings shaping the plan

• 2.3.1's structural intent is largely already delivered by Phase 2.2:
  scoring already runs outside the write lock (planner under the read lock;
  score_trim_jobs lock-free), and the dense adapter already packs query-centric
  batches into a 64-byte-aligned, 16-lane-padded, dimension-major SoA
  DensePointView.
• The genuinely unrealised seam is a pack → unpack → repack round-trip: the
  dense provider does not override batch_distances, so packed ids are turned
  into pairs and the query is re-derived inside the adapter.
• The distance cache is present on every production path, so batches reaching
  the adapter are the cache-miss subset — frequently too small to pack —
  capping any allocation/prefetch win. The plan makes this a measured question.

Plan shape

• Committed scope (always delivered): a same-thread write-lock-free-scoring
  invariant guard; a docs/chutoro-design.md §6.3 implementation-update; an
  ADR (adr-003) recording the adapter boundary and deferred structural levers;
  and a neighbour_scoring measurement harness (also the canonical 2.4.1
  artefact).
• Conditional scope (evidence-gated on a pre-registered cycle-count
  threshold): core-side allocation hygiene, a query-centric port override with
  aligned SoA buffer reuse, and packing-step prefetch behind a non-default
  simd_prefetch feature. A committed-scope-only outcome is an explicit success.
• Deeper structural levers (cross-node beam scoring, a secondary dimension-major
  SoA copy, batch_distances_into) are deferred to a separate
  evidence-first item, recorded in ADR-003.

All deltas are behaviour-preserving under fixed seeds; validation leans on the
existing search-correctness, idempotency, mutation, and backend-parity suites
plus the new invariant guard, with Criterion + critcmp + cycle counts +
hyperfine for performance evidence.

Validation of this PR

• make markdownlint passes (0 errors).
• make check-fmt passes.
• make lint passes.
• make typecheck passes.
• make test passes: 1061 passed, 1 skipped.

References

• Lody session: https://lody.ai/leynos/sessions/f71d4791-82ff-4612-b4dd-45616d8d10f9

🤖 Generated with Claude Code

[sourcery-ai]

Sorry @LodyAI[bot], you have reached your weekly rate limit of 2500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 2 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9d72caaf-58e6-44b4-8e13-576b0233ee72

📥 Commits

Reviewing files that changed from the base of the PR and between 55ffac7 and 752672d.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (29)

• .config/nextest.toml
• chutoro-benches/Cargo.toml
• chutoro-benches/benches/extraction.rs
• chutoro-benches/benches/hnsw.rs
• chutoro-benches/benches/mst.rs
• chutoro-benches/benches/neighbour_scoring.rs
• chutoro-benches/benches/neighbour_scoring/profiling.rs
• chutoro-benches/benches/neighbour_scoring/support.rs
• chutoro-benches/src/criterion_support.rs
• chutoro-benches/src/lib.rs
• chutoro-benches/src/neighbour_scoring/build_profile.rs
• chutoro-benches/src/neighbour_scoring/mod.rs
• chutoro-benches/src/neighbour_scoring/profiling.rs
• chutoro-benches/src/neighbour_scoring/report.rs
• chutoro-benches/tests/neighbour_scoring_support.rs
• chutoro-core/src/hnsw/cpu/internal.rs
• chutoro-core/src/hnsw/cpu/test_helpers.rs
• chutoro-core/src/hnsw/tests/mod.rs
• chutoro-core/src/hnsw/tests/write_lock.rs
• chutoro-core/tests/nextest_config.rs
• chutoro-test-support/src/lib.rs
• docs/adr-003-soa-prefetch-adapter-boundary.md
• docs/chutoro-design.md
• docs/developers-guide.md
• docs/execplans/2-2-6-property-based-backend-parity-suite.md
• docs/execplans/2-3-1-hnsw-neighbour-evaluation-using-packed-indices.md
• docs/roadmap.md
• docs/users-guide.md
• scripts/bench-neighbour-scoring.sh

Walkthrough

This PR adds a neighbour_scoring Criterion benchmark with build-profile and lane-utilisation CSV diagnostics, introduces test-only write-graph lock tracking with invariant tests asserting scoring never runs whilst the graph write lock is held, a shared EnvVarGuard test helper, a hyperfine wrapper script, and supporting documentation/roadmap/nextest updates.

Changes

Neighbour-scoring benchmark and write-lock invariant changes

Layer / File(s)	Summary
Write-graph marker and invariant tests chutoro-core/src/hnsw/cpu/internal.rs, chutoro-core/src/hnsw/cpu/test_helpers.rs, chutoro-core/src/hnsw/tests/mod.rs, chutoro-core/src/hnsw/tests/write_lock.rs, chutoro-core/tests/nextest_config.rs, .config/nextest.toml	Adds thread-local write-graph depth tracking, a WriteGraphScope guard, and test helpers; introduces write_lock.rs proptest/unit tests asserting scoring never runs whilst the write lock is held, panicking when violated; raises the criterion bench slow-timeout to 900s and adds a dedicated 600s override plus updated nextest config test assertions.
Neighbour-scoring benchmark support chutoro-benches/Cargo.toml, chutoro-benches/benches/hnsw.rs, chutoro-benches/benches/mst.rs, chutoro-benches/benches/extraction.rs, chutoro-benches/benches/neighbour_scoring.rs, chutoro-benches/benches/neighbour_scoring/*, chutoro-benches/src/criterion_support.rs, chutoro-benches/src/lib.rs, chutoro-benches/src/neighbour_scoring/*, chutoro-benches/tests/neighbour_scoring_support.rs	Adds camino/criterion/Arrow/proptest dependencies and a new neighbour_scoring bench target; adds shared criterion_support exact-probe/short-measurement helpers used across hnsw.rs, mst.rs, and extraction.rs; introduces the neighbour_scoring benchmark with a ProfilingSource wrapper, ScoringFixture/bucket generation, CSV report writers for lane utilisation and build profile, integer diagnostic helpers, and integration tests.
Environment guard and bench script chutoro-test-support/src/lib.rs, scripts/bench-neighbour-scoring.sh	Adds EnvVarGuard for serialised, restorable environment variable mutation in tests, plus a hyperfine-based shell script that builds and runs the neighbour_scoring benchmark executable.
Docs, roadmap, and execplan updates docs/adr-003-soa-prefetch-adapter-boundary.md, docs/chutoro-design.md, docs/developers-guide.md, docs/roadmap.md, docs/users-guide.md, docs/execplans/*	Adds ADR-003 on the SoA/prefetch adapter boundary, updates design/roadmap/user/developer guides describing the benchmark, scoring invariants, and rescoped roadmap item 2.3.1 with new items 2.3.3–2.3.5, plus the full 2.3.1 execplan and MD013 linting suppressions.

Sequence Diagram(s)

sequenceDiagram
  participant SearchTest
  participant CpuHnsw
  participant WriteGraphScope
  participant WriteLockAssertingSource

  SearchTest->>CpuHnsw: write_graph(closure)
  CpuHnsw->>WriteGraphScope: enter_if_enabled()
  SearchTest->>CpuHnsw: search(query)
  CpuHnsw->>WriteLockAssertingSource: batch_distances(query, candidates)
  WriteLockAssertingSource->>CpuHnsw: current_thread_holds_write_graph_for_test()
  CpuHnsw-->>WriteLockAssertingSource: false
  WriteLockAssertingSource-->>CpuHnsw: distances

Loading

sequenceDiagram
  participant NeighbourScoringBench
  participant SupportRs
  participant ProfilingSource
  participant CpuHnsw
  participant ReportRs

  NeighbourScoringBench->>SupportRs: make_fixture(dimension, candidate_count)
  SupportRs-->>NeighbourScoringBench: ScoringFixture
  NeighbourScoringBench->>SupportRs: write_lane_utilisation_report()
  SupportRs->>ReportRs: write_lane_utilisation_report_csv(rows)
  NeighbourScoringBench->>SupportRs: write_build_profile_report()
  SupportRs->>ProfilingSource: wrap SyntheticSource
  SupportRs->>CpuHnsw: build(profiling_source)
  SupportRs->>ProfilingSource: take_snapshot()
  SupportRs->>ReportRs: write_build_profile_report_csv(rows)
  NeighbourScoringBench->>ProfilingSource: batch_distances(query, candidates)

Loading

Possibly related PRs

• leynos/chutoro#122: Both PRs edit .config/nextest.toml to adjust nextest benchmark timeout behaviour via slow-timeout overrides and per-target override blocks.

Suggested labels: Roadmap

Suggested reviewers: codescene-delta-analysis, codescene-access

Poem

A lock held tight, no scoring near,
The write-graph guard stands sentinel here.
Lanes get counted, buckets scored,
CSVs written, timings stored.
🐇 Hop the benches, run the test,
ADR-003 lays the boundary to rest.

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (3 errors, 2 warnings)

Check name	Status	Explanation	Resolution
Testing (Overall)	❌ Error	Most helpers have direct tests, but neighbour_scoring orchestration is still only smoke-tested; a wrong report target or skipped loop would still pass.	Add assertions for neighbour_scoring_impl and write_build_profile_report that verify the output path, CSV contents, and all dimension/bucket iterations.
Unit Architecture	❌ Error	write_build_profile_report_for_point_counts ignores the supplied report_target when creating the file; it always writes BUILD_PROFILE_REPORT and only returns the caller path.	Create the file from the supplied target, or assert it matches report_path_value(...). Add a test for a mismatched target so the side-effect path stays explicit.
Rust Compiler Lint Integrity	❌ Error	FAIL: module-level #[expect(dead_code)] and #[expect(unused_imports)] remain on neighbour_scoring bench test modules, suppressing unused-code/import diagnostics instead of narrowing the boundary.	Remove those broad expectations; split the bench-test helpers into narrower cfg/module boundaries, or attach any unavoidable #[expect] to the exact item with a tracked reason.
Domain Architecture	⚠️ Warning	The build-profile writer returns report_target but always creates BUILD_PROFILE_REPORT under open_report_dir, so it can claim success for a path it never used.	Use report_target to derive the write path, or debug_assert it matches report_path_value(report_parent_dir, BUILD_PROFILE_REPORT) before creating the file.
Architectural Complexity And Maintainability	⚠️ Warning	write_build_profile_report_for_point_counts still ignores the caller-supplied target path and relies on a hidden convention; the module also keeps invalid clippy expectations.	Derive the write path from report_target (or assert it matches) and replace the bogus clippy expectations with valid lint names or remove them.

✅ Passed checks (15 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the PR’s main change and includes the required roadmap item reference 2.3.1.
Description check	✅ Passed	The description directly covers the 2.3.1 execplan and related review-follow-up changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
User-Facing Documentation	✅ Passed	docs/users-guide.md adds a Neighbour-scoring diagnostics section and links to the detailed measurements in the developers guide.
Developer Documentation	✅ Passed	PASS: docs/developers-guide.md, docs/chutoro-design.md, ADR-003, the execplan and docs/roadmap.md all document the benchmark/invariant and mark 2.3.1 complete.
Module-Level Documentation	✅ Passed	PASS: Every new or changed module begins with a //! doc comment that states its purpose and, where needed, its relationship to neighbouring components.
Testing (Unit And Behavioural)	✅ Passed	PASS: The PR adds focused unit/property tests for pure helpers plus behavioural/integration coverage for CSV output, env guards, CLI script wiring, and HNSW write-lock invariants.
Testing (Property / Proof)	✅ Passed	PASS: The PR adds substantive proptest suites for range/state invariants and Kani-backed write-lock checks, covering bounds, monotonicity, round-trips, and concurrency.
Testing (Compile-Time / Ui)	✅ Passed	PASS: no compile-time seam appears; structured CSV and script output is verified with focused exact-output and property tests, so trybuild/snapshots are unnecessary.
Observability	✅ Passed	PASS: keep this as non-applicable; the changes are confined to the non-publish benchmark crate and #[cfg(test)] HNSW seams, with CSV diagnostics rather than production operational paths.
Security And Privacy	✅ Passed	PASS: Keep secrets out, and confine env/path inputs to local benchmark/report helpers; shell args are escaped and CSV escaping is explicit.
Performance And Resource Use	✅ Passed	All new loops and collections are bounded by fixed benchmark inputs; snapshots drain batch-size buffers, reports stream to writers, and no new unbounded hot-path I/O or growth appears.
Concurrency And State	✅ Passed	Shared state is local and synchronised: EnvVarGuard restores under a global Mutex, ProfilingSource uses atomics plus a Mutex snapshot, and write-lock tests cover contention and thread scope.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 2-3-1-hnsw-neighbour-evaluation-using-packed-indices

---

_{Comment @coderabbitai help to get the list of available commands.}

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

chutoro-test-support/src/lib.rs

Comment on lines +46 to +57

        pub fn set(key: &'static str, value: &str) -> Self {
            let lock = env_lock();
            let guard = Self {
                key,
                previous: env::var_os(key),
                _lock: lock,
            };
            // SAFETY: all test environment mutations through this helper are
            // serialized by ENV_LOCK and restored while holding the lock.
            unsafe { env::set_var(key, value) };
            guard
        }

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: env.EnvVarGuard.remove,env.EnvVarGuard.set

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

chutoro-benches/src/neighbour_scoring/report.rs

Comment on file

}

fn csv_escape(field: &str) -> Cow<'_, str> {
    if field.contains(',') || field.contains('"') || field.contains('\n') || field.contains('\r') {

❌ New issue: Complex Conditional
csv_escape has 1 complex conditionals with 3 branches, threshold = 2

[sourcery-ai]

Sorry @leynos, your pull request is larger than the review limit of 150000 diff characters

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@chutoro-core/src/hnsw/cpu/internal.rs`:
- Around line 22-66: Add a brief comment near WRITE_GRAPH_MARKER_ENABLE_COUNT
and WRITE_GRAPH_DEPTH explaining the two-tier design: the enable flag in
enable_write_graph_marker/disable_write_graph_marker is process-wide so any
Rayon worker entering write_graph is tracked once tests opt in, while
current_thread_holds_write_graph and WriteGraphScope use thread-local depth so
only the thread actually executing the write closure reports ownership; make
clear this avoids cross-thread false positives/negatives and should not be
collapsed into a single global boolean.

In `@chutoro-core/src/hnsw/cpu/test_helpers.rs`:
- Line 13: The WriteGraphMarkerGuard type is currently defined as an empty-brace
struct, so update it to a unit struct and adjust its construction sites
accordingly. Locate WriteGraphMarkerGuard in the HNSW CPU test helpers and
remove the braces from the type definition, then replace any
WriteGraphMarkerGuard {} instantiations with the unit form to keep the API
idiomatic and consistent.

In `@chutoro-core/src/hnsw/tests/write_lock.rs`:
- Around line 101-119: In `distance_batch`, the write-lock assertion is
happening before the output-length validation, which increments `scoring_calls`
even on rejected mismatched inputs. Update the method in `write_lock` so the
`pairs.len() != out.len()` check runs first, and only call
`assert_not_writing_graph()` after the lengths are confirmed to match. Keep the
rest of `distance_batch` unchanged, including the `distance_value` loop and
error return.

In `@chutoro-test-support/src/lib.rs`:
- Around line 12-109: Document that EnvVarGuard is non-reentrant and only one
guard may be alive at a time on a thread, since EnvVarGuard holds the ENV_LOCK
MutexGuard for its full lifetime and env_lock() will block on nested
acquisition. Update the EnvVarGuard struct-level docs and/or the
with_env_mutation/set/remove docs to state this restriction plainly, using the
EnvVarGuard and env_lock symbols so it’s easy to find. Keep the wording explicit
that attempting to create a second guard while one is active will deadlock/hang
rather than being supported.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2f7be0ee-d3c5-4931-bffe-ee70dcf71250

📥 Commits

Reviewing files that changed from the base of the PR and between 670b8ff and f316b72.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (25)

• .config/nextest.toml
• .gitignore
• chutoro-benches/Cargo.toml
• chutoro-benches/benches/extraction.rs
• chutoro-benches/benches/neighbour_scoring.rs
• chutoro-benches/benches/neighbour_scoring/profiling.rs
• chutoro-benches/benches/neighbour_scoring/support.rs
• chutoro-benches/src/lib.rs
• chutoro-benches/src/neighbour_scoring/build_profile.rs
• chutoro-benches/src/neighbour_scoring/mod.rs
• chutoro-benches/src/neighbour_scoring/profiling.rs
• chutoro-benches/src/neighbour_scoring/report.rs
• chutoro-benches/tests/neighbour_scoring_support.rs
• chutoro-core/src/hnsw/cpu/internal.rs
• chutoro-core/src/hnsw/cpu/test_helpers.rs
• chutoro-core/src/hnsw/tests/mod.rs
• chutoro-core/src/hnsw/tests/write_lock.rs
• chutoro-test-support/src/lib.rs
• docs/adr-003-soa-prefetch-adapter-boundary.md
• docs/chutoro-design.md
• docs/developers-guide.md
• docs/execplans/2-2-6-property-based-backend-parity-suite.md
• docs/execplans/2-3-1-hnsw-neighbour-evaluation-using-packed-indices.md
• docs/roadmap.md
• scripts/bench-neighbour-scoring.sh

[pandalump]

@coderabbitai resume

[coderabbitai]

✅ Action performed

Reviews resumed.

[coderabbitai]

Actionable comments posted: 4

♻️ Duplicate comments (1)

.config/nextest.toml (1)

113-117: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Strip the duplicated [[profile.ci.overrides]] header — same defect as the default profile block.

Identical issue to the profile.default.overrides addition above: the header line appears twice consecutively before the comment/filter/slow-timeout fields. Remove the stray duplicate.

🔧 Proposed fix

-[[profile.ci.overrides]]
 [[profile.ci.overrides]]
 # Write-lock proptest can exceed the default timeout under CI load.
 filter = "test(/generated_hnsw_scoring_does_not_run_inside_write_graph_scope/)"
 slow-timeout = { period = "600s", terminate-after = 1, grace-period = "5s" }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.config/nextest.toml around lines 113 - 117, The `[[profile.ci.overrides]]`
block has a duplicated table header, which should be removed so the overrides
entry is declared only once. Update the `profile.ci.overrides` section in the
nextest config to keep a single `[[profile.ci.overrides]]` header before the
comment, filter, and slow-timeout fields, matching the intended structure used
in the `profile.default.overrides` block.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@chutoro-benches/benches/extraction.rs`:
- Line 19: Hoist the shared Criterion CLI probe out of the extraction bench so
it is not duplicated per benchmark file. Move is_exact_benchmark_probe into a
common support module such as chutoro-benches/src/lib.rs, then update
extraction.rs and the matching hnsw bench to call that shared helper instead of
keeping separate copies; if the same warm-up/measurement adjustment pattern is
repeated, centralize that too (for example by extracting the
configure_extraction_group-style logic).

In `@chutoro-benches/benches/hnsw.rs`:
- Around line 137-139: The exact-probe short-circuit and the `SourceBenchSpec`
label both hard-code the `"text_levenshtein"` string, so they can drift and
break the `should_short_circuit_exact_text_probe` behavior. Extract that label
into a shared constant in `hnsw.rs` and use it both in
`should_short_circuit_exact_text_probe` and where the `SourceBenchSpec` is
constructed. This keeps the `CpuHnsw` benchmark label and the exact-probe check
in sync and lets the compiler catch future renames.

In `@chutoro-benches/benches/neighbour_scoring.rs`:
- Around line 38-49: Stop duplicating the truthy environment flag parsing in
should_use_short_measurement_value and reuse the shared helper already used by
should_collect_build_profile_value. Extract the
trim/lowercase/"1"|"true"|"on"|"yes" predicate into a single exported helper in
build_profile.rs, re-export it through mod.rs with the other #[doc(hidden)]
utilities, and update both should_use_short_measurement() and
should_collect_build_profile_value() to delegate to that shared function. Add or
reuse tests so the parser behavior remains covered in one place.

In `@chutoro-benches/benches/neighbour_scoring/support.rs`:
- Line 29: The report directory name is duplicated, so keep a single source of
truth by exporting the existing REPORT_DIR_NAME from build_profile.rs (using the
same pub #[doc(hidden)] pattern as report_path_value) and re-exporting it from
mod.rs. Then update support.rs to import and use that shared constant instead of
declaring its own literal, so the directory creation logic and reported path
stay in sync.

---

Duplicate comments:
In @.config/nextest.toml:
- Around line 113-117: The `[[profile.ci.overrides]]` block has a duplicated
table header, which should be removed so the overrides entry is declared only
once. Update the `profile.ci.overrides` section in the nextest config to keep a
single `[[profile.ci.overrides]]` header before the comment, filter, and
slow-timeout fields, matching the intended structure used in the
`profile.default.overrides` block.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3d40e31e-06f2-424f-83d0-e70a7a95ef7d

📥 Commits

Reviewing files that changed from the base of the PR and between bddd53b and e03c888.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (28)

• .config/nextest.toml
• .gitignore
• chutoro-benches/Cargo.toml
• chutoro-benches/benches/extraction.rs
• chutoro-benches/benches/hnsw.rs
• chutoro-benches/benches/neighbour_scoring.rs
• chutoro-benches/benches/neighbour_scoring/profiling.rs
• chutoro-benches/benches/neighbour_scoring/support.rs
• chutoro-benches/src/lib.rs
• chutoro-benches/src/neighbour_scoring/build_profile.rs
• chutoro-benches/src/neighbour_scoring/mod.rs
• chutoro-benches/src/neighbour_scoring/profiling.rs
• chutoro-benches/src/neighbour_scoring/report.rs
• chutoro-benches/tests/neighbour_scoring_support.rs
• chutoro-core/src/hnsw/cpu/internal.rs
• chutoro-core/src/hnsw/cpu/test_helpers.rs
• chutoro-core/src/hnsw/tests/mod.rs
• chutoro-core/src/hnsw/tests/write_lock.rs
• chutoro-core/tests/nextest_config.rs
• chutoro-test-support/src/lib.rs
• docs/adr-003-soa-prefetch-adapter-boundary.md
• docs/chutoro-design.md
• docs/developers-guide.md
• docs/execplans/2-2-6-property-based-backend-parity-suite.md
• docs/execplans/2-3-1-hnsw-neighbour-evaluation-using-packed-indices.md
• docs/roadmap.md
• docs/users-guide.md
• scripts/bench-neighbour-scoring.sh

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

chutoro-benches/tests/neighbour_scoring_support.rs

Comment on lines +186 to +196

fn build_profile_report_target_honours_cargo_target_dir() {
    let report_parent_dir = report_parent_dir_value(Some("/tmp/chutoro-target-dir"));
    let actual_path = build_profile_report_target_value(Some("yes"), &report_parent_dir);

    assert_eq!(
        actual_path.as_deref(),
        Some(Utf8Path::new(
            "/tmp/chutoro-target-dir/benchmarks/neighbour_scoring_build_profile.csv",
        )),
    );
}

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: build_profile_report_target_honours_cargo_target_dir,build_profile_report_target_reads_cargo_target_dir_env

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@chutoro-benches/benches/neighbour_scoring/support.rs`:
- Around line 233-274: write_build_profile_report_for_point_counts is using
report_target only as a gate/return value, but still writes via
open_report_dir(report_parent_dir) and BUILD_PROFILE_REPORT, which can diverge
from the caller’s intended path. Update the function to derive the output file
from report_target directly, or validate that report_target matches
report_path_value(report_parent_dir, BUILD_PROFILE_REPORT) before opening the
file. Keep the success path and the actual write destination aligned so the
returned Utf8PathBuf always reflects where write_build_profile_report_csv wrote
the report.

In `@chutoro-benches/src/neighbour_scoring/mod.rs`:
- Around line 40-47: The #[expect] annotations in neighbour_scoring::mod are
using a non-existent Clippy lint name, so update the expectations to use only
valid lint IDs; remove clippy::integer_division_remainder_used or replace it
with the correct lint if that was intended, and apply the same correction to the
other two matching expectation blocks in this module.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 5bf01346-b4de-4402-beac-28588b70380c

📥 Commits

Reviewing files that changed from the base of the PR and between e03c888 and 55ffac7.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (29)

• .config/nextest.toml
• chutoro-benches/Cargo.toml
• chutoro-benches/benches/extraction.rs
• chutoro-benches/benches/hnsw.rs
• chutoro-benches/benches/mst.rs
• chutoro-benches/benches/neighbour_scoring.rs
• chutoro-benches/benches/neighbour_scoring/profiling.rs
• chutoro-benches/benches/neighbour_scoring/support.rs
• chutoro-benches/src/criterion_support.rs
• chutoro-benches/src/lib.rs
• chutoro-benches/src/neighbour_scoring/build_profile.rs
• chutoro-benches/src/neighbour_scoring/mod.rs
• chutoro-benches/src/neighbour_scoring/profiling.rs
• chutoro-benches/src/neighbour_scoring/report.rs
• chutoro-benches/tests/neighbour_scoring_support.rs
• chutoro-core/src/hnsw/cpu/internal.rs
• chutoro-core/src/hnsw/cpu/test_helpers.rs
• chutoro-core/src/hnsw/tests/mod.rs
• chutoro-core/src/hnsw/tests/write_lock.rs
• chutoro-core/tests/nextest_config.rs
• chutoro-test-support/src/lib.rs
• docs/adr-003-soa-prefetch-adapter-boundary.md
• docs/chutoro-design.md
• docs/developers-guide.md
• docs/execplans/2-2-6-property-based-backend-parity-suite.md
• docs/execplans/2-3-1-hnsw-neighbour-evaluation-using-packed-indices.md
• docs/roadmap.md
• docs/users-guide.md
• scripts/bench-neighbour-scoring.sh

[pandalump]

@coderabbitai resume

[coderabbitai]

✅ Action performed

Reviews resumed.

[leynos]

@coderabbitai Have the following failed checks now been resolved?

If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures.

Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope).

❌ Failed checks (3 errors, 2 warnings)

Check name	Status	Explanation	Resolution
Testing (Overall)	❌ Error	Most helpers have direct tests, but neighbour_scoring orchestration is still only smoke-tested; a wrong report target or skipped loop would still pass.	Add assertions for neighbour_scoring_impl and write_build_profile_report that verify the output path, CSV contents, and all dimension/bucket iterations.
Unit Architecture	❌ Error	write_build_profile_report_for_point_counts ignores the supplied report_target when creating the file; it always writes BUILD_PROFILE_REPORT and only returns the caller path.	Create the file from the supplied target, or assert it matches report_path_value(...). Add a test for a mismatched target so the side-effect path stays explicit.
Rust Compiler Lint Integrity	❌ Error	FAIL: module-level #[expect(dead_code)] and #[expect(unused_imports)] remain on neighbour_scoring bench test modules, suppressing unused-code/import diagnostics instead of narrowing the boundary.	Remove those broad expectations; split the bench-test helpers into narrower cfg/module boundaries, or attach any unavoidable #[expect] to the exact item with a tracked reason.
Domain Architecture	⚠️ Warning	The build-profile writer returns report_target but always creates BUILD_PROFILE_REPORT under open_report_dir, so it can claim success for a path it never used.	Use report_target to derive the write path, or debug_assert it matches report_path_value(report_parent_dir, BUILD_PROFILE_REPORT) before creating the file.
Architectural Complexity And Maintainability	⚠️ Warning	write_build_profile_report_for_point_counts still ignores the caller-supplied target path and relies on a hidden convention; the module also keeps invalid clippy expectations.	Derive the write path from report_target (or assert it matches) and replace the bogus clippy expectations with valid lint names or remove them.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== support.rs full ==='
cat -n chutoro-benches/benches/neighbour_scoring/support.rs

echo '=== neighbour_scoring.rs full ==='
cat -n chutoro-benches/benches/neighbour_scoring.rs

echo '=== profiling.rs (bench) ==='
cat -n chutoro-benches/benches/neighbour_scoring/profiling.rs

echo '=== search for expect(dead_code) / expect(unused_imports) ==='
rg -n '#\[expect\(' chutoro-benches/benches -g '*.rs'

echo '=== search for report_target usage ==='
rg -n 'report_target' chutoro-benches -g '*.rs'

echo '=== build_profile.rs ==='
cat -n chutoro-benches/src/neighbour_scoring/build_profile.rs

Length of output: 40380

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '=== search for clippy expect attributes ==='
rg -n 'expect\(clippy' chutoro-benches -g '*.rs' || echo "none found"

echo '=== search for compiler warnings suppressions across benches crate broadly ==='
rg -n '#\[allow\(' chutoro-benches/benches -g '*.rs'

echo '=== check tests file for neighbour_scoring_support.rs full context around report_target tests to see if mismatch test exists ==='
sed -n '1,220p' chutoro-benches/tests/neighbour_scoring_support.rs

Length of output: 937

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '1,220p' chutoro-benches/tests/neighbour_scoring_support.rs

Length of output: 7464

---

@leynos None of these are resolved yet. Here's what the current codegraph shows.

Unit Architecture / Domain Architecture — still broken, same root cause.
In chutoro-benches/benches/neighbour_scoring/support.rs, write_build_profile_report_for_point_counts (Lines 233–274) destructures report_target into path only to gate the early return (Line 239) and to hand back in Ok(Some(path)) (Line 273). The actual write at Lines 270–271 always goes through open_report_dir(report_parent_dir) then report_dir.create(BUILD_PROFILE_REPORT) — path never touches the filesystem call. The existing test build_profile_report_respects_optional_target_and_writes_file (Line 320) happens to pass a report_target equal to the hidden convention path, so it cannot expose the mismatch.

Rust Compiler Lint Integrity / part of Architectural Complexity — still broken.
Module-level #[expect] blanket-suppressions remain on entire mod tests blocks:

• chutoro-benches/benches/neighbour_scoring.rs Line 131 — #[expect(unused_imports, ...)]
• chutoro-benches/benches/neighbour_scoring/support.rs Line 277 — #[expect(unused_imports, ...)]
• chutoro-benches/benches/neighbour_scoring/profiling.rs Line 112 — #[expect(dead_code, ...)]

These cover every item in the module rather than the specific unused import or item. Note: I found no #[expect(clippy::...)] anywhere in chutoro-benches, so the "invalid clippy expectations" phrasing in the Architectural Complexity finding maps onto these same rustc-lint module-wide expects, not a separate clippy issue.

Testing (Overall) — still not resolved.
neighbour_scoring_impl (the report-generation-plus-dimension/bucket-loop orchestration) and write_build_profile_report (the fixed-point-count public wrapper) have no direct test coverage. A skipped dimension or bucket in the loop, or a wrong report path passed through, would currently pass CI silently.

None of this is out of scope — fix it directly in this PR.

AI coding-agent prompt

In chutoro-benches, fix the neighbour_scoring bench harness report-target handling, narrow the module-wide lint expectations, and add orchestration test coverage. Validate against the current codegraph before editing.

1. chutoro-benches/benches/neighbour_scoring/support.rs — write_build_profile_report_for_point_counts
   - The function currently ignores the `path` derived from `report_target` when creating the file: it always writes via `open_report_dir(report_parent_dir).create(BUILD_PROFILE_REPORT)`.
   - Fix: derive the write path from `report_target` itself. Split `path` into its parent directory and filename (e.g. via `Utf8Path::parent()`/`Utf8Path::file_name()`, or `Dir::open_ambient_dir` against the parent), and create the file there instead of hard-coding `report_parent_dir`/`BUILD_PROFILE_REPORT`.
   - Alternatively, if the two-argument (`report_parent_dir`, `report_target`) shape must stay, add a `debug_assert_eq!(path, report_path_value(report_parent_dir, BUILD_PROFILE_REPORT))` immediately after destructuring `report_target`, so any caller supplying a mismatched target fails loudly in debug builds instead of silently writing to the wrong location.
   - Add a regression test in the same `#[cfg(test)] mod tests` block that supplies a `report_target` under a *different* directory or filename than `report_path_value(report_parent_dir, BUILD_PROFILE_REPORT)` would produce, and asserts either that the file is written at the supplied target, or (if using the debug_assert approach) that the mismatch panics/fails in a debug build.

2. Narrow the lint expectations:
   - chutoro-benches/benches/neighbour_scoring.rs Line 131: replace the module-level `#[expect(unused_imports, ...)]` on `mod tests` with per-import handling — remove genuinely unused imports, or move `#[expect(unused_imports, reason = "...")]` onto the specific `use` statement that needs it.
   - chutoro-benches/benches/neighbour_scoring/support.rs Line 277: same treatment for its `mod tests` `#[expect(unused_imports, ...)]`.
   - chutoro-benches/benches/neighbour_scoring/profiling.rs Line 112: same treatment for its `mod tests` `#[expect(dead_code, ...)]` — attach it only to the specific unused struct/field/fn, not the whole module.
   - Keep the existing item-level `#[expect(dead_code, reason = "...")]` on `CandidateBucket::realistic_for_test` (support.rs Line 96) unchanged; it is already correctly scoped.

3. Add orchestration coverage:
   - chutoro-benches/benches/neighbour_scoring/support.rs: extract the `(10_000, 100_000)` point counts and `128` dimension currently inlined in `write_build_profile_report` into named `pub(super) const` values (e.g. `DEFAULT_BUILD_PROFILE_POINT_COUNTS: &[usize]` and `DEFAULT_BUILD_PROFILE_DIMENSION: usize`), and have `write_build_profile_report` reference them. Add a test asserting these constants have the expected values, and (separately) that `write_build_profile_report_for_point_counts` is what `write_build_profile_report` delegates to — refactor `write_build_profile_report` if needed so this delegation is directly assertable without running a full 100k-point HNSW build in a unit test.
   - chutoro-benches/benches/neighbour_scoring.rs: extract the nested `for &dimension in DIMENSIONS { for bucket in all_buckets() { ... } }` enumeration in `neighbour_scoring_impl` into a small pure function, e.g. `fn scoring_plan() -> Vec<(usize, CandidateBucket)>`, that `neighbour_scoring_impl` then iterates over. Add a test asserting `scoring_plan()` yields `DIMENSIONS.len() * all_buckets().count()` entries and covers every `(dimension, bucket)` pair exactly once, so a future accidental loop truncation fails a test rather than silently under-benchmarking.

Constraints:
- Do not change the public report-writing behaviour when `report_target` correctly matches the hidden convention (the common case).
- Do not run full-scale (100k-point) HNSW builds inside fast unit tests; keep any new build-profile tests to small `point_counts` such as `&[16]`.
- Keep existing tests passing; extend rather than replace them.