Skip to content

Commit 244e8fb

Browse files
committed
fix(hooks): update pair-sync ref after auto-sync to avoid false conflicts
1 parent 18198d9 commit 244e8fb

2 files changed

Lines changed: 185 additions & 13 deletions

File tree

jupyter_jcli/commands/hooks_cmd.py

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -343,29 +343,46 @@ def _apply_merge_and_decide(
343343
logger=None,
344344
) -> None:
345345
"""Write merged content and emit allow/deny based on which file changed."""
346+
from jupyter_jcli.canonicalize import canonicalize_py_text
347+
from jupyter_jcli import pair_baseline
346348
from jupyter_jcli.pair_io import emit_py_percent, update_ipynb_sources
347-
from jupyter_jcli.parser import parse_py_percent
349+
from jupyter_jcli.parser import ParsedFile, parse_py_percent
348350

349351
try:
350352
target_before = target.read_bytes()
351353
except OSError:
352354
return
353355

354356
wrote_target = False
357+
synced = False
358+
merged_py_text: str | None = None
359+
canonical_merged_py: str | None = None
355360

356-
if result.py_needs_update:
361+
def _prepare_merged_py() -> tuple[str | None, str | None]:
357362
try:
358363
py_parsed = parse_py_percent(str(py_path))
359-
from jupyter_jcli.parser import ParsedFile
360364
merged_parsed = ParsedFile(
361365
kernel_name=py_parsed.kernel_name,
362366
cells=result.merged_cells,
363367
source_path=py_parsed.source_path,
364368
front_matter_raw=py_parsed.front_matter_raw,
365369
)
366-
new_text = emit_py_percent(merged_parsed)
370+
merged_text = emit_py_percent(merged_parsed)
371+
return merged_text, canonicalize_py_text(merged_text)
372+
except Exception as exc: # noqa: BLE001
373+
if logger is not None:
374+
logger.record_exception(exc)
375+
return None, None
376+
377+
if result.py_needs_update:
378+
try:
379+
if merged_py_text is None:
380+
merged_py_text, canonical_merged_py = _prepare_merged_py()
381+
if merged_py_text is None:
382+
raise RuntimeError("could not prepare merged py text")
367383
if py_path.read_bytes() == target_before or py_path != target:
368-
py_path.write_text(new_text, encoding="utf-8")
384+
py_path.write_text(merged_py_text, encoding="utf-8")
385+
synced = True
369386
if py_path == target:
370387
wrote_target = True
371388
else:
@@ -382,6 +399,7 @@ def _apply_merge_and_decide(
382399
try:
383400
if target.read_bytes() == target_before or ipynb_path != target:
384401
update_ipynb_sources(ipynb_path, result.merged_cells)
402+
synced = True
385403
if ipynb_path == target:
386404
wrote_target = True
387405
else:
@@ -394,6 +412,12 @@ def _apply_merge_and_decide(
394412
logger.record_exception(exc)
395413
print(f"pair-drift-guard-pre: could not write {ipynb_path.name}: {exc}", file=sys.stderr)
396414

415+
if synced:
416+
if canonical_merged_py is None:
417+
_, canonical_merged_py = _prepare_merged_py()
418+
if synced and canonical_merged_py is not None:
419+
pair_baseline.write_baseline(py_path, canonical_merged_py)
420+
397421
if wrote_target:
398422
other = ipynb_path if target == py_path else py_path
399423
_emit_decision(
@@ -607,10 +631,30 @@ def _sync_pair_after_edit(
607631
logger=None,
608632
) -> None:
609633
"""Write the merge result to the OTHER side (not the one the agent just edited)."""
634+
from jupyter_jcli.canonicalize import canonicalize_py_text
635+
from jupyter_jcli import pair_baseline
610636
from jupyter_jcli.pair_io import emit_py_percent, update_ipynb_sources
611637
from jupyter_jcli.parser import ParsedFile, parse_py_percent
612638

613639
synced = False
640+
merged_py_text: str | None = None
641+
canonical_merged_py: str | None = None
642+
643+
def _prepare_merged_py() -> tuple[str | None, str | None]:
644+
try:
645+
py_parsed = parse_py_percent(str(py_path))
646+
merged_parsed = ParsedFile(
647+
kernel_name=py_parsed.kernel_name,
648+
cells=result.merged_cells,
649+
source_path=py_parsed.source_path,
650+
front_matter_raw=py_parsed.front_matter_raw,
651+
)
652+
merged_text = emit_py_percent(merged_parsed)
653+
return merged_text, canonicalize_py_text(merged_text)
654+
except Exception as exc: # noqa: BLE001
655+
if logger is not None:
656+
logger.record_exception(exc)
657+
return None, None
614658

615659
if result.ipynb_needs_update and ipynb_path != edited:
616660
try:
@@ -626,14 +670,11 @@ def _sync_pair_after_edit(
626670

627671
if result.py_needs_update and py_path != edited:
628672
try:
629-
py_parsed = parse_py_percent(str(py_path))
630-
merged_parsed = ParsedFile(
631-
kernel_name=py_parsed.kernel_name,
632-
cells=result.merged_cells,
633-
source_path=py_parsed.source_path,
634-
front_matter_raw=py_parsed.front_matter_raw,
635-
)
636-
py_path.write_text(emit_py_percent(merged_parsed), encoding="utf-8")
673+
if merged_py_text is None:
674+
merged_py_text, canonical_merged_py = _prepare_merged_py()
675+
if merged_py_text is None:
676+
raise RuntimeError("could not prepare merged py text")
677+
py_path.write_text(merged_py_text, encoding="utf-8")
637678
synced = True
638679
except Exception as exc: # noqa: BLE001
639680
if logger is not None:
@@ -643,6 +684,11 @@ def _sync_pair_after_edit(
643684
file=sys.stderr,
644685
)
645686

687+
if synced:
688+
if canonical_merged_py is None:
689+
_, canonical_merged_py = _prepare_merged_py()
690+
if synced and canonical_merged_py is not None:
691+
pair_baseline.write_baseline(py_path, canonical_merged_py)
646692
if synced:
647693
other = ipynb_path if edited == py_path else py_path
648694
_emit_decision(

tests/test_hooks_pair_drift.py

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,16 @@
33
from __future__ import annotations
44

55
import json
6+
import os
7+
import subprocess
68
from pathlib import Path
79
from unittest.mock import patch
810

911
import nbformat
1012
import pytest
1113
from click.testing import CliRunner
1214

15+
from jupyter_jcli import pair_baseline
1316
from jupyter_jcli.cli import main
1417
from jupyter_jcli.drift import DriftResult
1518

@@ -77,6 +80,43 @@ def _make_pair(tmp_path: Path, py_src: list[str], ipynb_src: list[str]) -> tuple
7780
return py, ipynb
7881

7982

83+
def _git(repo: Path, *args: str, env: dict[str, str] | None = None, check: bool = True) -> subprocess.CompletedProcess[str]:
84+
return subprocess.run(
85+
["git", *args],
86+
cwd=str(repo),
87+
check=check,
88+
capture_output=True,
89+
text=True,
90+
env=env,
91+
)
92+
93+
94+
def _git_env(ts: int) -> dict[str, str]:
95+
env = os.environ.copy()
96+
stamp = f"@{ts} +0000"
97+
env["GIT_AUTHOR_DATE"] = stamp
98+
env["GIT_COMMITTER_DATE"] = stamp
99+
return env
100+
101+
102+
@pytest.fixture
103+
def git_repo(tmp_path: Path) -> Path:
104+
subprocess.run(["git", "init"], cwd=str(tmp_path), check=True, capture_output=True)
105+
subprocess.run(
106+
["git", "config", "user.email", "test@test.com"],
107+
cwd=str(tmp_path),
108+
check=True,
109+
capture_output=True,
110+
)
111+
subprocess.run(
112+
["git", "config", "user.name", "Test User"],
113+
cwd=str(tmp_path),
114+
check=True,
115+
capture_output=True,
116+
)
117+
return tmp_path
118+
119+
80120
# ---------------------------------------------------------------------------
81121
# pair-drift-guard-pre no longer handles NotebookEdit (moved to notebook-edit-guard)
82122
# ---------------------------------------------------------------------------
@@ -552,6 +592,92 @@ def test_ipynb_edit_in_post_is_silent(self, tmp_path):
552592
assert _decision(out) is None # no output — Pre was the line of defense
553593

554594

595+
# ---------------------------------------------------------------------------
596+
# pair baseline integration — real git repo, no drift mock
597+
# ---------------------------------------------------------------------------
598+
599+
class TestConsecutiveEdits:
600+
def test_post_uses_sticky_ref_to_avoid_false_conflict(self, git_repo: Path, monkeypatch: pytest.MonkeyPatch):
601+
py, ipynb = _make_pair(git_repo, ["x = 1"], ["x = 1"])
602+
_git(git_repo, "add", "nb.py")
603+
_git(git_repo, "commit", "-m", "init", env=_git_env(100))
604+
605+
py.write_text(py.read_text(encoding="utf-8").replace("x = 1", "x = 10"), encoding="utf-8")
606+
monkeypatch.setenv("GIT_AUTHOR_DATE", "@150 +0000")
607+
monkeypatch.setenv("GIT_COMMITTER_DATE", "@150 +0000")
608+
code1, out1 = _invoke_post({"tool_name": "Edit", "tool_input": {"file_path": str(py)}})
609+
610+
assert code1 == 0
611+
assert "Auto-synced" in _additional_context(out1)
612+
first_ref = _git(
613+
git_repo,
614+
"log",
615+
"-1",
616+
"--format=%ct",
617+
pair_baseline._ref_name("nb.py"),
618+
).stdout.strip()
619+
nb1 = nbformat.read(str(ipynb), as_version=4)
620+
assert [cell.source for cell in nb1.cells if cell.source.strip()] == ["x = 10"]
621+
622+
py.write_text(py.read_text(encoding="utf-8").replace("x = 10", "x = 20"), encoding="utf-8")
623+
monkeypatch.setenv("GIT_AUTHOR_DATE", "@160 +0000")
624+
monkeypatch.setenv("GIT_COMMITTER_DATE", "@160 +0000")
625+
code2, out2 = _invoke_post({"tool_name": "Edit", "tool_input": {"file_path": str(py)}})
626+
627+
assert code2 == 0
628+
assert "Auto-synced" in _additional_context(out2)
629+
second_ref = _git(
630+
git_repo,
631+
"log",
632+
"-1",
633+
"--format=%ct",
634+
pair_baseline._ref_name("nb.py"),
635+
).stdout.strip()
636+
nb2 = nbformat.read(str(ipynb), as_version=4)
637+
assert [cell.source for cell in nb2.cells if cell.source.strip()] == ["x = 20"]
638+
assert int(second_ref) >= int(first_ref)
639+
640+
641+
class TestPreToPostChain:
642+
def test_pre_merge_updates_ref_so_following_post_does_not_conflict(
643+
self,
644+
git_repo: Path,
645+
monkeypatch: pytest.MonkeyPatch,
646+
):
647+
py, ipynb = _make_pair(git_repo, ["x = 1"], ["x = 1"])
648+
_git(git_repo, "add", "nb.py")
649+
_git(git_repo, "commit", "-m", "init", env=_git_env(100))
650+
651+
py.write_text(py.read_text(encoding="utf-8").replace("x = 1", "x = 10"), encoding="utf-8")
652+
monkeypatch.setenv("GIT_AUTHOR_DATE", "@150 +0000")
653+
monkeypatch.setenv("GIT_COMMITTER_DATE", "@150 +0000")
654+
code0, out0 = _invoke_post({"tool_name": "Edit", "tool_input": {"file_path": str(py)}})
655+
assert code0 == 0
656+
assert "Auto-synced" in _additional_context(out0)
657+
658+
nb = nbformat.read(str(ipynb), as_version=4)
659+
nb.cells[0].source = "x = 30"
660+
nbformat.write(nb, str(ipynb))
661+
662+
monkeypatch.setenv("GIT_AUTHOR_DATE", "@170 +0000")
663+
monkeypatch.setenv("GIT_COMMITTER_DATE", "@170 +0000")
664+
pre_code, pre_out = _invoke({"tool_name": "Edit", "tool_input": {"file_path": str(py)}})
665+
assert pre_code == 0
666+
assert _decision(pre_out) == "deny"
667+
assert "Re-read" in _reason(pre_out) or "Someone else edited" in _reason(pre_out)
668+
assert "x = 30" in py.read_text(encoding="utf-8")
669+
670+
py.write_text(py.read_text(encoding="utf-8").replace("x = 30", "x = 40"), encoding="utf-8")
671+
monkeypatch.setenv("GIT_AUTHOR_DATE", "@180 +0000")
672+
monkeypatch.setenv("GIT_COMMITTER_DATE", "@180 +0000")
673+
post_code, post_out = _invoke_post({"tool_name": "Edit", "tool_input": {"file_path": str(py)}})
674+
675+
assert post_code == 0
676+
assert "Auto-synced" in _additional_context(post_out)
677+
nb_after = nbformat.read(str(ipynb), as_version=4)
678+
assert [cell.source for cell in nb_after.cells if cell.source.strip()] == ["x = 40"]
679+
680+
555681
# ---------------------------------------------------------------------------
556682
# --debug smoke tests for pair-drift-guard-pre and pair-drift-guard-post
557683
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)