From d024eec10503789ab2a30200e49a222e085078b3 Mon Sep 17 00:00:00 2001 From: StillKnotKnown <192589389+StillKnotKnown@users.noreply.github.com> Date: Sun, 11 Jan 2026 08:23:33 +0200 Subject: [PATCH] fix(merge): resolve multiple merge-related issues (ACS-194, ACS-179, ACS-174, ACS-163) (#885) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(merge): resolve multiple merge-related issues (ACS-194, ACS-179, ACS-174, ACS-163) - ACS-179: Fix TypeError in print_conflict_info - handle both string and dict conflict formats - ACS-174: Handle git hook failures during merge - skip checkout if already on target branch - ACS-163: Fix merge failure fallthrough - return False immediately when merge fails - ACS-194: Improve AI merge success rate - add Haiku→Sonnet fallback with enhanced prompts All fixes include regression tests. * fix: improve merge robustness and fix test issues This commit addresses multiple issues related to merge functionality and test quality: 1. workspace.py: - Fix case-insensitive natural language pattern matching to detect AI explanations returned instead of code 2. worktree.py: - Guard against None stderr when handling git hook failures - Improve error messages for merge hook handling 3. workspace/display.py: - Improve print_conflict_info() to properly categorize conflicts (marker vs AI merge failures) - Add severity indicators (🔴 for high, 🟡 for medium) - Use shlex.quote() for proper git add command quoting - Provide clearer guidance for different conflict types 4. tests/test_merge_ai_resolver.py: - Fix test assertions to match actual AI_MERGE_SYSTEM_PROMPT content (check for "intelligently" and "task's intent" instead of non-existent "semantic understanding") 5. tests/test_workspace.py: - Add side-effect verification for merge failure tests - Remove unused fixtures - Add assertions for severity emoji indicators 6. tests/test_worktree.py: - Add subprocess return code checks for better test reliability Related: ACS-194, ACS-179, ACS-174, ACS-163 * fix: improve display formatting and use proper imports 1. display.py: - Fix trailing space when severity_icon is empty (low/unknown severity) - Only add leading space to icon when icon is non-empty 2. workspace/__init__.py: - Export AI_MERGE_SYSTEM_PROMPT and _build_merge_prompt - Add to __all__ for public API access 3. test_merge_ai_resolver.py: - Use standard imports from core.workspace package - Remove fragile importlib.util dynamic loading * fix: address all 6 review findings 1. workspace.py: - Fix parameter naming: max_thinking -> max_thinking_tokens (matches codebase convention used in 25+ locations) - Extract hardcoded model constants: MERGE_FAST_MODEL, MERGE_CAPABLE_MODEL, MERGE_FAST_THINKING, MERGE_COMPLEX_THINKING - Improve natural language detection: check patterns at START of line and require absence of code patterns to reduce false positives 2. display.py: - Add critical severity icon handling (⛔ for critical severity) 3. worktree.py: - Fix inconsistent stderr handling: use truthiness check for both branches (empty strings show '') 4. test_workspace.py: - Remove unused imports: patch, MagicMock from unittest.mock Related: ACS-194 --------- Co-authored-by: StillKnotKnown Co-authored-by: Andy <119136210+AndyMik90@users.noreply.github.com> --- apps/backend/core/workspace.py | 282 ++++++++++++++++-------- apps/backend/core/workspace/__init__.py | 4 + apps/backend/core/workspace/display.py | 64 +++++- apps/backend/core/worktree.py | 26 ++- tests/test_merge_ai_resolver.py | 40 ++++ tests/test_workspace.py | 137 +++++++++++- tests/test_worktree.py | 28 +++ 7 files changed, 479 insertions(+), 102 deletions(-) diff --git a/apps/backend/core/workspace.py b/apps/backend/core/workspace.py index 2190d55a..cd1c8ad5 100644 --- a/apps/backend/core/workspace.py +++ b/apps/backend/core/workspace.py @@ -277,6 +277,13 @@ def merge_existing_build( no_commit, stats, spec_name=spec_name, keep_worktree=True ) return True + else: + # Standard git merge failed - report error and don't continue + print() + print_status( + "Merge failed. Please check the errors above.", "error" + ) + return False elif smart_result.get("git_conflicts"): # Had git conflicts that AI couldn't fully resolve resolved = smart_result.get("resolved", []) @@ -1425,18 +1432,41 @@ import os _merge_logger = logging.getLogger(__name__) # System prompt for AI file merging -AI_MERGE_SYSTEM_PROMPT = """You are an expert code merge assistant. Your task is to perform a 3-way merge of code files. +AI_MERGE_SYSTEM_PROMPT = """You are an expert code merge assistant specializing in intelligent 3-way merges. Your task is to merge code changes from two branches while preserving all meaningful changes. -RULES: -1. Preserve all functional changes from both versions (ours and theirs) -2. Maintain code style consistency -3. Resolve conflicts by understanding the semantic purpose of each change -4. When changes are independent (different functions/sections), include both -5. When changes overlap, combine them logically or prefer the more complete version -6. Preserve all imports from both versions -7. Output ONLY the merged code - no explanations, no markdown, no code fences +CONTEXT: +- "OURS" = current main branch (target for merge) +- "THEIRS" = task worktree branch (changes being merged in) +- "BASE" = common ancestor before changes -IMPORTANT: Output the raw merged file content only. Do not wrap in code blocks.""" +MERGE STRATEGY: +1. **Preserve all functional changes** - Include all features, bug fixes, and improvements from both versions +2. **Combine independent changes** - If changes are in different functions/sections, include both +3. **Resolve overlapping changes intelligently**: + - Prefer the more complete/updated implementation + - Combine logic if both versions add value + - When in doubt, favor the version that better addresses the task's intent +4. **Maintain syntactic correctness** - Ensure the merged code is valid and compiles/runs +5. **Preserve imports and dependencies** from both versions + +HANDLING COMMON PATTERNS: +- New functions/classes: Include all from both versions +- Modified functions: Merge changes logically, prefer more complete version +- Imports: Union of all imports from both versions +- Comments/Documentation: Include relevant documentation from both +- Configuration: Merge settings, with conflict resolution favoring task-specific values + +CRITICAL RULES: +- Output ONLY the merged code - no explanations, no prose, no markdown fences +- If you cannot determine the correct merge, make a reasonable decision based on best practices +- Never output error messages like "I need more context" - always provide a best-effort merge +- Ensure the output is complete and syntactically valid code""" + +# Model constants for AI merge two-tier strategy (ACS-194) +MERGE_FAST_MODEL = "claude-haiku-4-5-20251001" # Fast model for simple merges +MERGE_CAPABLE_MODEL = "claude-sonnet-4-5-20250929" # Capable model for complex merges +MERGE_FAST_THINKING = 1024 # Lower thinking for fast/simple merges +MERGE_COMPLEX_THINKING = 16000 # Higher thinking for complex merges def _infer_language_from_path(file_path: str) -> str: @@ -1525,7 +1555,7 @@ def _build_merge_prompt( if len(base_content) > 10000: base_content = base_content[:10000] + "\n... (truncated)" base_section = f""" -BASE (common ancestor): +BASE (common ancestor before changes): ```{language} {base_content} ``` @@ -1537,20 +1567,22 @@ BASE (common ancestor): if len(worktree_content) > 15000: worktree_content = worktree_content[:15000] + "\n... (truncated)" - prompt = f"""Perform a 3-way merge for file: {file_path} -Task being merged: {spec_name} + prompt = f"""FILE: {file_path} +TASK: {spec_name} + +This is a 3-way code merge. You must combine changes from both versions. {base_section} -OURS (current main branch): +OURS (current main branch - target for merge): ```{language} {main_content} ``` -THEIRS (changes from task worktree): +THEIRS (task worktree branch - changes being merged): ```{language} {worktree_content} ``` -Merge these versions, preserving all meaningful changes from both. Output only the merged file content, no explanations.""" +OUTPUT THE MERGED CODE ONLY. No explanations, no markdown fences.""" return prompt @@ -1568,6 +1600,112 @@ def _strip_code_fences(content: str) -> str: return content +async def _attempt_ai_merge( + task: "ParallelMergeTask", + prompt: str, + model: str = MERGE_FAST_MODEL, + max_thinking_tokens: int = MERGE_FAST_THINKING, +) -> tuple[bool, str | None, str]: + """ + Attempt an AI merge with a specific model. + + Args: + task: The merge task with file contents + prompt: The merge prompt + model: Model to use for merge + max_thinking_tokens: Max thinking tokens for the model + + Returns: + Tuple of (success, merged_content, error_message) + """ + try: + from core.simple_client import create_simple_client + except ImportError: + return False, None, "core.simple_client not available" + + client = create_simple_client( + agent_type="merge_resolver", + model=model, + system_prompt=AI_MERGE_SYSTEM_PROMPT, + max_thinking_tokens=max_thinking_tokens, + ) + + response_text = "" + async with client: + await client.query(prompt) + + async for msg in client.receive_response(): + msg_type = type(msg).__name__ + if msg_type == "AssistantMessage" and hasattr(msg, "content"): + for block in msg.content: + block_type = type(block).__name__ + if block_type == "TextBlock" and hasattr(block, "text"): + response_text += block.text + + if response_text: + merged_content = _strip_code_fences(response_text.strip()) + + # Check if AI returned natural language instead of code (case-insensitive) + # More robust detection: (1) Check if patterns are at START of line, (2) Check for + # absence of code patterns like imports, function definitions, braces, etc. + natural_language_patterns = [ + "i need to", + "let me", + "i cannot", + "i'm unable", + "the file appears", + "i don't have", + "unfortunately", + "i apologize", + ] + + first_line = merged_content.split("\n")[0] if merged_content else "" + first_line_stripped = first_line.lstrip() + first_line_lower = first_line_stripped.lower() + + # Check if first line STARTS with natural language pattern (not just contains it) + starts_with_prose = any( + first_line_lower.startswith(pattern) + for pattern in natural_language_patterns + ) + + # Also check for absence of common code patterns to reduce false positives + has_code_patterns = any( + pattern in merged_content[:500] # Check first 500 chars for code patterns + for pattern in [ + "import ", # Python/JS/TypeScript imports + "from ", # Python imports + "def ", # Python functions + "function ", # JavaScript functions + "const ", # JavaScript/TypeScript const + "class ", # Class definitions + "{", # Braces indicate code + "}", # Braces indicate code + "#!", # Shebang + "