mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(transforms): pass through ragged tables instead of misaligning columns (#1713)
## Description
Issue #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 #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` (9fbd47ba), 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>
This commit is contained in:
parent
0dd24ecfb5
commit
c7665ca088
2 changed files with 55 additions and 0 deletions
|
|
@ -134,6 +134,13 @@ def parse_tabular(
|
|||
|
||||
if not headers or not rows:
|
||||
return None
|
||||
# Ragged tables (rows whose cell count differs from the header count)
|
||||
# can't be zipped into records without shifting values under the wrong
|
||||
# column — a compressed table must never state facts the original
|
||||
# didn't (#1652). Treat them as non-tabular and pass through verbatim.
|
||||
width = len(headers)
|
||||
if any(len(row) != width for row in rows):
|
||||
return None
|
||||
return headers, rows, fmt
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -153,6 +153,54 @@ def test_parse_markdown_table_drops_separator() -> None:
|
|||
assert all("---" not in cell for row in rows for cell in row)
|
||||
|
||||
|
||||
def test_parse_tabular_rejects_ragged_fixed_width(monkeypatch) -> None:
|
||||
# Rows with differing cell counts can't be zipped under the headers
|
||||
# without misattributing columns (#1652) — must pass through.
|
||||
import headroom.transforms.tabular_ingest as ti
|
||||
|
||||
monkeypatch.setattr(
|
||||
ti,
|
||||
"detect_content_type",
|
||||
lambda _c: DetectionResult(ContentType.TABULAR, 0.9, {"format": "fixed_width"}),
|
||||
)
|
||||
ragged = (
|
||||
"tool installed latest status\n"
|
||||
"rtk 0.42.4 0.43.0 update available\n"
|
||||
"rtk ✓ 0.42.4 0.42.4 - up-to-date"
|
||||
)
|
||||
assert ti.parse_tabular(ragged) is None
|
||||
|
||||
|
||||
def test_parse_tabular_rejects_ragged_markdown(monkeypatch) -> None:
|
||||
import headroom.transforms.tabular_ingest as ti
|
||||
|
||||
monkeypatch.setattr(
|
||||
ti,
|
||||
"detect_content_type",
|
||||
lambda _c: DetectionResult(ContentType.TABULAR, 0.9, {"format": "markdown"}),
|
||||
)
|
||||
ragged = "| a | b | c |\n| --- | --- | --- |\n| 1 | 2 | 3 |\n| 4 | 5 |"
|
||||
assert ti.parse_tabular(ragged) is None
|
||||
|
||||
|
||||
def test_compress_passes_through_ragged_table(monkeypatch) -> None:
|
||||
import headroom.transforms.tabular_ingest as ti
|
||||
|
||||
monkeypatch.setattr(
|
||||
ti,
|
||||
"detect_content_type",
|
||||
lambda _c: DetectionResult(ContentType.TABULAR, 0.9, {"format": "fixed_width"}),
|
||||
)
|
||||
ragged = (
|
||||
"tool installed latest status\n"
|
||||
"rtk 0.42.4 0.43.0 update available\n"
|
||||
"rtk ✓ 0.42.4 0.42.4 - up-to-date"
|
||||
)
|
||||
result = TabularCompressor().compress(ragged)
|
||||
assert not result.was_modified
|
||||
assert result.compressed == ragged
|
||||
|
||||
|
||||
def test_parse_tabular_returns_none_for_non_tabular() -> None:
|
||||
assert parse_tabular("just a normal paragraph here") is None
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue