Skip to content

fix: share language matcher with plain diff provider - #2895

Merged
IsmaelMartinez merged 1 commit into
The-PR-Agent:mainfrom
apoorvdarshan:fix/plain-diff-language-matcher
Aug 31, 2026
Merged

IsmaelMartinez merged 1 commit into
The-PR-Agent:mainfrom
apoorvdarshan:fix/plain-diff-language-matcher

Conversation

@apoorvdarshan

Copy link
Copy Markdown
Contributor

Summary

  • replace PlainDiffGitProvider's private extension inversion with the shared build_language_file_matcher
  • preserve case-sensitive extensions, full filename rules, and multipart suffix matching in plain-diff mode
  • add regression coverage for .C and Dockerfile, as requested in the issue

Why

The private implementation lowercased extensions and relied on os.path.splitext(). That classified .C as C instead of C++, dropped full filenames such as Dockerfile, and truncated multipart suffixes. Reusing the shared matcher keeps plain-diff behavior aligned with the other local providers.

Fixes #2894

Testing

  • PYTHONPATH=. uv run pytest tests/unittest -q — 2,758 passed, 1 skipped, 1 xfailed
  • uv run pre-commit run --files pr_agent/git_providers/plain_diff_provider.py tests/unittest/test_plain_diff_provider.py — passed
  • uv run ruff check --fix pr_agent/git_providers/plain_diff_provider.py tests/unittest/test_plain_diff_provider.py — passed

@github-actions github-actions Bot added the bug label Aug 30, 2026
@apoorvdarshan

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

qodo-code-review Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

Grey Divider

Sorry, something went wrong

We weren't able to complete the code review on our side. Please try again manually by commenting /agentic_review on this PR.

Grey Divider

Qodo Logo

@IsmaelMartinez IsmaelMartinez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving and merging as-is.

The revert probe confirms the test earns its place: restore main's plain_diff_provider.py and it goes red, returning {'C': 100.0} instead of {'C++': 50.0, 'Dockerfile': 50.0}.

The payoff runs deeper than the token counts. On main a diff of upper.C and Dockerfile reported C at 100 percent and then sorted both files into "Other", so the language it advertised and the buckets it built disagreed. They now come from the same matcher.

Thanks for turning this round within half an hour of the issue going up, and for adding the regression I asked for.

@IsmaelMartinez
IsmaelMartinez merged commit a06dc0c into The-PR-Agent:main Aug 31, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plain_diff_provider.get_languages is the last private copy of the filename matcher, and disagrees on 23 of 871

2 participants