fix(cli): report circular dependencies consistently

Author: duriantaco

docs/cli-output.md

@@ -120,6 +120,18 @@ verdicts, records the affected dead-code candidates as abstentions, emits
 `analysis_summary.grep_verify.status` and `incomplete_reason`; increase the
 budget and rerun before treating the dead-code result as complete.
 
+Circular dependencies (`SKY-CIRC`) are shown in rich, pretty, and concise
+output, and remain available in JSON under `circular_dependencies`. When
+source evidence is available, the finding points to an actual import in the
+cycle. Ordinary package re-exports do not by themselves form a cycle.
+
+`--strict` counts circular dependencies and exits with status `1` when they
+remain in the selected report. Without an explicit `--gate`, concise output
+also exits `1` for cycles, as it does for other findings. Ordinary non-strict
+gates keep their existing thresholds: cycles do not contribute to
+`max_quality` or grades. The existing `--force` override can bypass finding
+failures, but incomplete analysis still exits `2`.
+
 The legacy flags still work:
 
 ```bash

skylos/analysis/circular_deps.py

@@ -340,7 +340,29 @@ def analyze(self) -> List[CircularDependency]:
         return findings
 
     def get_findings(self) -> List[Dict[str, Any]]:
-        return [cd.to_dict() for cd in self.analyze()]
+        # Use recorded import edges, not filenames guessed from module names.
+        # A graph assembled without source evidence should remain locationless.
+        edge_locations = {}
+        for dep in self.all_deps:
+            source = self.modules.get(dep.from_module)
+            if source and dep.import_line > 0:
+                edge = (dep.from_module, dep.to_module)
+                location = (str(source), dep.import_line)
+                edge_locations[edge] = min(edge_locations.get(edge, location), location)
+
+        findings = []
+        for cd in self.analyze():
+            finding = cd.to_dict()
+            edges = zip(cd.cycle, cd.cycle[1:] + cd.cycle[:1])
+            locations = [
+                edge_locations[edge] for edge in edges if edge in edge_locations
+            ]
+            if locations:
+                # Stable across file discovery order and repeated imports. Only
+                # edges in this cycle qualify, not chords forming another cycle.
+                finding["file"], finding["line"] = min(locations)
+            findings.append(finding)
+        return findings
 
     def get_core_infrastructure(self) -> Set[str]:
         cycles = self.find_simple_cycles()

skylos/cli.py

@@ -2291,6 +2291,7 @@ def _concise_scan_exit_code(
     ("reliability", "reliability issue"),
     ("ai_defects", "AI defect"),
     ("quality", "quality issue"),
+    ("circular_dependencies", "circular dependency"),
     ("secrets", "secret"),
     ("custom_rules", "custom rule"),
     ("dependency_vulnerabilities", "dependency vulnerability"),

skylos/core/gatekeeper.py

@@ -424,6 +424,7 @@ def _check_strict_gate(
     reliability,
     ai_defects,
     quality,
+    circular_dependencies,
     secrets,
     dependencies,
 ):
@@ -434,6 +435,7 @@ def _check_strict_gate(
         + len(reliability)
         + len(ai_defects)
         + len(gate_quality)
+        + len(circular_dependencies)
         + len(secrets)
         + len(dependencies)
     )
@@ -629,6 +631,7 @@ def check_gate(results, config, strict=False, provenance=None):
             reliability=reliability,
             ai_defects=ai_defects,
             quality=gate_quality,
+            circular_dependencies=results.get("circular_dependencies", []) or [],
             secrets=secrets,
             dependencies=dependencies,
         )

skylos/ui/terminal_report.py

@@ -21,6 +21,7 @@
     ("reliability", "Reliability", "reliability issue"),
     ("secrets", "Secret", "secret detected"),
     ("quality", "Quality", "quality issue"),
+    ("circular_dependencies", "Architecture", "circular dependency"),
     ("custom_rules", "Custom", "custom rule"),
     ("dependency_vulnerabilities", "Dependency", "dependency vulnerability"),
     ("unused_functions", "Dead Code", "unused function"),
@@ -67,6 +68,7 @@
     "unused_classes",
     "ai_defects",
     "quality",
+    "circular_dependencies",
     "custom_rules",
     "danger",
     "reliability",

test/test_circular_deps.py

@@ -153,6 +153,88 @@ def test_direct_module_self_import_is_not_suppressed(mode):
     assert findings[0]["cycle"] == ["module"]
 
 
+@pytest.mark.parametrize("mode", ["ast", "raw"])
+def test_absolute_and_relative_package_reexports_match_reported_example(mode):
+    rule = _rule_for_sources(
+        {
+            "demo_pkg": (
+                "/project/demo_pkg/__init__.py",
+                "from demo_pkg.core import value\n__all__ = ['value']\n",
+            ),
+            "demo_pkg.core": ("/project/demo_pkg/core.py", "value = 1\n"),
+            "relative_pkg": (
+                "/project/relative_pkg/__init__.py",
+                "from .core import value\n__all__ = ['value']\n",
+            ),
+            "relative_pkg.core": ("/project/relative_pkg/core.py", "value = 2\n"),
+            "consumer": (
+                "/project/consumer.py",
+                "import demo_pkg\nimport relative_pkg\n",
+            ),
+        },
+        mode,
+    )
+
+    assert rule.analyze() == []
+    assert dict(rule._analyzer.dependencies) == {
+        "demo_pkg": {"demo_pkg.core"},
+        "relative_pkg": {"relative_pkg.core"},
+        "consumer": {"demo_pkg", "relative_pkg"},
+    }
+
+
+@pytest.mark.parametrize("mode", ["ast", "raw"])
+@pytest.mark.parametrize("reverse_files", [False, True])
+def test_circular_finding_has_stable_location_on_a_real_cycle_edge(mode, reverse_files):
+    sources = {
+        "alpha": (
+            "/project/alpha.py",
+            "import helper\n\nfrom beta import value\nfrom beta import other\n",
+        ),
+        "beta": ("/project/beta.py", "from alpha import value\n"),
+        "helper": ("/project/helper.py", ""),
+    }
+    if reverse_files:
+        sources = dict(reversed(list(sources.items())))
+    rule = _rule_for_sources(sources, mode)
+
+    findings = rule.analyze()
+
+    assert len(findings) == 1
+    assert set(findings[0]["cycle"]) == {"alpha", "beta"}
+    assert findings[0]["file"] == "/project/alpha.py"
+    assert findings[0]["line"] == 3
+
+
+@pytest.mark.parametrize("mode", ["ast", "raw"])
+def test_cycle_location_does_not_use_an_edge_from_another_cycle(mode):
+    rule = _rule_for_sources(
+        {
+            "alpha": ("/project/alpha.py", "import gamma\n\nimport beta\n"),
+            "beta": ("/project/beta.py", "import gamma\n"),
+            "gamma": ("/project/gamma.py", "import alpha\n"),
+        },
+        mode,
+    )
+
+    findings = rule.analyze()
+    long_cycle = next(finding for finding in findings if finding["cycle_length"] == 3)
+
+    assert long_cycle["file"] == "/project/alpha.py"
+    assert long_cycle["line"] == 3
+
+
+def test_manual_cycle_graph_does_not_invent_an_import_location():
+    analyzer = CircularDependencyAnalyzer()
+    analyzer.modules = {"a": "a.py", "b": "b.py"}
+    analyzer.dependencies = {"a": {"b"}, "b": {"a"}}
+
+    finding = analyzer.get_findings()[0]
+
+    assert "file" not in finding
+    assert "line" not in finding
+
+
 @pytest.mark.parametrize("child_source", ["value = 'label'", "import package"])
 def test_same_package_graph_has_python_native_cycle_parity(child_source):
     if circular_deps._fast_find_cycles is None:

test/test_cli.py

@@ -2518,6 +2518,151 @@ def test_main_json_strict_failure_exits_nonzero(monkeypatch):
     mock_print.assert_called_once_with(json.dumps(result))
 
 
+@pytest.mark.parametrize(
+    ("output_format", "extra_args", "incomplete", "expected_exit", "show_cycle"),
+    [
+        pytest.param("concise", [], False, 1, True, id="concise"),
+        pytest.param("concise", ["--limit", "0"], False, 1, False, id="concise-limit"),
+        pytest.param("json", ["--strict"], False, 1, True, id="json-strict"),
+        pytest.param(
+            "json", ["--strict", "--gate"], False, 1, True, id="json-strict-gate"
+        ),
+        pytest.param("pretty", ["--strict"], False, 1, True, id="pretty-strict"),
+        pytest.param("rich", ["--strict"], False, 1, True, id="rich-strict"),
+        pytest.param(
+            "concise",
+            ["--strict", "--select=SKY-L012"],
+            False,
+            0,
+            False,
+            id="concise-unselected",
+        ),
+        pytest.param(
+            "json",
+            ["--strict", "--gate", "--select=SKY-L012"],
+            False,
+            0,
+            False,
+            id="json-strict-gate-unselected",
+        ),
+        pytest.param("json", ["--gate"], False, 0, True, id="json-ordinary-gate"),
+        pytest.param("concise", ["--gate"], False, 0, True, id="concise-ordinary-gate"),
+        pytest.param("json", ["--strict", "--force"], False, 0, True, id="json-forced"),
+        pytest.param(
+            "json", ["--strict", "--gate"], True, 2, True, id="json-incomplete"
+        ),
+        pytest.param(
+            "concise", ["--force"], True, 2, True, id="concise-incomplete-forced"
+        ),
+    ],
+)
+def test_main_circular_dependency_reporting_and_exit_codes(
+    monkeypatch,
+    capsys,
+    output_format,
+    extra_args,
+    incomplete,
+    expected_exit,
+    show_cycle,
+):
+    cycle = {
+        "rule_id": "SKY-CIRC",
+        "kind": "circular_dependency",
+        "category": "ARCHITECTURE",
+        "severity": "MEDIUM",
+        "file": "pkg/left.py",
+        "line": 4,
+        "message": "Circular dependency: pkg.left → pkg.right → pkg.left",
+        "cycle": ["pkg.left", "pkg.right"],
+        "cycle_length": 2,
+        "suggested_break": "pkg.left → pkg.right",
+    }
+    result = {
+        "analysis_summary": {"total_files": 2},
+        "circular_dependencies": [cycle],
+    }
+    if incomplete:
+        result["analysis_errors"] = [
+            {
+                "rule_id": "SKY-ANALYSIS-INCOMPLETE",
+                "kind": "syntax_error",
+                "severity": "HIGH",
+                "file": "broken.py",
+                "line": 1,
+                "message": "invalid syntax",
+            }
+        ]
+    monkeypatch.setattr(
+        cli.sys,
+        "argv",
+        [
+            "skylos",
+            ".",
+            "--format",
+            output_format,
+            "--no-provenance",
+            "--no-upload",
+            *extra_args,
+        ],
+    )
+    terminal_output = StringIO()
+    fake_logger = Mock()
+    fake_logger.console = Console(
+        file=terminal_output,
+        width=160,
+        force_terminal=False,
+        theme=cli._skylos_console_theme(),
+    )
+    exit_code = 0
+    with (
+        patch("skylos.cli.setup_logger", return_value=fake_logger),
+        patch("skylos.cli.Progress", return_value=_progress_ctx()),
+        patch("skylos.cli.run_analyze", return_value=json.dumps(result)),
+        patch("skylos.cli.load_config", return_value={"gate": {"max_quality": 0}}),
+        patch("skylos.cli.print_badge"),
+    ):
+        try:
+            cli.main()
+        except SystemExit as exc:
+            exit_code = exc.code
+
+    output = capsys.readouterr().out + terminal_output.getvalue()
+    assert exit_code == expected_exit
+    if output_format == "json":
+        rendered = json.loads(output)
+        assert rendered.get("circular_dependencies", []) == (
+            [cycle] if show_cycle else []
+        )
+    elif show_cycle:
+        if output_format == "rich":
+            assert "Circular Dependencies" in output
+            assert "pkg.left → pkg.right → pkg.left" in output
+        else:
+            assert output.count("SKY-CIRC") == 1
+            assert "pkg/left.py:4" in output
+            assert cycle["message"] in output
+    else:
+        assert "SKY-CIRC" not in output
+        assert cycle["message"] not in output
+    if incomplete:
+        assert "SKY-ANALYSIS-INCOMPLETE" in output
+
+
+def test_concise_circular_dependency_without_location_remains_visible():
+    result = {
+        "circular_dependencies": [
+            {
+                "rule_id": "SKY-CIRC",
+                "message": "Circular dependency: left → right → left",
+            }
+        ]
+    }
+
+    assert cli._format_concise_results(result) == (
+        "?:1  SKY-CIRC  Circular dependency: left → right → left\n"
+    )
+
+
 def test_main_json_incomplete_analysis_exits_two_after_output(monkeypatch):
     result = {
         "analysis_summary": {

test/test_gatekeeper.py

@@ -247,6 +247,44 @@ def test_check_gate_strict_counts_reliability_findings():
     assert reasons == ["Strict mode: 1 issue(s) found"]
 
 
+def test_check_gate_strict_counts_circular_dependencies_but_not_advisory_quality():
+    results = {
+        "circular_dependencies": [
+            {"rule_id": "SKY-CIRC", "cycle": ["pkg.left", "pkg.right"]},
+            {"rule_id": "SKY-CIRC", "cycle": ["pkg.other", "pkg.last"]},
+        ],
+        "quality": [{"rule_id": "SKY-Q802", "advisory": True}],
+    }
+
+    passed, reasons = gk.check_gate(results, {}, strict=True)
+
+    assert passed is False
+    assert reasons == ["Strict mode: 2 issue(s) found"]
+
+
+def test_circular_dependencies_do_not_change_ordinary_gate_thresholds():
+    results = {
+        "circular_dependencies": [
+            {"rule_id": "SKY-CIRC", "severity": "HIGH", "cycle": ["left", "right"]}
+        ],
+    }
+
+    passed, reasons = gk.check_gate(
+        results,
+        {
+            "gate": {
+                "max_quality": 0,
+                "max_high": 0,
+                "max_security": 0,
+                "max_dead_code": 0,
+            }
+        },
+    )
+
+    assert passed is True
+    assert reasons == []
+
+
 def test_reliability_does_not_affect_security_thresholds():
     results = {
         "danger": [],