Skip to content

Commit 16ed73b

Browse files
agaonkerclaude
andauthored
fix(compression): keep container bodies compressible in code handler (#890)
## Description Two bugs in `CodeStructureHandler`'s tree-sitter path. (1) Container nodes (class/impl/trait/decorated definitions) were marked structural over their full span and `_spans_to_mask` never un-marks, so every method body inside a class was preserved and compression silently no-opped at confidence 0.95. (2) Discovered while testing: `tree-sitter-language-pack >= 1.0` switched to a Rust binding (methods, not attributes; `parse(str)`), so the handler raised `TypeError` on every call and silently fell back to regex. Closes # <!-- compression-handler review --> ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - `headroom/compression/handlers/code_handler.py`: containers emit a signature-only span (start to body start); recursion gives nested functions their own signature/body split; decorated definitions emit no whole-node span. - `headroom/compression/handlers/code_handler.py`: small compat shim supporting both the classic attribute API and the new Rust-binding method API. - `tests/test_compression/test_code_handler.py`: new file (the handler had zero dedicated tests) covering class/decorated/impl body compressibility, regex fallback, and a preservation-ratio bound. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ pytest tests/test_compression/ -q 92 passed, 8 skipped ``` ## Real Behavior Proof - Environment: local macOS, Python 3.11, tree-sitter-language-pack 1.8.1 installed, branch `fix/code-container-bodies`. - Exact command / steps: `pytest tests/test_compression/ -q`. - Observed result: Class method bodies are now compressible (preservation ratio drops from ~1.0 to roughly the signature fraction); the tree-sitter path runs instead of falling back to regex. - Not tested: End-to-end through the live proxy pipeline. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A — library change. See Test Output. ## Additional Notes PR 3 of 7; branched fresh from main (independent of #887/#889). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent d6f0f0f commit 16ed73b

2 files changed

Lines changed: 272 additions & 21 deletions

File tree

headroom/compression/handlers/code_handler.py

Lines changed: 121 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,50 @@ def _get_parser(language: str) -> Any:
6363
return _tree_sitter_parsers[language]
6464

6565

66+
# tree-sitter API compatibility. tree-sitter-language-pack switched to a
67+
# Rust binding (>=1.0) where node accessors are METHODS (kind(),
68+
# start_byte(), child(i)) and parse() takes str; the classic pybind API
69+
# uses attributes (.type, .start_byte, .children) and parse(bytes).
70+
# Without this shim the tree-sitter path raises TypeError on modern
71+
# installs and silently falls back to regex.
72+
73+
74+
def _ts_parse(parser: Any, content: str) -> Any:
75+
try:
76+
return parser.parse(content.encode("utf-8"))
77+
except TypeError:
78+
return parser.parse(content)
79+
80+
81+
def _ts_root(tree: Any) -> Any:
82+
root = tree.root_node
83+
return root() if callable(root) else root
84+
85+
86+
def _ts_kind(node: Any) -> str:
87+
kind = getattr(node, "type", None)
88+
if isinstance(kind, str):
89+
return kind
90+
return str(node.kind())
91+
92+
93+
def _ts_start_byte(node: Any) -> int:
94+
start = node.start_byte
95+
return int(start()) if callable(start) else int(start)
96+
97+
98+
def _ts_end_byte(node: Any) -> int:
99+
end = node.end_byte
100+
return int(end()) if callable(end) else int(end)
101+
102+
103+
def _ts_children(node: Any) -> list[Any]:
104+
children = getattr(node, "children", None)
105+
if children is not None and not callable(children):
106+
return list(children)
107+
return [node.child(i) for i in range(node.child_count())]
108+
109+
66110
class CodeLanguage(Enum):
67111
"""Supported programming languages."""
68112

@@ -175,6 +219,23 @@ class CodeSpan:
175219
],
176220
}
177221

222+
# Body child node types for container definitions (classes, impls,
223+
# traits). A container's span up to its body is structural (the
224+
# signature); the body itself is NOT marked — recursion into the body
225+
# emits signature spans for nested functions/methods, leaving their
226+
# bodies compressible.
227+
_CONTAINER_BODY_TYPES: frozenset[str] = frozenset(
228+
{
229+
"block", # python class body
230+
"statement_block", # js/ts
231+
"compound_statement", # c/cpp
232+
"class_body", # js/ts/java class body
233+
"interface_body", # java/ts interface body
234+
"declaration_list", # rust impl/trait body
235+
"enum_body", # java enum body
236+
}
237+
)
238+
178239
# Import patterns for fallback
179240
_IMPORT_PATTERNS: dict[str, re.Pattern[str]] = {
180241
"python": re.compile(r"^\s*(import\s+\w+|from\s+\w+\s+import)", re.MULTILINE),
@@ -301,42 +362,43 @@ def _extract_with_tree_sitter(
301362
HandlerResult with mask.
302363
"""
303364
parser = _get_parser(language)
304-
tree = parser.parse(content.encode("utf-8"))
365+
tree = _ts_parse(parser, content)
305366

306367
# Collect structural spans
307368
spans: list[CodeSpan] = []
308369

309370
def visit_node(node: Any, depth: int = 0) -> None:
310371
"""Visit AST node and collect structural spans."""
311-
node_type = node.type
372+
node_type = _ts_kind(node)
312373
structural_types = _STRUCTURAL_NODE_TYPES.get(language, set())
374+
children = _ts_children(node)
313375

314376
# Check if this is a structural node type
315377
if node_type in structural_types:
316378
# For functions, only the signature is structural
317379
if "function" in node_type or "method" in node_type:
318380
# Find the body node and exclude it
319381
body_node = None
320-
for child in node.children:
321-
if child.type in ("block", "statement_block", "compound_statement"):
382+
for child in children:
383+
if _ts_kind(child) in ("block", "statement_block", "compound_statement"):
322384
body_node = child
323385
break
324386

325387
if body_node:
326388
# Signature is from start to body start
327389
spans.append(
328390
CodeSpan(
329-
start=node.start_byte,
330-
end=body_node.start_byte,
391+
start=_ts_start_byte(node),
392+
end=_ts_start_byte(body_node),
331393
role="signature",
332394
is_structural=True,
333395
)
334396
)
335397
# Body is compressible
336398
spans.append(
337399
CodeSpan(
338-
start=body_node.start_byte,
339-
end=body_node.end_byte,
400+
start=_ts_start_byte(body_node),
401+
end=_ts_end_byte(body_node),
340402
role="body",
341403
is_structural=False,
342404
)
@@ -345,37 +407,75 @@ def visit_node(node: Any, depth: int = 0) -> None:
345407
# No body found, preserve whole thing
346408
spans.append(
347409
CodeSpan(
348-
start=node.start_byte,
349-
end=node.end_byte,
410+
start=_ts_start_byte(node),
411+
end=_ts_end_byte(node),
350412
role=node_type,
351413
is_structural=True,
352414
)
353415
)
416+
elif node_type == "decorated_definition":
417+
# Wrapper around decorator(s) + definition. Emit no
418+
# span: recursion marks the decorators and gives the
419+
# inner function its signature/body split. A whole-
420+
# node span here would preserve the function body.
421+
pass
354422
else:
355-
# Non-function structural nodes
356-
spans.append(
357-
CodeSpan(
358-
start=node.start_byte,
359-
end=node.end_byte,
360-
role=node_type,
361-
is_structural=True,
423+
# Container definitions (class, impl, trait): the
424+
# signature runs to the body start; the body is NOT
425+
# marked, so nested function bodies stay compressible
426+
# (recursion emits their signature spans). Leaf
427+
# declarations (imports, type aliases, structs) have
428+
# no such body child and are preserved whole.
429+
body_node = None
430+
for child in children:
431+
if _ts_kind(child) in _CONTAINER_BODY_TYPES:
432+
body_node = child
433+
break
434+
435+
if body_node is not None:
436+
spans.append(
437+
CodeSpan(
438+
start=_ts_start_byte(node),
439+
end=_ts_start_byte(body_node),
440+
role="signature",
441+
is_structural=True,
442+
)
362443
)
444+
else:
445+
spans.append(
446+
CodeSpan(
447+
start=_ts_start_byte(node),
448+
end=_ts_end_byte(node),
449+
role=node_type,
450+
is_structural=True,
451+
)
452+
)
453+
elif node_type == "decorator":
454+
# Decorators are structural (preserved) on their own so
455+
# the decorated_definition wrapper doesn't need a span.
456+
spans.append(
457+
CodeSpan(
458+
start=_ts_start_byte(node),
459+
end=_ts_end_byte(node),
460+
role="decorator",
461+
is_structural=True,
363462
)
463+
)
364464
elif node_type == "comment" and self.preserve_comments:
365465
spans.append(
366466
CodeSpan(
367-
start=node.start_byte,
368-
end=node.end_byte,
467+
start=_ts_start_byte(node),
468+
end=_ts_end_byte(node),
369469
role="comment",
370470
is_structural=True,
371471
)
372472
)
373473

374474
# Recurse into children
375-
for child in node.children:
475+
for child in children:
376476
visit_node(child, depth + 1)
377477

378-
visit_node(tree.root_node)
478+
visit_node(_ts_root(tree))
379479

380480
# Build mask from spans
381481
mask = self._spans_to_mask(spans, len(content))
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
"""Tests for code structure handler."""
2+
3+
import pytest
4+
5+
from headroom.compression.handlers.code_handler import (
6+
CodeStructureHandler,
7+
is_tree_sitter_available,
8+
)
9+
10+
requires_tree_sitter = pytest.mark.skipif(
11+
not is_tree_sitter_available(),
12+
reason="tree-sitter-language-pack not installed",
13+
)
14+
15+
16+
class TestCanHandle:
17+
@pytest.fixture
18+
def handler(self):
19+
return CodeStructureHandler()
20+
21+
def test_detects_python(self, handler):
22+
assert handler.can_handle("def foo():\n pass\n") is True
23+
24+
def test_detects_javascript(self, handler):
25+
assert handler.can_handle("function foo() { return 1; }") is True
26+
27+
def test_rejects_prose(self, handler):
28+
assert handler.can_handle("This is a plain sentence.") is False
29+
30+
31+
class TestRegexFallback:
32+
"""Regex path runs regardless of tree-sitter availability."""
33+
34+
@pytest.fixture
35+
def handler(self):
36+
return CodeStructureHandler(use_tree_sitter=False)
37+
38+
def test_python_signature_preserved_body_compressible(self, handler):
39+
code = "def hello(name: str) -> str:\n message = name\n return message\n"
40+
result = handler.get_mask(code, language="python")
41+
42+
assert result.metadata["parser"] == "regex"
43+
sig = "def hello(name: str) -> str:"
44+
start = code.index(sig)
45+
assert all(result.mask.mask[i] for i in range(start, start + len(sig)))
46+
47+
body_char = code.index("message = name")
48+
assert result.mask.mask[body_char] is False
49+
50+
def test_python_import_preserved(self, handler):
51+
code = "import os\n\nx = 1\n"
52+
result = handler.get_mask(code, language="python")
53+
assert all(result.mask.mask[i] for i in range(len("import os")))
54+
55+
56+
@requires_tree_sitter
57+
class TestTreeSitterContainers:
58+
"""Container bodies must stay compressible (signature-only spans).
59+
60+
Regression: class_definition / decorated_definition / impl_item were
61+
marked structural over their FULL span, so every method body inside a
62+
class (i.e. most real code) was preserved and compression no-opped at
63+
confidence 0.95.
64+
"""
65+
66+
@pytest.fixture
67+
def handler(self):
68+
return CodeStructureHandler()
69+
70+
def test_class_method_bodies_compressible(self, handler):
71+
code = (
72+
"class Foo:\n"
73+
" def method_a(self):\n"
74+
" body_line_a = 1\n"
75+
" return body_line_a\n"
76+
"\n"
77+
" def method_b(self):\n"
78+
" body_line_b = 2\n"
79+
" return body_line_b\n"
80+
)
81+
result = handler.get_mask(code, language="python")
82+
assert result.metadata["parser"] == "tree-sitter"
83+
84+
# Class signature and method signatures preserved
85+
assert all(result.mask.mask[i] for i in range(len("class Foo:")))
86+
sig = "def method_a(self):"
87+
start = code.index(sig)
88+
assert all(result.mask.mask[i] for i in range(start, start + len(sig)))
89+
90+
# Method bodies compressible
91+
for body in ("body_line_a = 1", "body_line_b = 2"):
92+
start = code.index(body)
93+
assert not any(result.mask.mask[i] for i in range(start, start + len(body))), (
94+
f"method body {body!r} must be compressible"
95+
)
96+
97+
def test_decorated_function_body_compressible(self, handler):
98+
code = "@decorator\ndef decorated():\n body_line = 4\n return body_line\n"
99+
result = handler.get_mask(code, language="python")
100+
101+
# Decorator and signature preserved
102+
assert all(result.mask.mask[i] for i in range(len("@decorator")))
103+
sig = "def decorated():"
104+
start = code.index(sig)
105+
assert all(result.mask.mask[i] for i in range(start, start + len(sig)))
106+
107+
# Body compressible
108+
start = code.index("body_line = 4")
109+
assert not any(result.mask.mask[i] for i in range(start, start + len("body_line = 4"))), (
110+
"decorated function body must be compressible"
111+
)
112+
113+
def test_module_function_body_compressible(self, handler):
114+
code = "def standalone():\n body_line = 3\n return body_line\n"
115+
result = handler.get_mask(code, language="python")
116+
117+
start = code.index("body_line = 3")
118+
assert not any(result.mask.mask[i] for i in range(start, start + len("body_line = 3")))
119+
120+
def test_rust_impl_method_bodies_compressible(self, handler):
121+
code = (
122+
"struct Foo { x: i32 }\n"
123+
"impl Foo {\n"
124+
" fn method(&self) -> i32 {\n"
125+
" let body_line = 5;\n"
126+
" body_line\n"
127+
" }\n"
128+
"}\n"
129+
)
130+
result = handler.get_mask(code, language="rust")
131+
132+
# impl signature preserved
133+
start = code.index("impl Foo")
134+
assert all(result.mask.mask[i] for i in range(start, start + len("impl Foo")))
135+
136+
# method body compressible
137+
start = code.index("let body_line = 5;")
138+
assert not any(
139+
result.mask.mask[i] for i in range(start, start + len("let body_line = 5;"))
140+
), "impl method body must be compressible"
141+
142+
def test_preservation_ratio_sane_for_class_code(self, handler):
143+
"""A class with substantial method bodies should NOT preserve
144+
everything — the whole point of the handler."""
145+
body = "\n".join(f" line_{i} = {i}" for i in range(20))
146+
code = f"class Big:\n def method(self):\n{body}\n return 0\n"
147+
result = handler.get_mask(code, language="python")
148+
assert result.preservation_ratio < 0.5, (
149+
f"class code preserved {result.preservation_ratio:.0%} — "
150+
"container bodies are leaking into the structural mask"
151+
)

0 commit comments

Comments
 (0)