Skip to content

fix(extract): invalidate tsconfig alias/baseUrl caches per run - #2918

Open
sashankh wants to merge 1 commit into
Graphify-Labs:v8from
sashankh:fix/tsconfig-cache-invalidation
Open

fix(extract): invalidate tsconfig alias/baseUrl caches per run#2918
sashankh wants to merge 1 commit into
Graphify-Labs:v8from
sashankh:fix/tsconfig-cache-invalidation

Conversation

@sashankh

Copy link
Copy Markdown

The problem

extract() opens by resetting the process-global caches whose backing files can change
between runs:

# Workspace package manifests/globs can change during watch or repeated extraction.
_WORKSPACE_PACKAGE_CACHE.clear()
_XAML_CSHARP_CLASS_CACHE.clear()
_MD_LINK_INDEX_CACHE.clear()

_MD_LINK_INDEX_CACHE spells the contract out:

extract() clears it at the start of each run (beside _WORKSPACE_PACKAGE_CACHE) so a
serial rerun in one process sees files created since the last run.

_TSCONFIG_ALIAS_CACHE and _TSCONFIG_BASEURL_CACHE are the same kind of state — keyed on
the config path string, no mtime component, no invalidation anywhere in the package — but
they were not in that block. A tsconfig.json is at least as mutable as a workspace
manifest, so an edit to compilerOptions.paths or baseUrl was never observed again for the
life of the process:

key = str(config)
if key not in _TSCONFIG_ALIAS_CACHE:
    _TSCONFIG_ALIAS_CACHE[key] = _read_tsconfig_aliases(config, candidate, seen=set())
return _TSCONFIG_ALIAS_CACHE[key]

That matters for exactly the callers the neighbouring comment was written for — graphify watch, the MCP server, and library code calling extract() in a loop. It fails silently:
imports keep resolving to the previous target directory, so the edges still exist and still
look plausible.

The existing suite could not catch it. tests/test_jsconfig_baseurl.py covers alias
precedence well, but every test calls extract() once under its own tmp_path, so each gets
a distinct config path and therefore a fresh cache key. The bug only appears when one process
extracts twice against the same config path.

The change

Two .clear() calls, beside the three already there. Clearing per run rather than per
file keeps the within-run caching these exist for entirely intact — identical in shape and
cost to how _WORKSPACE_PACKAGE_CACHE is already handled.

Tests

Two regression tests in tests/test_jsconfig_baseurl.py, both running extract() twice
against one config path:

  • test_edited_paths_alias_is_observed_by_a_later_extract — retarget @app/* from src/*
    to lib/* between runs; the second run must resolve to lib/.
  • test_edited_baseurl_is_observed_by_a_later_extract — same for baseUrl.

Both fail on v8 before the change and pass after:

# without the two .clear() calls
FAILED tests/test_jsconfig_baseurl.py::test_edited_paths_alias_is_observed_by_a_later_extract
FAILED tests/test_jsconfig_baseurl.py::test_edited_baseurl_is_observed_by_a_later_extract
2 failed, 12 passed

# with
14 passed in 1.58s

I also drafted a third test for "a tsconfig.json added after the first run", expecting the
miss to have been cached as {} — it passed unchanged, because _find_js_config returning
None short-circuits before the cache is touched. That case was already safe, so the test was
dropped rather than shipped as a fix it does not test.

Verification

Full suite, Windows 11 / Python 3.12.10, v8 @ b2cd362:

failures passed
origin/v8 50 4511
this branch 50 4513

The two failure sets are identical (set-diff of the FAILED ids is empty in both
directions); the delta is the two new tests. The 50 pre-existing failures are
optional-dependency and platform ones — terraform needs tree-sitter-hcl, skillgen needs
full git history, and several are POSIX-only fixtures — none related to this change.

Fixes #2917.

…ify-Labs#2917)

extract() already resets the process-global caches whose backing files can
change between runs, and _MD_LINK_INDEX_CACHE states the contract: cleared at
the start of each run 'so a serial rerun in one process sees files created
since the last run'.

_TSCONFIG_ALIAS_CACHE and _TSCONFIG_BASEURL_CACHE are the same kind of state -
keyed on the config path string, no mtime component, no invalidation anywhere -
but were not in that block. A tsconfig.json is at least as mutable as the
workspace manifests the neighbouring comment was written for, so an edit to
compilerOptions.paths or baseUrl was never observed again for the life of the
process. graphify watch, the MCP server, and any library caller looping over
extract() kept resolving imports to the previous target directory, silently:
the edges still exist and still look plausible.

Clearing per run rather than per file leaves the within-run caching these exist
for fully intact, so the cost is the same as _WORKSPACE_PACKAGE_CACHE's.

The existing suite could not see this - every test in test_jsconfig_baseurl.py
calls extract() once under its own tmp_path, so each gets a distinct config
path and a fresh cache key. The two new tests run extract() twice against one
config path, and both fail on v8 without this change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. No changes could be formally verified in this run.


Graphify review — findings

Clears _TSCONFIG_ALIAS_CACHE and _TSCONFIG_BASEURL_CACHE at the top of extract() alongside the existing per-run cache resets, so tsconfig/jsconfig paths and baseUrl edits are picked up on subsequent extract() calls in a long-lived process (graphify watch, MCP server). Adds two regression tests running extract() twice against the same config path to cover the edited-alias and edited-baseURL cases.

No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1554 functions depend on the 234 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_xaml() — 19 callers, 17 callees
  • new: extract_js() — 80 callers, 3 callees
  • new: dispatch_command() — 2 callers, 119 callees
  • new: _get_extractor() — 26 callers, 6 callees
  • new: run_pipeline() — 8 callers, 13 callees
  • new: collect_files() — 17 callers, 6 callees
  • …and 23 more — each is listed as a finding

Verification — 1554 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: 1406 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

· 31 more finding(s) on lines outside this diff (see the check run).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JS/TS: tsconfig alias and baseUrl caches are never invalidated, so an edit to compilerOptions is invisible for the life of the process (watch / MCP)

1 participant