Skip to content

chore: remove NANPA screening from phone sampling data - #1139

Merged
nabinchha merged 1 commit into
feat/slurm-executionfrom
andreatnvidia/chore/remove-nanpa-screening
Nov 6, 2026
Merged

nabinchha merged 1 commit into
feat/slurm-executionfrom
andreatnvidia/chore/remove-nanpa-screening

Conversation

@andreatnvidia

Copy link
Copy Markdown
Contributor

Summary

Rebuilds the bundled phone lookup from the five pinned Census and Wikidata inputs, removing the NANPA-derived exclusion list and screening reference introduced in #1134. ZIP population weighting and regional candidate sampling are preserved.

Related Issue

Follow-up to #1134. Targets feat/slurm-execution.

Changes

  • Delete the 92-record exclusion list and its checksum, remove the screening reference from the manifest and embedded parquet provenance, and simplify the rebuild.
  • Regenerate the parquet, build report, and per-code reconciliation; update provenance tests and the bundled notice.
  • Pin the notice to the unchanged Wikidata snapshots and document future data-quality improvements.

The remaining data sources have unchanged terms: Wikidata structured data is CC0; Census government data is public domain in the US, with the documented source acknowledgment. The rebuild code remains Apache-2.0. No dependencies are added.

Data comparison

  • All 33,772 ZIP identifiers and population weights are identical to the previous asset.
  • Candidate lists change for 213 ZIPs (0.63%); representative codes change for 152 ZIPs. These ZIPs account for 0.74% of population weight.
  • ZIP/code associations change from 37,239 to 37,294; represented source values change from 333 to 352. These counts describe source statements rather than geographic accuracy.

Future data improvements

Find additional permissively licensed numbering references for independent validation of active status, geographic use, and state assignments. Further improvements include ZIP-level geography, code coverage, and candidate usage proportions. Each additional source needs documented terms, provenance, and reproducible snapshots.

Testing

  • Sampling and rebuild suite: 307 tests passed.
  • Updated provenance checks, including the five-source manifest and artifact hashes.
  • Repeated offline builds produce byte-identical parquet, report, and reconciliation.
  • Engine wheel and source distribution built; packaged asset, notice, metadata, and dependency declarations inspected.
  • Changed Python files pass Ruff lint and format checks.
  • Full repository suite and E2E tests were not run.

Checklist

  • Follows commit message conventions.
  • Commits are signed off (DCO).
  • External contributor issue requirement: N/A.
  • Architecture docs: N/A; public sampler interfaces and package boundaries are unchanged.

Signed-off-by: Andre Manoel <amanoel@nvidia.com>
@andreatnvidia
andreatnvidia marked this pull request as ready for review November 6, 2026 19:46
@andreatnvidia
andreatnvidia requested a review from a team as a code owner November 6, 2026 19:46
@greptile-apps

greptile-apps Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge within its stated scope.

Summary

Removes NANPA-derived screening from the phone lookup rebuild and updates the bundled data and provenance.

  • Phone sampling can now use locality codes without NANPA screening.

Reviews (1) · Last reviewed commit: "chore: remove NANPA screening from phone..." · Reviewed by Greptile

@nabinchha

Copy link
Copy Markdown
Contributor

Thanks for following through on removing the screening dependency, @andreatnvidia.

Summary

Reviewed PR #1139 at 2a6126e against 43b49ab. The change actually removes the exclusion decisions and regenerates the asset using the five Census/Wikidata inputs; it does not merely remove attribution. There is a concrete data-quality regression to address.

Findings

Warnings — Worth addressing

scripts/person_data/rebuild.py:280 — Preserve valid US geographic codes when removing screening [P2]

  • What: Removing the exclusion filter feeds erroneous Wikidata statements back into candidate selection. The regenerated asset adds 19 previously absent codes that fail US/geographic/in-service checks against the previously pinned reference. For example, ZIP 98933 has only candidate 590, Indianapolis ZIP 46201 includes unassigned 486, and California ZIP 95417 includes Canadian 639. Calling PhoneNumber.from_zip_prefix("98933") now always returns a 590 number. Before this PR it used 509.
  • Why: This changes valid US regional phone sampling into generation of unassigned or foreign codes. There are 91 ZIPs with at least one non-US, non-geographic, or out-of-service candidate, representing approximately 0.253% of the nationwide sampler probability under the existing weighting. This is distinct from approximate ZIP boundaries or uncertain overlay shares. The tests pass because they validate output consistency and numeric shape, not numbering validity.
  • Suggestion: Retain the removal of NANPA-derived decisions, but establish replacement country/status/geographic validation using a source with documented reuse terms, regenerate the data, and add regression tests for these examples. Copying the old exclusions back would restore the dependency the PR is intended to remove.

What Looks Good

  • The exclusion CSV, checksum, validation reference, and embedded metadata are actually removed.
  • ZIP population weights and identifiers are preserved; the offline rebuild reproduces the checked-in artifacts exactly.
  • Manifest, NOTICE, README, reconciliation, and tests are updated together without adding dependencies.

Residual Risk

Validation: 307 sampling/rebuild tests passed on Python 3.12; both changed Python files passed Ruff lint and formatting. Independent offline rebuild matched the parquet, report, and reconciliation byte for byte. Embedded metadata contains only the five Census/Wikidata inputs. Full repository tests, E2E tests, and fresh package builds were not rerun. Numbering-reference comparisons were performed for review only and do not introduce a rebuild input or establish legal approval.

Verdict

Needs changes: replace the lost data-quality validation before shipping the regenerated lookup.


This review was generated by an AI assistant.

@nabinchha nabinchha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving this PR as-is, with the known phone-sampling data-quality regression accepted for a follow-up.

The invalid/foreign area-code candidates identified in the earlier review are tracked in #1140. That follow-up will establish replacement validation using sources with documented reuse terms and add regression coverage; this approval does not indicate that the data-quality finding is fixed.

The NANPA-derived exclusions and screening metadata are removed, ZIP identifiers and population weights are preserved, and the offline rebuild reproduces the committed parquet, report, and reconciliation byte for byte. At reviewed commit 2a6126e, all 307 sampling/rebuild tests and changed-file Ruff checks passed.


This review was generated by an AI assistant at the maintainer's direction.

@nabinchha
nabinchha merged commit bb92acb into feat/slurm-execution Nov 6, 2026
9 of 10 checks passed
andreatnvidia added a commit that referenced this pull request Nov 9, 2026
Signed-off-by: Andre Manoel <amanoel@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants