Repository navigation
Fix incremental caller loss after re-exported symbol rename - #1062
11 commits merged into
Conversation
Promote staging -> testing
Promote staging -> testing
Promote staging -> testing
Promote testing -> main
Bound tool calls and optimize change discovery process
Bound tool calls and optimize change discovery process
Improve test gap reporting and caller reachability classification
Improve test gap reporting and caller reachability classification
Release version 2.3.9 with schema updates and documentation fixes
Release v2.3.9 with schema updates and documentation fixes
|
Thank you for this PR. I tested renames through re-exports with update, watch and the MCP tool. The fix works for those cases. Before I can merge it, please fix these:
Please also point the PR at staging, not main. The failing tests are on the branch After you push the fix, I will test it again. |
There was a problem hiding this comment.
🟡 Changes recommended
A transient dependent read or parse failure can leave renamed-symbol caller edges indefinitely stale because subsequent updates fail to retry the unchanged dependent.
3 open issues
What changed in this PR
This PR addresses caller loss during incremental graph updates after a TypeScript export is renamed, bringing unchanged re-export callers in line with full rebuild results.
Changes:
- Reparse discovered dependents when a seed file’s content changes.
- Preserve duplicate-notification shortcuts and reuse cached hashes.
- Add a regression comparing incremental and full-build callers and checking no-op replay.
| File | Description |
|---|---|
| tests/test_incremental_reexport_regression.py | Tests renamed exports, unchanged re-export callers, and duplicate notifications. |
| code_review_graph/incremental.py | Makes dependent invalidation sensitive to seed content changes and bypasses dependent hash shortcuts. |
Review effort: Balanced
| if existing and existing[0].file_hash == current_hash: | ||
| continue |
| existing = store.get_nodes_by_file(full_path) | ||
| try: | ||
| current_hash = current_hashes.get(normalize_file_path(rel_path)) | ||
| if current_hash is None: |
| (tmp_path/name).write_text(files[name].replace('calculateFee', 'computeFee')) | ||
| result = incremental_update(tmp_path, store, changed_files=['fee.ts', 'barrel.ts', 'client.ts']) | ||
| assert not result['errors'] | ||
| target = str(tmp_path/'fee.ts') + '::computeFee' |
tirth8205
closed this pull request by merging all changes
into
tirth8205:staging
in
0160eb0
…ters (review of tirth8205#1062) Re-parsing an importer with unchanged bytes now replaces only its edges, so its flows and communities survive at every post-processing level. Only files with graph rows are hashed. A forced re-parse that fails is retried by the next update. Adds the maintainer's review tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # code_review_graph/incremental.py # code_review_graph/tools/build.py
Ruby methods always carry their full lexical scope (tirth8205#933), so the leaf collision index from tirth8205#1087 now covers Python only, and the docs and tests say so. Generators are not component wrappers' functions (tirth8205#972 with tirth8205#1131). Partial graph coverage counts tracked files only (tirth8205#881 with tirth8205#1094). A pending marker recomputes all derived data (tirth8205#1105), so that test starts from a finished graph, the deleted-seed case now passes (tirth8205#1062 with tirth8205#673), and hooks.json keeps its events under a hooks key (tirth8205#1102 with tirth8205#1126). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An incremental update after renaming an exported TypeScript function can lose callers in unchanged files that import through a re-export. The dependency walk finds those files, but the content-hash shortcut skips reparsing them, leaving edges pointed at the old symbol identity. A full rebuild finds all three callers; the incremental update finds only one.
Invalidate the discovered dependents when the seed file actually changed, while retaining the no-op shortcut for duplicate notifications. Reuse the hash read so parse-stage snapshot coherence is preserved.
Validation: the new synthetic rename/re-export regression fails on v2.3.9 and passes with this patch; all 140 incremental tests pass, including snapshot-race coverage. No application source is included.