fix(bfmatcher,index): reject mismatched descriptor widths and export the new types

[kalwalt]

Addresses the Qodo review on #183. Kept separate from the release PR: these are library bugs, not release work, so they belong in dev on their own and land in the changelog under the right heading.

These block the 0.15.0 tag. Two of the three affect the published surface of exactly the modules 0.15.0 exists to ship.

Fixed

bfmatcher silently corrupted matches on mismatched widths 🐞

match() and knnMatch() derived one stride from query.cols and used it to address both matrices:

const word_len = query.cols >> 2;
// ...
bfmatcher.hamming(qw, qoff, tw, ti * word_len, word_len);
//                             ^^^^^^^^^^^^^^ train offset, query stride

With train.cols !== query.cols the train reads walk across row boundaries, and past the end an out-of-range Int32Array index yields undefined — which XOR coerces to 0. So nothing threw: the matcher returned a complete set of confident, meaningless Hamming distances, and the caller's RANSAC just never found consensus.

The pre-existing words() check only verified each width was a multiple of 4; it never compared them. The docs said "same row width as query" but nothing enforced it. A new pairWords() validates the pair once, for both methods.

Worth noting this is the same failure mode the CvBackend contract guards with DescriptorMismatchError — the library underneath had the same hole one level down.

The new public types were unreachable 🐞

src/index.ts re-exports the data structures so consumers can annotate without deep paths (#92), but match_t/IMatch_T and pose_t/IPose_T were never added. Since the exports map only exposes . and ./package.json, a deep import cannot compensate either — there was no way for a consumer to name those types.

Not hypothetical: the CvBackend adapter being written against this library in webarkit/webarkit needs both.

pose_estimator allocated its scratch every frame ⚡

B (the K⁻¹·H product) was a fresh Float64Array(9) per call. An estimator is constructed once and reused across frames, so at 30–60 fps that is one allocation per frame for no benefit; it is now an instance field.

The review proposed borrowing from the shared cache instead. That is the wrong tool here — the pool exists for image-sized buffers, and balancing a get_buffer/put_buffer across the degenerate early return to save 72 bytes costs more in bookkeeping than it saves.

Rejected, with the cause fixed

Two findings claim pose_estimator violates the module rules: that it must extend core, and that it must not require new.

Both misclassify it. It is stateful — it holds K⁻¹ — and the public API constructs it with a K, so it belongs with matrix_t, keypoint_t and ransac_params_t, which AGENTS.md establishes remain constructors. Its static intrinsics() factory also requires the class itself to sit on the namespace; a singleton instance could not expose it.

The root cause is worth fixing though: AGENTS.md listed the constructor classes without mentioning pose_estimator or match_t, which is precisely what invited the misreading. It now names them and states the rationale, so the next reviewer — bot or human — does not repeat it.

Verification

• npm test — 308 passed (305 + 3 new)
• npm run typecheck, format-check, license-check — clean
• confirmed after a build that types/src/index.d.ts now carries both new type exports

dist/ and types/ are deliberately not committed here, per the convention that they are rebuilt at release. #183 will regenerate them with these fixes included once this merges.

Order

Merge this first, then update #183 (merge dev in, rebuild artifacts) before tagging 0.15.0.

[qodo-code-review]

PR Summary by Qodo

Validate BFMatcher widths and expose matcher and pose types

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Reject mismatched descriptor widths before brute-force Hamming matching.
• Export matcher and pose types from the package root.
• Reuse pose-estimation scratch storage and document constructor semantics.

Diagram

graph TD
  D["Descriptor pair"] --> V{"Widths equal?"} -->|Yes| M["Hamming matcher"]
  V -->|No| E["Width error"]
  P["Package root"] --> X["Public CV types"]
  P -->|Exposes| C["Pose estimator"] --> S["Reused B scratch"]

Loading

High-Level Assessment

The current approach is appropriate. Centralizing pair validation prevents match() and knnMatch() from diverging, and rejecting unequal dimensions is safer than supporting separate strides because Hamming distances are not meaningful across descriptor widths. Root type-only exports fit the restricted exports map, while a tiny per-instance pose scratch array avoids frame-by-frame allocation without unnecessary shared-cache bookkeeping.

Files changed (5) +93 / -9

Bug fix (2) +30 / -6

bfmatcher.tsReject incompatible descriptor widths before matching +28/-6

Reject incompatible descriptor widths before matching

• Adds a shared query/train validator that requires equal row widths before deriving the common word stride. Both match() and knnMatch() now fail explicitly instead of reading train rows at query-based offsets and returning corrupted Hamming distances.

src/bfmatcher/bfmatcher.ts

index.tsExport matcher and pose types from the package root +2/-0

Export matcher and pose types from the package root

• Adds type-only exports for match_t, IMatch_T, pose_t, and IPose_T. Consumers can now name these public structures without unsupported deep imports.

src/index.ts

Tests (1) +44 / -0

bfmatcher.test.tsCover descriptor-width mismatch failures +44/-0

Cover descriptor-width mismatch failures

• Adds regression coverage for match() and knnMatch() width validation, diagnostic width values, and the silent distance corruption caused by the previous shared-stride behavior.

tests/properties/bfmatcher.test.ts

Documentation (1) +2 / -1

AGENTS.mdClarify pose estimator constructor architecture +2/-1

Clarify pose estimator constructor architecture

• Documents match and pose structures as constructors and explains why the stateful pose estimator is neither a singleton nor a core subclass. This prevents automated reviews from incorrectly applying stateless-module rules.

AGENTS.md

Other (1) +17 / -2

pose_estimator.tsReuse homography-product scratch across estimates +17/-2

Reuse homography-product scratch across estimates

• Moves the nine-element B scratch array from estimate() into pose_estimator instance state. Each call fully overwrites the buffer, eliminating one small per-frame allocation without using the shared image-buffer cache.

src/pose_estimator/pose_estimator.ts

[qodo-code-review]

Code Review by Qodo

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

1. Channel widths remain unchecked ✓ Resolved 🐞 Bug ≡ Correctness

Description

pairWords treats equal cols as equal descriptor widths, although a matrix_t row's storage also
depends on channel and element type. Same-column matrices with different channel counts pass this
guard, then match and knnMatch use the query-derived stride for both buffers and silently
calculate distances from the wrong train rows.

Code

src/bfmatcher/bfmatcher.ts[R117-120]

+        if (query.cols !== train.cols) {
+            throw new Error(
+                `jsfeatNext.bfmatcher: query and train descriptors must have the same row width, ` +
+                    `got ${query.cols} and ${train.cols}`

Evidence

matrix_t.cols is only the number of columns, while channel is separate and element storage uses
(row * cols + col) * channel; allocation similarly multiplies columns by channel count and
data-type size. The matcher nevertheless exposes the full raw Int32Array, derives word_len
solely from query.cols, and uses that value for both query and train offsets, so the new equality
check does not establish equal physical row widths.

src/matrix_t/matrix_t.ts[52-70]
src/matrix_t/matrix_t.ts[97-105]
src/matrix_t/matrix_t.ts[139-157]
src/bfmatcher/bfmatcher.ts[91-97]
src/bfmatcher/bfmatcher.ts[116-130]
src/bfmatcher/bfmatcher.ts[145-157]
src/bfmatcher/bfmatcher.ts[216-226]

Agent prompt

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

## Issue description
`pairWords` compares only `cols`, but `matrix_t` row storage also depends on channel count and data type. Enforce the matcher's supported U8/C1 descriptor representation for both matrices, or compare their effective byte widths before deriving and sharing a stride.

## Issue Context
Both matcher entry points consume raw `buffer.i32` views. A same-`cols` U8/C1 query and U8/C2 train currently pass validation despite having different physical row widths, reproducing the silent row-stride corruption this guard is intended to prevent.

## Fix Focus Areas
- src/bfmatcher/bfmatcher.ts[91-124]
- tests/properties/bfmatcher.test.ts[114-160]

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

---

2. pose_estimator skips core base ✗ Dismissed 📘 Rule violation ⌂ Architecture

Description

The PR modifies the pose_estimator algorithm while explicitly retaining it as a class that does
not extend the base from src/core/core.ts. This violates the required inheritance structure for
modified algorithm implementations.

Code

src/pose_estimator/pose_estimator.ts[127]

+    private readonly B: Float64Array;

Evidence

The checklist requires modified algorithm classes to subclass the core base. The modified class is
declared as export class pose_estimator without an extends clause, and the changed architecture
documentation explicitly states that it deliberately does not extend core.

Rule 2965935: Algorithms must be defined in dedicated modules and subclass the core base
src/pose_estimator/pose_estimator.ts[112-131]
AGENTS.md[23-24]

Agent prompt

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

## Issue description
`pose_estimator` is a modified algorithm class but does not extend the shared core base.

## Issue Context
PR Compliance ID 2965935 requires each modified algorithm's main class to extend the base exported by `src/core/core.ts`. Preserve the stateful constructor and public API while adding the required inheritance.

## Fix Focus Areas
- src/pose_estimator/pose_estimator.ts[112-131]
- AGENTS.md[23-24]

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

---

3. B bypasses shared cache 📘 Rule violation ⌂ Architecture

Description

The new B scratch buffer is allocated directly with new Float64Array(9) rather than obtained
from shared_cache. This bypasses the mandatory shared scratch-buffer mechanism for algorithm
modules.

Code

src/pose_estimator/pose_estimator.ts[131]

+        this.B = new Float64Array(9);

Evidence

The rule requires algorithm scratch buffers to use shared_cache. The added comments identify B
as scratch and explicitly reject borrowing it from the shared cache, while the constructor directly
allocates the typed array.

Rule 2966049: Algorithm modules must use the shared_cache from src/core/core.ts for scratch buffers
src/pose_estimator/pose_estimator.ts[115-131]
src/cache/cache.ts[45-66]

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 `pose_estimator` algorithm directly allocates its `B` scratch array instead of using `shared_cache`.

## Issue Context
PR Compliance ID 2966049 requires algorithm scratch storage to come from the shared cache exported by `src/core/core.ts`. Ensure every borrowed buffer is returned exactly once on success, early-return, and exception paths.

## Fix Focus Areas
- src/pose_estimator/pose_estimator.ts[115-131]
- src/pose_estimator/pose_estimator.ts[194-202]

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

---

Context sources

✅ Compliance rules (platform): 25 rules

✅ Skills: license-header-adder

Review mode: ⚖️ Balanced

Tip of the day

💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!