Referee 3 — Data engineering and reproducibility (WP38)
Adversarial audit of the pipeline, 2026-11-01. Scope: code under src/, Makefile, config, ledger/manifest, cached artefacts, and the consistency between text and outputs. Nothing outside reports/ was modified; no pipeline step or LLM call was rerun. All numbers below were measured with .venv/bin/python on the artefacts present at commit 47d768d.
1. Five most serious threats
T1 — Match-validation precision is mis-computed (and the paper repeats the wrong number)
src/validation/match_validation.py:90-92 builds cons = llm_a.where(llm_a == llm_b) (NaN when annotators disagree) and then reports yes = (cons[tier] == "yes").mean(). A NaN compares as False, so the denominator is all 60 pairs, while the n printed beside it is the consensus count (56/54/52/48). The same bug affects no and precision_by_tier_comparable_only.
Check run: recomputed from data/processed/match_validation_sample.csv.
| tier | consensus n | yes | reported yes (= yes/60) |
correct yes/consensus |
|---|---|---|---|---|
| A | 56 | 55 | 0.917 | 0.982 |
| B | 54 | 54 | 0.900 | 1.000 |
| C | 52 | 46 | 0.767 | 0.885 |
| D | 48 | 10 | 0.167 | 0.208 |
paper/sections/data.tex:24 states "among consensus pairs the share judged the same product is 0.92 … 0.90 … 0.77 … 0.17", reports/phase2_data_audit.md §2.5, reports/limitations.md L-10 ("precision 0.90–0.92") and the abstract ("matching precision 0.90–0.92") all inherit the mismatch. The direction is conservative, but the text describes a statistic that is not what the code computes.
T2 — The audit sample is not reproducible from the pipeline and leaks across the clean/sibling boundary
category_audit.sample() draws 25 listings per category from f_analysis_assort with seed 20261101 and then refuses to overwrite. The sample on disk contains 1 listing (uid 09cadb97…, store cafewilliamspartivento.com) that is outside the current assortment sample because that store is a non-representative sibling (f_sibling_dup = True). The sample was therefore drawn before src/clean/siblings.py existed in the clean chain (ledger: clean.listings 17:48:50Z, category_audit.annotate 17:56:56Z, clean.siblings first at 17:52:34Z, second clean.listings 17:52:57Z — the sample itself is not ledgered). Re-running make clean + category_audit sample from a fresh clone will produce a different 500-listing sample, so the 0.46 precision, the 474-listing classifier hold-out and A-002's primacy decision cannot be regenerated bit-for-bit. Neither category_audit_sample.csv, match_validation_sample.csv nor category_train_sample.csv is registered in logs/manifest.json (59 entries, none under data/processed/*.csv), so there is no checksum tying the published figures to the cached labels.
Related leakage: uids are disjoint between audit and training (0 overlap, verified), but 92.2 % of audit stores also appear in the training sample. With merchants publishing near-duplicate titles, the "held-out" 0.88 accuracy is a within-merchant estimate, not the merchant-holdout the mandate (Phase 5) asks for.
T3 — H4 is built on survivors only; its docstring and "exits" are false by construction
src/models/h4_dynamics.py:47 reads li[li.f_published | (li.active == 0)] with the comment "all live-store listings incl. inactive". But listings_clean.parquet is filtered to f_published & f_live_store in src/clean/listings.py:44; verified: 390 108 rows, active ∈ {1} only, and 0 rows with last_seen before the outcome window (2026-09-28). Consequently (i) baseline portfolios (W36–W39) are reconstructed from listings that survived to the snapshot, so a category whose listings were all withdrawn before the snapshot looks "absent at baseline" and any later listing becomes a spurious "entry"; (ii) region_level_exits is [] in both namespaces because every baseline listing is, by construction, still present in W40–W44. The 73 618 inactive listings (source audit §0.2.3) exist in listings_raw.parquet and were simply never loaded.
T4 — Reproducibility claims in the paper/README do not match what is versioned
paper/sections/reproducibility.texsays "the labelled audit samples … the run ledger with every API call and its cost, and the SHA-256 of every output are in the repository"..gitignorelines 4 and 31 ignoredata/processed/*andlogs/;git ls-files logs data/processedreturns onlydata/processed/.gitkeep. The ledger records aggregate token counts per annotation run, not per call, and no cost.README.mdTL;DR reports "Cramér's V 0.107 vs 0.075 … trimmed-listing p = 0.46 … ρ = −0.20, CI [−0.62; 0.34]". The rule namespace gives V = 0.1049 / 0.0747 / p_T = 0.492; the model namespace (declared primary) gives 0.1114 / 0.0858 / 0.445 and ρ = 0.00 [−0.63; 0.60]. 0.107 matches neither; the README mixes a stale H1 run with rule-label H2 while the paper abstract correctly uses the model namespace..gitignoreignoresdata/processed/*but notdata/processed_model/(asymmetric glob), so 19 label-dependent artefacts are tracked, includingstore_panel.parquet(1 557 merchant domains with FSA centroidlat/lngandmedian_price) andh5_groups.parquet(2 203 listing-level prices). The paper states the catalogue data are not redistributed.- Neither the
category_modelsteps (sample|annotate|embed|train) norsrc.models.summary(hypothesis registry) appear in theMakefile, soWP38_LABEL=model make features …fails on a fresh clone (nocategory_model.parquet), andmake allfails atvalidatewithout the cached CSVs that are not in git. - Stale documentation:
reports/data_dictionary.mdstill definesregion_finalas "region_db, else region_pip" andregion_sourceas "db | pip | unknown" (A-001 madecitythe first rule, 393 stores);data/raw/README.mdpoints external data todata/raw/external/;reports/00_source_audit.md§0.1.1 says "No empirical result depends on an LLM" although the primary labelling (A-002) rests on claude-sonnet-5-5 labels and OpenAI embeddings.
T5 — Label namespaces and sensitivity samples: one silent misnomer, one silent fallback, no test coverage
src/clean/stores.py:98definesregion_dbfirst = region_pip.fillna(region_city).fillna(region_db)with the comment "this is the PIP-first sensitivity variant";region_category.py:34,h1_raa.py:41androbustness.py:44("S_R0_dbfirst_regions") use it for S_R0. The preregistration (§ samples) defines S_R0 as "stores with a Fabri-Ka region only (region_source = db)". Verified:region_dbfirst == region_pipfor 100 % of stores with a PIP value. So the S_R0 rows of Table h1 (label "S_R0_dbfirst_regions") are PIP-first regions on the 1 223 DB-labelled stores — not the preregistered sample, and the table label says the opposite.stores.py:23-45globs every.gpkg/.shp/.geojsonunderdata/external/and accepts the first layer whose name column covers ≥ 15 regions; if the MERN file is missing it silently falls back toqc_er_2021_statcan_cbf.gpkg(pre-2021 Estrie/Montérégie boundaries, the very artefact A-001 removed), logging only a filename.- Namespace separation itself is correct (verified:
data/processed_model/holds only label-dependent files; label-independentstore_panel.parquetis byte-identical in both; audit/matching artefacts live only in base dirs;category_model.parquetis written toBASE_PROC). Butledger()(common.py:45) never recordsLABEL: 139 ledger lines, 0 with a label field, and steps such asmodels.h1_raaappear 9 times andreports.make_tables18 times with no way to tell which namespace they wrote.tests/test_panel.py(7 tests, all pass in 0.85 s) covers none of: audit/train disjointness, namespace separation, S_R0 definition, consensus-precision arithmetic, H4 inputs.
2. Checks actually run
| check | command / artefact | result |
|---|---|---|
| Test suite | make test |
7 passed |
| Price sample currency | listings_clean[f_analysis_price] |
only CAD; min store_cad_share = 1.0 (≥ 0.95 rule holds) |
| Sibling rule | non-rep sibling stores in assortment; H5 groups spanning only siblings | 0 / 0 of 1 064 groups |
| Audit vs train | uid overlap | 0 (audit 500, train 6 481); store overlap 92.2 % |
| Audit ⊂ assortment | 499 / 500 (see T2) | |
| Match-validation precision | recomputed (T1) | reported = yes/60, not yes/consensus |
| H4 inputs | active values, last_seen < 2026-09-28 |
{1}: 390 108; 0 rows |
| Manifest integrity | sha256 of 59 manifest entries vs disk | 0 mismatches |
| Ledger | 139 records, 23 step types; API usage lines: audit 2×500 + repairs, matching 2×240 + repairs, embed 370 055 listings / 24.09 M tokens, classifier 6 481 calls / 8.37 M in / 0.26 M out | no label, no cost |
| Timing vs preregistration | prereg commit 17:57Z; first models.* ledger entry 17:58:18Z; category_model.train 18:40:59Z after decisions.md (18:22Z) |
consistent with D-007 and A-002 |
| Secrets | git grep for key patterns; .env ignored |
none tracked |
| Hard-coded paths | grep `/Users | /Desktopinsrc, tests, Makefile` |
| Large files tracked | git ls-files sizes |
coverage_map.pdf 2.2 MB ×2, main.pdf 2.0 MB; acceptable |
| Background annotation | grep nohup |
not used; annotation ran in foreground, logs in logs/*_annotate.log; CSVs written only at end of each annotator loop (checkpoint every 500 for the classifier) — a crash mid-loop loses that annotator's calls but never corrupts the file |
| Paper numbers | abstract/data.tex vs experiments_model/*.json, experiments/*.json |
H1 (0.111/0.086, p 0.21/0.45), H3, H5 (4.7 log pts, −0.116, p 0.005/0.001), classifier (6 481, 0.69, 0.88, 0.91, 474, 52.3 %), representativeness (0.91/0.89/0.93/0.84; 1.8–11.1; CV 0.43), H4 (22 920 cells, 104 entries, 82 stores), ECI ρ 0.67 — all match; matching precision (T1) and README (T4) do not |
| Namespaces | file sets of processed{,_model} and experiments{,_model} |
correct separation (T5) |
Idempotence of make clean |
reads Makefile:21 |
listings runs twice by design (circular dependency with siblings); final state stable, ledger/manifest get duplicate entries; tables figures: shared recipe runs twice when both targets are requested (18 vs 15 ledger runs) |
3. Unresolved limitations
- Determinism was verified only indirectly (manifest hashes match disk; seeds set everywhere; thread-pool results consumed in order); no two-run comparison exists because a rerun was out of scope.
- LLM annotations are intrinsically non-reproducible (reasoning models,
thinking: adaptive); only caching protects them, and the caches are unversioned and unhashed (T2/T4). region_piprests on FSA centroids geocoded upstream (zippopotam/GeoNames); the 40 straddling FSAs flagged infsa_to_region_2021.csvare never joined tostores_master.- The sample-flow step "of an enabled store with ≥ 1 published listing" removes 0 rows; the table does not say that
f_live_storeis redundant in this snapshot.
4. Remediation plan
Must-fix (before submission)
match_validation.report: computec = cons[df.tier == t].dropna()and reportyes = (c == "yes").mean(),no,unsure,n = len(c); same forcomparable_only. Regenerateexperiments/match_validation.json,paper/tables/match_validation.tex,phase2_data_audit.md,limitations.mdL-10,data.tex:24, abstract.- Register the three LLM sample CSVs in
logs/manifest.json(register_outputinsample()and after eachannotate()), add their sha256 toreports/reproducibility.md, and either commit them (they contain no personal data) or publish them as a release asset; drop the uid from the sibling store or document it as a frozen pre-sibling draw. - Reconcile
reproducibility.texandREADME.mdwith git: tracklogs/run_ledger.jsonlandlogs/manifest.json(or say they are published separately); fix the README numbers to the primary namespace (0.111 / 0.086; p = 0.45; ρ = 0.00 [−0.63; 0.60]). .gitignore: replacedata/processed/*bydata/processed*/(or deliberately track both with a privacy review ofstore_panel.parquet/h5_groups.parquet); state the policy indata/raw/README.md.- H4: load
listings_raw.parquetfiltered to live, non-sibling stores andlisting_status == 'published'regardless ofactive, so that baseline presence includes withdrawn listings; recompute entries/exits; or rewrite the docstring and the paper to say "entries among surviving listings; exits unobservable". - Rename
region_dbfirst→region_pipfirstand the robustness key toS_R0_pipfirst_regions; either add the preregistered "Fabri-Ka label only" variant or amend the preregistration explicitly (A-003).
Should-fix
7. Add label=LABEL to every ledger() record and a cost_usd field computed from the model price list; add a make model-labels target (sample annotate embed train) and a make registry target for summary.py; make make validate skip gracefully when CSVs are absent.
8. load_regions_layer: take the layer path from settings.yaml (data/external/geo/qc_regions_admin_mern_bdat100k.gpkg) and raise if missing.
9. Tests: audit∩train = ∅ and audit ⊂ assortment; data/processed_model contains no audit/matching files; store_panel identical across namespaces; H4 inputs include active == 0; consensus-precision denominators; region_final precedence city > db > pip.
10. Store-level holdout for the classifier (GroupKFold by store_id, and an audit-store-disjoint evaluation) reported next to the 0.88.
Nice-to-have
11. Regenerate reports/data_dictionary.md with the A-001 definitions (pull DESC for region_final, region_source, region_conflict from one place shared with stores.py); fix data/raw/README.md, 00_source_audit.md §0.1.1 and data/external/README.md.
12. Break the listings ↔ siblings cycle (compute siblings from listings_raw + norm_title) so make clean runs each step once; split the tables figures recipe.
13. Join the 40 straddling FSAs as a geo_uncertain flag and add it to the robustness list already promised in data/external/README.md.