Skip to content

docs: update notes#12130

Merged
kgryte merged 2 commits into
developfrom
philipp/fix-commit-review-2026-05-14
May 17, 2026
Merged

docs: update notes#12130
kgryte merged 2 commits into
developfrom
philipp/fix-commit-review-2026-05-14

Conversation

@Planeshifter
Copy link
Copy Markdown
Member

@Planeshifter Planeshifter commented May 14, 2026

Follow-up fixes for commits merged to develop between 2026-05-13 20:52 PDT (360877d) and 2026-05-14 02:11 PDT (80b1ef7).

Description

Audit of the 15 first-parent commits merged to develop over the last 24 hours surfaced two high-signal items, grouped by package below. Each commit on this branch corresponds to a single package fix.

blas/ext/base/cindex-of-column

  • Fixes incorrect JSDoc and TypeScript return descriptions introduced in 995d771lib/cindex_of_column.js, lib/ndarray.js, and docs/types/index.d.ts all said "unable to find a search vector" instead of "unable to find a matching column", contradicting both the README and the analogous dindex-of-column package.

blas/ext/base/zindex-of-column

  • Fixes the copy-paste error introduced in 62dad13 where lib/zindex_of_column.js, lib/ndarray.js, and docs/types/index.d.ts described the no-match condition as "unable to find a search vector" rather than "matching column", replacing all 5 occurrences to match the README and dindex-of-column reference implementation.

Related Issues

None.

Questions

None.

Other

Validation

Audited the 15-commit window with two parallel style-compliance reviewers (compared each substantive new file against established reference packages — dindex-of-column/sindex-of-column for the new column-search packages, transpose-operation-{str2enum,enum2str} for the kmeans enum utilities, ndarray/zeros/ndarray/ones for ndarray/nans) and two parallel bug reviewers (one diff-only, one with full source-file context). Findings were de-duplicated and each surviving issue re-verified against the diff before applying.

Items raised during the audit but deliberately excluded from this PR:

  • Native C _ndarray routines in cindex-of-column/zindex-of-column use signed strideA1 <= strideA2 for column-major detection while the JS path uses absolute-value isColumnMajor. The same divergence exists in the pre-existing dindex-of-column/sindex-of-column/gindex-of-column C sources, so addressing it would require a family-wide fix outside this window's scope.
  • "stride length for the first dimension" vs. "stride of the first dimension" wording in the new packages' JSDoc — the existing family is itself inconsistent (sindex-of-column uses "stride length for", dindex-of-column uses "stride of"); not a clear violation.
  • Re-adding the numpy.nans keyword to ndarray/nans/package.json — its removal in 65cbb12 is the explicit purpose of that commit.
  • Adding an extended keyword to the new cindex-of-column/zindex-of-column package.json files — the analogous dindex-of-column does not carry it, so no divergence exists.
  • dnrm2 @returns L2-norm tag in blas/base/ndarray/docs/types/index.d.ts — matches the wording already used by the standalone blas/base/dnrm2 and blas/base/ndarray/dnrm2 declarations.

Update

Originally this PR also proposed replacing the imported isNumber/isString guards in ml/base/kmeans/algorithm-str2enum and algorithm-enum2str with inline typeof checks to match the BLAS *2enum packages. Per review by @kgryte, the kmeans packages are correct as-is — it is the BLAS packages that should adopt the imported utility. Both commits have been dropped.

Checklist

AI Assistance

  • Yes
  • No

If you answered "yes" above, how did you use AI assistance?

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Documentation (including examples)
  • Research and understanding

Disclosure

This PR was authored by Claude Code as part of an automated daily review pass over commits merged to develop. Each finding was independently verified against the diff and the cited reference packages before applying. The PR is being opened as a draft for human audit; please do not promote to ready-for-review until the changes have been confirmed.


claude added 2 commits May 14, 2026 12:26
…lumn`

Replace `unable to find a search vector` with `unable to find a matching
column` in the JSDoc and TypeScript declarations to match the README and
the wording used by the analogous `dindex-of-column` package.
…lumn`

Replace `unable to find a search vector` with `unable to find a matching
column` in the JSDoc and TypeScript declarations to match the README and
the wording used by the analogous `dindex-of-column` package.
@stdlib-bot
Copy link
Copy Markdown
Contributor

stdlib-bot commented May 14, 2026

Coverage Report

Package Statements Branches Functions Lines
blas/ext/base/cindex-of-column $\color{green}548/548$
$\color{green}+0.00%$
$\color{green}54/54$
$\color{green}+0.00%$
$\color{green}4/4$
$\color{green}+0.00%$
$\color{green}548/548$
$\color{green}+0.00%$
blas/ext/base/zindex-of-column $\color{green}548/548$
$\color{green}+0.00%$
$\color{green}54/54$
$\color{green}+0.00%$
$\color{green}4/4$
$\color{green}+0.00%$
$\color{green}548/548$
$\color{green}+0.00%$

The above coverage report was generated for the changes in this PR.

@Planeshifter Planeshifter force-pushed the philipp/fix-commit-review-2026-05-14 branch from b4606f8 to acae714 Compare May 17, 2026 21:29
@kgryte kgryte marked this pull request as ready for review May 17, 2026 21:32
@kgryte kgryte requested a review from a team May 17, 2026 21:32
@kgryte kgryte changed the title fix: follow-up corrections for commits merged to develop on 2026-05-13/14 docs: update notes May 17, 2026
@stdlib-bot stdlib-bot added BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS). Needs Review A pull request which needs code review. labels May 17, 2026
@kgryte kgryte removed the Needs Review A pull request which needs code review. label May 17, 2026
@kgryte kgryte merged commit a6856ed into develop May 17, 2026
56 checks passed
@kgryte kgryte deleted the philipp/fix-commit-review-2026-05-14 branch May 17, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BLAS Issue or pull request related to Basic Linear Algebra Subprograms (BLAS).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants