Skip to content

chore: address follow-up review findings for commits merged to develop on 2026-05-31#12430

Merged
kgryte merged 4 commits into
developfrom
philipp/fix-commit-review-2026-06-01
Jun 1, 2026
Merged

chore: address follow-up review findings for commits merged to develop on 2026-05-31#12430
kgryte merged 4 commits into
developfrom
philipp/fix-commit-review-2026-06-01

Conversation

@Planeshifter
Copy link
Copy Markdown
Member

Follow-up fixes for commits merged to develop between afb44ae (2026-05-31 15:54 -0500) and d2f751e (2026-06-01 04:32 -0700).

Description

This pull request:

  • Fixes rffti to return the workspace array when N === 1 (62ee5f8, lib/node_modules/@stdlib/fft/base/fftpack/rffti/lib/main.js); the early-exit branch silently returned undefined, violating the documented return contract and breaking reference equality with the caller's workspace.
  • Corrects the capitalization of "Find" to "find" in the module JSDoc description of lib/node_modules/@stdlib/blas/base/ndarray/igamax/lib/index.js (1d9f9d1) to match every other BLAS ndarray package.
  • Replaces is-nanf with is-nan in lib/node_modules/@stdlib/blas/base/ndarray/isamax/benchmark/benchmark.js (4302ca2); isamax returns an integer index, and the sibling igamax benchmark already uses is-nan.
  • Drops the spurious .0 from the isamax JSDoc example in lib/node_modules/@stdlib/blas/base/ndarray/isamax/docs/types/index.d.ts (4302ca2); the return value is an integer index.
  • Drops the spurious .0 from the idamax and isamax JSDoc examples in lib/node_modules/@stdlib/blas/base/ndarray/docs/types/index.d.ts (52ca9a6); both routines return an integer index, consistent with the igamax entry already in the same file.

Related Issues

No.

Questions

No.

Other

Validation audit:

  • Checked: stdlib code style compliance (docs/style-guides/) and bug scan across the union diff of all first-parent commits merged to develop in the trailing 24-hour window.
  • Deliberately excluded: subjective concerns, possible/might-be issues, anything requiring context outside the diff to validate, and the pre-existing // returns 4.0 in lib/node_modules/@stdlib/blas/base/ndarray/idamax/docs/types/index.d.ts (out of window).

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)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

Disclosure

This PR was authored by Claude Code: a sonnet summarizer agent extracted author intent for the 36 commits in window, two sonnet style-compliance agents and two opus bug-scan agents reviewed the union diff in parallel, and findings reported by a single agent were independently re-verified against the diff before being applied. Only high-signal issues survived filtering. A maintainer should audit before promoting from draft.


@stdlib-js/reviewers


Generated by Claude Code

claude added 4 commits June 1, 2026 12:31
The JSDoc and TypeScript declaration both promise to return the workspace
array, but the `N === 1` short-circuit returned `undefined`, breaking the
documented `out === workspace` contract.
…ay/isamax`

The benchmark guarded the integer index return value with `isnanf`, which
is the float32 NaN predicate; switch to `isnan` to match the documented
integer return type and the sibling `igamax` benchmark. Also drop the
spurious `.0` from the TypeScript example, since the function returns an
integer index.
…e/ndarray`

The newly added `idamax` and `isamax` JSDoc examples in the namespace
declarations documented the return value as `4.0`, but both routines
return an integer index. Match the `igamax` entry, which already used
`returns 4`.
@stdlib-bot
Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
blas/base/ndarray $\color{red}4728/4943$
$\color{green}+95.65\%$
$\color{green}81/81$
$\color{green}+100.00\%$
$\color{red}0/40$
$\color{green}+0.00\%$
$\color{red}4728/4943$
$\color{green}+95.65\%$
blas/base/ndarray/igamax $\color{green}103/103$
$\color{green}+100.00\%$
$\color{green}3/3$
$\color{green}+100.00\%$
$\color{green}1/1$
$\color{green}+100.00\%$
$\color{green}103/103$
$\color{green}+100.00\%$
blas/base/ndarray/isamax $\color{green}103/103$
$\color{green}+100.00\%$
$\color{green}3/3$
$\color{green}+100.00\%$
$\color{green}1/1$
$\color{green}+100.00\%$
$\color{green}103/103$
$\color{green}+100.00\%$
fft/base/fftpack/rffti $\color{green}412/412$
$\color{green}+100.00\%$
$\color{green}12/12$
$\color{green}+100.00\%$
$\color{green}2/2$
$\color{green}+100.00\%$
$\color{green}412/412$
$\color{green}+100.00\%$

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

@Planeshifter Planeshifter changed the title Follow-up fixes for commits merged to develop on 2026-05-31 fix: address follow-up review findings for commits merged to develop on 2026-05-31 Jun 1, 2026
@Planeshifter Planeshifter changed the title fix: address follow-up review findings for commits merged to develop on 2026-05-31 chore: address follow-up review findings for commits merged to develop on 2026-05-31 Jun 1, 2026
@Planeshifter
Copy link
Copy Markdown
Member Author

Probably okay to use "chore:" even though a touched file is a "fix:", given how the package just landed and is unpublished.

@Planeshifter Planeshifter requested a review from kgryte June 1, 2026 13:44
@Planeshifter Planeshifter marked this pull request as ready for review June 1, 2026 13:44
@Planeshifter Planeshifter requested a review from a team June 1, 2026 13:44
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Jun 1, 2026
@kgryte kgryte removed the Needs Review A pull request which needs code review. label Jun 1, 2026
@kgryte kgryte merged commit 9f0e49e into develop Jun 1, 2026
46 of 47 checks passed
@kgryte kgryte deleted the philipp/fix-commit-review-2026-06-01 branch June 1, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants