fix(Pakistan): cluster_features was extracted from the household COVER SHEET — 4,656 rows silently collapsed and 1,101 households misattributed (GH #323, one instance)

[ligon]

What was silently dropped

Pakistan 1991 cluster_features was extracted from F00A.DTA — the household COVER SHEET with
idxvars: {v: clust, i: hid}. That emitted one row per HOUSEHOLD (4,957) into a table whose
declared index is (t, v) — 301 clusters. The surplus i level was reduced away with an
unchecked groupby().first().

4,656 rows silently discarded (4,957 → 301, 93.9%), with no warning, on every read.

And the rows that survived were wrong, not merely deduplicated:

declared column	actually wired to	is it a cluster attribute?
Region	religion	No — a household's religion code. 291/301 clusters got the literal value "1.0".
Language	langint	No — the language of the interview.

Neither is constant within a cluster (religion varies within 75/301 clusters, langint within
141/301), so first() was not collapsing identical rows — it elected one arbitrary household as
spokesperson for its cluster. Measured: 1,101 households were assigned a cluster value that
contradicts their own record (252 + 849). Rural was declared optional and never populated.

Root cause class

EXTRACTION_BUG — a household-grain extraction feeding a cluster-grain table. Two corrections to the
original #323 diagnosis, both verified on a cold, cache-cleared, pristine-development build, both
load-bearing:

1. The collapse is NOT at the bug: framework silently drops rows on duplicate canonical index (groupby().first()) #323 site. _normalize_dataframe_index receives a frame that is
   already 301 rows and already unique, so its RuntimeWarning never fires — warm or cold
   (a full cold build emits zero duplicate-tuple warnings). The real reducer is earlier and fully
   silent: Wave.cluster_features() (country.py:1167-1189, GH Uganda cluster_features: wrong grain, stringified-float District, missing GPS wiring #161) collapses household-grain
   cluster_features with groupby().first() on the stated premise that the columns are "invariant
   within a cluster by construction" — a precondition it never checks. So this instance was worse
   than briefed: not "silent unless cold", but silent always.
2. The labels were recoverable. Data/REGIONS.TXT (fetched lock-free via _ensure_dvc_pulled;
   never dvc pull) is the codebook: 1 Punjab, 2 Sind, 3 NWFP, 4 Balochistan; urbrural 1 urban, 2 rural. The province names are documentary, not inferred. (It spells "Sind", not "Sindh".)

The fix (config-only — zero library code)

New lsms_library/countries/Pakistan/1991/_/cluster_features.py (materialize: make) sources real
cluster geography from REGIONS.DTA — already sample's source, so no new data dependency.
province/urbrural are provably cluster-constant (0/300 clusters vary), so the reduction is
exact and lossless and the result is unique by construction on (t, v) — no reducer runs at all.
The script asserts its invariants (cluster-constancy; clust-prefix ↔ province) so the build
fails loudly if they ever break. Language removed (a modal reducer would invent an attribute the
survey never measured); Rural flipped optional → required.

A declared aggregation: key was considered and rejected: country.py:2387 only excludes it from
column parsing — nothing reads it as a policy. It would have been prose, not enforcement.

Where I refused to guess. Cluster 2202029 (1 household) is entirely absent from REGIONS.DTA.
Its province is inferable from the clust prefix (zero off-diagonal over 4,753 households) — but
that rule can only be validated on the 300 clusters where province is already known, and is
untestable on the one cluster where it is actually needed. Left <NA> (class-2, silently missing >
class-1, silently wrong); the prefix rule is used as a build-time guard only.

Before → after

	before (pristine development)	after
L2-wave parquet rows	4,957, index (t, v, i)	301, index (t, v)
duplicate declared-index rows	4,656	0
validated-scanner cells / dup rows	1 / 4,656	0 / 0
API rows	301	300 (index (t, v), unique)
API Region	religion code — "1.0"×291, "3.0"×6, "2.0"×4	Punjab 154 / Sind 83 / NWFP 42 / Balochistan 21
API Rural	never populated	Urban 150 / Rural 150
API Language	langint (present)	removed
silently-wrong attributions	1,101	0

Why 300, not 301 — and why the count must NOT "recover" to 4,957. cluster_features is a
cluster-level table: 301 is the correct cardinality, not 4,957. What is recovered is the
4,656 rows' worth of information that was being destroyed. The script writes 301 rows;
_finalize_result's pre-existing dropna(how='all') hollow-row policy (country.py:2217) omits
the all-NA orphan cluster 2202029, so the API returns 300. Informationally identical to a consumer
(a sample → cluster_features join yields NaN geography either way).

Correctness of the new data — validated where validation was needed, not where it was free
(the Guyana failure mode). Each household's own record (sample.strata = raw province code;
sample.Rural from an independently-authored, pre-existing YAML mapping) was joined against its
cluster's assigned value: Region 4,753/4,753 agree, Rural 4,753/4,753 agree, zero disagreements —
versus 1,101 misattributions pre-fix. On all rows, not only the trivially-determined ones. Bonus:
203 households with no REGIONS row of their own now gain geography from their cluster.

Proof nothing else moved

Structural. git diff --name-only development...fix/323-pakistan = 6 files: Pakistan/** config
(4), one new test, one ledger. Zero library code — country.py, local_tools.py, feature.py,
and the canonical lsms_library/data_info.yml are untouched. No other country's build path can
change.

Dynamic (three independent checks by the regression red-team, not trusting the structural
argument alone):

• Cross-country values. Feature('cluster_features') assembled over 15 countries under base and
  fix with private, isolated data roots: all 14 non-Pakistan countries return identical row counts
  AND identical md5 digests of their sorted value slices on the common columns.
• Pakistan's other tables. The 1991/_/data_info.yml edit changes Wave._input_hash for every
  Pakistan table, forcing a full rebuild of all of them — and household_roster (36,079), housing
  (4,789), interview_date (4,956), sample (4,967) and household_characteristics (4,794) all come
  back byte-identical (same md5) base vs fix. Only cluster_features moves.
• Warm-cache upgrade (the dangerous one — an existing user whose cache holds the poisoned build):
  a base-built cache (hash-stamped 4,957-row wave parquet at (t,v,i) + 301-row country parquet) was
  copied and the fix run against it. The v0.8.0 hash gate marks both stale and rebuilds → 300 rows,
  provinces. No silent stale-serve. The fix → base direction is also safe
  (_evict_hashless_wave_caches removes the script-written hashless wave parquet before the base
  rebuild descent).

Suites. tests/test_schema_consistency.py (parametrized over all countries): 218 passed.
tests/test_currency.py: base = 1 failed / 53 passed; fix = 1 failed / 53 passed — identical. The
single failure is test_feature_ghana_per_wave, the known-broken GH #589, confirmed failing
identically on pristine development; not touched. 226 passed total on the fix (218 + 8 Pakistan).

Tests have teeth

tests/test_pakistan_cluster_features.py — 8/8 pass on the fix; 6 FAIL on pristine development:
test_extraction_is_cluster_grain (4,957 rows, idx (t,v,i)), test_index_is_cluster_grain,
test_region_is_province_not_religion (values "1.0"/"2.0"/"3.0"),
test_rural_is_populated, test_language_is_gone,
test_cluster_without_region_is_absent_not_imputed.

The 2 that pass in both (test_geography_is_constant_within_cluster, test_v_aligns_with_sample)
are honest source-invariant guards, labelled as such in-file and not counted as detectors.

The author's first draft asserted "no #323 collapse warning fires" — it passed on the unfixed tree
(that warning never fires here). Provably vacuous; deleted. The detector now sits at the
extraction, upstream of the silent reducer.

The guards are not vacuous either: the soundness red-team permuted the source four ways and all four
corruptions were caught — row-level province permutation, row-level urbrural, a cluster-level
province permutation that preserves within-cluster constancy, and a shifted join key.

Red-team verdicts (all three, in full)

Three adversarial lenses. All three pass; all three filed nits; none blocking. Reproducing them here
rather than laundering them into silence.

1. correctness — pass, severity nit

The fix is correct and the duplicate count genuinely goes to zero — verified at BOTH the parquet
and the API, on cold isolated caches, with imports/config asserted to come from the branch under
review. Reproduced the loss on development (fresh LSMS_DATA_DIR): wave parquet 4,957 rows index
(t,v,i); 4,656 duplicate rows on the FULL declared index (t,v); API 301 rows,
Region={'1.0':291,'3.0':6,'2.0':4} (a religion code), Rural absent, Language present. Both
diagnosis corrections are true and load-bearing. Fetched the REGIONS.TXT blob (md5
43fbb4c274210bad51cc86dec2af6b57): labels are documentary, exactly as transcribed. Every raw claim
exact. Tests have teeth (6 fail pre-fix); the author caught and deleted their own vacuous test.

Nit (not blocking): tests/test_pakistan_cluster_features.py:160 reads
get_dataframe("lsms_library/countries/Pakistan/1991/Data/REGIONS.DTA") — a cwd-relative path
that raises dvc.exceptions.PathMissingError when pytest is invoked from outside the repo root.
Should be built from countries_root() so it also honors LSMS_COUNTRIES_ROOT. CI runs from the
repo root, so it passes today.

Unverified (stated, not assumed): the full pytest suite did not complete (it builds ~40
countries). The structural argument (zero library-code changes) is sound and was confirmed.

2. regression — pass, severity nit

Nothing else moves; verified three ways rather than trusting the structural argument alone (scope /
cross-country values / Pakistan's other tables — see above). Specifically hunted the regressions this
fix could plausibly have caused, and each held: warm-cache upgrade, column collision (Rural now
appears in both cluster_features and sample; legacy_locality at transformations.py:1026
selects cluster[['t','v','Region']] explicitly, so no pandas suffix collision, and
_join_v_from_sample never touches cluster_features — it's in the canonical skip list,
data_info.yml:65), no new data dependency (REGIONS.DTA and F00A.DTA were both already read
by sample), and golden snapshots (.coder/coverage/blessed.csv is header-only;
tests/test_coverage_matrix.py uses tmp_path fakes; no test outside the new file references
Pakistan or asserts cluster_features columns).

Nit (a) — cross-country output-SHAPE change, worth calling out explicitly:
Feature('cluster_features')() over all countries now loses the Language column entirely
(5,186 → 5,185 rows, 7 → 6 columns). It was all-NA for the other 14 countries, Pakistan was its only
declarer (every data_scheme.yml grepped), and it is not a canonical column. Intended — but an
external consumer doing df['Language'] now gets KeyError instead of NaN.

Nit (b): .coder/coverage/latest.csv still records
Pakistan,cluster_features,1991,...,301,warn: has_household_index — stale post-merge (300 rows, warn
gone). Not asserted anywhere; the cron refresh heals it.

Nit (c): same cwd-relative test path as correctness-nit above. Test-only.

Nit (d) — cosmetic: a cold build now emits UserWarning: Makefile not found in .../Pakistan/_. Unable to generate required data. before run_make_target's script fallback succeeds. NOT new to
the repo — Pakistan's interview_date is already a script-path table with no country _/Makefile
and already emits it identically on base (and still builds 4,956 rows). Misleading text, harmless.

Unverified: full pytest suite (same resource ceiling). Given the three checks above, a full-suite
regression outside Pakistan would require a test that reads Pakistan's cluster_features, and no such
test exists.

3. soundness — pass, severity nit

The recovered data is CORRECT, not merely present — proved with a source the fix does not use.
Checking REGIONS against itself would be circular, so an independent cross-file check was found:
the clust id (from F00A) has internal structure — digit 1 encodes province, digit 2 encodes
urban/rural. Crosstabbed against REGIONS.DTA, both are a perfect zero-off-diagonal over all 300
clusters (Punjab 154 / Sind 83 / NWFP 42 / Balochistan 21; Urban 150 / Rural 150). At the API level:
0/300 Region disagreements, 0/300 Rural disagreements, and 0/4,763 households whose assigned
cluster geography contradicts their own recorded province/urbrural. Could not find a single row
attributed to the wrong entity despite targeted effort.

F00A carries no geography column at all, so Region: religion was indeed nonsense. Language
is declared by Pakistan alone, absent from the canonical schema, referenced nowhere in code/tests —
removal is impact-free. Rural = 'Urban'/'Rural' matches canonical data_info.yml:132-140.
Cluster 2202029 left <NA> is the class-2 choice the standard explicitly endorses over class-1, and
it is not silent — a RuntimeWarning fires at build.

Nits only; nothing that should block landing.

GH #323 — this fixes ONE INSTANCE. The class stays open.

Do not close #323 on this PR. Closing the class on an instance fix is precisely how this bug
survived once already.

The real reducer here — Wave.cluster_features() at country.py:1180-1189 (GH #161) — still collapses
household-grain cluster_features with an unchecked groupby().first() on an "invariant by
construction" premise it never verifies. Independently grepped and confirmed to the digit:
39 wave-cells across 17 countries declare i: in cluster_features idxvars and route through
that same silent path. Post-merge: 38 / 16 (Pakistan exits).

The right remedy is central — verify the invariance premise before reducing, and name the offending
column and clusters when it fails — and is deliberately deferred to fix/323-framework (already in
flight) so it does not redden the 13 sibling worktrees now in the air.

🤖 Generated with Claude Code

[ligon]

Adversarial review — PR #612 (GH #323, Pakistan 1991 cluster_features)

Verdict: APPROVE_WITH_NOTES. I tried to break this and could not break the data. Every
headline number reproduced exactly, cold, on private isolated data roots. The findings below are
one test-coverage gap and a set of now-stale claims in permanent in-tree docs — nothing that
changes a returned value.

What I re-derived myself (nothing taken on trust)

Two git worktrees at origin/development (45aee170, = the merge base) and
origin/fix/323-pakistan (03af3a38); two private LSMS_DATA_DIRs with only dvc-cache
symlinked to ~/.local/share/lsms_library/dvc-cache; PYTHONPATH asserted per run
(assert 'wt_fix' in lsms_library.__file__). No dvc CLI at any point.

Raw source (get_dataframe, no cache):

claim	measured
F00A.DTA rows / distinct hid / distinct clust	4,957 / 4,957 / 301 ✓
F00A carries no geography	✓ — cid is constant 1; columns are dates/interviewer/religion/langint
hid is 7-digit-clust-prefixed	✓ all 4,957 (widths: hid 9, clust 7, hhcode 9)
REGIONS.DTA rows / clusters	4,763 / 300 ✓
province/urbrural constant within cluster	0 of 300 vary, and 0 nulls ✓ — not invariance-by-missingness
clust leading digit ↔ province	0 off-diagonal ✓ (and independently: 2nd digit ↔ urbrural, 150/150 perfect)
cluster in cover but not REGIONS	exactly [2202029] ✓ (204 hh without a REGIONS row; 10 REGIONS hh absent from the cover)
REGIONS.TXT md5 43fbb4c274210bad51cc86dec2af6b57	✓ read the blob: 1 Punjab / 2 Sind / 3 NWFP / 4 Balochistan, urbrural 1 urban 2 rural. Documentary, exactly as transcribed.
religion varies in 75/301, langint in 141/301; 252 + 849 = 1,101 misattributions	✓ reproduced to the unit

Cold builds:

	base (origin/development)	fix
L2-wave parquet	4,957 rows, idx (t,v,i), 4,656 dup on (t,v)	301 rows, idx (t,v), 0 dup
API	301 rows	300 rows, unique
Region	{'1.0': 291, '3.0': 6, '2.0': 4} (religion)	Punjab 154 / Sind 83 / NWFP 42 / Balochistan 21
Rural	absent	Urban 150 / Rural 150

Independent correctness check, run myself at the API (the thing that actually matters):
joined every household's own record against its cluster's assigned value —
sample.strata (raw province code) vs cluster_features.Region: 4,753 comparable, 4,753 agree,
0 disagree; sample.Rural (an independently-authored pre-existing YAML mapping) vs
cluster_features.Rural: 4,753/4,753 agree, 0 disagree. 203 households gain geography.

Attack angles that came back clean:

• Splitting real entities? N/A — no index level was added; grain went coarser. clust is globally
  unique (province-prefixed), cid is constant, so v: clust is the right identifier. D1 satisfied:
  no reducer, no aggregation: key; the drop_duplicates is guarded by a proven-exact invariant.
• Invariance by missingness? No — province/urbrural have zero nulls in REGIONS.DTA.
• Conservation / mass. household_characteristics(market='Region'): base 4,782 → fix 4,781.
  Exactly one row, the orphan cluster's single household. Nothing else moved.
• Tests discriminate. Ran the new file against the pre-fix tree with the base data root:
  6 failed, 2 passed — matching the PR's table exactly. 8/8 on the fix.
• Warm-cache upgrade. Copied the base-built cache and ran the fix against it: the v0.8.0 hash gate
  invalidates both tiers and rebuilds → 300 rows, provinces. No stale serve.
• LSMS_GRAIN_STRICT=1 (not in the PR body, and the strongest single result): base
  raises GrainCollapseError; post-fix cluster_features, sample, household_roster,
  housing, interview_date, household_characteristics all build clean.
• Downstream. locality() (4,967 rows, m = real provinces), market='Region', market='Rural'
  (newly possible), Feature('cluster_features')(['Pakistan']) (300 rows, (country, t, v)) all OK.
  Schema suite for Pakistan: 5 passed. No duplicate/competing work in origin/development.

---

Findings

1 · MEDIUM — the regression net does not encode the PR's own headline correctness evidence.
I mutation-tested it: swapping one dict, URBRURAL = {1: 'Rural', 2: 'Urban'}, mislabels all 300
clusters and contradicts sample.Rural for all 4,753 households — and the suite still returns
8 passed. test_rural_is_populated only asserts the value set is {Urban, Rural}; INVARIANT 2
compares the clust prefix to the province code, not to the label. A PROVINCE label
transposition (2: 'NWFP', 3: 'Sind') is invisible for the same reason. So the exact class-1
failure mode this PR exists to eliminate is unguarded going forward. The fix is one test, and the PR
already did the measurement — assert cluster_features.Rural == sample.Rural and
Region == {1:Punjab,…}[sample.strata] (I get 4,753/4,753 both ways). Data is right today; the
durable artifact doesn't keep it right.

2 · MEDIUM (docs) — "it never warns" is false against the tree this merges into.
The branch is up to date with development, which already ships the Site-2 instrumentation
(GrainCollapseWarning, country.py:4390, merged via #636/#640, with .coder/ledger/323-site2.md
listing Pakistan/1991 at 3,300 destroyed rows). My cold base build emits it verbatim:
GrainCollapseWarning: Pakistan/cluster_features/1991: … DESTROYED 3,300 of 4,957 rows ….
But CONTENTS.org ("whose groupby().first() emits no warning at all"), the test module
docstring ("the GH #323 warning NEVER fires -- warm or cold"), and ledger §2(1a) ("a full cold build
emits zero duplicate-tuple warnings") all assert the opposite. This was true when written; it is
false at merge. It is load-bearing prose — it is the stated reason two tests were not written — and
CONTENTS.org is permanent user-facing documentation. Same for §7's "recommended central fix …
deliberately deferred": that central fix has already landed. (Note the ledger contradicts itself
here too: §6's table row says the warning "fires on cold build" before, §2(1a) says it never
fires.) Please update the three prose sites; no behaviour change needed.

3 · LOW — two in-tree artifacts still carry the diagnosis the PR itself calls wrong.
1991/_/cluster_features.py's module docstring and the replacement comment in
1991/_/data_info.yml both say "_normalize_dataframe_index then dropped the surplus i level and
collapsed 4,957 → 301", which §2(1a) of the ledger explicitly corrects to Wave.cluster_features().
These are the two files a future maintainer reads first.

4 · LOW — the disclosed cwd-relative test path is real; I reproduced it.
tests/test_pakistan_cluster_features.py:160 reads
get_dataframe("lsms_library/countries/Pakistan/1991/Data/REGIONS.DTA"). Running pytest from /tmp:
1 failed, 7 passed — test_geography_is_constant_within_cluster dies inside DVC
(YAMLValidationError) after resolving the relative path against the wrong root. It also ignores
LSMS_COUNTRIES_ROOT, against CLAUDE.md's "resolve config paths via countries_root()" rule. CI
runs from the repo root so it is green today.

5 · LOW — drop_duplicates(subset='clust') keeps the first row, not the first non-null.
INVARIANT 1 uses nunique(dropna=True), so a future NaN province sitting alongside a real one in
the same cluster passes the guard, and drop_duplicates can then hand back the NA row. Class-2, not
class-1, and zero effect today (I verified 0 nulls in both columns). groupby('clust').first()
— NA-skipping completion, which is the house reading of NaN — would be robust and no less exact.

6 · LOW (note) — prior art not cited. development already ships
lsms_library/build_transforms.py::reduce_to_agreed / collapse_to_cluster_grain, a df_edit hook
that enforces precisely INVARIANT 1 (and raises, naming offenders). Ledger §3 doesn't mention it and
§4's "YAML alone cannot do this" is weaker than stated. This does not change the verdict — the
columns were semantically wrong, so collapse_to_cluster_grain would have (correctly) raised and
the source rewire was required regardless — but the hand-rolled guard duplicates tested machinery
and the ledger should say why it didn't reuse it.

---

What I could not break

I could not find a single row attributed to the wrong entity. I could not make the province or
urban/rural labels disagree with any independent source: the shipped codebook, the clust id's own
digit structure (1st digit ↔ province, 2nd digit ↔ urban/rural, both zero off-diagonal over 300
clusters), and sample's independently-authored YAML mapping all agree, on all 4,753 households
where a comparison exists. The 4,957 → 301 → 300 arithmetic is fully accounted for (301 real
clusters; the 301st, 2202029, is one household with no REGIONS row at all, left <NA>, warned
about at build, and dropped by the pre-existing hollow-row policy). I could not find a downstream
consumer that moves by more than that single household. Leaving #323 open is the right call.

(Ran cold on isolated LSMS_DATA_DIRs with only dvc-cache shared; never invoked the dvc CLI.)