diff --git a/.cg-docs/BRAIN-01.md b/.cg-docs/BRAIN-01.md index 01acb35..9ff338d 100644 --- a/.cg-docs/BRAIN-01.md +++ b/.cg-docs/BRAIN-01.md @@ -1,10 +1,10 @@ # 🧠 Project Brain — Part 1 -_Generated 2026-08-04_ +_Generated 2026-08-07_ -## Reporting_Level / Aaa.R / Pd_Deflation.R +## Reporting_Level / Aaa.R / Valid_Dlw_Load\(\) -_Keywords: `reporting_level` · `aaa.r` · `pd_deflation.r`_ · 75 entities +_Keywords: `reporting_level` · `aaa.r` · `valid_dlw_load()`_ · 83 entities - **[Migrate dplyr calls to collapse/data.table \(Phase 1: 3 files\)](.cg-docs/brainstorms/2026-04-27-dplyr-to-collapse-dt.md)** · `brainstorm` · _decided_ · `2026-04-27` > dplyr, tidyr, and tibble are used via `::` in 4 pipdata files but are **not declared in DESCRIPTION Imports** — phant… @@ -18,6 +18,10 @@ _Keywords: `reporting_level` · `aaa.r` · `pd_deflation.r`_ · 75 entities > `update_pip_inventory()` is ~350 lines handling 8+ responsibilities: version extraction from in-memory `proc_dta`, sk… - **[Inventory assembler: delta/update strategy](.cg-docs/brainstorms/2026-05-27-inventory-delta-strategy.md)** · `brainstorm` · _decided_ · `2026-05-27` > The `build_pip_inventory()` function (implemented 2026-05-22) used a "rebuild-from-scratch" strategy: query entire ca… +- **[valid_dlw_load review and inv_to_process rewrite](.cg-docs/brainstorms/2026-06-05-valid-dlw-load-review.md)** · `brainstorm` · _decided_ · `2026-06-05` + > Review of `valid_dlw_load()` and its helpers (`filter_aux_inv`, `inv_to_process`) to ensure the function accomplishes… +- **[Gate aux-change detection in valid_dlw_load on aux version change](.cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md)** · `brainstorm` · _draft_ · `2026-08-05` + > `valid_dlw_load()` always calls `valid_aux_load()`, which invokes `pipaux::compare_aux_releases()` and `pipaux::compa… - **[Enrich log report with success metrics and aux changes](.cg-docs/plans/2026-04-06-enrich-log-report.md)** · `plan` · _completed_ · `2026-04-06` > Extend the pipeline logging and `log_report()` so the markdown report includes: (a) the number of surveys successfull… - **[Remove redundant get_wrk_release\(\) guards from DLW call chains](.cg-docs/plans/2026-04-23-remove-redundant-get-wrk-release.md)** · `plan` · _completed_ · `2026-04-23` @@ -54,12 +58,18 @@ _Keywords: `reporting_level` · `aaa.r` · `pd_deflation.r`_ · 75 entities > Remove spurious `reporting_level.x` and `reporting_level.y` columns from the PIP master inventory. These are ALL-NA a… - **[Refactor inventory architecture: catalog-based assembler](.cg-docs/plans/2026-05-20-inventory-architecture-refactor.md)** · `plan` · _completed_ · `2026-05-20` > Replace the 350-line `update_pip_inventory()` + `format_vrs()` with a catalog-based assembler that reads version fact… +- **[Refactor valid_dlw_load: logging, abort, and content-hash comparison](.cg-docs/plans/2026-06-17-valid-dlw-load-refactor.md)** · `plan` · _completed_ · `2026-06-17` + > Implement the 5 decisions from the 2026-06-05 brainstorm: - **[save_pip_data: memory-resilient save order and GC guard](.cg-docs/plans/2026-06-18-save-pip-memory-resilience.md)** · `plan` · _done_ · `2026-06-18` > Prevent `cannot allocate buffer` failures in `save_pip_data()` when serializing large survey artifacts (e.g. `ARG_201… - **[pd_process_data: OOM crash fix for 4000+ survey runs](.cg-docs/plans/2026-06-24-pd-process-data-oom-crash.md)** · `plan` · _completed_ · `2026-06-24` > Step 1 (explicit `rm()` + `gc()` in `process_data()`'s success branch) and Step 3 (roxygen `@details`) were implement… - **[pd_process_data: RAM regression fix \(spec-once + per-survey gc\)](.cg-docs/plans/2026-06-28-pd-process-data-ram-regression.md)** · `plan` · _completed_ · `2026-06-28` > Running `pd_process_data()` on the current `var_tabmaker2` branch makes RAM explode, whereas commit `3e00987a` did no… +- **[2026-08-05-aux-version-gate-valid-dlw-load](.cg-docs/plans/2026-08-05-aux-version-gate-valid-dlw-load.md)** · `plan` · _—_ · `—` + > --- date: 2026-08-05 title: "Gate aux-change detection in valid_dlw_load on per-survey aux content-hash comparison" s… +- **[Aux version gate for valid_dlw_load — revised run-level hash design](.cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md)** · `plan` · _completed_ · `2026-08-06` + > Prevent unnecessary survey re-cleaning by comparing the current content hash of each requested auxiliary artifact wit… - **[2026-04-06-enrich-log-report-review](.cg-docs/reviews/2026-04-06-enrich-log-report-review.md)** · `review` · _—_ · `—` > **Review depth**: standard **Plan**: `.cg-docs/plans/2026-04-06-enrich-log-report.md` **Files reviewed**: 5 **Finding… - **[2026-04-16-review](.cg-docs/reviews/2026-04-16-review.md)** · `review` · _—_ · `—` @@ -100,6 +110,10 @@ _Keywords: `reporting_level` · `aaa.r` · `pd_deflation.r`_ · 75 entities > **Review depth**: standard **Files reviewed**: 3 (`R/update_pip_inventory.R`, `tests/testthat/test-update_pip_invento… - **[2026-05-20-inventory-architecture-refactor-review](.cg-docs/reviews/2026-05-20-inventory-architecture-refactor-review.md)** · `review` · _—_ · `2026-05-27` > **Review depth**: standard **Files reviewed**: 6 (`R/build_pip_inventory.R`, `R/pd_process_data.R`, `R/pd_deflation.R… +- **[2026-08-06-aux-version-gate-valid-dlw-load-revised-review](.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md)** · `review` · _—_ · `2026-08-07` + > **Review mode**: standard **Files reviewed**: 33 changed paths from `origin/refactor_dlw_load..HEAD` **Findings**: 17… +- **[2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review](.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review.md)** · `review` · _—_ · `2026-08-07` + > **Review mode**: mode:verify / light **Parent review**: `.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-… - **[Internal logmeta type markers polluting build_type_summary\(\) table](.cg-docs/solutions/bugs/2026-04-07-internal-logmeta-types-polluting-type-summary.md)** · `solution` · _—_ · `2026-04-07` > `log_report()` renders a "Summary by Type" table via `build_type_summary()`. The table was supposed to show only genu… - **[Conditional column initialization produces inconsistent data.table schema](.cg-docs/solutions/bugs/2026-04-29-conditional-column-init-inconsistent-schema.md)** · `solution` · _—_ · `2026-04-29` @@ -144,6 +158,8 @@ _Keywords: `reporting_level` · `aaa.r` · `pd_deflation.r`_ · 75 entities > After replacing `update_pip_inventory()` with `build_pip_inventory()`, `reporting_level` (and other stale columns) co… - **[joyn diagnostic columns in production data paths — reportvar and suffix discipline](.cg-docs/solutions/data-quality/2026-06-05-joyn-diagnostic-column-discipline.md)** · `solution` · _—_ · `2026-06-05` > Two distinct classes of bug in this codebase were caused by the same underlying pattern: `joyn::` join diagnostic col… +- **[Gate auxiliary re-cleaning with run-level content hashes and row-level changes](.cg-docs/solutions/data-quality/2026-08-07-aux-content-hash-gated-recleaning.md)** · `solution` · _—_ · `2026-08-07` + > `valid_dlw_load()` always invoked `valid_aux_load()`, which compares current auxiliary releases/vintages and returns … - **[Unified package environment: merging .pipdata / .pipdataenv / .logenv into one](.cg-docs/solutions/environment-issues/2026-04-30-unified-package-environment-accessor-pattern.md)** · `solution` · _—_ · `2026-04-30` > `pipdata` had three separate package-level environments: | Environment | Purpose | Defined in | |-------------|------… - **[Guard-at-entry-point pattern for release validation in pipdata](.cg-docs/solutions/git-workflows/2026-04-23-guard-at-entry-point-pattern.md)** · `solution` · _—_ · `2026-04-23` diff --git a/.cg-docs/BRAIN-log.md b/.cg-docs/BRAIN-log.md index c614e6b..03b2388 100644 --- a/.cg-docs/BRAIN-log.md +++ b/.cg-docs/BRAIN-log.md @@ -1,6 +1,6 @@ # 🧠 Project Brain — Chronological Log -_Generated 2026-08-04 · 103 artifacts (newest first) + 0 roadmap features_ +_Generated 2026-08-07 · 109 artifacts (newest first) + 0 roadmap features_ ## undated @@ -36,6 +36,27 @@ _Generated 2026-08-04 · 103 artifacts (newest first) + 0 roadmap features_ > **Review depth**: standard **Files reviewed**: 3 (`R/update_pip_inventory.R`, `tests/testthat/test-update_pip_invento… - **[2026-06-18-yaml-recode-dictionary](.cg-docs/plans/2026-06-18-yaml-recode-dictionary.md)** · `plan` · _done_ · `—` > The current variable recoding logic in `pd_dlw_clean.R` is hardcoded across four functions (`recode_edu()`, `recode_g… +- **[2026-08-05-aux-version-gate-valid-dlw-load](.cg-docs/plans/2026-08-05-aux-version-gate-valid-dlw-load.md)** · `plan` · _—_ · `—` + > --- date: 2026-08-05 title: "Gate aux-change detection in valid_dlw_load on per-survey aux content-hash comparison" s… + +## 2026-08-07 + +- **[2026-08-06-aux-version-gate-valid-dlw-load-revised-review](.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md)** · `review` · _—_ · `2026-08-07` + > **Review mode**: standard **Files reviewed**: 33 changed paths from `origin/refactor_dlw_load..HEAD` **Findings**: 17… +- **[2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review](.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review.md)** · `review` · _—_ · `2026-08-07` + > **Review mode**: mode:verify / light **Parent review**: `.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-… +- **[Gate auxiliary re-cleaning with run-level content hashes and row-level changes](.cg-docs/solutions/data-quality/2026-08-07-aux-content-hash-gated-recleaning.md)** · `solution` · _—_ · `2026-08-07` + > `valid_dlw_load()` always invoked `valid_aux_load()`, which compares current auxiliary releases/vintages and returns … + +## 2026-08-06 + +- **[Aux version gate for valid_dlw_load — revised run-level hash design](.cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md)** · `plan` · _completed_ · `2026-08-06` + > Prevent unnecessary survey re-cleaning by comparing the current content hash of each requested auxiliary artifact wit… + +## 2026-08-05 + +- **[Gate aux-change detection in valid_dlw_load on aux version change](.cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md)** · `brainstorm` · _draft_ · `2026-08-05` + > `valid_dlw_load()` always calls `valid_aux_load()`, which invokes `pipaux::compare_aux_releases()` and `pipaux::compa… ## 2026-08-04 @@ -73,7 +94,7 @@ _Generated 2026-08-04 · 103 artifacts (newest first) + 0 roadmap features_ ## 2026-06-17 -- **[Refactor valid_dlw_load: logging, abort, and content-hash comparison](.cg-docs/plans/2026-06-17-valid-dlw-load-refactor.md)** · `plan` · _active_ · `2026-06-17` +- **[Refactor valid_dlw_load: logging, abort, and content-hash comparison](.cg-docs/plans/2026-06-17-valid-dlw-load-refactor.md)** · `plan` · _completed_ · `2026-06-17` > Implement the 5 decisions from the 2026-06-05 brainstorm: ## 2026-06-05 diff --git a/.cg-docs/BRAIN.md b/.cg-docs/BRAIN.md index e98f68c..aef1996 100644 --- a/.cg-docs/BRAIN.md +++ b/.cg-docs/BRAIN.md @@ -1,6 +1,6 @@ # 🧠 Project Brain -_Generated 2026-08-04 · 103 entities · 2 topics · 27 edges_ +_Generated 2026-08-07 · 109 entities · 2 topics · 32 edges_ ## How to Use @@ -13,26 +13,26 @@ _Generated 2026-08-04 · 103 entities · 2 topics · 27 edges_ | # | Topic | Entities | File | |---|-------|----------|------| -| 1 | [Reporting_Level / Aaa.R / Pd_Deflation.R](BRAIN-01.md#reporting-level-aaa-r-pd-deflation-r) | 75 | BRAIN-01.md | +| 1 | [Reporting_Level / Aaa.R / Valid_Dlw_Load\(\)](BRAIN-01.md#reporting-level-aaa-r-valid-dlw-load) | 83 | BRAIN-01.md | | 2 | [Old_Files/ / Archive / Devtools::Document\(\)](BRAIN-01.md#old-files-archive-devtools-document) | 4 | BRAIN-01.md | ## Entity Summary | Type | Count | |------|-------| -| brainstorm | 8 | -| plan | 33 | -| review | 20 | -| solution | 41 | +| brainstorm | 9 | +| plan | 35 | +| review | 22 | +| solution | 42 | | strategy | 1 | -_24 entities not assigned to any topic_ +_22 entities not assigned to any topic_ ## Relationship Summary | Edge Type | Count | |-----------|-------| -| decided_from | 8 | -| references | 3 | -| reviews | 14 | -| verifies | 2 | +| decided_from | 9 | +| references | 4 | +| reviews | 16 | +| verifies | 3 | diff --git a/.cg-docs/active-state/current.json b/.cg-docs/active-state/current.json index 2243263..204e4b7 100644 --- a/.cg-docs/active-state/current.json +++ b/.cg-docs/active-state/current.json @@ -1,30 +1,40 @@ { - "plan": ".cg-docs/plans/2026-08-03-pipeline-wrapper-vignettes.md", - "executionReport": ".cg-docs/work-reports/2026-08-03-pipeline-wrapper-vignettes.md", - "currentPhase": 2, + "plan": ".cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md", + "executionReport": ".cg-docs/work-reports/2026-08-06-aux-version-gate-valid-dlw-load-revised.md", + "currentPhase": null, "completedPhases": [1, 2], - "lastUpdated": "2026-08-04", + "lastUpdated": "2026-08-07", "refs": { - "roadmapFeature": "new-orchestration-script", + "roadmapFeature": "aux-version-gate-valid-dlw-load", "roadmapMilestone": "Pipeline Alignment Audit" }, "decisions": [ - "Deflation is confirmed NOT integrated into pd_process_data(); vignettes must describe it as a standalone post-hoc step.", - "docs/pipeline_overview.qmd has two known stalenesses (purrr::map vs lapply; 'deflated' output claim) - do not copy verbatim.", - "No old .png diagrams (pipeline_flow.png, pd_functions.png) are reused in any rewritten/new vignette.", - "2026-08-04 mid-execution deviation (user-confirmed, plan amended first per deviation-policy 'ask'): added R6 - new Validating-Data.Rmd vignette covering pipdata_dlw_process() internals (pipdata_get_gmd, pipdata_validate_gmd); PIP-data-pipeline.Rmd cross-reference updated to link both companion articles; Processing-Data.Rmd scope unchanged.", - "2026-08-04 Step 3 complete: Processing-Data.Rmd fully rewritten around pd_process_data/deflation/log_report, knitr::knit() verified by user, all referenced functions confirmed exported. _pkgdown.yml updated (Step 5) with articles: and reference: indices; pkgdown not in DESCRIPTION Suggests so DESCRIPTION left untouched (C2) - V3 syntax/build verification still pending user run.", - "2026-08-04 Step 6 (final validation pass) complete: devtools::document() run by user, regenerated pd_process_data.Rd and log_report.Rd confirmed to exactly match prior manual patches. devtools::check() run by user - only 3 pre-existing NOTEs (future file timestamps; wbpip:::md_clean_data unexported import; object.size undefined global in save_pip_data), none attributable to this change (no touched file involved). Plan complete." + "Roadmap feature aux-version-gate-valid-dlw-load updated to status active and plan path repointed to the revised 2026-08-06 plan.", + "Phase 1 complete (Steps 1-2): get_aux_hashes() resolver added in R/utils.R; wired into pd_process_data() before aux loading; build_pip_inventory() gains aux_hashes param and writes aux__hash columns on current-run rows, initialised to NA on assembled master for legacy/retained rows.", + "Phase 2 complete (Step 3): valid_dlw_load() rewritten with two-stage aux gating. Master loaded once (shared dt_master) and passed to inv_to_process() (now accepts dt_master) and aux_hash_candidates(). Stage 1 builds candidates from per-survey aux hash comparison over full filtered/latest inventory; missing historical hash is a candidate; conflicting same-survey/content hashes abort. Stage 2 runs valid_aux_load() only for changed measures and intersects affected surveys with candidates. force=TRUE skips master/aux comparisons and processes all rows.", + "Tests: test-get-aux-hashes.R (7), test-build_pip_inventory.R (3 aux-hash), test-valid_dlw_load.R (9 new gating + updated existing), test-valid_aux_load.R (updated verbose propagation). Full suite PASS (2 pre-existing skips). roxygenise regenerated .Rd files.", + "Plan marked status: completed, completed-date: 2026-08-07. Roadmap feature aux-version-gate-valid-dlw-load set to done.", + "Verification review 2026-08-07: full R suite PASS (0 failures, 2 pre-existing skips). Both light verification agents returned no usable output due to model-service 'Response contained no choices' errors; not retried per protocol." ], "evidenceStatus": { - "V1": "passed (all three vignettes: PIP-data-pipeline.Rmd, Validating-Data.Rmd, Processing-Data.Rmd; knitr::knit() pandoc-free verification)", - "V2": "passed (all three vignettes; no stale/archived references, no deflated mischaracterization)", - "V3": "passed (_pkgdown.yml articles:/reference: index added; devtools::document()/check() ran cleanly against it, no pkgdown-related errors)", - "V4": "passed (devtools::check() - only 3 pre-existing NOTEs, none attributable to this change)", - "V5": "passed (no pipeline_flow.png reference in PIP-data-pipeline.Rmd)", - "V6": "passed (no pd_functions.png reference in Processing-Data.Rmd - grep-confirmed 2026-08-04)", - "V7": "passed (Validating-Data.Rmd renders; all referenced functions exist and exported)", - "V8": "passed (PIP-data-pipeline.Rmd cross-reference links both Validating-Data.html and Processing-Data.html)" + "V1": "passed (resolver tests: all six measures incl. PFW)", + "V2": "passed (missing/ambiguous/empty/failure/missing-hash abort tests)", + "V3": "passed (pd_process_data resolves hashes once before aux loading)", + "V4": "passed (build_pip_inventory persists hashes on current-run rows)", + "V5": "passed (legacy/retained rows keep NA; schema initialised)", + "V6": "passed (master inventory loaded exactly once)", + "V7": "passed (changed measures only invoke valid_aux_load)", + "V8": "passed (COL/ARG vs USA/GER intersection test)", + "V9": "passed (conflicting same-survey hashes abort)", + "V10": "passed (new/DLW-changed surveys remain selected)", + "V11": "passed (force mode skips master/aux comparisons)", + "V12": "passed (no .joyn column, no duplicate survey IDs)", + "V13": "passed (targeted valid_dlw_load/valid_aux_load tests)", + "V14": "passed (full suite, 2 pre-existing skips)", + "V15": "passed (roxygen regenerated; context.md unchanged - no canonical logging semantics changed)", + "verification": "full suite passed; agent verification incomplete due to model-service errors" }, - "nextCommand": "none - plan complete" + "nextCommand": "/cg-review mode:verify (rerun light agents if independent reviewer output is required)" } + + diff --git a/.cg-docs/brain-index.json b/.cg-docs/brain-index.json index c8d6926..9c4a3dc 100644 --- a/.cg-docs/brain-index.json +++ b/.cg-docs/brain-index.json @@ -1,5 +1,5 @@ { - "edge_count": 27, + "edge_count": 32, "edges": [ { "edge_type": "decided_from", @@ -49,6 +49,18 @@ "target": ".cg-docs/brainstorms/2026-08-03-pipeline-wrapper-vignettes.md", "target_missing": false }, + { + "edge_type": "decided_from", + "source": ".cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md", + "target": ".cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md", + "target_missing": false + }, + { + "edge_type": "reviews", + "source": ".cg-docs/reviews/2026-04-06-enrich-log-report-review.md", + "target": ".cg-docs/plans/2026-04-06-enrich-log-report.md", + "target_missing": false + }, { "edge_type": "reviews", "source": ".cg-docs/reviews/2026-04-27-dplyr-to-collapse-phase1-review.md", @@ -145,6 +157,18 @@ "target": ".cg-docs/plans/2026-05-20-inventory-architecture-refactor.md", "target_missing": false }, + { + "edge_type": "reviews", + "source": ".cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md", + "target": ".cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md", + "target_missing": false + }, + { + "edge_type": "verifies", + "source": ".cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review.md", + "target": ".cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md", + "target_missing": false + }, { "edge_type": "references", "source": ".cg-docs/brainstorms/2026-04-28-unified-logging.md", @@ -162,6 +186,12 @@ "source": ".cg-docs/brainstorms/2026-08-03-pipeline-wrapper-vignettes.md", "target": ".cg-docs/plans/2026-08-03-pipeline-wrapper-vignettes.md", "target_missing": false + }, + { + "edge_type": "references", + "source": ".cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md", + "target": ".cg-docs/plans/2026-08-05-aux-version-gate-valid-dlw-load.md", + "target_missing": false } ], "entities": [ @@ -394,6 +424,36 @@ "approach" ] }, + { + "date": "2026-08-05", + "entity_type": "brainstorm", + "path": ".cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md", + "slug": "2026-08-05-aux-version-gate-valid-dlw-load", + "status": "draft", + "summary": "`valid_dlw_load()` always calls `valid_aux_load()`, which invokes `pipaux::compare_aux_releases()` and `pipaux::compare_aux_vintages()`. These compare the *current* aux data against the *previous release/vintage* in the aux data's own history — **not** against what the pipeline actually used in its last run. As a result, every pipeline run detects \"changes\" relative to the previous aux release/vintage and re-cleans all affected surveys, even when the pipeline already cleaned them with that same aux data.", + "tags": [ + "valid_dlw_load", + "valid_aux_load", + "aux", + "stamp", + "content_hash", + "master-inventory", + "pipeline" + ], + "title": "Gate aux-change detection in valid_dlw_load on aux version change", + "top_keywords": [ + "valid_dlw_load()", + "compare_aux_*", + "valid_aux_load()", + "aux", + "st_catalog_query()", + "pd_aux_attr()", + "build_pip_inventory()", + "st_catalog_query", + "approach", + "valid_dlw_load" + ] + }, { "date": "2026-04-06", "entity_type": "plan", @@ -1111,7 +1171,7 @@ "entity_type": "plan", "path": ".cg-docs/plans/2026-06-17-valid-dlw-load-refactor.md", "slug": "2026-06-17-valid-dlw-load-refactor", - "status": "active", + "status": "completed", "summary": "Implement the 5 decisions from the 2026-06-05 brainstorm:", "tags": [ "pipeline", @@ -1311,6 +1371,57 @@ "devtools::check()" ] }, + { + "date": "", + "entity_type": "plan", + "path": ".cg-docs/plans/2026-08-05-aux-version-gate-valid-dlw-load.md", + "slug": "2026-08-05-aux-version-gate-valid-dlw-load", + "status": "", + "summary": "--- date: 2026-08-05 title: \"Gate aux-change detection in valid_dlw_load on per-survey aux content-hash comparison\" status: active scope: \"Standard\" brainstorm: \".cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md\" language: \"R\" estimated-effort: \"medium\" deviation-policy: \"ask\" artifact-schema-version: 1 tags: [valid_dlw_load, valid_aux_load, aux, stamp, content_hash, master-inventory, pipeline, incremental-processing] phases: 2 # convenience hint -- may be stale; always recount from ## Phase headers", + "tags": [], + "title": "2026-08-05-aux-version-gate-valid-dlw-load", + "top_keywords": [ + "valid_dlw_load()", + "valid_aux_load()", + "st_catalog_query()", + "inv_to_process()", + "compare_aux_*", + "pd_aux_attr()", + "pip_meta", + "content_hash", + "build_pip_inventory()", + "aux" + ] + }, + { + "date": "2026-08-06", + "entity_type": "plan", + "path": ".cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md", + "slug": "2026-08-06-aux-version-gate-valid-dlw-load-revised", + "status": "completed", + "summary": "Prevent unnecessary survey re-cleaning by comparing the current content hash of each requested auxiliary artifact with the hash recorded for previously cleaned surveys, while retaining the detailed `pipaux::compare_aux_*` filter that identifies which requested surveys were actually affected.", + "tags": [ + "valid_dlw_load", + "aux", + "stamp", + "content_hash", + "master-inventory", + "incremental-processing" + ], + "title": "Aux version gate for valid_dlw_load — revised run-level hash design", + "top_keywords": [ + "valid_dlw_load()", + "aux", + "content_hash", + "force = true", + "valid_aux_load()", + "build_pip_inventory()", + "pip_meta", + "st_latest()", + "run-level", + "pfw" + ] + }, { "date": "", "entity_type": "review", @@ -1751,6 +1862,50 @@ "fix" ] }, + { + "date": "2026-08-07", + "entity_type": "review", + "path": ".cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md", + "slug": "2026-08-06-aux-version-gate-valid-dlw-load-revised-review", + "status": "", + "summary": "**Review mode**: standard **Files reviewed**: 33 changed paths from `origin/refactor_dlw_load..HEAD` **Findings**: 17 (P0: 0, P1: 5, P2: 10, P3: 2)", + "tags": [], + "title": "2026-08-06-aux-version-gate-valid-dlw-load-revised-review", + "top_keywords": [ + "valid_dlw_load()", + "pip_id", + "survey_id", + "cg-documentation", + "build_pip_inventory()", + "aux_hashes", + "cg-testing", + "cg-reproducibility", + "cg-architecture", + "verbose" + ] + }, + { + "date": "2026-08-07", + "entity_type": "review", + "path": ".cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review.md", + "slug": "2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review", + "status": "", + "summary": "**Review mode**: mode:verify / light **Parent review**: `.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md` **Verification scope**: committed fixes in `origin/refactor_dlw_load..HEAD`", + "tags": [], + "title": "2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review", + "top_keywords": [ + "verification", + "valid_dlw_load()", + "2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md", + "incomplete", + "review", + "aux", + "result", + "version", + "valid_dlw_load", + "reviewer" + ] + }, { "date": "2026-04-07", "entity_type": "solution", @@ -2563,6 +2718,39 @@ "columns" ] }, + { + "date": "2026-08-07", + "entity_type": "solution", + "path": ".cg-docs/solutions/data-quality/2026-08-07-aux-content-hash-gated-recleaning.md", + "slug": "2026-08-07-aux-content-hash-gated-recleaning", + "status": "", + "summary": "`valid_dlw_load()` always invoked `valid_aux_load()`, which compares current auxiliary releases/vintages and returns changed country/year keys. This caused surveys to be considered for re-cleaning even when the pipeline had already processed them with the same auxiliary artifact. A global auxiliary table hash alone is insufficient: an updated CPI or PPP table may contain changes for countries that are not among the surveys requested in the current DLW inventory. The pipeline needs both a global artifact check and a row-level affected-survey check. The master inventory also has an important identity rule: its current snapshot contains one row per `(survey_id, pip_id)`, while stamp preserves...", + "tags": [ + "auxiliary-data", + "content_hash", + "stamp", + "valid_dlw_load", + "valid_aux_load", + "master-inventory", + "pip_id", + "survey_id", + "joyn", + "incremental-processing" + ], + "title": "Gate auxiliary re-cleaning with run-level content hashes and row-level changes", + "top_keywords": [ + "pip_id", + "valid_dlw_load()", + "survey_id", + "content_hash", + "content_hash_dlw", + "auxiliary", + "gate", + "hashes", + "current", + "aux" + ] + }, { "date": "2026-04-30", "entity_type": "solution", @@ -3009,13 +3197,13 @@ ] } ], - "entity_count": 103, - "generated": "2026-08-04", + "entity_count": 109, + "generated": "2026-08-07", "schema_version": "0.2.0", "topic_count": 2, "topics": [ { - "entity_count": 75, + "entity_count": 83, "entity_paths": [ ".cg-docs/brainstorms/2026-04-27-dplyr-to-collapse-dt.md", ".cg-docs/brainstorms/2026-04-28-unified-logging.md", @@ -3023,6 +3211,8 @@ ".cg-docs/brainstorms/2026-05-06-subnational-deflation-area-resolution.md", ".cg-docs/brainstorms/2026-05-20-inventory-architecture-refactor.md", ".cg-docs/brainstorms/2026-05-27-inventory-delta-strategy.md", + ".cg-docs/brainstorms/2026-06-05-valid-dlw-load-review.md", + ".cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md", ".cg-docs/plans/2026-04-06-enrich-log-report.md", ".cg-docs/plans/2026-04-23-remove-redundant-get-wrk-release.md", ".cg-docs/plans/2026-04-27-dplyr-to-collapse-phase1.md", @@ -3041,9 +3231,12 @@ ".cg-docs/plans/2026-05-19-fix-inventory-save-reporting-level.md", ".cg-docs/plans/2026-05-19-fix-reporting-level-duplication.md", ".cg-docs/plans/2026-05-20-inventory-architecture-refactor.md", + ".cg-docs/plans/2026-06-17-valid-dlw-load-refactor.md", ".cg-docs/plans/2026-06-18-save-pip-memory-resilience.md", ".cg-docs/plans/2026-06-24-pd-process-data-oom-crash.md", ".cg-docs/plans/2026-06-28-pd-process-data-ram-regression.md", + ".cg-docs/plans/2026-08-05-aux-version-gate-valid-dlw-load.md", + ".cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md", ".cg-docs/reviews/2026-04-06-enrich-log-report-review.md", ".cg-docs/reviews/2026-04-16-review.md", ".cg-docs/reviews/2026-04-27-branch-review.md", @@ -3064,6 +3257,8 @@ ".cg-docs/reviews/2026-05-19-fix-inventory-save-reporting-level-review.md", ".cg-docs/reviews/2026-05-19-fix-reporting-level-duplication-review.md", ".cg-docs/reviews/2026-05-20-inventory-architecture-refactor-review.md", + ".cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md", + ".cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review.md", ".cg-docs/solutions/bugs/2026-04-07-internal-logmeta-types-polluting-type-summary.md", ".cg-docs/solutions/bugs/2026-04-29-conditional-column-init-inconsistent-schema.md", ".cg-docs/solutions/bugs/2026-04-29-duplicate-logmeta-discriminator-key.md", @@ -3086,6 +3281,7 @@ ".cg-docs/solutions/data-quality/2026-05-19-guard-before-join-reentrant-column.md", ".cg-docs/solutions/data-quality/2026-05-27-legacy-column-persistence-in-on-disk-inventory.md", ".cg-docs/solutions/data-quality/2026-06-05-joyn-diagnostic-column-discipline.md", + ".cg-docs/solutions/data-quality/2026-08-07-aux-content-hash-gated-recleaning.md", ".cg-docs/solutions/environment-issues/2026-04-30-unified-package-environment-accessor-pattern.md", ".cg-docs/solutions/git-workflows/2026-04-23-guard-at-entry-point-pattern.md", ".cg-docs/solutions/performance-issues/2026-07-22-per-survey-logging-retains-large-survey-objects.md", @@ -3096,10 +3292,10 @@ "keywords": [ "reporting_level", "aaa.r", - "pd_deflation.r" + "valid_dlw_load()" ], - "label": "Reporting_Level / Aaa.R / Pd_Deflation.R", - "slug": "reporting-level-aaa-r-pd-deflation-r" + "label": "Reporting_Level / Aaa.R / Valid_Dlw_Load()", + "slug": "reporting-level-aaa-r-valid-dlw-load" }, { "entity_count": 4, diff --git a/.cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md b/.cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md new file mode 100644 index 0000000..3216afc --- /dev/null +++ b/.cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md @@ -0,0 +1,146 @@ +--- +date: 2026-08-05 +title: "Gate aux-change detection in valid_dlw_load on aux version change" +status: draft +chosen-approach: "Per-survey aux content_hash columns in master inventory, compared via st_catalog_query" +participants: + - User + - Copilot +tags: [valid_dlw_load, valid_aux_load, aux, stamp, content_hash, master-inventory, pipeline] +--- + +# Gate aux-change detection in `valid_dlw_load()` on aux version change + +## Context + +`valid_dlw_load()` always calls `valid_aux_load()`, which invokes +`pipaux::compare_aux_releases()` and `pipaux::compare_aux_vintages()`. These +compare the *current* aux data against the *previous release/vintage* in the +aux data's own history — **not** against what the pipeline actually used in +its last run. As a result, every pipeline run detects "changes" relative to +the previous aux release/vintage and re-cleans all affected surveys, even +when the pipeline already cleaned them with that same aux data. + +Goal: only run the `compare_aux_*` changes functions when the aux data +version actually differs from the version used in the last pipeline run. + +## Requirements + +- R1: Record the version (content hash) of each aux measure used when a + survey was cleaned. +- R2: Before running `compare_aux_*`, compare the current aux content hash + against the stored last-run hash; only run the changes functions when they + differ. +- R3: Track all aux measures (`pfw`, `cpi`, `ppp`, `pop`, `gdp`, `pce`). +- R4: Obtain the current aux content hash from `st_catalog_query()` on the + aux alias (no full aux data load). +- R5: Gate only the aux-change detection path. DLW-new surveys are still + always detected and processed. +- R6: Store the aux versions per-survey in the PIP master inventory (new + columns), not in survey metadata attributes. + +## Approaches Considered + +### Approach 1: Per-survey aux content_hash columns in master inventory (CHOSEN) + +Add columns to the master inventory such as `aux_cpi_hash`, `aux_ppp_hash`, +`aux_pfw_hash`, `aux_pop_hash`, `aux_gdp_hash`, `aux_pce_hash`. These are +populated per-survey during processing (captured in `pd_aux_attr()` from the +aux data actually used) and surfaced into the master inventory by +`build_pip_inventory()`. + +`valid_dlw_load()` then: +1. Queries the current aux content hashes via `st_catalog_query()` on the + aux alias. +2. Loads the master inventory and compares each survey's stored aux hashes + against the current hashes. +3. Only runs `valid_aux_load()` / `compare_aux_*` when a survey's stored aux + hash differs from the current aux hash. + +**Pros**: Precise per-survey tracking (surveys cleaned at different times +with different aux versions are handled correctly). Reuses the existing +master-inventory read path (`load_pip_master_inventory()`). Content-hash +comparison is robust to re-saves with identical content. No per-survey stamp +reads at comparison time (fast). +**Cons**: Adds columns to the master inventory schema (requires a +`legacy_cols` migration note if ever removed). Requires capturing aux hashes +during processing. +**Effort**: Medium. + +### Approach 2: Run-level aux-version snapshot artifact + +Store a single metadata artifact (e.g. `aux_versions` in `pip_meta`) holding +the aux hashes used in the last run, written at the end of each run. + +**Pros**: Single record, simple to read. +**Cons**: Loses per-survey precision; a survey cleaned in an earlier run with +older aux would not be re-cleaned when only that survey's aux changed. +**Effort**: Small. + +### Approach 3: Read stored aux versions directly from each survey's pip_meta + +**Pros**: No inventory schema change. +**Cons**: Requires per-survey stamp reads at comparison time (slower); +couples `valid_dlw_load()` to metadata internals. +**Effort**: Medium. + +## Decision + +**Approach 1: Per-survey aux content_hash columns in the master inventory.** + +- Aux versions are captured per-survey during processing (in `pd_aux_attr()`), + from the aux data actually used, and surfaced into the master inventory by + `build_pip_inventory()`. +- Current aux hashes come from `st_catalog_query()` on the aux alias. +- Comparison is content-hash based. +- Only the aux-change detection path is gated; DLW-new surveys are always + processed. + +## Refined Flow (confirmed 2026-08-05) + +The comparison happens **inside `valid_dlw_load()`** (not `valid_aux_load()`), +because it is the only place with access to both the stored aux hashes (master +inventory) and the current aux hashes (`st_catalog_query`). It applies to +**every measure** in the `aux_measures` param. + +Two-stage filter: + +1. **Stage 1 — aux version changed (cheap)**: For each previously-cleaned + survey, compare its stored per-survey aux hash (master inventory) against + the current aux hash (`st_catalog_query`), per measure. Surveys whose + stored hash differs from current → **candidate** set. New surveys (never + cleaned) are always processed. A previously-cleaned survey with **no stored + aux hash** (cleaned before this feature) is treated as changed → re-clean. +2. **Stage 2 — actual aux changes (detailed)**: For the **changed measures + only**, run `valid_aux_load()` / `compare_aux_*` (which accept measures, + not a survey set — pipaux is not modified), then **intersect** the affected + surveys with the candidate set. Only surveys with actual changes inside the + aux file are processed. + +`compare_aux_*` is **kept** but **gated**: it only runs for changed measures, +and its results are intersected with the candidate set. + +## Consequences + +- Master inventory schema gains aux hash columns (one per tracked measure). +- `valid_dlw_load()` needs the current aux hashes (via `st_catalog_query`) + and the stored per-survey hashes (via `load_pip_master_inventory()`). +- The `compare_aux_*` functions are only invoked when a real aux change is + detected, avoiding unnecessary re-cleaning. +- Existing logmeta entries (`aux_changes_inf`, `aux_no_changes_inf`, + `aux_changes_no_surveys_inf`, `surveys_to_clean_inf`) remain, but their + trigger conditions change to be based on the version comparison. + +## Next Steps + +1. Add aux content-hash capture in `pd_aux_attr()` (per-survey, from aux data + used). +2. Surface aux hash columns into the master inventory in `build_pip_inventory()`. +3. Add a helper to fetch current aux hashes via `st_catalog_query()`. +4. Rewrite the aux-change detection in `valid_dlw_load()` to implement the + two-stage filter: (a) per-survey stored-vs-current aux hash comparison to + build the candidate set, (b) run `valid_aux_load()`/`compare_aux_*` for + changed measures only and intersect with the candidate set. +5. Update tests for `valid_dlw_load()`, `valid_aux_load()`, `pd_aux_attr()`, + and `build_pip_inventory()`. +6. Update roxygen `@details` and `compound-gpid.context.md` logmeta notes. diff --git a/.cg-docs/plans/2026-06-17-valid-dlw-load-refactor.md b/.cg-docs/plans/2026-06-17-valid-dlw-load-refactor.md index 9d09ecb..5607890 100644 --- a/.cg-docs/plans/2026-06-17-valid-dlw-load-refactor.md +++ b/.cg-docs/plans/2026-06-17-valid-dlw-load-refactor.md @@ -1,13 +1,14 @@ --- date: 2026-06-17 title: "Refactor valid_dlw_load: logging, abort, and content-hash comparison" -status: active +status: completed +completed-date: 2026-08-04 scope: "Standard" brainstorm: ".cg-docs/brainstorms/2026-06-05-valid-dlw-load-review.md" language: "R" estimated-effort: "medium" phases: 2 -completed-phases: [] +completed-phases: [1, 2] deviation-policy: "ask" tags: [pipeline, inventory, valid_dlw_load, inv_to_process, filter_aux_inv, logging, content_hash] --- diff --git a/.cg-docs/plans/2026-08-05-aux-version-gate-valid-dlw-load.md b/.cg-docs/plans/2026-08-05-aux-version-gate-valid-dlw-load.md new file mode 100644 index 0000000..83d17db --- /dev/null +++ b/.cg-docs/plans/2026-08-05-aux-version-gate-valid-dlw-load.md @@ -0,0 +1,284 @@ +--- +date: 2026-08-05 +title: "Gate aux-change detection in valid_dlw_load on per-survey aux content-hash comparison" +status: active +scope: "Standard" +brainstorm: ".cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md" +language: "R" +estimated-effort: "medium" +deviation-policy: "ask" +artifact-schema-version: 1 +tags: [valid_dlw_load, valid_aux_load, aux, stamp, content_hash, master-inventory, pipeline, incremental-processing] +phases: 2 # convenience hint -- may be stale; always recount from ## Phase headers + +# Plan: Gate aux-change detection in `valid_dlw_load()` on per-survey aux content-hash comparison + +## Objective + +Make `valid_dlw_load()` only re-clean surveys whose auxiliary data actually +changed since they were last cleaned. Today it always runs +`valid_aux_load()` → `compare_aux_releases()`/`compare_aux_vintages()`, which +compare the current aux data against the previous aux release/vintage — not +against what the pipeline used last run — so every run re-cleans affected +surveys unnecessarily. + +## Context + +- `valid_dlw_load()` (R/valid_dlw_load.R) currently calls `valid_aux_load()` + unconditionally, then `filter_aux_inv()` to find affected surveys. +- `valid_aux_load()` (R/valid_aux_load.R) calls + `pipaux::compare_aux_releases()` and `pipaux::compare_aux_vintages()`, which + accept `measure` (not a survey set) — pipaux is a separate package and must + not be modified. +- Aux data is versioned via **stamp**. `st_catalog_query(alias)` returns latest + version metadata per artifact: `path`, `version_id`, `content_hash`, + `code_hash`, `size_bytes`, `created_at`. +- Aux data is loaded via `pipload::load_aux_data(measure)`, which resolves the + aux alias from the working release's `aux_data` folder. +- The master inventory is built by `build_pip_inventory()` (R/build_pip_inventory.R), + which upserts per-`pip_id` from stamp catalogs and already carries + `content_hash_dlw` (DLW source hash) per survey. It has a `legacy_cols` + drop-list applied to `old_inv` in Step 1. +- `inv_to_process()` already compares DLW `content_hash` vs `content_hash_dlw` + on `survey_id` (the `content-hash-reclean-trigger` feature, done). This plan + extends the same content-hash pattern to **aux** data. + +## Requirements + +| ID | Requirement | Source | +|----|-------------|--------| +| R1 | Capture the content hash of each aux measure used when a survey is cleaned, per-survey | Brainstorm | +| R2 | Surface per-survey aux hash columns into the master inventory | Brainstorm | +| R3 | Fetch current aux content hashes via `st_catalog_query()` (no full aux data load) | Brainstorm | +| R4 | In `valid_dlw_load()`, build a candidate set of previously-cleaned surveys whose stored aux hash differs from the current aux hash (per measure) | Brainstorm | +| R5 | New surveys (no master row) are always processed | Brainstorm | +| R6 | Previously-cleaned surveys with no stored aux hash are treated as changed (re-clean) | Brainstorm | +| R7 | Run `valid_aux_load()`/`compare_aux_*` only for the changed measures, then intersect affected surveys with the candidate set | Brainstorm | +| R8 | Gating applies to every measure in the `aux_measures` param | Brainstorm | +| R9 | DLW-new survey detection path (`inv_to_process()`) unchanged | Brainstorm | +| R10 | Existing logmeta discriminators (`aux_no_changes_inf`, `aux_changes_no_surveys_inf`, `aux_changes_inf`, `surveys_to_clean_inf`) preserved with updated trigger conditions | Brainstorm | + +## Phase 1: Capture and persist aux hashes + +### 1. Capture per-survey aux content hashes in `pd_aux_attr()` +- **Requirements**: R1 +- **Files**: `R/pd_aux_attr.R` +- **Details**: `pd_aux_attr()` receives `aux_list` (named list of aux + data.tables, one per measure). For each measure in `aux_list`, obtain its + current content hash via `st_catalog_query()` on the aux alias (resolved the + same way `load_aux_data()` does — from the working release's `aux_data` + folder). Attach the hash as a scalar attribute on each survey's attribute + list, e.g. `attr(ls, "aux_cpi_hash")` / a named element `aux_cpi_hash` in the + returned attribute list. Do **not** add these as columns to the cleaned + survey data.table — they belong in the metadata attribute list returned by + `pd_aux_attr()`. + - Add a small internal helper (e.g. `get_aux_hash(measure)`) that resolves + the aux alias and calls `st_catalog_query()` to return the latest + `content_hash` for the measure's artifact. Reuse the alias-resolution logic + pattern from `pipload::load_aux_data()` (via `pipfun::get_pip_folders("aux_data")` + + `stamp::st_alias_list()`). + - If a measure's hash cannot be resolved (artifact missing), fail loudly + (`cli::cli_abort`) per project convention — no silent fallback. +- **Test Scenarios**: + - happy path: `pd_aux_attr()` returns attribute lists each carrying the aux + hash for every measure in `aux_list`. + - edge case: `aux_list` contains a measure not in the catalog → abort. + - error path: `st_catalog_query()` fails → abort with clear message. +- **Tests**: `tests/testthat/test-pd-aux-attr.R` (new or extended). +- **Acceptance criteria**: Each survey's metadata attribute list carries the + aux content hash for every measure used. + +### 2. Surface aux hash columns into the master inventory in `build_pip_inventory()` +- **Requirements**: R2 +- **Files**: `R/build_pip_inventory.R` +- **Details**: `build_pip_inventory()` reads version facts from stamp catalogs + (`"pip"`, `"pip_meta"`, `"pip_inv"`) and joins them per `pip_id`. The aux + hashes are stored in the `"pip_meta"` metadata artifacts (as attributes from + Step 1). Add columns to the master inventory such as `aux_cpi_hash`, + `aux_ppp_hash`, `aux_pfw_hash`, `aux_pop_hash`, `aux_gdp_hash`, + `aux_pce_hash`, populated from the metadata attributes of each survey's + `pip_meta` artifact. + - Determine the cleanest read path: either (a) read the metadata attributes + from the `pip_meta` catalog/artifact for current-run pip_ids, or (b) if + `st_catalog_query` does not expose attributes, load each current-run + `pip_meta` artifact via `pip_read()` and extract the aux hash attributes. + Prefer the least-expensive path that is reliable. + - Add the new columns to the `legacy_cols` drop-list in Step 1 of + `build_pip_inventory()` so that if they are ever removed, on-disk masters + migrate cleanly (see + `.cg-docs/solutions/data-quality/2026-05-27-legacy-column-persistence-in-on-disk-inventory.md`). + - Ensure the upsert (`collapse::rowbind(new_versions, old_retained, fill = TRUE)`) + does not break on class mismatches for the new columns (they are character + hashes — no special class). +- **Test Scenarios**: + - happy path: master inventory rows for current-run pip_ids carry the aux + hash columns. + - edge case: a survey's `pip_meta` artifact lacks an aux hash attribute → + column is NA (survey cleaned before this feature). + - error path: `pip_meta` artifact cannot be read → abort or warn per existing + catalog-failure handling. +- **Tests**: `tests/testthat/test-build_pip_inventory.R` (extend). +- **Acceptance criteria**: Master inventory has one aux hash column per tracked + measure, populated for current-run surveys. + +## Phase 2: Compare and gate in valid_dlw_load + +### 3. Implement two-stage aux-change gating in `valid_dlw_load()` +- **Requirements**: R3, R4, R5, R6, R7, R8, R9, R10 +- **Files**: `R/valid_dlw_load.R` +- **Details**: Rewrite the aux-change detection portion of `valid_dlw_load()`. + The comparison happens **inside `valid_dlw_load()`** (not `valid_aux_load()`), + because it is the only place with access to both the stored aux hashes + (master inventory) and the current aux hashes (`st_catalog_query`). + + **Stage 1 — build candidate set (cheap)**: + - Load the master inventory (already done for `inv_to_process()`; reuse the + loaded `dt_master`). + - For each measure in `aux_measures`, fetch the current aux content hash via + `st_catalog_query()` (helper from Step 1, or a shared helper). + - For each previously-cleaned survey (present in master), compare its stored + per-survey aux hash (from the master's `aux__hash` columns) against + the current hash for that measure. If any measure's stored hash differs from + current → the survey is a **candidate**. + - New surveys (no master row) are always processed (unchanged path). + - Previously-cleaned surveys with **no stored aux hash** (NA) are treated as + changed → candidate (R6). + - Track which measures changed (for Stage 2). + + **Stage 2 — actual aux changes (detailed)**: + - For the **changed measures only**, call `valid_aux_load(measure = , compare = "all")` → `compare_aux_*` → `filter_aux_inv()` to find + surveys with actual changes inside the aux file. + - **Intersect** the affected surveys with the candidate set. Only surveys in + both sets are re-cleaned. + - If no measures changed → skip `valid_aux_load()` entirely, log + `aux_no_changes_inf`, `inv_aux = NULL`. + + **Logging**: preserve the existing discriminators with updated trigger + conditions: + - `aux_no_changes_inf` — no measure's aux hash changed (Stage 1 empty). + - `aux_changes_no_surveys_inf` — measures changed but no candidate survey is + actually affected (Stage 2 empty). + - `aux_changes_inf` — at least one survey is affected and will be re-cleaned. + - `surveys_to_clean_inf` — after combining/dedup, with counts. + + **Constraints**: all `joyn::` joins use `reportvar = FALSE`; `verbose` + propagated to all downstream I/O; `logmeta$info` stays a string discriminator. +- **Test Scenarios**: + - happy path: survey with unchanged stored aux hash is NOT re-cleaned. + - happy path: survey with changed stored aux hash IS re-cleaned. + - happy path: new survey always processed. + - edge case: previously-cleaned survey with no stored aux hash → re-cleaned. + - edge case: measure changed but no candidate actually affected → no re-clean, + `aux_changes_no_surveys_inf`. + - error path: `st_catalog_query()` fails → abort. + - regression: DLW-new detection (`inv_to_process()`) still runs and is + unchanged. +- **Tests**: `tests/testthat/test-valid_dlw_load.R` (extend), plus + `tests/testthat/test-valid_aux_load.R` if the helper is shared. +- **Acceptance criteria**: `valid_dlw_load()` only re-cleans surveys whose aux + actually changed; DLW-new surveys always processed; all logmeta + discriminators fire correctly. + +## Testing Strategy + +- Unit tests with `local_mocked_bindings()` for `st_catalog_query`, + `load_pip_master_inventory`, `valid_aux_load`, `filter_aux_inv` (existing + pattern in `test-valid_dlw_load.R`). +- Extend `test-build_pip_inventory.R` for the new aux hash columns. +- Extend/add `test-pd-aux-attr.R` for aux hash capture. +- Run `devtools::test(filter = "valid_dlw")` and `devtools::test(filter = + "build_pip_inventory")` after each phase; full `devtools::test()` at the end. + +## Documentation Checklist + +- Update roxygen `@details` on `valid_dlw_load()` describing the two-stage + gating and updated logmeta trigger conditions. +- Update `compound-gpid.context.md` canonical logmeta list if trigger + conditions change. +- Update `man/valid_dlw_load.Rd` via `devtools::document()`. + +## Risks & Mitigations + +| Risk | Mitigation | +|------|------------| +| `st_catalog_query()` cannot resolve aux artifact hashes (aux not in a queryable catalog) | Verify aux alias resolution; fall back to loading aux data and hashing if needed (documented decision) | +| Adding aux columns breaks the master-inventory upsert/rowbind (class mismatch) | New columns are character hashes; add to `legacy_cols` drop-list; test the upsert | +| Gating accidentally skips DLW-new surveys | Keep `inv_to_process()` path unchanged; add regression test (C5) | +| Per-survey hash comparison misses a survey cleaned with a different aux version | Per-survey columns handle this precisely; test multi-version scenario | +| `compare_aux_*` results not intersecting correctly with candidate set | Explicit intersection step; test both empty and non-empty intersections | + +## Out of Scope + +- Changing the DLW `content_hash` comparison in `inv_to_process()` (already done). +- Modifying `compare_aux_*` / `valid_aux_load()` behavior in pipaux. +- Deflation changes. +- Roadmap writes (handled by `@cg-roadmap`). + +## Completion Contract + +### Outcome + +`valid_dlw_load()` gates aux-change detection using a two-stage filter: (1) +per-survey stored-vs-current aux content-hash comparison (master inventory vs +`st_catalog_query`) builds a candidate set of surveys whose aux version +changed; (2) `valid_aux_load()`/`compare_aux_*` runs only for the changed +measures and its results are intersected with the candidate set. Aux versions +are captured per-survey during processing and persisted as new columns in the +master inventory. + +### Verification Surface + +| ID | Evidence Required | Command/Artifact | Required | +|----|-------------------|------------------|----------| +| V1 | Aux content hashes captured per-survey in `pd_aux_attr()` | Code review + unit test | yes | +| V2 | Aux hash columns surfaced into master inventory by `build_pip_inventory()` | Code review + unit test | yes | +| V3 | Helper fetches current aux hashes via `st_catalog_query()` | Code review + unit test | yes | +| V4 | Stage 1: survey with unchanged stored aux hash is NOT a candidate | Unit test (mock) | yes | +| V5 | Stage 1: survey with changed stored aux hash IS a candidate | Unit test (mock) | yes | +| V6 | Stage 1: new survey (no master row) always processed | Unit test (mock) | yes | +| V7 | Stage 1: previously-cleaned survey with no stored aux hash treated as changed | Unit test (mock) | yes | +| V8 | Stage 2: `valid_aux_load()` called only for changed measures | Unit test (mock) | yes | +| V9 | Stage 2: affected surveys intersected with candidate set | Unit test (mock) | yes | +| V10 | `devtools::test(filter = "valid_dlw")` passes | Test command | yes | +| V11 | `devtools::test()` full suite passes, no regressions | Test command | yes | +| V12 | Roxygen `@details` + `compound-gpid.context.md` logmeta notes updated | Code review | yes | + +### Constraints + +| ID | Constraint | Check | +|----|------------|-------| +| C1 | All `joyn::` joins use `reportvar = FALSE` | Code review | +| C2 | `logmeta$info`/`logmeta$error` remain string discriminators | Code review | +| C3 | New master-inventory columns added to `legacy_cols` drop-list if ever removed | Code review | +| C4 | Content-hash comparison (not version_id) is the change signal | Code review | +| C5 | DLW-new survey detection path unchanged (still always runs) | Code review | +| C6 | `verbose` propagated to all downstream I/O calls | Code review | +| C7 | `compare_aux_*` (pipaux) not modified; only measures passed, results intersected | Code review | +| C8 | Gating applies to every measure in `aux_measures` param | Code review | + +### Boundaries + +- Allowed: New aux hash columns in master inventory; new helper for current aux + hashes; two-stage gating logic in `valid_dlw_load()`; capture in + `pd_aux_attr()`; tests; docs. +- Out of scope: Changing the DLW `content_hash` comparison in `inv_to_process()` + (already done); modifying `compare_aux_*` in pipaux; deflation changes; + roadmap writes (handled by `@cg-roadmap`). + +### Iteration Policy + +1. Implement capture (Step 1) → persist (Step 2) → compare/gate (Step 3) in + order. +2. After each step, run targeted tests; fix failures before proceeding. +3. If a step reveals a schema or API conflict, stop and consult the user. +4. Final full-suite run must pass before completion. + +### Blocked-Stop Conditions + +- If `st_catalog_query()` cannot resolve aux artifact hashes (aux not in a + queryable catalog), stop and re-evaluate the "current aux hash" source. +- If adding aux columns to the master inventory breaks the upsert/rowbind + (class mismatch), stop and address schema migration. +- If the aux-change gating causes DLW-new surveys to be skipped, stop (this + violates C5). diff --git a/.cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md b/.cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md new file mode 100644 index 0000000..7b9f3b5 --- /dev/null +++ b/.cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md @@ -0,0 +1,294 @@ +--- +date: 2026-08-06 +title: "Aux version gate for valid_dlw_load — revised run-level hash design" +status: completed +completed-date: 2026-08-07 +scope: "Standard" +brainstorm: ".cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md" +prior-plan: ".cg-docs/plans/2026-08-05-aux-version-gate-valid-dlw-load.md" +language: "R" +estimated-effort: "medium" +deviation-policy: "ask" +artifact-schema-version: 1 +tags: [valid_dlw_load, aux, stamp, content_hash, master-inventory, incremental-processing] +phases: 2 +completed-phases: [1, 2] +execution-report: ".cg-docs/work-reports/2026-08-06-aux-version-gate-valid-dlw-load-revised.md" +--- + +# Plan: Aux version gate for `valid_dlw_load()` — revised run-level hash design + +## Objective + +Prevent unnecessary survey re-cleaning by comparing the current content hash of +each requested auxiliary artifact with the hash recorded for previously cleaned +surveys, while retaining the detailed `pipaux::compare_aux_*` filter that +identifies which requested surveys were actually affected. + +## Context + +`valid_dlw_load()` currently invokes `valid_aux_load()` on every run. The +comparison functions detect changes within auxiliary data, but the pipeline has +no gate showing whether the current auxiliary table is the same table already +used for prior cleaning. + +The confirmed design is deliberately limited: + +- Resolve current hashes once from `stamp::st_catalog_query(alias = "aux")`. +- Use the catalog `content_hash`; do not call `st_latest()`. +- Include every requested measure, including `pfw`. +- Keep the run-level hash map in memory and pass it explicitly through the run. +- Write hashes directly to master-inventory rows for successfully processed + surveys; do not add hash attributes to `pip_meta` or read serialized metadata + artifacts. +- Load the master inventory once in `valid_dlw_load()`. +- Retain both global hash gating and detailed `compare_aux_*` filtering. +- When `force = TRUE`, skip all comparisons and process every filtered/latest + survey. + +## Requirements + +| ID | Requirement | Source | +|----|-------------|--------| +| R1 | Resolve one current `content_hash` for every requested aux measure from the `aux` stamp catalog | User decision | +| R2 | Include all requested measures, including `pfw` | User decision | +| R3 | Resolve hashes once before aux data loading and pass the run-level map explicitly | User decision | +| R4 | Persist aux hash columns directly in master-inventory rows produced by the run | User decision | +| R5 | Load the master inventory at most once in `valid_dlw_load()` and share it with DLW and aux comparisons | User decision | +| R6 | Compare stored hashes against current hashes for previously cleaned surveys | Brainstorm/review | +| R7 | Run `valid_aux_load()` only for measures whose global hash changed | User decision | +| R8 | Intersect detailed aux changes with requested candidate surveys | User decision | +| R9 | Abort on conflicting aux hashes for the same `survey_id` and `content_hash_dlw` | User decision | +| R10 | Treat missing historical hashes as migration candidates | Review decision | +| R11 | Preserve new/DLW-content-changed survey processing | Existing behavior | +| R12 | `force = TRUE` bypasses master/hash/aux comparisons and processes all filtered/latest surveys | User decision | + +## Phase 1: Resolve and persist run-level hashes + +### 1. Add a run-level auxiliary hash resolver + +- **Requirements**: R1, R2, R3 +- **Files**: `R/pd_process_data.R`; likely a new internal helper in + `R/valid_dlw_load.R` or `R/utils.R`. +- **Details**: + - Query `stamp::st_catalog_query(alias = "aux")` once. + - For every value in `aux_measures`, match exactly the artifact whose path + is the measure basename with `.qs2`, such as `cpi.qs2`, `ppp.qs2`, + `pfw.qs2`, `pop.qs2`, `gdp.qs2`, or `pce.qs2`. + - Return a named character vector or data.table keyed by measure containing + the `content_hash` and, if useful for provenance, the matched path and + catalog `version_id`. + - Abort loudly when the aux alias is unavailable, a requested artifact is + missing, or multiple rows match a measure. + - Resolve the map before `lapply(aux_measures, pipload::load_aux_data, ...)`. + - Do not call `st_latest()` and do not compute hashes from loaded tables. +- **Test scenarios**: + - all six default measures resolve, including PFW; + - duplicate or missing catalog artifact aborts; + - an arbitrary subset of `aux_measures` resolves only those measures; + - catalog query failure aborts clearly. +- **Tests**: extend `tests/testthat/test-valid_dlw_load.R` or add a focused + resolver test file. +- **Acceptance criteria**: one deterministic current `content_hash` exists for + every requested measure before aux data loading begins. + +### 2. Thread hashes into `build_pip_inventory()` and persist them + +- **Requirements**: R3, R4, R10 +- **Files**: `R/pd_process_data.R`, `R/build_pip_inventory.R`, + `tests/testthat/test-build_pip_inventory.R`. +- **Details**: + - Pass the run-level hash map to `build_pip_inventory()` explicitly. + - Add columns such as `aux_cpi_hash`, `aux_ppp_hash`, `aux_pfw_hash`, + `aux_pop_hash`, `aux_gdp_hash`, and `aux_pce_hash` to rows produced for + successful current-run surveys. + - Preserve old master rows unchanged when they are not reprocessed. + - Ensure missing columns in legacy master inventories are filled with `NA` + and do not cause `collapse::rowbind(..., fill = TRUE)` class conflicts. + - If the assembler is called without a hash map by an existing caller, + define and test the compatible behavior rather than silently inventing + hashes. + - Do not add these values to `pd_aux_attr()` or `pip_meta`. +- **Test scenarios**: + - current-run rows receive all supplied hashes; + - old retained rows remain intact; + - missing historical hash columns migrate safely; + - partial requested measures leave non-requested measure columns as `NA`. +- **Acceptance criteria**: reloading the master inventory shows the expected + hashes on successfully processed current-run rows. + +## Phase 2: Gate and filter aux changes + +### 3. Load master once and implement two-stage aux filtering + +- **Requirements**: R5, R6, R7, R8, R9, R11, R12 +- **Files**: `R/valid_dlw_load.R`, `tests/testthat/test-valid_dlw_load.R`. +- **Details**: + - Define an explicit master-loading handoff. Load the master once in + `valid_dlw_load()` and pass the same object to both the DLW comparison + helper and the aux-hash comparison helper. Refactor `inv_to_process()` as + needed so it accepts the supplied master rather than loading it again. + - Preserve the existing master-load fallback semantics explicitly. If the + master cannot be loaded, do not claim previously cleaned surveys are + unchanged; treat the available DLW inventory as needing processing and + define the aux candidate behavior in tests. + - Apply module filtering and `last_ver_inv()` before survey-level comparison. + - Reduce master rows to one row per `survey_id` for the same + `content_hash_dlw`. Require all relevant rows in that group to have equal + aux hashes; abort on conflict. This protects the invariant that split + `pip_id`s for one survey/content version use the same aux versions. + - Stage 1: for each requested measure, compare the survey-level stored hash + to the current run's catalog hash. A mismatch or missing historical hash + makes the survey a candidate; new/DLW-content-changed surveys remain on + the normal `inv_to_process()` path. + - Stage 2: call `valid_aux_load(measure = changed_measures, compare = "all")` + only when at least one requested measure has a changed global hash. Use + `filter_aux_inv()` and intersect its affected survey IDs with the + requested candidate inventory. This preserves the distinction between a + globally changed table and changed rows for requested countries/surveys. + - Preserve existing logging discriminators with revised meanings: + `aux_no_changes_inf` when no requested global hash changed, + `aux_changes_no_surveys_inf` when changed measures produce no affected + requested survey, `aux_changes_inf` when the intersection is non-empty, + and `surveys_to_clean_inf` after final deduplication. + - When `force = TRUE`, do not load the master for comparison, resolve/use + no comparison candidates, and do not call `valid_aux_load()`; process all + filtered/latest surveys. The current run's hashes are still persisted by + Phase 1. + - Ensure all production joins retain `reportvar = FALSE` and that verbose + values propagate to downstream I/O. +- **Test scenarios**: + - unchanged hash skips that measure's aux comparison; + - changed hash invokes comparison only for that measure; + - changed CPI affecting USA/GER but not requested COL/ARG produces no COL/ARG + re-cleaning; + - changed CPI affecting a requested survey returns that survey; + - new and DLW-content-changed surveys remain processable; + - missing historical hash is a candidate but still must pass detailed + affected-row filtering; + - conflicting hashes for one survey/content-hash group abort; + - master loader is called once; + - force mode skips master/hash/aux comparisons and processes all rows; + - no `.joyn` column or duplicate survey IDs appear in output. +- **Acceptance criteria**: only requested surveys with actual changed aux rows + are added through the aux path, while new/DLW-changed surveys continue to be + selected normally. + +## Testing Strategy + +- Use `testthat::local_mocked_bindings()` for `stamp::st_catalog_query`, + `pipload::load_pip_master_inventory`, `valid_aux_load`, and the refactored + `inv_to_process()` handoff. +- Add resolver tests for every supported measure and catalog ambiguity. +- Add assembler tests for hash propagation and retained legacy rows. +- Add integration-style gating tests for global hash change versus detailed + country/survey change. +- Run targeted tests after each phase: + `devtools::test(filter = "valid_dlw_load")` and + `devtools::test(filter = "build_pip_inventory")`. +- Run `devtools::test()` as the final regression gate. + +## Documentation Checklist + +- Update `valid_dlw_load()` roxygen details to document the two-stage hash and + detailed-change filter, one-time master loading, conflict abort, migration + behavior, and force mode. +- Update `build_pip_inventory()` roxygen column provenance with aux hash + columns and the run-level source. +- Update `pd_process_data()` roxygen to describe resolving aux catalog hashes + once and passing them to inventory assembly. +- Update `compound-gpid.context.md` only if the canonical logging semantics or + inventory schema notes change. +- Regenerate affected `.Rd` files with `devtools::document()`. + +## Risks & Mitigations + +| Risk | Mitigation | +|------|------------| +| Aux artifact path matching is ambiguous | Match exact `.qs2` basename; abort on zero or multiple matches; test all requested measures | +| Hash map is lost before inventory assembly | Pass it explicitly through `pd_process_data()` to `build_pip_inventory()`; verify reload round trip | +| Master inventory is loaded twice or under inconsistent fallback semantics | Centralize the load in `valid_dlw_load()` and pass the object into comparison helpers; add call-count tests | +| Split `pip_id`s contain conflicting aux hashes | Reduce by `survey_id`/`content_hash_dlw`; abort on conflict | +| Global aux change affects unrelated countries | Keep `valid_aux_load()` and intersect affected keys with the requested candidate inventory | +| Force mode is accidentally gated | Add regression test proving no master/aux comparison calls occur when `force = TRUE` | +| Existing masters lack aux hash columns | Fill missing columns with `NA`; treat missing hashes as migration candidates; test one-time behavior | + +## Out of Scope + +- Aux hash attributes in `pd_aux_attr()` or `pip_meta`. +- Reading serialized metadata artifacts to recover hashes. +- `st_latest()` or hashing loaded aux tables. +- Modifying `pipaux::compare_aux_*`. +- Deflation changes. +- Rich metadata distinguishing non-applicable rows from not-captured legacy rows. + +## Completion Contract + +### Outcome + +The pipeline resolves current aux `content_hash` values once from the `aux` +stamp catalog, passes them through the run, and persists them directly in +master-inventory rows for successfully processed surveys. `valid_dlw_load()` +loads the master once and uses a global hash gate followed by detailed +`compare_aux_*` filtering; force mode processes all filtered/latest surveys +without comparison. + +### Verification Surface + +| ID | Evidence Required | Command/Artifact | Required | +|----|-------------------|------------------|----------| +| V1 | Exact current hash resolution for all requested measures, including PFW | Resolver tests | yes | +| V2 | Missing/ambiguous aux catalog artifacts abort | Resolver tests | yes | +| V3 | Run-level hashes resolved once before aux loading | `pd_process_data()` test | yes | +| V4 | Hashes persisted on successful current-run master rows | `build_pip_inventory()` test and reload | yes | +| V5 | Legacy master rows without hashes handled as migration candidates | Gating test | yes | +| V6 | Master inventory loaded exactly once | Call-count test | yes | +| V7 | Changed measures only invoke `valid_aux_load()` | Gating test | yes | +| V8 | Detailed aux changes intersect with requested surveys | COL/ARG versus USA/GER test | yes | +| V9 | Conflicting same-survey hashes abort | Validation test | yes | +| V10 | New and DLW-content-changed surveys remain selected | Regression test | yes | +| V11 | Force mode skips all comparison paths and processes all rows | Regression test | yes | +| V12 | No duplicate survey IDs or `.joyn` columns | Output assertions | yes | +| V13 | Targeted tests pass | `devtools::test(filter = "valid_dlw_load")` | yes | +| V14 | Full suite passes | `devtools::test()` | yes | +| V15 | Roxygen and context documentation reflect the revised behavior | Documentation review | yes | + +### Constraints + +| ID | Constraint | Check | +|----|------------|-------| +| C1 | Use `st_catalog_query(alias = "aux")` `content_hash`; never `st_latest()` | Code review | +| C2 | Include every requested `aux_measures` value, including `pfw` | Tests | +| C3 | Do not add aux hash attributes to `pip_meta` | Code review | +| C4 | Load master inventory at most once in `valid_dlw_load()` | Call-count test | +| C5 | Abort on same-survey/content-hash aux conflicts | Test | +| C6 | Retain both global hash and detailed row-level filters | Integration test | +| C7 | `force = TRUE` bypasses comparisons and processes all filtered/latest surveys | Test | +| C8 | Production `joyn::` joins use `reportvar = FALSE` | Code review | +| C9 | External `pipaux` package is not modified | Diff review | + +### Boundaries + +- Allowed: run-level aux hash resolver, explicit hash propagation, + master-inventory hash columns, single-load master handoff, two-stage aux + filtering, conflict validation, tests, and documentation. +- Out of scope: metadata attributes, serialized metadata reads, `st_latest`, + pipaux changes, deflation, and rich legacy applicability states. + +### Iteration Policy + +1. Implement and test the resolver. +2. Thread hashes into inventory assembly and verify reload persistence. +3. Refactor the single-master-load handoff and two-stage aux filtering. +4. Run targeted tests after each step. +5. Run the full suite before completion. +6. Stop and consult the user if the confirmed `aux` catalog layout or API does + not hold in the configured working release. + +### Blocked-Stop Conditions + +- A requested measure has zero or multiple matching `.qs2` catalog rows. +- Hash propagation cannot be verified in the reloaded master inventory. +- Same-survey/content-hash rows contain conflicting aux hashes. +- `force = TRUE` still invokes master or aux comparison. +- Detailed aux results cannot be intersected reliably with requested surveys. diff --git a/.cg-docs/reviews/2026-04-06-enrich-log-report-review.md b/.cg-docs/reviews/2026-04-06-enrich-log-report-review.md index ea1290b..c038b0c 100644 --- a/.cg-docs/reviews/2026-04-06-enrich-log-report-review.md +++ b/.cg-docs/reviews/2026-04-06-enrich-log-report-review.md @@ -1,3 +1,17 @@ +--- +plan: .cg-docs/plans/2026-04-06-enrich-log-report.md +findings: + P2.1: open + P2.2: open + P2.3: open + P2.4: open + P3.1: open + P3.2: open + P3.3: open + P3.4: open + P3.5: open +--- + ## Review Report **Review depth**: standard diff --git a/.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md b/.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md new file mode 100644 index 0000000..ccca014 --- /dev/null +++ b/.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md @@ -0,0 +1,136 @@ +--- +date: 2026-08-07 +depth: standard +type: standard +plan: .cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md +findings: + P1.1: fixed + P1.2: fixed + P1.3: fixed + P1.4: fixed + P1.5: fixed + P2.1: fixed + P2.2: fixed + P2.3: fixed + P2.4: fixed + P2.5: fixed + P2.6: fixed + P2.7: fixed + P2.8: fixed + P2.9: fixed + P2.10: fixed + P2.11: skipped + P3.1: fixed + P3.2: fixed +--- + +# Review Report: Aux version gate for `valid_dlw_load()` + +**Review mode**: standard +**Files reviewed**: 33 changed paths from `origin/refactor_dlw_load..HEAD` +**Findings**: 17 (P0: 0, P1: 5, P2: 10, P3: 2) + +## P1 — CRITICAL + +- **[P1.1]** `R/valid_dlw_load.R` — Historical master rows are joined only by `survey_id`. + **Why**: `aux_hash_candidates()` groups by `survey_id + content_hash_dlw` but joins to current inventory using only `survey_id`. A survey with multiple retained historical DLW hashes can violate `many-to-one` or compare against the wrong historical aux hash. + **Fix**: Match current `inv$content_hash` to `master$content_hash_dlw` together with `survey_id` before reducing and joining. Add a multiple-history regression test. + **Source**: `cg-code-quality`, `cg-testing`, `cg-reproducibility`, `cg-data-quality`. + +- **[P1.2]** `R/build_pip_inventory.R` — Partial successful `pip_id` replacement can leave conflicting aux hashes within one survey. + **Why**: Upsert removes retained rows by reprocessed `pip_id`, then applies hashes only to `new_versions`. If a split survey has only a subset of `pip_id`s successfully persisted, remaining rows retain old hashes. + **Fix**: When a survey is reprocessed, drop ALL of its old rows and replace them with the fresh catalog data for that survey (retention is now by `survey_id`, not `pip_id`). This ensures the survey's pip_id set matches the current reprocess exactly; stale pip_id rows (e.g. a welfare-type split that no longer exists) are removed and remain recoverable via stamp. Added a stale-pip_id-drop regression test. + **Source**: `cg-data-quality`. + +- **[P1.3]** `valid_dlw_load()` silently disables auxiliary detection when direct callers omit `aux_hashes`. + **Why**: Before this change, direct calls always invoked `valid_aux_load()`. With `aux_hashes = NULL`, the new code skips the aux path without warning. + **Fix**: Preserve compatibility by resolving hashes internally, or fail loudly when hashes are omitted; do not silently skip detection. + **Source**: `cg-architecture`. + +- **[P1.4]** Public positional argument compatibility is broken. + **Why**: `aux_hashes` was inserted before existing `verbose` in `valid_dlw_load()` and `build_pip_inventory()`, and `dt_master` before `verbose` in `inv_to_process()`. + **Fix**: Append new parameters after existing arguments or preserve old positional bindings explicitly. Add positional-call tests. + **Source**: `cg-architecture`. + +- **[P1.5]** `valid_dlw_load()` and `build_pip_inventory()` are not tested with a real stateful persistence round trip. + **Why**: `pip_write()` mocks return version metadata but do not capture written master/release objects and return them on reload. + **Fix**: Add a stateful write/load mock test proving aux hashes and retained rows survive persistence and reload. + **Source**: `cg-testing`. + +## P2 — IMPORTANT + +- **[P2.1]** `R/valid_dlw_load.R` — Failed master load can trigger a second load. + **Why**: `valid_dlw_load()` stores unavailable master as `NULL`; `inv_to_process(dt_master = NULL)` interprets that as permission to load again. + **Fix**: Use an explicit unavailable sentinel or separate supplied/available flag so the fallback state is shared without retrying. + **Source**: `cg-architecture`. + +- **[P2.2]** `aux_hashes` input is not validated. + **Why**: Unnamed, duplicate, invalid, or missing hash values can produce empty/ambiguous measure mappings and incorrect candidate behavior. + **Fix**: Validate character type, non-empty unique names, names matching requested measures, and non-missing hashes; abort with a classed error. + **Source**: `cg-code-quality`. + +- **[P2.3]** Catalog duplicates in `build_pip_inventory()` are selected by tied `created_at` order. + **Why**: Equal timestamps make `.SD[1L]` dependent on input row order. + **Fix**: Add a deterministic secondary key or abort on indistinguishable duplicates; test reordered tied rows. + **Source**: `cg-reproducibility`. + +- **[P2.4]** Current aux catalog artifact and loaded aux artifact are not cross-verified. + **Why**: Hashes come from `st_catalog_query(alias = "aux")`, while data comes from `pipload::load_aux_data()`. The same working-release/path relationship is assumed but not verified. + **Fix**: Verify or explicitly document the shared alias/path precondition and add an integration test. + **Source**: `cg-reproducibility`. + +- **[P2.5]** `valid_dlw_load()` documentation incorrectly says the function returns `NULL` when there is no work. + **Why**: The implementation aborts with class `piperr`. + **Fix**: Update roxygen and generated Rd return documentation. + **Source**: `cg-documentation`. + +- **[P2.6]** Retained aux-hash behavior is documented incorrectly. + **Why**: Existing retained rows preserve existing hash values; only absent columns are initialized to `NA`. Comments and the work report say retained rows keep `NA`. + **Fix**: Correct inline and work-report wording and tests. + **Source**: `cg-documentation`. + +- **[P2.7]** Stage 1 documentation says only previously-cleaned surveys are evaluated, but code passes the full filtered/latest inventory and treats new rows without hashes as candidates. + **Fix**: Restrict Stage 1 to master-present rows or revise documentation and logging semantics to describe full-inventory candidate behavior. + **Source**: `cg-documentation`. + +- **[P2.8]** The complete pipeline still loads the master multiple times. + **Why**: The master is loaded once in `valid_dlw_load()`, then again in `build_pip_inventory()`, and again for verification. + **Fix**: Consider a pipeline-level handoff or document the scope of the single-load guarantee and benchmark large inventories. + **Source**: `cg-performance`. + +- **[P2.9]** `aux_hash_candidates()` performs redundant full-master uniqueness scans. + **Fix**: Use one grouped reduction for conflict detection and survey-level output, and restrict to relevant survey IDs where safe. + **Source**: `cg-performance`. + +- **[P2.10]** Detailed aux results are materialized before candidate restriction. + **Fix**: Push the candidate restriction into the filtering path or prefilter the inventory before `filter_aux_inv()` while preserving the requested-survey semantics. + **Source**: `cg-performance`. + +## P3 — MINOR + +- **[P3.1]** `get_aux_hashes()` documents a `verbose` argument that is unused. + **Fix**: Remove it or implement its documented behavior. + **Source**: `cg-documentation`. + +- **[P3.2]** `valid_aux_load(compare = "all")` tests do not verify non-empty release/vintage merge semantics. + **Fix**: Add cases where both branches and only one branch contain changes. + **Source**: `cg-testing`. + +The force-mode test was strengthened to assert that `aux_hash_candidates()` is +also not called when `force = TRUE`. This is an additional safe test fix. + +## Repository hygiene notes + +- The implementation commits do not follow the repository's Conventional Commits convention (`Phase 1`, `Phase 2`). +- The current branch `refactor_dlw_load` does not follow the documented `type/short-description` naming convention. +- No tracked R dependency lockfile was found; this is a repository-level reproducibility gap. +- `.Rbuildignore` already excludes `.cg-docs/`; no P2 is needed for that check. +- `git diff --check` reported a trailing blank line in `.cg-docs/active-state/current.json`. +- No secrets or protected-artifact relocation/deletion issues were found. + +## Passed + +- No P0 findings. +- Production `joyn` calls in the changed path use `reportvar = FALSE`. +- Full R test suite passed before review with 2 pre-existing skips and no failures. +- `get_aux_hashes()` uses catalog `content_hash` and does not call `st_latest()`. diff --git a/.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review.md b/.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review.md new file mode 100644 index 0000000..348290c --- /dev/null +++ b/.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review.md @@ -0,0 +1,58 @@ +--- +date: 2026-08-07 +depth: light +parent-review: .cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md +type: verification +findings: +--- + +# Verification Review: Aux version gate for `valid_dlw_load()` + +**Review mode**: mode:verify / light +**Parent review**: `.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md` +**Verification scope**: committed fixes in `origin/refactor_dlw_load..HEAD` + +## Verification result + +The full R test suite passed after the fix-triage changes: + +- **Result**: PASS +- **Failures**: 0 +- **Skips**: 2 pre-existing empty-test placeholders +- **R version**: 4.5.1 + +The working tree was clean before verification. Fixes are committed in: + +- `439c98b` — Fix bugs 1 +- `845b37e` — P2 and P3 fixes + +## Verification coverage + +The committed changes include: + +- historical `survey_id + content_hash_dlw` matching for DLW/master joins; +- removal of stale `pip_id` rows when a survey is reprocessed; +- internal aux-hash resolution for direct `valid_dlw_load()` callers; +- backward-compatible argument ordering; +- stateful master persistence test; +- no-retry behavior after failed master loading; +- `aux_hashes` validation; +- deterministic catalog duplicate selection; +- documented aux catalog/working-release precondition; +- scoped single-master-load documentation; +- candidate prefiltering before detailed aux comparisons; +- `valid_aux_load(compare = "all")` merge-semantics tests. + +## Incomplete reviewer dispatches + +Both required light verification reviewers failed to return usable output due +to a model-service error: “Response contained no choices.” They were not retried +per protocol. + +- `@cg-code-quality`: incomplete — model-service error +- `@cg-testing`: incomplete — model-service error + +No new findings were recorded from the incomplete reviewer outputs. The full +suite result is the available verification evidence; a future `/cg-review +mode:verify` may rerun the light reviewers if independent agent output is +required. diff --git a/.cg-docs/solutions/data-quality/2026-08-07-aux-content-hash-gated-recleaning.md b/.cg-docs/solutions/data-quality/2026-08-07-aux-content-hash-gated-recleaning.md new file mode 100644 index 0000000..451d243 --- /dev/null +++ b/.cg-docs/solutions/data-quality/2026-08-07-aux-content-hash-gated-recleaning.md @@ -0,0 +1,157 @@ +--- +date: 2026-08-07 +title: "Gate auxiliary re-cleaning with run-level content hashes and row-level changes" +category: "data-quality" +language: "R" +tags: [auxiliary-data, content_hash, stamp, valid_dlw_load, valid_aux_load, master-inventory, pip_id, survey_id, joyn, incremental-processing] +root-cause: "Auxiliary row-change comparison ran on every pipeline invocation without first checking whether the current aux artifact had changed since the survey was cleaned" +severity: "P1" +--- + +# Gate auxiliary re-cleaning with run-level content hashes and row-level changes + +## Problem + +`valid_dlw_load()` always invoked `valid_aux_load()`, which compares current +auxiliary releases/vintages and returns changed country/year keys. This caused +surveys to be considered for re-cleaning even when the pipeline had already +processed them with the same auxiliary artifact. + +A global auxiliary table hash alone is insufficient: an updated CPI or PPP +table may contain changes for countries that are not among the surveys requested +in the current DLW inventory. The pipeline needs both a global artifact check +and a row-level affected-survey check. + +The master inventory also has an important identity rule: its current snapshot +contains one row per `(survey_id, pip_id)`, while stamp preserves previous +inventory snapshots as artifact versions. When a survey is reprocessed, stale +`pip_id` rows from the previous snapshot must be removed if the current +reprocess no longer produces them. + +## Root Cause + +The previous logic had no record of which auxiliary artifact content hash was +used for each successfully cleaned survey. It also joined survey-level master +rows only by `survey_id`, which is ambiguous when a survey has multiple +historical DLW `content_hash_dlw` values. + +The inventory upsert retained old rows by `pip_id`. If a reprocessed survey's +new result had fewer welfare-type rows, such as losing a `CON` or `INC` split, +the obsolete row could remain in the current master snapshot. + +## Solution + +### 1. Resolve current auxiliary hashes once + +Before loading auxiliary data, query the `aux` stamp catalog once and match +requested measures to exact `.qs2` artifacts: + +```r +aux_hashes <- get_aux_hashes(aux_measures, verbose = verbose) +aux_list <- lapply(aux_measures, pipload::load_aux_data, verbose = verbose) +``` + +`get_aux_hashes()` uses the catalog `content_hash`, includes PFW, fails loudly +on missing or ambiguous artifacts, and does not call `st_latest()` or hash the +loaded data tables. + +The hash map is passed explicitly through `pd_process_data()` to +`valid_dlw_load()` and `build_pip_inventory()`. + +### 2. Use a two-stage aux gate + +`valid_dlw_load()` performs: + +1. **Artifact gate** — compare the current run's aux hash with the stored + `aux__hash` for the matching survey and DLW content version. +2. **Affected-row gate** — call `valid_aux_load()` only for measures whose + artifact hash changed, then intersect its changed country/year keys with the + candidate surveys requested in `inv`. + +This prevents a CPI change for USA/GER from re-cleaning already-cleaned COL/ARG +surveys when COL/ARG rows were not changed. + +The master inventory is loaded once within `valid_dlw_load()` and shared with +DLW and aux comparisons. Direct callers that omit `aux_hashes` have hashes +resolved internally; `force = TRUE` bypasses all comparisons. + +### 3. Match the current DLW version explicitly + +For both DLW and aux comparisons, rename the current inventory's +`content_hash` to `content_hash_dlw` for the join and use joyn's explicit +same-name key: + +```r +inv_join <- data.table::copy(inv) +data.table::setnames(inv_join, "content_hash", "content_hash_dlw") + +inv_compare <- joyn::left_join( + inv_join, + dt_master_hash, + by = c("survey_id", "content_hash_dlw"), + relationship = "many-to-one", + reportvar = ".joyn", + verbose = FALSE +) +``` + +The `.joyn` report value identifies matched versus unmatched current DLW +versions. It is removed before returning production data. + +### 4. Replace all rows for a reprocessed survey + +`build_pip_inventory()` retains old master rows by `survey_id`, not by +`pip_id`: + +```r +reprocessed_surveys <- unique(new_versions$survey_id) +old_retained <- old_inv[!old_inv$survey_id %in% reprocessed_surveys] +run_inv <- collapse::rowbind(new_versions, old_retained, fill = TRUE) +``` + +Thus the current master snapshot's `pip_id` set exactly matches the latest +successful reprocess. Previous snapshots remain recoverable through stamp +version history. + +### 5. Validate and test the contracts + +The implementation validates supplied `aux_hashes` as a named, unique, +non-empty character vector with non-missing values. Tests cover: + +- PFW and all requested aux measures; +- historical DLW versions; +- conflicting aux hashes within the same survey/content version; +- stale welfare-type `pip_id` removal; +- direct callers and force mode; +- deterministic catalog tie-breaking; +- stateful master persistence/reload; +- requested-country intersection; +- release/vintage merge semantics in `valid_aux_load(compare = "all")`. + +## Prevention + +- Treat stamp `content_hash` as the artifact-level change gate; do not use + `version_id` as the content-change signal. +- Keep the artifact-level hash gate and the row-level `compare_aux_*` filter; + either one alone is insufficient. +- Always match survey-level master facts by both `survey_id` and the current + DLW `content_hash_dlw`. +- Replace the full current row set for a reprocessed survey; do not retain + stale welfare-type rows from an older snapshot. +- Use explicit `reportvar` settings on joyn joins. If `.joyn` is needed for + match classification, remove it before returning production data. +- Preserve stamp artifact versions for historical recovery, but do not confuse + artifact history with rows in the current loaded master snapshot. +- Run the full test suite after changes to inventory keys, stamp joins, or + aux-change selection logic. + +## Related + +- `.cg-docs/brainstorms/2026-08-05-aux-version-gate-valid-dlw-load.md` +- `.cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md` +- `.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-review.md` +- `.cg-docs/reviews/2026-08-06-aux-version-gate-valid-dlw-load-revised-verify-review.md` +- `.cg-docs/solutions/data-quality/2026-06-05-joyn-diagnostic-column-discipline.md` +- `.cg-docs/solutions/data-quality/2026-05-27-legacy-column-persistence-in-on-disk-inventory.md` +- `.cg-docs/solutions/data-quality/2026-05-27-catalog-validation-before-filter-spurious-warnings.md` +- `.cg-docs/solutions/bugs/2026-05-05-stamp-version-id-vs-content-hash.md` diff --git a/.cg-docs/work-reports/2026-08-04-valid-dlw-load-refactor.md b/.cg-docs/work-reports/2026-08-04-valid-dlw-load-refactor.md new file mode 100644 index 0000000..fb36299 --- /dev/null +++ b/.cg-docs/work-reports/2026-08-04-valid-dlw-load-refactor.md @@ -0,0 +1,138 @@ +--- +plan: ".cg-docs/plans/2026-06-17-valid-dlw-load-refactor.md" +started: 2026-08-04 +status: complete +--- + +# Work Report: Refactor valid_dlw_load — logging, abort, and content-hash comparison + +## Run 1 — 2026-08-04 — Phase 1 (Code changes in valid_dlw_load.R) + +### Step 1: Remove `max_year` clamp from `filter_aux_inv` + +Removed the three-line `max_year` clamp block and its "Temporary fix to test +data from Rossana" comment from `filter_aux_inv()`. The `joyn::inner_join` +below already discards unmatched `surveyid_year` values, so the clamp was +redundant. + +### Step 2: Split all-NULL guard into two distinct log conditions + +Replaced the single all-NULL `if` block in `valid_dlw_load()` with a 3-way +split: +- `"aux_no_changes_inf"` — no aux changes detected at all. +- `"aux_changes_no_surveys_inf"` — aux changed but no surveys match. +- `"aux_changes_inf"` — moved into the `else` branch, gated on `inv_aux` + being non-NULL/non-empty (fires exactly once). + +Deleted the standalone `if (!is.null(all_changes_aux))` block that previously +logged `"aux_changes_inf"`. + +### Step 3: Replace silent `return(NULL)` with `cli_abort` + +Replaced the combined-check `return(NULL)` with +`cli::cli_abort(..., class = "piperr")`. + +### Step 4: Add `surveys_to_clean_inf` summary log entry + +Added a `surveys_to_clean_inf` log entry after the rbind/dedup step with +`n_dlw_new`, `n_aux_changed`, `n_total_unique`, and +`aux_measures_triggered` counts. + +### Phase 1 boundary + +- Ran `devtools::test(filter = "valid_dlw")` — **3 PASS, 0 FAIL** (existing + `inv_to_process` and `valid_dlw_load` tests still pass with the new logging + and abort behavior). +- Updated plan frontmatter: `completed-phases: [1]`, `current-phase: 2`. +- Phase 1 evidence: V1 (max_year clamp removed), V2 (two distinct if branches + + 3 logmeta discriminators), V3 (cli_abort class "piperr"), V4 + (surveys_to_clean_inf log present), V8 (aux_changes_inf fires once in else + branch) — all confirmed by code review; V6 (targeted tests) passed. + +## Run 2 — 2026-08-04 — Phase 2 (Rewrite inv_to_process + tests + documentation) + +### Step 5: Rewrite `inv_to_process` with content_hash comparison + +Verified the blocked-stop precondition first: `content_hash_dlw` is populated +by `build_pip_inventory()` (documented in its `@details` column-provenance +list) and the master inventory's primary key is `c("survey_id", "pip_id")` +(confirmed at `R/build_pip_inventory.R:394,453`), validating the dedup +requirement. + +Replaced the entire `inv_to_process()` body: `joyn::left_join(inv, dt_master[, +.(survey_id, content_hash_dlw)], by = "survey_id", relationship = +"many-to-one", reportvar = FALSE)` after deduplicating `dt_master` via +`collapse::funique()`. Kept `verbose` in the signature, propagated to +`load_pip_master_inventory(verbose = verbose)` and gating the +`cli_alert_warning`. Removed the `_dlw` suffix-strip block and the old 3-key +anti-join. `content_hash_dlw` is dropped from the result after comparison. + +### Step 6: Update tests in test-valid_dlw_load.R + +Updated both existing `load_pip_master_inventory` mocks to the new +`survey_id`/`content_hash_dlw` schema (direct `inv_to_process` test and the +duplicate-survey_id `valid_dlw_load` test). Added a `make_master_hash()` +helper. Added: `filter_aux_inv` max_year-beyond-clamp test (Step 1 coverage); +`aux_no_changes_inf` / `aux_changes_no_surveys_inf` logmeta tests (Step 2 +coverage); `piperr`-abort test (Step 3 coverage); `surveys_to_clean_inf` +log-capture assertion added to the existing happy-path test (Step 4 +coverage); 6 `inv_to_process` content-hash scenarios (new-survey kept, +same-hash excluded, different-hash kept, multi-pip_id no-fan-out, +master-load-error fallback, no `.joyn` column) — all per plan Step 5 test +scenarios. + +### Step 7: Update roxygen @details and compound-gpid.context.md + +Updated `valid_dlw_load()` roxygen `@details` with the 3 new logmeta +discriminator strings (`aux_no_changes_inf`, `aux_changes_no_surveys_inf`, +`surveys_to_clean_inf`) plus the `piperr` abort note. Re-counted the actual +canonical logmeta list in `compound-gpid.context.md` at implementation +time — found **six** existing entries (not the plan's stale "four" +snapshot), added the 3 new ones, corrected the count to **nine**. + +### Phase 2 test runs and regression fix + +- `devtools::document()` — regenerated `man/valid_dlw_load.Rd` and + `man/inv_to_process.Rd`. +- `devtools::test(filter = "valid_dlw")` — **27 PASS, 0 FAIL**. +- `devtools::test()` (full suite) — **1 FAIL** outside + `test-valid_dlw_load.R`: `test-valid_aux_load.R:118` (`"valid_dlw_load + (verbose=FALSE) propagates verbose=FALSE to valid_aux_load()"`) asserted + the old silent `return(NULL)` behavior that Phase 1 Step 3 replaced with + `cli_abort(class = "piperr")` — a plan-named interface change, not an + inferred one. Fixed by wrapping the call in + `expect_error(..., class = "piperr")` while preserving the original + `verbose`-propagation assertion (1 fix attempt, targeted). +- Re-ran `devtools::test()` (full suite) — **425 PASS, 0 FAIL, 2 SKIP** + (pre-existing empty-test placeholders, unrelated to this change). + +### Phase 2 boundary — final phase (M=2) + +- Full-suite gate passed (425 PASS / 0 FAIL). No steps remain in + `failing-steps:`. +- Evidence gate: V1–V8 confirmed (see below). Updated plan frontmatter: + `completed-phases: [1, 2]`; `current-phase` removed (final phase). +- Proceeding directly to Step 3 quality checks / self-review / plan + completion / roadmap update / review handoff (no continue/stop prompt, + per Step 2.5 final-phase rule). + +### Final Verification Surface + +| ID | Evidence | Result | +|----|----------|--------| +| V1 | max_year clamp removed | passed (code review) | +| V2 | two if branches + 2 new discriminators | passed (code review + test) | +| V3 | cli_abort class "piperr" | passed (code review + test) | +| V4 | surveys_to_clean_inf log present | passed (code review + test) | +| V5 | inv_to_process verbose propagation + dedup + relationship="many-to-one" | passed (code review) | +| V6 | devtools::test(filter="valid_dlw") | passed (27 PASS / 0 FAIL) | +| V7 | devtools::test() full suite, no regressions | passed (425 PASS / 0 FAIL / 2 SKIP, after 1 targeted fix to a plan-named interface-change assertion) | +| V8 | aux_changes_inf fires once in else branch | passed (code review + test) | + +### Constraints + +C1 (reportvar=FALSE on all joyn calls), C2 (string-literal logmeta$info), +C3 (graceful fallback on master-load failure), C4 (Checksum not used as +fallback), C5 (context.md count re-verified: 6→9), C6 (dedup + +many-to-one), C7 (both master-inventory mocks updated) — all confirmed by +code review. diff --git a/.cg-docs/work-reports/2026-08-06-aux-version-gate-valid-dlw-load-revised.md b/.cg-docs/work-reports/2026-08-06-aux-version-gate-valid-dlw-load-revised.md new file mode 100644 index 0000000..89de949 --- /dev/null +++ b/.cg-docs/work-reports/2026-08-06-aux-version-gate-valid-dlw-load-revised.md @@ -0,0 +1,94 @@ +--- +plan: ".cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md" +started: 2026-08-06 +status: in-progress +--- + +# Work Report: Aux version gate for valid_dlw_load — revised run-level hash design + +## Run 1 — 2026-08-06 — Phase 1 (Resolve and persist run-level hashes) + +### Step 1: Add a run-level auxiliary hash resolver + +Added `get_aux_hashes()` in `R/utils.R` (internal, `@keywords internal`). It +queries `stamp::st_catalog_query(alias = "aux")` once, matches each requested +measure to exactly one `.qs2` artifact, and returns a named character +vector of `content_hash` values. Aborts loudly on: catalog query failure, +empty catalog, missing artifact, ambiguous (multiple matching) artifact, or +missing content_hash. Never uses `st_latest()` or hashes loaded tables. + +Wired into `pd_process_data()`: `aux_hashes <- get_aux_hashes(aux_measures, ...)` +is resolved before `lapply(aux_measures, pipload::load_aux_data, ...)`. + +Tests: `tests/testthat/test-get-aux-hashes.R` (7 scenarios: all six measures +incl. PFW, subset, missing artifact, ambiguous artifact, empty catalog, catalog +failure, missing hash). + +### Step 2: Thread hashes into build_pip_inventory() and persist them + +Added `aux_hashes = NULL` parameter to `build_pip_inventory()`. In Step 7b, +current-run rows receive one `aux__hash` column per requested measure +from the run-level map. In Step 10, missing aux hash columns are initialised to +`NA_character_` on the assembled master so the schema is always consistent; +old retained rows preserve any existing hash values. `pd_process_data()` passes +`aux_hashes` to `build_pip_inventory()`. + +Tests: extended `tests/testthat/test-build_pip_inventory.R` (3 scenarios: +current-run rows receive hashes; old retained rows keep NA; partial requested +measures leave non-requested columns absent). + +### Phase 1 verification + +- `testthat::test_local(filter = 'get_aux_hashes|build_pip_inventory')` — PASS. +- `testthat::test_local()` full suite — PASS (2 pre-existing skips, no failures). +- `roxygen2::roxygenise('.')` — regenerated `.Rd` files incl. `get_aux_hashes.Rd`, + `build_pip_inventory.Rd`, `pd_process_data.Rd`. Cosmetic link warning for + internal `get_aux_hashes` cross-reference (expected for internal functions). +- `get_aux_hashes` correctly not exported (absent from NAMESPACE). + +Phase 1 evidence: V1 (resolver tests), V2 (missing/ambiguous abort), V3 +(resolved once before aux loading), V4 (hashes persisted on current-run rows), +V5 (legacy rows NA) — all passed. + +## Run 2 — 2026-08-07 — Phase 2 (Gate and filter aux changes) + +### Step 3: Load master once and implement two-stage aux filtering + +Rewrote `valid_dlw_load()` in `R/valid_dlw_load.R`: + +- Added `aux_hashes = NULL` parameter. +- Loads the master inventory **once** (shared `dt_master`) and passes it to + both `inv_to_process()` (DLW comparison) and `aux_hash_candidates()` (aux + comparison). `inv_to_process()` now accepts `dt_master` to avoid a second + load. +- Stage 1: `aux_hash_candidates()` compares each previously-cleaned survey's + stored `aux__hash` against the current hash over the full + filtered/latest inventory. Missing historical hash → candidate (migration). + Aborts on conflicting aux hashes for the same `survey_id`/`content_hash_dlw`. +- Stage 2: `valid_aux_load()` runs only for the changed measures, then + `filter_aux_inv()` results are intersected with the candidate set. +- `force = TRUE` skips master load and all aux comparisons; processes all + filtered/latest surveys. +- Preserved logmeta discriminators with revised trigger conditions. + +`pd_process_data()` passes `aux_hashes` to `valid_dlw_load()`. + +Tests: updated `test-valid_dlw_load.R` (existing scenarios adapted to the +two-stage flow) and `test-valid_aux_load.R` (verbose propagation now exercises +Stage 2). Added 9 new gating tests: unchanged hash skips comparison, changed +hash invokes only changed measures, COL/ARG vs USA/GER non-affected, affected +requested survey returned, missing historical hash candidate, conflicting +hashes abort, master loaded once, force mode skips comparisons, no `.joyn` / +no duplicate survey IDs. + +### Phase 2 verification + +- `testthat::test_local(filter = 'valid_dlw_load|valid_aux_load')` — PASS. +- `testthat::test_local()` full suite — PASS (2 pre-existing skips, no failures). +- `roxygen2::roxygenise('.')` — regenerated `.Rd` files incl. `valid_dlw_load.Rd`, + `inv_to_process.Rd`, `aux_hash_candidates.Rd`. + +Phase 2 evidence: V6 (master loaded once), V7 (changed measures only invoke +valid_aux_load), V8 (COL/ARG vs USA/GER intersection), V9 (conflict abort), +V10 (new/DLW-changed surveys remain), V11 (force mode skips), V12 (no .joyn / +no duplicates), V13 (targeted tests), V14 (full suite) — all passed. diff --git a/Pipdata_script.R b/Pipdata_script.R index 2a52fd6..20c2f5a 100644 --- a/Pipdata_script.R +++ b/Pipdata_script.R @@ -35,16 +35,16 @@ pipfun::setup_working_release( # ) # ----- Load inventory to clean ----- -inv <- pipload::load_gmd_valid_inv(verbose = FALSE) -inv_all <- inv[module == "ALL", ] -# inv_3 <- inv[country_code %in% c("ARG", "MEX") & module == "ALL", ] +# inv <- pipload::load_gmd_valid_inv(verbose = FALSE) +# inv_all <- inv[module == "ALL", ] +# inv_5 <- inv[country_code %in% c("ARG", "COL", "CHN", "IDN","PHL"), ] #--------- Clean surveys and create metadata ----- old_pip_inv <- pipload::load_pip_master_inventory(verbose = FALSE) old_pip_release <- pipload::load_pip_release_inventory(verbose = FALSE) # new_pip_inv <- pd_process_data(inv = inv, force = TRUE, verbose = FALSE) -new_pip_inv <- pd_process_data(inv = inv_all) +new_pip_inv <- pd_process_data(force = TRUE, verbose = TRUE) # Compare inventories waldo::compare(old_pip_inv, new_pip_inv) diff --git a/R/build_pip_inventory.R b/R/build_pip_inventory.R index ca7a80c..ab10f3f 100644 --- a/R/build_pip_inventory.R +++ b/R/build_pip_inventory.R @@ -20,6 +20,11 @@ #' @param pip_id_map A `data.table` with exactly two columns: `survey_id` #' (DLW survey identifier) and `pip_id` (PIP identifier, uppercase). Built #' from successful `process_data()` calls in [pd_process_data()]. +#' @param aux_hashes A named character vector of current aux `content_hash` +#' values, one per requested auxiliary measure (e.g. `cpi`, `ppp`, `pfw`). +#' Resolved once per run by [get_aux_hashes()] and recorded on the +#' master-inventory rows produced for successfully processed surveys. +#' Default `NULL` (no aux hashes recorded). #' #' @return A `data.table`: the updated PIP master inventory. Does **not** #' include `reporting_level` — enrich after load via @@ -39,6 +44,10 @@ #' - `pipeline_version_dlw`, `latest_version_id_dlw`, `content_hash_dlw`, #' `Checksum_dlw`, `path_dlw` — renamed from DLW inventory columns. #' - `welfare_type` — derived from the 4th `_`-delimited segment of `pip_id`. +#' - `aux__hash` (e.g. `aux_cpi_hash`, `aux_ppp_hash`, `aux_pfw_hash`) +#' — current aux `content_hash` for each requested measure, from the +#' run-level `aux_hashes` map passed by [pd_process_data()]. Only populated +#' for surveys successfully processed in the current run. #' - `first_release_version_id`, `latest_release_version_id` — stamp version #' IDs of the release inventory (first appearance and most recent). #' @@ -51,7 +60,8 @@ build_pip_inventory <- function( inv_to_clean, pip_id_map, - verbose = getOption("pipdata.verbose", default = TRUE) + verbose = getOption("pipdata.verbose", default = TRUE), + aux_hashes = NULL ) { #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # Defensive assertions --------- @@ -220,13 +230,15 @@ build_pip_inventory <- function( ) } - # If same pip_id appears at multiple paths, keep the latest by created_at + # If same pip_id appears at multiple paths, keep the latest by created_at, + # with path as a deterministic secondary key so tied timestamps do not + # depend on catalog row order. if (anyDuplicated(cat_data$pip_id) != 0L) { - data.table::setorder(cat_data, pip_id, -created_at) + data.table::setorder(cat_data, pip_id, -created_at, path) cat_data <- cat_data[, .SD[1L], by = "pip_id"] } if (anyDuplicated(cat_meta$pip_id) != 0L) { - data.table::setorder(cat_meta, pip_id, -created_at) + data.table::setorder(cat_meta, pip_id, -created_at, path) cat_meta <- cat_meta[, .SD[1L], by = "pip_id"] } @@ -301,6 +313,19 @@ build_pip_inventory <- function( verbose = FALSE ) + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + # Step 7b: Attach run-level aux hashes --------- + # Record the current aux content_hash for each requested measure on the + # current-run rows. These columns let valid_dlw_load() gate aux-change + # detection against the aux data actually used in this run. + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + if (!is.null(aux_hashes) && length(aux_hashes) > 0L) { + for (m in names(aux_hashes)) { + col <- paste0("aux_", m, "_hash") + data.table::set(new_versions, j = col, value = aux_hashes[[m]]) + } + } + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # Step 8: Derive welfare_type from pip_id --------- # Format: COUNTRY_YEAR_ACRONYM_WELFARE_MODULE. @@ -334,12 +359,17 @@ build_pip_inventory <- function( #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # Step 9: Upsert into old master --------- - # Remove old rows for pip_ids reprocessed this run (replace with fresh - # catalog data). Retain all other surveys from old master unchanged. + # When a survey is reprocessed, ALL of its old rows are dropped and replaced + # by the fresh catalog data for that survey. This ensures the survey's pip_id + # set in the master exactly matches the current reprocess — stale pip_id rows + # from a previous content version (e.g. a welfare-type split that no longer + # exists) are removed. Historical versions remain recoverable via stamp. + # Surveys not reprocessed this run are retained unchanged. # Result: one row per pip_id. #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ if (!is.null(old_inv)) { - old_retained <- old_inv[!old_inv$pip_id %in% new_versions$pip_id] + reprocessed_surveys <- unique(new_versions$survey_id) + old_retained <- old_inv[!old_inv$survey_id %in% reprocessed_surveys] run_inv <- collapse::rowbind(new_versions, old_retained, fill = TRUE) } else { run_inv <- new_versions @@ -372,6 +402,18 @@ build_pip_inventory <- function( run_inv[, latest_release_version_id := NA_character_] } + # Initialise aux hash columns so the master schema is always consistent. + # Old retained rows preserve any existing hashes; missing columns are + # initialised to NA. Current-run rows were populated in Step 7b. + if (!is.null(aux_hashes) && length(aux_hashes) > 0L) { + for (m in names(aux_hashes)) { + col <- paste0("aux_", m, "_hash") + if (!col %in% names(run_inv)) { + run_inv[, (col) := NA_character_] + } + } + } + pfw <- pipload::load_aux_data("pfw", verbose = verbose) pfw_release <- pfw |> diff --git a/R/pd_process_data.R b/R/pd_process_data.R index 4942437..0cd6c34 100644 --- a/R/pd_process_data.R +++ b/R/pd_process_data.R @@ -25,6 +25,13 @@ #' auxiliary file changes and inventory verification are emitted by [valid_dlw_load()] #' and [build_pip_inventory()] respectively. #' +#' **Aux hashes**: the current `content_hash` for every requested auxiliary +#' measure is resolved once from the `"aux"` stamp catalog via +#' [get_aux_hashes()] before aux data is loaded. The run-level hash map is +#' passed to [build_pip_inventory()] and recorded in the master inventory so +#' that [valid_dlw_load()] can gate aux-change detection against the aux data +#' actually used in this run. +#' #' **Recode spec**: the recode specification is synced to stamp once via #' [sync_recode_spec()] before the per-survey loop and the resolved spec is #' threaded into each [process_data()] call, so [apply_recode_spec()] performs @@ -65,6 +72,11 @@ pd_process_data <- function( inv <- pipload::load_gmd_valid_inv(verbose = verbose) } + # Resolve current aux content hashes once, before aux data is loaded. + # These hashes are passed through the run and recorded in the master + # inventory so that aux-change detection can compare against them. + aux_hashes <- get_aux_hashes(aux_measures, verbose = verbose) + # Load aux data for metadata attributes and processing aux_list <- lapply(aux_measures, pipload::load_aux_data, verbose = verbose) names(aux_list) <- aux_measures @@ -74,6 +86,7 @@ pd_process_data <- function( inv = inv, aux_measures = aux_measures, force = force, + aux_hashes = aux_hashes, verbose = verbose ) @@ -155,6 +168,7 @@ pd_process_data <- function( new_pip_inv <- build_pip_inventory( inv_to_clean = inv_to_clean, pip_id_map = pip_id_map, + aux_hashes = aux_hashes, verbose = verbose ) diff --git a/R/utils.R b/R/utils.R index 8622490..7b07c79 100644 --- a/R/utils.R +++ b/R/utils.R @@ -399,3 +399,104 @@ unq_obs_dt <- function(dt, keyVar) { return(dt) } + +#' Resolve current content hashes for auxiliary measures from the aux catalog +#' +#' Queries the `"aux"` stamp catalog once and returns the current +#' `content_hash` for each requested auxiliary measure. Each measure is +#' matched to exactly one catalog artifact whose path basename is +#' `.qs2` (e.g. `cpi.qs2`, `ppp.qs2`, `pfw.qs2`). +#' +#' @param aux_measures Character vector of auxiliary measures to resolve. +#' @param verbose Logical. Retained for consistency with pipeline helpers; +#' catalog lookup itself emits no progress messages. Default: +#' `getOption("pipdata.verbose", default = TRUE)`. +#' +#' @return A named character vector of `content_hash` values, one per +#' requested measure. Names are the measure names. +#' +#' @details +#' This is the single source of the current aux hashes used to gate +#' aux-change detection in [valid_dlw_load()]. It must be called once per +#' pipeline run, before aux data is loaded, and the result passed through +#' the run so that the hashes recorded in the master inventory match the +#' aux data actually used. +#' +#' The function aborts loudly when the `"aux"` alias is unavailable, a +#' requested artifact is missing, or multiple catalog rows match a measure. +#' It never falls back to `stamp::st_latest()` or to hashing loaded aux +#' tables. +#' +#' **Precondition**: the `"aux"` catalog and [pipload::load_aux_data()] must +#' resolve through the same configured working release and storage root. The +#' hashes returned here are only meaningful if the aux data subsequently +#' loaded for processing comes from the same artifacts. Callers must ensure +#' the working release is set up consistently before calling this function. +#' +#' @family pd_process_data pipeline +#' @keywords internal +get_aux_hashes <- function( + aux_measures, + verbose = getOption("pipdata.verbose", default = TRUE) +) { + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + # computations --------- + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + cat_aux <- tryCatch( + stamp::st_catalog_query(alias = "aux"), + error = function(e) { + cli::cli_abort( + c( + "Could not query the aux stamp catalog.", + "i" = "Ensure the working release is set up with an aux_data folder.", + "x" = "Error: {conditionMessage(e)}" + ), + class = c("get_aux_hashes_catalog_failure", "piperr") + ) + } + ) + + if (is.null(cat_aux) || nrow(cat_aux) == 0L) { + cli::cli_abort( + "The aux stamp catalog is empty. Cannot resolve auxiliary hashes.", + class = c("get_aux_hashes_empty_catalog", "piperr") + ) + } + + # Derive the artifact basename (e.g. "cpi.qs2") from each catalog path. + cat_aux[, artifact := fs::path_file(path)] + + hashes <- vapply(aux_measures, function(m) { + target <- paste0(m, ".qs2") + matches <- cat_aux[artifact == target] + + if (nrow(matches) == 0L) { + cli::cli_abort( + "No aux catalog artifact found for measure {.val {m}} (expected {.val {target}}).", + class = c("get_aux_hashes_missing_artifact", "piperr") + ) + } + if (nrow(matches) > 1L) { + cli::cli_abort( + "Multiple aux catalog artifacts match measure {.val {m}} ({.val {target}}).", + class = c("get_aux_hashes_ambiguous_artifact", "piperr") + ) + } + + hash <- matches$content_hash[[1L]] + if (is.na(hash) || !nzchar(hash)) { + cli::cli_abort( + "Aux catalog artifact for measure {.val {m}} has no content_hash.", + class = c("get_aux_hashes_missing_hash", "piperr") + ) + } + + hash + }, character(1)) + + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + # Return --------- + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + return(hashes) +} diff --git a/R/valid_dlw_load.R b/R/valid_dlw_load.R index 950c7fb..6637114 100644 --- a/R/valid_dlw_load.R +++ b/R/valid_dlw_load.R @@ -20,15 +20,55 @@ #' Default: `c("ALL", "GROUP", "HIST", "GPWG", "BIN")`. #' @param force Logical. If `TRUE`, skip the comparison against the #' master inventory and process all surveys. +#' @param aux_hashes A named character vector of current aux `content_hash` +#' values, one per requested auxiliary measure. Resolved once per run by +#' [get_aux_hashes()] and used to gate aux-change detection. When `NULL` +#' (the default) and `force = FALSE`, the hashes are resolved internally so +#' that direct callers retain the previous behavior of always running +#' aux-change detection. #' @param verbose Logical. Print progress messages. Default: #' `getOption("pipdata.verbose", default = TRUE)`. #' -#' @return A `data.table` of surveys to process, or `NULL` if none. +#' @return A `data.table` of surveys to process. If no surveys require +#' processing, the function aborts with class `piperr`. #' #' @details -#' **Logging**: This function writes an `aux_changes_inf` entry to the `"pipdata_log"` -#' when changes are detected in any of the requested auxiliary measures. The logmeta -#' entry includes the measures that changed and the number of affected surveys. +#' **Aux-change gating (two-stage)**: aux-change detection is gated on the +#' current aux `content_hash` values passed via `aux_hashes`. +#' - Stage 1 (cheap): for each filtered/latest survey, compare its stored +#' per-survey aux hash (from the master inventory's `aux__hash` +#' columns) against the current hash for that measure. A mismatch or a +#' missing historical hash makes the survey a candidate. New surveys and +#' DLW-content-changed surveys are also retained through +#' [inv_to_process()] and are deduplicated with the aux candidates. +#' - Stage 2 (detailed): for the changed measures only, [valid_aux_load()] / +#' `compare_aux_*` identifies which requested surveys actually have changed +#' rows inside the aux file. The affected surveys are intersected with the +#' candidate set, so a globally changed aux table that only affects +#' non-requested countries does not re-clean requested surveys. +#' +#' The master inventory is loaded at most once within this function and shared +#' between the DLW comparison and the aux-hash comparison. This guarantee is +#' scoped to `valid_dlw_load()`; downstream steps such as +#' [build_pip_inventory()] load the master again for their own assembly and +#' verification. When `force = TRUE`, no master or aux comparison runs and all +#' filtered/latest surveys are processed. +#' +#' **Logging**: This function writes the following entries to the `"pipdata_log"`: +#' - `aux_changes_inf` — changes were detected in any of the requested auxiliary +#' measures and at least one survey is affected. Includes the measures that +#' changed and the number/list of affected surveys. +#' - `aux_no_changes_inf` — no auxiliary file changes were detected at all. +#' - `aux_changes_no_surveys_inf` — auxiliary files changed but no surveys in +#' the inventory were affected by those changes. +#' - `surveys_to_clean_inf` — emitted once after the DLW-new and aux-changed +#' inventories are combined and deduplicated; includes counts of new, +#' aux-changed, and total unique surveys, plus the aux measures that +#' triggered re-cleaning. +#' +#' When neither new DLW surveys nor auxiliary changes leave anything to +#' process, the function aborts with `cli::cli_abort(class = "piperr")` rather +#' than returning `NULL` silently. #' #' @family pd_process_data pipeline #' @export @@ -37,7 +77,8 @@ valid_dlw_load <- function( aux_measures = c("pfw", "cpi", "ppp", "pop", "gdp", "pce"), modules = c("ALL", "GROUP", "HIST", "GPWG", "BIN"), force = FALSE, - verbose = getOption("pipdata.verbose", default = TRUE) + verbose = getOption("pipdata.verbose", default = TRUE), + aux_hashes = NULL ) { #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # Defenses --------- @@ -51,68 +92,199 @@ valid_dlw_load <- function( # computations --------- #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - # Load changes in aux files - all_changes_aux <- valid_aux_load(measure = aux_measures, compare = "all", verbose = verbose) - ls_inv_aux <- lapply(all_changes_aux, filter_aux_inv, inv = inv) - - # Join release and vintage changes and select unique surveys - if ( - is.null(ls_inv_aux) || - length(ls_inv_aux) == 0 || - all(sapply(ls_inv_aux, is.null)) - ) { - cli::cli_alert_info("No changes in auxiliary files.") - inv_aux <- NULL - } else { - inv_aux <- ls_inv_aux |> - data.table::rbindlist() |> - collapse::funique() + # Filter inventory for specific modules and select last version of each + # survey before any comparison. + inv <- inv[module %in% modules] + inv_svy_full <- last_ver_inv(inv) + + # Load the master inventory once and share it between the DLW comparison + # and the aux-hash comparison. When force = TRUE, no master is loaded and + # all filtered/latest surveys are processed. + # + # master_available distinguishes "master was not supplied" (NULL) from + # "master was supplied but could not be loaded" (FALSE). This prevents + # inv_to_process() from re-loading the master when the load already failed. + dt_master <- NULL + master_available <- FALSE + if (!force) { + dt_master <- tryCatch( + pipload::load_pip_master_inventory(verbose = verbose), + error = function(e) { + if (verbose) { + cli::cli_alert_warning( + "Could not load PIP master inventory. Processing all surveys." + ) + } + NULL + } + ) + master_available <- !is.null(dt_master) } - # Log aux changes if any were detected - if (!is.null(all_changes_aux)) { - changed_measures <- unique(unlist(lapply(all_changes_aux, names))) - n_affected <- if (is.null(inv_aux)) 0L else nrow(inv_aux) - survey_ids_aux <- if (is.null(inv_aux)) character(0) else inv_aux$survey_id - pipfun::log_info( - "Auxiliary file changes detected.", - name = "pipdata_log", - logmeta = list( - info = "aux_changes_inf", - measures = changed_measures, - n_surveys_affected = n_affected, - surveys_affected = survey_ids_aux - ) + # Select valid surveys and compare to previous cleaning (DLW content hash). + # inv_svy holds the DLW-new / DLW-content-changed surveys (may be NULL). + inv_svy <- inv_svy_full + if (!force) { + inv_svy <- inv_to_process( + inv_svy_full, + dt_master = dt_master, + master_available = master_available, + verbose = verbose ) } - # Alert when surveys need re-cleaning due to aux changes. - # Console message is gated by verbose; the log entry above always fires. - if (!is.null(inv_aux) && nrow(inv_aux) > 0L) { - if (verbose) { - cli::cli_alert_warning( - "{nrow(inv_aux)} survey{?s} will be re-cleaned because auxiliary data changed." + # Resolve current aux hashes when the caller did not supply them, so that + # direct callers of valid_dlw_load() retain the previous behavior of always + # running aux-change detection (rather than silently skipping it). + if (!force && is.null(aux_hashes)) { + aux_hashes <- get_aux_hashes(aux_measures, verbose = verbose) + } + + # Validate the aux_hashes input when supplied: it must be a non-empty named + # character vector with unique, non-missing names and non-missing values. + if (!force && !is.null(aux_hashes)) { + if (!is.character(aux_hashes) || length(aux_hashes) == 0L) { + cli::cli_abort( + "aux_hashes must be a non-empty named character vector.", + class = c("valid_dlw_load_bad_aux_hashes", "piperr") + ) + } + if (is.null(names(aux_hashes)) || any(!nzchar(names(aux_hashes)))) { + cli::cli_abort( + "aux_hashes must have non-empty names (one per measure).", + class = c("valid_dlw_load_bad_aux_hashes", "piperr") + ) + } + if (anyDuplicated(names(aux_hashes)) > 0L) { + cli::cli_abort( + "aux_hashes names must be unique.", + class = c("valid_dlw_load_bad_aux_hashes", "piperr") + ) + } + if (any(is.na(aux_hashes)) || any(!nzchar(aux_hashes))) { + cli::cli_abort( + "aux_hashes values must be non-missing, non-empty content hashes.", + class = c("valid_dlw_load_bad_aux_hashes", "piperr") ) } } - # Filter inventory for specific modules and select last version of each survey (and random sample if needed) - # inv <- m_inv_valid(inv, seed = seed) # Mock function to select 20 random surveys from valid inventory - inv <- inv[module %in% modules] - inv_svy <- last_ver_inv(inv) + # Stage 1: build the aux candidate set from the per-survey aux hash + # comparison over the full filtered/latest inventory (all previously-cleaned + # surveys). Only runs when aux_hashes are supplied and the master is + # available (i.e. not force mode). + inv_aux <- NULL + changed_measures <- character(0) + + if (!force && !is.null(aux_hashes) && length(aux_hashes) > 0L && !is.null(dt_master) && nrow(inv_svy_full) > 0L) { + # Count surveys with a missing (NA) stored aux hash. These were cleaned + # before this feature and are ignored for the change comparison. Logged + # regardless of whether any candidate is found. + aux_cols <- paste0("aux_", names(aux_hashes), "_hash") + aux_cols <- intersect(aux_cols, names(dt_master)) + n_na_hash <- 0L + if (length(aux_cols) > 0L) { + n_na_hash <- sum( + Reduce(`|`, lapply(aux_cols, function(col) is.na(dt_master[[col]]))) + ) + } + if (n_na_hash > 0L) { + pipfun::log_info( + "Surveys with no stored aux hash (cleaned before this feature) are ignored for aux-change detection.", + name = "pipdata_log", + logmeta = list( + info = "aux_na_hash_inf", + n_surveys_na_hash = n_na_hash + ) + ) + } - # Select valid surveys and compare to previous cleaning - if (!force) { - inv_svy <- inv_to_process(inv_svy, verbose = verbose) - } else { - # Explicitly pass verbose even when force=TRUE (for consistency in call trace) + candidates <- aux_hash_candidates( + inv = inv_svy_full, + dt_master = dt_master, + aux_hashes = aux_hashes, + verbose = verbose + ) + + if (is.null(candidates) || nrow(candidates) == 0L) { + # No requested measure's aux hash changed for any previously-cleaned + # survey. + pipfun::log_info( + "No auxiliary file changes detected for survey cleaning.", + name = "pipdata_log", + logmeta = list(info = "aux_no_changes_inf") + ) + } else { + # Stage 2: for the changed measures only, run valid_aux_load() and + # intersect the affected surveys with the candidate set. The inventory + # is pre-filtered to the candidate survey IDs before the detailed aux + # comparison, so only candidate rows are materialized. + changed_measures <- attr(candidates, "changed_measures") + + all_changes_aux <- valid_aux_load( + measure = changed_measures, + compare = "all", + verbose = verbose + ) + inv_candidates <- inv[survey_id %in% candidates$survey_id] + ls_inv_aux <- lapply(all_changes_aux, filter_aux_inv, inv = inv_candidates) + + if (all(vapply(ls_inv_aux, is.null, logical(1)))) { + # Measures changed but no requested survey is actually affected. + pipfun::log_info( + "Auxiliary files changed but no surveys affected.", + name = "pipdata_log", + logmeta = list( + info = "aux_changes_no_surveys_inf", + measures = changed_measures + ) + ) + } else { + affected <- ls_inv_aux |> + data.table::rbindlist() |> + collapse::funique() + + # Intersect affected surveys with the candidate set. + inv_aux <- affected[survey_id %in% candidates$survey_id] + + if (nrow(inv_aux) > 0L) { + pipfun::log_info( + "Auxiliary file changes detected.", + name = "pipdata_log", + logmeta = list( + info = "aux_changes_inf", + measures = changed_measures, + n_surveys_affected = nrow(inv_aux), + surveys_affected = inv_aux$survey_id + ) + ) + if (verbose) { + cli::cli_alert_warning( + "{nrow(inv_aux)} survey{?s} will be re-cleaned because auxiliary data changed." + ) + } + } else { + pipfun::log_info( + "Auxiliary files changed but no surveys affected.", + name = "pipdata_log", + logmeta = list( + info = "aux_changes_no_surveys_inf", + measures = changed_measures + ) + ) + } + } + } } if ( (is.null(inv_svy) || nrow(inv_svy) == 0) && (is.null(inv_aux) || nrow(inv_aux) == 0) ) { - return(NULL) + cli::cli_abort( + "No surveys to process: all surveys are up to date and no auxiliary changes affect any survey.", + class = "piperr" + ) } # Bind with inventory from aux changes @@ -121,6 +293,19 @@ valid_dlw_load <- function( # Choose only unique inv_to_clean <- unique(inv_to_clean) + # Log summary of surveys identified for cleaning + pipfun::log_info( + "Surveys identified for cleaning.", + name = "pipdata_log", + logmeta = list( + info = "surveys_to_clean_inf", + n_dlw_new = if (is.null(inv_svy)) 0L else nrow(inv_svy), + n_aux_changed = if (is.null(inv_aux)) 0L else nrow(inv_aux), + n_total_unique = nrow(inv_to_clean), + aux_measures_triggered = changed_measures + ) + ) + # Order alphabetically setorder(inv_to_clean, survey_id) @@ -164,12 +349,6 @@ filter_aux_inv <- function(inv, changes_aux) { changes <- unique(rbindlist(changes, fill = TRUE)) - # Temporary fix to test data from Rossana - - max_year <- max(inv[!is.na(inv$surveyid_year), ]$surveyid_year) - - changes <- changes[changes$surveyid_year <= max_year, ] - # Merge inventory with aux changes inv_aux <- joyn::inner_join( @@ -249,12 +428,23 @@ fix_year_var <- function(dt) { #' Remove surveys already cleaned from the processing inventory #' -#' Anti-joins the current DLW inventory against the PIP master inventory -#' to keep only surveys that have not yet been cleaned. If the master -#' inventory cannot be loaded, all surveys are returned. +#' Compares the current DLW inventory against the PIP master inventory by +#' joining on `survey_id` and comparing `content_hash` (DLW) against +#' `content_hash_dlw` (master). Surveys are kept when they are new to the +#' master (no `content_hash_dlw`) or when their DLW content hash differs +#' from the previously cleaned value. If the master inventory cannot be +#' loaded, all surveys are returned. #' #' @param inv A `data.table` of DLW surveys (latest versions). #' @param verbose Logical. Print progress messages. +#' @param dt_master A `data.table` of the PIP master inventory, already loaded +#' by the caller ([valid_dlw_load()]) and shared with the aux-hash +#' comparison. Default `NULL`, in which case the master is loaded here. +#' @param master_available Logical. Whether the caller already attempted to +#' load the master. When `TRUE`, `dt_master` is used as-is. When `FALSE`, +#' the master was attempted but unavailable, so all surveys are returned +#' without re-loading. Default `NULL` (unknown — load here if `dt_master` +#' is `NULL`). #' #' @return A `data.table` of surveys still needing processing, or #' `NULL` if all surveys have already been cleaned. @@ -263,51 +453,65 @@ fix_year_var <- function(dt) { #' @keywords internal inv_to_process <- function( inv, - verbose = TRUE + verbose = TRUE, + dt_master = NULL, + master_available = NULL ) { - # Select valid surveys and compare to previous cleaning - inv_svy <- tryCatch( - expr = { - # Load master inventory to compare with previous cleaning - dt_master <- pipload::load_pip_master_inventory(verbose = verbose) - - # Remove _dlw suffix from master inventory to be able to compare with current inventory - dlw_cols <- grep("_dlw$", names(dt_master), value = TRUE) - if (length(dlw_cols) > 0) { - new_names <- sub("_dlw$", "", dlw_cols) - data.table::setnames(dt_master, dlw_cols, new_names) - } - - # keep only surveys not cleaned in previous version - key_inventory <- c("country_code", "surveyid_year", "survey_acronym") # Temporary fix until we have create keys in the inventory - - # if (!all(key_inventory %in% names(inv))) { - # cli::cli_abort( - # "The inventory should contain the following variables: country_code, surveyid_year and survey_acronym" - # ) - # } - - inv_svy <- inv |> - joyn::anti_join( - dt_master, - by = key_inventory, - verbose = FALSE, - reportvar = FALSE - ) + # Load master inventory to compare with previous cleaning, unless the + # caller already loaded it (shared single-load handoff). When the caller + # explicitly reports the master is unavailable (master_available = FALSE), + # do not re-load — return all surveys instead. + if (isFALSE(master_available)) { + return(inv) + } - inv_svy - }, - error = function(e) { - if (verbose) { - cli::cli_alert_warning( - "Could not load PIP master inventory. Returning all valid surveys without comparing to previous cleaning." - ) + if (is.null(dt_master)) { + dt_master <- tryCatch( + pipload::load_pip_master_inventory(verbose = verbose), + error = function(e) { + if (verbose) { + cli::cli_alert_warning( + "Could not load PIP master inventory. Processing all surveys." + ) + } + return(NULL) } - return(inv) - } + ) + } + + if (is.null(dt_master)) return(inv) + + # Deduplicate by survey_id + content_hash_dlw. A survey may have multiple + # historical DLW content hashes in the master; the join below matches the + # current DLW content hash so only the corresponding master row is used. + dt_master_hash <- collapse::funique(dt_master[, .(survey_id, content_hash_dlw)]) + + # Rename the current DLW inventory's content_hash to content_hash_dlw so the + # join can match it against the master's stored content_hash_dlw. + inv_join <- data.table::copy(inv) + data.table::setnames(inv_join, "content_hash", "content_hash_dlw") + + # Left-join on survey_id + content_hash_dlw. reportvar = TRUE adds the .joyn + # column: "matched" when the current DLW content hash matches the master's + # content_hash_dlw (survey already cleaned), "x" when the survey is new or + # its DLW content changed. + inv_compare <- joyn::left_join( + inv_join, + dt_master_hash, + by = c("survey_id", "content_hash_dlw"), + relationship = "many-to-one", + verbose = FALSE, + reportvar = ".joyn" ) - if (inv_svy[, .N] == 0) { + # Keep: new surveys or surveys whose DLW content changed (.joyn == "x"). + inv_changed <- inv_compare[.joyn == "x"] + # Restore the original column name for the DLW content hash, then drop the + # .joyn report column. + data.table::setnames(inv_changed, "content_hash_dlw", "content_hash") + inv_changed[, .joyn := NULL] + + if (nrow(inv_changed) == 0) { if (verbose) { cli::cli_alert_warning( "All surveys in the inventory have been cleaned in previous versions. No surveys to process." @@ -316,5 +520,139 @@ inv_to_process <- function( return(NULL) } - return(inv_svy) + return(inv_changed) +} + +#' Identify surveys whose stored aux hash differs from the current aux hash +#' +#' Stage 1 of the two-stage aux-change gate. For each previously-cleaned +#' survey (present in the master inventory), compares its stored per-survey +#' aux hash (from the master's `aux__hash` columns) against the +#' current aux `content_hash` for each requested measure. A survey is a +#' candidate when any requested measure's stored hash differs from the +#' current hash, or when the stored hash is missing (survey cleaned before +#' this feature — treated as changed). +#' +#' @param inv A `data.table` of DLW surveys (latest versions). +#' @param dt_master A `data.table` of the PIP master inventory. +#' @param aux_hashes A named character vector of current aux `content_hash` +#' values, one per requested measure. +#' @param verbose Logical. Print progress messages. +#' +#' @return A `data.table` of candidate surveys (subset of `inv`), with an +#' attribute `changed_measures` holding the measures whose hash changed. +#' Returns `NULL` when no survey is a candidate. +#' +#' @details +#' The master inventory is reduced to one row per `survey_id` for the same +#' `content_hash_dlw`. All rows in that group must have identical aux hashes; +#' a conflict aborts loudly (this protects the invariant that split `pip_id`s +#' for one survey/content version use the same aux versions). +#' +#' The current DLW inventory is joined to the master on both `survey_id` and +#' the DLW content hash (`inv$content_hash` matched to `master$content_hash_dlw`), +#' so a survey with multiple historical DLW versions is compared against the +#' aux hashes of its current version only. +#' +#' @family pd_process_data pipeline +#' @keywords internal +aux_hash_candidates <- function( + inv, + dt_master, + aux_hashes, + verbose = TRUE +) { + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + # computations --------- + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + # Reduce master to one row per survey_id for the same content_hash_dlw, + # carrying the aux hash columns. Abort on conflicting aux hashes within a + # survey/content-hash group. + aux_cols <- paste0("aux_", names(aux_hashes), "_hash") + aux_cols <- intersect(aux_cols, names(dt_master)) + + if (length(aux_cols) == 0L) { + # No stored aux hashes at all — every survey was cleaned before this + # feature. These are ignored for aux-change detection (no hash to compare), + # so there are no candidates. + return(NULL) + } + + # Build a survey-level master keyed by survey_id + content_hash_dlw, and + # detect conflicting aux hashes within a group in a single pass. A group + # with more than one distinct aux-hash combination for the same key is a + # conflict (split pip_id rows must share the same aux versions). + key_cols <- c("survey_id", "content_hash_dlw") + master_svy <- collapse::funique(dt_master[, c(key_cols, aux_cols), with = FALSE]) + + n_groups <- nrow(collapse::funique(master_svy[, key_cols, with = FALSE])) + if (n_groups != nrow(master_svy)) { + cli::cli_abort( + "Conflicting aux hashes found for the same survey_id and content_hash_dlw.", + class = c("aux_hash_candidates_conflict", "piperr") + ) + } + + # Rename the current DLW inventory's content_hash to content_hash_dlw so the + # join can match it against the master's stored content_hash_dlw. This + # ensures a survey with multiple historical DLW versions is compared against + # the aux hashes of its current version only. + inv_join <- data.table::copy(inv) + data.table::setnames(inv_join, "content_hash", "content_hash_dlw") + + inv_compare <- joyn::left_join( + inv_join, + master_svy, + by = c("survey_id", "content_hash_dlw"), + relationship = "many-to-one", + verbose = FALSE, + reportvar = ".joyn" + ) + + # Determine which measures changed for each survey. Surveys with a missing + # (NA) stored aux hash are ignored for the change comparison — they were + # cleaned before this feature and simply have no hash recorded yet. Only + # surveys with a populated stored hash are compared against the current + # hash. + changed_measures <- character(0) + candidate_idx <- rep(FALSE, nrow(inv_compare)) + + for (m in names(aux_hashes)) { + col <- paste0("aux_", m, "_hash") + if (!col %in% names(inv_compare)) { + # Measure column not present at all — nothing to compare for this + # measure; skip it (do not treat as changed). + next + } + stored <- inv_compare[[col]] + current <- aux_hashes[[m]] + is_na <- is.na(stored) + # Only compare surveys that have a populated stored hash. + is_changed <- !is_na & stored != current + candidate_idx <- candidate_idx | is_changed + if (any(is_changed, na.rm = TRUE)) { + changed_measures <- unique(c(changed_measures, m)) + } + } + + candidates <- inv_compare[candidate_idx] + # Drop the joined aux hash columns and the .joyn report column. + drop_cols <- intersect(c(aux_cols, ".joyn"), names(candidates)) + if (length(drop_cols) > 0L) { + candidates[, (drop_cols) := NULL] + } + # Restore the original column name for the DLW content hash. + data.table::setnames(candidates, "content_hash_dlw", "content_hash") + + if (nrow(candidates) == 0L) { + return(NULL) + } + + attr(candidates, "changed_measures") <- changed_measures + + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + # Return --------- + #~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + return(candidates) } diff --git a/compound-gpid.context.md b/compound-gpid.context.md index e94badc..4dfda49 100644 --- a/compound-gpid.context.md +++ b/compound-gpid.context.md @@ -29,9 +29,12 @@ For detailed technical walkthrough, see `docs/pipeline_overview.qmd`. - Error handling uses custom `piperr` conditions for graceful recovery without silencing failures - `dplyr`, `tidyr`, and `tibble` are **not** in `DESCRIPTION Imports` — use `data.table` (`:=`, `rbindlist`, `[, .N, by]`) and `collapse` (`fcase`, `ftransform`, `fmutate`) instead. Do not add new dplyr/tidyr/tibble calls anywhere. `dlw_scan_and_validate.R` still has ~19 legacy dplyr calls (Phase 2 migration, tracked in roadmap as `dplyr-to-collapse-phase2`). - Always qualify `fcase()` and `fifelse()` with `data.table::` (i.e. `data.table::fcase(...)`) even when called inside `collapse::ftransform()` or `collapse::fmutate()`. This makes the dependency surface explicit and avoids ambiguity with similarly-named collapse functions. -- The pipeline emits four canonical logmeta entry types, parsed by `log_report()` to build report sections. Their `info`/`error` field values are: +- The pipeline emits nine canonical logmeta entry types, parsed by `log_report()` to build report sections. Their `info`/`error` field values are: - `"process_summary_inf"` — emitted by `pd_process_data()` - - `"aux_changes_inf"` — emitted by `valid_dlw_load()` when auxiliary files change + - `"aux_changes_inf"` — emitted by `valid_dlw_load()` when auxiliary files change and at least one survey is affected + - `"aux_no_changes_inf"` — emitted by `valid_dlw_load()` when no auxiliary file changes were detected at all + - `"aux_changes_no_surveys_inf"` — emitted by `valid_dlw_load()` when auxiliary files changed but no surveys in the inventory were affected + - `"surveys_to_clean_inf"` — emitted by `valid_dlw_load()` once after combining/deduplicating DLW-new and aux-changed surveys; includes counts of new, aux-changed, and total unique surveys - `"null_svys_inf"` — emitted by `update_pip_inventory()` when surveys fail (NULL) - `"inv_update_inf"` — emitted by `update_pip_inventory()` for inventory verification (info if all confirmed, error if any missing) - `"release_write_err"` — emitted by `update_pip_inventory()` when the release inventory write fails (error-level; includes `condition_msg`) @@ -50,6 +53,7 @@ For detailed technical walkthrough, see `docs/pipeline_overview.qmd`. - **data.table `DT[i, ]` scoping: function arguments do NOT shadow columns**: Inside `DT[i, ]`, bare names resolve to **columns** before looking up in the parent frame. When a function argument shares a name with a column (e.g. `pip_id`), the expression `inv[inv$col == pip_id, ]` becomes a tautology — `pip_id` resolves to the column, so all rows match. Always break the name collision by renaming the local binding (`target_id <- pip_id`) or using the `..` prefix (`inv[col == ..pip_id, ]`). This is a silent bug — no error is thrown. See `.cg-docs/solutions/bugs/2026-05-19-datatable-scoping-column-vs-argument.md`. - **`pipdata.verbose` option controls downstream I/O messages — not structured logging**: `getOption("pipdata.verbose", default = TRUE)` is the single switch for verbosity of `pipload`/`stamp`/`pipaux` I/O calls (e.g., `pip_read()`, `pip_write()`, `load_pip_master_inventory()`, `load_aux_data()`). All exported functions read this option via `verbose = getOption("pipdata.verbose", default = TRUE)` in their formals and propagate the resolved value to **all** downstream calls without exception. Internal helpers accept `verbose` as an explicit parameter — they never call `getOption()` themselves. The only calls that stay `verbose = FALSE` unconditionally are `joyn::` join diagnostics (not I/O messages). Set `options(pipdata.verbose = FALSE)` at the top of batch/orchestration scripts. Structured pipeline logging (`pipfun::log_add()`, `pipfun::log_info()` into `pipdata_log`) is **never** affected by this option. See `?pipdata-options` and `.cg-docs/plans/2026-05-08-verbose-option-propagation.md`. - **All `joyn::` join calls feeding production data must include `reportvar = FALSE`**: `joyn::` functions append a `.joyn` factor column by default (`reportvar = TRUE`). Without `reportvar = FALSE`, this column leaks into downstream `rbind`/`unique()` operations and silently defeats deduplication — `unique()` treats `.joyn = "x"` and `.joyn = NA` (from `fill = TRUE`) as distinct rows for the same key. Separately, any column shared by both tables but absent from `by` is suffixed `.x`/`.y` — never persist raw `joyn::` output without auditing `names()` for these. Rule: every `joyn::anti_join`, `inner_join`, `left_join` call in a production data path must carry `reportvar = FALSE`. The `verbose = FALSE` argument controls message output only — it does not suppress the `.joyn` column. See `.cg-docs/solutions/data-quality/2026-06-05-joyn-diagnostic-column-discipline.md`. +- **Auxiliary re-cleaning requires two gates**: compare the current aux artifact `content_hash` from `stamp::st_catalog_query(alias = "aux")` with the per-survey hash stored in the master inventory, then run `valid_aux_load()`/`compare_aux_*` only for globally changed measures and intersect the changed country/year keys with requested surveys. Match master rows by both `survey_id` and the current DLW `content_hash_dlw`; when a survey is reprocessed, replace all its current master rows by `survey_id` so stale welfare-type `pip_id` rows are removed. Stamp retains prior master snapshots for recovery. See `.cg-docs/solutions/data-quality/2026-08-07-aux-content-hash-gated-recleaning.md`. ## Work in Progress diff --git a/man/add_attr.Rd b/man/add_attr.Rd index 2f10d44..285ae2c 100644 --- a/man/add_attr.Rd +++ b/man/add_attr.Rd @@ -27,12 +27,14 @@ result as a named element of the attribute list. } \seealso{ Other pd_process_data pipeline: +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/man/aux_hash_candidates.Rd b/man/aux_hash_candidates.Rd new file mode 100644 index 0000000..82162c4 --- /dev/null +++ b/man/aux_hash_candidates.Rd @@ -0,0 +1,64 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/valid_dlw_load.R +\name{aux_hash_candidates} +\alias{aux_hash_candidates} +\title{Identify surveys whose stored aux hash differs from the current aux hash} +\usage{ +aux_hash_candidates(inv, dt_master, aux_hashes, verbose = TRUE) +} +\arguments{ +\item{inv}{A \code{data.table} of DLW surveys (latest versions).} + +\item{dt_master}{A \code{data.table} of the PIP master inventory.} + +\item{aux_hashes}{A named character vector of current aux \code{content_hash} +values, one per requested measure.} + +\item{verbose}{Logical. Print progress messages.} +} +\value{ +A \code{data.table} of candidate surveys (subset of \code{inv}), with an +attribute \code{changed_measures} holding the measures whose hash changed. +Returns \code{NULL} when no survey is a candidate. +} +\description{ +Stage 1 of the two-stage aux-change gate. For each previously-cleaned +survey (present in the master inventory), compares its stored per-survey +aux hash (from the master's \verb{aux__hash} columns) against the +current aux \code{content_hash} for each requested measure. A survey is a +candidate when any requested measure's stored hash differs from the +current hash, or when the stored hash is missing (survey cleaned before +this feature — treated as changed). +} +\details{ +The master inventory is reduced to one row per \code{survey_id} for the same +\code{content_hash_dlw}. All rows in that group must have identical aux hashes; +a conflict aborts loudly (this protects the invariant that split \code{pip_id}s +for one survey/content version use the same aux versions). + +The current DLW inventory is joined to the master on both \code{survey_id} and +the DLW content hash (\code{inv$content_hash} matched to \code{master$content_hash_dlw}), +so a survey with multiple historical DLW versions is compared against the +aux hashes of its current version only. +} +\seealso{ +Other pd_process_data pipeline: +\code{\link[=add_attr]{add_attr()}}, +\code{\link[=build_pip_inventory]{build_pip_inventory()}}, +\code{\link[=create_attr]{create_attr()}}, +\code{\link[=data_to_dt]{data_to_dt()}}, +\code{\link[=filter_aux_data]{filter_aux_data()}}, +\code{\link[=filter_aux_inv]{filter_aux_inv()}}, +\code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, +\code{\link[=inv_dlw_load]{inv_dlw_load()}}, +\code{\link[=inv_to_process]{inv_to_process()}}, +\code{\link[=log_report]{log_report()}}, +\code{\link[=pd_aux_attr]{pd_aux_attr()}}, +\code{\link[=pd_deflation]{pd_deflation()}}, +\code{\link[=save_pip_data]{save_pip_data()}}, +\code{\link[=survey_id_to_attr]{survey_id_to_attr()}}, +\code{\link[=valid_dlw_load]{valid_dlw_load()}} +} +\concept{pd_process_data pipeline} +\keyword{internal} diff --git a/man/build_pip_inventory.Rd b/man/build_pip_inventory.Rd index d55f1f4..3876624 100644 --- a/man/build_pip_inventory.Rd +++ b/man/build_pip_inventory.Rd @@ -7,7 +7,8 @@ build_pip_inventory( inv_to_clean, pip_id_map, - verbose = getOption("pipdata.verbose", default = TRUE) + verbose = getOption("pipdata.verbose", default = TRUE), + aux_hashes = NULL ) } \arguments{ @@ -21,6 +22,12 @@ from successful \code{process_data()} calls in \code{\link[=pd_process_data]{pd_ \item{verbose}{Logical. Controls verbosity of downstream \code{\link[pipload:load_pip_master_inventory]{pipload::load_pip_master_inventory()}} and \code{\link[pipload:load_aux_data]{pipload::load_aux_data()}} calls. Default: \code{getOption("pipdata.verbose", default = TRUE)}.} + +\item{aux_hashes}{A named character vector of current aux \code{content_hash} +values, one per requested auxiliary measure (e.g. \code{cpi}, \code{ppp}, \code{pfw}). +Resolved once per run by \code{\link[=get_aux_hashes]{get_aux_hashes()}} and recorded on the +master-inventory rows produced for successfully processed surveys. +Default \code{NULL} (no aux hashes recorded).} } \value{ A \code{data.table}: the updated PIP master inventory. Does \strong{not} @@ -62,6 +69,10 @@ surveys are retained from the prior master. One row per \code{pip_id}, always. \item \code{pipeline_version_dlw}, \code{latest_version_id_dlw}, \code{content_hash_dlw}, \code{Checksum_dlw}, \code{path_dlw} — renamed from DLW inventory columns. \item \code{welfare_type} — derived from the 4th \verb{_}-delimited segment of \code{pip_id}. +\item \verb{aux__hash} (e.g. \code{aux_cpi_hash}, \code{aux_ppp_hash}, \code{aux_pfw_hash}) +— current aux \code{content_hash} for each requested measure, from the +run-level \code{aux_hashes} map passed by \code{\link[=pd_process_data]{pd_process_data()}}. Only populated +for surveys successfully processed in the current run. \item \code{first_release_version_id}, \code{latest_release_version_id} — stamp version IDs of the release inventory (first appearance and most recent). } @@ -69,11 +80,13 @@ IDs of the release inventory (first appearance and most recent). \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/man/create_attr.Rd b/man/create_attr.Rd index edd92a9..2848635 100644 --- a/man/create_attr.Rd +++ b/man/create_attr.Rd @@ -23,11 +23,13 @@ with a combination of version/year and reporting level. \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/man/data_to_dt.Rd b/man/data_to_dt.Rd index 6a741dd..3755b86 100644 --- a/man/data_to_dt.Rd +++ b/man/data_to_dt.Rd @@ -23,11 +23,13 @@ individual attributes, and assigns the PIP S3 class via \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/man/filter_aux_data.Rd b/man/filter_aux_data.Rd index b9d0b46..98b3df9 100644 --- a/man/filter_aux_data.Rd +++ b/man/filter_aux_data.Rd @@ -28,11 +28,13 @@ country and year. \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/man/filter_aux_inv.Rd b/man/filter_aux_inv.Rd index 22e646e..fddb2ae 100644 --- a/man/filter_aux_inv.Rd +++ b/man/filter_aux_inv.Rd @@ -24,11 +24,13 @@ identify surveys affected by those changes. \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/man/fix_year_var.Rd b/man/fix_year_var.Rd index 1ca2611..3b45a5d 100644 --- a/man/fix_year_var.Rd +++ b/man/fix_year_var.Rd @@ -21,11 +21,13 @@ Finds the column whose name contains \code{"year"}, renames it to \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/man/get_aux_hashes.Rd b/man/get_aux_hashes.Rd new file mode 100644 index 0000000..16e6acb --- /dev/null +++ b/man/get_aux_hashes.Rd @@ -0,0 +1,67 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{get_aux_hashes} +\alias{get_aux_hashes} +\title{Resolve current content hashes for auxiliary measures from the aux catalog} +\usage{ +get_aux_hashes( + aux_measures, + verbose = getOption("pipdata.verbose", default = TRUE) +) +} +\arguments{ +\item{aux_measures}{Character vector of auxiliary measures to resolve.} + +\item{verbose}{Logical. Retained for consistency with pipeline helpers; +catalog lookup itself emits no progress messages. Default: +\code{getOption("pipdata.verbose", default = TRUE)}.} +} +\value{ +A named character vector of \code{content_hash} values, one per +requested measure. Names are the measure names. +} +\description{ +Queries the \code{"aux"} stamp catalog once and returns the current +\code{content_hash} for each requested auxiliary measure. Each measure is +matched to exactly one catalog artifact whose path basename is +\verb{.qs2} (e.g. \code{cpi.qs2}, \code{ppp.qs2}, \code{pfw.qs2}). +} +\details{ +This is the single source of the current aux hashes used to gate +aux-change detection in \code{\link[=valid_dlw_load]{valid_dlw_load()}}. It must be called once per +pipeline run, before aux data is loaded, and the result passed through +the run so that the hashes recorded in the master inventory match the +aux data actually used. + +The function aborts loudly when the \code{"aux"} alias is unavailable, a +requested artifact is missing, or multiple catalog rows match a measure. +It never falls back to \code{stamp::st_latest()} or to hashing loaded aux +tables. + +\strong{Precondition}: the \code{"aux"} catalog and \code{\link[pipload:load_aux_data]{pipload::load_aux_data()}} must +resolve through the same configured working release and storage root. The +hashes returned here are only meaningful if the aux data subsequently +loaded for processing comes from the same artifacts. Callers must ensure +the working release is set up consistently before calling this function. +} +\seealso{ +Other pd_process_data pipeline: +\code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, +\code{\link[=build_pip_inventory]{build_pip_inventory()}}, +\code{\link[=create_attr]{create_attr()}}, +\code{\link[=data_to_dt]{data_to_dt()}}, +\code{\link[=filter_aux_data]{filter_aux_data()}}, +\code{\link[=filter_aux_inv]{filter_aux_inv()}}, +\code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=inv_dlw_load]{inv_dlw_load()}}, +\code{\link[=inv_to_process]{inv_to_process()}}, +\code{\link[=log_report]{log_report()}}, +\code{\link[=pd_aux_attr]{pd_aux_attr()}}, +\code{\link[=pd_deflation]{pd_deflation()}}, +\code{\link[=save_pip_data]{save_pip_data()}}, +\code{\link[=survey_id_to_attr]{survey_id_to_attr()}}, +\code{\link[=valid_dlw_load]{valid_dlw_load()}} +} +\concept{pd_process_data pipeline} +\keyword{internal} diff --git a/man/inv_dlw_load.Rd b/man/inv_dlw_load.Rd index ed4db93..efb32a2 100644 --- a/man/inv_dlw_load.Rd +++ b/man/inv_dlw_load.Rd @@ -21,12 +21,14 @@ survey-ID components as attributes, and adds the PIP class. \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, \code{\link[=pd_aux_attr]{pd_aux_attr()}}, diff --git a/man/inv_to_process.Rd b/man/inv_to_process.Rd index 2e913a9..2cfe47a 100644 --- a/man/inv_to_process.Rd +++ b/man/inv_to_process.Rd @@ -4,31 +4,46 @@ \alias{inv_to_process} \title{Remove surveys already cleaned from the processing inventory} \usage{ -inv_to_process(inv, verbose = TRUE) +inv_to_process(inv, verbose = TRUE, dt_master = NULL, master_available = NULL) } \arguments{ \item{inv}{A \code{data.table} of DLW surveys (latest versions).} \item{verbose}{Logical. Print progress messages.} + +\item{dt_master}{A \code{data.table} of the PIP master inventory, already loaded +by the caller (\code{\link[=valid_dlw_load]{valid_dlw_load()}}) and shared with the aux-hash +comparison. Default \code{NULL}, in which case the master is loaded here.} + +\item{master_available}{Logical. Whether the caller already attempted to +load the master. When \code{TRUE}, \code{dt_master} is used as-is. When \code{FALSE}, +the master was attempted but unavailable, so all surveys are returned +without re-loading. Default \code{NULL} (unknown — load here if \code{dt_master} +is \code{NULL}).} } \value{ A \code{data.table} of surveys still needing processing, or \code{NULL} if all surveys have already been cleaned. } \description{ -Anti-joins the current DLW inventory against the PIP master inventory -to keep only surveys that have not yet been cleaned. If the master -inventory cannot be loaded, all surveys are returned. +Compares the current DLW inventory against the PIP master inventory by +joining on \code{survey_id} and comparing \code{content_hash} (DLW) against +\code{content_hash_dlw} (master). Surveys are kept when they are new to the +master (no \code{content_hash_dlw}) or when their DLW content hash differs +from the previously cleaned value. If the master inventory cannot be +loaded, all surveys are returned. } \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=log_report]{log_report()}}, \code{\link[=pd_aux_attr]{pd_aux_attr()}}, diff --git a/man/log_report.Rd b/man/log_report.Rd index 258cdac..1111744 100644 --- a/man/log_report.Rd +++ b/man/log_report.Rd @@ -65,12 +65,14 @@ log_report(path = "log_report.md", overwrite = TRUE) \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=pd_aux_attr]{pd_aux_attr()}}, diff --git a/man/pd_aux_attr.Rd b/man/pd_aux_attr.Rd index 9f886c9..cd37892 100644 --- a/man/pd_aux_attr.Rd +++ b/man/pd_aux_attr.Rd @@ -29,12 +29,14 @@ survey metadata. \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/man/pd_deflation.Rd b/man/pd_deflation.Rd index a1ea7c1..a9bd71b 100644 --- a/man/pd_deflation.Rd +++ b/man/pd_deflation.Rd @@ -100,12 +100,14 @@ pd_deflation(pip_id = "CHN_2015_CHIP_INC_D1") \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/man/pd_process_data.Rd b/man/pd_process_data.Rd index 5610f7e..2c5b993 100644 --- a/man/pd_process_data.Rd +++ b/man/pd_process_data.Rd @@ -43,6 +43,13 @@ to the \code{"pipdata_log"}, summarizing totals and failed surveys. Additional e auxiliary file changes and inventory verification are emitted by \code{\link[=valid_dlw_load]{valid_dlw_load()}} and \code{\link[=build_pip_inventory]{build_pip_inventory()}} respectively. +\strong{Aux hashes}: the current \code{content_hash} for every requested auxiliary +measure is resolved once from the \code{"aux"} stamp catalog via +\code{\link[=get_aux_hashes]{get_aux_hashes()}} before aux data is loaded. The run-level hash map is +passed to \code{\link[=build_pip_inventory]{build_pip_inventory()}} and recorded in the master inventory so +that \code{\link[=valid_dlw_load]{valid_dlw_load()}} can gate aux-change detection against the aux data +actually used in this run. + \strong{Recode spec}: the recode specification is synced to stamp once via \code{\link[=sync_recode_spec]{sync_recode_spec()}} before the per-survey loop and the resolved spec is threaded into each \code{\link[=process_data]{process_data()}} call, so \code{\link[=apply_recode_spec]{apply_recode_spec()}} performs diff --git a/man/save_pip_data.Rd b/man/save_pip_data.Rd index 9910f2d..90b9b93 100644 --- a/man/save_pip_data.Rd +++ b/man/save_pip_data.Rd @@ -45,12 +45,14 @@ of \verb{cannot allocate buffer} errors from \code{qs2}. \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/man/survey_id_to_attr.Rd b/man/survey_id_to_attr.Rd index 94187f4..5e86cb3 100644 --- a/man/survey_id_to_attr.Rd +++ b/man/survey_id_to_attr.Rd @@ -23,12 +23,14 @@ columns to only welfare, weight, and demographic variables. \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/man/valid_dlw_load.Rd b/man/valid_dlw_load.Rd index af6db3a..8c9240a 100644 --- a/man/valid_dlw_load.Rd +++ b/man/valid_dlw_load.Rd @@ -9,7 +9,8 @@ valid_dlw_load( aux_measures = c("pfw", "cpi", "ppp", "pop", "gdp", "pce"), modules = c("ALL", "GROUP", "HIST", "GPWG", "BIN"), force = FALSE, - verbose = getOption("pipdata.verbose", default = TRUE) + verbose = getOption("pipdata.verbose", default = TRUE), + aux_hashes = NULL ) } \arguments{ @@ -26,9 +27,17 @@ master inventory and process all surveys.} \item{verbose}{Logical. Print progress messages. Default: \code{getOption("pipdata.verbose", default = TRUE)}.} + +\item{aux_hashes}{A named character vector of current aux \code{content_hash} +values, one per requested auxiliary measure. Resolved once per run by +\code{\link[=get_aux_hashes]{get_aux_hashes()}} and used to gate aux-change detection. When \code{NULL} +(the default) and \code{force = FALSE}, the hashes are resolved internally so +that direct callers retain the previous behavior of always running +aux-change detection.} } \value{ -A \code{data.table} of surveys to process, or \code{NULL} if none. +A \code{data.table} of surveys to process. If no surveys require +processing, the function aborts with class \code{piperr}. } \description{ Compares the current DLW inventory against previously cleaned data @@ -47,19 +56,58 @@ master inventory via \code{inv_to_process()}. \item Combines DLW-new and aux-changed surveys into a single inventory. } -\strong{Logging}: This function writes an \code{aux_changes_inf} entry to the \code{"pipdata_log"} -when changes are detected in any of the requested auxiliary measures. The logmeta -entry includes the measures that changed and the number of affected surveys. +\strong{Aux-change gating (two-stage)}: aux-change detection is gated on the +current aux \code{content_hash} values passed via \code{aux_hashes}. +\itemize{ +\item Stage 1 (cheap): for each filtered/latest survey, compare its stored +per-survey aux hash (from the master inventory's \verb{aux__hash} +columns) against the current hash for that measure. A mismatch or a +missing historical hash makes the survey a candidate. New surveys and +DLW-content-changed surveys are also retained through +\code{\link[=inv_to_process]{inv_to_process()}} and are deduplicated with the aux candidates. +\item Stage 2 (detailed): for the changed measures only, \code{\link[=valid_aux_load]{valid_aux_load()}} / +\verb{compare_aux_*} identifies which requested surveys actually have changed +rows inside the aux file. The affected surveys are intersected with the +candidate set, so a globally changed aux table that only affects +non-requested countries does not re-clean requested surveys. +} + +The master inventory is loaded at most once within this function and shared +between the DLW comparison and the aux-hash comparison. This guarantee is +scoped to \code{valid_dlw_load()}; downstream steps such as +\code{\link[=build_pip_inventory]{build_pip_inventory()}} load the master again for their own assembly and +verification. When \code{force = TRUE}, no master or aux comparison runs and all +filtered/latest surveys are processed. + +\strong{Logging}: This function writes the following entries to the \code{"pipdata_log"}: +\itemize{ +\item \code{aux_changes_inf} — changes were detected in any of the requested auxiliary +measures and at least one survey is affected. Includes the measures that +changed and the number/list of affected surveys. +\item \code{aux_no_changes_inf} — no auxiliary file changes were detected at all. +\item \code{aux_changes_no_surveys_inf} — auxiliary files changed but no surveys in +the inventory were affected by those changes. +\item \code{surveys_to_clean_inf} — emitted once after the DLW-new and aux-changed +inventories are combined and deduplicated; includes counts of new, +aux-changed, and total unique surveys, plus the aux measures that +triggered re-cleaning. +} + +When neither new DLW surveys nor auxiliary changes leave anything to +process, the function aborts with \code{cli::cli_abort(class = "piperr")} rather +than returning \code{NULL} silently. } \seealso{ Other pd_process_data pipeline: \code{\link[=add_attr]{add_attr()}}, +\code{\link[=aux_hash_candidates]{aux_hash_candidates()}}, \code{\link[=build_pip_inventory]{build_pip_inventory()}}, \code{\link[=create_attr]{create_attr()}}, \code{\link[=data_to_dt]{data_to_dt()}}, \code{\link[=filter_aux_data]{filter_aux_data()}}, \code{\link[=filter_aux_inv]{filter_aux_inv()}}, \code{\link[=fix_year_var]{fix_year_var()}}, +\code{\link[=get_aux_hashes]{get_aux_hashes()}}, \code{\link[=inv_dlw_load]{inv_dlw_load()}}, \code{\link[=inv_to_process]{inv_to_process()}}, \code{\link[=log_report]{log_report()}}, diff --git a/roadmap.json b/roadmap.json index 69eda9c..0b10bf6 100644 --- a/roadmap.json +++ b/roadmap.json @@ -468,7 +468,8 @@ "id": "content-hash-reclean-trigger", "title": "Use content_hash_dlw to detect DLW source changes in inv_to_process", "description": "Enhance inv_to_process filtering to compare the previous master inventory's content_hash_dlw against the current DLW inventory's content_hash for each survey. If they differ, the DLW source data changed since last clean and the survey should be re-processed even if it was previously cleaned successfully. This avoids unnecessary re-cleaning of unchanged surveys while catching updated ones.", - "status": "idea", + "status": "done", + "plan": ".cg-docs/plans/2026-06-17-valid-dlw-load-refactor.md", "tags": [ "inventory", "dlw", @@ -499,6 +500,20 @@ "subnational", "code-quality" ] + }, + { + "id": "aux-version-gate-valid-dlw-load", + "title": "Gate aux-change detection on per-survey aux content-hash comparison", + "description": "valid_dlw_load() always runs valid_aux_load()/compare_aux_*, which compare current aux data against the previous aux release/vintage — not against what the pipeline used last run — so every run re-cleans affected surveys unnecessarily. Fix: capture per-survey aux content hashes during processing (pd_aux_attr), surface them as new master-inventory columns (build_pip_inventory), and gate aux-change detection in valid_dlw_load() via a two-stage filter: (1) per-survey stored-vs-current aux hash comparison builds a candidate set; (2) valid_aux_load()/compare_aux_* runs only for changed measures, intersected with the candidate set.", + "status": "done", + "plan": ".cg-docs/plans/2026-08-06-aux-version-gate-valid-dlw-load-revised.md", + "tags": [ + "inventory", + "aux", + "incremental-processing", + "performance", + "stamp" + ] } ] } diff --git a/tests/testthat/test-build_pip_inventory.R b/tests/testthat/test-build_pip_inventory.R index 5a14f60..3d818cf 100644 --- a/tests/testthat/test-build_pip_inventory.R +++ b/tests/testthat/test-build_pip_inventory.R @@ -758,4 +758,368 @@ test_that("build_pip_inventory rowbinds correctly when old master has fs_bytes s expect_equal(nrow(result), 2L) expect_true(old_pip_id %in% result$pip_id) expect_true(new_pip_id %in% result$pip_id) +}) + +# --------------------------------------------------------------------------- +# Aux hashes: current-run rows receive the run-level aux hash columns +# --------------------------------------------------------------------------- + +test_that("build_pip_inventory records run-level aux hashes on current-run rows", { + pip_id <- "BOL_2022_EH_INC_ALL" + survey <- "BOL_2022_EH" + + inv_to_clean <- make_inv_to_clean( + survey, + country_codes = "BOL", + surveyid_years = 2022L, + survey_acronyms = "EH" + ) + pip_id_map <- make_pip_id_map(survey, list(pip_id)) + + aux_hashes <- c( + pfw = "hash_pfw", + cpi = "hash_cpi", + ppp = "hash_ppp", + pop = "hash_pop", + gdp = "hash_gdp", + pce = "hash_pce" + ) + + local_mocked_bindings( + st_catalog_query = function(alias = NULL) make_catalog(pip_id), + st_latest = function(...) "vid_bol", + .package = "stamp" + ) + local_mocked_bindings( + load_pip_master_inventory = function(...) NULL, + load_aux_data = function(measure, ...) make_pfw("BOL", 2022L, "EH"), + pip_write = function(x, id, alias, pk = NULL, ...) list(version_id = "v1"), + .package = "pipload" + ) + local_mocked_bindings( + log_add = null_log, + log_info = null_log, + log_error = null_log, + .package = "pipfun" + ) + + result <- build_pip_inventory(inv_to_clean, pip_id_map, aux_hashes = aux_hashes) + + expect_true("aux_cpi_hash" %in% names(result)) + expect_true("aux_ppp_hash" %in% names(result)) + expect_true("aux_pfw_hash" %in% names(result)) + expect_true("aux_pop_hash" %in% names(result)) + expect_true("aux_gdp_hash" %in% names(result)) + expect_true("aux_pce_hash" %in% names(result)) + expect_equal(result[result$pip_id == pip_id, aux_cpi_hash], "hash_cpi") + expect_equal(result[result$pip_id == pip_id, aux_ppp_hash], "hash_ppp") + expect_equal(result[result$pip_id == pip_id, aux_pfw_hash], "hash_pfw") +}) + +# --------------------------------------------------------------------------- +# Aux hashes: old retained rows keep NA when not reprocessed this run +# --------------------------------------------------------------------------- + +test_that("build_pip_inventory leaves old retained rows' aux hashes as NA", { + new_pip_id <- "BRA_2019_PNAD_INC_ALL" + new_survey <- "BRA_2019_PNAD" + old_pip_id <- "CHN_2018_HIES_INC_ALL" + old_survey <- "CHN_2018_HIES" + + inv_to_clean <- make_inv_to_clean( + new_survey, + country_codes = "BRA", + surveyid_years = 2019L, + survey_acronyms = "PNAD" + ) + pip_id_map <- make_pip_id_map(new_survey, list(new_pip_id)) + + old_master <- data.table::data.table( + survey_id = old_survey, + pip_id = old_pip_id, + version_id_data = "old_vid_data", + version_id_metadata = "old_vid_meta", + welfare_type = "INC", + country_code = "CHN", + surveyid_year = 2018L, + survey_acronym = "HIES", + first_release_version_id = NA_character_, + latest_release_version_id = NA_character_ + ) + + aux_hashes <- c(cpi = "hash_cpi", ppp = "hash_ppp") + + local_mocked_bindings( + st_catalog_query = function(alias = NULL) make_catalog(new_pip_id), + st_latest = function(...) "vid_bra", + .package = "stamp" + ) + local_mocked_bindings( + load_pip_master_inventory = function(...) old_master, + load_aux_data = function(measure, ...) make_pfw("BRA", 2019L, "PNAD"), + pip_write = function(x, id, alias, pk = NULL, ...) list(version_id = "v1"), + .package = "pipload" + ) + local_mocked_bindings( + log_add = null_log, + log_info = null_log, + log_error = null_log, + .package = "pipfun" + ) + + result <- build_pip_inventory(inv_to_clean, pip_id_map, aux_hashes = aux_hashes) + + # New row has the hash; old retained row has NA. + expect_equal(result[result$pip_id == new_pip_id, aux_cpi_hash], "hash_cpi") + expect_true(is.na(result[result$pip_id == old_pip_id, aux_cpi_hash])) + expect_true(is.na(result[result$pip_id == old_pip_id, aux_ppp_hash])) +}) + +# --------------------------------------------------------------------------- +# Aux hashes: partial requested measures leave non-requested columns absent +# --------------------------------------------------------------------------- + +test_that("build_pip_inventory only adds columns for requested aux measures", { + pip_id <- "BOL_2022_EH_INC_ALL" + survey <- "BOL_2022_EH" + + inv_to_clean <- make_inv_to_clean( + survey, + country_codes = "BOL", + surveyid_years = 2022L, + survey_acronyms = "EH" + ) + pip_id_map <- make_pip_id_map(survey, list(pip_id)) + + # Only cpi and ppp requested. + aux_hashes <- c(cpi = "hash_cpi", ppp = "hash_ppp") + + local_mocked_bindings( + st_catalog_query = function(alias = NULL) make_catalog(pip_id), + st_latest = function(...) "vid_bol", + .package = "stamp" + ) + local_mocked_bindings( + load_pip_master_inventory = function(...) NULL, + load_aux_data = function(measure, ...) make_pfw("BOL", 2022L, "EH"), + pip_write = function(x, id, alias, pk = NULL, ...) list(version_id = "v1"), + .package = "pipload" + ) + local_mocked_bindings( + log_add = null_log, + log_info = null_log, + log_error = null_log, + .package = "pipfun" + ) + + result <- build_pip_inventory(inv_to_clean, pip_id_map, aux_hashes = aux_hashes) + + expect_true("aux_cpi_hash" %in% names(result)) + expect_true("aux_ppp_hash" %in% names(result)) + expect_false("aux_pfw_hash" %in% names(result)) + expect_false("aux_pop_hash" %in% names(result)) +}) + +# --------------------------------------------------------------------------- +# P1.2 regression: reprocessing a survey drops stale pip_id rows +# --------------------------------------------------------------------------- + +test_that("build_pip_inventory drops stale pip_id rows when a survey is reprocessed", { + # Survey BOL_2022_EH previously had two pip_id rows (INC_ALL and INC_GPWG). + # This run reprocesses it with only INC_ALL. The stale INC_GPWG row must be + # dropped so the survey's pip_id set matches the current reprocess exactly. + new_pip_id <- "BOL_2022_EH_INC_ALL" + stale_pip_id <- "BOL_2022_EH_INC_GPWG" + survey <- "BOL_2022_EH" + + inv_to_clean <- make_inv_to_clean( + survey, + country_codes = "BOL", + surveyid_years = 2022L, + survey_acronyms = "EH" + ) + # Only INC_ALL is reprocessed this run. + pip_id_map <- make_pip_id_map(survey, list(new_pip_id)) + + # Old master has BOTH pip_id rows, with OLD aux hashes. + old_master <- data.table::data.table( + survey_id = c(survey, survey), + pip_id = c(new_pip_id, stale_pip_id), + version_id_data = c("old_v1", "old_v2"), + version_id_metadata = c("old_m1", "old_m2"), + welfare_type = c("INC", "INC"), + country_code = "BOL", + surveyid_year = 2022L, + survey_acronym = "EH", + aux_cpi_hash = c("old_cpi", "old_cpi"), + first_release_version_id = NA_character_, + latest_release_version_id = NA_character_ + ) + + aux_hashes <- c(cpi = "new_cpi_hash") + + local_mocked_bindings( + st_catalog_query = function(alias = NULL) make_catalog(new_pip_id), + st_latest = function(...) "vid_bol", + .package = "stamp" + ) + local_mocked_bindings( + load_pip_master_inventory = function(...) old_master, + load_aux_data = function(measure, ...) make_pfw("BOL", 2022L, "EH"), + pip_write = function(x, id, alias, pk = NULL, ...) list(version_id = "v1"), + .package = "pipload" + ) + local_mocked_bindings( + log_add = null_log, + log_info = null_log, + log_error = null_log, + .package = "pipfun" + ) + + result <- build_pip_inventory(inv_to_clean, pip_id_map, aux_hashes = aux_hashes) + + # Only the reprocessed INC_ALL row remains; the stale INC_GPWG row is dropped. + expect_equal(nrow(result), 1L) + expect_equal(result$pip_id, new_pip_id) + expect_equal(result$aux_cpi_hash, "new_cpi_hash") +}) + +# --------------------------------------------------------------------------- +# P1.5: stateful persistence round trip — written master is reloaded +# --------------------------------------------------------------------------- + +test_that("build_pip_inventory persists aux hashes and retained rows through a write/reload round trip", { + new_pip_id <- "BRA_2019_PNAD_INC_ALL" + new_survey <- "BRA_2019_PNAD" + old_pip_id <- "CHN_2018_HIES_INC_ALL" + old_survey <- "CHN_2018_HIES" + + inv_to_clean <- make_inv_to_clean( + new_survey, + country_codes = "BRA", + surveyid_years = 2019L, + survey_acronyms = "PNAD" + ) + pip_id_map <- make_pip_id_map(new_survey, list(new_pip_id)) + + old_master <- data.table::data.table( + survey_id = old_survey, + pip_id = old_pip_id, + version_id_data = "old_vid_data", + version_id_metadata = "old_vid_meta", + welfare_type = "INC", + country_code = "CHN", + surveyid_year = 2018L, + survey_acronym = "HIES", + first_release_version_id = NA_character_, + latest_release_version_id = NA_character_ + ) + + aux_hashes <- c(cpi = "hash_cpi", ppp = "hash_ppp") + + # Stateful persistence: capture the master object written under + # "pip_master_inventory", then return it from the verification reload. + written_master <- NULL + local_mocked_bindings( + st_catalog_query = function(alias = NULL) make_catalog(new_pip_id), + st_latest = function(...) "vid_bra", + .package = "stamp" + ) + local_mocked_bindings( + load_pip_master_inventory = function(...) { + # First call (Step 1) returns the old master; the verification reload + # (Step 12) returns the just-written master. + if (is.null(written_master)) old_master else written_master + }, + load_aux_data = function(measure, ...) make_pfw("BRA", 2019L, "PNAD"), + pip_write = function(x, id, alias, pk = NULL, ...) { + if (identical(id, "pip_master_inventory")) { + written_master <<- data.table::copy(x) + } + list(version_id = paste0(id, "_vid"), skipped = FALSE) + }, + .package = "pipload" + ) + local_mocked_bindings( + log_add = null_log, + log_info = null_log, + log_error = null_log, + .package = "pipfun" + ) + + result <- build_pip_inventory(inv_to_clean, pip_id_map, aux_hashes = aux_hashes) + + # The written master must contain the new survey with aux hashes and the + # retained old survey. + expect_false(is.null(written_master)) + expect_true(new_pip_id %in% written_master$pip_id) + expect_true(old_pip_id %in% written_master$pip_id) + expect_equal( + written_master[written_master$pip_id == new_pip_id, aux_cpi_hash], + "hash_cpi" + ) + expect_equal( + written_master[written_master$pip_id == new_pip_id, aux_ppp_hash], + "hash_ppp" + ) + # The returned result reflects the reloaded (persisted) master. + expect_equal(nrow(result), 2L) + expect_true(new_pip_id %in% result$pip_id) + expect_true(old_pip_id %in% result$pip_id) +}) + +# --------------------------------------------------------------------------- +# P2.3: deterministic catalog dedup when created_at ties +# --------------------------------------------------------------------------- + +test_that("build_pip_inventory resolves tied catalog timestamps deterministically by version_id", { + pip_id <- "BOL_2022_EH_INC_ALL" + survey <- "BOL_2022_EH" + + inv_to_clean <- make_inv_to_clean( + survey, + country_codes = "BOL", + surveyid_years = 2022L, + survey_acronyms = "EH" + ) + pip_id_map <- make_pip_id_map(survey, list(pip_id)) + + # Two catalog rows for the same pip_id with IDENTICAL created_at but + # different paths. The path tiebreaker must select deterministically, + # regardless of input row order. + cat_dup <- data.table::data.table( + path = c("/fake/a/bol_2022_eh_inc_all.qs2", "/fake/b/bol_2022_eh_inc_all.qs2"), + version_id = c("vid_a", "vid_b"), + content_hash = c("hash_a", "hash_b"), + code_hash = c("code_a", "code_b"), + size_bytes = c(1000, 1000), + created_at = c("2026-01-01T00:00:00", "2026-01-01T00:00:00") + ) + + local_mocked_bindings( + st_catalog_query = function(alias = NULL) { + if (identical(alias, "pip")) cat_dup else make_catalog(pip_id) + }, + st_latest = function(...) "vid_bol", + .package = "stamp" + ) + local_mocked_bindings( + load_pip_master_inventory = function(...) NULL, + load_aux_data = function(measure, ...) make_pfw("BOL", 2022L, "EH"), + pip_write = function(x, id, alias, pk = NULL, ...) list(version_id = "v1"), + .package = "pipload" + ) + local_mocked_bindings( + log_add = null_log, + log_info = null_log, + log_error = null_log, + .package = "pipfun" + ) + + result <- build_pip_inventory(inv_to_clean, pip_id_map) + + # The path tiebreaker (path "a" < path "b") must select deterministically. + expect_equal(nrow(result), 1L) + expect_equal(result$version_id_data, "vid_a") + expect_equal(result$content_hash_data, "hash_a") }) \ No newline at end of file diff --git a/tests/testthat/test-get-aux-hashes.R b/tests/testthat/test-get-aux-hashes.R new file mode 100644 index 0000000..94c558c --- /dev/null +++ b/tests/testthat/test-get-aux-hashes.R @@ -0,0 +1,174 @@ +# Tests for get_aux_hashes() — run-level aux content-hash resolver +# +# Covers: +# happy path — all six default measures resolve, including PFW +# subset — only requested measures resolve +# missing artifact aborts +# ambiguous (multiple matching) artifact aborts +# empty catalog aborts +# catalog query failure aborts +# missing content_hash aborts + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +# Returns a catalog data.table matching the schema from st_catalog_query(), +# with one row per measure artifact under the aux alias. +make_aux_catalog <- function(measures = c("cpi", "ppp", "pfw", "pop", "gdp", "pce")) { + if (length(measures) == 0L) { + return(data.table::data.table( + path = character(), + version_id = character(), + content_hash = character(), + code_hash = character(), + size_bytes = numeric(), + created_at = character() + )) + } + data.table::data.table( + path = paste0("/aux/20260401_test/", measures, ".qs2"), + version_id = paste0("vid_", measures), + content_hash = paste0("hash_", measures), + code_hash = paste0("code_", measures), + size_bytes = rep(1000, length(measures)), + created_at = rep("2026-04-06T00:00:00", length(measures)) + ) +} + +# --------------------------------------------------------------------------- +# Happy path: all six default measures resolve, including PFW +# --------------------------------------------------------------------------- + +test_that("get_aux_hashes resolves all six default measures including PFW", { + testthat::local_mocked_bindings( + st_catalog_query = function(alias = NULL) make_aux_catalog(), + .package = "stamp" + ) + + result <- get_aux_hashes(c("pfw", "cpi", "ppp", "pop", "gdp", "pce")) + + expect_named(result, c("pfw", "cpi", "ppp", "pop", "gdp", "pce")) + expect_equal(result[["pfw"]], "hash_pfw") + expect_equal(result[["cpi"]], "hash_cpi") + expect_equal(result[["ppp"]], "hash_ppp") + expect_equal(result[["pop"]], "hash_pop") + expect_equal(result[["gdp"]], "hash_gdp") + expect_equal(result[["pce"]], "hash_pce") +}) + +# --------------------------------------------------------------------------- +# Subset: only requested measures resolve +# --------------------------------------------------------------------------- + +test_that("get_aux_hashes resolves only the requested measures", { + testthat::local_mocked_bindings( + st_catalog_query = function(alias = NULL) make_aux_catalog(), + .package = "stamp" + ) + + result <- get_aux_hashes(c("cpi", "ppp")) + + expect_named(result, c("cpi", "ppp")) + expect_equal(result[["cpi"]], "hash_cpi") + expect_equal(result[["ppp"]], "hash_ppp") +}) + +# --------------------------------------------------------------------------- +# Missing artifact aborts +# --------------------------------------------------------------------------- + +test_that("get_aux_hashes aborts when a requested artifact is missing", { + # Catalog only has cpi and ppp; request pfw too. + testthat::local_mocked_bindings( + st_catalog_query = function(alias = NULL) make_aux_catalog(c("cpi", "ppp")), + .package = "stamp" + ) + + expect_error( + get_aux_hashes(c("cpi", "pfw")), + class = "get_aux_hashes_missing_artifact" + ) +}) + +# --------------------------------------------------------------------------- +# Ambiguous artifact aborts +# --------------------------------------------------------------------------- + +test_that("get_aux_hashes aborts when multiple catalog rows match a measure", { + cat <- make_aux_catalog(c("cpi", "ppp")) + # Add a duplicate cpi artifact at a different path. + cat <- rbind( + cat, + data.table::data.table( + path = "/aux/20260401_test/other/cpi.qs2", + version_id = "vid_cpi_dup", + content_hash = "hash_cpi_dup", + code_hash = "code_cpi_dup", + size_bytes = 2000, + created_at = "2026-04-07T00:00:00" + ) + ) + + testthat::local_mocked_bindings( + st_catalog_query = function(alias = NULL) cat, + .package = "stamp" + ) + + expect_error( + get_aux_hashes("cpi"), + class = "get_aux_hashes_ambiguous_artifact" + ) +}) + +# --------------------------------------------------------------------------- +# Empty catalog aborts +# --------------------------------------------------------------------------- + +test_that("get_aux_hashes aborts when the aux catalog is empty", { + testthat::local_mocked_bindings( + st_catalog_query = function(alias = NULL) make_aux_catalog(character(0)), + .package = "stamp" + ) + + expect_error( + get_aux_hashes("cpi"), + class = "get_aux_hashes_empty_catalog" + ) +}) + +# --------------------------------------------------------------------------- +# Catalog query failure aborts +# --------------------------------------------------------------------------- + +test_that("get_aux_hashes aborts when the catalog query fails", { + testthat::local_mocked_bindings( + st_catalog_query = function(alias = NULL) stop("catalog corrupt"), + .package = "stamp" + ) + + expect_error( + get_aux_hashes("cpi"), + class = "get_aux_hashes_catalog_failure" + ) +}) + +# --------------------------------------------------------------------------- +# Missing content_hash aborts +# --------------------------------------------------------------------------- + +test_that("get_aux_hashes aborts when a measure has no content_hash", { + cat <- make_aux_catalog(c("cpi", "ppp")) + # Set the ppp row's content_hash to NA (row 2). + cat[2L, content_hash := NA_character_] + + testthat::local_mocked_bindings( + st_catalog_query = function(alias = NULL) cat, + .package = "stamp" + ) + + expect_error( + get_aux_hashes("ppp"), + class = "get_aux_hashes_missing_hash" + ) +}) diff --git a/tests/testthat/test-valid_aux_load.R b/tests/testthat/test-valid_aux_load.R index 2951549..17d0b3b 100644 --- a/tests/testthat/test-valid_aux_load.R +++ b/tests/testthat/test-valid_aux_load.R @@ -93,30 +93,54 @@ test_that("valid_aux_load(verbose=FALSE, compare='all') passes verbose=FALSE to test_that("valid_dlw_load(verbose=FALSE) propagates verbose=FALSE to valid_aux_load()", { aux_verbose <- logical(0) - # Minimal inventory — only columns needed by the module-filter and setorder steps. + # Minimal inventory — only columns needed by the module-filter, setorder, + # and aux-hash join steps. fake_inv <- data.table::data.table( survey_id = "ABC_2015_TST_INC_D1", country_id = "ABC", year = 2015L, - module = "ALL" + module = "ALL", + content_hash = "h_1" ) - # Mock all three internal functions together so we isolate the - # verbose-propagation path and avoid any external I/O. + # Master already has this survey with a DIFFERENT aux hash than current, + # so Stage 1 marks it a candidate and Stage 2 calls valid_aux_load(). + master <- data.table::data.table( + survey_id = "ABC_2015_TST_INC_D1", + content_hash_dlw = "h_1", + aux_cpi_hash = "old_cpi_hash" + ) + + # Mock the internal functions so we isolate the verbose-propagation path. testthat::local_mocked_bindings( valid_aux_load = function(measure, compare, verbose = TRUE, ...) { aux_verbose <<- c(aux_verbose, verbose) NULL # no aux changes → inv_aux = NULL }, - # last_ver_inv requires many inventory columns; return empty table so the - # function hits the early-return branch (no surveys to process → NULL). - last_ver_inv = function(inv, ...) data.table::data.table(), + # last_ver_inv requires many inventory columns; return the input so the + # survey survives to the aux-hash comparison. + last_ver_inv = function(inv, ...) inv, inv_to_process = function(inv, ...) NULL, .package = "pipdata" ) + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + # With a changed aux hash, valid_aux_load() is called (Stage 2) with the + # changed measure and verbose=FALSE. inv_to_process returns NULL (no DLW-new + # surveys), so the function aborts (class "piperr") after the aux path. suppressMessages( - valid_dlw_load(inv = fake_inv, verbose = FALSE) + expect_error( + valid_dlw_load( + inv = fake_inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "new_cpi_hash"), + verbose = FALSE + ), + class = "piperr" + ) ) expect_true( @@ -124,3 +148,79 @@ test_that("valid_dlw_load(verbose=FALSE) propagates verbose=FALSE to valid_aux_l info = "valid_aux_load should receive verbose=FALSE from valid_dlw_load" ) }) + +# --------------------------------------------------------------------------- +# P3.2: valid_aux_load(compare = "all") merge semantics +# --------------------------------------------------------------------------- + +# Helper: build a mock compare_aux_* output. compare_aux_* returns a list +# keyed by measure, where each element is itself a named list of change +# data.tables (plus an optional diff_cols element). The key_cols attribute is +# set on the per-measure list (changes_release$cpi) so cln_changes()/ +# check_unique() can read it. +make_aux_changes <- function(country_codes, surveyid_years) { + dt <- data.table::data.table( + country_code = country_codes, + surveyid_year = surveyid_years + ) + inner <- stats::setNames(list(dt), "release") + attr(inner, "key_cols") <- c("country_code", "surveyid_year") + stats::setNames(list(inner), "cpi") +} + +test_that("valid_aux_load(compare='all') returns both release and vintage changes", { + testthat::local_mocked_bindings( + compare_aux_releases = function(measure, owner, verbose = TRUE, ...) { + make_aux_changes("COL", 2020L) + }, + compare_aux_vintages = function(measure, verbose = TRUE, ...) { + make_aux_changes("ARG", 2019L) + }, + .package = "pipaux" + ) + + result <- valid_aux_load(measure = "cpi", compare = "all", verbose = FALSE) + + expect_false(is.null(result)) + expect_true("release" %in% names(result)) + expect_true("vintage" %in% names(result)) + # Release branch has the COL change. + expect_equal(result$release$cpi$country_code, "COL") + expect_equal(result$release$cpi$surveyid_year, 2020L) + # Vintage branch has the ARG change. + expect_equal(result$vintage$cpi$country_code, "ARG") + expect_equal(result$vintage$cpi$surveyid_year, 2019L) +}) + +test_that("valid_aux_load(compare='all') returns NULL for an empty branch", { + testthat::local_mocked_bindings( + compare_aux_releases = function(measure, owner, verbose = TRUE, ...) { + make_aux_changes("COL", 2020L) + }, + compare_aux_vintages = function(measure, verbose = TRUE, ...) { + list() # no vintage changes + }, + .package = "pipaux" + ) + + result <- valid_aux_load(measure = "cpi", compare = "all", verbose = FALSE) + + expect_false(is.null(result)) + expect_true("release" %in% names(result)) + expect_true("vintage" %in% names(result)) + # Release has changes; vintage is NULL. + expect_equal(result$release$cpi$country_code, "COL") + expect_null(result$vintage) +}) + +test_that("valid_aux_load(compare='all') returns NULL when both branches are empty", { + testthat::local_mocked_bindings( + compare_aux_releases = function(measure, owner, verbose = TRUE, ...) list(), + compare_aux_vintages = function(measure, verbose = TRUE, ...) list(), + .package = "pipaux" + ) + + result <- valid_aux_load(measure = "cpi", compare = "all", verbose = FALSE) + + expect_null(result) +}) diff --git a/tests/testthat/test-valid_dlw_load.R b/tests/testthat/test-valid_dlw_load.R index 4014584..c47e7d4 100644 --- a/tests/testthat/test-valid_dlw_load.R +++ b/tests/testthat/test-valid_dlw_load.R @@ -1,9 +1,17 @@ # Tests for valid_dlw_load.R helper functions # # Covers: -# inv_to_process() — joyn::anti_join must not leak .joyn column -# valid_dlw_load() — no duplicate survey_id in return value when a survey -# appears in both new-survey and aux-changed sets +# filter_aux_inv() — no max_year clamp; joyn::inner_join discards +# unmatched years naturally +# valid_dlw_load() — aux_no_changes_inf / aux_changes_no_surveys_inf / +# aux_changes_inf logmeta discriminators; cli_abort +# (class "piperr") when nothing to clean; no +# duplicate survey_id in return value when a survey +# appears in both new-survey and aux-changed sets; +# surveys_to_clean_inf summary logmeta +# inv_to_process() — content_hash vs content_hash_dlw comparison on +# survey_id; joyn::left_join must not leak .joyn +# column or duplicate rows on multi-pip_id surveys # --------------------------------------------------------------------------- # Helpers @@ -48,12 +56,22 @@ make_dlw_inv <- function( ) } +# Minimal master-inventory fragment with survey_id + content_hash_dlw, the +# only two columns inv_to_process() needs from the master inventory. +make_master_hash <- function(survey_ids, content_hash_dlw) { + data.table::data.table( + survey_id = survey_ids, + content_hash_dlw = content_hash_dlw + ) +} + # --------------------------------------------------------------------------- # inv_to_process: .joyn column must not be present in result # --------------------------------------------------------------------------- test_that("inv_to_process does not add .joyn column to result", { - # A single survey not in the master inventory — anti_join should keep it. + # A single survey not in the master inventory — the left_join leaves + # content_hash_dlw as NA, so the survey is kept. inv <- make_dlw_inv( "COL_2020_GEIH", country_codes = "COL", @@ -61,12 +79,9 @@ test_that("inv_to_process does not add .joyn column to result", { survey_acronyms = "GEIH" ) - # Master contains a DIFFERENT survey so our survey survives the anti_join. - master_no_col <- data.table::data.table( - country_code = "BRA", - surveyid_year = 2019L, - survey_acronym = "PNADC" - ) + # Master contains a DIFFERENT survey so our survey has no matching + # content_hash_dlw (NA after the left_join) and is kept. + master_no_col <- make_master_hash("BRA_2019_PNADC", "h_9") testthat::local_mocked_bindings( load_pip_master_inventory = function(...) master_no_col, @@ -78,19 +93,24 @@ test_that("inv_to_process does not add .joyn column to result", { expect_false( ".joyn" %in% names(result), info = paste0( - "joyn::anti_join in inv_to_process() must be called with ", - "reportvar = FALSE to avoid the .joyn column leaking into inv_to_clean ", - "and causing duplicate survey_id rows" + "joyn::left_join in inv_to_process() must be called with ", + "reportvar = FALSE to avoid the .joyn column leaking into the ", + "result and causing duplicate survey_id rows" ) ) + expect_false( + "content_hash_dlw" %in% names(result), + info = "content_hash_dlw must be dropped from the result after comparison" + ) }) # --------------------------------------------------------------------------- # valid_dlw_load: no duplicate survey_ids when survey is in both new + aux sets # --------------------------------------------------------------------------- -test_that("valid_dlw_load returns no duplicate survey_ids when survey appears in new and aux-changed sets", { - # COL 2020 GEIH: not in master (new survey) AND in aux-change set for CPI. +test_that("valid_dlw_load returns no duplicate survey_ids when survey appears in DLW-changed and aux-changed sets", { + # COL 2020 GEIH: DLW-content-changed (kept by inv_to_process) AND + # aux-hash-changed (candidate + affected by CPI change). inv <- make_dlw_inv( "COL_2020_GEIH", country_codes = "COL", @@ -98,20 +118,21 @@ test_that("valid_dlw_load returns no duplicate survey_ids when survey appears in survey_acronyms = "GEIH" ) - # Master without this survey → inv_to_process keeps it in inv_svy. - master_empty <- data.table::data.table( - country_code = character(0), - surveyid_year = integer(0), - survey_acronym = character(0) + # Master has this survey with a DIFFERENT DLW content_hash (so inv_to_process + # keeps it) AND a DIFFERENT aux hash (so it is an aux candidate). + master_changed <- data.table::data.table( + survey_id = "COL_2020_GEIH", + content_hash_dlw = "h_0", # differs from DLW's "h_1" → DLW-changed + aux_cpi_hash = "old_cpi_hash" # differs from current → aux candidate ) - # Aux change for COL 2020 → filter_aux_inv will add the same survey to inv_aux. + # Aux change for COL 2020 → filter_aux_inv returns the same survey. cpi_changes <- list( data.table::data.table(country_code = "COL", surveyid_year = 2020L) ) testthat::local_mocked_bindings( - load_pip_master_inventory = function(...) master_empty, + load_pip_master_inventory = function(...) master_changed, .package = "pipload" ) testthat::local_mocked_bindings( @@ -122,9 +143,12 @@ test_that("valid_dlw_load returns no duplicate survey_ids when survey appears in .package = "pipdata" ) + pipfun::log_init("pipdata_log", overwrite = TRUE) + result <- valid_dlw_load( inv = inv, aux_measures = "cpi", + aux_hashes = c(cpi = "new_cpi_hash"), force = FALSE, verbose = FALSE ) @@ -138,4 +162,987 @@ test_that("valid_dlw_load returns no duplicate survey_ids when survey appears in 0L, info = "valid_dlw_load must return unique survey_id values (no duplicates)" ) + + # surveys_to_clean_inf summary logmeta must be present with correct counts. + log <- pipfun::log_get("pipdata_log") + summary_entries <- Filter( + function(x) !is.null(x) && identical(x$info, "surveys_to_clean_inf"), + log$logmeta + ) + expect_length(summary_entries, 1L) + expect_equal(summary_entries[[1]]$n_total_unique, nrow(result)) +}) + +# --------------------------------------------------------------------------- +# filter_aux_inv: no max_year clamp -- join discards unmatched years +# --------------------------------------------------------------------------- + +test_that("filter_aux_inv handles an aux-change year beyond the max inventory year without a clamp", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + # Aux change year (2099) is beyond max(inv$surveyid_year) == 2020. With the + # max_year clamp removed, joyn::inner_join must discard this naturally. + changes_aux <- list( + data.table::data.table(country_code = "COL", surveyid_year = 2099L) + ) + + result <- pipdata:::filter_aux_inv(inv, changes_aux) + + expect_null( + result, + info = paste0( + "filter_aux_inv must rely on joyn::inner_join to discard unmatched ", + "years -- no max_year clamp should remain" + ) + ) +}) + +# --------------------------------------------------------------------------- +# valid_dlw_load: aux-change logmeta discriminators (aux_no_changes_inf / +# aux_changes_no_surveys_inf) +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load logs aux_no_changes_inf when no aux hash changed", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + # Master has this survey with the SAME aux hash as current → no candidate. + master_same <- data.table::data.table( + survey_id = "COL_2020_GEIH", + content_hash_dlw = "h_1", + aux_cpi_hash = "hash_cpi" + ) + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master_same, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) NULL, + .package = "pipdata" + ) + + pipfun::log_init("pipdata_log", overwrite = TRUE) + + # inv_to_process returns NULL (survey already cleaned, same DLW hash), and + # aux_hash_candidates returns NULL (same aux hash) → nothing to clean → abort. + suppressMessages( + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "hash_cpi"), + force = FALSE, + verbose = FALSE + ), + class = "piperr" + ) + ) + + log <- pipfun::log_get("pipdata_log") + info_values <- vapply( + log$logmeta, + function(x) if (is.null(x) || is.null(x$info)) NA_character_ else x$info, + character(1) + ) + + expect_true("aux_no_changes_inf" %in% info_values) + expect_false("aux_changes_no_surveys_inf" %in% info_values) + expect_false("aux_changes_inf" %in% info_values) +}) + +test_that("valid_dlw_load logs aux_changes_no_surveys_inf when aux changes affect no requested surveys", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + # Master has this survey with a DIFFERENT aux hash than current → candidate. + # But valid_aux_load returns changes only for BRA (not COL), so the + # intersection with the candidate (COL) is empty → aux_changes_no_surveys_inf. + master_changed <- data.table::data.table( + survey_id = "COL_2020_GEIH", + content_hash_dlw = "h_1", + aux_cpi_hash = "old_cpi_hash" + ) + + # Aux change for a country/year combination absent from inv (BRA, not COL). + cpi_changes <- list( + data.table::data.table(country_code = "BRA", surveyid_year = 2019L) + ) + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master_changed, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) { + stats::setNames(list(cpi_changes), measure) + }, + .package = "pipdata" + ) + + pipfun::log_init("pipdata_log", overwrite = TRUE) + + # inv_to_process returns NULL (survey already cleaned, same DLW hash), and + # the aux intersection is empty → nothing to clean → abort. + suppressMessages( + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "new_cpi_hash"), + force = FALSE, + verbose = FALSE + ), + class = "piperr" + ) + ) + + log <- pipfun::log_get("pipdata_log") + info_values <- vapply( + log$logmeta, + function(x) if (is.null(x) || is.null(x$info)) NA_character_ else x$info, + character(1) + ) + + expect_true("aux_changes_no_surveys_inf" %in% info_values) + expect_false("aux_no_changes_inf" %in% info_values) + expect_false("aux_changes_inf" %in% info_values) +}) + +# --------------------------------------------------------------------------- +# valid_dlw_load: cli_abort (class "piperr") when nothing to clean +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load aborts with class piperr when nothing to clean", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + # Master already has this survey with the SAME content_hash -- already clean. + master_same <- make_master_hash("COL_2020_GEIH", "h_1") + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master_same, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) NULL, + .package = "pipdata" + ) + + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + force = FALSE, + verbose = FALSE + ), + class = "piperr" + ) +}) + +# --------------------------------------------------------------------------- +# inv_to_process: content_hash vs content_hash_dlw comparison scenarios +# --------------------------------------------------------------------------- + +test_that("inv_to_process keeps a survey absent from master (content_hash_dlw is NA)", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + master <- make_master_hash("BRA_2019_PNADC", "h_9") + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + + result <- pipdata:::inv_to_process(inv, verbose = FALSE) + + expect_false(is.null(result)) + expect_equal(nrow(result), 1L) + expect_equal(result$survey_id, "COL_2020_GEIH") +}) + +test_that("inv_to_process excludes a survey whose content_hash matches content_hash_dlw", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + # make_dlw_inv() sets content_hash = "h_1" for the first (only) row. + master <- make_master_hash("COL_2020_GEIH", "h_1") + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + + result <- pipdata:::inv_to_process(inv, verbose = FALSE) + + expect_null( + result, + info = "a survey whose content_hash is unchanged since last clean must be excluded" + ) +}) + +test_that("inv_to_process keeps a survey whose content_hash differs from content_hash_dlw", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + # content_hash is "h_1"; master's content_hash_dlw is different ("h_0"). + master <- make_master_hash("COL_2020_GEIH", "h_0") + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + + result <- pipdata:::inv_to_process(inv, verbose = FALSE) + + expect_false(is.null(result)) + expect_equal(nrow(result), 1L) + expect_false( + "content_hash_dlw" %in% names(result), + info = "content_hash_dlw must be dropped from the result after comparison" + ) +}) + +test_that("inv_to_process does not fan out rows when master has multiple pip_id rows per survey_id", { + inv <- make_dlw_inv( + "BOL_2022_EH", + country_codes = "BOL", + surveyid_years = 2022L, + survey_acronyms = "EH" + ) + # Two pip_id rows (e.g. INC_ALL / INC_GPWG) for the same survey_id, both + # carrying the same content_hash_dlw, and different from the DLW's "h_1". + master_dup <- make_master_hash( + c("BOL_2022_EH", "BOL_2022_EH"), + c("h_0", "h_0") + ) + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master_dup, + .package = "pipload" + ) + + result <- pipdata:::inv_to_process(inv, verbose = FALSE) + + expect_false(is.null(result)) + expect_equal( + nrow(result), + 1L, + info = "joining on survey_id against a non-deduplicated master must not fan out matching rows" + ) +}) + +test_that("inv_to_process returns all surveys when the master inventory cannot be loaded", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) stop("no master exists"), + .package = "pipload" + ) + + result <- pipdata:::inv_to_process(inv, verbose = FALSE) + + expect_false(is.null(result)) + expect_equal(nrow(result), nrow(inv)) + expect_equal(result$survey_id, inv$survey_id) +}) + +test_that("inv_to_process result never carries a .joyn diagnostic column", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + master <- make_master_hash("BRA_2019_PNADC", "h_9") + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + + result <- pipdata:::inv_to_process(inv, verbose = FALSE) + + expect_false(".joyn" %in% names(result)) +}) + +# --------------------------------------------------------------------------- +# Two-stage aux gating: unchanged hash skips that measure's aux comparison +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load does not call valid_aux_load when aux hash is unchanged", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + # Master has this survey with the SAME aux hash as current → no candidate. + master_same <- data.table::data.table( + survey_id = "COL_2020_GEIH", + content_hash_dlw = "h_1", + aux_cpi_hash = "hash_cpi" + ) + + aux_called <- FALSE + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master_same, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) { + aux_called <<- TRUE + NULL + }, + .package = "pipdata" + ) + + # inv_to_process returns NULL (same DLW hash) and no aux candidate → abort. + suppressMessages( + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "hash_cpi"), + force = FALSE, + verbose = FALSE + ), + class = "piperr" + ) + ) + + expect_false( + aux_called, + info = "valid_aux_load must not be called when no aux hash changed" + ) +}) + +# --------------------------------------------------------------------------- +# Two-stage aux gating: changed hash invokes comparison only for changed measures +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load calls valid_aux_load only for changed measures", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + # Master: cpi hash changed (old vs new), ppp hash unchanged. + master <- data.table::data.table( + survey_id = "COL_2020_GEIH", + content_hash_dlw = "h_1", + aux_cpi_hash = "old_cpi_hash", + aux_ppp_hash = "hash_ppp" + ) + + measures_seen <- character(0) + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) { + measures_seen <<- c(measures_seen, measure) + # Return a change for COL so the survey is affected. + stats::setNames( + list(list(data.table::data.table(country_code = "COL", surveyid_year = 2020L))), + measure + ) + }, + .package = "pipdata" + ) + + result <- valid_dlw_load( + inv = inv, + aux_measures = c("cpi", "ppp"), + aux_hashes = c(cpi = "new_cpi_hash", ppp = "hash_ppp"), + force = FALSE, + verbose = FALSE + ) + + expect_false(is.null(result)) + expect_equal( + measures_seen, + "cpi", + info = "valid_aux_load must be called only for the changed measure (cpi), not ppp" + ) +}) + +# --------------------------------------------------------------------------- +# Two-stage aux gating: changed CPI affecting non-requested countries does not +# re-clean requested surveys (COL/ARG vs USA/GER) +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load does not re-clean requested surveys when aux changed only for other countries", { + # Requested surveys: COL 2020 and ARG 2019. + inv <- make_dlw_inv( + c("COL_2020_GEIH", "ARG_2019_EPH"), + country_codes = c("COL", "ARG"), + surveyid_years = c(2020L, 2019L), + survey_acronyms = c("GEIH", "EPH") + ) + + # Both surveys already cleaned with the OLD cpi hash → both are candidates. + master <- data.table::data.table( + survey_id = c("COL_2020_GEIH", "ARG_2019_EPH"), + content_hash_dlw = c("h_1", "h_2"), + aux_cpi_hash = c("old_cpi_hash", "old_cpi_hash") + ) + + # CPI changed only for USA and GER — not for COL or ARG. + cpi_changes <- list( + data.table::data.table(country_code = c("USA", "GER"), surveyid_year = c(2020L, 2019L)) + ) + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) { + stats::setNames(list(cpi_changes), measure) + }, + .package = "pipdata" + ) + + # inv_to_process returns NULL (both surveys already cleaned, same DLW hash), + # and the aux intersection is empty (changes only for USA/GER) → abort. + suppressMessages( + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "new_cpi_hash"), + force = FALSE, + verbose = FALSE + ), + class = "piperr" + ) + ) +}) + +# --------------------------------------------------------------------------- +# Two-stage aux gating: changed CPI affecting a requested survey returns it +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load re-cleans a requested survey affected by changed aux", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + # Survey already cleaned with the OLD cpi hash → candidate. + master <- data.table::data.table( + survey_id = "COL_2020_GEIH", + content_hash_dlw = "h_1", + aux_cpi_hash = "old_cpi_hash" + ) + + # CPI changed for COL 2020 → the requested survey is affected. + cpi_changes <- list( + data.table::data.table(country_code = "COL", surveyid_year = 2020L) + ) + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) { + stats::setNames(list(cpi_changes), measure) + }, + .package = "pipdata" + ) + + result <- valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "new_cpi_hash"), + force = FALSE, + verbose = FALSE + ) + + expect_false(is.null(result)) + expect_equal(result$survey_id, "COL_2020_GEIH") +}) + +# --------------------------------------------------------------------------- +# Two-stage aux gating: missing (NA) historical hash is ignored, not a candidate +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load ignores a survey with missing historical aux hash", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + # Master has the survey but NO aux_cpi_hash column (cleaned before feature). + master <- data.table::data.table( + survey_id = "COL_2020_GEIH", + content_hash_dlw = "h_1" + ) + + aux_called <- FALSE + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) { + aux_called <<- TRUE + NULL + }, + .package = "pipdata" + ) + + # The survey has no stored aux hash → ignored for aux-change detection. + # inv_to_process returns NULL (same DLW hash) and no aux candidate → abort. + suppressMessages( + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "new_cpi_hash"), + force = FALSE, + verbose = FALSE + ), + class = "piperr" + ) + ) + + expect_false( + aux_called, + info = "valid_aux_load must not be called when the only survey has no stored aux hash" + ) +}) + +# --------------------------------------------------------------------------- +# Two-stage aux gating: NA-hash count is logged when some surveys lack hashes +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load logs the number of surveys with missing aux hash", { + # Two surveys: COL has a populated hash (unchanged), ARG has NA hash. + inv <- make_dlw_inv( + c("COL_2020_GEIH", "ARG_2019_EPH"), + country_codes = c("COL", "ARG"), + surveyid_years = c(2020L, 2019L), + survey_acronyms = c("GEIH", "EPH") + ) + + master <- data.table::data.table( + survey_id = c("COL_2020_GEIH", "ARG_2019_EPH"), + content_hash_dlw = c("h_1", "h_2"), + aux_cpi_hash = c("hash_cpi", NA_character_) + ) + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) NULL, + .package = "pipdata" + ) + + pipfun::log_init("pipdata_log", overwrite = TRUE) + + # COL hash unchanged → no candidate; ARG has NA hash → ignored. Nothing to + # process → abort. + suppressMessages( + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "hash_cpi"), + force = FALSE, + verbose = FALSE + ), + class = "piperr" + ) + ) + + log <- pipfun::log_get("pipdata_log") + na_entries <- Filter( + function(x) !is.null(x) && identical(x$info, "aux_na_hash_inf"), + log$logmeta + ) + expect_length(na_entries, 1L) + expect_equal(na_entries[[1]]$n_surveys_na_hash, 1L) +}) + +# --------------------------------------------------------------------------- +# Two-stage aux gating: conflicting aux hashes for same survey/content abort +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load aborts on conflicting aux hashes for same survey and content_hash_dlw", { + inv <- make_dlw_inv( + "BOL_2022_EH", + country_codes = "BOL", + surveyid_years = 2022L, + survey_acronyms = "EH" + ) + + # Two pip_id rows for the same survey_id + content_hash_dlw, but with + # DIFFERENT aux_cpi_hash values → conflict. + master_conflict <- data.table::data.table( + survey_id = c("BOL_2022_EH", "BOL_2022_EH"), + content_hash_dlw = c("h_1", "h_1"), + aux_cpi_hash = c("hash_a", "hash_b") + ) + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master_conflict, + .package = "pipload" + ) + + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "hash_c"), + force = FALSE, + verbose = FALSE + ), + class = "aux_hash_candidates_conflict" + ) +}) + +# --------------------------------------------------------------------------- +# Two-stage aux gating: master inventory loaded exactly once +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load loads the master inventory exactly once", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + master <- data.table::data.table( + survey_id = "COL_2020_GEIH", + content_hash_dlw = "h_1", + aux_cpi_hash = "old_cpi_hash" + ) + + load_count <- 0L + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) { + load_count <<- load_count + 1L + master + }, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) { + stats::setNames( + list(list(data.table::data.table(country_code = "COL", surveyid_year = 2020L))), + measure + ) + }, + .package = "pipdata" + ) + + result <- valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "new_cpi_hash"), + force = FALSE, + verbose = FALSE + ) + + expect_false(is.null(result)) + expect_equal( + load_count, + 1L, + info = "the master inventory must be loaded exactly once and shared" + ) +}) + +# --------------------------------------------------------------------------- +# Two-stage aux gating: force mode skips master/aux comparisons +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load force mode skips master and aux comparisons and processes all rows", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + master_loaded <- FALSE + aux_called <- FALSE + candidates_called <- FALSE + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) { + master_loaded <<- TRUE + NULL + }, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) { + aux_called <<- TRUE + NULL + }, + aux_hash_candidates = function(...) { + candidates_called <<- TRUE + NULL + }, + .package = "pipdata" + ) + + result <- valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "new_cpi_hash"), + force = TRUE, + verbose = FALSE + ) + + expect_false(is.null(result)) + expect_equal(result$survey_id, "COL_2020_GEIH") + expect_false(master_loaded, info = "force mode must not load the master inventory") + expect_false(aux_called, info = "force mode must not call valid_aux_load") + expect_false(candidates_called, info = "force mode must not call aux_hash_candidates") +}) + +# --------------------------------------------------------------------------- +# Two-stage aux gating: no .joyn column or duplicate survey IDs in output +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load output has no .joyn column and no duplicate survey_ids", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + master <- data.table::data.table( + survey_id = "COL_2020_GEIH", + content_hash_dlw = "h_1", + aux_cpi_hash = "old_cpi_hash" + ) + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) { + stats::setNames( + list(list(data.table::data.table(country_code = "COL", surveyid_year = 2020L))), + measure + ) + }, + .package = "pipdata" + ) + + result <- valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "new_cpi_hash"), + force = FALSE, + verbose = FALSE + ) + + expect_false(".joyn" %in% names(result)) + expect_equal(anyDuplicated(result$survey_id), 0L) +}) + +# --------------------------------------------------------------------------- +# P1.1 regression: multiple historical content_hash_dlw rows for one survey +# --------------------------------------------------------------------------- + +test_that("aux_hash_candidates matches the current DLW content version, not a historical one", { + # Survey has TWO historical DLW content hashes in the master. The current + # DLW inventory corresponds to content_hash "h_2". Only the master row with + # content_hash_dlw == "h_2" should be used for the aux comparison. + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + # make_dlw_inv sets content_hash = "h_1" for the first row; override to h_2. + inv[, content_hash := "h_2"] + + # Master has two historical rows: h_1 (old aux hash) and h_2 (current aux + # hash matching the current run). The current run's aux hash equals the + # h_2 row's stored hash, so the survey should NOT be a candidate. + master <- data.table::data.table( + survey_id = c("COL_2020_GEIH", "COL_2020_GEIH"), + content_hash_dlw = c("h_1", "h_2"), + aux_cpi_hash = c("old_cpi_hash", "hash_cpi") + ) + + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) master, + .package = "pipload" + ) + testthat::local_mocked_bindings( + valid_aux_load = function(measure, compare, verbose = TRUE) { + stop("valid_aux_load must not be called: current aux hash is unchanged") + }, + .package = "pipdata" + ) + + # inv_to_process returns NULL (survey already cleaned, same DLW hash h_2), + # and the current aux hash matches the h_2 master row → no candidate → abort. + suppressMessages( + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "hash_cpi"), + force = FALSE, + verbose = FALSE + ), + class = "piperr" + ) + ) +}) + +# --------------------------------------------------------------------------- +# P2.1: failed master load must not trigger a second load +# --------------------------------------------------------------------------- + +test_that("inv_to_process does not re-load the master when it was already unavailable", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + load_count <- 0L + testthat::local_mocked_bindings( + load_pip_master_inventory = function(...) { + load_count <<- load_count + 1L + stop("no master exists") + }, + .package = "pipload" + ) + + # master_available = FALSE means the caller already attempted and failed to + # load the master; inv_to_process must return all surveys without re-loading. + result <- pipdata:::inv_to_process( + inv, + dt_master = NULL, + master_available = FALSE, + verbose = FALSE + ) + + expect_false(is.null(result)) + expect_equal(nrow(result), nrow(inv)) + expect_equal( + load_count, + 0L, + info = "inv_to_process must not re-load the master when it was already unavailable" + ) +}) + +# --------------------------------------------------------------------------- +# P2.2: aux_hashes input validation +# --------------------------------------------------------------------------- + +test_that("valid_dlw_load aborts on unnamed aux_hashes", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c("hash_cpi"), # unnamed + force = FALSE, + verbose = FALSE + ), + class = "valid_dlw_load_bad_aux_hashes" + ) +}) + +test_that("valid_dlw_load aborts on duplicate aux_hashes names", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = "hash_a", cpi = "hash_b"), + force = FALSE, + verbose = FALSE + ), + class = "valid_dlw_load_bad_aux_hashes" + ) +}) + +test_that("valid_dlw_load aborts on missing aux_hashes values", { + inv <- make_dlw_inv( + "COL_2020_GEIH", + country_codes = "COL", + surveyid_years = 2020L, + survey_acronyms = "GEIH" + ) + + expect_error( + valid_dlw_load( + inv = inv, + aux_measures = "cpi", + aux_hashes = c(cpi = NA_character_), + force = FALSE, + verbose = FALSE + ), + class = "valid_dlw_load_bad_aux_hashes" + ) })