fix(pr_agent/algo/utils.py): returning an empty dict when every yaml fallback fails

pr_agent/algo/utils.py

@@ -940,6 +940,8 @@ def load_yaml(response_text: str, keys_fix_yaml: List[str] = [], first_key="", l
         else:
             get_logger().info("Successfully parsed AI prediction after fallbacks",
                               artifact={'response_text': response_text})
+    if data is None:
+        return {}
     return data
 
 

pr_agent/tools/pr_code_suggestions.py

@@ -766,6 +766,11 @@ def _prepare_pr_code_suggestions(self, predictions: str) -> Dict:
                          first_key="code_suggestions", last_key="label")
         if isinstance(data, list):
             data = {'code_suggestions': data}
+        if not isinstance(data, dict) or not isinstance(data.get("code_suggestions"), list):
+            get_logger().error("Failed to parse code suggestions from the AI prediction",
+                               artifact={"predictions": predictions})
+            self.parse_failure_count = getattr(self, "parse_failure_count", 0) + 1
+            return {"code_suggestions": []}
 
         # remove or edit invalid suggestions
         suggestion_list = []
@@ -1202,6 +1207,7 @@ def remove_line_numbers(self, patches_diff_list: List[str]) -> List[str]:
     async def prepare_prediction_main(self, model: str) -> dict:
         self.failed_chunk_count = 0
         self.total_chunk_count = 0
+        self.parse_failure_count = 0
         # get PR diff
         if get_settings().pr_code_suggestions.decouple_hunks:
             self.patches_diff_list = get_pr_multi_diffs(self.git_provider,
@@ -1270,7 +1276,7 @@ async def prepare_prediction_main(self, model: str) -> dict:
                     else:
                         prediction_list.append(prediction)
 
-            self.failed_chunk_count = len(chunk_errors)
+            self.failed_chunk_count = len(chunk_errors) + self.parse_failure_count
             if chunk_errors and not prediction_list:
                 raise chunk_errors[0]
             self.prediction_list = prediction_list

tests/unittest/test_load_yaml.py

@@ -85,7 +85,7 @@ def test_load_yaml_sanitized_to_empty_does_not_return_none_silently(self):
         sink_id = get_logger().add(lambda msg: captured.append(msg), level="WARNING")
         try:
             result = load_yaml('\x08\x08\x08')
-            assert result is None
+            assert result == {}
             assert any("Initial failure to parse AI prediction" in m for m in captured)
         finally:
             get_logger().remove(sink_id)
@@ -97,7 +97,7 @@ def test_load_yaml_genuinely_empty_input_unaffected(self):
         sink_id = get_logger().add(lambda msg: captured.append(msg), level="WARNING")
         try:
             result = load_yaml('')
-            assert result is None
+            assert result == {}
             assert not any("Preprocessing/sanitization removed all content" in m for m in captured)
         finally:
             get_logger().remove(sink_id)
@@ -122,8 +122,8 @@ def test_yaml_info_string_is_case_insensitive(self, label):
     # started with the stray label and a plain-scalar body came back as a
     # folded string instead of None.
     def test_non_yaml_info_string_not_parsed_as_yaml_snippet(self):
-        assert load_yaml("```text\nhello world\n```") is None
-        assert load_yaml("```python\nname: John\n```") is None
+        assert load_yaml("```text\nhello world\n```") == {}
+        assert load_yaml("```python\nname: John\n```") == {}
 
     # A fenced block that only becomes reachable through the snippet fallback
     # (the initial parse fails because of surrounding text) must be extracted

tests/unittest/test_load_yaml_unparseable.py

@@ -0,0 +1,83 @@
+"""Degrade gracefully when an AI prediction cannot be parsed as YAML."""
+import pytest
+
+from pr_agent.algo.utils import load_yaml
+
+UNPARSEABLE = "::: not : valid : yaml :::\n\t- ["
+
+
+def test_return_an_empty_mapping_for_an_unparseable_prediction():
+    """Return an empty mapping so callers can use `in` and `[]` without a None check."""
+    assert load_yaml(UNPARSEABLE) == {}
+
+
+def test_a_parseable_prediction_is_unchanged():
+    """Keep returning the parsed document for valid input."""
+    assert load_yaml("review:\n  score: 8") == {"review": {"score": 8}}
+
+
+def test_membership_test_does_not_raise():
+    """Support `'review' not in data`, which is the first thing pr_reviewer does."""
+    data = load_yaml(UNPARSEABLE)
+    assert "review" not in data
+
+
+def test_get_does_not_raise():
+    """Support `.get(...)`, which is the first thing pr_description and pr_help_message do."""
+    assert load_yaml(UNPARSEABLE).get("pr_files", []) == []
+
+
+def test_reviewer_reports_the_parse_failure_instead_of_crashing():
+    """Reach pr_reviewer's own 'Failed to parse review data' path."""
+    from pr_agent.tools.pr_reviewer import PRReviewer
+
+    reviewer = PRReviewer.__new__(PRReviewer)
+    reviewer.prediction = UNPARSEABLE
+
+    assert reviewer._prepare_pr_review() == ""
+
+
+def test_code_suggestions_returns_an_empty_list_instead_of_crashing():
+    """Guard pr_code_suggestions, which subscripts the parsed result."""
+    from pr_agent.tools.pr_code_suggestions import PRCodeSuggestions
+
+    tool = PRCodeSuggestions.__new__(PRCodeSuggestions)
+
+    assert tool._prepare_pr_code_suggestions(UNPARSEABLE) == {"code_suggestions": []}
+
+
+def test_generate_labels_membership_check_does_not_raise():
+    """Support `'labels' in self.data`, which pr_generate_labels does before anything else."""
+    from pr_agent.tools.pr_generate_labels import PRGenerateLabels
+
+    tool = PRGenerateLabels.__new__(PRGenerateLabels)
+    tool.prediction = UNPARSEABLE
+    tool._prepare_data()
+
+    assert tool.data == {}
+    assert "labels" not in tool.data
+
+
+@pytest.mark.parametrize("payload", ["code_suggestions:\n", "code_suggestions: 5\n",
+                                     "code_suggestions:\n  a: 1\n"])
+def test_a_non_list_code_suggestions_value_is_rejected(payload):
+    """Return an empty result when code_suggestions is present but not a list."""
+    from pr_agent.tools.pr_code_suggestions import PRCodeSuggestions
+
+    tool = PRCodeSuggestions.__new__(PRCodeSuggestions)
+
+    assert tool._prepare_pr_code_suggestions(payload) == {"code_suggestions": []}
+
+
+def test_an_unparseable_chunk_is_recorded_so_the_coverage_footer_still_reports_it():
+    """The empty result must not read as a successful chunk (#2867 counts failed chunks)."""
+    from pr_agent.tools.pr_code_suggestions import PRCodeSuggestions
+
+    tool = PRCodeSuggestions.__new__(PRCodeSuggestions)
+
+    assert tool._prepare_pr_code_suggestions(UNPARSEABLE) == {"code_suggestions": []}
+    assert tool.parse_failure_count == 1
+
+    tool.failed_chunk_count = tool.parse_failure_count
+    tool.total_chunk_count = 2
+    assert "1 of 2 analysis chunks failed" in tool._get_suggestions_coverage_footer()

tests/unittest/test_try_fix_yaml.py

@@ -385,7 +385,7 @@ def test_try_fix_yaml_fallbacks_do_not_log_success_on_none(self):
         sink_id = get_logger().add(lambda msg: captured.append(msg), level="INFO")
         try:
             result = load_yaml('\x08\x08\x08')
-            assert result is None
+            assert result == {}
             assert not any("Successfully parsed" in m for m in captured)
             assert any("Failed to parse AI prediction after fallbacks" in m for m in captured)
         finally:
@@ -398,7 +398,7 @@ def test_diff_marker_fallback_does_not_log_success_on_none(self):
         sink_id = get_logger().add(lambda msg: captured.append(msg), level="INFO")
         try:
             result = load_yaml('-\n-#x')
-            assert result is None
+            assert result == {}
             assert not any("normalizing diff removal markers" in m for m in captured)
             assert any("Failed to parse AI prediction after fallbacks" in m for m in captured)
         finally: