Skip to content

Commit 15f9d8c

Browse files
committed
feat(cli): show human-readable violation explanations in text output
- Render `explain_violation()` output instead of raw violation messages - Improve dependency/node explanation phrasing and add dep-type verb mapping - Document example CLI output and explanation behavior - Add/adjust tests to assert explanations across verbosity levels
1 parent c8f57a3 commit 15f9d8c

6 files changed

Lines changed: 235 additions & 31 deletions

File tree

docs/cli.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,21 @@ pacta scan . --model architecture.yml --rules rules.pacta.yml --baseline baselin
4646
pacta scan . --model architecture.yml --rules rules.pacta.yml --format json
4747
```
4848

49+
**Example output:**
50+
51+
```
52+
✗ 1 violations (1 error)
53+
54+
✗ ERROR [no-domain-infra] Domain cannot depend on Infrastructure @ src/domain/service.py:5:1
55+
status: new
56+
"app.domain.BillingService" in domain layer imports "app.infra.PostgresClient" in infra layer
57+
```
58+
59+
Violations are displayed with human-readable explanations:
60+
61+
- **For dependency violations:** Shows which module imports/calls/uses another, with their respective layers
62+
- **For node violations:** Shows the element type and where it was found (layer, container, context)
63+
4964
## snapshot save
5065

5166
Save architecture snapshot without running rules.

pacta/reporting/renderers/text.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from typing import Literal
22

33
from pacta.reporting.types import EngineError, Report, Violation
4+
from pacta.rules.explain import explain_violation
45

56
Verbosity = Literal["quiet", "normal", "verbose"]
67

@@ -166,7 +167,8 @@ def _render_violation(self, v: Violation, verbosity: Verbosity) -> list[str]:
166167
# Always show status
167168
out.append(f" status: {v.status}")
168169

169-
out.append(f" {v.message}")
170+
# Always show human-readable explanation
171+
out.append(f" {explain_violation(v)}")
170172

171173
# Show suggestion in normal and verbose modes
172174
if verbosity in ("normal", "verbose") and v.suggestion:

pacta/rules/explain.py

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,48 +10,65 @@
1010

1111
def explain_violation(v: Violation) -> str:
1212
"""
13-
Turn a Violation into a compact explanation suitable for CLI/IDE tooltips.
13+
Turn a Violation into a human-readable explanation suitable for CLI/IDE tooltips.
1414
"""
1515
ctx = v.context or {}
1616
target = ctx.get("target")
1717

1818
if target == "dependency":
19-
dep_type = ctx.get("dep_type", "?")
20-
src = ctx.get("src_fqname") or ctx.get("src_id") or "<?>"
21-
dst = ctx.get("dst_fqname") or ctx.get("dst_id") or "<?>"
19+
dep_type = ctx.get("dep_type", "dependency")
20+
src = ctx.get("src_fqname") or ctx.get("src_id") or "unknown"
21+
dst = ctx.get("dst_fqname") or ctx.get("dst_id") or "unknown"
2222
src_layer = ctx.get("src_layer")
2323
dst_layer = ctx.get("dst_layer")
2424

25-
parts: list[str] = []
26-
parts.append(f"Dependency violation ({dep_type}).")
27-
parts.append(f"{src} -> {dst}")
28-
if src_layer or dst_layer:
29-
parts.append(f"layers: {src_layer or '?'} -> {dst_layer or '?'}")
30-
return " ".join(parts)
25+
# Build natural language explanation
26+
verb = _dep_type_verb(dep_type)
27+
28+
if src_layer and dst_layer:
29+
return f'"{src}" in {src_layer} layer {verb} "{dst}" in {dst_layer} layer'
30+
else:
31+
return f'"{src}" {verb} "{dst}"'
3132

3233
if target == "node":
33-
ident = ctx.get("fqname") or ctx.get("node_id") or "<?>"
34-
kind = ctx.get("kind", "?")
34+
ident = ctx.get("fqname") or ctx.get("node_id") or "unknown"
35+
kind = ctx.get("kind", "element")
3536
layer = ctx.get("layer")
36-
context = ctx.get("context")
37+
context_name = ctx.get("context")
3738
container = ctx.get("container")
3839

39-
parts = [f"Node violation ({kind}): {ident}"]
40-
extras = []
40+
# Build natural language explanation
41+
location_parts = []
4142
if layer:
42-
extras.append(f"layer={layer}")
43-
if context:
44-
extras.append(f"context={context}")
43+
location_parts.append(f"{layer} layer")
4544
if container:
46-
extras.append(f"container={container}")
47-
if extras:
48-
parts.append(f"({', '.join(extras)})")
49-
return " ".join(parts)
45+
location_parts.append(f"container {container}")
46+
if context_name:
47+
location_parts.append(f"context {context_name}")
48+
49+
if location_parts:
50+
location = ", ".join(location_parts)
51+
return f'{kind} "{ident}" found in {location}'
52+
else:
53+
return f'{kind} "{ident}" violates architectural constraint'
5054

5155
# Fallback
5256
return v.message
5357

5458

59+
def _dep_type_verb(dep_type: str) -> str:
60+
"""Convert dependency type to a human-readable verb."""
61+
verbs = {
62+
"import": "imports",
63+
"call": "calls",
64+
"inherit": "inherits from",
65+
"instantiate": "instantiates",
66+
"use": "uses",
67+
"reference": "references",
68+
}
69+
return verbs.get(dep_type, f"depends on ({dep_type})")
70+
71+
5572
def explain_rule(rule: Rule) -> str:
5673
"""
5774
Explain a compiled Rule at a high level (for listing rules, debug output).

tests/cli/test_cli.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,79 @@ def test_scan_mode_changed_only(self, tmp_path):
298298
assert mock_scan.call_args.kwargs["mode"] == "changed_only"
299299

300300

301+
class TestHumanReadableOutput:
302+
"""Tests for human-readable violation output (always enabled by default)."""
303+
304+
def test_scan_shows_human_readable_explanation(self, tmp_path, capsys):
305+
"""Test that scan command shows human-readable explanations by default."""
306+
repo_root = tmp_path / "repo"
307+
repo_root.mkdir()
308+
309+
violation = Violation(
310+
rule=RuleRef(
311+
id="test_rule",
312+
name="Test Rule",
313+
severity=Severity.ERROR,
314+
),
315+
message="Test violation message",
316+
location=None,
317+
status="new",
318+
context={
319+
"target": "dependency",
320+
"dep_type": "import",
321+
"src_fqname": "app.domain",
322+
"dst_fqname": "app.infra",
323+
"src_layer": "domain",
324+
"dst_layer": "infra",
325+
},
326+
)
327+
328+
mock_report = create_test_report(repo_root, violations=[violation])
329+
330+
with patch("pacta.cli.scan.run_engine_scan", return_value=mock_report):
331+
exit_code = main(["scan", str(repo_root)])
332+
333+
assert exit_code == 1 # Has violations
334+
captured = capsys.readouterr()
335+
# Human-readable explanation is always shown
336+
assert "app.domain" in captured.out
337+
assert "app.infra" in captured.out
338+
assert "imports" in captured.out
339+
340+
def test_scan_verbose_shows_human_readable_explanation(self, tmp_path, capsys):
341+
"""Test that verbose mode also shows human-readable explanations."""
342+
repo_root = tmp_path / "repo"
343+
repo_root.mkdir()
344+
345+
violation = Violation(
346+
rule=RuleRef(
347+
id="test_rule",
348+
name="Test Rule",
349+
severity=Severity.ERROR,
350+
),
351+
message="Test violation message",
352+
location=None,
353+
status="new",
354+
context={
355+
"target": "dependency",
356+
"dep_type": "import",
357+
"src_fqname": "app.domain",
358+
"dst_fqname": "app.infra",
359+
},
360+
)
361+
362+
mock_report = create_test_report(repo_root, violations=[violation])
363+
364+
with patch("pacta.cli.scan.run_engine_scan", return_value=mock_report):
365+
exit_code = main(["scan", str(repo_root), "--verbose"])
366+
367+
assert exit_code == 1
368+
captured = capsys.readouterr()
369+
# Should have explanation and verbose details
370+
assert "imports" in captured.out
371+
assert "Summary:" in captured.out # verbose marker
372+
373+
301374
class TestErrorHandling:
302375
"""Tests for CLI error handling."""
303376

tests/reporting/test_reporting.py

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,101 @@ def test_text_renderer_normal_no_violations(runinfo):
448448
assert "✓ No violations" in text
449449

450450

451+
# ----------------------------
452+
# Tests: TextReportRenderer human-readable explanations (always enabled)
453+
# ----------------------------
454+
455+
456+
def test_text_renderer_shows_human_readable_explanation(runinfo):
457+
from pacta.reporting.builder import DefaultReportBuilder
458+
from pacta.reporting.renderers.text import TextReportRenderer
459+
460+
violations = [
461+
{
462+
"rule": {"id": "r1", "name": "No domain -> infra", "severity": "error"},
463+
"message": "Domain depends on Infra",
464+
"status": "new",
465+
"context": {
466+
"target": "dependency",
467+
"dep_type": "import",
468+
"src_fqname": "app.domain.service",
469+
"dst_fqname": "app.infra.database",
470+
"src_layer": "domain",
471+
"dst_layer": "infra",
472+
},
473+
}
474+
]
475+
report = DefaultReportBuilder(tool="pacta", version="0.1.0").build(run=runinfo, violations=violations)
476+
text = TextReportRenderer(verbosity="normal").render(report)
477+
478+
# Should contain human-readable explanation by default
479+
assert "app.domain.service" in text
480+
assert "app.infra.database" in text
481+
assert "domain layer" in text
482+
assert "infra layer" in text
483+
assert "imports" in text
484+
485+
486+
def test_text_renderer_node_violation_shows_explanation(runinfo):
487+
from pacta.reporting.builder import DefaultReportBuilder
488+
from pacta.reporting.renderers.text import TextReportRenderer
489+
490+
violations = [
491+
{
492+
"rule": {"id": "r2", "name": "No services in domain", "severity": "warning"},
493+
"message": "Services should not be in domain layer",
494+
"status": "new",
495+
"context": {
496+
"target": "node",
497+
"fqname": "app.domain.BillingService",
498+
"kind": "class",
499+
"layer": "domain",
500+
"container": "backend",
501+
},
502+
}
503+
]
504+
report = DefaultReportBuilder(tool="pacta", version="0.1.0").build(run=runinfo, violations=violations)
505+
text = TextReportRenderer(verbosity="normal").render(report)
506+
507+
# Should contain human-readable node violation explanation
508+
assert "class" in text
509+
assert "app.domain.BillingService" in text
510+
assert "domain layer" in text
511+
assert "container backend" in text
512+
513+
514+
def test_text_renderer_explanation_works_with_all_verbosity_levels(runinfo):
515+
from pacta.reporting.builder import DefaultReportBuilder
516+
from pacta.reporting.renderers.text import TextReportRenderer
517+
518+
violations = [
519+
{
520+
"rule": {"id": "r1", "name": "Test", "severity": "error"},
521+
"message": "Test message",
522+
"status": "new",
523+
"context": {
524+
"target": "dependency",
525+
"dep_type": "import",
526+
"src_fqname": "a.b",
527+
"dst_fqname": "c.d",
528+
},
529+
}
530+
]
531+
report = DefaultReportBuilder(tool="pacta", version="0.1.0").build(run=runinfo, violations=violations)
532+
533+
# Test with normal verbosity - should show human-readable explanation
534+
text_normal = TextReportRenderer(verbosity="normal").render(report)
535+
assert "imports" in text_normal
536+
537+
# Test with verbose verbosity - should also show explanation
538+
text_verbose = TextReportRenderer(verbosity="verbose").render(report)
539+
assert "imports" in text_verbose
540+
541+
# quiet mode doesn't show violations
542+
text_quiet = TextReportRenderer(verbosity="quiet").render(report)
543+
assert "imports" not in text_quiet # quiet mode only shows summary
544+
545+
451546
# ----------------------------
452547
# Edge-cases: normalization of partial locations
453548
# ----------------------------

tests/rules/test_explain.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ def test_explain_violation_dependency_contains_key_info():
3232
)
3333

3434
text = explain_violation(v)
35-
assert "Dependency violation" in text
36-
assert "import" in text
35+
# Human-readable format: "X" in Y layer imports "Z" in W layer
3736
assert "app.domain" in text
3837
assert "app.infra.db" in text
39-
assert "domain" in text
40-
assert "infra" in text
38+
assert "domain layer" in text
39+
assert "infra layer" in text
40+
assert "imports" in text
4141

4242

4343
def test_explain_violation_dependency_falls_back_to_ids_if_fqname_missing():
@@ -53,8 +53,10 @@ def test_explain_violation_dependency_falls_back_to_ids_if_fqname_missing():
5353
)
5454

5555
text = explain_violation(v)
56+
# Falls back to IDs when fqname not available
5657
assert "python://repo::a" in text
5758
assert "python://repo::b" in text
59+
assert "imports" in text
5860

5961

6062
def test_explain_violation_node_contains_kind_and_identity():
@@ -72,12 +74,12 @@ def test_explain_violation_node_contains_kind_and_identity():
7274
)
7375

7476
text = explain_violation(v)
75-
assert "Node violation" in text
77+
# Human-readable format: kind "name" found in layer, container, context
7678
assert "class" in text
7779
assert "app.domain.BillingService" in text
78-
assert "layer=domain" in text
79-
assert "context=billing" in text
80-
assert "container=billing" in text
80+
assert "domain layer" in text
81+
assert "context billing" in text
82+
assert "container billing" in text
8183

8284

8385
def test_explain_violation_unknown_target_falls_back_to_message():

0 commit comments

Comments
 (0)