feat(ingestion): reconcile San Acacia points against Ocotillo wells - #852
Merged
Conversation
Task 3.2's first half: a report saying, per monitoring point, whether a matching
well exists. Read-only on both sides.
Also corrects the "33 wells" figure that has run through this plan from the
start. It came from Aqueduct's docs/sources/san_acacia.md, in a sentence about
`/locations/{projectName}` -- an endpoint that does not exist. That same
document supplied the doubled /api/api/ path, the claim the source is
unauthenticated, and the gs/vrd payload shape, all disproved against the live
API. The count has no more standing than the rest of it, so 38 is not a
discrepancy to explain but the number to use, and this was never the blocker it
was treated as.
Coordinate proximity, the third matching signal the plan called for, is not
available: MonitoringPoint is {id, name}. That removes the only fuzzy signal and
leaves two exact ones, so every match is defensible rather than probabilistic.
The module never picks a winner. More than one candidate is ambiguous and
escalates; none is unmatched and escalates. Ingestion does not create wells, and
choosing between two plausible ones is exactly the judgement that must not be
automated -- the duplicate Geographic Area groups in this database are the
standing reminder.
Names compare on significant characters, so SO-0125, so 0125 and SO0125 are one
identifier while SO-0126 stays a different well. report.ready is false unless
every point resolved, and false for empty input, because a partial load produces
a series that looks complete and is not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 79.30% total — gate is 75%. Coverage for the Python files changed in this PR
|
Contributor
Coverage✅ 79.38% total — gate is 75%. Coverage for the Python files changed in this PR
|
I inferred the module path from the table name: thing_id_link became db.thing_id_link, and the class ThingIDLink. Both wrong -- the class is ThingIdLink and it is defined in db.thing. The query is now exercised against a real database rather than only imported, so the SQL is verified and not just the syntax. Every import in both new modules is checked to resolve, which is the class of mistake this was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 79.31% total — gate is 75%. Coverage for the Python files changed in this PR
|
Reconciling against staging answered 3.2: all 38 Diver-HUB points match Ocotillo wells by name, nothing ambiguous, nothing unmatched. The wells already exist, so the seeding half creates none. The same data showed external-id matching is unsafe here. thing_id_link holds 11,148 links from nine organization/relation pairs that disagree with each other. SO-0131 carries NMBGMR "BRN-E04B (shallow)" plus an unattributed "BRN-E04A", while SO-0132 carries NMBGMR "BRN-E04A (deep)" plus an unattributed "BRN-E04B" -- the two sources swap which physical well is A and which is B. Matching BRN-E04A against that returns one confident hit on SO-0131, contradicting NMBGMR, because the parenthetical suffix stops the collision registering as ambiguous. That is worse than the ambiguity the module was built to escalate: a wrong answer delivered with no sign of trouble. So the fallback is opt-in, and a test pins those exact rows. It costs nothing today, since every point matches by name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 79.31% total — gate is 75%. Coverage for the Python files changed in this PR
|
All 38 Diver-HUB points match Ocotillo wells by name in production too, with the same thing ids as staging -- SO-0125 is 2343 in both -- so the two agree by construction and 3.2's reconciliation is settled for both environments. The seeding half creates no wells. Also records that the identifier contradictions are production data rather than a staging artifact. SO-0131/SO-0132 and SO-0262/SO-0263 are paired shallow/deep piezometers whose A/B designations disagree between identifier sources, which matters to anyone reasoning about those wells through the BRN- or NRCS names even though ingestion itself is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Task 3.2's first half — a report saying, per monitoring point, whether a
matching Ocotillo well exists. Read-only on both sides. 12 tests.
The "33 wells" figure was wrong, and was never a blocker
I have been treating 38-vs-33 as a discrepancy to resolve before 3.2 could
start. Tracing it: the 33 came from Aqueduct's
docs/sources/san_acacia.md, ina sentence about an endpoint that does not exist —
That is the same document that supplied the doubled
/api/api/path, the claimthe source is unauthenticated, and the
gs/vrdpayload shape — all fourdisproved against the live API. The count has no more standing than the rest of
it: a FROST-era snapshot, not a Bureau record.
38 is the number. Whether all 38 are in scope — some may be decommissioned,
or belong to a neighbouring project — is a question the per-well report answers
concretely, rather than one that had to be settled first. Corrected throughout
the plan and the source doc.
Coordinate proximity is not available
The plan called for matching on name, external id, and coordinate proximity.
MonitoringPointis{id, name}— no coordinates.That removes the only fuzzy signal and leaves two exact ones, which is the
better position: every match here is defensible rather than probabilistic.
What it will not do
It never picks a winner. More than one candidate is
ambiguous; none isunmatched. Both escalate. Ingestion does not create wells, and choosingbetween two plausible ones is exactly the judgement that must not be automated —
the duplicate Geographic Area groups in this database are the standing reminder
that "looks like a new record" is not proof.
report.readyis false unless every point resolved, and false for emptyinput. A partial load produces a series that looks complete and is not. The
script exits non-zero accordingly, so it can gate a later step without relying
on someone reading the output.
Matching detail
Names compare on significant characters —
SO-0125,so 0125andSO0125areone identifier, while
SO-0126stays a different well. Name beats external idwhen both hit: the name is what the Bureau uses now, a link records an
association someone made earlier.
To run
export DIVERHUB_USERNAME=... DIVERHUB_PASSWORD=... uv run --group ingestion python -m automated_ingestion.scripts.reconcile_san_acaciaNeeds database read access, which is the same grant the duplicate report needed.
Not in this PR
The seeding half of 3.2 — creating missing
Location/Thing, the DTWParameter, theVanEssenDiverSensor, aDeploymentper well, the vendoruidas external identifier, andDataProvenance. That should wait on thereport's output: what needs creating depends on what it finds.
🤖 Generated with Claude Code