Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 39 additions & 7 deletions marimo/_session/file_change_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,13 +213,35 @@ def _handle_file_change_locked(
error=f"Session path mismatch: {session.app_file_manager.path} != {file_path}",
)

# Check if the file content matches the last save
# to avoid reloading our own writes
if session.app_file_manager.file_content_matches_last_save():
LOGGER.debug(
f"File {file_path} content matches last save, skipping reload"
)
return FileChangeResult(handled=False)
# Read the file once and run all pre-reload skip checks against
# the same snapshot. If the read itself fails, fall through to
# ``reload()`` which has its own error handling below.
Comment thread
akshayka marked this conversation as resolved.
Outdated
try:
current_content = session.app_file_manager.read_file()
except Exception as e:
LOGGER.debug(f"Error reading {file_path}: {e}")
current_content = None

if current_content is not None:
# Skip our own writes.
if session.app_file_manager.content_matches_last_save(
current_content
):
LOGGER.debug(
f"File {file_path} content matches last save, "
"skipping reload"
)
return FileChangeResult(handled=False)

# Skip when the file is mid-merge so the notebook isn't replaced
# with unparsable cells while the user resolves conflicts (e.g.
# via git-mediate). See issue #9613.
if _has_conflict_markers(current_content):
LOGGER.warning(
f"File {file_path} contains git conflict markers, "
"skipping reload until conflicts are resolved"
)
return FileChangeResult(handled=False)

# Reload the file manager to get the latest code. ``reload``
# mutates the existing document in place via ``apply()`` and
Expand All @@ -242,6 +264,16 @@ def _handle_file_change_locked(
)


def _has_conflict_markers(content: str) -> bool:
"""Return True if ``content`` contains a git conflict start marker.
Comment thread
akshayka marked this conversation as resolved.
Outdated

Git writes ``<<<<<<<`` at the start of a line to mark the beginning of
Comment thread
akshayka marked this conversation as resolved.
Outdated
a conflict hunk; that's a strong signal the file is mid-merge and
shouldn't be reparsed as Python.
"""
return any(line.startswith("<<<<<<<") for line in content.splitlines())


def create_reload_strategy(
mode: SessionMode, config_manager: MarimoConfigManager
) -> ReloadStrategy:
Expand Down
20 changes: 14 additions & 6 deletions marimo/_session/notebook/file_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -616,20 +616,28 @@ def read_file(self) -> str:
)
return self.storage.read(self._filename)

def content_matches_last_save(self, content: str) -> bool:
"""Check if the given content matches the last save.

Used to avoid reloading the file when we detect our own writes.
Prefer this over :meth:`file_content_matches_last_save` when the
caller already has the file contents in hand.
"""
if self._last_saved_content is None:
return False
return content.strip() == self._last_saved_content

def file_content_matches_last_save(self) -> bool:
"""Check if current file content matches the last save.

Used to avoid reloading the file when we detect our own writes.

Returns:
True if content matches last save, False otherwise
"""
if self._filename is None or self._last_saved_content is None:
return False

try:
current_content = self.storage.read(self._filename)
return current_content.strip() == self._last_saved_content
return self.content_matches_last_save(
self.storage.read(self._filename)
)
except Exception as e:
LOGGER.debug(
f"Error reading file to check if content matches: {e}"
Expand Down
46 changes: 46 additions & 0 deletions tests/_session/test_file_change_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import pytest

from marimo import _loggers
from marimo._ast.cell import CellConfig
from marimo._ast.cell_manager import CellManager
from marimo._config.manager import get_default_config_manager
Expand Down Expand Up @@ -744,6 +745,51 @@ def broken(
assert result.error is None


async def test_file_change_coordinator_skips_conflict_markers(
tmp_path: Path,
mock_session: MagicMock,
caplog: pytest.LogCaptureFixture,
) -> None:
"""When git writes conflict markers into the notebook, the watcher
must not reload — otherwise cells become unparsable and conflict
resolution tools like git-mediate break. See issue #9613."""
test_file = tmp_path / "test.py"
test_file.write_text(SINGLE_CELL_NOTEBOOK)
mock_session.app_file_manager = AppFileManager(filename=str(test_file))

strategy = MagicMock()
coordinator = FileChangeCoordinator(strategy)

test_file.write_text(
dedent(
"""\
import marimo
app = marimo.App()

@app.cell
def cell1():
<<<<<<< HEAD
x = 1
=======
x = 2
>>>>>>> other-branch
return x
"""
)
)

with caplog.at_level("WARNING"):
_loggers.marimo_logger().propagate = True
result = await coordinator.handle_change(test_file, mock_session)
Comment thread
mscolnick marked this conversation as resolved.
Outdated

assert not result.handled
assert result.error is None
strategy.handle_reload.assert_not_called()
assert any(
"conflict markers" in record.message for record in caplog.records
)


async def test_file_change_coordinator_path_mismatch(
tmp_path: Path, mock_session: MagicMock
) -> None:
Expand Down
Loading