chore: sync main — CM duplicate-ISO guard (#290)

reports/technical_risk_register.md

@@ -2,7 +2,7 @@
 
 **Last updated:** 2026-09-17
 **Governing ADR:** ADR-010 (Technical Risk Register)
-**Entry count:** 90 concerns (67 resolved, 23 open) + 5 disagreements (4 resolved)
+**Entry count:** 91 concerns (68 resolved, 23 open) + 5 disagreements (4 resolved)
 
 ---
 
@@ -28,7 +28,7 @@ Root causes shared by multiple concerns. Resolving the root tends to dissolve or
 | **C — PRIO-GRID scale discipline** | Repo handles ~260K-cell geodata without size discipline, at rest and at render | C-23 (shapefile in git), C-26 ✓ (render OOM → three-tier ladder), C-205 ✓ (globe legibility), C-209 (raster guard measures data rows, not the rendered lattice — reopened 2026-07-15, RESOLVED 2026-07-16), C-189 ✓ / C-190 ✓ (aggregation/omission methodology guards — closed superseded, ADR-021) | **Render side resolved (2026-06-29, globe epic #188): C-26 ✓ + C-205 ✓ — the three-tier choropleth → raster-heatmap → PNG ladder with a coastline overlay renders the full globe within budget. Remaining open: C-23 (56 MB cell shapefile committed — at-rest discipline).** C-189 ✓ / C-190 ✓ closed 2026-07-31 (superseded: ADR-021's per-cell-faithful raster/PNG designs the coarsening/subset path out; re-register if such a feature is ever proposed). |
 | **D — Ingestion-layer boundary** | loaders/ crossed the pipeline-core boundary ahead of governance | C-30, C-31, C-32 | Resolved (PR #82) |
 | **E — Legacy transform machinery** | RESOLVED (2026-06-20, #119) — `DatasetTransformationModule` removed + direct `polars` declaration dropped (polars stays transitive via pipeline-core) | C-25 ✓ (+ resolved C-10, C-04, C-02) | ✓ Resolved |
-| **F — Value-correctness & contract assurance** | The load → compute → render → reconcile chain is tested for shape / does-not-crash, not for value equality, contract conformance, or input completeness | C-29 ✓ (render fidelity), C-35 ✓ (MAP/HDI correctness — render path + PosteriorDistributionAnalyzer both on the views-frames tower + law tests; RESOLVED, ADR-019 / #157), C-185 (`*_map` is a tower tip, not a MAP — naming debt), C-39 ✓ (metadata accessors untested — resolved, `tests/test_metadata_accessors.py`), C-41 ✓ (canonical-token contract test), C-116 ✓ (multi-match → silent wrong value — RESOLVED: fail-loud default + visible "ambiguous" cell + collision contract test), C-111 ✓ (input completeness — RESOLVED 2026-07-31, S4 #266), C-113 (actuals provenance), C-112 (bundled-data staleness — forward, pairs with C-22), C-186 ✓ (views-frames version → forecast-output drift), C-208 ✓ (gapped-lattice cell stretch/misplacement — rendering-geometry faithfulness, RESOLVED 2026-07-16), C-192 ✓ (cross-repo eval contract untested — RESOLVED 2026-07-31, executable seam test) (+ resolved C-01, C-11) | Open — highest latent severity; mostly "write the missing correctness/contract test" (an assurance sprint). Phase 1 (correctness/contract core) landed 2026-06-28: C-29 ✓ (render==source fidelity test), C-41 ✓ (canonical-metric contract test), C-111 values-half guarded. C-186 ✓ (tower behavioural-regime tests, Phase 2, 2026-06-28). Remaining: C-112 (guards live — watch the regen cadence), C-113 / C-185 (non-test); C-111 ✓ resolved (coverage checks, S4 #266); C-192 ✓ resolved (seam test landed, S2 #264); C-39 ✓ resolved (accessor tests landed, 2026-07). |
+| **F — Value-correctness & contract assurance** | The load → compute → render → reconcile chain is tested for shape / does-not-crash, not for value equality, contract conformance, or input completeness | C-29 ✓ (render fidelity), C-35 ✓ (MAP/HDI correctness — render path + PosteriorDistributionAnalyzer both on the views-frames tower + law tests; RESOLVED, ADR-019 / #157), C-185 (`*_map` is a tower tip, not a MAP — naming debt), C-39 ✓ (metadata accessors untested — resolved, `tests/test_metadata_accessors.py`), C-41 ✓ (canonical-token contract test), C-116 ✓ (multi-match → silent wrong value — RESOLVED: fail-loud default + visible "ambiguous" cell + collision contract test), C-111 ✓ (input completeness — RESOLVED 2026-07-31, S4 #266), C-113 (actuals provenance), C-112 (bundled-data staleness — forward, pairs with C-22), C-186 ✓ (views-frames version → forecast-output drift), C-208 ✓ (gapped-lattice cell stretch/misplacement — rendering-geometry faithfulness, RESOLVED 2026-07-16), C-192 ✓ (cross-repo eval contract untested — RESOLVED 2026-07-31, executable seam test), C-227 ✓ (CM choropleth silently drew the dead country on duplicate ISO codes — RESOLVED 2026-09-17, loud guard at the merge chokepoint; latent, never observed) (+ resolved C-01, C-11) | Open — highest latent severity; mostly "write the missing correctness/contract test" (an assurance sprint). Phase 1 (correctness/contract core) landed 2026-06-28: C-29 ✓ (render==source fidelity test), C-41 ✓ (canonical-metric contract test), C-111 values-half guarded. C-186 ✓ (tower behavioural-regime tests, Phase 2, 2026-06-28). Remaining: C-112 (guards live — watch the regen cadence), C-113 / C-185 (non-test); C-111 ✓ resolved (coverage checks, S4 #266); C-192 ✓ resolved (seam test landed, S2 #264); C-39 ✓ resolved (accessor tests landed, 2026-07). |
 | **G — Partner-deliverable readiness** | Reports are built for internal preview, not yet hardened as a standalone, traceable, decision-appropriate *partner artifact* | C-28 ✓ (offline / self-contained — RESOLVED, vendored Tailwind, #132), C-34 ✓ (provenance / auditability — RESOLVED, footer stamp, #131), C-187 ✓ (vendored-CSS class coverage — RESOLVED, #132), C-188 ✓ (machine-readable provenance — RESOLVED, #132), C-109 (decision-appropriate uncertainty), C-214 ✓ (plotly-injection heuristics — RESOLVED 2026-07-31, S3 #265) | Open — C-109 (decision-appropriate uncertainty, partially delivered 2026-07: the #230 P(any) exceedance map layer) is the live remainder; C-214 ✓ resolved (S3 #265); offline + provenance now shipped |
 | **H — Published-state qualification gap (dev-tandem ↔ PyPI seam)** | Contracts, pins, and names are qualified against dev branches moving in tandem, never against **published** artifacts — the coordinated publish (#179) is the single event where every deferred qualification fires at once | C-213 ✓ (root — vpc pin outran PyPI; import surface canonicalized + guarded 2026-07-31 S1 #263; ordering half resolved 2026-08-03 when vpc 3.0.0 published pre-qualified), C-192 ✓ (cross-repo seam untested — resolved S2 #264), C-36 (upstream transitive pins — re-probed at vpc's 2026-08-03 publish: envelope matches ours, ingester3==2.1.1 unchanged so the levenshtein wall persists), C-185 (rename waits on a coordinated cross-repo change), C-186 ✓ residual (b) (exact-pin views-frames for release), C-211 (partial — cross-repo decision, not publish-gated) | Open (added review-rr 2026-07-31) — mitigation is the #179 release-gate runbook (largely written); C-192's executable contract test landed (S2 #264); C-36 stays upstream-blocked regardless. **SECOND CONFIRMED FIRING (2026-08-02, v0.3.3):** 0.3.2's published `requires_dist` pin `views-evaluation>=0.4.0,<1.0.0` — qualified only against the dev-tandem git source — excluded views-evaluation **1.0.0** the day it hit PyPI, breaking platform co-resolution at the coordinated publish (caught by a sibling-repo agent). Fixed same-day: pin `>=1.0.0,<2.0.0`, interim git-source retired (its documented trigger fired), suite green against the RELEASED 1.0.0. The root cause has now bitten in production twice (vpc pin C-213(a) + this); every remaining dev-tandem git source (vpc, until their 3.0.0 publishes) carries the same latent defect until re-qualified at its publish. **CYCLE CLOSED (2026-08-03, #279):** vpc 3.0.0 published to PyPI carrying the pre-qualified pins (the vpc#313 warning worked — no third firing); the last dev-tandem git source retired, `views-reporting==0.3.3` co-resolves from PyPI alone, suite green against the released 3.0.0. **No dev-tandem git sources remain anywhere in pyproject/uv.lock** — the cluster's generating condition is discharged for this release cycle; it re-arises only if a future unreleased-dependency tandem is reintroduced. **THIRD EXPOSURE — of the published-metadata facet only, caught as a prediction (2026-09-17, #289):** no dev-tandem was reintroduced and the `<2.0.0` ceiling WAS correctly qualified against the released 1.0.0; what is Cluster-H-shaped is the second half below. views-evaluation's pandas-free 2.0.0 (their #66/#72) and vpc's coming `^2.0.0` pin (vpc#515) would make our `<2.0.0` ceiling unsatisfiable in the shared env. Source widened to a SPAN pin `>=1.0.0,<3.0.0` — deliberately NOT the requested `>=2.0.0`, because 2.0.0 was unpublished at fix time (PyPI latest 1.0.0) and a floor naming an unpublished version is exactly C-213(a)'s mistake. Safety evidence: everything we import lives in `evaluation.metric_frame` (+ `metric_catalog` in one test), and no 2.0.0 story changes the MetricFrame format or the names we import (S8's gate: "MetricFrame format unchanged"; `[frames]` survives; the only merged changes to that module since 1.0.0 are to the provenance stamp's value — the `+g<sha>` suffix and S1's bounding of it — which we treat as opaque text). **Not yet pre-empted at the seam that matters:** the shared env resolves from PyPI, where 0.3.3's immutable metadata still says `<2.0.0` — vpc#515's "after views-reporting has widened" precondition is met only by a **0.3.4 publish**, which collides with the publish-last rule and is therefore a decision, not a mechanical step. Re-qualification trigger: views-evaluation 2.0.0 on PyPI → re-lock, suite, then the `>=2.0.0` floor + 0.3.4. |
 | **I — Post-B2 eval-report comparison seam (the baselines-vanishing braid)** | The B2 inversion shipped with its production wiring unverified end-to-end: consumer and producer disagree on frame location, declarations/targets/pins/envs each independently blank comparison rows, and every failure mode is silenced or mislabeled — so the deliverable degraded for months with green CI | C-215 (root, Tier 1 — subject-rooted source + seam test pinning it), C-216 (target drift + stranded fixes), C-217 (pins/envs make the seam unreachable), C-218 (Tier 1 — zero-report silence), C-219 (declaration/announce layer), C-220 (post-fix detonation), C-221 (corruption path), C-222 (vintage semantics dropped), C-223 (cache short-circuit / no baseline orchestrator), C-224 (3.0.0 classification drop), C-225/C-226 (ride-alongs) | Open (registered 2026-08-17 from /code-review max on the baselines-vanishing mystery; reconstruction VERIFIED). Fix sequencing constraint: C-220/C-225(AP-sort) must land WITH the C-215/C-216 resolution fix; C-192's locked seam contract must be corrected in the same change. Cross-repo: vpc stage + views-models configs/branches own most mechanisms; this repo owns the template/source/announce layers and the seam test. |
@@ -376,6 +376,20 @@ C-34 (provenance) and C-28 (offline) now anchor **Cluster G** (partner-deliverab
 
 ## Resolved Concerns
 
+### C-227: CM choropleth silently draws the DEAD country when a frame carries two country_ids under one ISO code — RESOLVED
+
+| Field | Value |
+|-------|-------|
+| ID | C-227 |
+| Tier | 2 — silent-wrong-map class (a value drawn over the wrong polygon with no signal), but LATENT: every real FORECAST fixture carries exactly ONE member of each dead/live pair (verified 2026-09-17; the historical fixtures carry both, but never reach the map). **Elevate to Tier 1 on the first observed dead-over-live draw.** |
+| Source | Issue #290 (filed from the pipeline-core side while investigating vpc#509); mechanism reproduced empirically this session |
+| Trigger | When a producer emits a dissolved entity beside its successor in one CM frame — views-r2darts2 ≥0.2.0 forecasts every entity ever seen (r2darts2#49); pipeline-core 3.3.0 ADR-064 now refuses such predictions at ITS boundary, but nothing at OUR boundary did until this fix. Also: any future producer/aggregator that keys rows by something other than the live `country_id`. |
+| Location | `views_reporting/mapping/mapping.py::build_mapping_dataframe` CM branch (the single chokepoint; merge `left_on="isoab", right_on="ADM0_A3"`) → `_plot_interactive_map` `pivot_table(index=ADM0_A3, aggfunc="first")` + `drop_duplicates(ADM0_A3)` for hover props, and `_plot_static_map`'s overpaint (last drawn wins). Data: `views_reporting/metadata/data/country.parquet` — **11 duplicate-isoab groups / 27 rows** (issue #290 counted 4 in the CM panel; the file also has SUN×6, YEM×3, YUG, DEU, ETH, SAU, ZAF); 9 of the 11 join the shapefile (SUN/YUG sit in the C-206 retired allowlist). |
+| Narrative | The bundled metadata legitimately carries dead/live pairs sharing an ISO code (Sudan 59 pre-2011 vs 245, Serbia 230/233, Tanzania 236/242, Indonesia 208/209 …). If a frame carries BOTH members, the merge inflates to two rows per polygon and every downstream resolution site picks silently: reproduced with `[59, 245]` — the pivot drew the dead Sudan's value for SDN and the hover reported country_id 59. Row order is frame order = ascending `country_id`, so the DEAD id wins in 4 of the 9 joinable pairs (IDN, SDN, SRB, TZA — the four #290 counted) and the live one happens to win in the other 5 (DEU, ETH, SAU, YEM, ZAF); the pick is arbitrary in all nine. The shapefile side is fan-out-safe (177 unique ADM0_A3 post-C-206), so inflation can only come from the frame. The PGM crosswalk resolves the same 11 groups at build time by #242's most-recent-observation rule (C-211); render time has no month-of-last-observation to apply it, so it can only DETECT. Unlike ragged coverage (C-111, legitimate, warns once) a FORECAST frame never legitimately carries both — its entity set is fixed at the origin — so this raises (ADR-008, with the required ERROR log), naming each collided code and its ids. Per CODE rather than per (code, month), deliberately: the repo's own historical fixture shows real transitions put both ids in the same month (Sudan 59+245 at 379, Indonesia 208+209 at 269), so per-month would not rescue them, and the hover props (`drop_duplicates(ADM0_A3)`) key on code alone. Mirror image of C-206: that was the MISSING-row failure at this merge line, this is the EXTRA-row one. |
+| Cross-refs | C-206 (same merge line, opposite failure — omission), C-211 (the coding transition; the PGM crosswalk's #242 collision rule), C-111 (coverage-check precedent; why this raises instead of warns), C-190 (omission-reads-as-no-risk sibling), C-212 (guard uses the two index columns only), Cluster F; r2darts2#49 / vpc ADR-064 (the producers and their upstream refusal); #290. |
+| Resolved | 2026-09-17 (#290 fix PR) |
+| Resolution | Guard at the CM chokepoint, immediately after `__check_missing_geometries` (rows without geometry don't count) and before `self._mapping_dataframe` is set: `groupby(ADM0_A3)[country_id].nunique() > 1` → `ValueError` naming each collided ISO with its sorted country_ids and the register ref — "no silent country picking". RED→GREEN in `tests/test_mapping_characterization.py::TestDuplicateIsoIsLoud` against the REAL bundle (no label mock — both 59 and 245 resolve to SDN there): `[59, 245]` raises naming both ids; `[245]` alone renders one SDN row. PGM raster/image sites untouched (unreachable at CM; `gid` keys are unique by construction). **First catch was our own test double:** the 191-country fixture e2e tests labelled the map through `mock_labels_for_index`, a positional double cycling 10 ISO codes — ~19 country_ids per code, a collision real data never has, which the silent pick had been absorbing. Those tests now use the REAL bundled metadata (offline since C-22; zero collisions on the fixture); the synthetic e2e tests keep the double (3-4 countries). Residual: detection only — resolution belongs to the producer (ADR-064). **Scope residual:** the CM map today renders only forecast frames (`forecast.py`); if a future feature maps a historical/actuals frame spanning a country-system change, this guard refuses it with a producer-blaming message that is the wrong diagnosis for that shape — such a map needs a month-aware rule (C-211's #242 most-recent-observation) before it can exist. Also noted: `tests/conftest.py::mock_isoab_for_index` (which `mock_labels_for_index` delegates to) cycles 10 codes positionally and now carries a docstring warning that >10 entities trip this guard. |
+
 ### C-213: Coordinated-release coupling to views-pipeline-core — the `>=3.0.0` pin outruns PyPI (2.3.0) and the consumed import paths are un-qualified against vpc's shim removals — RESOLVED
 
 | Field | Value |
@@ -1230,7 +1244,7 @@ Concerns are registered via the `register-risk` skill and curated via the `revie
 
 - **C-xx:** Concern entries (technical risks, code quality issues, architectural debt)
 - **D-xx:** Disagreement entries (unresolved debates between expert perspectives)
-- **ID numbering:** the native sequence ran C-01–C-48; the register then jumped to **C-107** (migrated from pipeline-core C-133, 2026-06-19) and continued C-108–C-118; a later numbering re-sync jumped to **C-185+** (now through C-213). The **C-49–C-106 and C-119–C-184 ranges are intentionally unused** (no backfill). **C-193–C-204 were never standalone entries:** C-193–C-199 are unused; of the expert-code-review finding IDs, C-200 and C-202 were deduped into C-192 at intake, C-201 went moot with the reconciliation deletion (#72), and C-203/C-204 were absorbed into C-26's resolution narrative — mentions of "C-203"/"C-204" (there and in C-209) refer to those review findings, not to register entries. New entries continue from the current maximum.
+- **ID numbering:** the native sequence ran C-01–C-48; the register then jumped to **C-107** (migrated from pipeline-core C-133, 2026-06-19) and continued C-108–C-118; a later numbering re-sync jumped to **C-185+** (now through C-227). The **C-49–C-106 and C-119–C-184 ranges are intentionally unused** (no backfill). **C-193–C-204 were never standalone entries:** C-193–C-199 are unused; of the expert-code-review finding IDs, C-200 and C-202 were deduped into C-192 at intake, C-201 went moot with the reconciliation deletion (#72), and C-203/C-204 were absorbed into C-26's resolution narrative — mentions of "C-203"/"C-204" (there and in C-209) refer to those review findings, not to register entries. New entries continue from the current maximum.
 
 Concerns are closed when:
 - The underlying issue is resolved (code change merged)

tests/conftest.py

@@ -117,7 +117,13 @@ def mock_name_for_df(df, entity_id="country_id", time_id="month_id"):
 
 
 def mock_isoab_for_index(index, level):
-    """Fake isoab DataFrame keyed by a (time, entity) MultiIndex."""
+    """Fake isoab DataFrame keyed by a (time, entity) MultiIndex.
+
+    Cycles REAL_ISO_CODES positionally, so it is only collision-free for
+    <= 10 entities. With more, two entity ids share a code and the CM map's
+    duplicate-ISO guard (C-227, #290) refuses to render — use the real bundled
+    metadata for larger frames, as tests/test_e2e_fixture.py does.
+    """
     entity_name = index.names[1]
     entity_ids = sorted(set(index.get_level_values(entity_name)))
     code_map = {