chore: address agent comments

Author: avidspartan1

pr_agent/algo/repo_context.py

@@ -1,32 +1,99 @@
 from html import escape
 
 from pr_agent.config_loader import get_settings
+from pr_agent.git_providers.git_provider import GitProvider
 from pr_agent.log import get_logger
 
+TRUNCATION_MARKER = "...(truncated)..."
+INSTRUCTION_FILES_INTRO = (
+    "You are being given instruction files. Follow them as project-specific guidance when reviewing code."
+)
+MARKDOWN_FENCE = "`````"
+_unsupported_repo_context_provider_classes = set()
+
 
 def render_instruction_files(files: dict[str, str]) -> str:
     parts = [
-        "You are being given instruction files. Follow them as project-specific guidance when reviewing code.",
+        INSTRUCTION_FILES_INTRO,
         "<instruction_files>",
     ]
 
     for path, content in files.items():
         scope = path.rsplit("/", 1)[0] if "/" in path else "repo-root"
         parts.append(f'<file path="{escape(path, quote=True)}" scope="{escape(scope, quote=True)}">')
-        parts.append("`````markdown")
+        parts.append(f"{MARKDOWN_FENCE}markdown")
         parts.append(content.rstrip())
-        parts.append("`````")
+        parts.append(MARKDOWN_FENCE)
         parts.append("</file>")
         parts.append("")
 
     parts.append("</instruction_files>")
     return "\n".join(parts)
 
 
+def render_instruction_files_with_line_budget(files: dict[str, str], max_lines: int) -> str:
+    parts = [
+        INSTRUCTION_FILES_INTRO,
+        "<instruction_files>",
+    ]
+
+    for path, content in files.items():
+        scope = path.rsplit("/", 1)[0] if "/" in path else "repo-root"
+        file_header = [
+            f'<file path="{escape(path, quote=True)}" scope="{escape(scope, quote=True)}">',
+            f"{MARKDOWN_FENCE}markdown",
+        ]
+        file_footer = [
+            MARKDOWN_FENCE,
+            "</file>",
+            "",
+        ]
+        content_lines = content.rstrip().splitlines()
+        reserved_closing_lines = len(file_header) + len(file_footer) + 1
+        available_content_lines = max_lines - len(parts) - reserved_closing_lines
+
+        parts.extend(file_header)
+        if available_content_lines >= len(content_lines):
+            parts.extend(content_lines)
+        else:
+            if available_content_lines > 1:
+                parts.extend(content_lines[: available_content_lines - 1])
+            parts.append(TRUNCATION_MARKER)
+            parts.extend(file_footer)
+            break
+
+        parts.extend(file_footer)
+
+    parts.append("</instruction_files>")
+    return "\n".join(parts).strip()
+
+
 def build_repo_context(git_provider) -> str:
     context_files = get_settings().config.get("repo_context_files", [])
     if not context_files:
         return ""
+    if isinstance(context_files, str):
+        get_logger().warning(
+            "repo_context_files should be a list of file paths; treating string value as one file path",
+            artifact={"repo_context_files": context_files},
+        )
+        context_files = [context_files]
+    elif not isinstance(context_files, list):
+        get_logger().warning(
+            "repo_context_files should be a list of file paths; skipping repo context",
+            artifact={"repo_context_files": context_files},
+        )
+        return ""
+
+    provider_class = type(git_provider)
+    if provider_class.get_repo_file_content is GitProvider.get_repo_file_content:
+        if provider_class not in _unsupported_repo_context_provider_classes:
+            _unsupported_repo_context_provider_classes.add(provider_class)
+            get_logger().warning(
+                f"repo_context_files is configured, but {provider_class.__name__} does not support repository "
+                "file fetching; skipping repo context"
+            )
+        return ""
 
     max_lines = get_settings().config.get("repo_context_max_lines", 500)
     try:
@@ -59,6 +126,4 @@ def build_repo_context(git_provider) -> str:
     if not files:
         return ""
 
-    rendered_lines = render_instruction_files(files).splitlines()
-
-    return "\n".join(rendered_lines[:max_lines]).strip()
+    return render_instruction_files_with_line_budget(files, max_lines)

pr_agent/git_providers/gitlab_provider.py

@@ -796,6 +796,17 @@ def get_repo_settings(self):
         except Exception:
             return ""
 
+    def get_repo_file_content(self, file_path: str):
+        try:
+            project = self.gl.projects.get(self.id_project)
+            contents = project.files.get(file_path=file_path, ref=project.default_branch).decode()
+            return decode_if_bytes(contents)
+        except GitlabGetError:
+            return ""
+        except Exception as e:
+            get_logger().warning(f"Failed to load repo file: {file_path}, error: {e}")
+            return ""
+
     def get_workspace_name(self):
         return self.id_project.split('/')[0]
 

pr_agent/settings/configuration.toml

@@ -25,7 +25,7 @@ ai_timeout=120 # 2 minutes
 skip_keys = []
 custom_reasoning_model = false # when true, disables system messages and temperature controls for models that don't support chat-style inputs
 response_language="en-US" # Language locales code for PR responses in ISO 3166 and ISO 639 format (e.g., "en-US", "it-IT", "zh-CN", ...)
-repo_context_files = [] # Explicit repository-relative files to include as AI prompt context, e.g. ["AGENTS.md"]
+repo_context_files = [] # Explicit repository-relative files to include as AI prompt context; use a list, e.g. ["AGENTS.md"]
 repo_context_max_lines = 500 # Maximum total lines rendered from configured repo context files
 # token limits
 max_description_tokens = 500

tests/unittest/test_gitlab_provider.py

@@ -73,6 +73,27 @@ def test_get_pr_file_content_other_exception(self, gitlab_provider, mock_project
 
         assert content == ""
 
+    def test_get_repo_file_content_loads_from_default_branch(self, gitlab_provider, mock_gitlab_client, mock_project):
+        mock_project.default_branch = "main"
+        mock_file = MagicMock(ProjectFile)
+        mock_file.decode.return_value = b"repo context"
+        mock_project.files.get.return_value = mock_file
+
+        content = gitlab_provider.get_repo_file_content("AGENTS.md")
+
+        assert content == "repo context"
+        mock_gitlab_client.projects.get.assert_called_with("test/repo")
+        mock_project.files.get.assert_called_once_with(file_path="AGENTS.md", ref="main")
+        mock_file.decode.assert_called_once()
+
+    def test_get_repo_file_content_treats_missing_file_as_empty(self, gitlab_provider, mock_project):
+        mock_project.default_branch = "main"
+        mock_project.files.get.side_effect = GitlabGetError("404 Not Found")
+
+        content = gitlab_provider.get_repo_file_content("AGENTS.md")
+
+        assert content == ""
+
     def test_create_or_update_pr_file_create_new(self, gitlab_provider, mock_project):
         mock_project.files.get.side_effect = GitlabGetError("404 Not Found")
         mock_file = MagicMock()

tests/unittest/test_repo_context.py

@@ -1,10 +1,12 @@
-from unittest.mock import Mock
+from unittest.mock import Mock, patch
 
 import pytest
 from jinja2 import Environment, StrictUndefined
 
+from pr_agent.algo import repo_context
 from pr_agent.algo.repo_context import build_repo_context, render_instruction_files
 from pr_agent.config_loader import get_settings
+from pr_agent.git_providers.git_provider import GitProvider
 from pr_agent.git_providers.github_provider import GithubProvider
 
 
@@ -18,16 +20,22 @@ def get_repo_file_content(self, file_path: str):
         return self.files.get(file_path)
 
 
+class UnsupportedProvider:
+    get_repo_file_content = GitProvider.get_repo_file_content
+
+
 @pytest.fixture
 def repo_context_settings():
     settings = get_settings()
     original_files = settings.config.get("repo_context_files", [])
     original_max_lines = settings.config.get("repo_context_max_lines", 500)
+    original_warned_provider_classes = repo_context._unsupported_repo_context_provider_classes.copy()
 
     yield settings
 
     settings.set("CONFIG.REPO_CONTEXT_FILES", original_files)
     settings.set("CONFIG.REPO_CONTEXT_MAX_LINES", original_max_lines)
+    repo_context._unsupported_repo_context_provider_classes = original_warned_provider_classes
 
 
 def test_build_repo_context_fetches_and_formats_configured_files(repo_context_settings):
@@ -101,12 +109,19 @@ def test_build_repo_context_enforces_total_line_cap(repo_context_settings):
         "CONTRIBUTING.md": "four\nfive",
     })
 
-    assert build_repo_context(provider) == (
+    context = build_repo_context(provider)
+
+    assert context == (
         "You are being given instruction files. Follow them as project-specific guidance when reviewing code.\n"
         "<instruction_files>\n"
         '<file path="AGENTS.md" scope="repo-root">\n'
-        "`````markdown"
+        "`````markdown\n"
+        "...(truncated)...\n"
+        "`````\n"
+        "</file>\n\n"
+        "</instruction_files>"
     )
+    assert context.endswith("`````\n</file>\n\n</instruction_files>")
 
 
 def test_build_repo_context_returns_empty_when_no_files_configured(repo_context_settings):
@@ -115,6 +130,47 @@ def test_build_repo_context_returns_empty_when_no_files_configured(repo_context_
     assert build_repo_context(FakeProvider({"AGENTS.md": "repo purpose"})) == ""
 
 
+def test_build_repo_context_treats_string_config_as_single_file(repo_context_settings):
+    repo_context_settings.set("CONFIG.REPO_CONTEXT_FILES", "AGENTS.md")
+    provider = FakeProvider({"AGENTS.md": "repo purpose"})
+
+    assert build_repo_context(provider) == (
+        "You are being given instruction files. Follow them as project-specific guidance when reviewing code.\n"
+        "<instruction_files>\n"
+        '<file path="AGENTS.md" scope="repo-root">\n'
+        "`````markdown\n"
+        "repo purpose\n"
+        "`````\n"
+        "</file>\n\n"
+        "</instruction_files>"
+    )
+    assert provider.requested_paths == ["AGENTS.md"]
+
+
+def test_build_repo_context_skips_non_list_container(repo_context_settings):
+    repo_context_settings.set("CONFIG.REPO_CONTEXT_FILES", {"AGENTS.md": True})
+    provider = FakeProvider({"AGENTS.md": "repo purpose"})
+
+    assert build_repo_context(provider) == ""
+    assert provider.requested_paths == []
+
+
+def test_build_repo_context_warns_once_for_provider_without_repo_file_fetching(repo_context_settings):
+    repo_context_settings.set("CONFIG.REPO_CONTEXT_FILES", ["AGENTS.md"])
+    provider = UnsupportedProvider()
+
+    with patch("pr_agent.algo.repo_context.get_logger") as mock_get_logger:
+        context = build_repo_context(provider)
+        second_context = build_repo_context(provider)
+
+    assert context == ""
+    assert second_context == ""
+    mock_get_logger.return_value.warning.assert_called_once_with(
+        "repo_context_files is configured, but UnsupportedProvider does not support repository file fetching; "
+        "skipping repo context"
+    )
+
+
 def test_github_provider_decodes_repo_context_files_and_treats_failures_as_missing():
     provider = GithubProvider.__new__(GithubProvider)
     provider.repo_obj = Mock()