-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: Implement the documented "add_repo_metadata" functionality #2304
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
1ef62f9
3b25b61
d1b8507
bebb2f3
10994bc
772dbaa
38af09e
7b9f60e
9a85654
5bdc1a9
7454325
87672ea
524ef53
29b6a1d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,6 +174,21 @@ def get_repo_settings(self): | |
| get_logger().error(f"Failed to get repo settings, error: {e}") | ||
| return "" | ||
|
|
||
| def get_repo_file(self, file_path: str) -> str: | ||
| try: | ||
| contents = self.azure_devops_client.get_item_content( | ||
| repository_id=self.repo_slug, | ||
| project=self.workspace_slug, | ||
| download=False, | ||
| include_content_metadata=False, | ||
| include_content=True, | ||
| path=file_path, | ||
| ) | ||
| content = list(contents)[0] | ||
| return content.decode("utf-8") if isinstance(content, bytes) else content | ||
|
Comment on lines
+177
to
+195
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Azure reads merged commit AzureDevOpsProvider.get_repo_file() reads files at self.pr.last_merge_commit (merge result) instead of the PR source/head commit, so metadata may reflect the merged preview rather than the branch under review. This is inconsistent with other providers in this PR that explicitly read from the PR/MR source branch or head SHA. Agent Prompt
|
||
| except Exception: | ||
| return "" | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||
|
|
||
| def get_files(self): | ||
| files = [] | ||
| for i in self.azure_devops_client.get_pull_request_commits( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,6 +89,18 @@ def get_repo_settings(self): | |
| except Exception: | ||
| return "" | ||
|
|
||
| def get_repo_file(self, file_path: str) -> str: | ||
| try: | ||
| # Read from the PR's source branch so metadata files reflect the branch under review | ||
| url = (f"https://api.bitbucket.org/2.0/repositories/{self.workspace_slug}/{self.repo_slug}/src/" | ||
| f"{self.pr.source_branch}/{file_path}") | ||
| response = requests.request("GET", url, headers=self.headers) | ||
| if response.status_code == 404: | ||
| return "" | ||
| return response.text | ||
|
Comment on lines
+92
to
+101
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 4. Bitbucket file fetch can hang BitbucketProvider.get_repo_file performs an HTTP GET without a timeout, so enabling add_repo_metadata can block apply_repo_settings indefinitely on stalled connections. Because apply_repo_settings is called before request handling logic, this can stall the whole PR-agent flow. Agent Prompt
|
||
| except Exception: | ||
| return "" | ||
|
|
||
| def get_git_repo_url(self, pr_url: str=None) -> str: #bitbucket does not support issue url, so ignore param | ||
| try: | ||
| parsed_url = urlparse(self.pr_url) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,6 +85,49 @@ def apply_repo_settings(pr_url): | |
| except Exception as e: | ||
| get_logger().error(f"Failed to remove temporary settings file {repo_settings_file}", e) | ||
|
|
||
| # Repository metadata: fetch well-known instruction files (AGENTS.md, QODO.md, CLAUDE.md, …) | ||
| # from the PR's head branch root and inject their contents into every tool's extra_instructions. | ||
| # See: https://qodo-merge-docs.qodo.ai/usage-guide/additional_configurations/#bringing-additional-repository-metadata-to-pr-agent | ||
| if get_settings().config.get("add_repo_metadata", False): | ||
| try: | ||
| metadata_files = get_settings().config.get("add_repo_metadata_file_list", | ||
| ["AGENTS.md", "QODO.md", "CLAUDE.md"]) | ||
|
|
||
| # Collect contents of all metadata files that exist in the repo | ||
| metadata_content_parts = [] | ||
| for file_name in metadata_files: | ||
|
Comment on lines
+142
to
+179
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. add_repo_metadata_file_list not validated add_repo_metadata_file_list is used directly without validating/normalizing its type/contents, which can lead to incorrect behavior (e.g., iterating over characters if a string is provided) or runtime errors. The checklist requires normalizing and validating user-provided settings before using them in logic. Agent Prompt
|
||
| try: | ||
| content = git_provider.get_repo_file(file_name) | ||
| if content and content.strip(): | ||
| metadata_content_parts.append(content.strip()) | ||
| get_logger().info(f"Loaded repository metadata file: {file_name}") | ||
| except Exception as e: | ||
| get_logger().debug(f"Failed to load metadata file {file_name}: {e}") | ||
|
|
||
| # Append combined metadata to extra_instructions for every tool that supports it. | ||
| if metadata_content_parts: | ||
| combined_metadata = "\n\n".join(metadata_content_parts) | ||
| tool_sections = [ | ||
| "pr_reviewer", | ||
| "pr_description", | ||
| "pr_code_suggestions", | ||
| "pr_add_docs", | ||
| "pr_update_changelog", | ||
| "pr_test", | ||
| "pr_improve_component", | ||
| ] | ||
| for section in tool_sections: | ||
| section_obj = get_settings().get(section, None) | ||
| if section_obj is not None and hasattr(section_obj, 'extra_instructions'): | ||
| existing = section_obj.extra_instructions or "" | ||
| if existing: | ||
| new_value = f"{existing}\n\n{combined_metadata}" | ||
| else: | ||
| new_value = combined_metadata | ||
| get_settings().set(f"{section}.extra_instructions", new_value) | ||
| except Exception as e: | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||
| get_logger().debug(f"Failed to load repository metadata files: {e}") | ||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||
|
|
||
| # enable switching models with a short definition | ||
| if get_settings().config.model.lower() == 'claude-3-5-sonnet': | ||
| set_claude_model() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| """ | ||
| Tests for the add_repo_metadata feature in apply_repo_settings(). | ||
|
|
||
| When config.add_repo_metadata is true, metadata files (AGENTS.md, QODO.md, | ||
| CLAUDE.md by default) are fetched from the PR's head branch and their contents | ||
| are appended to extra_instructions for every tool that supports it. | ||
| """ | ||
|
|
||
| import pytest | ||
|
|
||
| from pr_agent.config_loader import get_settings | ||
| from pr_agent.git_providers.utils import apply_repo_settings | ||
|
|
||
|
|
||
| class FakeGitProvider: | ||
| """Minimal git provider stub for testing repo metadata loading.""" | ||
|
|
||
| def __init__(self, repo_files=None): | ||
| """ | ||
| Args: | ||
| repo_files: dict mapping file names to their content strings. | ||
| Files not in the dict will return "" (not found). | ||
| """ | ||
| self._repo_files = repo_files or {} | ||
|
|
||
| def get_repo_settings(self): | ||
| return "" | ||
|
|
||
| def get_repo_file(self, file_path: str) -> str: | ||
| return self._repo_files.get(file_path, "") | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def _reset_extra_instructions(): | ||
| """Reset extra_instructions for all tool sections before each test.""" | ||
| tool_sections = [ | ||
| "pr_reviewer", "pr_description", "pr_code_suggestions", | ||
| "pr_add_docs", "pr_update_changelog", "pr_test", "pr_improve_component", | ||
| ] | ||
| original_values = {} | ||
| for section in tool_sections: | ||
| section_obj = get_settings().get(section, None) | ||
| if section_obj is not None: | ||
| original_values[section] = getattr(section_obj, 'extra_instructions', "") | ||
|
|
||
| yield | ||
|
|
||
| for section, value in original_values.items(): | ||
| get_settings().set(f"{section}.extra_instructions", value) | ||
|
|
||
|
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
|
||
|
|
||
| class TestRepoMetadata: | ||
| def test_metadata_disabled_by_default(self, monkeypatch): | ||
| """When add_repo_metadata is false, no metadata files are loaded.""" | ||
| provider = FakeGitProvider(repo_files={"AGENTS.md": "should not appear"}) | ||
| monkeypatch.setattr( | ||
| "pr_agent.git_providers.utils.get_git_provider_with_context", | ||
| lambda pr_url: provider, | ||
| ) | ||
| get_settings().set("config.add_repo_metadata", False) | ||
|
|
||
| apply_repo_settings("https://example.com/pr/1") | ||
|
|
||
| assert "should not appear" not in (get_settings().pr_reviewer.extra_instructions or "") | ||
|
|
||
| def test_metadata_appended_to_extra_instructions(self, monkeypatch): | ||
| """When enabled, metadata file contents are appended to extra_instructions.""" | ||
| provider = FakeGitProvider(repo_files={"AGENTS.md": "Review with care"}) | ||
| monkeypatch.setattr( | ||
| "pr_agent.git_providers.utils.get_git_provider_with_context", | ||
| lambda pr_url: provider, | ||
| ) | ||
| get_settings().set("config.add_repo_metadata", True) | ||
| get_settings().set("config.add_repo_metadata_file_list", ["AGENTS.md"]) | ||
|
|
||
| apply_repo_settings("https://example.com/pr/1") | ||
|
|
||
| assert "Review with care" in get_settings().pr_reviewer.extra_instructions | ||
| assert "Review with care" in get_settings().pr_code_suggestions.extra_instructions | ||
|
|
||
| def test_multiple_metadata_files_combined(self, monkeypatch): | ||
| """Contents of multiple metadata files are joined together.""" | ||
| provider = FakeGitProvider(repo_files={ | ||
| "AGENTS.md": "Agent instructions", | ||
| "CLAUDE.md": "Claude instructions", | ||
| }) | ||
| monkeypatch.setattr( | ||
| "pr_agent.git_providers.utils.get_git_provider_with_context", | ||
| lambda pr_url: provider, | ||
| ) | ||
| get_settings().set("config.add_repo_metadata", True) | ||
| get_settings().set("config.add_repo_metadata_file_list", ["AGENTS.md", "CLAUDE.md"]) | ||
|
|
||
| apply_repo_settings("https://example.com/pr/1") | ||
|
|
||
| instructions = get_settings().pr_reviewer.extra_instructions | ||
| assert "Agent instructions" in instructions | ||
| assert "Claude instructions" in instructions | ||
|
|
||
| def test_missing_metadata_files_skipped(self, monkeypatch): | ||
| """Files that don't exist in the repo are silently skipped.""" | ||
| provider = FakeGitProvider(repo_files={"AGENTS.md": "Found this one"}) | ||
| monkeypatch.setattr( | ||
| "pr_agent.git_providers.utils.get_git_provider_with_context", | ||
| lambda pr_url: provider, | ||
| ) | ||
| get_settings().set("config.add_repo_metadata", True) | ||
| get_settings().set("config.add_repo_metadata_file_list", | ||
| ["AGENTS.md", "NONEXISTENT.md"]) | ||
|
|
||
| apply_repo_settings("https://example.com/pr/1") | ||
|
|
||
| instructions = get_settings().pr_reviewer.extra_instructions | ||
| assert "Found this one" in instructions | ||
| assert "NONEXISTENT" not in instructions | ||
|
|
||
| def test_metadata_appended_to_existing_extra_instructions(self, monkeypatch): | ||
| """Metadata is appended to (not replacing) any pre-existing extra_instructions.""" | ||
| provider = FakeGitProvider(repo_files={"AGENTS.md": "From agents file"}) | ||
| monkeypatch.setattr( | ||
| "pr_agent.git_providers.utils.get_git_provider_with_context", | ||
| lambda pr_url: provider, | ||
| ) | ||
| get_settings().set("config.add_repo_metadata", True) | ||
| get_settings().set("config.add_repo_metadata_file_list", ["AGENTS.md"]) | ||
| get_settings().set("pr_reviewer.extra_instructions", "Existing instructions") | ||
|
|
||
| apply_repo_settings("https://example.com/pr/1") | ||
|
|
||
| instructions = get_settings().pr_reviewer.extra_instructions | ||
| assert "Existing instructions" in instructions | ||
| assert "From agents file" in instructions | ||
|
|
||
| def test_custom_file_list(self, monkeypatch): | ||
| """Users can specify a custom list of metadata files to search for.""" | ||
| provider = FakeGitProvider(repo_files={"CUSTOM.md": "Custom content"}) | ||
| monkeypatch.setattr( | ||
| "pr_agent.git_providers.utils.get_git_provider_with_context", | ||
| lambda pr_url: provider, | ||
| ) | ||
| get_settings().set("config.add_repo_metadata", True) | ||
| get_settings().set("config.add_repo_metadata_file_list", ["CUSTOM.md"]) | ||
|
|
||
| apply_repo_settings("https://example.com/pr/1") | ||
|
|
||
| assert "Custom content" in get_settings().pr_reviewer.extra_instructions | ||
Uh oh!
There was an error while loading. Please reload this page.