feat(groups): consolidate duplicate Geographic Area groups (BDMS-1143) - #821
Open
jirhiker wants to merge 4 commits into
Open
feat(groups): consolidate duplicate Geographic Area groups (BDMS-1143)#821jirhiker wants to merge 4 commits into
jirhiker wants to merge 4 commits into
Conversation
The domain/ layer introduced by ADR4 was never added to [tool.setuptools] packages, so it was missing from the installed distribution. Any `oco` command reaching db.thing -> services.util -> domain.units died with ModuleNotFoundError, which is most of the CLI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Data migrations that delete rows or re-point foreign keys need their planned changes reviewed before anything is written, but run() took no arguments and there was no way to preview one. Adds an optional dry_run callable to DataMigration, a runner entry point that checks the alembic gate, rolls back afterwards and never records history, and a --dry-run flag on `oco data-migrations run`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BDMS-1143. A project can exist twice in the group table: once as the project record and once as a Geographic Area carrying the boundary geometry. The migration copies project_area onto the project record, re-points linked things and child groups, then deletes the Geographic Area. Both FKs into group are ON DELETE CASCADE, so re-pointing is re-queried at apply time rather than replayed from the plan. group_type turned out not to be provenance. project_area_import.py matches only Geographic Area rows and creates one when it misses, while group_transfer.py matches by name with no type filter and only upgrades a row to Monitoring Plan when a well is currently monitored. Whichever ran first decides the surviving row's type, so a legacy project may live as a Monitoring Plan, as NULL, or as a Geographic Area. Merge targets are therefore any non-Geographic-Area group, and the target's type is left as found. That collision also makes some deletions destructive: a Geographic Area whose name is a legacy project name IS that project's row. PROTECTED_NAMES refuses those outright, ahead of any manual mapping. Contested targets claimed by two areas are reported rather than silently half-applied. Manual pairs were chosen from well membership -- ProjectLocations.csv joined to Location.csv, point-in-polygon against each boundary -- not from name similarity, because Monitoring Plan names come from legacy NM_Aquifer and Geographic Area names from the current ArcGIS layer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records the branching rule from docs/release-flow.md in the agent playbook, including the hotfix exception. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
@jirhiker Took a quick look this morning, will dig a little deeper this afternoon and reach out with questions. |
Member
Author
|
Thanks no rush. We should talk with Liz about this before any action is taken |
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.
Summary
BDMS-1143. Merges duplicate Geographic Area groups into the record that already represents the project, so one project is one row.
Adds
data_migrations/migrations/20260810_0001_consolidate_geographic_area_groups.py, plus dry-run support in the data-migration framework, because a migration that deletes rows and re-points foreign keys should be reviewable before it writes.What the investigation changed about the ticket
The ticket assumes Geographic Area rows are duplicates of Monitoring Plans. Two findings complicate that, and both are encoded in the migration:
group_typeis not provenance.cli/project_area_import.pymatches onlygroup_type == 'Geographic Area'and creates a row when it misses.transfers/group_transfer.pymatches by name with no type filter and only upgrades a row toMonitoring Planwhen one of its wells is currently monitored. Whichever ran first decides the surviving row's type, so a legacy NM_Aquifer project may live as a Monitoring Plan, asgroup_type = NULL, or as a Geographic Area. Merge targets are therefore any non-Geographic-Area group, and a target'sgroup_typeis left exactly as found.Some Geographic Areas are not duplicates — they are the project. Where a Geographic Area's name is also a legacy project name, that row carries the project's identity and its well links. Both FKs into
groupareON DELETE CASCADE, so merging one would take its wells with it.PROTECTED_NAMESrefuses those outright, ahead of any manual mapping:Albuquerque Basin·Colfax County·Eastern Tularosa Basin·Eddy County·Mimbres Basin·Quay County·Rio Rancho·San Miguel County·Torrance CountyWorth noting
Eastern Tularosa Basin→Tularosa Basinlooks like an obvious merge by name. It would have destroyed a project.How pairs were chosen
Not by name similarity. Monitoring Plan names come from legacy NM_Aquifer (
Projects.csv); Geographic Area names come from the current ArcGIS layer (maps.nmt.edu/.../Water_Resources/MapServer/17). Different systems, so the names rarely align — exact normalized matching found 1 pair out of 46.Instead,
ProjectLocations.csvwas joined toLocation.csvand every legacy well tested point-in-polygon against each boundary. A pair qualifies only when both shares are high: the share of the project's wells inside the boundary and the share of the boundary's wells belonging to that project. High on one alone is containment, not identity — which is whySan Juan Basin→Animas Riverwas rejected despite scoring 96%.Results on the dev database
Geographic Area rows 46 → 32. Fourteen consolidated: 1 by normalized name, 9 by name-only readings (abbreviations, qualifiers, word order), 4 by well membership.
Left for a human decision, documented in the module docstring:
Tiffany Firecontains bothTiffany Fire RecoveryandTiffany Fire Restorationat 100% with identical well counts — those two plans are duplicates of each other, a separate ticket.Eastern Tularosa BasinandNortheastern Tularosa Basinboth point at oneTularosa Basin; many-to-one merges are refused rather than half-applied.Questa AreavsQuesta Red Riverwas merged on 81.8% / 77.1% well overlap.The remaining 23 unmatched areas have no counterpart at all — counties and basins that are genuinely standalone. Together with the 9 protected rows, 32 of the original 46 should stay Geographic Areas. That answers the ticket's open question about retiring the
group_type: it should not be retired.Ordering note for staging and production
Run consolidation before
20260714_0001_publish_project_areas. Theogc_project_areasview filtersrelease_status = 'public'; a boundary copied onto a target after that migration has run staysdraftand drops out of the view until it is published again.Also included
fix(build):domain/was missing from[tool.setuptools] packages, which broke everyococommand that reachesdb.thing.docs(agents): records the branch-off-stagingrule fromdocs/release-flow.md.Testing
16 tests covering the merge, geometry conflicts, unmatched and protected areas, untyped targets, contested targets, idempotency, and dry-run-writes-nothing. Full suite green,
blackandflake8clean.@ksmuczynski — could you verify and validate the projects list before this goes past staging? Two asks specifically:
MANUAL_MATCHESare my reading of the evidence, not a decision anyone signed off on. The dry run prints every planned change with its source, so a review pass over that output is the check I would trust most.Tiffany Fire Recovery/Tiffany Fire Restorationduplicate pair, the case-only duplicatesWater Level Network/water Level Network, the two Tularosa Basin areas competing for one plan, and the 23 unmatched Geographic Areas — some of which may just be stale.Nothing here has run outside the local dev database.
🤖 Generated with Claude Code