chore(energy-atlas): backfill productClass on oil pipelines + enforce enum#3383
Merged
Conversation
…orce enum
Prior state: 12/75 oil pipelines carried a `productClass: "crude"` tag;
63/75 did not. The field had zero consumers anywhere in the codebase
(no validator, no server handler, no frontend reader) — orphan metadata
from partial curation. Inconsistency spotted during the energy-data
audit after the Energy Atlas PR chain landed.
Changes:
1. Backfill all 63 missing entries with one of three values based on
the pipeline's name/operator/route:
- `crude` (70 total): crude-oil trunks, gathering lines, export
systems. Covers Druzhba, Enbridge Mainline, Keystone-XL, CPC,
BTC, ESPO, Sumed, Forties, Brent, OCP, OCENSA, EACOP, LAPSSET,
etc.
- `products` (4 total): explicit refined-product pipelines —
Abqaiq-Yanbu Products Line, Vadinar-Kandla, Yangzi-Hefei-Hangzhou,
Tuxpan-Mexico City.
- `mixed` (1 total): Salina Cruz-Minatitlán, the only dual-use
crude/products bridge in the set.
2. Promote productClass from orphan metadata to a schema invariant:
- Oil pipelines MUST declare one of {crude, products, mixed}.
- Gas pipelines MUST NOT carry the field (commodity IS its own
class there).
- Enforced in scripts/_pipeline-registry.mjs::validateRegistry.
3. Five new test assertions in tests/pipelines-registry.test.mts
cover both the data invariant (every oil entry has a valid value;
no gas entry has one) and the validator behavior (rejects missing,
rejects unknown enum value, rejects gas-with-productClass).
File formatting: the oil registry mixes two styles — multi-line (each
field on its own line) and compact (several fields packed onto one
line). The insertion preserves the local style for each entry by
reusing the whitespace that follows `"commodityType": "oil",`.
No runtime consumers yet; this lands the data hygiene so future
downstream work (crude-vs-products split on the map, refined-product
shock calcs) can rely on the field being present and valid.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryThis PR backfills Confidence Score: 4/5Safe to merge — the change is additive, all logic is correct, and the new invariant is fully tested. Only P2 findings: the inline tests/pipelines-registry.test.mts — inline enum copy could drift from the registry definition. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[validateRegistry called] --> B{data is object\nwith pipelines?}
B -- No --> FAIL[return false]
B -- Yes --> C{entries.length\n>= 8?}
C -- No --> FAIL
C -- Yes --> D[for each pipeline entry]
D --> E{commodityType?}
E -- oil --> F{VALID_OIL_PRODUCT_CLASSES\nhas productClass?}
F -- No --> FAIL
F -- Yes --> G[validate country codes,\ncapacity, coords, evidence]
E -- gas --> H{productClass\n!== undefined?}
H -- Yes --> FAIL
H -- No --> G
E -- other --> FAIL
G -- all pass --> PASS[return true]
G -- any fail --> FAIL
Reviews (1): Last reviewed commit: "chore(energy-atlas): backfill productCla..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }); | ||
|
|
||
| describe('pipeline registries — productClass', () => { | ||
| const VALID = new Set(['crude', 'products', 'mixed']); |
Contributor
There was a problem hiding this comment.
VALID is defined inline here as new Set(['crude', 'products', 'mixed']), mirroring VALID_OIL_PRODUCT_CLASSES in _pipeline-registry.mjs. If a new class (e.g. condensate) is added to the registry's set, this local copy won't automatically update — the data test would start reporting valid pipelines as failing before the validator rejects them, creating a confusing gap. Consider exporting VALID_OIL_PRODUCT_CLASSES (or a derived OIL_PRODUCT_CLASSES constant) from _pipeline-registry.mjs and importing it here instead of redefining it.
…of redefining Greptile P2 on #3383: the test file defined its own inline `const VALID = new Set(['crude', 'products', 'mixed'])`, mirroring the registry's `VALID_OIL_PRODUCT_CLASSES`. If a future PR adds a new class (e.g. `condensate`) to the registry, the inline copy wouldn't update — the data test would start reporting valid pipelines as failing before the validator rejects them, creating a confusing diagnostic gap. Export `VALID_OIL_PRODUCT_CLASSES` from `scripts/_pipeline-registry.mjs` and import it in the test. Single source of truth; no drift possible.
5 tasks
koala73
added a commit
that referenced
this pull request
Apr 25, 2026
…plan U1-U4) (#3397) * feat(energy-atlas): GEM pipeline import infrastructure (PR 1, plan U1-U4) Lands the parser, dedup helper, validator extensions, and operator runbook for the Global Energy Monitor (CC-BY 4.0) pipeline-data refresh — closing ~3.6× of the Energy Atlas pipeline-scale gap once the operator runs the import. Per docs/plans/2026-04-25-003-feat-energy-parity-pushup-plan.md PR 1. U1 — Validator + schema extensions: - Add `'gem'` to VALID_SOURCES in scripts/_pipeline-registry.mjs and to the evidence-bearing-source whitelist in derivePipelinePublicBadge so GEM- sourced offline rows derive a `disputed` badge via the external-signal rule (parity with `press`/`satellite`/`ais-relay`). - Export VALID_SOURCES so tests assert against the same source-of-truth the validator uses (matches the VALID_OIL_PRODUCT_CLASSES pattern from PR #3383). - Floor bump (MIN_PIPELINES_PER_REGISTRY 8→200) intentionally DEFERRED to the follow-up data PR — bumping it now would gate the existing 75+75 hand-curated rows below the new floor and break seeder publishes before the GEM data lands. U2 — GEM parser (test-first): - scripts/import-gem-pipelines.mjs reads a local JSON file (operator pre- converts GEM Excel externally — no `xlsx` dependency added). Schema- drift sentinel throws on missing columns. Status mapping covers Operating/Construction/Cancelled/Mothballed/Idle/Shut-in. ProductClass mapping covers Crude Oil / Refined Products / mixed-flow notes. Capacity-unit conversion handles bcm/y, bbl/d, Mbd, kbd. - 22 tests in tests/import-gem-pipelines.test.mjs cover schema sentinel, fuel split, status mapping, productClass mapping, capacity conversion, minimum-viable-evidence shape, registry-shape conformance, and bad- coordinate rejection. U3 — Deduplication (pure deterministic): - scripts/_pipeline-dedup.mjs: dedupePipelines(existing, candidates) → { toAdd, skippedDuplicates }. Match rule: haversine ≤5km AND name Jaccard ≥0.6 (BOTH required). Reverse-direction-pair-aware. - 19 tests cover internal helpers, match logic, id collision, determinism, and empty inputs. U4 — Operator runbook (data import deferred): - docs/methodology/pipelines.mdx: 7-step runbook for the operator to download GEM, pre-convert Excel→JSON, dry-run with --print-candidates, merge with --merge, bump the registry floor, and commit with provenance metadata. - The actual data import is intentionally OUT OF SCOPE for this agent- authored PR because GEM downloads are registration-gated. A follow-up PR will commit the imported scripts/data/pipelines-{gas,oil}.json + bump MIN_PIPELINES_PER_REGISTRY → 200 + record the GEM release SHA256. Tests: typecheck clean; 67 tests pass across the three test files. Codex-approved through 8 review rounds against origin/main @ 0500733. * fix(energy-atlas): wire --merge to dedupePipelines + within-batch dedup (PR1 review) P1 — --merge was a TODO no-op (import-gem-pipelines.mjs:291): - Previously exited with code 2 + a "TODO: wire dedup once U3 lands" message. The PR body and the methodology runbook both advertised --merge as the operator path. - Add mergeIntoRegistry(filename, candidates) helper that loads the existing envelope, runs dedupePipelines() against the candidate list, sorts new entries alphabetically by id (stable diff on rerun), validates the merged registry via validateRegistry(), and writes to disk only after validation passes. CLI --merge now invokes it for both gas and oil + prints a per-fuel summary. - Source attribution: the registry envelope's `source` field is upgraded to mention GEM (CC-BY 4.0) on first merge so the data file itself documents provenance. P2 — dedup transitive-match bug (_pipeline-dedup.mjs:120): - Pre-fix loop checked each candidate ONLY against the original `existing` array. Two GEM rows that match each other but not anything in `existing` would BOTH be added, defeating the dedup contract for same-batch duplicates (real example: a primary GEM entry plus a duplicate row from a regional supplemental sheet). - Now compares against existing FIRST (existing wins on cross-set match — preserves richer hand-curated evidence), then falls back to the already-accepted toAdd set. Within-batch matches retain the FIRST accepted candidate (deterministic by candidate-list order). Tests: 22 in tests/pipeline-dedup.test.mjs (3 new) cover the within-batch dedup, transitive collapse, and existing-wins-over- already-accepted scenarios. typecheck clean. * fix(energy-atlas): cross-file-atomic --merge (PR1 review #2) P1 — partial-import on disk if oil validation fails after gas writes (import-gem-pipelines.mjs:329 / :350): - Previous flow ran `mergeIntoRegistry('pipelines-gas.json', gas)` which wrote to disk, then `mergeIntoRegistry('pipelines-oil.json', oil)`. If oil validation failed, the operator was left with a half-imported state: gas had GEM rows committed to disk but oil didn't. - Refactor into a two-phase API: 1. prepareMerge(filename, candidates) — pure, no disk I/O. Builds the merged envelope, validates it, throws on validation failure. 2. mergeBothRegistries(gasCandidates, oilCandidates) — calls prepareMerge for BOTH fuels first; only writes to disk after BOTH pass validation. If oil's prepareMerge throws, gas was never touched on disk. - CLI --merge now invokes mergeBothRegistries. The atomicity guarantee is documented inline in the helper. typecheck clean. No new tests because the existing dedup + validate suites cover the underlying logic; the change is purely about call ordering for atomicity. * fix(energy-atlas): deterministic lastEvidenceUpdate + clarify test comment (PR1 review #3) P2 — lastEvidenceUpdate was non-deterministic (Greptile P2): - Previous code used new Date().toISOString() per parser run, so two runs of parseGemPipelines on the same input on different days produced byte-different output. Quarterly re-imports would produce noisy full-row diffs even when the upstream GEM data hadn't changed. - New: resolveEvidenceTimestamp(envelope) derives the timestamp from envelope.downloadedAt (the operator-recorded date) or sourceVersion if it parses as ISO. Falls back to 1970-01-01 sentinel when neither is set — deliberately ugly so reviewers spot the missing field in the data file diff rather than getting silent today's date. - Computed once per parse run so every emitted candidate gets the same timestamp. P2 — misleading test comment (Greptile P2): - Comment in tests/import-gem-pipelines.test.mjs:136 said "400_000 bbl/d ÷ 1000 = 400 Mbd" while the assertion correctly expects 0.4 (because the convention is millions, not thousands). Rewrote the comment to state the actual rule + arithmetic clearly. 3 new tests for determinism: (a) two parser runs produce identical output, (b) timestamp derives from downloadedAt, (c) missing date yields the epoch sentinel (loud failure mode).
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.
Summary
productClasson the 63/75 oil pipelines that were missing it; distribution lands at 70 crude / 4 products / 1 mixed.productClassfrom orphan metadata (zero consumers prior to this PR) to a schema invariant: oil pipelines MUST declare one of{crude, products, mixed}; gas pipelines MUST NOT carry the field.Spotted during the post-merge energy-data audit after the Energy Atlas PR chain (#3366/#3371/#3377/#3378/#3379).
grep productClass scripts/ server/ src/ proto/showed the field only ever lived in `scripts/data/pipelines-oil.json` — no handler, no validator, no frontend read it. Filling it consistently now so downstream work (crude-vs-products map split, refined-product shock calcs) can rely on the field being present and valid.Classification notes
crude(70): every pipeline named "Crude Pipeline / Trunk / Feed / Gathering / Export" + unambiguous crude trunks like Druzhba, CPC, ESPO, BTC, Sumed, Forties, Brent, OCP, OCENSA, EACOP, LAPSSET, Keystone-XL, Enbridge Mainline / Line 3 / Line 5 / Flanagan South / Seaway / Marketlink / Spearhead.products(4): Abqaiq–Yanbu Products Line, Vadinar–Kandla Products, Yangzi–Hefei–Hangzhou Products, Tuxpan–Mexico City Products.mixed(1): Salina Cruz–Minatitlán Products/Crude Bridge — dual-use, moves both crude and refined products between upstream fields and refineries.Formatting
`scripts/data/pipelines-oil.json` mixes two styles in the same file: multi-line (each field on its own line) and compact (several fields on one line). The insertion preserves the local style per-entry by capturing and reusing the whitespace that follows `"commodityType": "oil",` — no gratuitous reformatting.
Test plan