Skip to content

chore: clean-up - #15425

Merged
kgryte merged 2 commits into
developfrom
philipp/fix-commit-review-2026-09-22
Sep 22, 2026
Merged

kgryte merged 2 commits into
developfrom
philipp/fix-commit-review-2026-09-22

Conversation

@Planeshifter

Copy link
Copy Markdown
Member

Follow-up fixes for commits merged to develop between 2026-09-21 18:41 PDT (1e6660288) and 2026-09-22 03:35 PDT (a75d488c1).

Description

What is the purpose of this pull request?

This pull request:

  • In 5dad2c572, lib/node_modules/@stdlib/blas/ext/base/ztril2triu/lib/main.js (L24) requires ./ztril2triu without the .js extension, violating stdlib/require-file-extensions; fix to ./ztril2triu.js to match dtril2triu, ctriu2tril, dtriu2tril, etc.
  • Fixes blas/ext/base/ztril2triu's docs/types/test.ts (introduced in 5dad2c572) to require( './index' ) instead of the package path, so the test actually type-checks the local index.d.ts.
  • stridesY/stridesZ mistyped as strideY/strideZ in all nine API headings of lib/node_modules/@stdlib/ndarray/base/kernels/generic/binary-strided1d/blocked/README.md (introduced in 5f6b7b358); fixed to match the parameter lists and 2d.js JSDoc (same typo exists in the unblocked sibling's README, left out of scope here).

Grouped by package, one commit each:

  • blas/ext/base/ztril2triu — style: (both module-path fixes)
  • ndarray/base/kernels/generic/binary-strided1d/blocked — docs:

Both new ztril2triu defects were inherited from the pre-existing ztril, which the package was templated from; ztril/ztriu carry the same two deviations but are outside this window and were left alone.

Related Issues

Does this pull request have any related issues?

No.

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

Window reviewed. 31 commits, 210 files. 24 are test: migrations to ULP-based assertions (issue #11352), 4 are new packages (blas/ext/base/{ctril2triu,ztril2triu,ztriu2tril}, ndarray/base/kernels/generic/binary-strided1d/blocked), 3 are misc (a3e346300 whitespace, 4ef16a954 API_SUFFIX README cleanup, ef08b9f5a hypergeometric/variance refactor).

Validation. Checked: style-guide compliance of each new package against its established sibling (dtril2triu, ctriu2tril, dtriu2tril, unary-strided1d/blocked, binary-strided1d/unblocked); copy-paste contamination across dtype variants and across the nine lib/{2..10}d.js kernels; bug scan over the union diff; memory safety and argv ordering in the new src/main.c/src/addon.c; leftover dead declarations across all 80 touched test files.

Came back clean and deliberately not changed:

  • ef08b9f5a rewrites hypergeometric variance as p = K/N; n*p*(1-p)*((N-n)/(N-1)). (N-K)/N and 1.0-p are not bit-identical, but the rewrite matches the package's existing C implementation and reduces max error against the Julia fixtures from 29 ULP to 1, consistent with the new isAlmostSameValue( y, expected[ i ], 1 ) tolerance. Correct as written.
  • The three new BLAS packages' reflection logic in src/main.c and lib/base.js is logic-identical to the established siblings; fuzzed against a naive reference over random stride/offset/k configurations with no mismatches.
  • The blocked ndarray kernels agree with binary-strided1d/unblocked for ndims 2–10 in both memory orders, including shapes large enough to force block splits.
  • No dead var delta/tol/abs/EPS declarations or unused requires left behind by the ULP migrations.

Deliberately excluded from this PR: the identical strideY/strideZ typo in binary-strided1d/unblocked/README.md and the same two module-path deviations in blas/ext/base/{ztril,ztriu} — all outside the commit window. Also excluded: anything requiring interpretation or a judgment call, and any fix that would have needed changes beyond the window's diff to stay correct.

Note: node_modules is not installed in the environment this ran in, so the package test suites could not be executed. lib/main.js was verified to load under Node after the require change; the other two edits are a TypeScript import path and README headings.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

AI Assistance

When authoring the changes proposed in this PR, did you use any kind of 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)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

Disclosure

If you answered "yes" to using AI assistance, please provide a short disclosure indicating how you used AI assistance.

This PR was authored by Claude Code running as a scheduled review of the last 24 hours of commits to develop. Claude enumerated the commit window, ran parallel style and bug reviews against established reference packages, cross-verified each finding against the repository before accepting it, and applied the fixes. Every change here is a one-line module path or a documentation typo; the analysis summarized above under "Validation" is likewise AI-generated and should be spot-checked rather than taken on trust. Opened as a draft for maintainer audit.


@stdlib-js/reviewers

🤖 Generated with Claude Code

https://claude.ai/code/session_01WPx5veUPahKt8Gwe8N4MdA


Generated by Claude Code

Resolve `lib/main.js` require with an explicit `.js` extension, as
required by `stdlib/require-file-extensions`, and import the local
declarations in `docs/types/test.ts` rather than the package path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WPx5veUPahKt8Gwe8N4MdA
…ided1d/blocked`

The nine API headings named `strideY`/`strideZ`, while the parameter
lists below them and the `lib/*.js` JSDoc use `stridesY`/`stridesZ`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WPx5veUPahKt8Gwe8N4MdA
@stdlib-bot stdlib-bot added the Good First PR A pull request resolving a Good First Issue. label Sep 22, 2026
@Planeshifter Planeshifter changed the title Follow-up fixes for commits merged to develop on 2026-09-21/22 style: correct module paths and documented parameter names in recently added packages Sep 22, 2026
@stdlib-bot

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
blas/ext/base/ztril2triu $\\color{green}598/598$
$\\color{green}+100.00\\%$
$\\color{green}43/43$
$\\color{green}+100.00\\%$
$\\color{green}5/5$
$\\color{green}+100.00\\%$
$\\color{green}598/598$
$\\color{green}+100.00\\%$
ndarray/base/kernels/generic/binary-strided1d/blocked $\\color{red}1680/3387$
$\\color{green}+49.60\\%$
$\\color{green}11/11$
$\\color{green}+100.00\\%$
$\\color{red}0/10$
$\\color{green}+0.00\\%$
$\\color{red}1680/3387$
$\\color{green}+49.60\\%$

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

@Planeshifter
Planeshifter marked this pull request as ready for review September 22, 2026 15:39
@Planeshifter
Planeshifter requested review from a team and kgryte September 22, 2026 15:39
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Sep 22, 2026
@Planeshifter Planeshifter changed the title style: correct module paths and documented parameter names in recently added packages chore: correct module paths and documented parameter names in recently added packages Sep 22, 2026
@kgryte kgryte changed the title chore: correct module paths and documented parameter names in recently added packages chore: clean-up Sep 22, 2026
@kgryte
kgryte merged commit 4e396e9 into develop Sep 22, 2026
60 of 61 checks passed
@kgryte
kgryte deleted the philipp/fix-commit-review-2026-09-22 branch September 22, 2026 19:31
@stdlib-bot stdlib-bot removed the Needs Review A pull request which needs code review. label Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Good First PR A pull request resolving a Good First Issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants