peter
5f872b4b32
#015 — deeply nested JSON raised RecursionError (a RuntimeError, not ValueError) and became an unhandled 500 on two paths: - json-kind content validation now catches RecursionError -> 400. - Added a custom DRF EXCEPTION_HANDLER (artifacts/exception_handlers.py, wired via REST_FRAMEWORK) mapping otherwise-unhandled RecursionError -> 400, covering the parser-level crash on deeply nested request bodies (metadata etc.) too. #016 — wrap_csv_viewer handed Grid.js rows without reconciling width to the header, so over-long rows silently dropped cells. Now normalizes every row to the max width: long rows widen the columns (auto-named "Column N"), short rows pad with "". No cell is dropped. Both verified live; 4 regression tests added. 106 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
94 lines
3.6 KiB
Markdown
94 lines
3.6 KiB
Markdown
# 016 — Ragged CSV rows silently lose data in the rendered table (server doesn't normalize row width)
|
|
|
|
**Status:** Fixed
|
|
**Severity:** Low-Medium — silent data loss in the primary rendered view;
|
|
raw sidecar is unaffected, so it's a fidelity/display bug, not permanent
|
|
loss.
|
|
|
|
Round-3 probe of the `csv` kind (ragged rows, from the #013 handover).
|
|
Confirmed with an actual headless-Chrome render, not just server inspection.
|
|
|
|
## Description
|
|
|
|
`wrap_csv_viewer` (`artifacts/views.py`) parses CSV server-side and hands
|
|
Grid.js `{columns: rows[0], data: rows[1:]}` **without reconciling each
|
|
data row's width against the header**. Grid.js keys body cells by
|
|
column-id, so any row that doesn't match the header column count renders
|
|
wrong:
|
|
|
|
- **Row longer than header → extra cells silently dropped.**
|
|
- **Row shorter than header → missing cells rendered blank.**
|
|
|
|
Daybook's stated principle is "the server owns normalization," but row
|
|
width isn't normalized here, so the display silently disagrees with the
|
|
data.
|
|
|
|
## Reproduction (verified via headless Chrome render)
|
|
|
|
Posted `csv` content:
|
|
|
|
```
|
|
a,b,c
|
|
1,2,3,4,5
|
|
6,7
|
|
```
|
|
|
|
Server hands Grid.js: `{"columns": ["a","b","c"], "data": [["1","2","3","4","5"], ["6","7"]]}`
|
|
|
|
Rendered DOM (headless Chrome, JS executed):
|
|
|
|
```
|
|
| a | b | c |
|
|
| 1 | 2 | 3 | <- values 4 and 5 are GONE, not shown anywhere
|
|
| 6 | 7 | | <- third cell blank
|
|
```
|
|
|
|
The `4` and `5` from the first data row do not appear in the rendered
|
|
table at all. `GET /a/{id}?raw=1` still returns the original CSV intact, so
|
|
the data isn't destroyed — but a user browsing the artifact (the normal
|
|
path, and the shared-link path) sees fewer values than the CSV contains,
|
|
with no truncation indicator.
|
|
|
|
This is realistic input: trailing commas, inconsistent exports, and
|
|
hand-edited CSVs routinely produce rows that don't all match the header
|
|
width.
|
|
|
|
## Other CSV edge cases tested (all fine)
|
|
|
|
For the record — these were probed in the same pass and behaved correctly:
|
|
|
|
- Embedded commas, quotes (`""` escaping), and newlines inside quoted
|
|
fields: parsed correctly (Python's `csv` module).
|
|
- `duplicate columns` (`id,id,id`): rendered (3 same-named headers) — a
|
|
bit odd for sorting but not a data bug.
|
|
- `header only`: empty table, fine.
|
|
- `empty content`: rejected `400`, consistent with `json`.
|
|
- `single giant cell` (20 KB): rendered fine.
|
|
- `just commas`: empty-named columns, rendered.
|
|
|
|
So the parsing is robust; the gap is specifically row-width normalization
|
|
before the data reaches Grid.js.
|
|
|
|
## Suggested Fix
|
|
|
|
In `wrap_csv_viewer`, normalize each data row to `len(columns)` before
|
|
serializing — pad short rows with `""`, and for over-long rows either (a)
|
|
truncate but surface it, or better (b) widen `columns` to the maximum row
|
|
width (auto-naming the extra columns, e.g. `column_4`, `column_5`) so no
|
|
cell is ever dropped. Option (b) preserves all data, which matches the
|
|
"server owns normalization, never silently lose content" spirit. Whichever
|
|
is chosen, the rendered table should never contain fewer values than the
|
|
raw sidecar.
|
|
|
|
## Resolution
|
|
|
|
Implemented option (b) in `wrap_csv_viewer`: normalize every row to the
|
|
maximum row width before handing data to Grid.js. Rows longer than the
|
|
header widen the column set (extra columns auto-named `Column N`); rows
|
|
shorter than the width are padded with empty strings. No cell is ever
|
|
dropped, matching "server owns normalization, never silently lose content."
|
|
|
|
Verified live with the issue's exact input (`a,b,c` / `1,2,3,4,5` / `6,7`):
|
|
grid data becomes `columns=[a,b,c,Column 4,Column 5]`,
|
|
`data=[[1,2,3,4,5],[6,7,"","",""]]` — the previously-dropped `4` and `5`
|
|
now render. Test added in `tests/api/test_artifacts.py`.
|