Skip to content

feat(cli): chemistry LIMS ingestion with Google Drive sync (BDMS-1034)#775

Open
jirhiker wants to merge 4 commits into
stagingfrom
feat/BDMS-1034-chemistry-ingestion-cli
Open

feat(cli): chemistry LIMS ingestion with Google Drive sync (BDMS-1034)#775
jirhiker wants to merge 4 commits into
stagingfrom
feat/BDMS-1034-chemistry-ingestion-cli

Conversation

@jirhiker

@jirhiker jirhiker commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

Adds an oco water-chemistry CLI to ingest lab LIMS .xlsx workbooks into the legacy NMA chemistry tables (NMA_MajorChemistry / NMA_MinorTraceChemistry), as an interim workaround while the fuller solution is pursued. Analyte mapping, non-detect handling, and EPA-200.7 / low-bromide dedup are ported from AMPAPI chemfile.py.

Interim process: Sianin drops LIMS .xlsx files in a shared Google Drive folder; an engineer (Kelsey) runs the ingest on demand; results are recorded in a GCS manifest. See docs/chemistry-ingestion-runbook.md.

Commands

  • oco water-chemistry bulk-upload --file batch.xlsx — load a single workbook.
  • oco water-chemistry sync-drive [--dry-run] — on-demand, engineer-triggered ingest of new/changed files from CHEMISTRY_DRIVE_FOLDER_ID. No polling/scheduling. An ingested-file manifest (keyed by Drive file id + md5) lives in GCS so re-runs skip unchanged files.

Append semantics (BDMS-1034 core)

  • SamplePointID in the LIMS file is the base well PointID.
  • Each distinct lab sample (WCLab_ID / SampleNumber) becomes a new NMA_Chemistry_SampleInfo at base + next letter incrementorA, B, … Z, AA, … (bijective base-26, matching the ~11k-row migration data where A is the first sample).
  • A WCLab_ID already recorded for the well is skipped (idempotent).
  • Data-quality problems (unmapped analyte, no matching well) abort the whole file.

Also

  • New cli dependency group (openpyxl, google-api-python-client): CI syncs it with --group cli; the production requirements export (uv export --no-dev) excludes it, so these deps stay out of the API runtime.
  • Drop the CD refresh-materialized-views step (now handled by the nightly pg_cron job).

Testing

  • ENABLE_PG_CRON=0 uv run pytest tests/test_chemistry_lims.py tests/test_chemistry_drive.py23 passed (analyte mapping, ND handling, dedup, suffix base-26 roundtrip, append/skip, Drive sync + manifest with fakes).
  • flake8 + black clean.

Notes / open

  • Duplicate detection is WCLab_ID-only (documented as a known gap).
  • staging/testing deploys don't set ENABLE_PG_CRON, so they get no view refresh now — parity change deferred, not in this PR.

🤖 Generated with Claude Code

Refs BDMS-1034

jirhiker and others added 2 commits July 8, 2026 10:08
Add `oco water-chemistry` commands to ingest lab LIMS .xlsx workbooks into the
legacy NMA chemistry tables (NMA_MajorChemistry / NMA_MinorTraceChemistry),
porting the analyte mapping, non-detect handling, and EPA-200.7 / low-bromide
dedup from the AMPAPI chemfile.py script.

- bulk-upload: parse a LIMS .xlsx and load a single file
- sync-drive: on-demand, engineer-triggered ingest of new/changed files from a
  Google Drive folder, with an ingested-file manifest stored in GCS
- append semantics: each distinct lab sample (WCLab_ID) becomes a new sample
  point using the next letter incrementor on the base PointID
  (MG-030A, MG-030B, ... Z, AA); a lab sample already recorded for the well is
  skipped, so re-runs are idempotent
- add a `cli` dependency group (openpyxl, google-api-python-client); CI syncs it
  explicitly, the production requirements export excludes it
- add docs/chemistry-ingestion-runbook.md

Refs BDMS-1034

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Materialized-view refresh is handled by the nightly pg_cron job, so the
production/staging/testing deploy workflows no longer need to run
`python -m cli.cli refresh-materialized-views`.

Refs BDMS-1034

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jirhiker
jirhiker requested review from ksmuczynski and lizalino July 8, 2026 16:08
jirhiker and others added 2 commits July 8, 2026 11:49
- CLI serializes its own JSON from the result payload; drop the pretty_json
  flag threaded through the service and the pre-baked `stdout` field on
  ChemistryUploadResult.
- Remove the unused `--output json` option from `water-chemistry bulk-upload`
  and `sync-drive`; they are engineer-facing and have no scripted consumer
  (programmatic callers use the service functions directly).
- Replace the FMapper/AnalyteField classes with a frozen `AnalyteMapping`
  dataclass, a flat `_ANALYTE_MAPPINGS` list, and a `lookup_analyte()` helper
  backed by a case-insensitive dict; drop the dead list/tuple and reverse-lookup
  branches.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reflect the renamed analyte map (`lookup_analyte` / `_ANALYTE_MAPPINGS`, was
`FMapper`) and the removal of the `--output json` option.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment on lines +164 to +167
- **Duplicate detection is WCLab_ID-only.** A re-ingest is recognized by the lab
`WCLab_ID` (SampleNumber). A genuinely new lab sample with a reused SampleNumber
would be treated as a duplicate and skipped; a re-run of the same sample under a
new SampleNumber would append a spurious extra lettered sample.

@ksmuczynski ksmuczynski Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How likely is this to happen? Should dup detection be across multiple fields, e.g. SampleNumber + AnalysisTime + Param? (field names are from an example lab .xlsx from Sianin)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nevermind, I see this is already suggested as an improvement in the fuller solution down below.

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.

2 participants