[binaryaaron]

📋 Summary

Children forked after a parent run generated identical sampler data, because samplers draw from numpy's global RandomState and Faker's shared Random, and a fork copies their state. This PR adds an os.register_at_fork(after_in_child=...) hook that reseeds both from OS entropy in the child, as CPython already does for the stdlib random module.

🔗 Related Issue

Fixes #1076

🔄 Changes

• sampling_gen/utils.py: _reseed_shared_rngs_after_fork() calls numpy.random.seed() and faker.generator.random.seed() in a forked child. It reseeds only modules already imported, so the hook adds no imports. It is registered next to check_random_state(), which data_designer.interface imports, so the hook is in place before a user can fork.
• architecture/sampling.md: a design-decision entry on shared random state, in-process seeding and the fork hook.

In-process seeding is unchanged: np.random.seed(n) and Faker.seed(n) still make sampler output repeatable in one process, which scripts/benchmarks/benchmark_engine_v2.py relies on. The parent's generators are not touched. A forked child that needs repeatable output seeds itself after the fork, as with the stdlib random module.

I considered giving each run its own generator seeded from OS entropy instead of reusing the global one. That fixes forks too, but breaks repeatable output for anyone who seeds numpy.random, including the benchmark, so I kept the global generators and reseed only in the child.

🧪 Testing

• make test passes: one pre-existing failure, see below
• Unit tests added/updated
• E2E tests added/updated (if applicable): N/A, no model endpoint needed

Test	Checks	Fails without the fix
test_forked_children_draw_different_samples (engine)	two children forked after the parent drew from numpy and Faker draw different values from each	yes; the Faker assertion also fails on its own when only the Faker reseed is removed
test_forked_child_reseed_does_not_change_parent_draws (engine)	a seeded parent's sequence is the same whether or not it forks	n/a, guards the parent
test_sibling_forked_children_generate_different_sampler_data (interface)	the issue's reproduction: two children forked after a parent preview() generate different category and uniform columns	yes

The fork tests run in a fresh interpreter through the existing helpers (tests/engine/fork_probe.py, and the subprocess pattern in test_fork_safety.py).

make test (CPython 3.14.0, uv 0.13.4): config 671 passed, engine 2610 passed, data-designer 1204 passed, 3 skipped, and one failure, test_create_dataset_e2e_with_file_contents_seed_source_unreadable_file_raises_generation_error, which fails on unmodified main when run as root. Ruff check and format pass on the changed files.

✅ Checklist

• Follows commit message conventions
• Commits are signed off (DCO)
• If I am an external contributor, the linked issue is open and labeled triaged: N/A
• Architecture docs updated (if applicable)

🤖 Generated with Claude Code

[github-actions]

Code Review: PR #1077 — fix: reseed shared sampler RNGs in forked children

Note: the review-code skill is set to user-invocation only, so the agent could not run it in this CI session. This review was done by hand: I read the diff, checked the surrounding code, and ran a few local probes (results below).

Summary

The PR registers an os.register_at_fork(after_in_child=...) hook in sampling_gen/utils.py. In a forked child, the hook reseeds numpy's global RandomState and Faker's shared random.Random from OS entropy, the same way CPython already reseeds the stdlib random module. This fixes #1076: children forked after a parent run no longer generate identical sampler data. Seeding inside a single process (np.random.seed, Faker.seed) still works, and the parent's RNGs are never touched. The change is small, well documented, and has tests that fail without it.

Verified locally (CPython 3.14.3, numpy 2.4.3):

• The new tests and the existing ones in test_utils.py / test_fork_safety.py pass (9 passed). ruff check and ruff format --check are clean on the changed files.
• A manual end-to-end probe, with a parent that only imports DataDesigner and then forks two children that each call preview() with UNIFORM and PERSON_FROM_FAKER columns, gives different data in each child.
• faker.generator.random is the same object that Faker.seed() reseeds (Generator.seed), so the Faker reseed hits the right RNG.

Findings

Medium / Low

1. The hook can import numpy.random in the child, which goes against its own comment — sampling_gen/utils.py, _reseed_shared_rngs_after_fork
   The comment says "Only reseed modules already imported, to keep the hook from importing them." But the guard looks for sys.modules.get("numpy") and then calls numpy.random.seed(). Since numpy 2.x, numpy.random loads lazily through numpy.__getattr__, so import numpy alone does not load it. I checked this directly: with numpy loaded but numpy.random not loaded in the parent, numpy.random is in sys.modules in the child right after os.fork(). The hook imported it.
   The result is still correct. But every fork from a process that has numpy without numpy.random (for example, a user's own multiprocessing workers) now pays for that import inside an at-fork hook, which is what the comment says the code avoids. In that case the reseed isn't needed either: the child creates its global RandomState fresh from entropy on first use.
   Suggested fix:

   np_random = sys.modules.get("numpy.random")
   if np_random is not None:
       np_random.seed()

   A small test would lock this in: fork with only numpy imported, then assert "numpy.random" not in sys.modules in the child.

Nits

2. The PR description overstates when the hook is registered. It says the hook is registered next to check_random_state(), which data_designer.interface imports. In practice, import data_designer.interface (and import data_designer.config / data_designer.engine) does not load sampling_gen.utils. Only from data_designer.interface import DataDesigner does, through the lazy attribute. Normal use goes through DataDesigner, and samplers always import utils before they draw, so I don't see a real failure case. The wording in the PR body and in architecture/sampling.md could just be more precise. The leftover gap: a process that imports numpy, forks, and only imports sampling_gen.utils in the children would leave sibling children sharing the copied global RandomState. That is very unlikely and arguably the caller's job.

3. Per-instance Faker RNGs are not covered, which is expected. Faker.seed_instance() gives a Faker instance its own Random, which the hook doesn't reseed. That matches the "a child that wants repeatable output seeds itself" contract, and the engine's PeopleGenFaker uses the shared RNG, so nothing needs to change. A short mention in the architecture note could save someone from confusion later.

4. Test style: test_sibling_forked_children_generate_different_sampler_data repeats the multiprocessing fork/queue boilerplate inline instead of reusing run_forked_child from tests/engine/fork_probe.py. This matches the existing test_fork_safety.py pattern, and the helper lives in another package's test tree, so this is acceptable as is.

Positives

• The fix is small and targeted, and it copies CPython's own behavior for random, so it's easy to reason about.
• Rejecting the per-run-entropy alternative was the right call: keeping np.random.seed(n) repeatable protects existing users and the benchmark script.
• The tests cover both sides: forked children diverge, and a seeded parent's sequence is unchanged when it forks. Each test runs in a fresh interpreter, which avoids the ENOMEM-on-fork issue that fork_probe.py documents.
• The hasattr(os, "register_at_fork") guard keeps Windows safe.
• The architecture doc is updated.

Structural Impact (graphify 0.9.90, 8.3s)

Risk: LOW (localized change)

• 3 Python files, 4 AST entities, 1/205 clusters

High-Connectivity Changes

• sampling_gen/utils.py (8 deps) in packages/data-designer-engine/src/data_designer/engine/sampling_gen/utils.py
• check_random_state() (6 deps) in packages/data-designer-engine/src/data_designer/engine/sampling_gen/utils.py

The change adds a module-level side effect (registering the at-fork hook) to a module with many dependents. It doesn't change any API or import direction. Engine → config layering is kept, and the hook doesn't import anything new.

Verdict

Looks good, with one small fix suggested. The fix is correct and well tested. Before merging, I'd recommend guarding on sys.modules.get("numpy.random") so the hook really doesn't import anything in the child (finding 1). The rest are nits.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable defects were established.

Summary

The PR reseeds shared NumPy and Faker generators after a fork without changing the parent’s generators.

• Forked children now start sampler runs with fresh random state.

No actionable issues were established. Tests were not run during this review.

_{Reviews (1) · Last reviewed commit: "fix: reseed shared sampler RNGs in forke..." · Reviewed by Greptile}