-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: sanitize changes diagram input #2212
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
9bc50a7
feat: sanitize changes diagram input and add unit tests
nagai9999 51966fb
fix: add input validation to sanitize_diagram function
nagai9999 be9fe13
fix: update sanitize_diagram to require mermaid code fence
nagai9999 9251f62
fix: refactor sanitize_diagram to improve readability and maintainabi…
nagai9999 8733f30
fix: reorder imports to follow isort convention in test_pr_description
nagai9999 61faae5
fix: clean up sanitize_diagram function and remove unnecessary import…
nagai9999 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| import pytest | ||
| import yaml | ||
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| from unittest.mock import MagicMock, patch | ||
| from pr_agent.tools.pr_description import PRDescription, sanitize_diagram | ||
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| KEYS_FIX = ["filename:", "language:", "changes_summary:", "changes_title:", "description:", "title:"] | ||
|
|
||
| def _make_instance(prediction_yaml: str): | ||
| """Create a PRDescription instance, bypassing __init__.""" | ||
| with patch.object(PRDescription, '__init__', lambda self, *a, **kw: None): | ||
| obj = PRDescription.__new__(PRDescription) | ||
| obj.prediction = prediction_yaml | ||
| obj.keys_fix = KEYS_FIX | ||
| obj.user_description = "" | ||
| return obj | ||
|
|
||
|
|
||
| def _mock_settings(): | ||
| """Mock get_settings used by _prepare_data.""" | ||
| settings = MagicMock() | ||
| settings.pr_description.add_original_user_description = False | ||
| return settings | ||
|
|
||
|
|
||
| def _prediction_with_diagram(diagram_value: str) -> str: | ||
| """Build a minimal YAML prediction string that includes changes_diagram.""" | ||
| return yaml.dump({ | ||
| 'title': 'test', | ||
| 'description': 'test', | ||
| 'changes_diagram': diagram_value, | ||
| }) | ||
|
|
||
|
|
||
| class TestPRDescriptionDiagram: | ||
|
|
||
| @patch('pr_agent.tools.pr_description.get_settings') | ||
| def test_diagram_not_starting_with_fence_is_removed(self, mock_get_settings): | ||
| mock_get_settings.return_value = _mock_settings() | ||
| obj = _make_instance(_prediction_with_diagram('graph LR\nA --> B')) | ||
| obj._prepare_data() | ||
| assert 'changes_diagram' not in obj.data | ||
|
|
||
| @patch('pr_agent.tools.pr_description.get_settings') | ||
| def test_diagram_missing_closing_fence_is_appended(self, mock_get_settings): | ||
| mock_get_settings.return_value = _mock_settings() | ||
| obj = _make_instance(_prediction_with_diagram('```mermaid\ngraph LR\nA --> B')) | ||
| obj._prepare_data() | ||
| assert obj.data['changes_diagram'] == '\n```mermaid\ngraph LR\nA --> B\n```' | ||
|
|
||
| @patch('pr_agent.tools.pr_description.get_settings') | ||
| def test_backticks_inside_label_are_removed(self, mock_get_settings): | ||
| mock_get_settings.return_value = _mock_settings() | ||
| obj = _make_instance(_prediction_with_diagram('```mermaid\ngraph LR\nA["`file`"] --> B\n```')) | ||
| obj._prepare_data() | ||
| assert obj.data['changes_diagram'] == '\n```mermaid\ngraph LR\nA["file"] --> B\n```' | ||
|
|
||
| @patch('pr_agent.tools.pr_description.get_settings') | ||
| def test_backticks_outside_label_are_kept(self, mock_get_settings): | ||
| mock_get_settings.return_value = _mock_settings() | ||
| obj = _make_instance(_prediction_with_diagram('```mermaid\ngraph LR\nA["`file`"] -->|`edge`| B\n```')) | ||
| obj._prepare_data() | ||
| assert obj.data['changes_diagram'] == '\n```mermaid\ngraph LR\nA["file"] -->|`edge`| B\n```' | ||
|
|
||
| @patch('pr_agent.tools.pr_description.get_settings') | ||
| def test_normal_diagram_only_adds_newline(self, mock_get_settings): | ||
| mock_get_settings.return_value = _mock_settings() | ||
| obj = _make_instance(_prediction_with_diagram('```mermaid\ngraph LR\nA["file.py"] --> B["output"]\n```')) | ||
| obj._prepare_data() | ||
| assert obj.data['changes_diagram'] == '\n```mermaid\ngraph LR\nA["file.py"] --> B["output"]\n```' | ||
|
|
||
| def test_none_input_returns_empty(self): | ||
| assert sanitize_diagram(None) == '' | ||
|
|
||
| def test_non_string_input_returns_empty(self): | ||
| assert sanitize_diagram(123) == '' | ||
|
|
||
| def test_non_mermaid_fence_returns_empty(self): | ||
| assert sanitize_diagram('```python\nprint("hello")\n```') == '' | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.