fix: make Config.fingerprint deterministic across processes#5877
fix: make Config.fingerprint deterministic across processes#5877etonlels wants to merge 1 commit into
Config.fingerprint deterministic across processes#5877Conversation
007aeef to
93675d4
Compare
`Config.fingerprint` is `crc32(pickle.dumps(config.dict()))` and is used as part of the on-disk model cache key. Config fields such as `linter.rules` are `set`s, and set/frozenset iteration order is not stable across Python processes. Pickling them directly therefore produces different bytes each run, so the fingerprint — and every cache entry keyed by it — changes on every invocation. The practical effect is that the model definition cache never hits across processes: every model is re-parsed on each load. On a 45-project monorepo (~1340 models) this made a warm-cache load re-parse everything through the fork pool, ~74s -> ~44s once the cache actually hits. Fix by canonicalizing the config dict before hashing: recursively sort set/frozenset members into lists so serialization is order-stable while preserving contents. Lists/tuples/dicts are recursed into; other values are unchanged. Co-Authored-By: OpenCode google-vertex/claude-opus-4-8@default <noreply@opencode.ai> Signed-off-by: etonlels <etonlels@gmail.com>
93675d4 to
24b22f5
Compare
|
@etonlels Thnks for this PR! One thought - do you think it's worth considering also sorting dict keys in |
|
@StuffbyYuki I'm curious what your thoughts are. Python |
|
That's a good point. Let's keep it as is. I'll merge it once it ran workflows again |
Description
Config.fingerprintiscrc32(pickle.dumps(config.dict()))and is used as part of the on-disk model cache key. Config fields such aslinter.rulesaresets, and set/frozenset iteration order is not stable across Python processes. Pickling them directly therefore produces different bytes each run, so the fingerprint — and every cache entry keyed by it — changes on every invocation.The practical effect is that the model definition cache never hits across processes: every model is re-parsed on each load. On a 45-project monorepo (~1340 models) this made a warm-cache load re-parse everything through the fork pool, ~74s -> ~44s once the cache actually hits.
Fix by canonicalizing the config dict before hashing: recursively sort set/frozenset members into lists so serialization is order-stable while preserving contents. Lists/tuples/dicts are recursed into; other values are unchanged.
Test Plan
Added unit + regression tests in
tests/core/test_config.py:test_canonicalize_sorts_sets,test_canonicalize_recurses_into_containers,test_canonicalize_preserves_non_set_values_and_types— cover the new_canonicalizehelper: sets/frozensets sort to lists, nested dicts/lists/tuples recurse, list/tuple types are preserved, and non-set values pass through unchanged.test_config_fingerprint_is_deterministic_across_processes— the regression guard. It builds aConfigwith a set-valuedlinter.rulesand computesconfig.fingerprintin two subprocesses run with differentPYTHONHASHSEEDvalues, asserting the results match. This reproduces the original bug: it fails onmain(the two fingerprints differ) and passes with this change.Also verified end-to-end on a real 45-project (~1340 model) monorepo: before the fix the model-definition cache had zero cross-process hits (every model re-parsed on each run); after the fix a warm-cache load hits the cache and drops from ~74s to ~44s.
Checklist
make styleand fixed any issuesmake fast-test)git commit -s) per the DCO