Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Normalize whitespace when comparing patch context lines #6541

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

neubig
Copy link
Contributor

@neubig neubig commented Jan 30, 2025

This PR fixes an issue where patches fail to apply due to whitespace mismatches in context lines.
Related to #6140

Problem:

  • When applying patches, the resolver strictly compares context lines including whitespace
  • This causes patches to fail when there are differences in indentation or whitespace type (spaces vs tabs)
  • Example: A patch generated from a file with 4-space indentation fails to apply to a file with 2-space indentation

Solution:

  • Added whitespace normalization when comparing context lines
  • The comparison now ignores differences in indentation level and whitespace type
  • Original whitespace is preserved in the output file

Added tests:

  • test_patch_whitespace_mismatch: Verifies that patches apply successfully even with whitespace mismatches
  • test_patch_whitespace_match: Verifies that patches still work with matching whitespace

Example of a patch that now works:

@@ -25,6 +25,10 @@ class PatchPrediction(BaseModel):
       None,
       description="Path to the prediction file that contains this prediction"
   )
+    patch_statistics: list[dict[str, float]] = Field(
+        default_factory=list,
+        description="List of statistics for each patch in the issue's patches list"
+    )
   
   class Config:
       arbitrary_types_allowed = True

This patch previously failed because the indentation used 4 spaces while the source file used 8 spaces. With this fix, the patch now applies successfully.


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:046189c-nikolaik   --name openhands-app-046189c   docker.all-hands.dev/all-hands-ai/openhands:046189c

This change makes the patch application more robust by normalizing whitespace
when comparing context lines. This helps in cases where the patch and the
source file have different indentation levels or different types of whitespace
(e.g., spaces vs tabs).

The change is particularly useful when applying patches that were generated
from files with different editors or different indentation settings.

Added tests to verify the behavior with both matching and mismatching
whitespace.
@malhotra5
Copy link
Contributor

Look like we're failing test cases

original_content = """class Example:
def method(self):
pass

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you mean by

Suggested change

otherwise this test won't pass



def test_patch_whitespace_mismatch():
"""Test that the patch application fails correctly when whitespace doesn't match."""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems to contradict to the behavior?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants