Skip to content

Commit 96f9d22

Browse files
Wolfvinclaude
andcommitted
fix(formatters): render backend function names in markdown diff (closes #299)
_md_diff() read `fn` from backend node entries, but _diff_backend() renames the field to `name` on the way out, so every function name fell through to the empty-string default. The diff reported correct counts with blank identities: "- + `` (a.py)". Fixed on the consumer side — commands/diff.py, dashboard and MCP already consume `name`, so changing the producer would break them. Tests pin the contract against real _diff_backend() output, not a hand-written stand-in, and fail if the fix is reverted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 61d70a8 commit 96f9d22

2 files changed

Lines changed: 102 additions & 3 deletions

File tree

scripts/formatters/markdown.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,12 +1155,13 @@ def _md_diff(data: Dict, lines: list) -> None:
11551155
removed = be.get("removed_count", 0)
11561156
changed = be.get("changed_count", 0)
11571157
lines.append(f"### Backend — +{added} / -{removed} / ~{changed}")
1158+
# `name`, not `fn`: _diff_backend() renames the field on the way out.
11581159
for node in be.get("added_nodes", [])[:5]:
1159-
lines.append(f"- + `{node.get('fn', '')}` ({node.get('file', '')})")
1160+
lines.append(f"- + `{node.get('name', '')}` ({node.get('file', '')})")
11601161
for node in be.get("removed_nodes", [])[:5]:
1161-
lines.append(f"- - `{node.get('fn', '')}`")
1162+
lines.append(f"- - `{node.get('name', '')}`")
11621163
for node in be.get("changed_nodes", [])[:5]:
1163-
lines.append(f"- ~ `{node.get('fn', '')}`")
1164+
lines.append(f"- ~ `{node.get('name', '')}`")
11641165
if be.get("new_dead"):
11651166
lines.append(f"- **New dead:** {len(be['new_dead'])}")
11661167
lines.append("")

tests/test_markdown_diff.py

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
"""Tests for the markdown registry-diff renderer (issue #299).
2+
3+
_diff_backend() emits backend node entries keyed `name`, while _md_diff()
4+
read `fn` and silently rendered every function name as an empty string.
5+
These tests pin the producer/consumer contract on both sides.
6+
"""
7+
8+
import os
9+
import sys
10+
import unittest
11+
12+
SCRIPT_DIR = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "scripts")
13+
sys.path.insert(0, SCRIPT_DIR)
14+
15+
from diff_engine import _diff_backend # noqa: E402
16+
from formatters.markdown import to_markdown # noqa: E402
17+
18+
19+
def _render(backend, frontend=None):
20+
return to_markdown(
21+
{"status": "ok", "frontend": frontend or {}, "backend": backend}, "diff"
22+
)
23+
24+
25+
class TestBackendNodeNamesRendered(unittest.TestCase):
26+
"""Backend function names must reach the markdown output."""
27+
28+
def test_added_node_name_is_rendered(self):
29+
out = _render({
30+
"added_count": 1, "removed_count": 0, "changed_count": 0,
31+
"added_nodes": [{"name": "freshFn", "file": "a.py", "status": "active"}],
32+
"removed_nodes": [], "changed_nodes": [],
33+
})
34+
self.assertIn("freshFn", out)
35+
self.assertIn("a.py", out)
36+
self.assertNotIn("- + ``", out)
37+
38+
def test_removed_node_name_is_rendered(self):
39+
out = _render({
40+
"added_count": 0, "removed_count": 1, "changed_count": 0,
41+
"added_nodes": [], "changed_nodes": [],
42+
"removed_nodes": [{"name": "goneFn", "file": "a.py"}],
43+
})
44+
self.assertIn("goneFn", out)
45+
self.assertNotIn("- - ``", out)
46+
47+
def test_changed_node_name_is_rendered(self):
48+
out = _render({
49+
"added_count": 0, "removed_count": 0, "changed_count": 1,
50+
"added_nodes": [], "removed_nodes": [],
51+
"changed_nodes": [
52+
{"name": "movedFn", "file": "a.py",
53+
"ref_count": {"from": 2, "to": 1}}
54+
],
55+
})
56+
self.assertIn("movedFn", out)
57+
self.assertNotIn("- ~ ``", out)
58+
59+
60+
class TestContractWithProducer(unittest.TestCase):
61+
"""Render real _diff_backend() output, not a hand-written stand-in."""
62+
63+
def test_real_diff_backend_output_renders_names(self):
64+
old = {
65+
"nodes": [{"id": "a.py:1", "fn": "goneFn", "file": "a.py",
66+
"ref_count": 1, "status": "active"}],
67+
"edges": [],
68+
}
69+
new = {
70+
"nodes": [{"id": "a.py:9", "fn": "freshFn", "file": "a.py",
71+
"ref_count": 1, "status": "active"}],
72+
"edges": [],
73+
}
74+
75+
out = _render(_diff_backend(old, new))
76+
77+
self.assertIn("freshFn", out)
78+
self.assertIn("goneFn", out)
79+
80+
81+
class TestFrontendNotRegressed(unittest.TestCase):
82+
"""The frontend block already read `name` correctly — keep it that way."""
83+
84+
def test_frontend_class_names_still_rendered(self):
85+
out = _render(
86+
{},
87+
frontend={
88+
"added_count": 1, "removed_count": 0, "changed_count": 0,
89+
"added_classes": [{"name": "btn-primary", "status": "active"}],
90+
"removed_classes": [], "changed_classes": [],
91+
"added_ids": [], "removed_ids": [], "changed_ids": [],
92+
},
93+
)
94+
self.assertIn("btn-primary", out)
95+
96+
97+
if __name__ == "__main__":
98+
unittest.main()

0 commit comments

Comments
 (0)