Skip to content

feat: import v2 timeseries from the benchmark prep repo - #975

Merged
jacobvjk merged 7 commits into
epic/v2from
feat/timeseries-import
Oct 6, 2026
Merged

jacobvjk merged 7 commits into
epic/v2from
feat/timeseries-import

Conversation

@jacobvjk

@jacobvjk jacobvjk commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Part of #915 and #902. Stacked on #974.

scripts/import-benchmark-data.ts brings the output of RMI/tpr_benchmark_data_preparation into src/data. The prep repo writes one pathwayTimeseries.v2 JSON file per dataset.

npx ts-node --esm scripts/import-benchmark-data.ts --in <prep-repo-output-dir> --dry-run

It converts nothing; it is the gate between the two repos.

  • Checks: every file must pass the v2 schema and validateTimeseries against the metadata already in src/data. That means a declared geography, segments of the row's sector, known pathway ids, and unique dataset ids.
  • Destination:
    • A file whose id matches an existing timeseries overwrites it in place, keeping irregular names such as ATS-2024_timeseries.json.
    • A new file goes next to its first pathway's metadata.
    • Existing files missing from the input are reported, never deleted.
  • Same blocking contract as import-pathway-data.ts: any error blocks the whole import, nothing is written, and every problem is listed. The prep repo can use --dry-run to check its output against this repo before handing it over.

What the prep repo needs to change (RMI/tpr_benchmark_data_preparation)

Building on its feat/region-names branch:

  1. Write v2 JSON, one file per timeseries dataset, instead of (or alongside) the CSV.

    • Header: $schema (the v2 id), id, pathwayId[], name, description (must end with a full stop), publication, pathwayName, emissionsScope.
    • Rows: year, geography, sector, sectorSegment[], technology, metric, value, unit.
    • sector, technology and metric use TPR's camelCase keys (power, absoluteEmissions), not display names.
  2. Geography labels must equal the pathway's metadata keys. data/region_names.csv already gives ACE ASEAN and IEA Southeast Asia; JRC and TZ South East Asia also match after feat: timeseries schema v2 — published geography and sectorSegment #974.

  3. Replace sector_scope with a sector_segment list per row (R/20_legacy_benchmark_transform.R).

    • Refresh the vocabulary from TPR: the local copies still say Transmission & Distribution where TPR says Transmission and distribution.

    • Allowed values are the segments of the row's sector, with no sentinels. Legacy values map like this:

      Legacy value Becomes
      Grid-level storage Energy storage
      Upstream fuel Fuel extraction and processing
      Transmission and Distribution Transmission and distribution
      Captive power generation Power generation (the captive note belongs in data availability's scope limitations)
  4. Check output against TPR by running this importer with --dry-run from a TPR checkout (TPR_REPO_PATH). No second validator in R is needed.

  5. Update the README flow diagram and the tests/testthat cases, which still use sector_scope.

Checked

  • 9 fixture tests: placement, in-place overwrite, each kind of error blocking, duplicate ids, v1 input.
  • Round trip on the 16 current files: all map back to their own paths and the content is identical. Some hand-formatted header objects come out shaped differently, because the importer always writes the same format; that churn is not committed here.

🤖 Generated with Claude Code

scripts/import-benchmark-data.ts takes the output of
RMI/tpr_benchmark_data_preparation (one pathwayTimeseries.v2 JSON file
per dataset) into src/data. It converts nothing; it is the gate between
the two repos:

- every file must pass the v2 schema and validateTimeseries against the
  metadata already in src/data (declared geography, segments of the
  row's sector, known pathway ids), and ids must be unique
- a file whose id matches an existing timeseries overwrites it in place,
  keeping irregular names such as ATS-2024_timeseries.json; a new one
  goes next to its first pathway's metadata
- existing files missing from the input are reported, never deleted
- any error blocks the whole import and lists every problem, the same
  contract as import-pathway-data.ts; --dry-run lets the prep repo check
  its output against this repo

Round trip on the 16 current files: all map back in place and the
content is identical.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Expected version change and release notes

🚨 WARNING: This PR is not expected to trigger a new version

To trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-975.westus2.2.azurestaticapps.net

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Unsafe destination derivation, empty pathway acceptance, incomplete error reporting, and non-transactional writes violate the importer contract.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds a benchmark timeseries importer that validates prepared v2 datasets before placing them in src/data.

Changes:

  • Adds schema and cross-file validation with dry-run support.
  • Preserves existing paths and places new datasets beside pathway metadata.
  • Adds importer tests and usage documentation.
File Description
scripts/​import-benchmark-data.ts Implements validation, import planning, reporting, and writing.
scripts/​import-benchmark-data.test.ts Tests placement and validation failures.
src/​data/​README.md Documents the benchmark import workflow.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/import-benchmark-data.ts Outdated
jacobvjk and others added 2 commits October 6, 2026 12:43
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From the #975 review:

- A duplicate dataset id used to skip the file's remaining checks, so a
  second problem in the same file only surfaced on the next run. Every
  check now runs before the file is skipped.
- A new file is named after its id, so an id with "/" or a leading "."
  could write outside the pathway's folder. Such ids are now an error.
- A missing metadata path is an explicit error rather than a fallback
  to a guessed folder (unreachable today: unknown pathway ids are
  rejected, and #974 now requires a non-empty pathwayId).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Expected version change and release notes

🚨 WARNING: This PR is not expected to trigger a new version

To trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-975.westus2.2.azurestaticapps.net

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Destination collisions can overwrite unrelated data, and sequential writes violate the documented all-or-nothing guarantee.

Review effort: Balanced
Findings: 2 High severity

Open (2)
Resolved since last review (1)

Comment thread scripts/import-benchmark-data.ts Outdated
Comment thread scripts/import-benchmark-data.ts Outdated
jacobvjk and others added 2 commits October 6, 2026 12:53
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hing writes

From the #975 review:

- File names do not always follow ids: ACE's ATS-2024_timeseries.json
  holds ACE-ATS-2024_timeseries. An input with id ATS-2024_timeseries
  missed the id lookup and was placed on that same path, overwriting
  ACE's data. The corpus now records every path under src/data, and a
  new file may not land on one (or on another new file's path).
- Writes were sequential, so a failure partway through left a half
  import despite the all-or-nothing promise. Every file is now
  formatted before any is written, and commitWrites restores the files
  already written (or removes new ones) if a later write fails.

Checked on the real repo: the ACE case is refused with nothing written,
and re-importing the 16 current files still yields identical content.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Expected version change and release notes

🚨 WARNING: This PR is not expected to trigger a new version

To trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-975.westus2.2.azurestaticapps.net

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Write failures can still corrupt or delete existing destination files despite the documented all-or-nothing guarantee.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (2)

Comment thread scripts/import-benchmark-data.ts Outdated
Comment thread scripts/import-benchmark-data.ts
From the #975 review:

- A write can truncate its target before it rejects (ENOSPC, say), but
  the target was only marked for rollback after a successful write. It
  is now marked before the write, so the failed file is restored too.
- Any read error counted as "file does not exist", so an existing file
  that could be written but not read got no backup, and a rollback
  would have deleted it. Only ENOENT now means "new"; any other read
  error aborts before anything is written.
- A restore that fails no longer stops the others; the error names the
  files left as they are.

Tests cover a failing write that truncates its target, an unbackable
file, and, on the real file system, a write-only file surviving.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Expected version change and release notes

🚨 WARNING: This PR is not expected to trigger a new version

To trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-975.westus2.2.azurestaticapps.net

Copilot AI left a comment

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.

Copilot review overview

🟡 Changes recommended

Case-insensitive path collisions can overwrite unrelated files, and some placement errors are deferred to later runs.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Run placement checks even when semantic validation fails

scripts/​import-benchmark-data.ts:146

This early exit still prevents placement checks from running for a schema-valid file that has a semantic validation error. For example, an undeclared geography combined with a destination occupied by unrelated metadata reports only the geography; after that is fixed, the collision appears on a second run. This contradicts the importer's stated one-run contract to list every problem. Compute the candidate destination and collect its errors before deciding whether to add the write plan.

Comment thread scripts/import-benchmark-data.ts Outdated
…me run

From the #975 review:

- On the default macOS and Windows file systems iea-x.json and
  IEA-X.json are one file, but the collision check compared paths
  exactly, so a new id differing only by case could overwrite an
  unrelated file. Paths are now compared case-insensitively.
- A file that failed another check (say an undeclared geography) never
  reached the destination check, so a taken path only surfaced on the
  next run. Each file's destination is now worked out and checked
  before deciding to skip it, and all its problems are reported at once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Expected version change and release notes

🚨 WARNING: This PR is not expected to trigger a new version

To trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-975.westus2.2.azurestaticapps.net

Base automatically changed from feat/timeseries-v2-schema to epic/v2 October 6, 2026 16:47
@jacobvjk
jacobvjk merged commit 87bcf2f into epic/v2 Oct 6, 2026
10 checks passed
@jacobvjk
jacobvjk deleted the feat/timeseries-import branch October 6, 2026 16:48
@github-actions github-actions Bot mentioned this pull request Oct 6, 2026
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.

3 participants