fix: follow-up fixes for commits merged to develop on 2026-07-05/06#13327
Draft
Planeshifter wants to merge 5 commits into
Draft
fix: follow-up fixes for commits merged to develop on 2026-07-05/06#13327Planeshifter wants to merge 5 commits into
develop on 2026-07-05/06#13327Planeshifter wants to merge 5 commits into
Conversation
…base/gindex-of-same-value` Follow-up to 0cf99ac: - `docs/repl.txt`: second signature block describes the ndarray method but was missing the `.ndarray` suffix on the header line. - `test/test.ndarray.js`: three assertions in the accessor variant passed `N = x.length` with nonzero `offsetX`, walking reads past the end of the 6-element accessor array; mirror the correct `x.length-1`/`x.length-2` bounds already used in the non-accessor block directly above.
Follow-up to 6812e0b. The REPL example wrapped the raw string `'no-transpose'` in `from-scalar` with no dtype. The ndarray callee expects `trans` as an int8-encoded enum resolved via `@stdlib/blas/base/transpose-operation-resolve-enum`, as done in the same package's main.js JSDoc, README, examples, benchmarks, and tests. Route the string through `resolveEnum` and set `dtype: 'int8'` so the block matches the rest of the package.
Follow-up to 6570c4f. The `@example` block in main.js and the identical examples in docs/types/index.d.ts and docs/repl.txt only supplied one valid `(x, y)` pair followed by four NaN-poisoned calls, so the correlation never accumulated a second data point and the result stayed at 0.0 for every follow-up call — failing to demonstrate that NaN inputs are ignored. Interleave a second valid pair `accumulator( 1.0, 90.0 )` (mirroring the README) so subsequent NaN calls correctly show `~1.0`.
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
Hoist the `toAccessorArray( [ 1.0, 2.0, 3.0 ] )` literal into a local variable in the accessor-variant `N <= 0` test so the two assertion lines fit within the 80-character limit. Pre-existing warnings surfaced on this PR because the `Lint Changed Files` action re-lints the whole file.
This reverts commit 96db5e8.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up fixes for commits merged to
developbetween5de852b(2026-07-05 13:52 -0500) andbe45b3657(2026-07-06 03:54 -0700). A 24-hour code-review sweep across the 41 commits in that window surfaced three high-signal fixes on this branch, grouped into two package commits (+ one lint cleanup):blas/ext/base/gindex-of-same-value— Fix missing.ndarraysuffix in the second signature header ofblas/ext/base/gindex-of-same-value/docs/repl.txt(line 52); header didn't match the documented ndarray method, unlike sibling packages. Introduced in 0cf99ac.blas/ext/base/gindex-of-same-value— Fix out-of-bounds reads in the accessorstest.ndarray.jsforgindex-of-same-value(0cf99ac): theoffsetX=1/2cases still usedN = x.lengthinstead ofx.length-1/x.length-2like the non-accessor block above, walking past the 6-element array; tests only passed because the match preceded the overrun.stats/incr/nanpcorr2— Fix nanpcorr2 example in 6570c4f:lib/main.jsnever fed a second(x, y)pair before the NaN calls, so the accumulator stayed pinned at 0.0 instead of demonstrating NaN-skipping at ~1.0. Same bug duplicated indocs/types/index.d.tsanddocs/repl.txt; all three now insertaccumulator( 1.0, 90.0 )to match the README.Related Issues
None.
Questions
No.
Other
Validation. The review checked (1) stdlib style-guide compliance for every touched package against a sibling reference package, and (2) objective bugs surfaceable from the diff alone (wrong loop bounds, swapped arguments, copy-paste leftovers, wrong return values, broken assertions, mismatched signatures, doc examples that don't match code). Excluded, deliberately: subjective preferences, "might be" concerns, anything requiring interpretation, and anything that would require touching code outside the window's diff to validate. Five candidate findings that failed those bars were dropped — most notably a suggestion to override commit
dbaac4d3's (Reines) deliberate widening of the alt-hypothesis ndarray type on thedztest/dztest2/sztest/sztest2type declarations, and a cgemv REPL fix that would have required registering a new REPL alias for@stdlib/blas/base/transpose-operation-resolve-enum(out of scope).Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was authored by a scheduled Claude Code review routine that swept the last 24h of commits on
develop, ran four parallel reviewer agents (two style-guide, two bug-hunt), merged and de-duplicated their findings against a high-signal filter, and applied the surviving fixes. A human maintainer will audit before promoting from draft.