fix(diff): key node diff by identity, not line-bearing id (closes #300) - #302
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
_diff_backend() keyed nodes by n["id"], which embeds a line number, so adding a comment to a file reported every function in it as added AND removed and fired false new_dead / resolved_dead. On the real 4,188-node polyglot workspace a pure line shift with zero real changes produced 1,137 false adds, 1,137 false removes and 260 false dead-code alarms. False new_dead is the dangerous half: an agent reading "this function just became dead" can delete live code. Same class as #293 and #294. Nodes now key on (file, impl_for, fn) via the _node_key/_index_nodes pair factored out of #297's _endpoint_key, so both halves of the diff share one identity scheme. Functions that share (file, impl_for, fn) — closures like `visit` redefined in a scope, already flagged duplicate_define — get an occurrence index in line order, so deleting one of two is still detected while a shift keeps the keys stable. Edge resolution still needs raw ids, so nodes are indexed twice: by id for endpoints, by identity for the diff. Recovers lost signal too: a shifted node was never compared against its old self, so its ref_count/status change vanished. changed_nodes now survives a line shift. Output shape unchanged. Full suite: 19 failures on this branch, 19 on main, identical list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #300. Stacked on #298 — base is
feat/issue-297-edge-diff, reuses the identity helper landed there. Merge #298 first; this retargets tomainautomatically.Masalah
_diff_backend()mengunci node dengann["id"], dan node id memuat nomor baris. Menambahkan satu komentar di atas file — tanpa menyentuh satu fungsi pun — melaporkan setiap fungsi di file itu sebagai ditambah DAN dihapus, plusnew_dead/resolved_deadpalsu.Registry nyata (
smart-tax-assistance/app, 4.188 node), geser baris seluruh codebase, 0 perubahan sungguhan:mainhari ini260 alarm "fungsi ini baru saja mati" yang palsu. Itu paruh yang berbahaya: agent yang percaya
new_deadbisa menghapus kode hidup. Kelas yang sama dengan #293 (Go/Java false-dead) dan #294 (JSX handler false-dead). Pemicunya cuma "ada yang menambah docstring atau merapikan import" — jadi kemungkinan besar ini sudah terjadi diam-diam di banyak PR.Yang berubah
Identitas node =
(file, impl_for, fn)lewat_node_key/_index_nodes, difaktorkan dari_endpoint_keymilik #297 — satu skema identitas untuk kedua paruh diff, bukan dua.Fungsi yang berbagi
(file, impl_for, fn)— closurevisit/_walkyang didefinisikan berulang di satu scope, yang registry sendiri sudah tandaiduplicate_define— dapat occurrence index urut baris. Jadi menghapus satu dari dua tetap terdeteksi, sementara geser baris menjaga kuncinya stabil. (Mengunci tanpa index akan menggabungkan mereka dan menyembunyikan penghapusan nyata.)Edge tetap butuh id mentah, jadi node diindeks dua kali: by id untuk endpoint, by identity untuk diff.
Bonus: sinyal yang selama ini hilang, kembali
Node yang bergeser baris tidak pernah dibandingkan dengan dirinya yang lama — jadi perubahan
ref_count/status-nya lenyap begitu saja. Membuang derau sekaligus memulihkan ini:changed_nodessekarang selamat melintasi geser baris. Ada testnya.Verifikasi (dijalankan)
tests/test_diff_engine_nodes.py): geser baris → nol; add/remove nyata → terdeteksi; newly-dead nyata → terdeteksi; sudah-dead + geser → bukan new_dead; hapus 1 dari 2 duplicate_define → terdeteksi; pindah file → terhitung; bentuk output tetap.main, daftar identik — nol regresi.Bentuk output tidak berubah. Command count tetap 12. Bug fix →
skip-design-doc.🤖 Generated with Claude Code