perf(ids): memoize normalize_id - #3645
abhay-codes07 wants to merge 1 commit into
Conversation
normalize_id is a pure, deterministic str->str transform — up to six casefold+NFKC iterations plus two regex passes (~1.3us each) — and every node id in the pipeline flows through it via make_id, almost always on a repeating handful of file stems and symbol names (a file's stem is normalized once per node it owns; the same identifiers recur across files). lru_cache collapses those repeats: ~13x on the repeated-input pattern (31.6ms -> 2.4ms per 24k calls). The result depends only on the input string, so there is nothing to invalidate; the cache is bounded so a pathological corpus cannot grow it without limit. Output and the documented invariants (idempotent, caseless-stable, word-only) are unchanged, verified against the pre-memo implementation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q
![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.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Memoizes normalize_id with a bounded lru_cache (262144 entries) so the repeated stems and identifiers every node id runs through via make_id are computed once instead of re-running six casefold+NFKC passes and two regex substitutions each time. Adds tests pinning cache behaviour against a verbatim pre-memo reference oracle and confirming the documented invariants (idempotent, caseless-stable, \w-only output) and hit/miss counts still hold.
No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 3029 functions depend on the 14 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 645 callers, 45 callees - new:
_rebuild_code()— 129 callers, 54 callees - new:
build_from_json()— 214 callers, 20 callees - new:
deduplicate_entities()— 77 callers, 24 callees - new:
build_merge()— 76 callers, 14 callees - new:
save_semantic_cache()— 63 callers, 9 callees - new:
_extract_generic()— 18 callers, 29 callees - new:
extract_bash()— 45 callers, 10 callees - …and 106 more — each is listed as a finding
Verification — 3029 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: 704 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
113 of 287 test file(s) selected (39%) via static blast radius.
tests/test_analyze.py— impacttests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_build.py— impacttests/test_build_merge_dedup_scope.py— impacttests/test_build_merge_hyperedges_and_prune.py— impacttests/test_build_merge_shrink_guard.py— impacttests/test_cache.py— impacttests/test_carried_hyperedge_remap.py— impacttests/test_charmap_encoding.py— impacttests/test_chunking.py— impacttests/test_cjs_module_extension.py— impacttests/test_cli_export.py— impacttests/test_cluster.py— impacttests/test_confidence.py— impacttests/test_corrupt_graph_json.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_csharp_type_resolution.py— impacttests/test_dart.py— impacttests/test_dedup.py— impacttests/test_dedup_remaps_hyperedges.py— impacttests/test_dedup_survivor_richness.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_evidence_binding.py— impacttests/test_export.py— impacttests/test_export_control_characters.py— impacttests/test_external_stub_endpoints.py— impacttests/test_extract.py— impacttests/test_extract_cache_location.py— impacttests/test_extract_cli.py— impacttests/test_falkordb_integration.py— impacttests/test_file_label_disambiguation.py— impacttests/test_forwarding_review_findings.py— impacttests/test_global_graph.py— impacttests/test_go_qualified_resolution.py— impacttests/test_hyperedge_member_shapes.py— impacttests/test_hyperedge_roundtrip.py— impacttests/test_hypergraph.py— impacttests/test_id_normalization_contract.py— impacttests/test_import_extension_resolution.py— impacttests/test_import_self_loops.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_issue_3472_source_file_collision.py— impact- … and 63 more
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.
Formal verification
No difference found (not proven): No behavior difference found in normalize\_id (not a proof).
The verifier ran both versions of normalize\_id on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 114 more finding(s) on lines outside this diff (see the check run).
What
normalize_idis the canonical-id transform every node id flows through (viamake_id/_make_id, called from the engine, extract and build — 64/92/20 call sites). Each call runs up to sixcasefold+NFKCiterations (they don't commute, so the code iterates to a fixpoint — see the function's own docstring) plus two regex passes: ~1.3µs each. And it is called on a small, heavily-repeating set of strings — a file's stem is normalized once per node it owns, and the same symbol names recur across files.The change
Memoize
normalize_idwith a boundedfunctools.lru_cache. It is a pure, deterministicstr -> strfunction of its input alone — nothing to invalidate — so caching is exact; the bound keeps a pathological corpus from growing it without limit.Measured
Microbenchmark over the repeating-input pattern the pipeline exhibits (best of 5):
normalize_id, 24k callsCorrectness
Output is unchanged, and the three documented, test-enforced invariants still hold — idempotent (
normalize_id(normalize_id(s)) == normalize_id(s)), caseless-stable (normalize_id(s) == normalize_id(s.casefold())), and word-only output — verified against the pre-memo implementation across ASCII, casing, punctuation, whitespace, and combining-mark/Unicode cases (İstanbul,résumé,straße, Greek ypogegrammeni). Thetest_id_normalization_contractsuite (131 tests) passes unchanged.Tests
tests/test_normalize_id_memo.py— equivalence with the unmemoized reference; the documented invariants; repeated inputs hit the cache (1 miss / 99 hits); and distinct inputs map distinctly through the cache. The full suite matches a freshv8(0.9.63) baseline (zero new failures).🤖 Generated with Claude Code
https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q