chore: address further agent comments

Author: avidspartan1

pr_agent/git_providers/gitea_provider.py

@@ -241,7 +241,7 @@ def publish_persistent_comment(self, pr_comment: str,
     def publish_comment(self, comment: str,is_temporary: bool = False) -> None:
         """Publish a comment to the pull request"""
         if is_temporary and not get_settings().config.publish_output_progress:
-            get_logger().debug(f"Skipping publish_comment for temporary comment")
+            get_logger().debug("Skipping publish_comment for temporary comment")
             return None
 
         if self.enabled_issue:
@@ -753,12 +753,12 @@ def get_repo_file_content(self, file_path: str) -> str:
         """
         try:
             if not self.owner or not self.repo:
-                self.logger.warning(f"Cannot get repo file content: owner or repo not set")
+                self.logger.warning("Cannot get repo file content: owner or repo not set")
                 return ""
 
             ref = self.base_sha or self.base_ref or self.sha
             if not ref:
-                self.logger.warning(f"Cannot get repo file content: ref not set")
+                self.logger.warning("Cannot get repo file content: ref not set")
                 return ""
 
             content = self.repo_api.get_file_content(

pr_agent/git_providers/utils.py

@@ -287,10 +287,11 @@ def apply_repo_settings(pr_url):
                             os.close(fd)
 
                     try:
-                        dynconf_kwargs = {'core_loaders': [],  # DISABLE default loaders, otherwise will load toml files more than once.
+                        dynconf_kwargs = {'core_loaders': [],
+                             # Disable default loaders, otherwise TOML files load more than once.
                              'loaders': ['pr_agent.custom_merge_loader'],
-                             # Use a custom loader to merge sections, but overwrite their overlapping values. Don't involve ENV variables.
-                             'merge_enabled': True  # Merge multiple files; ensures [XYZ] sections only overwrite overlapping keys, not whole sections.
+                             # Merge sections and overwrite overlapping values without involving environment variables.
+                             'merge_enabled': True
                          }
 
                         new_settings = Dynaconf(settings_files=repo_settings_files,
@@ -364,13 +365,21 @@ def handle_configurations_errors(config_errors, git_provider):
                 err_message = err['error']
                 config_type = err['category']
                 header = f"❌ **PR-Agent failed to apply '{config_type}' repo settings**"
-                body = f"{header}\n\nThe configuration file needs to be a valid [TOML](https://qodo-merge-docs.qodo.ai/usage-guide/configuration_options/), please fix it.\n\n"
+                body = (
+                    f"{header}\n\nThe configuration file needs to be a valid "
+                    "[TOML](https://qodo-merge-docs.qodo.ai/usage-guide/configuration_options/), please fix it.\n\n"
+                )
                 body += f"___\n\n**Error message:**\n`{err_message}`\n\n"
-                if git_provider.is_supported("gfm_markdown"):
-                    body += f"\n\n<details><summary>Configuration content:</summary>\n\n```toml\n{configuration_file_content}\n```\n\n</details>"
+                if config_type == "global":
+                    body += "\n\nThe invalid configuration came from the organization's global settings file."
+                elif git_provider.is_supported("gfm_markdown"):
+                    body += (
+                        "\n\n<details><summary>Configuration content:</summary>\n\n"
+                        f"```toml\n{configuration_file_content}\n```\n\n</details>"
+                    )
                 else:
                     body += f"\n\n**Configuration content:**\n\n```toml\n{configuration_file_content}\n```\n\n"
-                get_logger().warning(f"Sending a 'configuration error' comment to the PR", artifact={'body': body})
+                get_logger().warning("Sending a 'configuration error' comment to the PR", artifact={'body': body})
                 # git_provider.publish_comment(body)
                 if hasattr(git_provider, 'publish_persistent_comment'):
                     git_provider.publish_persistent_comment(body,
@@ -380,7 +389,7 @@ def handle_configurations_errors(config_errors, git_provider):
                 else:
                     git_provider.publish_comment(body)
     except Exception as e:
-        get_logger().exception(f"Failed to handle configurations errors", e)
+        get_logger().exception("Failed to handle configurations errors", e)
 
 
 def set_claude_model():

tests/unittest/test_git_provider_utils.py

@@ -48,6 +48,7 @@ def test_apply_repo_settings_merges_global_before_local_settings(monkeypatch):
     original_extra_instructions = settings.pr_reviewer.extra_instructions
     original_enable_intro_text = settings.pr_reviewer.enable_intro_text
     monkeypatch.setattr(utils, "get_git_provider_with_context", lambda pr_url: FakeSettingsProvider())
+    monkeypatch.delenv("AUTO_CAST_FOR_DYNACONF", raising=False)
 
     try:
         apply_repo_settings("https://github.example.com/org/service/pull/1")
@@ -129,7 +130,7 @@ def test_handle_configurations_errors_publishes_each_error():
             "category": "local",
         },
         {
-            "settings": b"[pr_reviewer]\nnum_max_findings =",
+            "settings": b"[github]\nuser_token = \"secret-token\"\n[pr_reviewer]\nnum_max_findings =",
             "error": "Second error",
             "category": "global",
         },
@@ -139,7 +140,9 @@ def test_handle_configurations_errors_publishes_each_error():
     assert "First error" in provider.comments[0]
     assert "[config]\nmodel =" in provider.comments[0]
     assert "Second error" in provider.comments[1]
-    assert "[pr_reviewer]\nnum_max_findings =" in provider.comments[1]
+    assert "organization's global settings file" in provider.comments[1]
+    assert "secret-token" not in provider.comments[1]
+    assert "num_max_findings" not in provider.comments[1]
 
 
 def test_handle_configurations_errors_ignores_empty_sentinel_entry():

tests/unittest/test_repo_context.py

@@ -112,7 +112,7 @@ def test_build_repo_context_refreshes_process_cache_after_ttl(repo_context_setti
     first_provider = FakeProvider({"AGENTS.md": "Repo purpose"}, pr_url="https://example.com/org/repo/pull/1")
     second_provider = FakeProvider({"AGENTS.md": "Changed repo purpose"}, pr_url="https://example.com/org/repo/pull/1")
 
-    with patch("pr_agent.algo.repo_context.time.monotonic", side_effect=[100, 100, 2000, 2000, 2000]):
+    with patch("pr_agent.algo.repo_context.time.monotonic", side_effect=[100, 100, 2000, 2000, 2000, 2000]):
         first_context = build_repo_context(first_provider)
         second_context = build_repo_context(second_provider)
 
@@ -128,7 +128,7 @@ def test_build_repo_context_refreshes_empty_process_cache_after_ttl(repo_context
     first_provider = FakeProvider({}, pr_url="https://example.com/org/repo/pull/1")
     second_provider = FakeProvider({"AGENTS.md": "Repo purpose"}, pr_url="https://example.com/org/repo/pull/1")
 
-    with patch("pr_agent.algo.repo_context.time.monotonic", side_effect=[100, 100, 2000, 2000, 2000]):
+    with patch("pr_agent.algo.repo_context.time.monotonic", side_effect=[100, 100, 2000, 2000, 2000, 2000]):
         first_context = build_repo_context(first_provider)
         second_context = build_repo_context(second_provider)