You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
the files in src/schema have useful docstrings that you may want to look at in particular
includes v1 to v2 script, will get removed when we load in the new data from the working excel file (stacked PR)
At this point, this is a "plumbing" PR. I'm intending that this will have the UI one, and one for a better import of the touched data files (not just placeholder values), extended on top of it and then we can merge that whole block as one (using the new stacks feature on GH)
Summary
Introduces pathwayMetadata.v2 — the foundation for epic #860 (best-effort /
inheritance search). Each of the 11 keyFeatures becomes an array of {sector, geography, value} entries so a pathway can hold different values for
different parts of its coverage, plus new coreDrivers, dependencies and pathwayDescription fields, and an optional dataAvailability section. There is
no transitionAssessment field: v1's "Application to Transition Assessment"
section is retired, as src/data/README.md records.
Scope was deliberately narrowed from the ticket: 7 of 56 data files (4 ACE,
3 IEA) are migrated here; the other 49 follow in a separate PR. v1 and v2 coexist
via $schema$id routing, but only v2 documents are loaded, so the app
currently shows 7 pathways and logs how many were skipped.
Reviewing this
The diff is ~27.6k added lines, but 23.4k of that is generated — please skim
rather than read:
src/utils/keyFeatureScope.ts — scope containment for search
src/utils/validateScopes.ts — the cross-field check draft-07 can't express
scripts/codemod-v1-to-v2.ts — the v1→v2 migration
src/utils/searchUtils.ts — the two facets that had to change
Known gaps, deliberately left
coreDrivers is scaffolded all-null. The v1 "Core Drivers" prose doesn't map
onto the 7 named fields mechanically (4 paragraphs exceed the 500-char cap, the
labels don't correspond 1:1). Consequence: those 7 detail pages show ~1 KB less
text than before. Needs an authoring pass.
Scope-aware behaviour isn't visible yet. Every migrated pathway has exactly one
entry at its widest scope, so keyFeatureScope never actually narrows. The tests
are the only place the mechanism is currently observable.
609 unit tests pass (193 new), incl. new coverage for the two keyFeature-backed
search facets, which previously had none — that's why the v1→v2 shape change
broke them silently.
Codemod verified by dry-run then applied; npm run schema:check validates all 81
documents including the cross-field scope checks.
Manually verified in the running app: 7 pathways load, all 11 key features render
values matching the source files, both new prose fields display.
Checklist
PR is focused on a single concern
Tests pass locally and in CI
Docs updated for user-visible changes (src/data/README.md rewritten for v2)
AI-assisted portions declared — the large majority of this PR was written with
Claude Code, reviewed commit-by-commit.
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces pathway metadata schema v2 (scoped keyFeatures entries plus coreDrivers / dependencies / pathwayDescription), migrates a first slice of pathway JSON to v2, and updates validation, search, and rendering so the app can operate on v2-shaped data.
Changes:
Add pathwayMetadata.v2.json + v2 scope subschemas, generated TS types, and generated HTML schema docs.
Add cross-field validation for scoped entries (sector/geography references) and update search facets + rendering to handle scoped keyFeature values.
Migrate fixtures + a subset of real data files to v2, and add a codemod + tests to support migrating the remaining corpus.
Reviewed changes
Copilot reviewed 35 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
File
Description
testdata/valid/pathwayMetadata_v2_minimal.json
Adds a minimal v2-valid fixture for schema/validator coverage.
testdata/valid/pathwayMetadata_v2_full.json
Adds a comprehensive v2 fixture exercising scoped keyFeatures and new fields.
src/utils/validateScopes.ts
Implements cross-field scope reference validation for v2 (sector/geography must be declared).
src/utils/validateScopes.test.ts
Unit tests for scoped entry validation behavior and error reporting.
src/utils/validateData.test.tsx
Extends validation tests to cover v2 routing/coexistence and v2 required fields.
src/utils/searchUtils.ts
Updates facet option building and filtering to support scoped keyFeature entries.
src/utils/searchUtils.scopedFacets.test.ts
Adds targeted test coverage for scoped facets (emissionsTrajectory, policyAmbition).
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 35 out of 39 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/schema/pathwayMetadata.v2.json:37
Schema description has a duplicated word: "Type of the pathway pathway." This will propagate into generated TypeScript types and the HTML schema docs on the next regen, so it’s worth correcting in the source schema.
"pathwayType": {
"description": "Type of the pathway pathway.",
"type": "string",
"enum": ["Normative", "Exploratory", "Predictive"]
src/schema/pathwayMetadata.v2.json:340
The policyAmbition field description is missing punctuation/wording between “ones” and “Scoped”, reading “… beyond currently legislated ones Scoped: …”. This is grammatically incorrect and will flow into generated types/docs.
"policyAmbition": {
"description": "Represents the overall stringency and intent of modeled policies relative to climate targets, often reflecting if and how far the included policies go beyond currently legislated ones Scoped: see keyFeatures.",
"type": "array",
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 34 out of 38 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/utils/validateScopes.ts:55
cross-region is treated as an always-allowed geography sentinel here (new Set([GLOBAL_SCOPE, CROSS_REGION])). But per the scopeGeography.v2 schema comment, cross-region is reserved for non-global multi-region aggregates. Allowing it unconditionally means a v2 doc can pass validateScopedEntries with geography: "cross-region" even when the pathway is global: true or has no declared ISO coverage; in that case the app-side resolver (keyFeatureScope.entryISOSet) resolves it to the pathway’s ISO coverage (often empty for global-only pathways) and the entry matches nothing.
Consider only allowing cross-region when the pathway is not global and declares more than one region/country (i.e. when it can actually represent a meaningful aggregate scope).
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 34 out of 38 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/utils/searchUtils.scopedFacets.test.ts:41
This test fixture uses scoped entries with geography: "Global", but the v2 cross-field validator (validateScopedEntries) only allows "Global" when the pathway sets geography.global: true. Marking the fixture as global keeps tests aligned with the v2 authoring/validation rules.
Selecting Sector=None alongside emissionsTrajectory or policyAmbition returned
no pathways, even when a matching pathway existed. A pathway with no sectors
holding "Significant decrease" at cross-sector/Global matched Sector=None on its
own, but adding the keyFeature facet dropped it.
The ABSENT/"None" token was reaching entriesInScope as though it were a scope.
geographyScopeContains already ignored it, but sectorScopeContains compared it as
a sector name, so no entry could ever match and valuesInScope came back empty —
making the pathway look as if it held no values at any scope.
The underlying mistake was treating the None bucket as a scope at all. It is a
predicate about the pathway ("has no sectors" / "has no geography"), so it must
not constrain which scope a value is read from. Fixed by stripping the token from
both axes in entriesInScope rather than patching only the sector helper, so the
two cannot disagree again; the guard inside geographyScopeContains stays for
direct callers and is commented as such.
No user-visible impact today: all seven currently-loaded pathways declare
sectors, so nothing in the corpus could reach the broken path. It would have
surfaced as soon as a sector-less pathway landed.
Tests assert the symmetry rather than just the reported case, since the
asymmetry is what caused the bug: the token is ignored on the sector axis, on
the geography axis, and on both together; concrete tokens still narrow when
combined with None, so the axis is not merely switched off; and the end-to-end
filterPathways case is pinned, including that a value the pathway does not hold
still correctly matches nothing.
Reported by Copilot on PR #898.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 34 out of 38 changed files in this pull request and generated no new comments.
Suppressed comments (8)
Previously missed (4) — in code that hasn't changed since the last review.
src/schema/pathwayMetadata.v2.json:35
Schema description has a duplicated word ("pathway pathway"), which will also propagate into generated type docs. Consider fixing the wording to avoid confusing schema readers.
"description": "Type of the pathway pathway.",
src/schema/pathwayMetadata.v2.json:91
Grammar in this schema description is off ("so merging them read" -> "so merging them reads"), and the text is used in generated docs/types. Fixing it here keeps downstream docs clean.
"description": "Narrative description of the pathway. Replaces v1's expertOverview: in the v1 corpus this is the '#### Pathway Description' section of it. v1's separate pathwayOverview field is retired without replacement, not merged in here -- the two texts restate each other, so merging them read as immediate self-repetition. null means no description is available.",
src/pages/PathwaySearch.test.tsx:71
This test fixture uses an emissionsTrajectory value ("foo") that is not one of the schema's allowed enum values. Using a real enum value keeps fixtures representative and avoids hiding issues in code that relies on known keyFeature values.
This issue also appears in the following locations of the same file:
This test fixture uses an emissionsTrajectory value ("foo") that is not one of the schema's allowed enum values. Using a real enum value keeps fixtures representative and avoids hiding issues in code that relies on known keyFeature values.
This issue also appears on line 48 of the same file.
This test fixture uses an emissionsTrajectory value ("bar") that is not one of the schema's allowed enum values. Using a real enum value keeps fixtures representative and avoids hiding issues in code that relies on known keyFeature values.
This test fixture uses an emissionsTrajectory value ("bar") that is not one of the schema's allowed enum values. Using a real enum value keeps fixtures representative and avoids hiding issues in code that relies on known keyFeature values.
This test fixture uses an emissionsTrajectory value ("foo") that is not one of the schema's allowed enum values. Using a real enum value keeps fixtures representative and avoids hiding issues in code that relies on known keyFeature values.
This test fixture uses an emissionsTrajectory value ("bar") that is not one of the schema's allowed enum values. Using a real enum value keeps fixtures representative and avoids hiding issues in code that relies on known keyFeature values.
metricDisplayNames indexed `sectorDef.metrics[metricKey]` unguarded. That was
safe while every SECTORS_BY_KEY entry carried `metrics`; the #858 pass made the
axis optional and added Steel and Aviation without it, for their
data-availability vocabularies. Before, an unknown sector key hit
`if (!sectorDef) continue`; now it resolves, and the next line throws.
Latent -- all 16 timeseries files are Power-only -- but it would have broken the
detail page the day a Steel series landed. The test reproduces the TypeError
without the guard.
`tsc` reported it (TS18048). Nothing in CI runs `tsc`, which is how it got
through.
Refs #898
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
geographyScopeOverlaps documents that an unrecognised token "matches nothing
rather than everything", but its Global-entry shortcut ran first and answered
one anyway. An unknown token resolves to an empty ISO set; that is now checked
before the shortcut. No visible effect today -- the pathway-level geography
facet excludes such a pathway before keyFeature matching runs -- but the
function now does what its comment says. The existing test of this case only
tried a non-Global entry.
Also makes a PathwaySearch fixture consistent: a pathway declaring no geography
named `DE` in a keyFeature entry, a state validateScopedEntries would reject.
It now carries `across regions`, the only token such an entry could name.
Both from the Copilot review on #898.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…red into
It was destructured and then read off `result` again two lines later, which
code scanning flagged as an unused variable on #898.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
scopeGeography.v2 rejected every three-letter token with a `not` clause, while
geography.v1 accepts any non-blank region key. So a pathway could declare a
region called `SSA` and then never scope a key feature or an availability row
to it -- AJV refused the token before the cross-field check ran, with the
unhelpful message "must NOT be valid".
The cookbook keeps a multi-country region's label exactly as the publication
writes it, and model regions are often three letters: REMIND, which NGFS
publishes from, uses SSA, LAM, EUR, MEA and others. Nothing in the current data
or the workbook uses one yet, so the first such import would have failed on
legitimate data.
The clause also guarded nothing. Its plausible purpose -- keeping an ISO
alpha-3 code like `THA` out where `TH` belongs -- is already served by
validateScopedEntries, which allows only declared region labels, their alpha-2
members, declared countries, `Global` and `across regions`. A test pins that
an undeclared `THA` is still rejected there.
From the Copilot review on #898.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This only permits Global as a key-feature/data-availability scope when geography.global is true. However, Global is the widest value sentinel, and the inheritance contract explicitly allows a pathway with narrower coverage to resolve a Global-scoped value; coverage filtering separately prevents a global-only pathway from matching a sub-global query. As written, a valid regional/country pathway with a Global value is rejected by schema:check, preventing that fallback case. Add Global to the allowed scope tokens independently of the pathway's coverage flag.
@AlexAxthelm I updated the schema so it is in sync with the cookbook. It also now covers all pathway level components and variables, including data availability
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
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.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
jacobvjk
left a comment
Alex's TL;DR here, Claude's summary below:
publicchanges (generated docs)src/schemahave useful docstrings that you may want to look at in particularAt this point, this is a "plumbing" PR. I'm intending that this will have the UI one, and one for a better import of the touched data files (not just placeholder values), extended on top of it and then we can merge that whole block as one (using the new stacks feature on GH)
Summary
Introduces
pathwayMetadata.v2— the foundation for epic #860 (best-effort /inheritance search). Each of the 11
keyFeaturesbecomes an array of{sector, geography, value}entries so a pathway can hold different values fordifferent parts of its coverage, plus new
coreDrivers,dependenciesandpathwayDescriptionfields, and an optionaldataAvailabilitysection. There isno
transitionAssessmentfield: v1's "Application to Transition Assessment"section is retired, as
src/data/README.mdrecords.Scope was deliberately narrowed from the ticket: 7 of 56 data files (4 ACE,
3 IEA) are migrated here; the other 49 follow in a separate PR. v1 and v2 coexist
via
$schema$idrouting, but only v2 documents are loaded, so the appcurrently shows 7 pathways and logs how many were skipped.
Reviewing this
The diff is ~27.6k added lines, but 23.4k of that is generated — please skim
rather than read:
.d.tsThe parts worth real attention:
src/schema/pathwayMetadata.v2.json+common/scope{Sector,Geography}.v2.jsonsrc/utils/keyFeatureScope.ts— scope containment for searchsrc/utils/validateScopes.ts— the cross-field check draft-07 can't expressscripts/codemod-v1-to-v2.ts— the v1→v2 migrationsrc/utils/searchUtils.ts— the two facets that had to changeKnown gaps, deliberately left
coreDriversis scaffolded all-null. The v1 "Core Drivers" prose doesn't maponto the 7 named fields mechanically (4 paragraphs exceed the 500-char cap, the
labels don't correspond 1:1). Consequence: those 7 detail pages show ~1 KB less
text than before. Needs an authoring pass.
entry at its widest scope, so
keyFeatureScopenever actually narrows. The testsare the only place the mechanism is currently observable.
Related issues
Refs: #858, #801 · Enables: #869, #859
Testing
search facets, which previously had none — that's why the v1→v2 shape change
broke them silently.
npm run schema:checkvalidates all 81documents including the cross-field scope checks.
values matching the source files, both new prose fields display.
Checklist
src/data/README.mdrewritten for v2)Claude Code, reviewed commit-by-commit.