fix(utils): prevent numeric overflow to NaN in getColorFromString for long strings - #818
fix(utils): prevent numeric overflow to NaN in getColorFromString for long strings#818rudra496 wants to merge 3 commits into
Conversation
… long strings Fixes tscircuit#651 by coercing hash calculation to 32-bit integer arithmetic and using Math.abs(hash) % 360 to guarantee valid HSL color strings.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d979b004f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // 32-bit integer pseudo-random hash from string to prevent overflow to Infinity | ||
| const hash = string.split("").reduce((acc, char) => { | ||
| return acc * 31 + char.charCodeAt(0) | ||
| return (acc * 31 + char.charCodeAt(0)) | 0 |
There was a problem hiding this comment.
When the existing SVG snapshot tests run, this truncates every intermediate hash to 32 bits, so ordinary finite IDs change color even though they did not hit the long-string overflow path; for example connectivity_net1 in the committed example snapshots goes from hsl(40, ...) with the previous reducer to hsl(196, ...) here. The solver snapshot tests compare the generated SVG colors exactly via toMatchSolverSnapshot, and no snapshot fixtures were updated in this commit, so the visual test suite will fail for snapshots containing these IDs unless the finite-string hues are preserved or the affected snapshots are regenerated.
Useful? React with 👍 / 👎.
…nity overflow in getColorFromString
Summary
Fixes #651.
Previously, `getColorFromString` computed its hash accumulator with floating-point arithmetic without bitwise integer coercion (`acc * 31 + char.charCodeAt(0)`). For long net/chip strings (e.g. `"n".repeat(300)`), this overflowed to `Infinity`, resulting in `hsl(NaN, 100%, 50%, 1)`.
Changes
Verification