Skip to content

Conversation

@jirhiker
Copy link
Member

Add nma_formation_zone to Thing

Why

  • The legacy WellData FormationZone (nvarchar(25)) needs to be preserved on Thing.
  • We already store a normalized formation code, but the raw legacy value is needed for audit and fidelity.

What changed

  • Added nma_formation_zone to Thing (nullable String(25)) and exposed it in Thing schemas for create/update/response.
  • Updated well transfer to copy FormationZone into nma_formation_zone while keeping existing normalization logic.
  • Added a migration to add nma_formation_zone to thing and thing_version.
  • Added a small schema unit test for nma_formation_zone.

Files touched

  • db/thing.py
  • schemas/thing.py
  • transfers/well_transfer.py
  • alembic/versions/f1a2b3c4d5e6_add_nma_formation_zone_to_thing.py
  • tests/test_thing.py

Testing

  • pytest tests/test_thing.py -k nma_formation_zone

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds the nma_formation_zone field to the Thing model to preserve the raw FormationZone value from legacy WellData for audit and fidelity purposes, alongside the existing normalized formation code.

Changes:

  • Added nma_formation_zone column to Thing and thing_version tables
  • Updated well transfer logic to populate the new field from legacy data
  • Exposed the field in Thing schemas for create, update, and response operations

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
db/thing.py Added nma_formation_zone column definition to Thing model
schemas/thing.py Exposed nma_formation_zone in CreateWell, UpdateWell, and WellResponse schemas
transfers/well_transfer.py Updated well transfer logic to populate nma_formation_zone from legacy FormationZone
alembic/versions/f1a2b3c4d5e6_add_nma_formation_zone_to_thing.py Migration to add nma_formation_zone to thing and thing_version tables
tests/test_thing.py Added unit test for UpdateWell schema with nma_formation_zone

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2dc12b9363

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ksmuczynski
Copy link
Contributor

The GeologicFormation model has a formation_code field that ultimately should/will be used to store NMA FormationZone values. If formation zone info needs to be stored in the Thing model, too, for the puposes of migrating these legacy tables that's fine, just wanted to point out that Ocotillo does currently have a place for FormationZone info.

@jacob-a-brown
Copy link
Contributor

The formation zone of the well's completion is found in the formation_completion_code field of the WellResponse model

@jirhiker
Copy link
Member Author

@ksmuczynski @jacob-a-brown although Ocotillo has a place for FormationZone, a large number of the formation zones in NMA are not in the Ocotillo lexicon, and in many cases there are multiple formation zones for a given well, e.g. 211DKOT/217PRGR/110AVMB. nma_formation_zone is to facilitate bringing this data over as is. Refactoring into the GeologicFormation model will be done later

@jirhiker jirhiker merged commit 611eb8b into staging Jan 15, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants