electron-rare/Aperant
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(merge): include files with content changes even when semantic analysis is empty (#986)

Browse Source 
* fix(merge): include files with content changes even when semantic analysis is empty

The merge system was discarding files that had real code changes but no
detected semantic changes. This happened because:

1. The semantic analyzer only detects imports and function additions/removals
2. Files with only function body modifications returned semantic_changes=[]
3. The filter used Python truthiness (empty list = False), excluding these files
4. This caused merges to fail with "0 files to merge" despite real changes

The fix uses content hash comparison as a fallback check. If the file content
actually changed (hash_before != hash_after), include it for merge regardless
of whether the semantic analyzer could parse the specific change types.

This fixes merging for:
- Files with function body modifications (most common case)
- Unsupported file types (Rust, Go, etc.) where semantic analysis returns empty
- Any file where the analyzer fails to detect the specific change pattern

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* refactor(merge): add TaskSnapshot.has_modifications property and handle DIRECT_COPY

Address PR review feedback:

1. DRY improvement: Add `has_modifications` property to TaskSnapshot
   - Centralizes the modification detection logic
   - Checks semantic_changes first, falls back to content hash comparison
   - Handles both complete tasks and in-progress tasks safely

2. Fix for files with empty semantic_changes (Cursor issue #2):
   - Add DIRECT_COPY MergeDecision for files that were modified but
     couldn't be semantically analyzed (body changes, unsupported languages)
   - MergePipeline returns DIRECT_COPY when has_modifications=True but
     semantic_changes=[] (single task case)
   - Orchestrator handles DIRECT_COPY by reading file directly from worktree
   - This prevents silent data loss where apply_single_task_changes would
     return baseline content unchanged

3. Update _update_stats to count DIRECT_COPY as auto-merged

The combination ensures:
- Files ARE detected for merge (has_modifications check)
- Files ARE properly merged (DIRECT_COPY reads from worktree)
- No silent data loss (worktree content used instead of baseline)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(merge): handle DIRECT_COPY in merge_tasks() and log missing files

- Add DIRECT_COPY handling to merge_tasks() for multi-task merges
  (was only handled in merge_task() for single-task merges)
- Add warning logging when worktree file doesn't exist during DIRECT_COPY
  in both merge_task() and merge_tasks()

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(merge): remove unnecessary f-string prefixes

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* fix(merge): properly fail DIRECT_COPY when worktree file missing

- Extract _read_worktree_file_for_direct_copy() helper to DRY up logic
- Set decision to FAILED when worktree file not found (was silent success)
- Add warning when worktree_path is None in merge_tasks
- Use `is not None` check for merged_content to allow empty files
- Fix has_modifications for new files with empty hash_before
- Add debug_error() to merge_tasks exception handling for consistency

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* style(merge): fix ruff formatting for long line

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Andy
2026-01-13 10:12:59 +01:00
committed by AndyMik90
parent 68fe0860b2
commit 4736b6b61a
4 changed files with 145 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/merge/file_evolution/evolution_queries.py
+2 -2
View File
@@ -102,7 +102,7 @@ class EvolutionQueries:
modifications = []
for file_path, evolution in evolutions.items():
snapshot = evolution.get_task_snapshot(task_id)
if snapshot and snapshot.semantic_changes:
if snapshot and snapshot.has_modifications:
modifications.append((file_path, snapshot))
return modifications
@@ -125,7 +125,7 @@ class EvolutionQueries:
for file_path, evolution in evolutions.items():
for snapshot in evolution.task_snapshots:
if snapshot.task_id in task_ids and snapshot.semantic_changes:
if snapshot.task_id in task_ids and snapshot.has_modifications:
if file_path not in file_tasks:
file_tasks[file_path] = []
file_tasks[file_path].append(snapshot.task_id)
apps/backend/merge/merge_pipeline.py
+12
View File
@@ -75,6 +75,18 @@ class MergePipeline:
# If only one task modified the file, no conflict possible
if len(task_snapshots) == 1:
snapshot = task_snapshots[0]
# Check if file has modifications but semantic analysis returned empty
# This happens for: function body changes, unsupported file types (Rust, Go, etc.)
# In this case, signal that the caller should use the worktree version directly
if snapshot.has_modifications and not snapshot.semantic_changes:
return MergeResult(
decision=MergeDecision.DIRECT_COPY,
file_path=file_path,
merged_content=None, # Caller must read from worktree
explanation=f"File modified by {snapshot.task_id} but no semantic changes detected - use worktree version",
)
merged = apply_single_task_changes(baseline_content, snapshot, file_path)
return MergeResult(
decision=MergeDecision.AUTO_MERGED,
apps/backend/merge/orchestrator.py
+97 -2
View File
@@ -203,6 +203,58 @@ class MergeOrchestrator:
)
return self._merge_pipeline
def _read_worktree_file_for_direct_copy(
self,
file_path: str,
worktree_path: Path | None,
) -> tuple[str | None, bool]:
"""
Read file content from worktree for DIRECT_COPY merge.
Args:
file_path: Relative path to the file
worktree_path: Path to the worktree directory
Returns:
Tuple of (content, success). If success is False, content is None
and the caller should mark the merge as FAILED.
"""
if not worktree_path:
logger.warning(
f"DIRECT_COPY: No worktree path provided for file: {file_path}"
)
debug_warning(
MODULE,
"DIRECT_COPY: No worktree path provided",
file=file_path,
)
return None, False
worktree_file = worktree_path / file_path
if not worktree_file.exists():
logger.warning(f"DIRECT_COPY: Worktree file not found: {worktree_file}")
debug_warning(
MODULE,
"DIRECT_COPY: Worktree file not found",
file=str(worktree_file),
)
return None, False
try:
content = worktree_file.read_text(encoding="utf-8")
debug_detailed(
MODULE,
f"Read file from worktree for direct copy: {file_path}",
)
return content, True
except UnicodeDecodeError:
content = worktree_file.read_text(encoding="utf-8", errors="replace")
debug_detailed(
MODULE,
f"Read file from worktree with encoding fallback: {file_path}",
)
return content, True
def merge_task(
self,
task_id: str,
@@ -275,6 +327,20 @@ class MergeOrchestrator:
task_snapshots=[snapshot],
target_branch=target_branch,
)
# Handle DIRECT_COPY: read file directly from worktree
# This happens when file has modifications but semantic analysis
# couldn't parse the changes (body modifications, unsupported languages)
if result.decision == MergeDecision.DIRECT_COPY:
content, success = self._read_worktree_file_for_direct_copy(
file_path, worktree_path
)
if success:
result.merged_content = content
else:
result.decision = MergeDecision.FAILED
result.error = "Worktree file not found for DIRECT_COPY"
report.file_results[file_path] = result
self._update_stats(report.stats, result)
debug_verbose(
@@ -374,12 +440,41 @@ class MergeOrchestrator:
task_snapshots=snapshots,
target_branch=target_branch,
)
# Handle DIRECT_COPY: read file directly from worktree
# For multi-task merges, use the first task's worktree that modified this file
if result.decision == MergeDecision.DIRECT_COPY:
# Find the worktree path from the first task that modified this file
worktree_path = None
for tid in modifying_tasks:
for req in requests:
if req.task_id == tid and req.worktree_path:
worktree_path = req.worktree_path
break
if worktree_path:
break
content, success = self._read_worktree_file_for_direct_copy(
file_path, worktree_path
)
if success:
result.merged_content = content
else:
result.decision = MergeDecision.FAILED
result.error = "Worktree file not found for DIRECT_COPY"
report.file_results[file_path] = result
self._update_stats(report.stats, result)
report.success = report.stats.files_failed == 0
except Exception as e:
debug_error(
MODULE,
"Merge failed for tasks",
task_ids=[r.task_id for r in requests],
error=str(e),
)
logger.exception("Merge failed")
report.success = False
report.error = str(e)
@@ -589,7 +684,7 @@ class MergeOrchestrator:
written = []
for file_path, result in report.file_results.items():
if result.merged_content:
if result.merged_content is not None:
out_path = output_dir / file_path
out_path.parent.mkdir(parents=True, exist_ok=True)
out_path.write_text(result.merged_content, encoding="utf-8")
@@ -640,7 +735,7 @@ class MergeOrchestrator:
)
stats.conflicts_auto_resolved += len(result.conflicts_resolved)
if result.decision == MergeDecision.AUTO_MERGED:
if result.decision in (MergeDecision.AUTO_MERGED, MergeDecision.DIRECT_COPY):
stats.files_auto_merged += 1
elif result.decision == MergeDecision.AI_MERGED:
stats.files_ai_merged += 1
apps/backend/merge/types.py
+34 -1
View File
@@ -133,6 +133,7 @@ class MergeDecision(Enum):
AI_MERGED = "ai_merged" # AI resolved the conflict
NEEDS_HUMAN_REVIEW = "needs_human_review" # Flagged for human
FAILED = "failed" # Could not merge
DIRECT_COPY = "direct_copy" # Use worktree version directly (no semantic merge)
@dataclass
@@ -414,6 +415,34 @@ class TaskSnapshot:
raw_diff=data.get("raw_diff"),
)
@property
def has_modifications(self) -> bool:
"""
Check if this snapshot represents actual file modifications.
Returns True if the file was modified, using content hash comparison
as the source of truth. This handles cases where the semantic analyzer
couldn't detect changes (e.g., function body modifications, unsupported
file types like Rust) but the file was actually changed.
Also returns True for newly created files (where content_hash_before
is empty but content_hash_after is set).
"""
# If we have semantic changes, the file was definitely modified
if self.semantic_changes:
return True
# Handle new files: if before is empty but after has content, it's a new file
if not self.content_hash_before and self.content_hash_after:
return True
# Fall back to content hash comparison for files where semantic
# analysis returned empty (body modifications, unsupported languages)
if self.content_hash_before and self.content_hash_after:
return self.content_hash_before != self.content_hash_after
return False
@dataclass
class FileEvolution:
@@ -534,7 +563,11 @@ class MergeResult:
@property
def success(self) -> bool:
"""Check if merge was successful."""
return self.decision in {MergeDecision.AUTO_MERGED, MergeDecision.AI_MERGED}
return self.decision in {
MergeDecision.AUTO_MERGED,
MergeDecision.AI_MERGED,
MergeDecision.DIRECT_COPY,
}
@property
def needs_human_review(self) -> bool: