Feat/metadata v2 schema - #898
Open
AlexAxthelm wants to merge 23 commits into
Open
Conversation
Foundation for epic #860 (best-effort / inheritance search): each of the 11 keyFeatures fields becomes an array of {sector, geography, value} entries so a pathway can hold different values for different parts of its coverage, and the #869 resolver can serve the most specific value for a search scope and fall back to broader ones. Also adds coreDrivers, dependencies, pathwayDescription and transitionAssessment. Additive only. v1 stays present and loadable, PathwayMetadataType still points at v1, and no data file changes here — the loader is repointed at v2 in a later commit once data carries the v2 $schema. Nothing consumes v2 yet.
Migrates the 4 ASEAN Centre for Energy and 3 IEA metadata files to pathwayMetadata.v2 via a new re-runnable codemod. src/data now holds 7 v2 and 49 v1 documents, which coexist because validateData routes each by its own $schema $id. Nothing reads v2 yet — the loader still points at v1, so the app is unchanged. All 7 resolve to a single widest-scope entry per keyFeature: cross-sector/South East Asia for ACE, cross-sector/Global for IEA. Both IEA files that carry pathwayOverview fold it into pathwayDescription as the lead paragraph; it has no readers in the app, so nothing observable moves. - scripts/codemod-v1-to-v2.ts splits v1's expertOverview into its three sections, wraps each keyFeature as one scoped entry, and scaffolds coreDrivers/dependencies. It skips files already on v2, so the remaining 49 are a re-run rather than a rewrite. A development tool only: there is no runtime v1 conversion, so un-migrated files simply will not load once the loader moves to v2. - The splitter accepts a bare line matching a section title as a heading. That exists for ACE-CNS-2024, whose "Core Drivers" heading lost its #### markers; without it, 1.5 KB of core-drivers prose folds into pathwayDescription and pushes it from 1204 to 2727 chars. - coreDrivers is scaffolded all-null per #858 rather than populated. The v1 "#### Core Drivers" prose does not map onto the 7 named fields mechanically: four paragraphs already exceed the 500-char cap, the italic labels ("Technology shifts", "Falling energy demand", "Economic growth") do not correspond 1:1 to the field names, and every section has unlabeled paragraphs with no destination. The codemod prints the prose it is not carrying so the hand-authoring ticket starts from the text. - transitionAssessment's maxLength goes 2500 -> 3000. 2500 was chosen for symmetry with pathwayDescription rather than measured; the longest section in the corpus is 2655 chars (ACE-RAS-2024), which made the codemod's own output invalid. pathwayDescription's 2500 is confirmed correct — the longest across all 56 files is 2459. Fixtures are added rather than converted, so the v1 fixtures stay v1 and the new coexistence tests can assert both halves. pathwayMetadata_v2_full carries several entries per field at different scopes, which #869 and #859 will need; _v2_minimal proves an all-empty keyFeatures document validates. One coexistence test documents a sharp edge deliberately: validateDataCollect filters entries to the single $id it is handed, so documents of the other version are dropped as neither valid nor invalid. That is what makes a mixed corpus work, and it is why repointing the loader has to report the count it skipped. Refs #858, #801. Co-Authored-By: Claude Opus 5 <[email protected]>
Points the loader at pathwayMetadata.v2 and moves every consumer onto the
scoped {sector, geography, value} shape. Only v2 documents load, so the app
now shows the 7 migrated ACE/IEA pathways; the 49 still on v1 are skipped
by $schema routing until they migrate.
That skip is silent by construction — validateDataCollect drops non-matching
documents as neither valid nor invalid — so pathwayMetadata.ts counts and
logs them. Without it, 49 missing pathways look like a data bug.
New src/utils/keyFeatureScope.ts answers "which entries apply to what the
user is looking at": containment on both axes, where cross-sector means the
union of the pathway's own declared sectors (not a universal match), and a
geography scope contains a query when the query's ISO set is a subset of the
entry's. Broader answers narrower, never the reverse. Deliberately no cost
model, no ranking, no fallback — that is #869, and it is what will turn a
non-match at the queried scope into a ranked broader-scope match rather than
an exclusion.
The emissionsTrajectory and policyAmbition facets now match like the sector
and metric facets — ANY/ALL over a value list, empty list meaning absent —
restricted to the entries whose scope contains the active sector/geography
selection. The two near-identical 30-line arms collapse into one helper.
concrete.includes(v) against an array is always false, so selecting either
returned zero pathways, and option building emitted "[object Object]".
Neither arm had any test coverage before — no filterPathways test passed
either filter — which is why the whole suite stayed green while both were
broken. Adding that coverage caught a regression that would otherwise have
shipped: in v1 a missing field contributed undefined, which
buildOptionsFromValues read as the absent bucket, but in v2 an empty entry
array contributes no elements, so the "None" option disappeared from both
dropdowns while the filter still honoured the token. Fixed with
withAbsentOption, matching how the sector facet does it.
Rendering keeps its current output. KeyFeatures reads through widestValue,
a deliberately provisional stand-in for #869's resolver: it picks the value
at the broadest declared scope, which reproduces v1 exactly for
codemod-migrated data (one entry, at its widest scope). #859 replaces it and
adds the badge naming the scope.
PathwayDetailPage renders pathwayDescription and transitionAssessment under
separate subheadings. v1's single expertOverview blob was three sections, so
rendering only the description would have visibly dropped the Application
to Transition Assessment text. The "Expert Overview" heading is left alone;
naming is #859's call.
Verified against the running app: 7 pathways load, the skip warning fires
without error, and all 11 key features on IEA-NZE render values matching the
source file — including the multi-select branch, which degrades silently
rather than throwing when handed the wrong shape.
Refs #858. Enables #869, #859.
Co-Authored-By: Claude Opus 5 <[email protected]>
Closes out #858's checklist. src/data/README.md described a format that no longer exists — and in the R example's case, one that never validated. Beyond the expected v1 leftovers (expertOverview, npm run json:check, a pbtar_schema.json link), the example used `name` as a bare string, the pre-#783 flat geography array, top-level publisher/publicationYear, and a `dataSource` field absent from every version of the schema. It would have failed against v1 as readily as against v2. Rewritten around v2: the two coexisting schema versions and the fact that only v2 documents are loaded, the scoped keyFeatures shape with its sentinels and the widest-scope rule, coreDrivers/dependencies/pathwayDescription/ transitionAssessment, the codemod for migrating an existing file, and the commands that actually exist. The R example is now verified rather than asserted: its blocks were extracted from this file, executed, and the resulting JSON validated against v2. Doing that corrected a wrong claim in an earlier draft — R's list() preserves NULL elements; the actual pitfall is jsonlite writing NULL as {}, which is why the helper passes null = "null". The validate_json R helper is dropped rather than repaired. The schema is split across common/*.json with absolute $refs that a single-URL jsonvalidate::json_validate() cannot resolve, so it documented a validation route that cannot succeed. Authors are pointed at npm run schema:check, which resolves the refs and additionally runs the cross-field scope checks that JSON Schema draft-07 cannot express. Tests: adds v2 counterparts to the existing v1 required-field cases — all 12 required fields, each of the 7 coreDrivers keys, unknown keys, and the dependencies enums. Also pins that pathwayDescription accepts null but not absence, the nullable-but-required distinction v2 relies on. The v1 REQ array still lists expertOverview on purpose: those cases validate v1 documents against v1, where it remains required. Finally, migrates the seven `keyFeatures: { emissionsTrajectory: "foo" }` stubs in ComparisonPage and PathwaySearch tests to the v2 shape. They kept their deliberately-invalid values, which exist to test degradation; the point is that a v1-shaped scalar in a v2 fixture silently exercises nothing. Refs #858. Co-Authored-By: Claude Opus 5 <[email protected]>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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). |
| src/utils/keyFeatureScope.ts | Adds helpers for reading scoped keyFeature entries (containment + widest-value fallback). |
| src/utils/keyFeatureScope.test.ts | Unit tests for scope containment, ISO resolution, and widest-value selection. |
| src/types/pathwayMetadata.v2.d.ts | Adds generated TS types for the v2 schema. |
| src/types/index.ts | Switches PathwayMetadataType to v2 and exports both v1/v2 types during migration. |
| src/types/common/scopeSector.v2.d.ts | Adds generated TS type for the v2 sector scope sentinel enum. |
| src/types/common/scopeGeography.v2.d.ts | Adds generated TS type for v2 geography scope (open string). |
| src/schema/pathwayMetadata.v2.test.ts | Adds schema self-guard tests to prevent keyFeatures wrapper/value drift. |
| src/schema/pathwayMetadata.v2.json | Introduces the v2 pathway metadata JSON Schema definition. |
| src/schema/common/scopeSector.v2.json | Adds schema for the v2 sector scope axis (sector names + cross-sector). |
| src/schema/common/scopeGeography.v2.json | Adds schema for the v2 geography scope axis (open string with basic guards). |
| src/schema/common/index.ts | Registers new v2 common schemas so AJV can resolve $refs. |
| src/pages/PathwaySearch.test.tsx | Updates integration test fixtures to use scoped v2 keyFeatures. |
| src/pages/PathwayDetailPage.tsx | Renders v2 pathwayDescription / transitionAssessment in place of v1 expertOverview. |
| src/pages/ComparisonPage.test.tsx | Updates comparison page test fixtures to use scoped v2 keyFeatures. |
| src/data/README.md | Updates contributor docs for v2 format, migration approach, and validation commands. |
| src/data/pathwayMetadata.ts | Switches loader to v2 schema and warns when v1 metadata files are skipped. |
| src/data/iea/IEA-STEPS-2024.json | Migrates this pathway metadata file to v2 structure. |
| src/data/iea/IEA-NZE-2024.json | Migrates this pathway metadata file to v2 structure. |
| src/data/iea/IEA-APS-2024.json | Migrates this pathway metadata file to v2 structure. |
| src/data/asean-centre-for-energy/ACE-RAS-2024.json | Migrates this pathway metadata file to v2 structure. |
| src/data/asean-centre-for-energy/ACE-CNS-2024.json | Migrates this pathway metadata file to v2 structure. |
| src/data/asean-centre-for-energy/ACE-BAS-2024.json | Migrates this pathway metadata file to v2 structure. |
| src/data/asean-centre-for-energy/ACE-ATS-2024.json | Migrates this pathway metadata file to v2 structure. |
| src/components/KeyFeatures.tsx | Updates rendering to read v2 scoped entries via a “widest value” helper. |
| src/components/KeyFeatures.test.tsx | Updates keyFeatures rendering tests to use v2 scoped-entry fixtures. |
| scripts/schema-check-files.ts | Adds a second-pass v2 scope reference check after AJV validation. |
| scripts/codemod-v1-to-v2.ts | Adds a codemod to migrate v1 metadata docs to v2 format. |
| scripts/codemod-v1-to-v2.test.ts | Adds tests for the codemod’s section splitting and scope selection logic. |
| public/schema/scopeSector.v2.html | Adds generated HTML documentation for scopeSector.v2.json. |
| public/schema/scopeGeography.v2.html | Adds generated HTML documentation for scopeGeography.v2.json. |
| PLAN.md | Adds an implementation/migration plan and rationale for v2 schema + rollout. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Expected version change and release notes:1.17.0-dev.1 (v1.16.1-dev.2...feat/metadata-v2-schema ) (2026-08-29T09:08 UTC)Features
Fixes
DocsOther
|
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-898.westus2.2.azurestaticapps.net |
Contributor
There was a problem hiding this comment.
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",
added 2 commits
August 19, 2026 13:50
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-898.westus2.2.azurestaticapps.net |
Contributor
There was a problem hiding this comment.
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-regionis treated as an always-allowed geography sentinel here (new Set([GLOBAL_SCOPE, CROSS_REGION])). But per the scopeGeography.v2 schema comment,cross-regionis reserved for non-global multi-region aggregates. Allowing it unconditionally means a v2 doc can passvalidateScopedEntrieswithgeography: "cross-region"even when the pathway isglobal: trueor 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).
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-898.westus2.2.azurestaticapps.net |
AlexAxthelm
marked this pull request as ready for review
August 19, 2026 15:37
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-898.westus2.2.azurestaticapps.net |
Contributor
There was a problem hiding this comment.
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 setsgeography.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 <[email protected]>
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-898.westus2.2.azurestaticapps.net |
Contributor
There was a problem hiding this comment.
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:
- line 85
- line 113
- line 128
emissionsTrajectory: [
{ sector: "cross-sector", geography: "DE", value: "foo" },
],
src/pages/ComparisonPage.test.tsx:31
- 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.
keyFeatures: {
emissionsTrajectory: [
{ sector: "cross-sector", geography: "Global", value: "foo" },
],
},
src/pages/PathwaySearch.test.tsx:131
- 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.
keyFeatures: {
emissionsTrajectory: [
{ sector: "cross-sector", geography: "DE", value: "bar" },
],
src/pages/ComparisonPage.test.tsx:52
- 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.
keyFeatures: {
emissionsTrajectory: [
{ sector: "cross-sector", geography: "DE", value: "bar" },
],
},
src/pages/PathwaySearch.test.tsx:88
- 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.
keyFeatures: {
emissionsTrajectory: [
{ sector: "cross-sector", geography: "DE", value: "foo" },
],
src/pages/PathwaySearch.test.tsx:116
- 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.
keyFeatures: {
emissionsTrajectory: [
{ sector: "cross-sector", geography: "JP", value: "bar" },
],
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-898.westus2.2.azurestaticapps.net |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 38 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
src/schema/pathwayMetadata.v2.json:333
- Missing punctuation in this description: "...go beyond currently legislated ones Scoped..." should include a period before "Scoped" for readability.
"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.",
…nment
Combining a region geography filter with emissionsTrajectory or policyAmbition
returned nothing for every region-scoped pathway. Verified on the real corpus:
geography="Southeast Asia" kept ACE-ATS-2024 on its own, but adding that
pathway's own emissionsTrajectory value ("Moderate increase") dropped it. All
four ACE pathways were affected — the entire region-scoped half of the loaded
corpus.
The geography axis required the query's ISO set to be a subset of the entry's.
That can essentially never hold, because the query vocabulary and each
publication's region membership are separately maintained lists (#783): the
filter token "Southeast Asia" carries 11 codes including TL, while ACE's own
"South East Asia" carries 10 and omits it. One country's difference was enough
to filter out every entry, leaving the field looking empty.
It also put this layer at odds with filterPathways' own geography arm, which has
always used overlap (isoSets.some(overlaps)). The two could therefore disagree
about whether the same pathway matched the same region — which is why the bug
only surfaced once a second filter was added.
geographyScopeContains is now geographyScopeOverlaps: a non-empty intersection
is a match. Renamed rather than changed behind the old name, since "contains"
would no longer be true. Two behaviours are deliberately preserved:
- "Global" stays a distinct predicate rather than expanding to every ISO code,
mirroring the facet's `wantGlobal && pGlobal`. Under plain overlap a Global
query would have matched every pathway.
- An empty ISO set — an unrecognised token, or a region the publication never
mapped — still matches nothing. Worth noting this needed care in the other
direction: containment over an empty set is vacuously true and required an
explicit size check, whereas overlap gets it right by construction.
The sector axis keeps containment. It compares against a closed enum shared by
every pathway, so there are no competing vocabularies to reconcile. Strict
containment is still the right primitive for #869, where it drives ranking
rather than acting as a hard filter.
Tests: one expectation genuinely inverts — an entry scoped to TH now answers a
"Southeast Asia" query — so that case was rewritten rather than deleted, since
the old assertion encoded the bug. Added coverage for the two ways the
vocabularies diverge (a publication label overlapping the query token, and the
same label used as a query token matching nothing, because only "Southeast Asia"
is in the filter vocabulary), plus a filterPathways-level assertion that the
geography facet alone and the combined filter agree. That divergence was the
defect, so it is now pinned directly.
Found during a review pass over the PR.
Co-Authored-By: Claude Opus 5 <[email protected]>
Two entries at the same (sector, geography) carrying different values validated
cleanly, then disagreed downstream. The schema's `uniqueItems` compares whole
entries, so it only catches byte-identical duplicates — two entries differing
solely in `value` are "unique" to the schema while being contradictory as data.
Verified before the fix: a document with cross-sector/Global set to both
"Significant decrease" and "Minor increase" passed npm run schema:check, after
which widestValue returned the first by document order (so the detail page showed
only that one) while valuesInScope returned both (so search matched the pathway
under a value its own page does not display).
validateScopedEntries now enforces one value per scope, per keyFeatures field,
and names the entry a repeat collides with so the fix is obvious in a long list.
Every repeat is reported and all point back at the first occurrence, rather than
chaining 1->2, so a three-way collision reads as one problem.
Rejecting rather than resolving is deliberate. Choosing a winner by document
order would silently discard authored content, and the likely intent — an
override of a broader scope — is not something the data can express. Better to
fail loudly than to guess.
The composite key separates its parts with \\u0000 so that ("Power", "SG") cannot
collide with ("Power SG", ...); a test pins that, since a plain join would make
the check quietly wrong for region labels containing the sector name.
No effect on the current corpus — all 81 documents still validate. This matters
now because hand-authoring finer scopes is the immediate next step, and adding a
narrower override is exactly the edit that would have produced this.
Found during a review pass over the PR.
Co-Authored-By: Claude Opus 5 <[email protected]>
The un-migrated-file count was an unconditional module-scope console.warn, so a production deploy printed "49 metadata file(s) still use schema v1 ... Migrate them with scripts/codemod-v1-to-v2.ts (#858)" to every visitor's console — naming an internal script and issue number. It also broke this module's own convention: assembleData logs only when a caller opts in via opts.warn, which this one does not. Gated on a new isViteDev() in loadData.ts, keyed off import.meta.env.DEV, which Vite statically replaces with false in production builds. isViteDev reuses an extracted readViteEnv() rather than re-implementing the env read, which is awkward enough to be worth having once: a try/catch around import.meta (typeof import breaks esbuild) plus a globalThis shim fallback. decideIncludeInvalid now shares it, with behaviour unchanged. It could not simply reuse decideIncludeInvalid, which answers a different question and returns false in dev unless VITE_INCLUDE_INVALID is set. Tests exercise the production path via vi.stubEnv("DEV", false) rather than the globalThis shim: vitest supplies a real import.meta.env, so readViteEnv prefers it and the shim is never consulted. Found during a review pass over the PR. Co-Authored-By: Claude Opus 5 <[email protected]>
Three small fixes from Copilot's review pass. pathwayType's description read "Type of the pathway pathway." — inherited verbatim from v1, which the v2 generator copied. Fixed in both schemas rather than only v2, since leaving them divergent over a two-word correction helps nobody. policyAmbition's v2 description ran two sentences together: "...beyond currently legislated ones Scoped: see keyFeatures." Its v1 text is the only one of the eleven keyFeature descriptions that does not end in a period, so appending the scope note produced a run-on. Copilot flagged the line as another duplicated word; it is a missing period, and policyAmbition is the only field affected. schema-check-files.ts built GitHub annotation paths with join(r.dir, p.name), but p.name already carries the directory — getJsonFilesRecursive constructs it with join(base, d.name) — so annotations pointed at "src/data/src/data/foo.json" and resolved to nothing. Pre-existing, and not specific to the scope-check failures Copilot attributed it to: it affected every invalid file equally. It stayed hidden because it only surfaces when validation actually fails, which CI rarely sees. Verified by corrupting a file and confirming the emitted path resolves. Generated types and docs regenerated; the diffs are description-only. Co-Authored-By: Claude Opus 5 <[email protected]>
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-898.westus2.2.azurestaticapps.net |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 43 changed files in this pull request and generated no new comments.
Suppressed comments (6)
Previously missed (2) — in code that hasn't changed since the last review.
src/pages/PathwaySearch.test.tsx:72
- The fixture uses an
emissionsTrajectoryvalue ("foo") that is not part of the v2 schema enum. InKeyFeatures, unknown values are treated as "No information", so this fixture can silently exercise a different UI path than intended and makes the test less representative of real data.
This issue also appears in the following locations of the same file:
- line 85
- line 113
- line 128
keyFeatures: {
emissionsTrajectory: [
{ sector: "cross-sector", geography: "DE", value: "foo" },
],
},
src/pages/ComparisonPage.test.tsx:31
- The comparison fixtures use
emissionsTrajectoryvalues ("foo") that are not valid v2 enum members. In the UI, that will be treated as an unknown token and can render as "No information", making the fixture less realistic.
This issue also appears on line 48 of the same file.
keyFeatures: {
emissionsTrajectory: [
{ sector: "cross-sector", geography: "Global", value: "foo" },
],
},
src/pages/PathwaySearch.test.tsx:89
- This fixture uses an
emissionsTrajectoryvalue ("foo") that is outside the schema’s allowed values. Using a real enum member avoids tests passing while the UI would render the value as "No information" (unknown token).
keyFeatures: {
emissionsTrajectory: [
{ sector: "cross-sector", geography: "DE", value: "foo" },
],
},
src/pages/PathwaySearch.test.tsx:117
- This fixture uses an
emissionsTrajectoryvalue ("bar") that is not a valid v2 enum member, which can cause the component layer to treat it as "No information" and hide regressions tied to real tokens.
keyFeatures: {
emissionsTrajectory: [
{ sector: "cross-sector", geography: "JP", value: "bar" },
],
},
src/pages/PathwaySearch.test.tsx:132
- This fixture uses an invalid
emissionsTrajectorytoken ("bar"). Switching to a schema-valid value keeps the test aligned with the production dataset and avoids accidental fallbacks to "No information" in rendering.
keyFeatures: {
emissionsTrajectory: [
{ sector: "cross-sector", geography: "DE", value: "bar" },
],
},
src/pages/ComparisonPage.test.tsx:52
- This fixture uses an invalid
emissionsTrajectoryvalue ("bar"). Using a schema-valid token helps keep the test aligned with v2 behavior and avoids masking issues in code paths that expect known values.
keyFeatures: {
emissionsTrajectory: [
{ sector: "cross-sector", geography: "DE", value: "bar" },
],
},
Any of the 31 technologies in technology.v1.json was legal under any of the 15 sectors in sector.v1.json: 465 pairings, of which the data uses 37. Jacob flagged the resulting generated type on #898 as "a bit of a random list ... not scoped to a sector". This constrains it. Enforced in validateScopedEntries rather than in the schema. #461 suggests mirroring the if/then sector conditional from pathwayTimeseries.v1.json, but that keyword pair defeats json-schema-to-typescript: the timeseries `data` items use exactly that shape and generate as `{ [k: string]: unknown }[]`. Applying it to sectors.items would collapse the `{ name; technologies }` object type and break every consumer of Sector, making the complaint worse rather than better. scopeSector.v2.json already documents the same tradeoff for the cross-sector constraint. Closed by default: a sector with no definition in timeseriesTaxonomy.ts accepts only an empty list. Letting undefined sectors through would mean the next data round populates technologies for a new sector and nothing checks them, which is the failure this exists to prevent. The error names the sector and says where to add its list. Only Power is populated, from the ten technologies POWER_SECTOR_DEFINITION already carries, and the allowlist is derived from it rather than duplicated so adding a sector is a single edit. The other 14 sectors' lists are content for the data round, not this PR. technologyBelongsToSector is tri-state ("yes" | "no" | "unknown") rather than boolean: a boolean would answer false for the 14 undefined sectors, indistinguishable from a real rejection. Validation treats unknown as a failure, while #869's technology axis can treat it as a match instead of silently dropping every non-Power pathway. The codemod reports offenders and leaves them in place. Deleting a technology someone recorded on purpose, or inventing a sector's taxonomy, would both be worse than a line in the report plus a refusal from schema:check. One fixture line changes: pathwayMetadata_v2_full.json gave Steel ["Hydrogen Use"], the only document in the repo the rule rejects. All 72 files in src/data pass untouched, Power being the only sector any of them populates. schema:check reports 81 valid / 0 invalid; 668 tests pass, up from 634. tsc --build goes from 245 to 247 errors, both the TS5097 forced by the required .ts import extension — loadData.ts:1 carries the identical error for the identical reason, and tsc is not a CI gate. Co-Authored-By: Claude Opus 5 <[email protected]>
Data availability has been derived, not authored: availabilityFor in searchUtils.ts guesses "Download" | "Link" | "Unavailable" from whether the timeseries index has an entry for the pathway and whether any publication link is described as "data". One coarse value per pathway, which cannot answer the question users actually have -- for this metric, in this sector, at what resolution, and can I get it without paying? This adds the stored form and its validation. The table UI is not here: it is specified to sit under the #872 filter bar, which is still open, and the example values are still pending. Landing the schema first unblocks the content authoring, which is the critical path. availabilityFor is left untouched -- it backs a live search facet, and retiring it while dataAvailability is authored for 0 of 72 files would regress search for every pathway. dataAvailability is optional, so existing files stay valid and authoring is incremental: { overall, byMetric[] }, one row per (metricName, sector, sectorSegment, geography). The object wrapper exists because #870's "Overall" row is a summary of the hosted timeseries file plus anything that does not fit the per-metric rows, and the array has nowhere to put free text. Rows carry a scopeGeography.v2 token, the same one keyFeatures uses, so entriesInScope/geographyScopeOverlaps can scope the table when the UI lands. #870's field list has only geographyCoverage, which is a coverage class rather than a scope, leaving the "respects the #872 selection" criterion with no field to match on. Two new common schemas: sectorSegment.v1.json, and dataAvailability.v1.json for the vocabularies with no other home. dataFormat splits into dataFormat + access rather than #870's single enum with parenthetical free|paywall -- the acceptance criteria want the tables/text and free/paywall indicators as separate signals, and a flat five-member enum would make the table string-match to recover them. Most of the work was already done by #461. metricName, sectorSegment and granularity are all sector-conditional, so timeseriesTaxonomy's technology helpers generalize to three axes behind vocabularyFor and membership; granularity reuses technologyBelongsToSector outright, since technology list. The tri-state is now TaxonomyMembership, with TechnologyMembership kept as an alias so #461's call sites do not churn. Eight cross-field checks join their siblings in validateScopedEntries, gated by schema:check and CI: declared sector, resolvable geography, metric the pathway reports, metric of the sector, segment of the sector, granularity of the sector, access null iff dataFormat is "In tool", and one row per scope. That last one matters because uniqueItems compares whole entries, so two rows agreeing on the scope and disagreeing on everything else validate cleanly and then have one cell to render in. Two things the plan got wrong, both found by building it: The metric axis is open rather than closed by default. Technologies and segments have no other constraint, so closing them is the only thing between a typo and production. metricName already has one -- it must appear in the pathway's own `metric` array -- so closing it too would add no safety while making dataAvailability unauthorable for the fourteen sectors whose metrics nobody has defined, which is the blockage UNSEGMENTED exists to avoid. Segments and granularity stay closed. A sector defining an *empty* vocabulary produced "(allowed: )", which reads as a bug in the checker rather than an answer. Now "-- that sector defines no metrics.", via a shared allowedClause helper covering all three cases; #461's sectors[].technologies message had the same defect and is fixed with it. Note the sector/metric check cannot currently reject real data: Power defines all five members of metric.v1.json's enum and every other sector is "unknown". It goes live when a second sector defines metrics. Tested directly, with a test pinning the reason. schema:check stays 81 valid / 0 invalid; 718 tests pass, up from 668; schema:generate is idempotent; tsc --build holds at its 247-error baseline. Each of the eight rules was confirmed to reject a broken copy of the fixture with its own message naming the offender. Co-Authored-By: Claude Opus 5 <[email protected]>
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-898.westus2.2.azurestaticapps.net |
| @@ -0,0 +1 @@ | |||
| {"root":["./src/app.test.tsx","./src/app.tsx","./src/main.tsx","./src/components/additionalinfobox.tsx","./src/components/badge.test.tsx","./src/components/badge.tsx","./src/components/badgearray.test.tsx","./src/components/badgearray.tsx","./src/components/colophon.tsx","./src/components/comparisonkeyfeatures.tsx","./src/components/comparisonplots.test.tsx","./src/components/comparisonplots.tsx","./src/components/comparisonribbon.test.tsx","./src/components/comparisonribbon.tsx","./src/components/donutchart.tsx","./src/components/downloaddataset.test.tsx","./src/components/downloaddataset.tsx","./src/components/dropdownfacetshell.test.tsx","./src/components/dropdownfacetshell.tsx","./src/components/environmentbanner.tsx","./src/components/footer.test.tsx","./src/components/footer.tsx","./src/components/header.test.tsx","./src/components/header.tsx","./src/components/headernav.test.tsx","./src/components/headernav.tsx","./src/components/highlightedtext.test.tsx","./src/components/highlightedtext.tsx","./src/components/keyfeatures.test.tsx","./src/components/keyfeatures.tsx","./src/components/markdown.test.tsx","./src/components/markdown.tsx","./src/components/multilinechart.tsx","./src/components/multiselectdropdown.test.tsx","./src/components/multiselectdropdown.tsx","./src/components/neutralscale.test.tsx","./src/components/neutralscale.tsx","./src/components/normalizedstackedareachart.tsx","./src/components/numericrange.test.tsx","./src/components/numericrange.tsx","./src/components/numericrangedropdown.test.tsx","./src/components/numericrangedropdown.tsx","./src/components/numericrangeslider.tsx","./src/components/pathwaycard.test.tsx","./src/components/pathwaycard.tsx","./src/components/plotselector.test.tsx","./src/components/plotselector.tsx","./src/components/publicationblock.tsx","./src/components/radarchart.tsx","./src/components/regionmemberstooltip.test.tsx","./src/components/regionmemberstooltip.tsx","./src/components/resourcesdropdown.test.tsx","./src/components/resourcesdropdown.tsx","./src/components/searchbox.tsx","./src/components/searchsection.test.tsx","./src/components/searchsection.tsx","./src/components/sentimentscale.test.tsx","./src/components/sentimentscale.tsx","./src/components/stepbystepguide.tsx","./src/components/steppage.tsx","./src/components/steppagenumericrange.tsx","./src/components/steppageremap.tsx","./src/components/textwithtooltip.test.tsx","./src/components/textwithtooltip.tsx","./src/context/comparisoncontext.test.tsx","./src/context/comparisoncontext.tsx","./src/context/filtercontext.test.tsx","./src/context/filtercontext.tsx","./src/data/index.gen.ts","./src/data/pathwaymetadata.ts","./src/pages/comparisonpage.disclaimer.test.tsx","./src/pages/comparisonpage.test.tsx","./src/pages/comparisonpage.tsx","./src/pages/contactpage.tsx","./src/pages/landingpage.test.tsx","./src/pages/landingpage.tsx","./src/pages/legalpage.tsx","./src/pages/pathwaydetailpage.disclaimer.test.tsx","./src/pages/pathwaydetailpage.test.tsx","./src/pages/pathwaydetailpage.tsx","./src/pages/pathwaysearch.test.tsx","./src/pages/pathwaysearch.tsx","./src/pages/resources/resourcesfaqpage.tsx","./src/pages/resources/resourceshowtochooseapathwaypage.tsx","./src/pages/resources/resourcesmethodologypage.test.tsx","./src/pages/resources/resourcesmethodologypage.tsx","./src/pages/resources/resourcesupdatespage.tsx","./src/pages/resources/resourcesusecasespage.tsx","./src/pages/resources/index.ts","./src/schema/pathwaymetadata.v2.test.ts","./src/schema/common/index.ts","./src/test/failonreactwarnings.ts","./src/test/setup.ts","./src/types/index.ts","./src/types/pathwaymetadata.v1.d.ts","./src/types/pathwaymetadata.v2.d.ts","./src/types/pathwaytimeseries.v1.d.ts","./src/types/vite-env.d.ts","./src/types/common/countrycode.v1.d.ts","./src/types/common/emissionsscope.v1.d.ts","./src/types/common/geography.v1.d.ts","./src/types/common/geographyitem.v1.d.ts","./src/types/common/label.v1.d.ts","./src/types/common/metric.v1.d.ts","./src/types/common/publication.v1.d.ts","./src/types/common/scopegeography.v2.d.ts","./src/types/common/scopesector.v2.d.ts","./src/types/common/sector.v1.d.ts","./src/types/common/technology.v1.d.ts","./src/utils/numericrangelimits.ts","./src/utils/absent.test.ts","./src/utils/absent.ts","./src/utils/capitalizewords.tsx","./src/utils/charttooltiplayout.test.ts","./src/utils/charttooltiplayout.ts","./src/utils/facets.test.ts","./src/utils/facets.ts","./src/utils/filterregions.test.ts","./src/utils/filterregions.ts","./src/utils/geographyutils.test.tsx","./src/utils/geographyutils.ts","./src/utils/gettemperaturecolor.ts","./src/utils/keyfeaturescope.test.ts","./src/utils/keyfeaturescope.ts","./src/utils/loaddata.test.tsx","./src/utils/loaddata.ts","./src/utils/normalizeabsent.test.ts","./src/utils/normalizeabsent.ts","./src/utils/searchutils.scopedfacets.test.ts","./src/utils/searchutils.test.tsx","./src/utils/searchutils.ts","./src/utils/sortutils.test.tsx","./src/utils/sortutils.ts","./src/utils/timeseriesavailability.test.ts","./src/utils/timeseriesavailability.ts","./src/utils/timeseriesindex.test.ts","./src/utils/timeseriesindex.ts","./src/utils/timeseriestaxonomy.test.ts","./src/utils/timeseriestaxonomy.ts","./src/utils/tooltiputils.test.tsx","./src/utils/tooltiputils.ts","./src/utils/validatedata.test.tsx","./src/utils/validatedata.ts","./src/utils/validatescopes.test.ts","./src/utils/validatescopes.ts"],"errors":true,"version":"6.0.3"} | |||
Comment on lines
+294
to
+298
| allowedClause([...(defined ?? []), UNSEGMENTED], "segments") + | ||
| (defined | ||
| ? "" | ||
| : ` No segments are defined for that sector; add them to` + | ||
| ` SECTORS_BY_KEY in src/utils/timeseriesTaxonomy.ts.`), |
Comment on lines
261
to
263
| <h2 className="text-xl font-semibold text-rmigray-800 mb-3"> | ||
| Expert Overview | ||
| </h2> |
Comment on lines
+68
to
+72
| keyFeatures: { | ||
| emissionsTrajectory: [ | ||
| { sector: "cross-sector", geography: "DE", value: "foo" }, | ||
| ], | ||
| }, |
Draft
One conflict: public/schema/pathwayMetadata.v1.html, deleted on main and modified here. Resolved in main's favour by removing the whole of public/schema -- b372e37 ("docs: remove unused JSON Schema HTML docs") dropped the generated HTML reference, its generator script and the schema:generate:docs wiring, so regenerating those files is no longer part of the build. The merge left five behind because this branch added rather than modified them (pathwayMetadata.v2, scopeGeography.v2, scopeSector.v2, and #870's dataAvailability.v1 and sectorSegment.v1), so main's deletion did not cover them. Removed too, rather than shipping generated docs that nothing regenerates. Also drops this branch's src/data/README.md sentence pointing at the HTML reference in public/schema/, which would otherwise document a directory that no longer exists. Co-Authored-By: Claude Opus 5 <[email protected]>
| continue; | ||
| } | ||
|
|
||
| const { scope, coreDriversProse, droppedPathwayOverview } = result; |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-898.westus2.2.azurestaticapps.net |
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
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.
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,dependencies,pathwayDescriptionandtransitionAssessmentfields.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.