Skip to content

Commit 2f5be4c

Browse files
rubinderclaude
andcommitted
feat: pair renames by field ID, split widening from additive, record enum drift
Closes #1. The agent reported `checkNumber -> check_reference` as two findings -- a `breaking` drop and an `additive` add -- on the one table whose headline property is that a rename costs nothing because the field ID does not move. The evidence was already being fetched and thrown away: schema_history() returns field_ids, observe() read only ["columns"]. `build_rename_map` groups every historical column name by field ID, so a name gone from the latest schema whose ID is still present is a rename, not a drop. Transitive renames (a -> b -> c) resolve in one hop because the map keys on the ID rather than on consecutive pairs. A field ID absent from the latest schema stays a genuine drop and stays breaking. Measured at full scale, no ops.monitor_results history: all -> agent 0 findings (unchanged) all -> drift-demo -> agent 5 -> 4 findings, 2 incidents all -> monitor -> drift-demo -> agent 6 -> 5 findings, 3 incidents `renaming` is ACTIONABLE: no data moved, but the published contract now names a column that does not exist, and that needs a recorded decision. `widening` is not -- an int -> long and a brand-new column are both safe, but only one means the contract is behind on a column that already existed. Enum drift lands as `enum_watch`, a new contract key, NOT an expectation. Expectations are fail-closed gates and `_evaluate` raises on unknown types, which is what stops a typo'd check reading green forever; a watch that must never block would have needed a branch that always passes -- the vacuous check that design exists to prevent. baseType keeps its accepted_values gate and is deliberately not watched: one column, one semantics. A watch that reads no values reports breaking rather than passing, and one aimed at a >200-cardinality column reports itself misconfigured. A guard test in the first draft was vacuous. It supplied `renames` as a fixture, so it exercised detect() and never build_rename_map, and a planted position-matching mutant passed all 19 tests. A rename does not move a column, so ordinal matching gets plain renames right and breaks only when a column is deleted from the middle. Three tests now build the map from a {a:1,b:2,c:3} -> {a:1,c_new:3} history where position and field ID disagree; the position mutant and a name-similarity mutant both fail against them, as does a real Iceberg table with a delete and a rename in one step. Written up in docs/ai-sdlc/decisions/0006. Reviewed: 246 tests pass (224 before), ruff clean, both drift sequences re-run end to end at full scale and the README numbers replaced with what was measured. The settlementDays widening still reports as `additive` because drift-demo adds that column before widening it, so no narrower type was ever declared; stated in the README rather than papered over by editing the contract, which would make the clean run report a phantom drop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199ePw5w34FyfGAB41mkv3G
1 parent 8630a7c commit 2f5be4c

11 files changed

Lines changed: 896 additions & 51 deletions

README.md

Lines changed: 69 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ A complete data platform, built end to end, small enough to read:
3838
feature independently recomputed by a second implementation on every training
3939
row.
4040
- **An agentic ops layer** — LangGraph, rules-first, dry-run by default, offline.
41+
Schema changes are classified `breaking` / `renaming` / `widening` / `additive`
42+
/ `enum_drift`, with renames paired **by Iceberg field ID** so one rename is one
43+
finding rather than a phantom drop plus a phantom add.
44+
- **Enum drift** on 7 categorical columns — new values are recorded and reported,
45+
never used to block a build. Deliberately not the same mechanism as the
46+
fail-closed `accepted_values` gate on `baseType`.
4147
- **17 data-quality monitors + 8 arrival checks** with persisted metric history
4248
and robust (median/MAD) baselines. 10 of the 17 are declared in YAML; the
4349
other 7 are column-type checks generated per contract field.
@@ -185,12 +191,12 @@ To see the whole story in order:
185191

186192
```bash
187193
uv run make all && uv run make agent # clean: 0 findings
188-
uv run make drift-demo && uv run make agent # after drift: 5 findings, 2 incidents
194+
uv run make drift-demo && uv run make agent # after drift: 4 findings, 2 incidents
189195
```
190196

191197
Both transcripts below are from that exact sequence, on a warehouse with no
192198
`ops.monitor_results` history. Slot a `make monitor` in before `drift-demo` and
193-
the post-drift run reports **6** findings and 3 incidents, not 5 and 2: the
199+
the post-drift run reports **5** findings and 3 incidents, not 4 and 2: the
194200
persisted baseline of 250,000 added records lets `bronze_txn_row_count` breach
195201
as well, which the agent folds in as a `monitor_breach` finding on top of the
196202
volume anomaly its own sensor already found. That is the monitors working, not
@@ -438,7 +444,7 @@ sense ──▶ monitor ──▶ classify ──┬──▶ act ──▶ END
438444
|---|---|
439445
| `sense` | Reads Iceberg **metadata** — schema history, snapshot log, per-snapshot added-records, newest date. No data scan where metadata suffices. |
440446
| `monitor` | Runs the `src/ops` monitors and arrival SLAs; converts breaches into findings. |
441-
| `classify` | Severity per finding: `breaking` / `additive` / `benign`. **Rules first**; the optional Claude call handles only unmatched cases. |
447+
| `classify` | Severity per finding: `breaking` / `renaming` / `widening` / `additive` / `enum_drift` / `benign`. **Rules first**; the optional Claude call handles only unmatched cases. |
442448
| `act` | Writes `docs/incidents/<slug>.md` and would file a GitHub issue. **Dry-run by default.** |
443449

444450
**Classification is rules-first because CI must never depend on a model call**
@@ -453,28 +459,78 @@ $ uv run make agent # clean lakehouse
453459
agent: as_of=2026-06-30 findings=0
454460

455461
$ uv run make drift-demo && uv run make agent
456-
agent: as_of=2026-06-30 findings=5
457-
[breaking] column 'checkNumber' declared in contract but absent from table
458-
[additive] column 'check_reference' present in table but not declared in contract
462+
agent: as_of=2026-06-30 findings=4
463+
[renaming] column 'checkNumber' was renamed to 'check_reference' (field id 20); the contract still declares the old name
459464
[additive] column 'merchantCategoryCode' present in table but not declared in contract
460465
[additive] column 'settlementDays' present in table but not declared in contract
461466
[breaking] latest snapshot added 5 rows, below 50% of trailing median 250,000
462-
-> incident: bronze-yodlee_transactions_raw-checkNumber-3955ba01.md
463-
-> DRY-RUN: would run `gh issue create ...` -> [breaking] schema_drift on bronze.yodlee_transactions_raw
467+
-> incident: bronze-yodlee_transactions_raw-checkNumber-7b8d24a4.md
468+
-> DRY-RUN: would run `gh issue create ...` -> [renaming] schema_drift on bronze.yodlee_transactions_raw
464469
-> incident: bronze-yodlee_transactions_raw-volume_anomaly-f0497e07.md
465470
-> DRY-RUN: would run `gh issue create ...` -> [breaking] volume_anomaly on bronze.yodlee_transactions_raw
466471
agent: dry-run (pass --execute to file GitHub issues)
467472
```
468473

469-
Five findings, **two** incident files — breaking only. Incident slugs are
474+
Four findings, **two** incident files — actionable only. Incident slugs are
470475
sha256-derived and therefore stable across processes, so re-running rewrites the
471476
same two files instead of accumulating duplicates. Committed examples are in
472477
[`docs/incidents/`](docs/incidents/).
473478

474-
The rename classified as `breaking` is the right answer, not noise: Silver reads
475-
Bronze by name and the contract declares columns by name, so a rename *is* a drop
476-
to every consumer that selects it, even though Iceberg lost nothing. Storage
477-
fine, consumers broken — exactly what a human should review.
479+
### The rename used to be two findings, and that was the bug
480+
481+
An earlier version of this agent reported the rename as **five** findings, not
482+
four:
483+
484+
```
485+
[breaking] column 'checkNumber' declared in contract but absent from table
486+
[additive] column 'check_reference' present in table but not declared in contract
487+
```
488+
489+
The reasoning behind `breaking` was not wrong as far as it went — Silver reads
490+
Bronze by name and the contract declares columns by name, so to a by-name
491+
consumer a rename really does read as a disappearance. Storage fine, consumers
492+
broken, worth a human's time. All of that still holds, which is why `renaming`
493+
is in `ACTIONABLE` and still files an incident.
494+
495+
What was wrong is that **one event was reported as two unrelated ones**, and
496+
neither of them named what happened. A reader was told a column had vanished and,
497+
separately, that an unfamiliar column had appeared — and left to guess these were
498+
the same column. On the exact table whose headline property is that a rename
499+
costs nothing because the field ID does not move, the agent could not see the
500+
field ID: `schema_history()` returned `field_ids` and `observe()` discarded them.
501+
502+
`build_rename_map` now groups every historical column name by field ID, so a name
503+
that has vanished from the latest schema while its ID is still present is a
504+
rename, not a drop — and `checkNumber` and `check_reference` are both field 20.
505+
Transitive renames (`a → b → c`) resolve to the current name in one hop, because
506+
the map is keyed on the ID rather than on consecutive pairs.
507+
508+
**Pairing by field ID is not a stylistic preference over pairing by name or
509+
position, and the test suite has to prove that.** A rename does not move a
510+
column, so ordinal matching gets a plain rename right and looks correct; it
511+
breaks only when a column is deleted from the middle and everything after it
512+
shifts up, at which point it invents a rename that never happened *and* hides the
513+
real drop. A first draft of these tests supplied the rename map as a fixture
514+
rather than building it, and a deliberately planted position-matching mutant
515+
passed all 19 of them. The tests now build the map from schema history for a
516+
`{a:1, b:2, c:3} → {a:1, c_new:3}` evolution, where position and field ID give
517+
different answers, and both the position mutant and a name-similarity mutant fail
518+
against them — as does a real Iceberg table with a column deleted and another
519+
renamed in the same step.
520+
521+
`widening` likewise stopped being a synonym for `additive`. A widened `int → long`
522+
and a brand-new column are both safe and neither pages anyone, but only one of
523+
them means the contract is behind on a column that already existed.
524+
525+
**One honest gap in the demo**: `drift-demo`'s own `settlementDays int → long`
526+
step shows up above as `additive`, not `widening`. That is correct — `drift-demo`
527+
*adds* `settlementDays` before widening it, so the shipped contract never declared
528+
it and the agent has no narrower type to compare against. The `widening` class
529+
fires when a column the contract declares gets wider, which is covered against a
530+
real table by `test_real_widening_classifies_as_widening`. Declaring
531+
`settlementDays` in the contract to make the demo prettier would make the clean
532+
run report a phantom dropped column, so the demo stays as it is and the gap is
533+
stated instead.
478534

479535
The incident file is careful about *which* consumers, and that took a fix. The
480536
generated reasoning used to end "its absence will fail the next Silver build",

contracts/bronze_yodlee_transactions.yaml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,30 @@ expectations:
3939
- {type: accepted_values, column: baseType, values: [CREDIT, DEBIT]}
4040
- {type: row_count, min: 1000}
4141
- {type: freshness, column: transactionDate, max_lag_days: 3}
42+
43+
# Categorical columns watched for NEW values. Deliberately separate from
44+
# `expectations`: an `accepted_values` expectation is a fail-closed gate that
45+
# stops the build, which is right for `baseType` (the pipeline's sign
46+
# convention branches on it -- an unrecognised value would silently mis-sign
47+
# spend) and wrong for everything below, where upstream adding a category
48+
# breaks nothing but must not pass unnoticed.
49+
#
50+
# `baseType` is deliberately NOT repeated here. One column, one semantics:
51+
# it is a gate, not a watch.
52+
enum_watch:
53+
- column: category
54+
known_values: [Restaurants/Dining, General Merchandise, Clothing/Shoes,
55+
Personal Care, Other Expenses]
56+
- column: detailCategory
57+
known_values: [Coffee Shop, Fast Food, Department Store, Athletic Apparel,
58+
Pizza, Cosmetics, Uncategorized]
59+
- column: subType
60+
known_values: [PAYMENT, CREDIT]
61+
- column: categoryType
62+
known_values: [EXPENSE, INCOME]
63+
- column: status
64+
known_values: [POSTED]
65+
- column: categorySource
66+
known_values: [SYSTEM]
67+
- column: sourceType
68+
known_values: [AGGREGATED]
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
# 0006 — Renames paired by field ID, and the guard test that was vacuous
2+
3+
**Type:** feature ([#1](https://github.com/rubinder/autonomous_data_platform/issues/1)),
4+
plus a self-inflicted vacuous test caught by mutation
5+
**Made:** during implementation, by planting a position-matching mutant
6+
**Shipped:** `sensors.build_rename_map()` groups historical column names by
7+
Iceberg field ID; `classify()` gains `renaming`, `widening` and `enum_drift`
8+
9+
## What changed
10+
11+
The agent used to report `checkNumber → check_reference` as two findings — a
12+
`breaking` drop and an `additive` add — on the one table whose headline property
13+
is that a rename is metadata-only because the field ID does not move. The
14+
evidence to do better was already being fetched and then discarded:
15+
`pyiceberg_engine.schema_history()` returns `field_ids`, and `observe()` read
16+
only `["columns"]`.
17+
18+
Measured, at full scale, on a warehouse with no `ops.monitor_results` history:
19+
20+
| Sequence | Before | After |
21+
|---|---|---|
22+
| `all → agent` | 0 findings | 0 findings |
23+
| `all → drift-demo → agent` | 5 findings, 2 incidents | **4 findings, 2 incidents** |
24+
| `all → monitor → drift-demo → agent` | 6 findings, 3 incidents | **5 findings, 3 incidents** |
25+
26+
`renaming` is in `ACTIONABLE`: no data moved, but the published contract now
27+
names a column that does not exist, and that needs a recorded decision. The
28+
severity split only earns its keep if it changes what the agent *does*, so
29+
there is a test asserting exactly which severities act.
30+
31+
## The vacuous guard test
32+
33+
The first draft included a test called
34+
`test_rename_pairing_does_not_capture_an_unrelated_added_column`, written
35+
specifically to prove the pairing was by field ID and not by position or name.
36+
It constructed an `ObservedState` with `renames=` supplied as a **fixture**
37+
so it exercised `detect()`'s use of the map and never `build_rename_map` itself.
38+
39+
A planted mutant that replaced field-ID grouping with ordinal position matching
40+
**passed all 19 tests.**
41+
42+
The reason it survives is worth recording, because it is not obvious: *a rename
43+
does not move a column*. Ordinal matching therefore gets a plain rename right,
44+
and gets the rename-plus-add case right too, since PyIceberg appends new columns
45+
at the end. It breaks only when a column is deleted from the middle and every
46+
later column shifts up by one — at which point it invents a rename that never
47+
happened and simultaneously hides a real drop.
48+
49+
So the discriminating case had to be built deliberately:
50+
51+
```
52+
history: {a:1, b:2, c:3} -> {a:1, c_new:3}
53+
by position: [a,b,c] vs [a,c_new] -> pairs b -> c_new (wrong, twice)
54+
by field id: id 2 is gone (a real drop); id 3 changed name (the rename)
55+
```
56+
57+
Three tests now cover it: a unit test on `build_rename_map` with that history, an
58+
end-to-end `detect()` test asserting `b` is still `breaking`, and a real-Iceberg
59+
test that deletes `detailCategoryId` and renames `checkNumber` in one step. The
60+
position mutant and a name-similarity mutant both fail against them.
61+
62+
**The generalisable point:** a test that supplies the thing under test as a
63+
fixture is testing the consumer, not the producer, however precisely its name
64+
describes the producer's contract. This is the third instance of the same shape
65+
in this repository (Task 8's dedup test, Task 16's widening test), and the only
66+
reliable way any of the three were found was planting the mutant and watching the
67+
suite stay green.
68+
69+
## Enum drift is a separate contract key, not an expectation
70+
71+
`enum_watch` is a new top-level key in the contract YAML rather than a new
72+
`expectations` entry. Everything in `expectations` is a fail-closed gate:
73+
`assert_valid` raises on failure, and `_evaluate` raises on any type it does not
74+
recognise — which is precisely what stops a typo'd check from reading green
75+
forever. An enum watch must never block a build, so putting it in `expectations`
76+
would have required a branch that always passes: a check that displays green
77+
while measuring nothing, which is the exact defect
78+
`silver_txn_quarantine_rate: SELECT 0.0` already was once.
79+
80+
`baseType` is deliberately *not* watched. It already has a fail-closed
81+
`accepted_values` expectation because the pipeline's sign convention branches on
82+
it, and an unrecognised value there would silently mis-sign spend. One column,
83+
one semantics: a gate or a watch, never both.
84+
85+
A watch whose column produces no values classifies `breaking`, not silent-pass —
86+
a watch that cannot see its column occupies the slot where the alert would have
87+
been. A watch on a column with more than `ENUM_CARDINALITY_LIMIT` (200) distinct
88+
values reports itself misconfigured rather than emitting a finding carrying the
89+
whole cardinality.
90+
91+
## Known gap, stated rather than hidden
92+
93+
`drift-demo`'s `settlementDays int → long` step reports as `additive`, not
94+
`widening`, because `drift-demo` adds that column before widening it — the
95+
shipped contract never declared it, so there is no narrower declared type to
96+
compare against. `widening` fires when a *declared* column gets wider, covered
97+
against a real table by `test_real_widening_classifies_as_widening`. Adding
98+
`settlementDays` to the contract to make the demo read better would make the
99+
clean run report a phantom dropped column, which is a worse trade.

docs/incidents/bronze-yodlee_transactions_raw-checkNumber-3955ba01.md

Lines changed: 0 additions & 19 deletions
This file was deleted.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# Schema Drift — bronze.yodlee_transactions_raw
2+
3+
**Severity:** renaming
4+
5+
**Detected as of:** 2026-06-30
6+
7+
## What was observed
8+
9+
column 'checkNumber' was renamed to 'check_reference' (field id 20); the contract still declares the old name
10+
11+
## Why this severity
12+
13+
'checkNumber' was renamed to 'check_reference'. Both are field id 20: Iceberg resolves columns by ID, not by name, so every file written under the old name still reads back correctly and no data was rewritten or lost. What is wrong is the published contract, which still declares 'checkNumber'. That needs a recorded decision, not a rollback.
14+
15+
## Evidence
16+
17+
```
18+
{'change': 'renamed', 'column': 'checkNumber', 'renamed_to': 'check_reference', 'field_id': 20, 'declared_type': 'string', 'observed_type': 'string', 'type_compatible': True}
19+
```

0 commit comments

Comments
 (0)