fix(extract): invalidate tsconfig alias and baseUrl caches on edit (#2917) - #2940
fix(extract): invalidate tsconfig alias and baseUrl caches on edit (#2917)#2940ousamabenyounes wants to merge 1 commit into
Conversation
…raphify-Labs#2917) `_TSCONFIG_ALIAS_CACHE` and `_TSCONFIG_BASEURL_CACHE` were keyed on the config path alone, with no mtime component and no invalidation anywhere. Anything that calls `extract()` more than once in one process — `graphify watch`, the MCP server, library loops — kept resolving imports against the `compilerOptions` read on the first run, so retargeting a `paths` alias mid-session silently wired every subsequent rebuild to the previous directory. Both caches now key on the config's mtime, mirroring the manifest-mtime key `_load_workspace_packages` already uses, so direct `extract_js()` callers expire too; and `extract()` clears them beside the workspace cache, which covers an alias inherited through an `extends` chain, whose base config is not in the leaf's key.
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 1 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Keys the tsconfig/jsconfig paths and baseUrl caches (_TSCONFIG_ALIAS_CACHE, _TSCONFIG_BASEURL_CACHE) by config path + mtime via a new _js_config_cache_key, and clears both at the start of extract() so an extends base-config edit invalidates too. This lets graphify watch, the MCP server, and repeated extract()/extract_js() calls pick up an edited config instead of pinning imports to the previous alias/baseUrl target for the life of the process (#2917). Adds tests covering alias and baseUrl retargeting across a second extract and directly through the cache without a full extract.
Worth a look
- extends-base config edit not caught by mtime cache key without per-run clear —
graphify/extractors/resolution.py:210· 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 — 1956 functions depend on the 553 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 478 callers, 42 callees - new:
_rebuild_code()— 98 callers, 50 callees - new:
_extract_generic()— 18 callers, 24 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_objc()— 27 callers, 9 callees - new:
extract_js()— 80 callers, 3 callees - new:
dispatch_command()— 2 callers, 119 callees - new:
_resolve_js_module_path()— 27 callers, 6 callees - …and 41 more — each is listed as a finding
Verification — 1956 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: 1808 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract.
The verifier did not have enough to check extract, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `cache_root` is annotated `Path | None` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_load\_tsconfig\_aliases.
The verifier did not have enough to check \_load\_tsconfig\_aliases, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `start_dir` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_load\_tsconfig\_base\_url.
The verifier did not have enough to check \_load\_tsconfig\_base\_url, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `start_dir` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 48 more finding(s) on lines outside this diff (see the check run).
| return str((str(config), mtime)) | ||
|
|
||
|
|
||
| def _load_tsconfig_aliases(start_dir: Path) -> dict[str, list[str]]: |
There was a problem hiding this comment.
_load_tsconfig_aliases()
8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Fixes #2917.
The bug
_TSCONFIG_ALIAS_CACHE/_TSCONFIG_BASEURL_CACHEare keyed on the config pathstring, with no mtime component and no invalidation anywhere in the package —
unlike
_WORKSPACE_PACKAGE_CACHE, which is both mtime-keyed and cleared per run.So for any process that calls
extract()more than once (graphify watch, theMCP server, library loops), an edit to
compilerOptions.pathsorbaseUrlisnever observed again. It fails silently: the import edges still exist and still
look plausible, they just point at the previous target.
The issue's repro, verbatim, on
v8(b2cd362):and after this PR:
The fix
Two layers, because the two callers differ:
_load_workspace_packagesalready uses in the same module. This coversextract_js(), which reads both caches directly and never goes throughextract().extract()clears both caches beside the workspace/XAML/markdown caches.This is not redundant: an alias inherited through an
extendschain is keyedon the leaf config only, so an edit to the base config needs the run
boundary. That leaf-only key is the same scope
_load_workspace_packageshas(root manifests, not transitive includes) — happy to widen the key to the
whole
extendschain if you'd rather close that case too.A
stat()on a config that vanished between theexists()probe and the keyread falls back to a distinct sentinel key, so the miss is recomputed rather
than served stale.
Test verification (RED → GREEN)
RED — the three new tests against unmodified
v8, production files stashed:with the leak visible in the assertion:
GREEN — same file with the fix:
Full suite, unchanged against the
v8baseline (the 4 failures are pre-existingin my local environment — 3
test_ollamabackend-detection tests read env vars Ihave set, plus
test_collect_files_skips_hidden— all 4 fail identically onclean
v8):ruff checkclean ongraphify/and the touched test file; the fivetools.skillgenvalidators andgraphify --helpall pass.The two existing per-run alias tests (
test_tsconfig_paths_alias_unchanged,test_tsconfig_wins_when_both_configs_present) are untouched and still pass, soalias precedence (#1269, #927, #1531, #2153) is unaffected.