Skip to content

fix: correct misleading lint hint for crypto.getRandomValues - #1729

Closed
AGRO-CODEX wants to merge 1 commit into
heygen-com:mainfrom
AGRO-CODEX:fix/crypto-hint-non-deterministic
Closed

fix: correct misleading lint hint for crypto.getRandomValues#1729
AGRO-CODEX wants to merge 1 commit into
heygen-com:mainfrom
AGRO-CODEX:fix/crypto-hint-non-deterministic

Conversation

@AGRO-CODEX

Copy link
Copy Markdown
Contributor

What

Corrects the fixHint message for the crypto.getRandomValues() pattern in the non_deterministic_code lint rule (packages/core/src/lint/rules/core.ts).

Why

The previous hint said "Remove time-dependent code. Use a seeded PRNG for deterministic renders." but crypto.getRandomValues() is not time-dependent — it is a source of randomness. The "time-dependent" label is accurate for Date.now(), new Date(), and performance.now(), but is factually wrong for the crypto case.

Change

The new hint follows the same pattern as the Math.random() entry:

Replace with a seeded PRNG (e.g. a simple mulberry32) so renders are deterministic across frames.

No logic changes — lint behaviour, severity, and error codes are untouched.

The hint for the crypto.getRandomValues() non-determinism rule incorrectly
said 'Remove time-dependent code', but crypto.getRandomValues() is a source
of randomness, not time-dependence. Replaced with the same pattern used by
the Math.random() hint, which correctly points to a seeded PRNG.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving per James — tiny correctness fix on the non_deterministic_code lint rule.

crypto.getRandomValues() was getting the "Remove time-dependent code" hint, which is the right text for Date.now() / new Date() / performance.now() but wrong for a randomness source. The new hint mirrors the Math.random() entry ("Replace with a seeded PRNG …"), which is the structurally identical case. No logic change, no rule-behavior change, no severity change. Free quality.

— Jerrai

@jrusso1020

Copy link
Copy Markdown
Collaborator

@AGRO-CODEX can you sign your commit please? it's not mergable without that

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Closing in the backlog value sweep. This is documentation, test-only, dependency upkeep, refactoring, or narrow diagnostic/ergonomic polish without enough current user impact or an active merge path to justify carrying it open. Please reopen or resubmit against current main if the need is still concrete.

@miguel-heygen miguel-heygen reopened this Jul 14, 2026
jrusso1020 added a commit that referenced this pull request Sep 4, 2026
Builds on #1729 in the extracted lint package.

Co-authored-by: AGRO-CODEX <brstopo@gmail.com>
@jrusso1020

Copy link
Copy Markdown
Collaborator

Closing now that the refreshed, attributed successor #3659 has merged. It preserves this contribution and was checked against the current implementation and reviewed before landing. Thanks for the original work.

@jrusso1020 jrusso1020 closed this Sep 4, 2026
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.

3 participants