Conversation
Refs #174. criterion 0.5.1 (May 2023) is ~3 years stale; bump to the current stable line (resolves to 0.8.2 today). The only API break in the 0.5 → 0.8 span affecting our benches is the removal of `criterion::black_box` (deprecated in 0.6, gone in 0.7+); switch the three bench files to `std::hint::black_box` and keep the rest of the imports unchanged. - crates/core/Cargo.toml: bump dev-dep to `criterion = "0.8"` - crates/core/benches/{marker,feature_map,simd}_bench.rs: import `std::hint::black_box`; drop `criterion::black_box` - docs/design/issue-174-criterion-upgrade.md: design doc from the `/brainstorming` session that scoped this change Verified: `cargo build --benches --all-features`, `cargo fmt`, CI's `cargo clippy --workspace -- -D warnings`, and `cargo test --all-features` (463 tests) all green. All three benches produce output under criterion 0.8. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Refs #174. Re-runs the SIMD benchmark suite under criterion 0.8 on the same hardware (Windows x86_64) and updates the medians + speedup column. Scalar medians compare cleanly against the pre-upgrade table (<15% machine variance), confirming no measurement-methodology drift from the 0.5 → 0.8 jump. - Refresh "SIMD Performance" table with new medians (rgba_to_gray 2.64x, dot_product 5.42x, box_filter_v 3.03x, box_filter_h 1.07x) - Pin "Tooling" line to `criterion 0.8` - Add toolchain note and the `RUSTFLAGS="-C target-feature=+sse4.1"` reminder so future readers can reproduce the SIMD numbers without hunting for the gate Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…atures (#180) PR 1 of 5 in the #180 series to close the gap between CI's clippy gate and `cargo clippy --all-targets --all-features -- -D warnings`. Local baseline drops from 60 errors to 43; remaining lints partition cleanly into the PR 2-5 backlog (FFI shims, field-reassign-with-default, SIMD too_many_arguments, CI tightening). Trivial mechanical fixes only — zero behavior change: - ar/image_proc.rs: silence unused-vars in luma_hist_and_box_filter_with_bias (variables used only under specific target_feature gates); convert manual range check to (100..200).contains(&thresh). - ar/math.rs, ar/pattern.rs: move items above the test module so clippy::items_after_test_module is satisfied. - ar/matrix.rs, ar/pattern.rs, ar2/feature_map.rs, ar2/image_set.rs: convert index-only loops to iterator / enumerate forms (clippy::needless_range_loop). - ar/bch.rs: drop no-op `as i32` cast. - arlog.rs: drop needless `fn main` in doctest. - version.rs: use char-array split pattern. - kpm/freak/homography.rs: drop needless `*pt` deref. - kpm/freak/math.rs: convert manual inclusive range to (0.0..=360.0).contains. Adds .claude/issue-180-plan.md with the brainstormed 5-PR plan, Decision Log, assumptions, and risks for the remaining series.
Moving `mat_mul_dff`, `mat_mul_fff` (math.rs) and the `dot_product*` family (pattern.rs) above their respective test modules in the previous commit tripped codecov/patch (0% diff coverage, target 54.63%) — the code is byte-identical, but the new line positions registered as additions, exposing a pre-existing gap that none of these helpers had direct unit tests. Add minimal scalar tests: - mat_mul_dff: identity case + known-values case (translation column). - mat_mul_fff: identity case + composed-translation case. - dot_product_scalar: known values, all-zero input, negative values. - dot_product (dispatcher): asserts SIMD/scalar paths agree at runtime. SIMD variants (dot_product_simd_wasm, dot_product_simd_x86) remain covered only via the dispatcher equivalence test and the existing pattern-matching integration tests; direct cfg-gated SIMD unit tests are out of scope for #180 PR 1 and tracked separately.
PR 2 of 5 in the #180 series. Clears all 17 dead `webarkit_cpp_*` extern declarations flagged by `cargo clippy --all-targets --all-features -- -D warnings` (baseline 43 → 24 errors). Root cause: each extern block was gated `#[cfg(feature = "dual-mode")]` but its only callers live in `#[cfg(all(test, feature = "dual-mode"))]` modules in the same file. When clippy compiles the lib target with `--all-features`, dual-mode is on but `test` is not — so the extern is present, callers are gone, and the dead-code lint fires legitimately. Fix: tighten each block to `#[cfg(all(test, feature = "dual-mode"))]`, matching its caller exactly. The extern now exists only when its caller does, the lint disappears, and dual-mode test binaries are unchanged. Files: - kpm/freak/clustering.rs — fast_random/array_shuffle/bhc_build_and_query - kpm/freak/homography.rs — mat3_exp_pade_via_eigen/preemptive_/robust_ - kpm/freak/hough.rs — auto_adjust_xy_num_bins - kpm/freak/math.rs — fast_atan2/sqrt1/exp6 + solve_* + partial_sort - kpm/freak/matcher.rs — match_features_brute/indexed/guided Plan doc: Decision Log #2 updated to record the switch from the originally-planned `#[allow(dead_code)]` + rationale approach to the cfg-tightening approach, with the diagnostic evidence that drove it. Verified: - cargo clippy --all-targets --all-features -- -D warnings: 17 shim lints clear; remaining 24 errors all in PR 3-4 scope. - cargo test --all-features: 471 passed, 8 ignored. - cargo test -p webarkitlib-rs --lib --features dual-mode: 445 passed. - cargo clippy --workspace -- -D warnings (existing CI gate): clean. - cargo clippy -p webarkitlib-rs -- -D warnings (pure-Rust gate): clean.
…nit (#180) PR 3 of 5 in the #180 series. Clears all 22 `field_reassign_with_default` lints flagged by `cargo clippy --all-targets --all-features -- -D warnings` (baseline 24 → 2; the remaining 2 are the SIMD `too_many_arguments` pair earmarked for PR 4). Mechanical conversion across 4 files: - ar/marker.rs — 17 sites (ARMarkerInfo: 6 sites, ARHandle: 11 sites). Tests for finalize_marker_id_cf_dir and history-merge/resurrection pipelines. - ar/param.rs — 3 sites (ARParam in ar_param_change_size tests). - ar/param_gl.rs — 1 site (ARParam in make_param helper). - ar/pattern.rs — 1 site (ARMarkerInfo in ar_patt_save test). All sites are test-only code; #180 noted the lint as pre-M9 churn, nothing pre-existing breaks. Where the original code interleaved non-field reassigns (e.g. `src.mat[0] = [...]` array indexing) the let-binding stays `mut` so subsequent index assignments still compile. Verified: - cargo clippy --all-targets --all-features -- -D warnings: 22 lints clear; remaining 2 errors are the documented PR 4 SIMD sites. - cargo test --all-features: 471 passed, 8 ignored. - cargo clippy --workspace -- -D warnings (existing CI gate): clean. - cargo clippy -p webarkitlib-rs -- -D warnings (pure-Rust gate): clean.
…180) PR 4 of 5 in the #180 series. Adds `#[allow(clippy::too_many_arguments)]` with rationale on the two SIMD `get_similarity_*` impls in `crates/core/src/ar2/feature_map.rs` (lines 275 and 387), clearing the final two lib lints flagged by `cargo clippy --all-targets --all-features -- -D warnings`. These functions are runtime-dispatched SIMD variants of the scalar `get_similarity`. Their 9-argument signatures are locked: they must match the scalar fallback and each sibling SIMD impl so the dispatcher can swap between them via `is_x86_feature_detected!`. Bundling args into a struct would force every caller to materialize it on a hot path with no design win. The plan (.claude/issue-180-plan.md Decision #3) called for an inline `#[allow]` over a `clippy.toml` threshold raise to keep the exception scoped to these two sites. Library is now fully strict-clean: - `cargo clippy -p webarkitlib-rs --lib --all-features -- -D warnings`: clean - `cargo clippy --workspace -- -D warnings` (existing CI gate): clean Surfaced during verification: clearing the lib unmasked 24 pre-existing lints in examples/tests/benches that had been hidden because cargo halts on lib errors before checking those targets. They are out of scope for PR 4 and will be addressed in a follow-up cleanup before PR 5 tightens CI to `--all-targets --all-features`.
PR 4.5 of the #180 series. Clears all clippy lints in non-library targets that were previously masked by lib compilation halting cargo on errors. After PR 4 made the library strict-clean, these surfaced under `cargo clippy --all-targets --all-features -- -D warnings`. Workspace is now fully strict-clean — PR 5 can safely flip CI to the `--all-targets --all-features -- -D warnings` invocation. ## Categories cleared | Category | Count | Files | |---|---:|---| | `excessive_precision` (file-scoped allow + rationale) | 26 | tests/kpm_regression.rs | | `field_reassign_with_default` | 10 | examples/{barcode,dump_patt,generate_patt,simple}.rs + benches/marker_bench.rs | | `unnecessary_cast` (usize→usize) | 7 | examples/generate_patt.rs | | `or_fun_call` (call inside expect) | 4 | examples/{debug_labeling,simple}.rs | | `needless_range_loop` | 5 | examples/{dump_patt,simple_nft,simple_nft_dual}.rs | | `ptr_arg` (&Vec → &[_]) | 3 | examples/generate_patt.rs | | `redundant_closure` | 3 | examples/generate_patt.rs | | `unnecessary_lazy_evaluations` | 1 | examples/simple_nft_dual.rs (.then → .then_some) | | `manual_is_multiple_of` | 1 | examples/generate_patt.rs | | `manual_range_contains` | 1 | tests/nft_pipeline.rs | | `let_unit_value` | 1 | benches/marker_bench.rs (let _ = .unwrap()) | | `redundant_pattern_matching` | 1 | examples/barcode.rs (if let Ok(_) → .is_ok()) | | `type_complexity` | 1 | examples/generate_patt.rs (RunReport type alias) | ## Notes - **tests/kpm_regression.rs**: file-scoped `#![allow(clippy::excessive_precision)]` with rationale rather than truncating each of 26 baseline floats. The literals were captured verbatim from C++ `kpm_dump_fixtures`; truncating would round-trip to the same f32 but lose source-traceability to the baseline. - **examples**: `ARHandle`/`ARParamLTf`/`ARPattHandle` struct-init conversions follow the PR 3 pattern. Method-call mutations (`set_pixel_format`, etc.) stay outside the initializer. - **examples/simple_nft\*.rs**: `for r in 0..3 { cam_pose[r][i] }` → `for row in cam_pose { row[i] }`. - **examples/generate_patt.rs**: introduced `type RunReport = (...)` alias to clear `type_complexity` on `run_once` signature. ## Verified - cargo fmt --all -- --check: clean. - cargo clippy --all-targets --all-features -- -D warnings: No issues found. - cargo clippy --workspace -- -D warnings (existing CI gate): clean. - cargo clippy -p webarkitlib-rs -- -D warnings (pure-Rust gate): clean. - cargo test --all-features: 471 passed, 8 ignored. - Regression smoke tests on marker pipelines: - examples/simple: Hiro template match, CF=0.8925, ICP=0.0841 (identical to PR 3). - examples/barcode: matrix code ID=5, CF=1.0 (identical to PR 3).
The codecov/patch gate fails on PRs touching examples/ or benches/ because cargo-llvm-cov (and tarpaulin) don't instrument example binaries or bench harnesses by default. When a cleanup PR touches those paths (#180 PR 4.5: 137 lines in examples/, 8 in benches/, 8 in tests/), the diff shows ~0% coverage and the patch threshold trips even though the library and integration tests are unaffected. Add a project codecov.yml that: - Ignores crates/core/examples/**, crates/core/benches/**, crates/wasm/**, and benchmarks/**. These are demonstration, perf-measurement, or build-system targets — not the library under test. - Sets project + patch targets to `auto` with a 1% slack threshold, so genuine regressions in src/ or tests/ still fail the gate but PRs don't fail on float-noise coverage swings. The library (crates/core/src/) and integration tests (crates/core/tests/) remain instrumented. A real regression there will still fail codecov, which is what we want.
Two coordinated fixes for the codecov/patch failure on PRs that touch files the coverage runner doesn't instrument: 1. .github/workflows/coverage.yml: the existing `--exclude-files "tests/*"` glob is workspace-root relative and never matched our tests at crates/core/tests/*.rs. tarpaulin was silently instrumenting them, then reporting 0% on lines that the coverage workflow's default-features test run never executes (e.g. tests gated on --features ffi-backend). Change to `**/tests/*` so the exclusion actually fires. 2. codecov.yml: mirror tarpaulin's exclusion by adding crates/core/tests/** to the ignore list. Without this, codecov still sees the tests/ paths in the diff, finds no coverage data, and counts every added line as missed in the patch report. (Codecov treats "file in diff with no report data" as 0% covered unless the file is on the ignore list.) Also revert the patch threshold-100% workaround from the previous commit — with the two fixes above, the patch gate stays meaningful (target: auto, threshold: 1%) and only counts real src/ changes. Surfaced during #180 PR 4.5.
PR 5 of 5 — finale of the #180 series. Locks in everything PRs 1-4.5 cleaned up by flipping the `build-and-test` job's clippy step from `cargo clippy --workspace -- -D warnings` to the strict variant: cargo clippy --workspace --all-targets --all-features -- -D warnings What this catches that the old gate didn't: - Lints in examples/, tests/, and benches/ (--all-targets). - Lints in ffi-backend, dual-mode, and simd-* code paths (--all-features) — the original ~70-lint gap on dev that #180 was opened to close. The `pure-rust-build` job keeps its current `cargo clippy -p webarkitlib-rs -- -D warnings` invocation deliberately. That job exists to prove the no-features build stays clean (no accidental dependency on ffi-backend / dual-mode leaks into the pure-Rust path) — a different invariant from the strict gate, so it shouldn't be conflated. Also updates .claude/issue-180-plan.md to document PR 4.5 (the unplanned cleanup PR that cleared 26 examples/tests/benches lints unmasked once the lib went strict-clean in PR 4). Series totals (after this lands): 60 lib lints + 26 non-lib lints cleared across 5 PRs, plus a coverage workflow bug fix unrelated to clippy. CLAUDE.md §5's strict command is now enforced by CI.
First push of PR 5 added the strict gate to `build-and-test`, which failed because that job's checkout deliberately omits git submodules (it's the pure-Rust path verification). `--all-features` enables `ffi-backend`, which makes `build.rs` panic looking for the WebARKitLib C++ sources at crates/core/third_party/WebARKitLib/lib/SRC/KPM/FreakMatcher. Move the strict clippy step to `kpm-build (ubuntu-latest)` where the prerequisites (submodules + libclang-dev) already exist for the ffi-backend build / dual-mode tests. The step is Linux-only since lints are OS-agnostic — running it once is sufficient. `build-and-test` keeps its `cargo clippy --workspace -- -D warnings` step. The three clippy invocations in CI now correspond to three distinct invariants: - build-and-test (no submodules, default features, all crates) — proves the workspace stays clean against the lints any contributor sees by default. - pure-rust-build (single-crate, no features) — proves the pure-Rust path never accidentally depends on ffi-backend. - kpm-build ubuntu (submodules, --all-targets --all-features) — proves examples/tests/benches AND ffi-backend/dual-mode/simd-* paths all stay strict-clean. This is the #180 gate. Plan doc updated to record the relocation and the diagnostic that drove it.
The #180 cleanup series surfaced patterns worth codifying for both contributors and AI assistants: - CLAUDE.md gains §7 "Codebase clippy conventions (#180 lessons)" with six patterns (struct-init, FFI shim cfg alignment, items-above-tests, SIMD allow + rationale, fixture allow, re-run after clean), each linked to the PR that established it. - .agents/instructions.md gains Phase 4 covering the same patterns briefly, referencing CLAUDE.md §7 for the full text. Also un-ignore .agents/ from .gitignore and commit the rules / skills tracked under it, so the Antigravity agent rules and the license-header-adder skill are shareable. roadmap.md and gemini_context.md stay untracked pending a content review (they were marked obsolete). These conventions weren't taught by clippy's lint messages alone — they came out of the diagnostic work in PRs #184-189. Codifying them prevents the next contributor (human or AI) from recreating the lints we just cleared.
Adds a new `miri` CI job that runs `cargo +nightly miri test -p webarkitlib-rs --lib` to catch undefined behavior in the freshly- ported KPM/FREAK/AR/AR2 code: use-after-free, OOB reads, invalid `unsafe` invariants, uninitialized-memory reads, and reference-aliasing violations that regular tests miss. Scope decisions (full rationale in docs/design/issue-182-miri-plan.md): - `continue-on-error: true` initially; ratchet to required after UB fix PRs land. Mirrors the #180 strict-clippy ratcheting pattern. - PR-scoped via job-level `if:` guard (dev/main only) — Miri is 5-30x slower than `cargo test`, so push-to-branch runs add cost without value. - Pinned nightly date inline (`MIRI_NIGHTLY`) so a bad nightly can't red the job for reasons unrelated to our code. - Excludes `ffi-backend` (Miri can't execute C++) and `simd-x86-*` (Miri's x86 SIMD support is incomplete). Scalar fallbacks ARE validated; SIMD parity is covered by existing scalar/SIMD tests. - `-Zmiri-strict-provenance` enabled to catch pointer-provenance bugs loose provenance silently allows. Adds docs/miri.md covering scope, exclusions, local-run instructions, failure investigation, the nightly-pin bump procedure, and the continue-on-error -> required gate ratchet plan. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…182) First CI run surfaced that strict provenance fires inside crossbeam-epoch (a transitive dep of rayon) when ar2::feature_map's par_chunks_mut spins up the thread pool. The dep uses integer-to-pointer casts internally and hasn't migrated to the Strict Provenance APIs. Strict provenance against unmigrated third-party deps produces failures we cannot fix here. Keep regular Miri (the core UB net) and revisit strict provenance once the ecosystem catches up. Developers can still spot-check their own unsafe code locally with the flag. docs/miri.md updated with the rationale. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ive (#182) Second CI run surfaced a Stacked Borrows aliasing error entirely inside crossbeam-epoch -> rayon (called from ar2::feature_map par_chunks_mut). The backtrace never enters our code; the violation is a known incompatibility between Miri's default Stacked Borrows model and crossbeam's internal patterns (crossbeam-rs/crossbeam#545 family). Switch CI to the newer Tree Borrows aliasing model (-Zmiri-tree-borrows), which accepts crossbeam's patterns while still catching real UB in our own unsafe code. docs/miri.md updated to explain the choice and how to spot-check our own code under Stacked Borrows locally if desired. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
) Third CI run surfaced an isolation error: ar2::feature_set roundtrip uses tempfile, which calls `open()` — blocked by Miri's default isolation. Add -Zmiri-disable-isolation so disk I/O passes through to the host. This is the standard Miri configuration for test suites that include filesystem-touching tests; it does not weaken the UB checks. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Miri (CI job from #182) flagged `hamming_distance_96` for UB: transmuting `&[u8; 96]` to `&[u32; 24]` requires 4-byte alignment on the input, which `&[u8; 96]` does not guarantee — the byte array only needs 1-byte alignment, and Miri produces a case where it isn't. Replace the transmute with `u32::from_ne_bytes` on 4-byte chunks. This removes the `unsafe` entirely while producing identical numerical output (the same 32-bit popcount summed over the same 24 words). On aligned inputs the codegen is equivalent to the transmute version; on unaligned inputs it is now correct instead of UB. Verified locally: the full library test suite (`cargo test -p webarkitlib-rs --lib`) is green (419 passed, 2 ignored). The C++ parity is preserved because the byte order, word count, and hamming_distance_32 reduction are unchanged. Audit: grep across the whole `crates/` tree confirms this was the only `transmute::<&[u8; ...], &[u32; ...]>` site — no further unaligned-transmute fixes needed in `kpm/freak/{math,matcher, visual_database}.rs`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…re) (#194) The Miri CI job added in #191 was hitting 1-2 hour runtimes on PRs. Root cause: ~14 unit tests run *full pipelines* (BHC tree construction over hundreds of descriptors, DoG keypoint detection on real benchmark images, full ar2_gen_feature_map runs). Native `cargo test` finishes them in milliseconds via compiled code + rayon parallelism; Miri interprets MIR single-threaded, so each test takes 10-40 minutes under interpretation -- and none of them exercise `unsafe` boundaries that targeted unit tests don't already cover. Annotate them with `#[cfg_attr(miri, ignore)]`. The tests still run under normal `cargo test`; they are skipped only under `cargo miri test`. Targeted unit tests on the actual `unsafe` surface (hamming_distance_*, descriptor pack/unpack, byteorder reads, image_proc indexing) stay enabled -- those are what Miri is meant to validate. Tests annotated: * clustering.rs: test_bhc_max_nodes_to_pop_widens_candidate_set (200 descriptors x2 builds; the test that hung CI on #191), test_bhc_max_nodes_to_pop_zero_is_tied_min_only, test_bhc_set_num_centers_preserves_num_hypotheses, test_bhc_set_num_hypotheses_then_build (80 descriptors each). * detector.rs: test_dog_detector_finds_keypoints_on_real_image, test_dog_detector_keypoints_within_image_bounds. * keyframe.rs: test_find_features_populates_keyframe, test_keyframe_build_index_populates_index, test_keyframe_build_index_is_idempotent. * visual_database.rs: test_visual_database_add_image_builds_index, test_visual_database_add_keyframe_builds_index_when_absent, test_visual_database_add_keyframe_preserves_caller_built_index. * feature_map.rs: test_feature_map_small_synthetic, test_feature_map_mindpi_maxdpi. (test_feature_map_produces_points already has #[ignore], skipped.) docs/miri.md updated: new section explaining the convention and how to run a Miri-ignored test locally for investigation. Verified locally: cargo test -p webarkitlib-rs --lib still passes all 419 tests (cfg_attr only affects Miri runs). Expected effect: Miri CI runtime drops from 1-2 hours to ~10 minutes, restoring per-PR Miri as a practical safety net. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
First Miri run on PR #195 still hit GitHub Actions' 6-hour ceiling. Timestamps in the log show clustering tests finished in 15 seconds (annotations worked), but kpm/freak/descriptor.rs tests took ~2.5h each — they load found.jpg and run FREAK extraction. Original audit missed the `load_grayscale()` pattern. This commit annotates every remaining test that touches found.jpg / img.jpg: * kpm/freak/descriptor.rs: 5 tests (test_freak_descriptor_length_one_keypoint and 4 siblings) * kpm/freak/visual_database.rs: 4 tests (test_visual_database_add_and_query_same_image, query_different_image_returns_no_match, add_same_id_returns_err, erase_removes_keyframe) * kpm/rust_backend.rs: 3 tests (test_rust_freak_matcher_implements_backend, extract_features, add_freak_features) Audit method this time: `grep -rn load_grayscale` across the whole crate to enumerate every real-image test before annotating, so we catch them all in one pass. test_dual_mode_no_divergence_on_pinball stays unannotated — already cfg-gated to `dual-mode` which Miri CI does not enable. Verified locally: cargo test -p webarkitlib-rs --lib still passes all 419 tests. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PR #195's Miri run failed on `test_cauchy_cost_known_values`: assertion failed: approx_eq(c2, 26.0_f32.ln(), 1e-6) This is not UB. Miri intentionally makes `f32::ln` non-deterministic by a few ULPs across calls within a single run. The test computes `.ln()` twice — once inside `cauchy_cost_2d`, once on the assert's RHS — and the two values can diverge by more than the 1e-6 native tolerance under Miri. Widen the tolerance to 1e-5, which: * still tests mathematical correctness (f32 has ~7 decimal digits of precision at value 3.26 — well within 1e-5), * keeps the test running under both native cargo test AND Miri (vs. the alternative of `#[cfg_attr(miri, ignore)]` which would shrink Miri's coverage), * documents in-source why the tolerance is wider than the sibling scalar test (which only calls .ln() once and is fine at 1e-6). Audit: this is the only homography.rs assertion that compares .ln()/.sin()/.sqrt() outputs at a tolerance tight enough to trip Miri's transcendental non-determinism. Other Cauchy tests assert exact zero (test_cauchy_cost_zero_residual, the projective tests), which is FP-impl-independent. Verified locally: 42 homography tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PR #195's Miri run failed at process teardown: error: the main thread terminated without waiting for all remaining threads This is rayon's global thread pool: it spawns worker threads up-front and never joins them. When the test binary exits, those threads are still alive, and Miri's default leak check flags it. Add -Zmiri-ignore-leaks. This is the standard configuration for any rayon-using test suite (the rayon docs and Miri docs both recommend it). It does NOT weaken UB detection on the heap-allocation / aliasing / provenance side — only the leak check is disabled. After this, PR #195's Miri job should finally go green end-to-end. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…182) Final step in the #182 plan. The Miri safety net is now green end-to- end (PR #195 closed out the runtime issues and the one real UB finding in #192/#193), so flip it from advisory to required. Changes: * Split the Miri job from `.github/workflows/ci.yml` into its own workflow file `.github/workflows/miri.yml`. This gives it a dedicated status badge (GitHub workflow badges are workflow-scoped, not job- scoped) and lets the trigger rules live at workflow level instead of inside an `if:` guard. * Drop `continue-on-error: true` — Miri failures now block PR merges, same as every other CI job. * Move the trigger from the job-level `if:` to a workflow-level `on:` block targeting pushes and PRs to `dev`/`main`. * Add a Miri status badge to README.md, next to the existing CI badge. * Update `docs/miri.md`: reference the new workflow path, refresh the "CI gate status" section to describe the required-gate state, and reference #192 as the first real UB the net caught. No code changes — workflow + docs only. `cargo fmt --check` clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Now that `protect-main` / `protect-dev` rulesets mark the Miri job as a required check, docs-only PRs (README typos, docs/ updates, asset swaps) would block at merge because the real Miri workflow's `paths-ignore` makes the check "missing." Branch protection can't tell the difference between "skipped because nothing to validate" and "didn't run for some other reason." GitHub's documented workaround: a companion workflow that declares the same `name:` and the same job `name:`, triggers on exactly the paths the real workflow ignores, and emits a passing check. Branch protection matches by name and is satisfied either way. Changes: * miri.yml: add `paths-ignore` excluding `**/*.md`, `docs/**`, `assets/**`, `LICENSE*`, `.gitignore`, `.gitattributes`. * miri-skip.yml: new file. Same `name: Miri` and job `name: miri (pure-Rust UB validation)`. Triggers on the inverse path list. Single step that just `echo`s a "skipped — docs only" message. * docs/miri.md: document the shim pattern and the "keep both path lists in sync" rule. Effect: README-only PRs no longer spin up the nightly toolchain (saves ~5 minutes of CI per docs PR) while still satisfying the required- check rule. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…zation (#209) * perf(kpm): criterion benchmark for Gaussian scale-space pyramid (#200) Establish the scalar baseline for the Gaussian pyramid SIMD series (#200 -> #201 -> #202). Unlike the BoxFilterPyramid8u trilogy (#131-133, dead code per #203), GaussianScaleSpacePyramid is the pyramid the KPM/FREAK detector actually uses via DoGScaleInvariantDetector. - Promote the three hot helpers to `pub` with scalar-only dispatchers, following the `pyramid::downsample` precedent from #131: binomial_4th_order_u8_to_f32{,_scalar} binomial_4th_order_f32_to_f32{,_scalar} downsample_bilinear_f32{,_scalar} `build` calls the dispatchers, so #201/#202 only fill in SIMD variants and wire dispatch. Behavior unchanged (dual-mode C++ parity test still green). - Add crates/core/benches/gaussian_pyramid_bench.rs benchmarking each scalar helper plus end-to-end `build` (num_octaves from num_octaves_for(w, h, 8), matching rust_backend) at 640x480, 1280x720 and 1920x1080 with fixed-seed PRNG input. - Wire `[[bench]] name = "gaussian_pyramid_bench"` into Cargo.toml. Baseline (median, this machine): binomial_f32 640x480 710 us | 1280x720 2.04 ms | 1920x1080 5.26 ms binomial_u8 640x480 546 us | 1280x720 1.60 ms | 1920x1080 3.38 ms bilinear 640x480 138 us | 1280x720 381 us | 1920x1080 1.16 ms build (e2e) 640x480 3.11 ms | 1280x720 9.95 ms | 1920x1080 26.0 ms Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(kpm): parallelize Gaussian pyramid filter passes with rayon (#207) (#208) The binomial filter and bilinear downsample of GaussianScaleSpacePyramid are memory-bandwidth bound, so the SIMD attempt (#201/#206) gave ~1.0× — a single core can't saturate DRAM bandwidth. Multiple cores can, so parallelize over rows with rayon (native; wasm stays single-threaded). - Per-row helpers (binomial_{h,v}_row_{f32,u8}, bilinear_row_f32) are the single source of truth for the arithmetic; serial and rayon paths both call them, so output is bit-for-bit identical regardless of threading (rows are independent). Validated by serial-vs-rayon parity tests + the existing dual-mode C++ parity test (scalar == C++, rayon == scalar). - Added *_rayon variants (par_chunks_mut over rows) for both binomial variants and the bilinear downsample, cfg(not(wasm32)). - Dispatchers pick rayon above PARALLEL_MIN_PIXELS (600k px), else serial; wasm always serial. The threshold keeps <=480p serial (where thread overhead regressed the binomial filter) and parallelizes 720p+. - Bench extended with a `rayon` arm per group. Speedup (scalar -> rayon, same-run medians, this machine): binomial_f32 720p 2.75->1.71 ms (1.61x) | 1080p 4.33->2.84 ms (1.52x) binomial_u8 720p 1.69->1.42 ms (1.19x) | 1080p 3.55->2.36 ms (1.50x) bilinear 720p 0.40->0.17 ms (2.36x) | 1080p 1.01->0.57 ms (1.77x) (<=480p stays serial by design.) Closes #207. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * test(kpm): skip large rayon-parity tests under Miri; drop redundant h alias The #207 rayon parity tests build pyramids at up to 800×800 — astronomically slow under Miri's interpreter (the run was stuck for 2h). #207 adds no unsafe (rayon is safe), so Miri has nothing to validate there. Annotate the four large-image tests with #[cfg_attr(miri, ignore)], matching the existing #194 idiom; they still run natively. Also replace the redundant `let h = height;` alias in binomial_v_row_{f32,u8} with `height` directly — the elided binding showed as 2 uncovered lines in the tarpaulin/codecov patch report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…#211) * perf(kpm): add criterion benchmark for Pyramid downsample (#131) Establish the scalar baseline for the pyramid downsample trilogy so the SIMD path (#132) and any chunked-layout work (#133) can prove speedups against stable numbers. - Promote the private `downsample` to `pub fn downsample_scalar` and add a `pub fn downsample` dispatcher (scalar-only for now; #132 wires in the SIMD paths). Both are `pub` so the criterion bench (a separate crate) can measure them directly; `Pyramid::build` is the stable API. - Add `crates/core/benches/pyramid_bench.rs` benchmarking `downsample_scalar` and `Pyramid::build` (num_levels=4) at 640x480, 1280x720 and 1920x1080 with a fixed-seed PRNG input. - Wire `[[bench]] name = "pyramid_bench"` into Cargo.toml. Baseline (median, this machine): downsample 640x480 ~116 us | 1280x720 ~341 us | 1920x1080 ~884 us pyramid_build 640x480 ~159 us | 1280x720 ~597 us | 1920x1080 ~1.87 ms Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(kpm): SIMD path for Pyramid downsample (AVX2/SSE4.1/wasm32) (#132) Add runtime-dispatched SIMD implementations of the pyramid downsample, each byte-identical to the scalar baseline established in #131. - `downsample_avx2`: 32 output cols/iter via 256-bit lanes (`maddubs` horizontal pairwise add + `packus` + `permute4x64` to undo the per-lane interleave). - `downsample_sse41`: 16 output cols/iter via 128-bit lanes. - `downsample_wasm`: 16 output cols/iter via `simd128` (`extadd_pairwise_u8x16` + `narrow`). - `downsample` dispatches AVX2 -> SSE4.1 -> scalar on x86_64 (runtime `is_x86_feature_detected!`) and simd128 on wasm32; scalar remains the fallback. Each `unsafe` site carries a `// SAFETY:` comment. - Shared `downsample_dims` + `downsample_row_tail_scalar` helpers define the rounding in one place; the SIMD main loops handle full blocks and defer the per-row remainder to the scalar tail. - Block counts are sized so loads/stores stay in bounds for any dimension (incl. odd / sub-block widths). Correctness: property tests compare each SIMD path against scalar over 16 sizes (odd dims, tiny images, block boundaries) plus a dispatcher-parity test. `cargo clippy --all-targets --all-features -- -D warnings` clean. Speedup (downsample, median ns -> us, this machine): 640x480 scalar 85.8 | sse41 7.4 (11.6x) | avx2 5.8 (14.7x) 1280x720 scalar 251 | sse41 25.0 (10.0x) | avx2 18.6 (13.5x) 1920x1080 scalar 773 | sse41 96.1 (8.0x) | avx2 75.9 (10.2x) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(kpm): exhaustive width-sweep parity for SIMD downsample (#132) Add a 2..=160 column sweep (x4 row counts) comparing both the AVX2 and SSE4.1 paths against scalar, covering every remainder case of the 16- and 32-wide main loops. Strengthens confidence beyond the hand-picked sizes that each SIMD path is byte-identical to scalar for all dimensions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Decision on #203: keep BoxFilterPyramid8u / Pyramid as a tested reference rather than remove or wire it in. Add a module note making clear it is not wired into any pipeline (the live KPM/FREAK detector uses GaussianScaleSpacePyramid) so future readers aren't misled. Closes #203. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The benchmarks job built a C baseline by downloading external sources (jpeg-9f from ijg.org, WebARKitLib) at CI time; a TLS/network flake there (and a missing `svn`) failed the whole gate even though the Rust code was fine (#204). - Install `subversion` (the bootstrap warned `svn not found`). - Cache the downloaded C sources keyed on libraries.json hash. - Run the C baseline as a NON-BLOCKING step (`continue-on-error`) with a 3x retry on the flaky bootstrap download — it only yields an optional comparison log. - Keep `cargo bench` (marker_bench) as the required gate signal. Now a transient external-download failure degrades gracefully instead of failing the benchmarks gate. Closes #204. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Build hygiene for the post-M9-3 pure-Rust wasm target: - Make `cc` and `bindgen` optional build-dependencies, enabled only by the `ffi-backend` feature (`dep:cc`, `dep:bindgen`); gate their build.rs usage with `#[cfg(feature = "ffi-backend")]`. The default / wasm32 build now pulls no C/C++ toolchain crates (verified via `cargo tree --target wasm32-unknown-unknown`). The FFI backend still builds. - Gate `PARALLEL_MIN_PIXELS` with `#[cfg(not(target_arch = "wasm32"))]` — it is only used by the rayon dispatchers (already wasm-gated from #207), so it was dead code on wasm and warned there. Verified: `cargo build --target wasm32-unknown-unknown -p webarkitlib-wasm` clean (no warnings, no bindgen/cc); `cargo build -p webarkitlib-rs --features ffi-backend` still compiles the C++; `cargo clippy --all-targets --all-features -D warnings` clean. Refs #161. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sset (#161 goals 2,3) - Logger (goal 2): enable `log-helpers` by default on the wasm crate and call `ar_log_init_wasm()` from `init_wasm()` so `arlog_*!` output reaches the browser console via `console_log`. - SIMD wiring (goal 3): validated the `+simd128 --features simd` wasm build (the build-dual.sh "SIMD" variant). Silenced two pre-existing unreachable-fallback warnings surfaced by that build in the `rgba_to_gray` and `dot_product` dispatchers (`#[allow(unreachable_code)]`, matching the pyramid dispatchers) — wasm `+simd128` build is now warning-free. - Add `crates/wasm/www/assets/pinball.fset3` (KPM reference data; required by detection — was missing from the demo assets). Refs #161. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The wasm NFT scaffolding only did AR2 tracking from a hardcoded pose — there
was no KPM detection binding. Add it so the browser demo runs the real
pure-Rust KPM detection (the simple_nft.rs steps 3a + 4).
- core: add `KpmRefDataSet::load_from_bytes` (refactor `load` to share a
reader-based parser) so `.fset3` can be loaded from fetched bytes in the
browser (no filesystem). Native parity test vs `load(path)`.
- wasm: add `WasmKpmHandle` — `new(param, w, h)` builds a `RustFreakMatcher`
+ `KpmHandle`; `load_ref_data(.fset3 bytes)`; `detect(rgba)` runs
`kpm_matching` and returns `{ pose:[12], page, error }` (or null).
- demo: `www/simple_nft_example.html` now fetches `pinball.fset3`, creates a
`WasmKpmHandle`, and the "Detect (KPM)" button runs real detection, prints
the 3x4 pose, and feeds it into AR2 tracking (replacing the hardcoded pose).
- test: headless `wasm-bindgen-test` (run via `wasm-pack test`) covering
construction + `.fset3` load + pre-load detect guard.
Verified in-browser (manual): KPM detection on pinball-demo.jpg yields
page 0, error 5.0879, and a 3x4 pose matching the native simple_nft
pipeline (~5.09 error) — the pure-Rust KPM->pose pipeline runs end-to-end in
the browser. Also: `cargo build --target wasm32-unknown-unknown -p
webarkitlib-wasm --tests` clean; `cargo clippy --all-targets --all-features
-D warnings` clean; native kpm tests green.
Closes #161.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…atureStore slices (#148) Mirror `VisualDatabaseFacade`'s Group-A accessors so the Rust port is a drop-in for the C++ facade surface, and callers don't have to loop `0..num_features()` element-by-element. Pure additive getters — no behavior change. - `FeatureStore`: add slice accessors `points() -> &[FeaturePoint]` and `descriptors() -> &[u8]` (counterparts to per-element `point(i)`/`descriptor(i)`). - `VisualDatabase`: add `get_feature_points(id)`, `get_descriptors(id)`, `get_query_feature_points()`, `get_query_descriptors()`, `get_width(id)`, `get_height(id)` (Option-returning, 1-liners over the slice accessors). - Tests for both (deterministic, no image pipeline). Group B (3D feature points — RustFreakMatcher/NFT domain) and Group C (computeFreakFeaturesAndDescriptors — no caller) are out of scope. Refs #148. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…_from_keyframe (#147) (#217) Recover part of the C++ query onion. Step 1 (no public API change): extract the per-keyframe matching loop into `match_against_database` and the per-query reset into `reset_query_state`. Step 2: expose `query_from_keyframe(Keyframe)` — runs only the matching loop against the database (skips pyramid build + feature extraction), for deterministic testing and callers that already hold a `Keyframe`. Mirrors C++ `query(const keyframe_t*)`. `query(&Matrix<u8>)` is now a thin wrapper (build keyframe → match → stash), behaviorally identical (existing query tests unchanged + green). Skipped per YAGNI: `query_from_pyramid` — KPM uses GaussianScaleSpacePyramid while AR2 uses BoxFilterPyramid8u, so there is no possible cross-pipeline caller (documented in #147). Test: query_from_keyframe self-matches an identical keyframe. Closes #147. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…218) * refactor(kpm): factor VisualDatabase::query matching loop + add query_from_keyframe (#147) Recover part of the C++ query onion. Step 1 (no public API change): extract the per-keyframe matching loop into `match_against_database` and the per-query reset into `reset_query_state`. Step 2: expose `query_from_keyframe(Keyframe)` — runs only the matching loop against the database (skips pyramid build + feature extraction), for deterministic testing and callers that already hold a `Keyframe`. Mirrors C++ `query(const keyframe_t*)`. `query(&Matrix<u8>)` is now a thin wrapper (build keyframe → match → stash), behaviorally identical (existing query tests unchanged + green). Skipped per YAGNI: `query_from_pyramid` — KPM uses GaussianScaleSpacePyramid while AR2 uses BoxFilterPyramid8u, so there is no possible cross-pipeline caller (documented in #147). Test: query_from_keyframe self-matches an identical keyframe. Closes #147. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(kpm): raise M9 coverage — exclude examples + edge tests (#177) - coverage: exclude `examples/**` from tarpaulin (the 30-line simple_nft_dual.rs diagnostic at 0% was the single biggest gap; examples are demos, not test code — the issue's preferred fix). - clustering: add a degenerate-input test (BHC build over all-identical descriptors → zero-distance k-medoids split + query). - visual_database: add a no-match test via query_from_keyframe (#147) — maximally-distant descriptors short-circuit try_match_one. Codecov on this PR confirms the resulting M9 module coverage; any residual per-file gaps (rust_backend DualFreakMatcher divergence, hough 1-liner) are quick follow-ups. Refs #177. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…backend (#179) `nft_marker_gen` now produces a COMPLETE NFT marker (.iset + .fset + .fset3) in a single pure-Rust invocation, with no C++ toolchain. This removes the last user-facing dependence on `ffi-backend`. - Swap the `.fset3` step from `CppFreakMatcher` to `RustFreakMatcher` (`KpmRefDataSet::generate` is backend-agnostic — `&mut dyn FreakMatcherBackend`). Drop the `#[cfg(feature = "ffi-backend")]` gates on the `.fset3` block and the `KPM_SURF_FEATURE_DENSITY` const. - Remove the now-pointless `--yes` flag and the "no .fset3" warning/prompt. - Update the example header docs and README to the single-invocation workflow. Parity (per the agreed policy — accept tolerance if downstream pose holds): the pure-Rust path generated a complete marker for pinball-893x1117-120dpi.jpg (.fset3 667 KB vs the C++ 625 KB — comparable, not byte-identical). Downstream round-trip: feeding the Rust-generated .fset3 into simple_nft detection yields a KPM match (page 0, error 6.05 vs the C++ baseline 5.09) — the marker works. Rust↔C++ FREAK extraction parity is additionally CI-gated by `cross_stack_parity` (pose) and `dual-mode`. Closes #179. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…it shim (#134) Swatinem/rust-cache@v2 intermittently restored a stale `cargo` shim into ~/.cargo/bin on the macOS kpm-build runner. The cached shim resolved to `rustup-init`, so `cargo build` was parsed as `rustup-init build` and the job failed non-deterministically. Linux and Windows were unaffected. Set `cache-bin: false` on this job's cache step. kpm-build installs no cargo-managed binaries, so disabling bin caching removes the flake at no cost to cache effectiveness. Refs #134 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PR #117 fixed build.rs to link libc++ (not libstdc++) on Apple targets, but its dual-mode CI step was scoped to --lib, so the integration tests and the generate_patt example were never re-built on macOS after the fix. This left open whether the libc++ fix was sufficient or whether a separate macOS linker issue remained. Add a macOS-gated step to the kpm-build matrix that builds the ffi-backend integration tests (--no-run) and the generate_patt example, confirming they link against libc++. Use --no-run because executing the pose-sensitive assertions cross-platform is out of scope — that float-drift question is tracked in #118, which keeps test_full_pipeline_pose gated to Linux. Refs #119 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Refresh the status note: KPM/NFT is now a complete pure-Rust pipeline (native + WASM), no longer "early stage". - Bump install snippets 0.7 -> 0.8. - Add a v0.8.0 "Pure-Rust completeness, validation & polish" roadmap entry (WASM/NFT #161, pure-Rust .fset3 #179, VisualDatabase #147/#148, Gaussian pyramid SIMD/rayon #200-#207, coverage/CI hardening) and prune the now-completed short-term goals. - ARCHITECTURE: add simd-x86-avx2 to the feature table, document the freak::visual_database orchestrator and freak::gaussian_pyramid modules, and the wasm crate's NFT bindings + browser demo. Refs #161 #179 #147 #148 #177 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bump the workspace and npm package versions 0.7.0 -> 0.8.0, regenerate the wasm pkg (standard + SIMD), and prepend the 0.8.0 changelog section (git-cliff) for the "Pure-Rust completeness, validation & polish" milestone. Release prep per MAINTAINERS.md §2. The dev->main merge and the v0.8.0 tag (which triggers the crates.io + npm publish) are done separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promote
devtomainfor the v0.8.0 release — "Pure-Rust completeness, validation & polish".Merging this creates the release merge commit on
main(same flow as #181). After merge, tagv0.8.0onmainto triggerrelease.yml(GitHub Release + crates.io + npm publish).Contents since v0.7.0: full WASM/NFT support (#161), pure-Rust
.fset3marker generation (#179), VisualDatabase query refactor + facade accessors (#147/#148), Gaussian pyramid SIMD/rayon perf (#200–#207), M9 coverage (#177), and CI/validation hardening (#134/#119/#204/#194/#182/#180). Version bumped to 0.8.0 with regenerated wasm pkg and CHANGELOG.🤖 Generated with Claude Code