fix(cli): pin PYTHONHASHSEED for update/extract/cluster-only/label - #3743
ayushcodes10 wants to merge 3 commits into
Conversation
The generated git hooks already export PYTHONHASHSEED=0 because networkx's Louvain implementation iterates string keyed sets whose order is randomized per process, so community assignments otherwise churn between runs with no code change. A bare graphify update, which the CLAUDE.md template tells agents to run after every code change, skipped this, clustering differently every time and producing a large, spurious diff in the output directory. PYTHONHASHSEED is read once at interpreter startup, so setting it on os.environ from inside an already running process has no effect on that process's own hash randomization. The only way to pin it for a command already in flight is to restart the interpreter with it set from the start, so main() now re execs itself with the seed set before doing anything else, for exactly the commands whose output depends on clustering, only when the caller has not already chosen a seed themselves, degrading instead of raising if the re exec itself fails so an unusual host that disallows it can still run graphify. Skips this entirely while running under pytest, detected via the PYTEST_CURRENT_TEST variable pytest itself sets for the duration of a running test. Dozens of existing tests call main() directly with a monkeypatched argv to simulate a full CLI run in process, which only worked because main() was previously free of this kind of side effect; without the guard, every one of those calls replaced the test process running them the moment PYTHONHASHSEED happened to be unset, which it normally is in a dev or CI environment, with no traceback or error, just an empty, silently green test run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
Since a real re exec replaces the process it runs in, every test that needs to observe whether it fired runs a small probe in its own subprocess with a deliberately constructed environment, rather than mocking inside the pytest process itself, which pytest's own handling of its current test marker would defeat anyway. Cover each of the four affected commands re executing with the seed pinned when unset, an explicit seed never being overridden, unrelated commands never re executing, the guard against firing while pytest itself is mid test, a failed re exec being survived rather than raised, and a full real invocation of update still completing successfully end to end through the whole re exec. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Graphify review — findings
Pins PYTHONHASHSEED=0 for graphify update/extract/cluster-only/label by re-execing the interpreter via _pin_hash_seed_if_needed before the CLI runs, so Louvain community assignments stay stable run-to-run and a bare graphify update . no longer produces a spurious re-clustered diff on every invocation. Leaves an explicitly-set PYTHONHASHSEED alone, skips the re-exec entirely under pytest (PYTEST_CURRENT_TEST), and degrades silently rather than raising if os.execvpe fails so unusual hosts can still run graphify. Adds tests/test_pin_hash_seed.py, which drives the guard from real subprocesses to observe the re-exec and its pinned env.
Worth a look
- main() can now replace an embedding process —
graphify/__main__.py:537· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Hash-seed reexec runs Windows console launcher as Python source —
graphify/__main__.py:527· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 420 functions depend on the 236 functions this change touches.
Health — this change adds coupling hotspots:
- new:
main()— 98 callers, 3 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 420 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 420 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
291 of 291 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/test_affected_cli.py— impact, full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.py— impact, full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_merge_dedup_scope.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cluster.py— full-run-safetytests/test_codebuddy.py— impact, full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— full-run-safetytests/test_cross_language_call_resolution.py— full-run-safetytests/test_cross_repo_external_call_guards.py— full-run-safetytests/test_cross_repo_member_calls.py— full-run-safetytests/test_cross_repo_shared_types.py— full-run-safetytests/test_csharp_call_site_generic_args.py— full-run-safety- … and 241 more
non-code file(s) changed (
CHANGELOG.md) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
CHANGELOG.md) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
· 1 grounded finding(s) anchored inline below; 2 more finding(s) on lines outside this diff (see the check run).
| pass | ||
|
|
||
|
|
||
| def main() -> None: |
There was a problem hiding this comment.
main()
98 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Shipped in v0.9.66 (on PyPI). Cherry-picked with authorship preserved so it shows under your GitHub contributions. Thanks @ayushcodes10! (Nice — the re-exec approach is exactly right for pinning the hash seed.) |
2 participants
Fixes #3641.
The generated git hooks already export
PYTHONHASHSEED=0because networkx's Louvain implementation iterates string-keyed sets whose order is randomized per-process, so community assignments otherwise churn between runs with no code change. A baregraphify update .— which theCLAUDE.mdtemplate tells agents to run after every code change — skipped this, re-clustering differently every time and producing a large, spuriousgraphify-outdiff (24% of node-community assignments flipped in one real repo per the issue's own measurement, also triggering the GitHub diff timeout from #3413).PYTHONHASHSEEDis read once at interpreter startup — I verified empirically that setting it onos.environfrom inside an already-running process has zero effect on that process's own hash randomization (confirmed by hashing the same string twice, with and without a mid-processos.environwrite, in separate runs — the hash still differs run to run). The only correct fix is to restart the interpreter with it set from the start, somain()now re-execs itself viaos.execvpebefore doing anything else, scoped to exactlyupdate/extract/cluster-only/label(the commands whose output depends on clustering), and only when the caller hasn't already chosen a seed themselves. Degrades instead of raising if the re-exec itself fails.A real bug I hit and fixed along the way: my first version had no pytest guard, and broke ~34+ existing tests across multiple files that call
graphify.__main__.main()directly with a monkeypatchedsys.argvto simulate a full CLI run in-process (e.g.test_extract_cli.py). Sincemain()now called a realos.execvpeas its first action, every such test call replaced the pytest worker process itself — silently, with zero output and no traceback, just an empty green-looking run. Fixed by skipping the pin whilePYTEST_CURRENT_TEST(the env var pytest itself sets for the duration of a running test) is present.Verified end-to-end via real subprocess invocation (
python -m graphify update .withPYTHONHASHSEEDunset) that the fix works and the re-exec chain correctly avoids an infinite loop — the second pass, now with the seed already set, sees the guard and proceeds normally.Tests in
tests/test_pin_hash_seed.py: since a real re-exec replaces the process it runs in, every test that needs to observe whether it fired runs a small probe in its own subprocess with a deliberately constructed environment, rather than mocking inside the pytest process itself (which pytest's own re-setting ofPYTEST_CURRENT_TESTfor the "call" phase — after fixtures resolve — would defeat anyway). Covers each affected command re-executing when unset, an explicit seed never being overridden, unrelated commands never re-executing, the pytest guard itself, graceful degradation on a failed re-exec, and a full realupdateinvocation completing successfully end-to-end. Full suite (5721 passed, including the previously-broken CLI test files) is green.🤖 Generated with Claude Code
https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh