Skip to content

Commit 1f043f2

Browse files
committed
feat: anti-pattern detection, health scoring, context-aware critiques, UX fixes
1 parent 2617938 commit 1f043f2

7 files changed

Lines changed: 553 additions & 15 deletions

File tree

tests/test_server.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -797,3 +797,84 @@ async def test_no_crash_without_data_flow():
797797
resp = await ac.get("/script/noflow.py")
798798
assert resp.status_code == 200
799799
assert "Data Journey" not in resp.text
800+
801+
802+
# --- Sprint 8: anti-pattern detection UI ---
803+
804+
805+
@pytest.mark.anyio
806+
async def test_antipattern_callouts_render():
807+
"""Script with anti-patterns should show callout cards."""
808+
steps = [Step(line_number=i, type="output", description=f"print('msg{i}')") for i in range(10)]
809+
script = AnalyzedScript(path="messy.py", steps=steps)
810+
project = AnalyzedProject(path="/tmp", scripts=[script])
811+
app = create_app(project)
812+
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac:
813+
resp = await ac.get("/script/messy.py")
814+
assert resp.status_code == 200
815+
assert "No logging framework" in resp.text
816+
817+
818+
@pytest.mark.anyio
819+
async def test_clean_script_no_callouts():
820+
"""Clean script should not show anti-pattern callouts."""
821+
steps = [
822+
Step(line_number=1, type="api_call", description="requests.get()"),
823+
Step(line_number=2, type="decision", description="try/except block"),
824+
Step(line_number=3, type="output", description="logger.info('done')"),
825+
]
826+
script = AnalyzedScript(path="clean.py", steps=steps)
827+
project = AnalyzedProject(path="/tmp", scripts=[script])
828+
app = create_app(project)
829+
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac:
830+
resp = await ac.get("/script/clean.py")
831+
assert resp.status_code == 200
832+
assert "No logging framework" not in resp.text
833+
assert "Repetitive error handling" not in resp.text
834+
835+
836+
@pytest.mark.anyio
837+
async def test_health_badge_on_card():
838+
"""Overview cards should show health badge for scripts with findings."""
839+
steps = [Step(line_number=i, type="output", description=f"print('msg{i}')") for i in range(10)]
840+
script = AnalyzedScript(path="messy.py", steps=steps, is_entry_point=True)
841+
project = AnalyzedProject(path="/tmp", scripts=[script], entry_points=["messy.py"])
842+
app = create_app(project)
843+
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac:
844+
resp = await ac.get("/")
845+
assert resp.status_code == 200
846+
assert "issue" in resp.text.lower()
847+
848+
849+
@pytest.mark.anyio
850+
async def test_phase_proportion_percentage():
851+
"""Phase with >40% of steps should show percentage."""
852+
# 15 output (reporting) + 5 transform (processing) = 75% reporting
853+
steps = [Step(line_number=i, type="output", description=f"print('{i}')") for i in range(15)]
854+
steps += [Step(line_number=i + 20, type="transform", description="sorted()") for i in range(5)]
855+
script = AnalyzedScript(path="heavy.py", steps=steps)
856+
project = AnalyzedProject(path="/tmp", scripts=[script])
857+
app = create_app(project)
858+
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac:
859+
resp = await ac.get("/script/heavy.py")
860+
assert resp.status_code == 200
861+
assert "75%" in resp.text
862+
863+
864+
@pytest.mark.anyio
865+
async def test_credential_dedup_overview():
866+
"""Business view should deduplicate translated credential labels."""
867+
script = AnalyzedScript(path="test.py", steps=[])
868+
project = AnalyzedProject(
869+
path="/tmp",
870+
scripts=[script],
871+
secrets=["PANDADOC_API_KEY", "PANDADOC_TOKEN", "PANDADOC_SECRET", "STRIPE_API_KEY"],
872+
)
873+
app = create_app(project)
874+
async with AsyncClient(transport=ASGITransport(app=app), base_url="http://test") as ac:
875+
resp = await ac.get("/")
876+
# Business view: "PandaDoc credentials" should appear only once, not 3x
877+
text = resp.text
878+
biz_count = text.count("PandaDoc credentials")
879+
# The dedup list renders once; raw technical list has all 3 PANDADOC_* entries
880+
assert biz_count == 1

tests/test_translate.py

Lines changed: 199 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,16 @@
44

55
import pytest
66

7-
from visualpy.models import Service, Step, Trigger
7+
from visualpy.models import AnalyzedScript, Service, Step, Trigger
88
from visualpy.translate import (
99
BUSINESS_LABELS,
1010
PHASE_LABELS,
1111
PHASE_ORDER,
1212
TECHNICAL_LABELS,
1313
TECHNICAL_LABELS_SHORT,
14+
compute_health,
1415
deduplicate_steps,
16+
detect_antipatterns,
1517
explain_pattern,
1618
group_steps_by_phase,
1719
infer_phase,
@@ -622,10 +624,10 @@ class TestExplainPattern:
622624
"""Tests for the pattern insight generator."""
623625

624626
def test_displays_message(self):
625-
steps = [_step("output", "print()") for _ in range(20)]
627+
steps = [_step("output", "print()") for _ in range(5)]
626628
result = explain_pattern("Displays message", steps)
627629
assert "Status logging" in result
628-
assert "20" in result
630+
assert "5" in result
629631

630632
def test_records_activity(self):
631633
steps = [_step("output", "logger.info()") for _ in range(5)]
@@ -641,7 +643,7 @@ def test_records_error(self):
641643
def test_handles_potential_errors(self):
642644
steps = [
643645
Step(line_number=i, type="decision", description=f"try/except {i}")
644-
for i in range(9)
646+
for i in range(4)
645647
]
646648
result = explain_pattern("Handles potential errors", steps)
647649
assert "Defensive coding" in result
@@ -731,3 +733,196 @@ def test_exception_returns_fallback(self):
731733
result = explain_pattern(None, [_step("output", "print()")])
732734
assert isinstance(result, str)
733735
assert "Repeated" in result or "pattern" in result.lower()
736+
737+
738+
# --- Sprint 8: context-aware explain_pattern ----------------------------------
739+
740+
741+
class TestExplainPatternContextAware:
742+
"""explain_pattern should critique at high counts, praise at low counts."""
743+
744+
def test_error_handling_low_count_praise(self):
745+
steps = [_step("decision", "try/except block") for _ in range(3)]
746+
result = explain_pattern("Handles potential errors", steps)
747+
assert "Defensive coding" in result
748+
749+
def test_error_handling_high_count_critique(self):
750+
steps = [_step("decision", "try/except block") for _ in range(8)]
751+
result = explain_pattern("Handles potential errors", steps)
752+
assert "Repetitive" in result
753+
assert "8" in result
754+
755+
def test_print_low_count_neutral(self):
756+
steps = [_step("output", "print()") for _ in range(5)]
757+
result = explain_pattern("Displays message", steps)
758+
assert "Status logging" in result
759+
760+
def test_print_high_count_critique(self):
761+
steps = [_step("output", "print()") for _ in range(20)]
762+
result = explain_pattern("Displays message", steps)
763+
assert "Excessive" in result
764+
assert "logging" in result.lower()
765+
766+
def test_logger_output_high_count_stays_neutral(self):
767+
"""Logger-based output should not be critiqued even at high counts."""
768+
steps = [_step("output", "logger.info()") for _ in range(20)]
769+
result = explain_pattern("Records activity", steps)
770+
assert "Status logging" in result
771+
772+
773+
# --- Sprint 8: detect_antipatterns -------------------------------------------
774+
775+
776+
def _script_with_steps(steps: list[Step]) -> AnalyzedScript:
777+
return AnalyzedScript(path="test.py", steps=steps)
778+
779+
780+
class TestDetectAntipatterns:
781+
def test_print_spam_detected(self):
782+
steps = [_step("output", f"print('msg{i}')") for i in range(10)]
783+
findings = detect_antipatterns(_script_with_steps(steps))
784+
ids = [f["id"] for f in findings]
785+
assert "print_spam" in ids
786+
787+
def test_print_spam_not_triggered_with_logger(self):
788+
steps = [_step("output", f"print('msg{i}')") for i in range(10)]
789+
steps.append(_step("output", "logger.info('start')"))
790+
findings = detect_antipatterns(_script_with_steps(steps))
791+
ids = [f["id"] for f in findings]
792+
assert "print_spam" not in ids
793+
794+
def test_print_spam_threshold(self):
795+
"""Exactly 3 prints → no finding; 4 → finding."""
796+
three = [_step("output", "print('x')") for _ in range(3)]
797+
assert "print_spam" not in [f["id"] for f in detect_antipatterns(_script_with_steps(three))]
798+
four = [_step("output", "print('x')") for _ in range(4)]
799+
assert "print_spam" in [f["id"] for f in detect_antipatterns(_script_with_steps(four))]
800+
801+
def test_phase_imbalance_detected(self):
802+
# 20 output steps (→ reporting) + 2 transform steps (→ processing)
803+
steps = [_step("output", "print('x')") for _ in range(20)]
804+
steps += [_step("transform", "sorted()") for _ in range(2)]
805+
findings = detect_antipatterns(_script_with_steps(steps))
806+
ids = [f["id"] for f in findings]
807+
assert "phase_imbalance" in ids
808+
809+
def test_phase_imbalance_balanced(self):
810+
# Even split across types
811+
steps = [_step("output", "print('x')") for _ in range(5)]
812+
steps += [_step("transform", "sorted()") for _ in range(5)]
813+
steps += [_step("api_call", "requests.get()") for _ in range(5)]
814+
findings = detect_antipatterns(_script_with_steps(steps))
815+
ids = [f["id"] for f in findings]
816+
assert "phase_imbalance" not in ids
817+
818+
def test_error_handling_bulk(self):
819+
steps = [_step("decision", "try/except block") for _ in range(8)]
820+
steps += [_step("api_call", "requests.get()") for _ in range(3)]
821+
findings = detect_antipatterns(_script_with_steps(steps))
822+
ids = [f["id"] for f in findings]
823+
assert "error_handling_bulk" in ids
824+
825+
def test_no_error_handling(self):
826+
steps = [_step("api_call", "requests.get()") for _ in range(12)]
827+
findings = detect_antipatterns(_script_with_steps(steps))
828+
ids = [f["id"] for f in findings]
829+
assert "no_error_handling" in ids
830+
831+
def test_no_error_handling_small_script(self):
832+
"""Scripts with ≤10 steps don't trigger no_error_handling."""
833+
steps = [_step("api_call", "requests.get()") for _ in range(5)]
834+
findings = detect_antipatterns(_script_with_steps(steps))
835+
ids = [f["id"] for f in findings]
836+
assert "no_error_handling" not in ids
837+
838+
def test_no_error_handling_requires_io(self):
839+
"""Scripts with only transforms don't need error handling."""
840+
steps = [_step("transform", "sorted()") for _ in range(15)]
841+
findings = detect_antipatterns(_script_with_steps(steps))
842+
ids = [f["id"] for f in findings]
843+
assert "no_error_handling" not in ids
844+
845+
def test_transform_heavy(self):
846+
steps = [_step("transform", "sorted()") for _ in range(20)]
847+
findings = detect_antipatterns(_script_with_steps(steps))
848+
ids = [f["id"] for f in findings]
849+
assert "transform_heavy" in ids
850+
851+
def test_clean_script(self):
852+
steps = [
853+
_step("api_call", "requests.get()"),
854+
_step("decision", "try/except block"),
855+
_step("transform", "sorted()"),
856+
_step("output", "logger.info('done')"),
857+
]
858+
findings = detect_antipatterns(_script_with_steps(steps))
859+
assert findings == []
860+
861+
def test_empty_script(self):
862+
findings = detect_antipatterns(_script_with_steps([]))
863+
assert findings == []
864+
865+
def test_exception_safe(self):
866+
"""Broken input should not crash — returns empty list."""
867+
findings = detect_antipatterns(None)
868+
assert findings == []
869+
870+
871+
class TestComputeHealth:
872+
def test_clean_score(self):
873+
steps = [_step("api_call", "requests.get()"), _step("decision", "try/except block")]
874+
health = compute_health(_script_with_steps(steps))
875+
assert health["score"] == "clean"
876+
assert health["color"] == "green"
877+
878+
def test_warning_score(self):
879+
steps = [_step("output", f"print('{i}')") for i in range(10)]
880+
health = compute_health(_script_with_steps(steps))
881+
# print_spam is a warning — expect yellow or amber depending on count
882+
assert health["color"] in ("yellow", "amber")
883+
assert len(health["findings"]) >= 1
884+
885+
def test_multi_warning_score(self):
886+
# print_spam + error_handling_bulk → 2 warnings → amber
887+
steps = [_step("output", f"print('{i}')") for i in range(10)]
888+
steps += [_step("decision", "try/except block") for _ in range(8)]
889+
health = compute_health(_script_with_steps(steps))
890+
assert health["color"] == "amber"
891+
assert health["score"] == "has issues"
892+
893+
def test_concern_score(self):
894+
steps = [_step("api_call", "requests.get()") for _ in range(12)]
895+
health = compute_health(_script_with_steps(steps))
896+
assert health["color"] == "red"
897+
assert health["score"] == "needs attention"
898+
899+
def test_exception_safe(self):
900+
"""Broken input cascades safely through detect_antipatterns → clean."""
901+
health = compute_health(None)
902+
assert health["score"] == "clean"
903+
assert health["color"] == "green"
904+
assert health["findings"] == []
905+
906+
907+
# --- Sprint 8: condition simplification --------------------------------------
908+
909+
910+
class TestSimplifyCondition:
911+
"""_simplify_condition improvements for business view."""
912+
913+
def test_dict_access(self):
914+
result = translate_step(_step("decision", "if item['matched']"))
915+
assert "matched" in result.lower()
916+
assert "['" not in result
917+
918+
def test_get_call(self):
919+
result = translate_step(_step("decision", "if item.get('needs_review')"))
920+
assert "needs" in result.lower() or "review" in result.lower()
921+
assert ".get(" not in result
922+
923+
def test_complex_condition_cleaned(self):
924+
result = translate_step(
925+
_step("decision", "if item['matched'] and (not item.get('needs_review'))")
926+
)
927+
assert "['" not in result
928+
assert ".get(" not in result

visualpy/server.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,22 @@ def create_app(project: AnalyzedProject) -> FastAPI:
4343
templates.env.globals["translate_connection"] = translate_connection
4444

4545
# Phase inference globals (Sprint 6.5).
46-
from visualpy.translate import PHASE_LABELS, deduplicate_steps, explain_pattern, group_steps_by_phase, infer_phase
46+
from visualpy.translate import (
47+
PHASE_LABELS,
48+
compute_health,
49+
deduplicate_steps,
50+
detect_antipatterns,
51+
explain_pattern,
52+
group_steps_by_phase,
53+
infer_phase,
54+
)
4755
templates.env.globals["infer_phase"] = infer_phase
4856
templates.env.globals["group_steps_by_phase"] = group_steps_by_phase
4957
templates.env.globals["phase_labels"] = PHASE_LABELS
5058
templates.env.globals["deduplicate_steps"] = deduplicate_steps
5159
templates.env.globals["explain_pattern"] = explain_pattern
60+
templates.env.globals["detect_antipatterns"] = detect_antipatterns
61+
templates.env.globals["compute_health"] = compute_health
5262

5363
# Fallback diagrams for error cases.
5464
_GRAPH_FALLBACK = 'graph LR\n error["Graph generation failed"]'

visualpy/templates/overview.html

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,26 @@ <h2 class="text-lg font-semibold mb-3">
124124
<span x-show="!biz">Secrets</span>
125125
</h2>
126126
{% if project.secrets %}
127-
<ul class="space-y-1 text-sm">
127+
<!-- Business view: deduplicated labels -->
128+
<ul x-show="biz" class="space-y-1 text-sm">
129+
{% set seen_biz = [] %}
130+
{% for secret in project.secrets %}
131+
{% set label_biz = translate_secret(secret) %}
132+
{% if label_biz not in seen_biz %}
133+
{% if seen_biz.append(label_biz) %}{% endif %}
134+
<li class="flex items-center gap-2">
135+
<span class="w-2 h-2 rounded-full bg-yellow-500 inline-block"></span>
136+
{{ label_biz }}
137+
</li>
138+
{% endif %}
139+
{% endfor %}
140+
</ul>
141+
<!-- Technical view: all raw env var names -->
142+
<ul x-show="!biz" class="space-y-1 text-sm">
128143
{% for secret in project.secrets %}
129144
<li class="flex items-center gap-2">
130145
<span class="w-2 h-2 rounded-full bg-yellow-500 inline-block"></span>
131-
<span x-show="biz">{{ translate_secret(secret) }}</span>
132-
<span x-show="!biz" class="font-mono">{{ secret }}</span>
146+
<span class="font-mono">{{ secret }}</span>
133147
</li>
134148
{% endfor %}
135149
</ul>

visualpy/templates/partials/script_card.html

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,22 @@
2222
{% if script.summary %}
2323
<p class="text-xs text-gray-500 dark:text-gray-400 mb-1.5 line-clamp-2">{{ script.summary }}</p>
2424
{% endif %}
25+
<!-- Health indicator (business view only) -->
26+
{% set health = compute_health(script) %}
27+
{% if health.findings %}
28+
<div x-show="biz" class="flex items-center gap-1.5 mb-1.5">
29+
{% if health.color == "red" %}
30+
<span class="w-2 h-2 rounded-full bg-red-500 shrink-0"></span>
31+
<span class="text-xs text-red-600 dark:text-red-400">{{ health.findings|length }} issue{{ "s" if health.findings|length != 1 }}</span>
32+
{% elif health.color == "amber" %}
33+
<span class="w-2 h-2 rounded-full bg-amber-500 shrink-0"></span>
34+
<span class="text-xs text-amber-600 dark:text-amber-400">{{ health.findings|length }} issue{{ "s" if health.findings|length != 1 }}</span>
35+
{% elif health.color == "yellow" %}
36+
<span class="w-2 h-2 rounded-full bg-yellow-500 shrink-0"></span>
37+
<span class="text-xs text-yellow-600 dark:text-yellow-400">{{ health.findings|length }} issue{{ "s" if health.findings|length != 1 }}</span>
38+
{% endif %}
39+
</div>
40+
{% endif %}
2541
<!-- Business view: phase summary -->
2642
<div x-show="biz" class="flex flex-wrap gap-1.5 text-xs text-gray-500 dark:text-gray-400">
2743
<span>{{ script.steps|length }} steps</span>

0 commit comments

Comments
 (0)