fix(recorders): a flat dependency graph is not a ranking (#2556)

gitgalaxy/recorders/llm_recorder.py

@@ -469,17 +469,33 @@ def _build_markdown(
             reverse=True,
         )[:5]
         lines.append("### Top 5 Structural Pillars (Highest 'Imported By' / Blast Radius)")
-        lines.append(
-            "These are the most interconnected files relative to the rest of this repository. On a repo with dense "
-            "internal coupling, that means core load-bearing infrastructure -- changes carry real cascading-break "
-            "risk. On a repo with a flatter internal architecture, the gap between #1 and #5 may be small, and this "
-            "list is a weaker signal accordingly; compare the connection counts below before treating it as a verdict.\n"
-        )
-        for rank, file_data in enumerate(pillars, 1):
-            name = file_data.get("name", "Unknown")
-            path = file_data.get("path", "Unknown")
-            count = file_data.get("telemetry", {}).get("popularity", 0)
-            lines.append(f"{rank}. **{name}** (`{path}`) — {count} inbound connections")
+        # #2556: ranking by popularity is meaningless when the maximum is 0 --
+        # the sort is stable, so an all-zero graph just emits the first five
+        # files in scan order (documentation, usually) under a heading that
+        # calls them "the most interconnected files". An LLM consuming this
+        # brief repeats that as fact. A flat graph is a real finding; say so
+        # instead of dressing scan order up as a ranking.
+        if not any(f.get("telemetry", {}).get("popularity", 0) > 0 for f in pillars):
+            lines.append(
+                "No file in this repository is imported by another file that GitGalaxy could resolve, so there is "
+                "no blast-radius ranking to report. That is itself a finding: either the codebase genuinely has no "
+                "internal dependency structure (a collection of scripts, documents or configuration rather than a "
+                "coupled system), or its import style is one the engine does not resolve for this language. Do not "
+                "infer that any file is load-bearing from this section.\n"
+            )
+        else:
+            lines.append(
+                "These are the most interconnected files relative to the rest of this repository. On a repo with "
+                "dense internal coupling, that means core load-bearing infrastructure -- changes carry real "
+                "cascading-break risk. On a repo with a flatter internal architecture, the gap between #1 and #5 may "
+                "be small, and this list is a weaker signal accordingly; compare the connection counts below before "
+                "treating it as a verdict.\n"
+            )
+            for rank, file_data in enumerate(pillars, 1):
+                name = file_data.get("name", "Unknown")
+                path = file_data.get("path", "Unknown")
+                count = file_data.get("telemetry", {}).get("popularity", 0)
+                lines.append(f"{rank}. **{name}** (`{path}`) — {count} inbound connections")
         lines.append("")
 
         orchestrators = sorted(
@@ -488,14 +504,29 @@ def _build_markdown(
             reverse=True,
         )[:5]
         lines.append("### Top 5 Orchestrators (Highest 'Imports' / Fragility Index)")
-        lines.append(
-            "These files pull in the most external dependencies. They are highly coupled and fragile to API changes.\n"
-        )
-        for rank, file_data in enumerate(orchestrators, 1):
-            name = file_data.get("name", "Unknown")
-            path = file_data.get("path", "Unknown")
-            count = len(file_data.get("raw_imports", [])) if isinstance(file_data.get("raw_imports"), list) else 0
-            lines.append(f"{rank}. **{name}** (`{path}`) — {count} outbound dependencies")
+
+        def _outbound(file_data):
+            raw = file_data.get("raw_imports", [])
+            return len(raw) if isinstance(raw, list) else 0
+
+        # #2556: the same zero-guard as the pillar list above. This section had
+        # the identical defect and the issue did not mention it -- with no
+        # resolvable imports anywhere it called five files with 0 outbound
+        # dependencies "highly coupled and fragile to API changes".
+        if not any(_outbound(f) > 0 for f in orchestrators):
+            lines.append(
+                "No file in this repository declares an import that GitGalaxy resolved, so there is no coupling "
+                "ranking to report. See the note above -- the same caveat applies.\n"
+            )
+        else:
+            lines.append(
+                "These files pull in the most external dependencies. They are highly coupled and fragile to API "
+                "changes.\n"
+            )
+            for rank, file_data in enumerate(orchestrators, 1):
+                name = file_data.get("name", "Unknown")
+                path = file_data.get("path", "Unknown")
+                lines.append(f"{rank}. **{name}** (`{path}`) — {_outbound(file_data)} outbound dependencies")
         lines.append("")
 
         import heapq

tests/ruff_audit_baseline.json

@@ -22,7 +22,7 @@
   "gitgalaxy/recorders/audit_recorder.py:298: C414": "Unnecessary `list()` call within `sorted()`",
   "gitgalaxy/recorders/audit_recorder.py:358: PERF401": "Use `list.extend` to create a transformed list",
   "gitgalaxy/recorders/gpu_recorder.py:285: C414": "Unnecessary `list()` call within `sorted()`",
-  "gitgalaxy/recorders/llm_recorder.py:1018: SIM102": "Use a single `if` statement instead of nested `if` statements",
+  "gitgalaxy/recorders/llm_recorder.py:1049: SIM102": "Use a single `if` statement instead of nested `if` statements",
   "gitgalaxy/recorders/record_keeper.py:187: W291": "Trailing whitespace",
   "gitgalaxy/recorders/record_keeper.py:188: W291": "Trailing whitespace",
   "gitgalaxy/recorders/record_keeper.py:189: W291": "Trailing whitespace",

tests/tools_recorders/test_llm_recorder.py

@@ -313,3 +313,68 @@ def test_generate_artifacts_integration(recorder, mock_pipeline_state, tmp_path)
 
     assert (tmp_path / "TestProject_galaxy_llm.md").exists()
     assert (tmp_path / "TestProject_galaxy_graph.sqlite").exists()
+
+
+# ==============================================================================
+# #2556: A FLAT DEPENDENCY GRAPH IS NOT A RANKING
+# ==============================================================================
+def _flat_graph_state(mock_pipeline_state):
+    """The same state with every inbound and outbound connection removed."""
+    parsed, unparsable, summary, session = mock_pipeline_state
+    flat = []
+    for f in parsed:
+        g = dict(f)
+        g["telemetry"] = {**g.get("telemetry", {}), "popularity": 0}
+        g["raw_imports"] = []
+        flat.append(g)
+    return flat, unparsable, summary, session
+
+
+def test_structural_pillars_reports_no_ranking_on_a_flat_graph(recorder, mock_pipeline_state):
+    """
+    #2556: scanning cicsdev/cics-genapp produced a "Top 5 Structural Pillars
+    (Highest 'Imported By')" list of five files with 0 inbound connections
+    each -- presented as "the most interconnected files". Ranking by a key
+    that is 0 for everything just emits scan order, and an LLM consuming the
+    brief repeats it as fact.
+    """
+    parsed, unparsable, summary, session = _flat_graph_state(mock_pipeline_state)
+
+    md_text = recorder._build_markdown(parsed, unparsable, summary, session, {})
+
+    assert "Top 5 Structural Pillars" in md_text, "the heading itself should still be present"
+    assert "no blast-radius ranking to report" in md_text
+    assert "0 inbound connections" not in md_text
+    assert "most interconnected files" not in md_text
+
+
+def test_orchestrators_reports_no_ranking_on_a_flat_graph(recorder, mock_pipeline_state):
+    """
+    The neighbouring section had the identical defect, which #2556 does not
+    mention: with no resolvable imports it called five files with 0 outbound
+    dependencies "highly coupled and fragile to API changes".
+    """
+    parsed, unparsable, summary, session = _flat_graph_state(mock_pipeline_state)
+
+    md_text = recorder._build_markdown(parsed, unparsable, summary, session, {})
+
+    assert "Top 5 Orchestrators" in md_text
+    assert "no coupling ranking to report" in md_text
+    assert "0 outbound dependencies" not in md_text
+    assert "highly coupled and fragile" not in md_text
+
+
+def test_rankings_still_render_when_the_graph_has_any_connection(recorder, mock_pipeline_state):
+    """The guard must fire only on an all-zero graph, never on a sparse one."""
+    parsed, unparsable, summary, session = _flat_graph_state(mock_pipeline_state)
+    parsed[0]["telemetry"]["popularity"] = 1
+    parsed[0]["raw_imports"] = ["os"]
+
+    md_text = recorder._build_markdown(parsed, unparsable, summary, session, {})
+
+    assert "most interconnected files" in md_text
+    assert "1 inbound connections" in md_text
+    assert "no blast-radius ranking to report" not in md_text
+    assert "highly coupled and fragile" in md_text
+    assert "1 outbound dependencies" in md_text
+    assert "no coupling ranking to report" not in md_text