fix(test-linker): fingerprint-keyed test-edge cache + staleness + reroute (TAP-4080)#194
Conversation
…oute (TAP-4080) The reduced-scope #193 shipped an UNCONDITIONAL fixed-path cache that was never fingerprint-keyed, never checked staleness (serving edges from an old call graph as a hit — a correctness bug), and was dead code: the three diff_impact call sites still called build_test_edges directly, and the "test_edges" stats provider only registered on a lazy import inside load_or_build. This finishes the spec on the ADR-0029 substrate. - Cache at .tapps-mcp/test-edges-index.json now embeds the call-graph fingerprint (index.fingerprint) and validates it with FingerprintStaleness on load: same fingerprint => hit, changed => miss + rebuild + rewrite. No stale edges are ever served. - New load_or_build_test_edges_for_index(project_root, index) is the fast path for callers already holding the index (no redundant graph rebuild). - All three diff_impact call sites (analyze_diff_impact, per-symbol lookup, export_test_map) route through the cached path; force_rebuild threaded through export_test_map. - register_cache_stats("test_edges", ...) fires eagerly via a side-effect import at the end of test_linker.py, which is imported on the diff_impact / call-graph paths a normal tapps_stats run exercises — so "test_edges" now appears in tapps_stats.caches without a prior load_or_build call. - Added _reset_test_edges_stats() to conftest cache reset. - Tests: cache save/load and load_or_build updated to the new path/signature; added test_cache_invalidates_on_fingerprint_change (old fp => hit, changed fp => miss + fresh rebuild) and test_stats_registered_in_collect_cache_stats. Reuses AtomicJsonCache + FingerprintStaleness (ADR-0029); no parallel atomic-write or staleness code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1faef9f3ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cached = load_test_edges_cache(project_root, fingerprint) | ||
| if cached is not None: | ||
| return cached |
There was a problem hiding this comment.
When tapps_diff_impact(force_rebuild=True) rebuilds the call graph, analyze_diff_impact() still reaches this hit path with force_rebuild=False. In git checkouts the call-graph fingerprint is based on HEAD plus the set of dirty file names, so editing the contents of an already-dirty test file can produce a fresh index with the same index.fingerprint as the old TESTS-edge cache; this returns stale edges from the previous test body even though the caller explicitly rebuilt the graph. Please either propagate the force flag through analyze_diff_impact or validate the cached edges against the supplied index instead of returning on fingerprint alone.
Useful? React with 👍 / 👎.
1 participant
What
Finishes TAP-4080 correctly. PR #193 shipped a reduced-scope version that an independent verifier refuted: it wrote an unconditional fixed-path cache (
.tapps-mcp/test-edges.json) that was never fingerprint-keyed, never checked staleness (a stale cache from an old call graph was served as a hit — a correctness bug), and was dead code — the threediff_impactcall sites still calledbuild_test_edgesdirectly, so the cache was never exercised by the path it was meant to speed up. This PR meets every acceptance criterion on the ADR-0029 substrate.Changes
.tapps-mcp/test-edges-index.jsonand the payload embeds the call-graph fingerprint (index.fingerprint), the same fingerprint the call-graph cache uses.load_test_edges_cachevalidates the cached fingerprint withFingerprintStaleness— same fingerprint => hit; mismatch => miss + rebuild + rewrite. A stale cache is never served.diff_impact.pysites (analyze_diff_impact, per-symbol lookup,export_test_map) route through the newload_or_build_test_edges_for_index(project_root, index)fast path — no redundant graph rebuild;force_rebuildthreaded throughexport_test_map.register_cache_stats("test_edges", ...)now fires via a side-effect import at the end oftest_linker.py, which is imported on the diff_impact / call-graph paths a normaltapps_statsrun exercises. "test_edges" now appears intapps_stats.cacheswithout a priorload_or_buildcall. Added_reset_test_edges_stats()to conftest cache reset.test_cache_invalidates_on_fingerprint_change(old fp => hit, changed fp => miss + fresh rebuild) andtest_stats_registered_in_collect_cache_stats.Reuses
AtomicJsonCache+FingerprintStaleness(ADR-0029) — no parallel atomic-write or staleness code.Tests
uv run pytest packages/tapps-mcp/tests/unit/test_test_linker.py -o addopts="" -v— 7 passed (incl. the new invalidation + stats tests).uv run pytest packages/tapps-mcp/tests -k 'diff_impact' -m 'not slow' -o addopts=""— 15 passed.export_test_map/ call-graph tests — 3 passed.🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com