feat(motion_estimator): injectable RNG for ransac/lmeds minimal-sample draws

[kalwalt]

Summary

get_subset draws its minimal samples from the global Math.random directly. This meant:

• No way to get cv::findHomography-style reproducibility (OpenCV seeds its own RNG with a fixed value per call).
• This codebase's own tests had to fall back to globally mocking Math.random via vi.spyOn — works, but can't run in parallel safely and every downstream consumer wanting determinism had to reinvent it.
• webarkit/webarkit's cv-backend-jsfeatnext adapter hit exactly this gap: a test flaky ~1 in 45 runs, worked around with a loosened tolerance in webarkit/webarkit#8.

Per #189's acceptance criteria:

• RandomFn = () => number added to src/types.ts, re-exported from the package root.
• ransac_params_t gains an rng field (5th, optional constructor param), defaulting to a lazy () => Math.random() wrapper — not a bare Math.random reference, which would freeze whatever function object Math.random was at construction time into the instance, permanently bypassing a vi.spyOn(Math, "random") mock installed afterward (this codebase's own established pattern: construct params, then seed — caught this exact regression against the existing parity suite during development, see Testing below).
• get_subset accepts an optional trailing rng param (default Math.random, unchanged behavior for direct callers); ransac()/lmeds() now pass params.rng through.
• math.mulberry32(seed) — a small seedable generator returning a RandomFn, reachable via the existing singleton (jsfeatNext.math.mulberry32(...)), matching the issue's suggested name/location.
• tests/parity/motion_estimator.test.ts migrated off the global mock for the jsfeatNext side — the vendored jsfeat oracle itself still needs the global mock, since it's a frozen third-party dependency this repo doesn't own or modify.

No behavior change for existing callers that pass nothing: get_subset's default still resolves Math.random fresh per call, exactly as before.

Testing

• npm test: 326/326 passing (up from 320 before this PR — new tests below), run twice to confirm determinism.
• New tests: mulberry32 reproducibility/divergence/range (tests/properties/math.test.ts), ransac_params_t.rng default + injection (tests/properties/data-structures.test.ts), end-to-end find_homography() reproducibility via an injected seed with no global Math.random mock anywhere in the test (tests/properties/find_homography.test.ts).
• npx tsc --noEmit, prettier --check, check-license-headers.mjs: clean.
• npx vitest run --coverage: patch coverage clean (learned from motion_estimator.ransac: doc comment promises a refit over all inliers; jsfeatNext has no layer that performs one #185/fix(motion_estimator): add find_homography, the missing refit layer RANSAC/LMEDS never had #190's Codecov regression — verified no new uncovered branches).
• /code-review (two-axis, Standards + Spec against motion_estimator.get_subset uses the global Math.random; make the RNG injectable #189): 0 hard findings on both axes. One minor Spec-axis note (a documented-non-determinism mention in README vs. JSDoc-only) reviewed and left as-is — consistent with how motion_estimator.ransac: doc comment promises a refit over all inliers; jsfeatNext has no layer that performs one #185/feat(cv_backend): descriptor selection & capability declaration #128/feat(cv_backend): optional filterMatches step (GMS seam) #129 were documented (README explicitly delegates detailed API docs to TSDoc/TypeDoc, never duplicates per-parameter notes there).

Closes #189.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[qodo-code-review]

PR Summary by Qodo

Add injectable RNGs for deterministic RANSAC and LMEDS sampling

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Adds injectable RNGs for deterministic RANSAC and LMEDS minimal-sample selection.
• Provides seeded Mulberry32 generation while preserving lazy Math.random defaults.
• Replaces global mocks in owned paths and verifies reproducibility, range, and parity.

Diagram

sequenceDiagram
    actor Caller
    participant Math as math.mulberry32
    participant Params as ransac_params_t
    participant Estimator as motion_estimator
    participant Subset as get_subset
    participant Kernel as MotionKernel
    Caller->>Math: Create seeded RNG
    Math-->>Caller: Return RandomFn
    Caller->>Params: Inject RNG
    Caller->>Estimator: Run RANSAC or LMEDS
    Estimator->>Subset: Pass params.rng
    loop Minimal-sample draws
        Subset->>Kernel: Validate subset
        Kernel-->>Subset: Accept or retry
    end
    Subset-->>Estimator: Return valid sample

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Accept a numeric seed

• ➕ Offers a smaller caller-facing configuration surface.
• ➕ Lets the estimator own generator creation and lifecycle.
• ➖ Restricts callers to one internal PRNG implementation.
• ➖ Makes RNG state-sharing and custom deterministic sequences harder.
• ➖ Requires defining seed lifecycle semantics across repeated estimator calls.

2. Seed or mock global randomness

• ➕ Avoids changing estimator parameter APIs.
• ➕ Matches the existing vendored-oracle testing mechanism.
• ➖ Mutates process-wide state and prevents safe parallel execution.
• ➖ Couples consumers and tests to global mocking behavior.
• ➖ Cannot isolate independent estimator instances.

Recommendation: Keep the injectable RandomFn design. It preserves existing behavior, supports arbitrary deterministic or application-specific generators, avoids global state, and cleanly threads randomness through the existing parameter object. The bundled mulberry32 helper retains the convenience of a numeric seed without constraining the estimator API to one generator.

Files changed (9) +161 / -22

Enhancement (5) +71 / -11

index.tsExport the public RandomFn type +1/-1

Export the public RandomFn type

• Adds 'RandomFn' to the package-root type exports so consumers can type custom or seeded generators without importing internal modules.

src/index.ts

math.tsAdd the seeded Mulberry32 generator +25/-0

Add the seeded Mulberry32 generator

• Adds 'math.mulberry32(seed)', which returns a deterministic 'RandomFn' producing values in '[0, 1)'. This provides a built-in generator for reproducible robust estimation.

src/math/math.ts

motion_estimator.tsUse injected randomness for minimal-sample draws +11/-8

Use injected randomness for minimal-sample draws

• Extends 'get_subset' with an optional RNG and uses it instead of directly calling 'Math.random'. Both RANSAC and LMEDS now pass 'params.rng' into subset selection while direct callers retain the existing default behavior.

src/motion_estimator/motion_estimator.ts

ransac_params_t.tsStore an injectable estimator RNG +26/-2

Store an injectable estimator RNG

• Adds an optional fifth constructor parameter and public 'rng' field. The default uses a lazy wrapper around 'Math.random', preserving compatibility with mocks installed after parameter construction.

src/motion_estimator/ransac_params_t.ts

types.tsDefine the RandomFn contract +8/-0

Define the RandomFn contract

• Introduces the reusable 'RandomFn' type for functions producing 'Math.random'-compatible values in '[0, 1)'.

src/types.ts

Tests (4) +90 / -11

motion_estimator.test.tsInject seeded RNGs into parity runs +9/-11

Inject seeded RNGs into parity runs

• Migrates jsfeatNext RANSAC and LMEDS parity runs from global 'Math.random' mocks to injected seeded generators. The vendored jsfeat oracle continues using global mocks because it lacks RNG injection.

tests/parity/motion_estimator.test.ts

data-structures.test.tsTest ransac_params_t RNG behavior +21/-0

Test ransac_params_t RNG behavior

• Verifies that the default RNG produces values in the expected range and that an injected function is stored and invoked directly.

tests/properties/data-structures.test.ts

find_homography.test.tsVerify end-to-end seeded homography reproducibility +28/-0

Verify end-to-end seeded homography reproducibility

• Adds an integration test proving that fresh generators with the same seed produce identical homography models and masks without mocking global randomness.

tests/properties/find_homography.test.ts

math.test.tsValidate Mulberry32 determinism and range +32/-0

Validate Mulberry32 determinism and range

• Tests equal-seed reproducibility, different-seed divergence, and the '[0, 1)' output contract across repeated draws.

tests/properties/math.test.ts

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Repeated draws hang sampling ✓ Resolved 🐞 Bug ☼ Reliability

Description

get_subset retries duplicate indices in an unbounded inner loop, so a contract-compliant RNG such
as () => 0 hangs forever whenever need_cnt > 1. The outer 1000-attempt limit never advances
while this loop is stuck, blocking both RANSAC and LMEDS.

Code

src/motion_estimator/motion_estimator.ts[107]

+                    idx_i = indices[i] = Math.floor(rng() * max_cnt) | 0;

Evidence

The new public RandomFn contract only requires values in [0, 1), which includes a constant-zero
function. get_subset draws at line 107 and rejects duplicates in the surrounding while (!ok)
without incrementing the outer ssiter retry counter; both estimators now route the injected
function into this path.

src/types.ts[71-78]
src/motion_estimator/motion_estimator.ts[93-117]
src/motion_estimator/motion_estimator.ts[248-251]
src/motion_estimator/motion_estimator.ts[373-376]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An injected RNG that repeatedly produces the same index causes `get_subset` to remain forever in its duplicate-rejection loop. Ensure minimal-sample selection terminates even for a degenerate but type-compliant `RandomFn`.

## Issue Context
The existing `max_try` counter covers rejected subsets, but it is not incremented while retrying duplicate indices. Preserve deterministic seeded behavior while adding a finite failure path or using a bounded without-replacement sampling algorithm.

## Fix Focus Areas
- src/motion_estimator/motion_estimator.ts[93-117]
- tests/properties/find_homography.test.ts[470-496]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. mulberry32 lacks dedicated module ✗ Dismissed 📘 Rule violation ⌂ Architecture

Description

The new mulberry32 algorithm is implemented inside the existing multi-algorithm math module
instead of its own matching subdirectory and file. This mixes a PRNG with the module’s existing
Gaussian-kernel, quicksort, and median implementations.

Code

src/math/math.ts[R601-604]

+    mulberry32(seed: number): RandomFn {
+        let a = seed >>> 0;
+        return () => {
+            a |= 0;

Evidence

PR Compliance IDs 2965935 and 2965984 require distinct algorithms to have dedicated matching modules
and prohibit placing multiple conceptually distinct implementations in one module. The added code
implements the complete mulberry32 PRNG at src/math/math.ts[587-610], while the same file
documents that math already implements Gaussian-kernel generation, quicksort, and median
operations at src/math/math.ts[52-57].

Rule 2965935: Algorithms must be defined in dedicated modules and subclass the core base
Rule 2965984: Place each algorithm implementation in its own subdirectory under src/
src/math/math.ts[587-610]
src/math/math.ts[52-57]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `mulberry32` PRNG algorithm is implemented directly in the existing `math` module rather than a dedicated algorithm subdirectory and matching file.

## Issue Context
Compliance requires each distinct algorithm implementation to live in its own module. Preserve the public `jsfeatNext.math.mulberry32` API through a thin delegation or namespace wiring layer after relocating the implementation, and ensure the conforming algorithm class extends the shared core base.

## Fix Focus Areas
- src/math/math.ts[587-610]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 27 rules

✅ Skills: license-header-adder

Review mode: ⚖️ Balanced: This is a behavior-affecting public API and algorithm change spanning RNG generation, RANSAC/LMEDS sampling, defaults, and reproducibility tests, warranting a complete single-pass review.

Tip of the day

💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗