fix(versioning): 修复首改文件 diff 基准丢失导致全文件计新增
根因:track_edit 在编辑前备份的 v1(修改前内容)只存在于该消息的
中间行,make_snapshot 用最终行(v2 修改后内容)整体替换该行后,
v1 不再被任何快照引用。对会话中首次被跟踪的文件,diff 配对找不到
上一条消息里的基准,按 added 处理并把全文件计为新增(+479 -0)。
26f31a09 修的是重复行配对,未覆盖这一数据丢失路径,故跨平台复现。
修复(不改数据模型,纯读取侧恢复):
- 新增 _resolve_diff_base_backup:优先用上一条消息快照里的备份;
缺失时利用备份名确定性(sha256(绝对路径)@vN)与版本逐文件连续的
不变量,回退到 v(N-1) 作为消息前状态(新建文件 v1 不落盘,自然
仍判 added);首次出现即 v1 说明消息未改动该文件,与自身比较得 0。
- get_diff_stats / get_file_diff_stats / get_file_patch_lines 三处
统一走该解析;旧 state 数据(v1 备份文件仍在)透明愈合,亦兼容
MAX_SNAPSHOTS 逐出旧行的场景。
- 新增 5 个回归测试(生产顺序:track_edit 先于写入);修正 2 个旧
测试的模拟顺序使其与生产一致(原顺序先写后 track,把新建文件
误模拟成 v1 首现)。
验证:11/11 通过;旧代码跑新测试 4/5 失败(复现);用事故对话
conv_20260731_114736_581 真实数据只读复算,#1 由 +479/-0 added
修正为 +4/-1 modified(与实际编辑一致),#2 保持 +0/-0;全量测试
失败/错误数与 HEAD 基线完全相同(2+10,均为预存在/环境问题)。
This commit is contained in:
parent
1760ca66f3
commit
77be044c44
@ -25,6 +25,29 @@ Concurrency: mutations (track_edit / make_snapshot) run inside a per-state-file
|
|||||||
process lock as reload -> mutate -> save critical sections, so short-lived
|
process lock as reload -> mutate -> save critical sections, so short-lived
|
||||||
manager instances (one per edit / per task end) can never lose each other's
|
manager instances (one per edit / per task end) can never lose each other's
|
||||||
rows. Readers only see whole files because writes go through os.replace.
|
rows. Readers only see whole files because writes go through os.replace.
|
||||||
|
|
||||||
|
Diff bases (version fallback)
|
||||||
|
-----------------------------
|
||||||
|
The diff shown for a message compares its final snapshot against the previous
|
||||||
|
message's snapshot. A file first tracked in the target message is absent from
|
||||||
|
every earlier snapshot: make_snapshot replaces the message's intermediate
|
||||||
|
track_edit row (which held the pre-edit backup) with the final row, so the
|
||||||
|
pre-edit state can no longer be found via snapshot pairing.
|
||||||
|
|
||||||
|
It is recovered through two invariants of the backup store:
|
||||||
|
|
||||||
|
- backup file names are deterministic: sha256 of the resolved absolute path,
|
||||||
|
suffixed with @v<version>;
|
||||||
|
- backup versions are sequential per file: track_edit creates v1 (pre-edit),
|
||||||
|
each make_snapshot bumps at most one version, and backup files are never
|
||||||
|
garbage-collected.
|
||||||
|
|
||||||
|
Hence for a first-appearance backup at version N >= 2, version N-1 is exactly
|
||||||
|
the pre-message state (for a file created by the message no v1 exists on disk,
|
||||||
|
so the base stays "missing" and the file correctly shows as added). A
|
||||||
|
first-appearance version-1 backup means the message left the file unchanged
|
||||||
|
since track_edit. This also heals state files written before the fix and
|
||||||
|
survives MAX_SNAPSHOTS eviction of older rows.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
@ -474,6 +497,40 @@ class ShallowVersioningManager:
|
|||||||
break
|
break
|
||||||
return target_snapshot, previous_snapshot
|
return target_snapshot, previous_snapshot
|
||||||
|
|
||||||
|
def _resolve_diff_base_backup(
|
||||||
|
self,
|
||||||
|
tracking_path: str,
|
||||||
|
target_backup: FileBackup,
|
||||||
|
previous_snapshot: Optional[ShallowSnapshot],
|
||||||
|
) -> Optional[FileBackup]:
|
||||||
|
"""Resolve the backup representing the file's state BEFORE the target message.
|
||||||
|
|
||||||
|
Normal case: the previous message snapshot references the file, so its
|
||||||
|
backup is the diff base. A file first tracked in the target message is
|
||||||
|
absent from every earlier snapshot (its intermediate track_edit row was
|
||||||
|
replaced by make_snapshot); the pre-message state is then recovered via
|
||||||
|
the version fallback documented in the module docstring: version N-1 of
|
||||||
|
the deterministic backup name, used only when that backup file still
|
||||||
|
exists on disk. A first-appearance version-1 backup means the file was
|
||||||
|
left unchanged since track_edit, so it is diffed against itself.
|
||||||
|
"""
|
||||||
|
if previous_snapshot is not None:
|
||||||
|
previous = previous_snapshot.tracked_file_backups.get(tracking_path)
|
||||||
|
if previous is not None:
|
||||||
|
return previous
|
||||||
|
if target_backup.version >= 2:
|
||||||
|
resolved = self._resolve_file_path(tracking_path)
|
||||||
|
candidate_name = self._backup_file_name(str(resolved), target_backup.version - 1)
|
||||||
|
if self._backup_path(candidate_name).exists():
|
||||||
|
return FileBackup(
|
||||||
|
backup_file_name=candidate_name,
|
||||||
|
version=target_backup.version - 1,
|
||||||
|
backup_time=target_backup.backup_time,
|
||||||
|
)
|
||||||
|
elif target_backup.version == 1 and target_backup.backup_file_name is not None:
|
||||||
|
return target_backup
|
||||||
|
return None
|
||||||
|
|
||||||
def get_diff_stats(self, message_id: str) -> Optional[Dict[str, Any]]:
|
def get_diff_stats(self, message_id: str) -> Optional[Dict[str, Any]]:
|
||||||
"""Compute insertions/deletions/files_changed between the target snapshot and the previous snapshot."""
|
"""Compute insertions/deletions/files_changed between the target snapshot and the previous snapshot."""
|
||||||
target_snapshot, previous_snapshot = self._find_snapshot_pair(message_id)
|
target_snapshot, previous_snapshot = self._find_snapshot_pair(message_id)
|
||||||
@ -489,7 +546,7 @@ class ShallowVersioningManager:
|
|||||||
if not target_backup:
|
if not target_backup:
|
||||||
continue
|
continue
|
||||||
|
|
||||||
previous_backup = previous_snapshot.tracked_file_backups.get(tracking_path) if previous_snapshot else None
|
previous_backup = self._resolve_diff_base_backup(tracking_path, target_backup, previous_snapshot)
|
||||||
previous_lines = self._read_backup_lines(previous_backup)
|
previous_lines = self._read_backup_lines(previous_backup)
|
||||||
target_lines = self._read_backup_lines(target_backup)
|
target_lines = self._read_backup_lines(target_backup)
|
||||||
|
|
||||||
@ -525,7 +582,7 @@ class ShallowVersioningManager:
|
|||||||
if not target_backup:
|
if not target_backup:
|
||||||
continue
|
continue
|
||||||
|
|
||||||
previous_backup = previous_snapshot.tracked_file_backups.get(tracking_path) if previous_snapshot else None
|
previous_backup = self._resolve_diff_base_backup(tracking_path, target_backup, previous_snapshot)
|
||||||
previous_lines = self._read_backup_lines(previous_backup)
|
previous_lines = self._read_backup_lines(previous_backup)
|
||||||
target_lines = self._read_backup_lines(target_backup)
|
target_lines = self._read_backup_lines(target_backup)
|
||||||
|
|
||||||
@ -558,7 +615,7 @@ class ShallowVersioningManager:
|
|||||||
if not target_backup:
|
if not target_backup:
|
||||||
return {"lines": [], "truncated": False}
|
return {"lines": [], "truncated": False}
|
||||||
|
|
||||||
previous_backup = previous_snapshot.tracked_file_backups.get(tracking_path) if previous_snapshot else None
|
previous_backup = self._resolve_diff_base_backup(tracking_path, target_backup, previous_snapshot)
|
||||||
previous_lines = self._read_backup_lines(previous_backup)
|
previous_lines = self._read_backup_lines(previous_backup)
|
||||||
target_lines = self._read_backup_lines(target_backup)
|
target_lines = self._read_backup_lines(target_backup)
|
||||||
|
|
||||||
|
|||||||
@ -49,18 +49,21 @@ class ShallowVersioningPairingTest(unittest.TestCase):
|
|||||||
# realistic multi-instance flow (the original bug scenario)
|
# realistic multi-instance flow (the original bug scenario)
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
def test_diff_pairs_with_previous_message_across_instances(self) -> None:
|
def test_diff_pairs_with_previous_message_across_instances(self) -> None:
|
||||||
|
# Production order: track_edit runs BEFORE the tool applies the edit
|
||||||
|
# (tools_execution backs up first), so for newly created files
|
||||||
|
# track_edit sees a missing file (v1=None) and make_snapshot stores v2.
|
||||||
# message 1: AI creates two new files (each track_edit runs in its own instance)
|
# message 1: AI creates two new files (each track_edit runs in its own instance)
|
||||||
self._write("a.txt", "a1\na2\n")
|
|
||||||
_new_manager(self.project, self.data).track_edit("a.txt", "msg_1")
|
_new_manager(self.project, self.data).track_edit("a.txt", "msg_1")
|
||||||
self._write("b.txt", "b1\nb2\n")
|
self._write("a.txt", "a1\na2\n")
|
||||||
_new_manager(self.project, self.data).track_edit("b.txt", "msg_1")
|
_new_manager(self.project, self.data).track_edit("b.txt", "msg_1")
|
||||||
|
self._write("b.txt", "b1\nb2\n")
|
||||||
_new_manager(self.project, self.data).make_snapshot("msg_1")
|
_new_manager(self.project, self.data).make_snapshot("msg_1")
|
||||||
|
|
||||||
# message 2: modify a.txt, create c.txt
|
# message 2: modify a.txt, create c.txt
|
||||||
|
_new_manager(self.project, self.data).track_edit("a.txt", "msg_2") # early-return: already tracked
|
||||||
self._write("a.txt", "a1\na2 changed\na3\n")
|
self._write("a.txt", "a1\na2 changed\na3\n")
|
||||||
_new_manager(self.project, self.data).track_edit("a.txt", "msg_2")
|
|
||||||
self._write("c.txt", "c1\n")
|
|
||||||
_new_manager(self.project, self.data).track_edit("c.txt", "msg_2")
|
_new_manager(self.project, self.data).track_edit("c.txt", "msg_2")
|
||||||
|
self._write("c.txt", "c1\n")
|
||||||
_new_manager(self.project, self.data).make_snapshot("msg_2")
|
_new_manager(self.project, self.data).make_snapshot("msg_2")
|
||||||
|
|
||||||
# state file must hold exactly one row per message
|
# state file must hold exactly one row per message
|
||||||
@ -130,15 +133,16 @@ class ShallowVersioningPairingTest(unittest.TestCase):
|
|||||||
# accidentally correct pre-fix; lazy view was not)
|
# accidentally correct pre-fix; lazy view was not)
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
def test_single_new_file_message(self) -> None:
|
def test_single_new_file_message(self) -> None:
|
||||||
self._write("a.txt", "a1\n")
|
# Production order: track_edit before the edit is applied.
|
||||||
_new_manager(self.project, self.data).track_edit("a.txt", "msg_1")
|
_new_manager(self.project, self.data).track_edit("a.txt", "msg_1")
|
||||||
|
self._write("a.txt", "a1\n")
|
||||||
_new_manager(self.project, self.data).make_snapshot("msg_1")
|
_new_manager(self.project, self.data).make_snapshot("msg_1")
|
||||||
|
|
||||||
# msg_2 edits a.txt and tracks one new file b.txt
|
# msg_2 edits a.txt and tracks one new file b.txt
|
||||||
|
_new_manager(self.project, self.data).track_edit("a.txt", "msg_2") # early-return: already tracked
|
||||||
self._write("a.txt", "a1\na2\n")
|
self._write("a.txt", "a1\na2\n")
|
||||||
_new_manager(self.project, self.data).track_edit("a.txt", "msg_2")
|
|
||||||
self._write("b.txt", "b1\nb2\nb3\n")
|
|
||||||
_new_manager(self.project, self.data).track_edit("b.txt", "msg_2")
|
_new_manager(self.project, self.data).track_edit("b.txt", "msg_2")
|
||||||
|
self._write("b.txt", "b1\nb2\nb3\n")
|
||||||
_new_manager(self.project, self.data).make_snapshot("msg_2")
|
_new_manager(self.project, self.data).make_snapshot("msg_2")
|
||||||
|
|
||||||
reader = _new_manager(self.project, self.data)
|
reader = _new_manager(self.project, self.data)
|
||||||
@ -201,5 +205,128 @@ class ShallowVersioningPairingTest(unittest.TestCase):
|
|||||||
self.assertIsNone(reader.get_snapshot_by_seq(4))
|
self.assertIsNone(reader.get_snapshot_by_seq(4))
|
||||||
|
|
||||||
|
|
||||||
|
class ShallowVersioningFirstEditDiffBaseTest(unittest.TestCase):
|
||||||
|
"""Regression tests for the diff base of files first tracked in a message.
|
||||||
|
|
||||||
|
Production order is: file exists on disk -> track_edit backs up the
|
||||||
|
pre-edit content (v1) -> the edit is applied -> make_snapshot stores the
|
||||||
|
post-edit backup (v2), replacing the intermediate track_edit row. The
|
||||||
|
pre-edit backup then survives only on disk; diffs must recover it via the
|
||||||
|
version fallback instead of showing the whole file as added (+all -0).
|
||||||
|
"""
|
||||||
|
|
||||||
|
def setUp(self) -> None:
|
||||||
|
self.tmp = Path(tempfile.mkdtemp(prefix="shallow_ver_base_test_"))
|
||||||
|
self.project = self.tmp / "project"
|
||||||
|
self.data = self.tmp / "data"
|
||||||
|
self.project.mkdir(parents=True)
|
||||||
|
self.data.mkdir(parents=True)
|
||||||
|
|
||||||
|
def tearDown(self) -> None:
|
||||||
|
shutil.rmtree(self.tmp, ignore_errors=True)
|
||||||
|
|
||||||
|
def _write(self, rel: str, content: str) -> None:
|
||||||
|
path = self.project / rel
|
||||||
|
path.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
path.write_text(content, encoding="utf-8")
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# the reported bug: first message edits a pre-existing file
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
def test_first_message_edit_of_preexisting_file_shows_real_diff(self) -> None:
|
||||||
|
self._write("main.py", "line1\nline2\nline3\n")
|
||||||
|
_new_manager(self.project, self.data).track_edit("main.py", "msg_1")
|
||||||
|
self._write("main.py", "line1\nline2 changed\nline3\nline4\n")
|
||||||
|
_new_manager(self.project, self.data).make_snapshot("msg_1")
|
||||||
|
|
||||||
|
# the intermediate track_edit row was replaced: only the final row
|
||||||
|
# remains, referencing v2; the pre-edit v1 survives only on disk.
|
||||||
|
rows = _read_state_rows(self.data)
|
||||||
|
self.assertEqual(len(rows), 1)
|
||||||
|
self.assertEqual(rows[0]["tracked_file_backups"]["main.py"]["version"], 2)
|
||||||
|
|
||||||
|
reader = _new_manager(self.project, self.data)
|
||||||
|
files = {f["path"]: f for f in reader.get_file_diff_stats("msg_1")}
|
||||||
|
self.assertEqual(set(files), {"main.py"})
|
||||||
|
self.assertEqual(files["main.py"]["status"], "modified")
|
||||||
|
self.assertEqual((files["main.py"]["insertions"], files["main.py"]["deletions"]), (2, 1))
|
||||||
|
stats = reader.get_diff_stats("msg_1") or {}
|
||||||
|
self.assertEqual((stats.get("insertions"), stats.get("deletions")), (2, 1))
|
||||||
|
|
||||||
|
patch = reader.get_file_patch_lines("main.py", "msg_1")
|
||||||
|
adds = [l["content"] for l in patch["lines"] if l["type"] == "add"]
|
||||||
|
removes = [l["content"] for l in patch["lines"] if l["type"] == "remove"]
|
||||||
|
self.assertEqual(removes, ["line2"])
|
||||||
|
self.assertEqual(adds, ["line2 changed", "line4"])
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# a file created by the first message must still show as added
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
def test_first_message_new_file_still_added(self) -> None:
|
||||||
|
_new_manager(self.project, self.data).track_edit("new.txt", "msg_1") # absent at track time
|
||||||
|
self._write("new.txt", "n1\nn2\n")
|
||||||
|
_new_manager(self.project, self.data).make_snapshot("msg_1")
|
||||||
|
|
||||||
|
reader = _new_manager(self.project, self.data)
|
||||||
|
files = {f["path"]: f for f in reader.get_file_diff_stats("msg_1")}
|
||||||
|
self.assertEqual(set(files), {"new.txt"})
|
||||||
|
self.assertEqual(files["new.txt"]["status"], "added")
|
||||||
|
self.assertEqual((files["new.txt"]["insertions"], files["new.txt"]["deletions"]), (2, 0))
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# an edit that leaves content identical yields no changes (v1 kept)
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
def test_first_message_unchanged_edit_shows_no_changes(self) -> None:
|
||||||
|
self._write("same.txt", "s1\ns2\n")
|
||||||
|
_new_manager(self.project, self.data).track_edit("same.txt", "msg_1")
|
||||||
|
_new_manager(self.project, self.data).make_snapshot("msg_1")
|
||||||
|
|
||||||
|
rows = _read_state_rows(self.data)
|
||||||
|
self.assertEqual(rows[0]["tracked_file_backups"]["same.txt"]["version"], 1)
|
||||||
|
reader = _new_manager(self.project, self.data)
|
||||||
|
self.assertEqual(reader.get_file_diff_stats("msg_1"), [])
|
||||||
|
self.assertFalse(reader.has_any_changes("msg_1"))
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# first message deletes a pre-existing file -> full deletions
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
def test_first_message_delete_shows_deletions(self) -> None:
|
||||||
|
self._write("gone.txt", "g1\ng2\ng3\n")
|
||||||
|
_new_manager(self.project, self.data).track_edit("gone.txt", "msg_1")
|
||||||
|
(self.project / "gone.txt").unlink()
|
||||||
|
_new_manager(self.project, self.data).make_snapshot("msg_1")
|
||||||
|
|
||||||
|
reader = _new_manager(self.project, self.data)
|
||||||
|
files = {f["path"]: f for f in reader.get_file_diff_stats("msg_1")}
|
||||||
|
self.assertEqual(set(files), {"gone.txt"})
|
||||||
|
self.assertEqual(files["gone.txt"]["status"], "deleted")
|
||||||
|
self.assertEqual((files["gone.txt"]["insertions"], files["gone.txt"]["deletions"]), (0, 3))
|
||||||
|
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
# the normal previous-snapshot path keeps winning for later messages
|
||||||
|
# ------------------------------------------------------------------
|
||||||
|
def test_second_message_diff_uses_previous_snapshot_row(self) -> None:
|
||||||
|
self._write("a.txt", "a1\n")
|
||||||
|
_new_manager(self.project, self.data).track_edit("a.txt", "msg_1")
|
||||||
|
self._write("a.txt", "a1\na2\n")
|
||||||
|
_new_manager(self.project, self.data).make_snapshot("msg_1")
|
||||||
|
|
||||||
|
# already tracked: track_edit early-returns in production, so a plain
|
||||||
|
# content change followed by make_snapshot is the realistic flow.
|
||||||
|
self._write("a.txt", "a1\na2\na3\n")
|
||||||
|
_new_manager(self.project, self.data).make_snapshot("msg_2")
|
||||||
|
|
||||||
|
reader = _new_manager(self.project, self.data)
|
||||||
|
files = {f["path"]: f for f in reader.get_file_diff_stats("msg_2")}
|
||||||
|
self.assertEqual(set(files), {"a.txt"})
|
||||||
|
self.assertEqual(files["a.txt"]["status"], "modified")
|
||||||
|
self.assertEqual((files["a.txt"]["insertions"], files["a.txt"]["deletions"]), (1, 0))
|
||||||
|
|
||||||
|
# msg_1 diff still resolves through the fallback (pre-existing file)
|
||||||
|
files1 = {f["path"]: f for f in reader.get_file_diff_stats("msg_1")}
|
||||||
|
self.assertEqual(files1["a.txt"]["status"], "modified")
|
||||||
|
self.assertEqual((files1["a.txt"]["insertions"], files1["a.txt"]["deletions"]), (1, 0))
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user