mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
adf8fed9bd
## Description `CodeAwareCompressor` (AST-based code compression, `headroom/transforms/code_compressor.py`) had two bugs in its structure-reassembly path, found while investigating a reported Go brace-duplication issue (the Go bug itself — `statement_list` row-range swallowing a block's closing brace — was already fixed on `main` in #1668; this PR fixes what was *actually* still broken): 1. **TS/JS `export` keyword duplication.** `export function foo() {}` / `export class Foo {}` compressed to `export export function foo() {}` — invalid syntax, silently discarded by `_verify_syntax`'s fallback (the caller never sees an error, compression just quietly no-ops). Root cause: `_compress_function_ast` / `_compress_class_ast` slice a node's source by **line**, not by byte offset, deliberately — to preserve leading indentation for definitions nested inside classes. But when a node shares its *first* line with a preceding sibling (the `export` keyword is a sibling of the function inside tree-sitter's `export_statement` node, not part of the function node itself), that line-based slice pulled the sibling's text in too. The `export_statement` handler then re-prepended the same `export` text on top, producing the duplicate. 2. **Doc-comment displacement (all languages).** A `/** ... */` or `//` doc comment directly above a top-level function/class/type got detached from its declaration during AST extraction and re-emitted in one cluster at the very end of the compressed output, instead of staying attached to what it documents. Root cause: doc comments are top-level *siblings* of the declaration they document, not children of it — the extractor didn't attach them to anything, so they fell through to a "leftover top-level code" bucket that gets flushed as a single block after all functions. Also tightens `test_actual_go_compression`, which — per its own comment — was written to *tolerate* the Go bug (`compression_ratio may be 1.0 if compression produces invalid syntax`) rather than catch it. Since the underlying Go bug is already fixed on `main`, this now asserts real compression (`compression_ratio < 1.0`), matching its JS/Python siblings. Closes #1905 ## 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 Two commits: the fix itself, then the tests that prove it — bisectable independently, both pass the full suite on their own. **Commit 1 — `fix(code):`** - `headroom/transforms/code_compressor.py`: add `_get_node_lines()` — line-based node slicing that still preserves indentation, but trims a preceding sibling's text from the first line when that prefix isn't pure whitespace (i.e. an `export` keyword sharing the line), so callers that re-add the sibling text themselves don't get a duplicate; used by `_compress_function_ast` and `_compress_class_ast`. - `headroom/transforms/code_compressor.py`: add `_get_leading_comment_text()` — walks a node's `prev_sibling` chain to collect contiguous doc-comment nodes immediately above it (no blank line in between) and returns them for the caller to prepend, also marking their byte ranges as captured so they aren't independently swept into the leftover top-level-code bucket; wired into every capture branch in `_extract_structure` (package, import, export statement, decorator, function, class, type). - `CHANGELOG.md`: added an entry under `### Fixed`. **Commit 2 — `test(code):`** - `tests/test_transforms/test_code_compressor.py`: `test_actual_go_compression` now asserts `compression_ratio < 1.0` instead of tolerating a 1.0 fallback. - `tests/test_code_aware_brace_comment_regressions.py` (new): 4 regression tests — TS `export` not duplicated + valid syntax, TS doc comments stay attached, Go doc comments stay attached, and a real-TS-compression parity test matching the existing JS/Python/Go "actual compression" tests. ## 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 $ ruff check headroom/transforms/code_compressor.py tests/test_transforms/test_code_compressor.py tests/test_code_aware_brace_comment_regressions.py All checks passed! $ ruff format --check headroom/transforms/code_compressor.py tests/test_transforms/test_code_compressor.py tests/test_code_aware_brace_comment_regressions.py 3 files already formatted $ pytest tests/test_transforms/test_code_compressor.py tests/test_code_aware_regressions.py tests/test_code_aware_brace_comment_regressions.py -q 83 passed in 6.23s $ pytest -q # full suite 7912 passed, 5 failed, 442 skipped in 417.43s (0:06:57) # The 5 failures are pre-existing and unrelated: confirmed to fail identically # with this PR's changes stashed out (clean upstream/main checkout). # - test_wrap_marker_is_stale_when_pid_reused (PID-reuse detection, env-specific) # - test_read_cached_oauth_token_falls_back_to_gh_cli (leaks real local `gh` credentials) # - test_rtk_reader_returns_none_on_nonzero_exit / test_lean_ctx_reader_returns_none_on_failure_and_logs # (pass in isolation; fail only in full-suite order — pre-existing test-pollution, unrelated to code_compressor.py) # - test_parser_usable_in_thread_pool (test itself passes a str to parser.parse(), # which tree-sitter's binding has always required as bytes — a pre-existing test # bug unrelated to this change; separate fix in progress on another branch) $ mypy headroom Success: no issues found in 408 source files ``` ## Real Behavior Proof - Environment: macOS (Darwin 24.6.0), Python 3.14.5, headroom-ai dev checkout built via `uv sync --extra dev` + `maturin develop -m crates/headroom-py/Cargo.toml` (real `headroom._core` build, not mocked), `tree-sitter==0.25.2` / `tree-sitter-language-pack` per the pinned `[code]` extra. - Exact command / steps: ran `CodeAwareCompressor(CodeCompressorConfig(min_tokens_for_compression=10, enable_ccr=False)).compress(open("sdk/typescript/src/client.ts").read(), language="typescript")` identically against `git stash`-ed (pre-fix) and current (post-fix) trees; full snippet and additional samples below. - Observed result: `client.ts` (real 20KB SDK file in this repo) went from `compression_ratio=1.0` with a silent fallback (`export export class HeadroomClient` in the raw AST attempt, invalid syntax) to `compression_ratio=0.942`, `syntax_valid=True` — real compression, no duplication; full before/after table below. - Not tested: real-world repos beyond this repo's own SDK sample and the bundled benchmark fixture — broader corpus testing may follow as a comment on this PR. **Exact command, full snippet:** ```python from headroom.transforms.code_compressor import CodeAwareCompressor, CodeCompressorConfig compressor = CodeAwareCompressor(CodeCompressorConfig(min_tokens_for_compression=10, enable_ccr=False)) with open("sdk/typescript/src/client.ts") as f: code = f.read() result = compressor.compress(code, language="typescript") ``` **Observed result, before vs. after, real code:** | Sample | Before (main) | After (this fix) | |---|---|---| | `sdk/typescript/src/client.ts` (real 20KB SDK file, this repo) | `compression_ratio=1.0`, silent fallback — `export export class HeadroomClient` in the raw AST attempt, invalid syntax | `compression_ratio=0.942`, `syntax_valid=True` — real compression, no duplication | | TS fixture exercising both bugs (exported fn/class + doc comments) | `compression_ratio=1.0`, silent fallback | `compression_ratio=0.993`, `syntax_valid=True` | | `middleware/ratelimit.go` (bundled benchmark sample) | `compression_ratio=0.862`, `syntax_valid=True` — unaffected (Go bug already fixed on `main` by #1668) | `compression_ratio=0.862`, `syntax_valid=True` — unchanged, confirms no regression | | `generate_go_code(3)` (existing test fixture) | `compression_ratio=0.498` | `compression_ratio=0.498` — unchanged, confirms no regression | On code shaped to actually exercise elision (function bodies long enough to exceed `max_body_lines=5`), TypeScript compresses in line with other languages once the correctness bug stops blocking it entirely: | Language | Compression savings (synthetic fixture, ~10-line function bodies) | |---|---| | Python | 64.4% | | Go | 52.3% | | TypeScript | 49.0% | | JavaScript | 42.8% | (`client.ts`'s real-world 5.8% savings is lower than the synthetic TypeScript number above because most of its methods are ≤5 lines — under the elision threshold regardless of language — not because of a language-specific limitation.) ## 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 - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes The Go brace-duplication bug that motivated this investigation was already fixed on `main` (#1668, merged before this branch was based) — confirmed via the minimal repro and `ratelimit.go`, both compress cleanly with no duplicated braces. This PR fixes what was still actually broken: the TS/JS `export`-duplication bug and the doc-comment displacement bug (both present across languages), found empirically while verifying the original bug report against the current `main`. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: JD Davis <mxjerrett@gmail.com> |
||
|---|---|---|
| .. | ||
| __init__.py | ||
| test_code_compressor.py | ||
| test_code_compressor_cjk.py | ||
| test_content_router.py | ||
| test_detect_fallback_1123.py | ||
| test_diff_compressor.py | ||
| test_diff_compressor_rust_parity.py | ||
| test_html_extractor.py | ||
| test_kompress_compressor.py | ||
| test_kompress_deadline.py | ||
| test_kompress_size_gate.py | ||
| test_ort_dylib.py | ||
| test_pipeline_waste_signal_limit.py | ||
| test_read_lifecycle.py | ||
| test_smart_crusher_attribution.py | ||
| test_smart_crusher_audit_safe.py | ||
| test_smart_crusher_bugs.py | ||
| test_smart_crusher_ccr_retrieve_exemption.py | ||
| test_smart_crusher_ccr_roundtrip.py | ||
| test_smart_crusher_lossless_default.py | ||
| test_smart_crusher_rust_parity.py | ||
| test_tag_protector.py | ||
| test_text_crusher.py | ||
| test_text_crusher_parity.py | ||
| test_text_crusher_routing.py | ||
| test_tree_sitter_thread_safety.py | ||