fix(ai-cost): heal seat_status onto the AI dev-usage contract on upgrade - #2629
Conversation
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I don't think we need to build the silver models |
constructorfabric#2607 added the column to the class contract but to no migration, so only a fresh cluster got it: the DDL snapshot is CREATE TABLE IF NOT EXISTS, and dbt appends it (on_schema_change=append_new_columns) only when the silver model runs — which happens at connector-sync time, after the deploy. The migrate hook builds gold in between, and ai_metric_evidence reads seat_status, so upgrading an existing installation failed the hook and rolled the release back. The staging heal is not optional. A class unions its staging members positionally, and each member gains the column on its own connector's sync, so an install with two AI vendors gets `UNION different number of columns` on the first sync after the upgrade — a sync failure traded for the deploy failure. Healing all four contributors in one deploy closes that window. AFTER _version, matching the position the column holds in the staging projections and in the DDL snapshot; MODIFY converges an instance where an out-of-band ALTER placed it elsewhere. Signed-off-by: Gregory Gogin <grigoriy.gogin@constructor.tech>
69c105f to
79afca4
Compare
79afca4 to
0819878
Compare
fixed |
Closes #2617.
Two defects from #2607: one broke
helm upgrade, the other breaks a connector sync.The column never reached an existing installation
seat_statusentered the class contract in three places — the staging projections, the DDL snapshot, the gold model that reads it — and in no migration. So only a fresh cluster ever had it: the snapshot isCREATE TABLE IF NOT EXISTS, and dbt appends the column (on_schema_change=append_new_columns) only when the silver model runs, which is connector-sync time. The migrate hook builds gold in between,ai_metric_evidencereads the column, and the hook fails the release.A migration adds it to
silver.class_ai_dev_usage,AFTER _version— the position it holds in the staging projections and in the snapshot, and the one the positional incremental insert requires.MODIFYfollows theADDso an instance where an out-of-bandALTERplaced it elsewhere converges too, which is the state at least one environment is in.A class heals one contributor at a time, and the union does not tolerate that
The silver class unions its staging members with a positional
SELECT * UNION ALL, and each member gains the column on its own connector's sync. Between the first such sync and the last, the union has members of different widths:So a migration on silver alone would have traded the deploy failure for a sync failure on any installation with more than one AI vendor. The staging heal already in the hook now carries the same two
ALTERs, converging all four contributors in one deploy. Staging is healed there rather than by a migration because those tables are absent from the DDL snapshot and exist only after their connector's first sync, so the statements need thech_table_existsguard.Scope
An earlier revision also had the hook build the silver classes before gold, so that a release adding a silver column could not fail on its own gold model. Dropped on review: the migration and the heal are sufficient (re-verified below with the selector unchanged), and the hook change bought little — the column enters the contract through staging, which heals only on its own sync, so the
ALTERhas to be written by hand either way. It cost a three-term selector, 48 incremental models on the deploy path, and a wider blast radius for a mechanismdrop_silver_placeholders_at_startalready records a plan to retire.No DDL snapshot regeneration:
silver.sqlalready carriesseat_statusfrom #2607, and the AI staging tables are deliberately absent from the snapshot. Gold needs nothing — every gold model ismaterialized: table, rebuilt whole on each run.Test plan
Against a throwaway
clickhouse/clickhouse-server:25.7and the deployed toolbox image (insight-toolbox:2026.08.18.02.16-0297cb1) with this tree mounted, running the realapply-ch-migrations.shend to end — the same script the Hook Job runs, with its selector untouched:main— a warm install rolled back to the pre-fix(ai-cost): stop the Claude Team path losing data it already has #2607 shape (silver and both staging tables at 27 columns, rows in silver): exit 1,PASS=18 ERROR=1 SKIP=2,code: 47 … Unknown expression or function identifier seat_status … ai_metric_evidence. The reported failure, reproducedNullable(String); the rows that were in silver survived;ai_metric_evidencerebuiltmain: the silver build fails withUNION different number of columns (TYPE_MISMATCH)bash -n,git diff --checkNot verified: a real
helm upgrade, which needs a release through gitops.