fix(ingestion): do not overwrite approved observations by default - #854
Merged
Conversation
Ocotillo already holds 542,161 approved observations for 14 of the 38 San Acacia wells, from the AMPAPI transfer and running to August 2022. The upsert would have replaced every one of them with a vendor reading and downgraded it to provisional, silently, on any backfill covering that window. DO UPDATE exists so a vendor correction can revise our own provisional readings. Applying it to reviewed history from another source is a different act, and it should be one somebody chooses: overwrite_approved defaults to False and the conflict clause skips approved rows. Rows with NULL maturity still update. Unknown is not approved, and treating it as such would freeze the 394,086 legacy rows with no QC record against every future correction. Still owed: those AMPAPI rows were loaded under whatever datum that pipeline used, and ours are ground-surface centimetres converted to feet. Overlapping timestamps should be compared before any window covering 2016-2022 is loaded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 79.33% total — gate is 75%. Coverage for the Python files changed in this PR
|
Contributor
|
Your pull request is automatically being deployed to Dagster Cloud.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Querying production for existing San Acacia data turned up something the plan
assumed away: 14 of the 38 wells already hold 542,161 observations, from the
AMPAPI transfer, running 2016-07-08 to 2022-08-03. They carry a real QC status,
so
data_maturitybackfilled them asapproved.The bug
The upsert was
DO UPDATE SET value = excluded.value, data_maturity = excluded.data_maturity, withprovisionalas the default maturity.A Mode A backfill over 2016–2022 would therefore have replaced 542,161
Bureau-approved values with vendor readings and downgraded them to
provisional — silently, reporting success.
DO UPDATEexists so a vendor correction can revise our own provisionalreadings. Applying it to reviewed history from another source is a different
act, and it should be one somebody chooses.
The fix
overwrite_approveddefaults toFalse, and the conflict clause skips rowswhose maturity is
approved. PassingTruesays the incoming data is betterthan what was reviewed — a judgement for a person.
Rows with NULL maturity still update:
IS DISTINCT FROMrather than!=.Unknown is not approved, and treating it as such would freeze the 394,086 legacy
rows with no QC record against every future correction.
Three tests: approved rows survive,
overwrite_approved=Truegets through, NULLrows still update.
Still owed before loading historical windows
Those AMPAPI rows were loaded under whatever datum that pipeline used; ours are
Diver-HUB ground-surface centimetres converted to feet. A few coinciding
timestamps should be compared before any window covering 2016–2022 is loaded.
Same failure shape as the
WaterLevelReferencequestion — plausible numbers,wrong meaning.
Context recorded in the plan
The wider table: 2,180,989 approved, 7,351 provisional, 394,086 NULL. That NULL
cohort is 176 deployments on a single parameter spanning 2016 to February 2025
with no AMPAPI provenance — a separate network, and none of the 38 San Acacia
wells are in it. Worth identifying independently of this work.
Also: the watermark now starts at 2022-08-03 for those 14 wells rather than the
2015 floor, so a normal run fetches a four-year gap rather than a decade.
🤖 Generated with Claude Code