Skip to content

Commit c7665ca

Browse files
Parideboyclaude
andauthored
fix(transforms): pass through ragged tables instead of misaligning columns (headroomlabs-ai#1713)
## Description Issue headroomlabs-ai#1652 reports the proxy's compression layer surfacing an "impossible mixed" status line — a row combining fields from two different rows of a version-status table (Docker row `0.42.4 → 0.43.0 update available` blended with WSL row `0.42.4 → 0.42.4 up-to-date`). The reporter's follow-up refined the claim: the stored canonical content was intact, but the compression path presents a lossier view that invites exactly this misattribution. There is a concrete mechanism for that in the tabular bridge: `parse_tabular` (`headroom/transforms/tabular_ingest.py`) hands parsed rows to `to_records`, which **silently pads/truncates every row to the header width**. For ragged tables — rows whose cell count differs from the header row, exactly what mixed-shape status tables like the reporter's produce (`✓` and `-` placeholder cells change the token count per row) — this shifts values under the wrong column before SmartCrusher compaction. The compressed output can then state column/value pairings the original never contained. Fix: `parse_tabular` now rejects ragged tables (any row width ≠ header width) and returns `None`, so the content passes through verbatim, per the issue's requirement that a lossy summary "must not create impossible mixed facts". Aligned tables compress exactly as before. The Rust `log_template` Drain miner was also examined; its template rendering only emits tokens that are constant across all rows of a run, so no defect was found there and it is left untouched. Fixes headroomlabs-ai#1652 ## 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/transforms/tabular_ingest.py`: `parse_tabular` returns `None` when any parsed row's cell count differs from the header count, instead of letting `to_records` pad/truncate rows into the wrong columns. `TabularCompressor.compress` then takes its existing pass-through branch (`was_modified=False`). - `tests/test_transforms_tabular.py`: three new tests — ragged fixed-width table rejected (reproducing the issue's rtk version-status shape), ragged markdown table rejected, and end-to-end `TabularCompressor.compress` pass-through of a ragged table. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [ ] Manual testing performed ### Test Output ```text $ python -m pytest tests/test_transforms_tabular.py -q 39 passed $ ruff check headroom/transforms/tabular_ingest.py tests/test_transforms_tabular.py All checks passed! $ ruff format --check headroom/transforms/tabular_ingest.py tests/test_transforms_tabular.py 2 files already formatted $ mypy headroom --ignore-missing-imports Success: no issues found (note-level messages only) ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.13, local checkout branched from `upstream/main` (9fbd47b), Rust core built locally. - Exact command / steps: constructed the issue's table shape (Docker row with 4 cells, WSL row with 6 cells under 4 headers) and ran it through `TabularCompressor().compress()` before and after the change; ran the full `tests/test_transforms_tabular.py` suite. - Observed result: before — `to_records` turned the WSL row into `{'tool': 'rtk', 'installed': '✓', 'latest': '0.42.4', 'status': '0.42.4'}`: the `up-to-date` status is dropped and a version number lands under `status` — precisely the misattributed-fact class from the issue. After — `parse_tabular` returns `None`, `compress` returns the original text unmodified (`was_modified=False`, byte-identical pass-through), and all 39 tests pass (36 pre-existing + 3 new). - Not tested: the reporter's exact end-to-end session (OMP → headroom proxy on 8787 → sticky-router on 4140); the Rust BuildOutput `log_template` path, which was reviewed and found to only emit run-constant tokens. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 0dd24ec commit c7665ca

2 files changed

Lines changed: 55 additions & 0 deletions

File tree

‎headroom/transforms/tabular_ingest.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,13 @@ def parse_tabular(
134134

135135
if not headers or not rows:
136136
return None
137+
# Ragged tables (rows whose cell count differs from the header count)
138+
# can't be zipped into records without shifting values under the wrong
139+
# column — a compressed table must never state facts the original
140+
# didn't (#1652). Treat them as non-tabular and pass through verbatim.
141+
width = len(headers)
142+
if any(len(row) != width for row in rows):
143+
return None
137144
return headers, rows, fmt
138145

139146

‎tests/test_transforms_tabular.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,54 @@ def test_parse_markdown_table_drops_separator() -> None:
153153
assert all("---" not in cell for row in rows for cell in row)
154154

155155

156+
def test_parse_tabular_rejects_ragged_fixed_width(monkeypatch) -> None:
157+
# Rows with differing cell counts can't be zipped under the headers
158+
# without misattributing columns (#1652) — must pass through.
159+
import headroom.transforms.tabular_ingest as ti
160+
161+
monkeypatch.setattr(
162+
ti,
163+
"detect_content_type",
164+
lambda _c: DetectionResult(ContentType.TABULAR, 0.9, {"format": "fixed_width"}),
165+
)
166+
ragged = (
167+
"tool installed latest status\n"
168+
"rtk 0.42.4 0.43.0 update available\n"
169+
"rtk ✓ 0.42.4 0.42.4 - up-to-date"
170+
)
171+
assert ti.parse_tabular(ragged) is None
172+
173+
174+
def test_parse_tabular_rejects_ragged_markdown(monkeypatch) -> None:
175+
import headroom.transforms.tabular_ingest as ti
176+
177+
monkeypatch.setattr(
178+
ti,
179+
"detect_content_type",
180+
lambda _c: DetectionResult(ContentType.TABULAR, 0.9, {"format": "markdown"}),
181+
)
182+
ragged = "| a | b | c |\n| --- | --- | --- |\n| 1 | 2 | 3 |\n| 4 | 5 |"
183+
assert ti.parse_tabular(ragged) is None
184+
185+
186+
def test_compress_passes_through_ragged_table(monkeypatch) -> None:
187+
import headroom.transforms.tabular_ingest as ti
188+
189+
monkeypatch.setattr(
190+
ti,
191+
"detect_content_type",
192+
lambda _c: DetectionResult(ContentType.TABULAR, 0.9, {"format": "fixed_width"}),
193+
)
194+
ragged = (
195+
"tool installed latest status\n"
196+
"rtk 0.42.4 0.43.0 update available\n"
197+
"rtk ✓ 0.42.4 0.42.4 - up-to-date"
198+
)
199+
result = TabularCompressor().compress(ragged)
200+
assert not result.was_modified
201+
assert result.compressed == ragged
202+
203+
156204
def test_parse_tabular_returns_none_for_non_tabular() -> None:
157205
assert parse_tabular("just a normal paragraph here") is None
158206

0 commit comments

Comments
 (0)