Skip to content

Commit e525111

Browse files
paddymulclaude
andcommitted
docs(csv-import): ADR — record final review outcomes (decimal->float #150, non-nullable no-op, tz gotchas punted)
D8: decimal maps to float64 (exact decimal deferred to #150); non-nullable is a no-op label. D9: timestamp tz/precision ships with two punted, un-guarded polars-reader gotchas (tz-attach-on-naive, ns->us truncation). D2: the echo is the existing schema return field, no separate field/nudge. D4 dialect vocabulary stays deferred. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent de1960b commit e525111

1 file changed

Lines changed: 26 additions & 14 deletions

File tree

plans/adr-intelligent-csv-import.md

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ parse validates the inferred types against the whole file (a row that violates
9797
aborts → escalation, D6) and the typed parquet is then durable, a no-schema
9898
success is deterministic for that file.
9999

100+
The echo is the build result's existing `schema` field — a no-schema call's result
101+
schema *is* the inferred schema — so there is no separate `inferred_schema` field or
102+
"pin this" nudge; a structured suggestion rides with the #5 UI followon (D7).
103+
100104
### D3 — #141: a schema spec that is total, name-bound by default, position-bound on request
101105

102106
`schema` accepts three shapes, routed by Python type (the universal
@@ -196,22 +200,29 @@ errors). **Alternative — a structured `csv_suggestion` field on the MCP return
196200
(machine-readable, UI-renderable) — is deferred to the #5 UI followon; it needs
197201
`BuildError` to carry a structured attribute that `_run_and_record` surfaces.
198202

199-
### D8 — #144: cover `time`, `decimal`, and non-nullable ibis types
203+
### D8 — #144: cover `time`, non-nullable, and `decimal` (as float) ibis types
200204

201-
Extend `_IBIS_TO_POLARS` / `_polars_overrides` so a valid ibis schema never
202-
spuriously raises: map `time``pl.Time`, `decimal(p, s)``pl.Decimal(p, s)`
203-
(parameterised, not a fixed precision), and strip ibis nullability (`!`-suffixed
205+
`_polars_overrides` inspects the ibis dtype object so a valid schema never
206+
spuriously raises: map `time``pl.Time`, and strip ibis nullability (`!`-suffixed
204207
types like `int64` non-null) before lookup — polars CSV columns are nullable
205-
regardless, and a non-nullable *intent* is not a CSV parse concern. An unmapped
206-
type still raises the loud, listed error (the escape hatch stays honest).
208+
regardless, so a non-nullable *intent* is a **no-op label** at the parse layer
209+
(constraint validation is a future UI-guided concern, not raised here). `decimal`
210+
maps to **`pl.Float64`**, not an exact `pl.Decimal`: exact-decimal parsing is
211+
deferred to a UI-driven buckaroo autoclean step (**#150**). An unmapped type still
212+
raises the loud, listed error (the escape hatch stays honest).
207213

208-
### D9 — #145: preserve timestamp tz and precision
214+
### D9 — #145: preserve timestamp tz and precision (with two punted gotchas)
209215

210216
Map `timestamp``pl.Datetime("us")` (naive) but `timestamp(scale)` → the
211-
matching `pl.Datetime` precision (`s`/`ms`→"ms" where representable, `us`, `ns`),
212-
and `timestamp('UTC')` / tz-aware → `pl.Datetime(time_unit, time_zone=tz)`,
213-
instead of flattening everything to naive `pl.Datetime`. Round-trips the tz and
214-
sub-µs precision the ibis type declares.
217+
matching `pl.Datetime` precision (scale `≤3`→"ms", `≤6`→"us", else "ns"), and
218+
`timestamp('UTC')` / tz-aware → `pl.Datetime(time_unit, time_zone=tz)`, instead of
219+
flattening everything to naive `pl.Datetime`. Two polars-CSV-reader behaviours are
220+
**known and punted** (verified empirically, not guarded — the behaviour is
221+
defensible and a guard would mean sniffing the data): (a) a tz-aware override on
222+
*offset-less naive* text **attaches** the tz to the wall-clock rather than
223+
converting (wrong only if naive-local data is mislabelled with a tz); (b) `ns`
224+
precision is silently truncated to `µs` by `scan_csv` (the `ns` dtype label is kept
225+
but sub-µs digits are dropped). Bad values still raise loudly, never null.
215226

216227
## Alternatives considered
217228

@@ -236,9 +247,10 @@ sub-µs precision the ibis type declares.
236247

237248
## Consequences / open questions
238249

239-
- **#144/#145 defaults (D8/D9) were not grilled** — they surfaced only in the full
240-
review list. The mappings above are defensible defaults; revisit if a different
241-
tz/precision or decimal policy is wanted.
250+
- **#144/#145 were reviewed after the fact** (they surfaced only in the full review
251+
list). Outcomes: `decimal``float64` with exact-decimal deferred to UI (#150);
252+
non-nullable left a no-op label (constraint validation is UI-guided); the two
253+
timestamp tz/precision gotchas (D9) punted, shipped un-guarded.
242254
- **#142 ragged is deferred** to the holistic edge-case pass that ports the
243255
pandas/polars/duckdb pathology suites onto tallyman. The empirical detection
244256
finding (polars cannot raise on short rows; `pyarrow.csv` is the fail-loud,

0 commit comments

Comments
 (0)