mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
2 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Parideboy
|
c7665ca088
|
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` (
|
||
|
Ashish
|
d789a7c528
|
feat(transforms): tabular + spreadsheet (.xlsx/.xls) compression (#1128)
## Description Adds a content-type-aware path for **tabular data** — CSV/TSV, markdown tables, fixed-width text, and binary `.xlsx`/`.xls` spreadsheets — by routing them through the existing, battle-tested `SmartCrusher` instead of letting them fall through to `PLAIN_TEXT → Kompress`. The pipeline already compressed tables losslessly when handed a JSON array of records. This wires up the missing front door: detect tabular text (and ingest binary spreadsheets), convert to JSON records, and reuse `SmartCrusher.crush()`. No new compression algorithm. Closes # ## Type of Change - [ ] Bug fix (non-breaking change that fixes an issue) - [x] 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 - **Detection** (`content_detector.py`): new `ContentType.TABULAR` + `_try_detect_tabular()` for CSV/TSV, markdown tables, and fixed-width columns. Ordered after search/log (which also look "delimited") and before code, with a prose-rejection guard so it never steals `file:line:content` search output, `key: value` logs, or sentences with incidental commas. Rust backend returns `plain_text` for unknown types and the router already falls back to the Python detector, so **no Rust change**. - **Bridge** (`tabular_ingest.py`): stdlib parsers + `to_records()` + a `TabularCompressor` that parses → JSON records → `SmartCrusher` (lossless `csv-schema` first; lossy row-drop with reversible `<<ccr:HASH>>` markers stays SmartCrusher's built-in fallback). Only adopts a result when it actually saves bytes. - **Spreadsheets** (`spreadsheet_ingest.py`): `.xlsx`/`.xls` → per-sheet CSV text at the SDK boundary. Optional deps (`pip install headroom-ai[spreadsheet]`) fail loudly with an install hint, never silently degrade. - **Routing** (`content_router.py`): `CompressionStrategy.TABULAR`, `enable_tabular_compressor` flag, lazy getter, apply branch, strategy maps, Kompress fallback eligibility. - **SDK** (`compress.py`): `compress_spreadsheet(path, ...)` helper (one message per sheet). - **Packaging** (`pyproject.toml`): new `[spreadsheet]` extra; `openpyxl` added to `[dev]` so the xlsx path is exercised in CI. - **Docs/demo**: `examples/tabular_compression_demo.py` + README entry. ### Design note: lossless-only Compact, all-unique tables with no query yield ~0 savings — this is correct, not a bug. SmartCrusher returns `skip:unique_entities_no_signal` and won't drop unique rows without a duplicate/relevance signal. Real wins come from verbose/redundant tables and query-driven selection. A pressure-driven lossy row sampler was considered and intentionally not added. ## 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 $ python -m pytest tests/test_transforms_tabular.py -q collected 20 items tests/test_transforms_tabular.py .................... [100%] ============================== 20 passed in 7.15s ============================== $ ruff check headroom/transforms/tabular_ingest.py headroom/transforms/spreadsheet_ingest.py All checks passed! $ mypy headroom/transforms/tabular_ingest.py headroom/transforms/spreadsheet_ingest.py Success: no issues found in 2 source files ``` `tests/test_transforms_tabular.py` (20 tests): detection true positives + no-misroute negatives (search/log/JSON/prose), parser units (incl. fixed-width), the CSV→SmartCrusher bridge, router routing + disable flag, and `.xlsx` ingestion (skipif openpyxl missing) + error paths. `spreadsheet_ingest` 100% / `tabular_ingest` 90% line coverage. ## Real Behavior Proof - **Environment:** local checkout of `feat/tabular-compression`, Python 3.x, `pip install -e ".[dev]"`. - **Exact command / steps:** `python examples/tabular_compression_demo.py` (no API key required). - **Observed result:** ```text === Raw tabular text (ContentRouter, char-level) === compact unique CSV strat=tabular chars 1306 -> 1072 ( 17.9% saved) redundant CSV strat=tabular chars 2661 -> 1350 ( 49.3% saved) verbose markdown strat=tabular chars 2019 -> 1580 ( 21.7% saved) === Full pipeline (real tokenizer) === redundant CSV tokens 768 -> 394 ( 48.7% saved) === Binary spreadsheet (.xlsx) === 2-sheet workbook tokens 1092 -> 683 ( 37.5% saved) ``` - **Not tested:** legacy `.xls` binary path (needs optional `xlrd` + binary fixture; `# pragma: no cover`); base64-embedded `.xlsx` inside multimodal blocks (out of scope, noted as a follow-up). ## 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 - [x] 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 ## Additional Notes - CHANGELOG/version are intentionally untouched: this repo uses **release-please**, which bumps the version and CHANGELOG via automated `chore: release main` PRs, not per-feature PRs. - The `.xls` path is `# pragma: no cover` (legacy, needs optional `xlrd` + a binary fixture). - Follow-up (out of scope): base64-embedded `.xlsx` inside tool-result/multimodal blocks; porting tabular parsers into the Rust core for parity. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |