Repository navigation
feat(key-features): forbidden key-feature pairs carry no value (Electrification · Power) - #1045
Merged
Merged
Conversation
A Power-only pathway (JETP-CIPP-2023, CGS-IESR, TZ, SDSN LA/TH/MM) has no across sectors row for Electrification to inherit. The resolver ran out of scopes and the page said "No information" with "Nothing is recorded for this pathway at any scope", as if the pathway could have said something there and didn't. Electrification of Power cannot exist, so the page now says that. The forbidden field x sector pairs are declared once again, in keyFeatureRules, and the resolver reads the declaration rather than a "Not Applicable" cell. A forbidden pair is to carry no value at all. The validator change that enforces this follows separately. * restore FORBIDDEN_KEY_FEATURE_SECTORS / isForbiddenKeyFeatureSector (removed in 693e3a7) and drop MEASURE_NOT_APPLICABLE * resolver: leave a forbidden sector out of the chain without reading it, and set notApplicableSector on unavailable results as well as resolved * KeyFeatures: an unavailable result with notApplicableSector shows a "Not applicable" badge, hover "<Feature> does not apply to the <Sector> sector."; "No information" keeps its meaning * scopeAvailability: ignore entries at a forbidden pair, whatever they hold * tests: rules, resolver, KeyFeatures, scopeAvailability, and a Power-only fixture in PathwayDetailPage.scope Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Electrification x Power now has no value at all, so the data can no longer say "Not Applicable" or "No information" there. The pairs are declared once, in keyFeatureRules; the CI check and the workbook importer both read that declaration, so neither the importer nor a hand-edit can bring a value back. This makes schema:check fail on src/data until the 701 existing entries are removed (next commit). * validateScopes: any keyFeatures entry at a forbidden field x sector is an error, whatever its value * schema: drop "Not Applicable" from the electrification enum and note the rule in its description; regenerate pathwayMetadata.v2.d.ts * import-pathway-data: a non-blank cell at a forbidden pair is an authoring error (nothing written, as for unmapped regions), one per pathway naming its rows; retire decision 0023's "Not Applicable" in the isAbsent comment * codemod-v1-to-v2: emit no electrification for a Power-only pathway * testdata: move the v2 fixture's Power electrification entry to Steel * tests: validateScopes, importer buildKeyFeatures and keyFeaturesFromV1, codemod, schema and KeyFeatures allow-lists Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Electrification cannot apply to Power, and a forbidden pair now carries no value at all (previous commit). Remove the 701 entries the data still had there: 695 "Not Applicable" (cookbook decision 0023's marker, now retired) and 6 "No information" in the SDSN Singapore files. Nothing else in these files changes. The pages are unaffected: the resolver already never read these cells. Power-only pathways with no across-sectors row now show "Not applicable", and the others keep inheriting their across-sectors value. * 71 files across CGS-IESR, IEA, JETP, JRC, NGFS, PHDOE, TransitionZero, UN SDSN/CW, UTS-ISF and EREA/DEA * 23 of them are left with an empty electrification array Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-1045.westus2.2.azurestaticapps.net |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Two unresolved moderate findings remain: a focus side effect inside a state updater in useDropdown, and a useElementWidth observer that never attaches when the page switches to the Power sector, leaving those charts at the fallback size.
No open issues 🎉
What changed in this PR
The PR description focuses on one rule: a key-feature pair that cannot apply (Electrification for the Power sector) should carry no value. It should show "Not applicable" instead of "No information", and the validator, resolver, and importer should enforce that. The diff under review is broader. It also adds the v2 timeseries and sector-segment schemas, the comparison page's scope, ribbon, dependency and plot work, geography and region cleanup, plot year capping, a design-token build with a CI check, and README import docs.
Changes:
- Adds v2 schemas and types for scoped key features, sector segments, data availability, and timeseries. Regions must have at least two members.
- Adds comparison-page scope handling: shared sector and per-column geography, ribbon, plots, and dependencies. Tray restore drops incompatible selections.
- Adds reusable hooks and components for URL state, dropdowns, element width, tooltips with a button mode, and scope badges.
- Adds timeseries build and CSV changes, schema-check cross-file validation, and a design-token generator with a CI job.
| File | Description |
|---|---|
| testdata/valid/pathwayMetadata_v2_minimal.json | Minimal v2 fixture for schema tests. |
| testdata/valid/pathwayMetadata_sample_03.json | Region now has real member countries. |
| testdata/valid/pathwayMetadata_sample_02.json | Region now has real member countries. |
| testdata/valid/pathwayMetadata_minimal.json | Region now has real member countries. |
| src/utils/validateTimeseries.ts | Cross-file checks for timeseries geography and segments. |
| src/utils/tooltipUtils.ts | Sector-segment tooltips from the taxonomy; adds SMR tooltip. |
| src/utils/tooltipUtils.test.tsx | Tests segment tooltips against the taxonomy. |
| src/utils/timeseriesSegments.ts | Derives plotted segments for a series. |
| src/utils/timeseriesSegments.test.ts | Tests segment derivation and ordering. |
| src/utils/timeseriesIndex.ts | Adds a synchronous index getter. |
| src/utils/timeseriesAvailability.ts | Exact geography matching; tolerates missing metrics axis. |
| src/utils/timeseriesAvailability.test.ts | Tests exact matching and missing metrics axis. |
| src/utils/searchUtils.ts | Scope-aware filters for emissions trajectory and policy ambition. |
| src/utils/searchUtils.test.tsx | Updates region fixtures to two or more members. |
| src/utils/scopeLabel.ts | Names the active scope in prose. |
| src/utils/scopeLabel.test.ts | Tests scope labels. |
| src/utils/scopeAvailability.ts | Decides which scope tokens are selectable. |
| src/utils/scopeAvailability.test.ts | Tests scope availability rules. |
| src/utils/plotYears.ts | Caps plots at 2050 and reports overflow. |
| src/utils/plotYears.test.ts | Tests the 2050 cap. |
| src/utils/loadData.ts | Extracts a dev-mode check, isViteDev. |
| src/utils/loadData.test.tsx | Tests isViteDev. |
| src/utils/geographyUtils.ts | Updates disclaimer wording; adds spelling-fold key. |
| src/utils/geographyUtils.test.tsx | Updates fixtures to two-member regions. |
| src/utils/geographyFallback.ts | Resolves the geography a plot falls back to. |
| src/utils/filterRegions.ts | Doc updates for the resolver's source of region membership. |
| src/types/pathwayTimeseries.v2.d.ts | Generated type for v2 timeseries. |
| src/types/pathwayMetadata.v1.d.ts | Generated type: pathway type doc and new technologies. |
| src/types/index.ts | Exposes v2 metadata and scope selection types. |
| src/types/common/technology.v1.d.ts | Adds new technology display names. |
| src/types/common/sectorSegment.v1.d.ts | Generated type for sector segments. |
| src/types/common/scopeSector.v2.d.ts | Generated type for scope sector. |
| src/types/common/scopeGeography.v2.d.ts | Generated type for scope geography. |
| src/types/common/metric.v1.d.ts | Drops sectorScope from the type. |
| src/types/common/geography.v1.d.ts | Generated type: regions need two or more members. |
| src/types/common/dataAvailabilityMetric.v1.d.ts | Generated type for data-availability metrics. |
| src/types/common/dataAvailability.v1.d.ts | Generated type for data-availability vocabularies. |
| src/test/mockIntersectionObserver.ts | Test mock accepts a bounding rectangle. |
| src/schema/pathwayTimeseries.v2.test.ts | Tests v2 timeseries schema rules. |
| src/schema/pathwayMetadata.v1.json | Doc wording fix. |
| src/schema/common/technology.v1.json | Adds new technology names. |
| src/schema/common/sectorSegment.v1.json | New sector-segment vocabulary. |
| src/schema/common/scopeSector.v2.json | New scope-sector vocabulary. |
| src/schema/common/scopeGeography.v2.json | New scope-geography rule. |
| src/schema/common/metric.v1.json | Removes the sectorScope field. |
| src/schema/common/index.ts | Registers the new common schemas. |
| src/schema/common/geography.v1.json | Regions need at least two members. |
| src/schema/common/dataAvailabilityMetric.v1.json | New data-availability metric schema. |
| src/schema/common/dataAvailability.v1.json | New data-availability vocabularies. |
| src/pages/resources/ResourcesMethodologyPage.tsx | Updates the region note and new feature lists. |
| src/pages/resources/ResourcesMethodologyPage.test.tsx | Tests the updated region note. |
| src/pages/PathwaySearch.test.tsx | Uses scoped key-feature entries in fixtures. |
| src/pages/PathwayDetailPage.test.tsx | Deep-links to the scope tab; removes unmapped-region test. |
| src/pages/PathwayDetailPage.disclaimer.test.tsx | Deep-links to the scope tab; uses two-member region. |
| src/pages/LandingPage.tsx | Copy fix. |
| src/pages/ComparisonPage.disclaimer.test.tsx | Shares a sector across fixtures; raises timeouts. |
| src/index.css | Adds a rose colour family for segment badges. |
| src/hooks/useUrlParamState.ts | URL-backed selection state hook. |
| src/hooks/useUrlParamState.test.tsx | Tests the URL state hook. |
| src/hooks/useSessionFlag.ts | Session-scoped on/off preference hook. |
| src/hooks/useSessionFlag.test.tsx | Tests the session flag hook. |
| src/hooks/useElementWidth.ts | Measures element width for charts. |
| src/hooks/useElementWidth.test.tsx | Tests element width measurement. |
| src/hooks/useDropdown.ts | Shared dropdown open, close, and escape handling. |
| src/data/pathwayMetadata.ts | Warns in dev about unmigrated v1 metadata. |
| src/context/ComparisonContext.tsx | Restores the tray and drops incompatible selections. |
| src/context/ComparisonContext.test.tsx | Uses real pathway ids in restore tests. |
| src/context/ComparisonContext.restore.test.tsx | Tests tray restore rules with mocked data. |
| src/components/TextWithTooltip.tsx | Adds a button trigger mode. |
| src/components/TextWithTooltip.test.tsx | Tests the button trigger mode. |
| src/components/Tabs.test.tsx | Tests tabs and URL-backed active tab. |
| src/components/SectionIntro.tsx | Short intro text under section headings. |
| src/components/ScopeFilterNotice.tsx | Says how many rows a scope hides. |
| src/components/ScopeAxisRow.tsx | Shared scope axis row with a show-more control. |
| src/components/RegionMembersTooltip.tsx | Always shows a country count. |
| src/components/RegionMembersTooltip.test.tsx | Updates tests for the count wording. |
| src/components/PlotSelector.tsx | Adds segment field to plot data points. |
| src/components/PlotSelector.test.tsx | Adds segment field to fixtures. |
| src/components/PathwayCard.tsx | Blocks adding pathways with no shared sector. |
| src/components/PathwayCard.test.tsx | Updates fixtures; tests stale ids vs. limit. |
| src/components/PathwayCard.comparison.test.tsx | Tests add-to-comparison blocking rules. |
| src/components/NormalizedStackedAreaChart.tsx | Adds geothermal colour; removes renewables. |
| src/components/NormalizedStackedAreaChart.test.ts | Checks colour coverage for power technologies. |
| src/components/GeographyDivergenceNotice.tsx | Explains when comparison columns differ. |
| src/components/DropdownFacetShell.tsx | Uses the shared dropdown hook. |
| src/components/DownloadDataset.tsx | Notes when a download runs past 2050. |
| src/components/DownloadDataset.test.tsx | Tests the past-2050 note. |
| src/components/CoreDrivers.tsx | Renders core drivers, including "not a driver". |
| src/components/CoreDrivers.test.tsx | Tests core driver rendering. |
| src/components/ComparisonRibbon.tsx | Carries scope into the comparison URL. |
| src/components/ComparisonKeyFeatures.tsx | Adds core drivers to the comparison grid. |
| src/components/ComparisonDependencies.tsx | New dependencies grid for comparisons. |
| src/components/Badge.tsx | Adds segment and scope variants; fixes -pub colours. |
| src/components/Badge.test.tsx | Tests new variants and theme colour tokens. |
| scripts/timeseries-csv.ts | CSV helpers for timeseries download. |
| scripts/timeseries-csv.test.ts | Tests CSV helpers. |
| scripts/schema-check-files.ts | Adds v2 and cross-file checks; fixes annotation paths. |
| scripts/codemod-timeseries-v1-to-v2.test.ts | Tests the timeseries v1 to v2 codemod. |
| scripts/build-timeseries-files.ts | Writes segment columns and CSV via shared helpers. |
| scripts/build-design-tokens.ts | Generates colour tokens from the stylesheet. |
| scripts/build-design-tokens.test.ts | Tests token generation. |
| README.md | Documents adding pathways and benchmark data. |
| package.json | Adds tokens:build script and exceljs dev dependency. |
| docs/design/README.md | Describes the design token and UI kit files. |
| .gitignore | Ignores .venv/ and TypeScript build info. |
| .github/workflows/node-build-committed-files.yml | Adds CI check for design tokens. |
Review effort: Lite
AlexAxthelm
marked this pull request as ready for review
October 29, 2026 19:31
Expected version change and release notes🚨 WARNING: This PR is not expected to trigger a new versionTo trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/ |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-1045.westus2.2.azurestaticapps.net |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The comparison grid in ComparisonKeyFeatures.tsx still shows "No information" with no tooltip for forbidden sector pairs, so it doesn't match the new "Not applicable" handling in KeyFeatures.tsx.
2 open issues
Review effort: Lite
The header called Electrification x Power a sibling-dependent rule beyond draft-07. It reads only the entry's own sector, so the schema could express it per item. It lives in validateScopes so the validator, importer and page share the one declaration in keyFeatureRules.ts.
Expected version change and release notes🚨 WARNING: This PR is not expected to trigger a new versionTo trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/ |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-1045.westus2.2.azurestaticapps.net |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The importer refusal depends on Electrification cells in an external workbook that cannot be checked from this repo, and the browser checks were manual while the cookbook 0023 text sits outside the diff, so the key behavior cannot be verified from the review evidence.
No open issues 🎉
2 resolved since last review
Review effort: Lite
Collaborator
|
@AlexAxthelm wouldn't it be possible to expect that power x electrification have a specific value ("Not applicable") rather than expecting it to be empty? The new expectation requires an update of the cookbook and updates to all authored data sets. Expecting "Not applicable" would require zero updates |
Cookbook decision 0023 has authors write "Not Applicable" for Electrification x Power. Accept it there, as well as a blank cell, and write no entry either way, so the workbook keeps 0023's convention while the data still carries no value at the pair. Any other value there, "No information" included, is still refused.
Collaborator
Author
Sounds good. I have the imported accept it, and then not write it to the imported JSON files, so we can keep it explicit in the workbooks, but then use the normal fallback behavior that we're introducing here. |
Expected version change and release notes🚨 WARNING: This PR is not expected to trigger a new versionTo trigger a version bump, use at least one conventional commit message in this branch. See: https://www.conventionalcommits.org/en/v1.0.0/ |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://proud-glacier-0f640931e-1045.westus2.2.azurestaticapps.net |
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
A moderate-severity hidden finding in scripts/codemod-v1-to-v2.ts shows that a non-blank electrification value can be dropped from Power-only v1 files without any note in the run report, so a human reviewer needs to assess this undisclosed data-loss risk before the change can be approved.
No open issues 🎉
Review effort: Lite
jacobvjk
approved these changes
Oct 30, 2026
| feature: FeatureConfig, | ||
| notApplicableSector: string, | ||
| ): string { | ||
| return `${feature.label} does not apply to the ${notApplicableSector} sector.`; |
Collaborator
There was a problem hiding this comment.
nb: would be easier to understand if we explained why, but this may introduce some complexity. E.g. in this case: "Does not apply to the Power sector: Power is not an end-use sector, so Electrification cannot be measured."
AlexAxthelm
pushed a commit
that referenced
this pull request
Oct 30, 2026
Memoizes the grid's per-column resolvers, so a page re-render for state the grid ignores no longer rebuilds every column's geography chain. Adds a grid test for Electrification on a Power-only pathway reading "Not applicable" (#1045), and makes the page test check each column's cell rather than finding the value anywhere on the page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Summary
Electrification cannot apply to the Power sector. Until now the data marked that pair with "Not Applicable" (cookbook decision 0023), and the resolver skipped the cell. A Power-only pathway with no across-sectors row then ran out of scopes, and its page said "No information", with the hover "Nothing is recorded for this pathway at any scope". That reads as if the pathway could have said something there and didn't. It affects 57 live pages, including CGS-IESR, JETP-CIPP, TransitionZero and SDSN LA/TH/MM.
This PR makes a forbidden pair carry no value at all. The pairs are declared once, in
src/utils/keyFeatureRules.ts, and four places read that declaration:Today the only forbidden pair is Electrification · Power.
Behaviour
"No information" keeps its meaning: the pathway could say something here and doesn't.
Commits
feat(key-features): say "Not applicable" where a feature cannot apply. This is the page change on its own, and it works against the old data.FORBIDDEN_KEY_FEATURE_SECTORS/isForbiddenKeyFeatureSector, which 693e3a7 removed.notApplicableSectoron both resolved and unavailable results.KeyFeaturesshows the "Not applicable" badge in the unavailable case.scopeAvailabilityignores entries at forbidden pairs.feat(validation): reject any value at a forbidden key-feature pair.validateScopedEntries(npm run schema:check) rejects any entry at a forbidden pair, whatever its value.import-pathway-datatreats a non-blank cell at a forbidden pair as an authoring error, so nothing is written. This is the same pattern as the unmapped-region refusal (Fix/single country regions #1035).fix(data): drop Electrification values for the Power sector.Commit 2 on its own fails
schema:checkonsrc/data, and commit 3 fixes it, so please keep those two together. Commit 1 stands alone.Workbook follow-up
src/datais regenerated from the workbook, so the workbook has to change too. Every Electrification cell on a Power row indraft_key_featuresmust be blank: 701 cells across 71 pathways. Until then the importer refuses to write anything, which is deliberate, so a re-import can't undo commit 3. Cookbook decision 0023's text needs the matching update. The details are in the workbook issue.Testing
npx vitest run --exclude '**/.claude/**': 92 files, 1487 tests, all passing.npm run schema:check: 129 valid items, 0 invalid.tsc -p tsconfig.app.jsonreports 202 errors, against 201 onepic/v2. The new one is TS5097 on the.ts-extension import ofkeyFeatureRules.tsinvalidateScopes.ts, the same error the existingtimeseriesTaxonomy.tsimport raises; ts-node needs the extension.🤖 Generated with Claude Code