Bump dependencies - #1766
Conversation
fix lfm2.5 vl template
|
CI errors blocked by microsoft/onnxruntime#32464, but a fix is in progress at microsoft/onnxruntime#32469 |
|
nice, CI is green :) |
There was a problem hiding this comment.
🟡 Changes recommended
It introduces a translation crash, silently drops documented Node 18 support, disables MusicGen coverage, and conflicts with the stated ONNX Runtime versions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates core runtime, tokenizer, template, and image dependencies, with compatibility fixes and adjusted tests.
Changes:
- Bumps Jinja, tokenizers, ONNX Runtime, and Sharp.
- Adapts RFFT/DFT handling and adds tensor remainder operations.
- Updates model expectations, typings, and Node.js CI versions.
File summaries
| File | Description |
|---|---|
pnpm-lock.yaml |
Resolves upgraded dependency trees. |
packages/transformers/package.json |
Updates direct dependencies. |
.github/workflows/tests.yml |
Changes tested Node.js versions. |
packages/transformers/src/utils/tensor.js |
Adds remainder operations and adapts RFFT input. |
packages/transformers/src/ops/registry.js |
Updates the serialized DFT operator. |
packages/transformers/src/tokenization_utils.js |
Handles revised post-processor configuration. |
packages/transformers/src/utils/image.js |
Updates the Sharp type reference. |
packages/transformers/src/models/vits/tokenization_vits.js |
Suppresses a tokenizer decoder type error. |
packages/transformers/tests/utils/tensor.test.js |
Tests remainder behavior. |
packages/transformers/tests/models/musicgen/test_modeling_musicgen.js |
Skips MusicGen tests. |
packages/transformers/tests/models/lfm2/test_modeling_lfm2.js |
Updates generated-token expectations. |
packages/transformers/tests/models/arcee/test_modeling_arcee.js |
Updates generated-token expectations. |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 11/12 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…transformers.js into bump-dependencies-2
There was a problem hiding this comment.
🔵 Needs a closer look
Remainder mishandles zero divisors for integer tensors and RFFT silently normalizes invalid axes.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
packages/transformers/src/utils/tensor.js:361
- A zero divisor silently produces incorrect results for Number-backed integer tensors:
%yieldsNaN, which integer typed arrays coerce to0(while bigint tensors throw). Since this API advertises Python-style modulo semantics, reject zero before entering the loop so all numeric dtypes behave consistently.
packages/transformers/src/utils/tensor.js:1183 - Using modulo to normalize the axis also accepts invalid dimensions: for a rank-2 input, axis
2becomes0and axis-3becomes1, whereas the documentedtorch.fft.rfftbehavior must reject axes outside[-2, 1]. Validate the original scalar axis with the existingsafeIndexhelper before applyingremainder.
- Files reviewed: 18/19 changed files
- Comments generated: 0 new
- Review effort level: Balanced
nico-martin
left a comment
There was a problem hiding this comment.
two nit-picks, otherwise lftm :)
There was a problem hiding this comment.
🟡 Changes recommended
The remainder calculation is numerically incorrect for large divisors, and the advertised Node.js floor does not match Sharp’s requirement.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
packages/transformers/package.json:62
sharp@0.35.4requires Node>=20.9.0(pnpm-lock.yaml:1611-1613), but this published package still has noenginesdeclaration and the updated compatibility docs promise Node 20+. Consequently Node 20.0–20.8 are advertised as supported even though a direct dependency rejects them. Declare the package minimum as>=20.9.0and align the changed compatibility text (or use a compatible Sharp release).
"sharp": "^0.35.4"
- Files reviewed: 19/20 changed files
- Comments generated: 1
- Review effort level: Balanced
sharp->0.35.4@huggingface/jinja->0.5.10@huggingface/tokenizers->0.2.0onnxruntime-web->1.30.0-dev.20260909-e76036b8eonnxruntime-node->1.30.0-dev.20260909-e76036b8ecommit log looks a bit messy, but it's because it's including everything from #1665 (won't be shown when we squash)
increases required node.js version to >= 20 due to
sharpdependency.Closes #1687
Closes #1707
Closes #1718
Closes #1727
Closes #1729
Closes #1730
Closes #1731
Closes #1761