fix(csv-generate): make numeric zero seeds deterministic - #519
Open
wanghanyu654321-cell wants to merge 1 commit into
Open
wanghanyu654321-cell wants to merge 1 commit into
wanghanyu654321-cell wants to merge 1 commit into
Conversation
Normalize zero to the existing nonzero starting seed so repeatability does not introduce an all-empty ASCII recurrence. Cover sync, callback, stream and bounded first-record behavior without changing stream accounting.
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
1 participant
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.
Numeric
seed: 0currently selectsMath.random(), so identical generator options produce different output despite the documented numeric-seed repeatability contract.Normalize numeric zero to the existing nonzero starting seed (
1). This makes zero repeatable while leavingfalse, the default, booleans and nonzero seeds on their existing paths. Zero intentionally shares the sequence of seed1: letting the recurrence run from zero instead would produce only empty ASCII fields and prevent an unbounded object stream from yielding.Adds public sync, callback and async-iteration regressions, plus a first-record regression in a child process with a 10-second deadline and 32 MB heap cap. The original implementation fails the three repeatability tests; the final change passes all nine seed tests. No stream accounting changes or new dependencies.
Validation on Node 20.20.2, 22.23.2 and 24.21.0 / Linux:
npm run test -- --concurrency=1: all 14 project test tasks, including configured TypeScript checks, pass.npm run lint:check -- .: passes.npm run build -- --concurrency=1: all 7 build tasks pass.Dependencies were installed with
npm ci --ignore-scripts; the reviewed official DuckDB prebuilt binding was installed for the existing sample test. Node 20 emits existing development-tool engine warnings, but all checks above finish successfully. Early Windows quoting/CRLF and Docker interruption attempts were excluded from the passing evidence; final checks used Git LF sources and one container at a time.AI assistance: Codex implemented the change and tests and a separate Codex review assessed the diff. This disclosure does not claim personal human review or successful upstream CI.