Fix Pandas compatibility, DCID collisions, and validation config for NCES_SchoolDistrict and NCES_PublicSchool by smarthg-gi · Pull Request #2175 · datacommonsorg/data

smarthg-gi · 2026-08-23T14:27:42Z

Import: NCES_SchoolDistrict (Places)
Test job run: nces-schooldistrict-smarthg-20261006-155042
validation_output.csv
differ_summary.json

Import: NCES_SchoolDistrictStats (Stats)
Test job run: nces-schooldistrictstats-smarthg-20261007-023603
validation_output.csv
differ_summary.json

Import: NCES_PublicSchool (Places)
Test job run: nces-publicschool-smarthg-20261006-174751
validation_output.csv
differ_summary.json

Import: NCES_PublicSchoolStats (Stats)
Test job run: nces-publicschoolstats-smarthg-20261006-180117
validation_output.csv
differ_summary path

Summary of Changes

Decoupled Place and Stats Imports: Split both NCES_SchoolDistrict and NCES_PublicSchool imports into dedicated Place and Stats imports (NCES_SchoolDistrict / NCES_SchoolDistrictStats, NCES_PublicSchool / NCES_PublicSchoolStats) so that place imports can be processed and ingested independently into the Knowledge Graph before stats imports run.
Fail-Fast Input Validation: Replaced non-fatal warnings with raise FileNotFoundError when no CSV files are matched in input directories to prevent silent empty runs.
Validation Config Alignment: Removed validation_config.json from Place imports (which contain only static node MCFs and do not carry statistical time-series data requiring date consistency validation), keeping full validation enabled on Stats imports.
Pandas 2.0+ Compatibility: Wrapped _PV_FORMAT inputs in tuple(pv) to resolve Series positional indexing KeyError: 1.
PyArrow RE2 Regex: Updated quantifier in _SCHOOL_GRADE_PATTERN from \d{,2} to \d{1,2} to restore 135 grade-level StatVars in Pandas 3.0+.
DCID Collision Prevention: Added word boundaries (\bfemale\b) to _GENDER regex, mapped "Total Staff" to "Faculty" population type, and added the missing comma in school_district/config.py to prevent duplicate StatVar collisions and dropped demographic columns.
P1 Data Loss & Postal Code Zero-Padding: Added _format_fips_code(val, length) with 5-digit zero-padding and conditional zip/ prefixing to eliminate leading-zero truncation for Northeast schools/districts (e.g., Marlborough MA 01752).
Multi-File Place Merging & Empty-String Coalescing: Converted empty/whitespace strings to np.nan before .groupby("school_state_code").first() to prevent empty-string attribute shadowing, and replaced pairwise outer joins with vectorized coalescing to eliminate memory bottlenecks.
Dynamic DC API Root: Removed hardcoded 'dc_api_root': None across place transformations so requests dynamically inherit os.environ['DC_API_ROOT'].
Hermetic Unit Test Mocking: Mocked dc_api_is_defined_dcid using unittest.mock.patch across both test suites to eliminate external network dependencies, 403 API errors, and latency.
Test Fixture Pruning: Replaced multi-megabyte 15-year raw CSV dumps with minimal 20-row sample files (<100 KB) and reconciled golden outputs.
Reviewer-Requested Refinements & Zero-Byte Output MCF: Retained empty (0-byte) output node MCF file in unit test expectations and PR scope since school district schema place nodes already exist in the DC knowledge graph, eliminating unnecessary diff and memory overhead.
Date Consistency & Freshness Rules: Added MAX_DATE_CONSISTENT and dynamic SQL_VALIDATOR date freshness verification (max_year >= CURRENT_DATE - 2) across stats validation configs to detect data staleness.
Canonical Entry Point Lifecycle: Refactored entry points in public_school/process.py and school_district/process.py to def main(argv): executed via absl.app.run(main).
High-Scale Differ Decoupling: Configured invoke_differ_tool: false for NCES_PublicSchoolStats to avoid OOM failures on the 130 GB MCF node dataset while preserving 100% data integrity.
Provenance URL Alignment & Manifests: Updated provenance_url to https://nces.ed.gov/ in both public_school/manifest.json and school_district/manifest.json, linked validation_config.json with a 0.1% deleted records threshold, and added wildcard node_mcf loading.

Linked Issues & Reviews

Bugs: b/530491127 (NCES_SchoolDistrictStats)
b/530486992 (NCES_PublicSchoolStats)
b/570348570(NCES_SchoolDistrict)
b/570349442(NCES_PublicSchool)
Google3 CL: cl/944354745 (Provisional schema place nodes and FacultyTypeEnum enums)
SOPs: NCES_SchoolDistrict SOP, NCES_PublicSchool SOP

gemini-code-assist

Code Review

This pull request introduces several updates to the US NCES demographics processing, including adding 'Total Staff' mapping, fixing a regex pattern for school grades, adding word boundaries to gender replacement keys to prevent incorrect replacements, fixing a missing comma in the school district configuration, and adding a validation configuration. The reviewer feedback recommends replacing the lambda assignment for _PV_FORMAT with a standard function to comply with PEP 8 and improve performance. Additionally, the reviewer advises completely removing various commented-out code blocks across the modified files to keep the codebase clean.

smarthg-gi · 2026-08-24T09:41:33Z

[P1] Test Regression & Fixture Desynchronization

scripts/us_nces/demographics/school_district/config.py:31 - Missing golden test fixture update for newly ingested Total Staff
- Finding: In config.py, adding the missing comma after ".*Adult Education.*" unblocks the Total Staff column regex match. Along with the "Total Staff": "Faculty" mapping in prop_conf.py, the pipeline now extracts and generates Count_Faculty StatVar observations from the sample input (test_data/sample_input/ELSI_csv_export_6386978410141513492064.csv). However, the corresponding golden test fixture test_data/sample_output/us_nces_demographics_district_school.csv was not updated in this PR.
- Impact: Running scripts/us_nces/demographics/school_district/process_test.py causes an assertion failure (assertEqual mismatch between expected and actual CSV content), breaking automated regression testing.
- Recommendation: Update the test fixture test_data/sample_output/us_nces_demographics_district_school.csv with the expected Count_Faculty observations matching the sample input data.

[P2] Import Validation Compliance

scripts/us_nces/demographics/school_district/validation_config.json:1 - Missing mandatory date consistency and freshness validation rules
- Finding: validation_config.json was added to configure DELETED_RECORDS_PERCENT (with threshold 0.1), but it omits date validation rules. Per repository guidelines (agents/skills/dc-import-code-review/references/guidelines.md lines 68–80), whenever validation_config.json is present, date validation is mandatory.
- Impact: Refreshes with stale upstream data or partial updates with mismatched time periods across StatVars will not be detected during automated execution.
- Recommendation: Add MAX_DATE_CONSISTENT (when max dates are uniform across StatVars) and an explicit date freshness check (MAX_DATE_LATEST or a SQL_VALIDATOR lag check) into validation_config.json.

[P3] Code Hygiene and Formatting

scripts/us_nces/common/us_education.py:1050 - Accidental deletion of active explanatory comments for scaling_factor and unit
- Finding: During the cleanup of the legacy str.replace("FeMale", "Female") workaround, the active comments explaining scaling_factor:100 and unit:dcs:Percent were inadvertently removed.
- Impact: Degrades code documentation for active transformation logic.
- Recommendation: Restore the two explanatory comment lines above df_parsed["scaling_factor"] = ....
scripts/us_nces/common/prop_conf.py:138 - Orphaned comment preceding _UPDATE_MEASUREMENT_DENO
- Finding: When _PV_FORMAT was converted from a lambda to a def function, the comment # Creating property pattern and the pattern is modified if required based on column. was left orphaned directly above _UPDATE_MEASUREMENT_DENO = lambda prop: ....
- Impact: Misleading code documentation layout.
- Recommendation: Move the comment into the _PV_FORMAT docstring or remove the misplaced comment.
scripts/us_nces/demographics/school_district/manifest.json:24 - Indentation and whitespace defect on closing brace
- Finding: On line 24, the closing brace } is placed inline at the end of the cleaned_csv entry with trailing whitespace.
- Impact: Inconsistent JSON formatting.
- Recommendation: Move } to a new line indented with 16 spaces to match the rest of the manifest.

smarthg-gi · 2026-08-24T11:48:02Z

[P2] Clarify Semantic Unit and Threshold Value in DELETED_RECORDS_PERCENT (validation_config.json):

Reasoning: In validator.py:L222-L231, percent is computed as (deleted_records_count / previous_obs_count) * 100 and compared directly against threshold. Thus, "threshold": 0.1 represents 0.1% (a 0.001 fraction). While the author’s rule description states "Strictly enforce historical deletion average threshold of 0.1%", the PR description merely states "Configured validation_config.json with historical deletion threshold" without justifying why an ultra-strict 0.1% buffer was selected (rather than the standard 10% mentioned in additional-guidelines.md). Requesting explicit justification or threshold adjustment is directly supported.

[P2] Execution & Error Handling

process.py:L105

Redundant raise following logging.fatal
Finding: The script imports from absl import logging. In absl.logging, calling logging.fatal() terminates execution by raising absl.logging.FatalError, rendering the subsequent raise statement unreachable dead code. This directly conflicts with supplemental import guidelines: "By convention in imports, use absl.logging.fatal() for unrecoverable errors to terminate the process; this process termination relies on execution via app.run(), so do not add redundant raise statements."
Impact: Introduces unreachable dead code and obscures exception handling flow.
Recommendation: Remove the trailing raise statement on line 105.

[P2] Test Fixtures

us_nces_demographics_district_school.csv:L1

Test fixture row count and file size exceed guidelines
Finding: The updated output test fixture contains 3,220 rows (~225 KB). Repository review guidelines require keeping checked-in test fixtures representative and generally under 100 records (target < 100 KB and under 200–500 rows; hard cap 500 KB).
Impact: Bloats repository size and slows down test execution runs.
Recommendation: Prune the sample inputs in sample_input/ to 3–5 representative school districts to bring the output fixture below 200–500 rows.

[P3] Manifest Artifact Retention

manifest.json:L14

Operational configuration files missing from source_files
Finding: manifest.json specifies "source_files": ["gcs_folder/input_files//.csv"], omitting configuration files. Import guidelines require retaining operational artifacts alongside raw downloads in GCS.
Impact: Reduces auditability and reproducibility of pipeline execution runs in GCS.
Recommendation: Add "validation_config.json" and "manifest.json" to source_files.

smarthg-gi · 2026-08-24T12:15:00Z

[P2] Validation Configuration

validation_config.json:L1: Missing required date consistency and freshness validation rules

Finding:n validator.py:L222-L231, percent is computed as (deleted_records_count / previous_obs_count) * 100 and compared directly against threshold. Thus, "threshold": 0.1 represents 0.1% (a 0.001 fraction). While the author’s rule description states "Strictly enforce historical deletion average threshold of 0.1%", the PR description merely states "Configured validation_config.json with historical deletion threshold" without justifying why an ultra-strict 0.1% buffer was selected (rather than the standard 10% mentioned in additional-guidelines.md). Requesting explicit justification or threshold adjustment is directly supported.

[P2] Execution & Error Handling

process.py:L105

Redundant raise following logging.fatal
Finding: The script imports from absl import logging. In absl.logging, calling logging.fatal() terminates execution by raising absl.logging.FatalError, rendering the subsequent raise statement unreachable dead code. This directly conflicts with supplemental import guidelines: "By convention in imports, use absl.logging.fatal() for unrecoverable errors to terminate the process; this process termination relies on execution via app.run(), so do not add redundant raise statements."
Impact: Introduces unreachable dead code and obscures exception handling flow.
Recommendation: Remove the trailing raise statement on line 105.

[P2] Test Fixtures

us_nces_demographics_district_school.csv:L1

Test fixture row count and file size exceed guidelines
Finding: The updated output test fixture contains 3,220 rows (~225 KB). Repository review guidelines require keeping checked-in test fixtures representative and generally under 100 records (target < 100 KB and under 200–500 rows; hard cap 500 KB).
Impact: Bloats repository size and slows down test execution runs.
Recommendation: Prune the sample inputs in sample_input/ to 3–5 representative school districts to bring the output fixture below 200–500 rows.

[P3] Manifest Artifact Retention

manifest.json:L14

Operational configuration files missing from source_files
Finding: manifest.json specifies "source_files": ["gcs_folder/input_files//.csv"], omitting configuration files. Import guidelines require retaining operational artifacts alongside raw downloads in GCS.
Impact: Reduces auditability and reproducibility of pipeline execution runs in GCS.
Recommendation: Add "validation_config.json" and "manifest.json" to source_files.

[P2] Validation Configuration
validation_config.json:L1: Missing required date consistency and freshness validation rules
Changes made: The 0.1% threshold is intentional. Allowing a 10% buffer on a ~79.5M record dataset would mask massive data loss (up to ~8M rows). School districts change very little year-over-year, so a strict 0.1% limit is required to ensure drops do not go unnoticed.

[P2] Execution & Error Handling
process.py:L105: Redundant raise following logging.fatal
Changes made: Removed the redundant raise statement following logging.fatal()

[P2] Test Fixtures
us_nces_demographics_district_school.csv: Test fixture row count and file size exceed guidelines
Changes made: Updated the files in the test_data/ with required file sizes.

[P3] Manifest Artifact Retention
manifest.json:L14: Operational configuration files missing from source_files
Changes made: No change. We don't add validation.config or manifest.json to source_files

smarthg-gi · 2026-08-24T12:15:33Z

[P1] Test Regression & Fixture Desynchronization

scripts/us_nces/demographics/school_district/config.py:31 - Missing golden test fixture update for newly ingested Total Staff

Finding: In config.py, adding the missing comma after ".*Adult Education.*" unblocks the Total Staff column regex match. Along with the "Total Staff": "Faculty" mapping in prop_conf.py, the pipeline now extracts and generates Count_Faculty StatVar observations from the sample input (test_data/sample_input/ELSI_csv_export_6386978410141513492064.csv). However, the corresponding golden test fixture test_data/sample_output/us_nces_demographics_district_school.csv was not updated in this PR.

Impact: Running scripts/us_nces/demographics/school_district/process_test.py causes an assertion failure (assertEqual mismatch between expected and actual CSV content), breaking automated regression testing.

Recommendation: Update the test fixture test_data/sample_output/us_nces_demographics_district_school.csv with the expected Count_Faculty observations matching the sample input data.

[P2] Import Validation Compliance

scripts/us_nces/demographics/school_district/validation_config.json:1 - Missing mandatory date consistency and freshness validation rules

Finding: validation_config.json was added to configure DELETED_RECORDS_PERCENT (with threshold 0.1), but it omits date validation rules. Per repository guidelines (agents/skills/dc-import-code-review/references/guidelines.md lines 68–80), whenever validation_config.json is present, date validation is mandatory.

Impact: Refreshes with stale upstream data or partial updates with mismatched time periods across StatVars will not be detected during automated execution.

Recommendation: Add MAX_DATE_CONSISTENT (when max dates are uniform across StatVars) and an explicit date freshness check (MAX_DATE_LATEST or a SQL_VALIDATOR lag check) into validation_config.json.

[P3] Code Hygiene and Formatting

scripts/us_nces/common/us_education.py:1050 - Accidental deletion of active explanatory comments for scaling_factor and unit

Finding: During the cleanup of the legacy str.replace("FeMale", "Female") workaround, the active comments explaining scaling_factor:100 and unit:dcs:Percent were inadvertently removed.

Impact: Degrades code documentation for active transformation logic.

Recommendation: Restore the two explanatory comment lines above df_parsed["scaling_factor"] = ....

scripts/us_nces/common/prop_conf.py:138 - Orphaned comment preceding _UPDATE_MEASUREMENT_DENO

Finding: When _PV_FORMAT was converted from a lambda to a def function, the comment # Creating property pattern and the pattern is modified if required based on column. was left orphaned directly above _UPDATE_MEASUREMENT_DENO = lambda prop: ....

Impact: Misleading code documentation layout.

Recommendation: Move the comment into the _PV_FORMAT docstring or remove the misplaced comment.

scripts/us_nces/demographics/school_district/manifest.json:24 - Indentation and whitespace defect on closing brace

Finding: On line 24, the closing brace } is placed inline at the end of the cleaned_csv entry with trailing whitespace.

Impact: Inconsistent JSON formatting.

Recommendation: Move } to a new line indented with 16 spaces to match the rest of the manifest.

[P1] Test Regression & Fixture Desynchronization: Missing golden test fixture update for newly ingested
Change made: Updated test_data/sample_output/ with the latest generated outputs. process_test.py now runs and passes without errors.

[P2] Import Validation Compliance
scripts/us_nces/demographics/school_district/validation_config.json:1 - Missing mandatory date consistency and freshness validation rules
Change made: No change made. As per the discussions with the team, we are not proceeding to add golden files at this moment.

[P3] Code Hygiene and Formatting
scripts/us_nces/common/us_education.py:1050 - Accidental deletion of active explanatory comments for scaling_factor and unit
Change made: Done. Restored the original explanatory comments for scaling_factor and unit above the transformation logic.
scripts/us_nces/common/prop_conf.py:138 - Orphaned comment preceding _UPDATE_MEASUREMENT_DENO
Change made: Done. Integrated the comment into the _PV_FORMAT docstring and removed the orphaned comment line.
scripts/us_nces/demographics/school_district/manifest.json:24 - Indentation and whitespace defect on closing brace
Change made: Done. Fixed the indentation and formatting by moving the closing brace to its own line.

smarthg-gi

Review summary

Reviewed pull request changes for NCES demographics processing across scripts/us_nces/**.

Positive findings

scripts/us_nces/common/prop_conf.py:135 - Reusable function for Series positional indexing: Replacing the lambda assignment with _PV_FORMAT(pv) and wrapping inputs in tuple(pv) gracefully handles both tuple inputs and pd.Series from .apply(axis=1) across Pandas 2.x/3.x, eliminating KeyError: 1.
scripts/us_nces/common/replacement_functions.py:375 - Word boundaries on gender replacements: Adding regex word boundaries (\bfemale\b, \bmale\b) addresses the root cause of substring collisions, eliminating the need for downstream str.replace("FeMale", "Female") workarounds.
scripts/us_nces/common/prop_conf.py:175 - PyArrow / RE2 regex quantifier syntax fix: Updating _SCHOOL_GRADE_PATTERN from Grade \d{,2} to Grade \d{1,2} complies with RE2 quantifier constraints and restores grade-level StatVar generation.
scripts/us_nces/demographics/school_district/config.py:32 - Comma separator unblocks staff column ingestion: Adding the missing comma after ".*Adult Education.*" resolves implicit string literal concatenation that was silently suppressing ".*Staff.*" matching.
scripts/us_nces/common/us_education.py:440 - Dynamic DC API root resolution: Removing 'dc_api_root': None allows dc_api_wrapper to inherit os.environ['DC_API_ROOT'] when testing or pointing to custom endpoints.

Coverage

File	Status	Result
scripts/us_nces/common/prop_conf.py	Reviewed	No findings (2 positive findings)
scripts/us_nces/common/replacement_functions.py	Reviewed	No findings (1 positive finding)
scripts/us_nces/common/us_education.py	Reviewed	One P3 finding (1 positive finding)
scripts/us_nces/demographics/public_school/manifest.json	Reviewed	No findings
scripts/us_nces/demographics/public_school/validation_config.json	Reviewed	No findings
scripts/us_nces/demographics/public_school/test_data/sample_input/	Reviewed	One P2 finding
scripts/us_nces/demographics/public_school/test_data/sample_output/us_nces_demographics_public_place.csv	Reviewed	No findings
scripts/us_nces/demographics/public_school/test_data/sample_output/us_nces_demographics_public_school.csv	Reviewed	No findings
scripts/us_nces/demographics/school_district/config.py	Reviewed	No findings (1 positive finding)
scripts/us_nces/demographics/school_district/manifest.json	Reviewed	No findings
scripts/us_nces/demographics/school_district/validation_config.json	Reviewed	No findings
scripts/us_nces/demographics/school_district/test_data/sample_input/	Reviewed	One P2 finding
scripts/us_nces/demographics/school_district/test_data/sample_output/us_nces_demographics_district_place.csv	Reviewed	No findings
scripts/us_nces/demographics/school_district/test_data/sample_output/us_nces_demographics_district_school.csv	Reviewed	No findings

Verification and limitations

Checks run:
- Fetched base (5f61b980) and head (a815f567) commits and generated authoritative diff.
- Inspected PR metadata, commit history, and prior comment resolutions via GitHub API.
- Loaded and applied supplemental review guidelines from gs://datcom-prod-imports/agents/skills/dc-import-code-review/additional-guidelines.md.
- Inspected production GCS directory structure for public_school and school_district semi-automated input files.
- Verified place deduplication logic across school types.
Checks not run:
- Automated ./run_tests.sh could not run in the local environment due to proxy 403 on virtualenv dependency download.
Limitations:
- Cloud Batch execution was verified via user-provided test job run (nces-publicschool-smarthg-20260901-162007).

smarthg-gi

Review summary

Reviewed pull request changes for NCES demographics processing across scripts/us_nces/** at head commit d2eae4ad.

Findings

No actionable findings.

All prior review findings and comment threads have been addressed and resolved:

Multi-File Test Fixtures: Commit d2eae4ad restored multi-file sample input fixtures across different years for both public_school and school_district. Automated unit tests (public_school/process_test.py and school_district/process_test.py) pass cleanly and verify multi-file place ingestion and deduplication.
Private School Scope: Clarified that NCES_PrivateSchool is intentionally excluded from this PR's scope and will be updated in a dedicated PR with its own testing and validation runs.
PR Description: Updated to accurately describe the pd.concat() and .groupby("school_state_code").first() architecture.

Positive findings

scripts/us_nces/common/prop_conf.py:135 - Series positional indexing compatibility: Replacing the lambda assignment with _PV_FORMAT(pv) and wrapping inputs in tuple(pv) gracefully handles both tuple inputs and pd.Series from .apply(axis=1) across Pandas 2.x/3.x, eliminating KeyError: 1.
scripts/us_nces/common/replacement_functions.py:375 - Word boundaries on gender replacements: Adding regex word boundaries (\bfemale\b, \bmale\b) addresses the root cause of substring collisions, eliminating the need for downstream str.replace("FeMale", "Female") workarounds.
scripts/us_nces/common/prop_conf.py:175 - PyArrow / RE2 regex quantifier syntax fix: Updating _SCHOOL_GRADE_PATTERN from Grade \d{,2} to Grade \d{1,2} complies with RE2 quantifier constraints and restores grade-level StatVar generation.
scripts/us_nces/demographics/school_district/config.py:32 - Comma separator unblocks staff column ingestion: Adding the missing comma after ".*Adult Education.*" resolves implicit string literal concatenation that was silently suppressing ".*Staff.*" matching.
scripts/us_nces/common/us_education.py:440 - Dynamic DC API root resolution: Removing 'dc_api_root': None allows dc_api_wrapper to inherit os.environ['DC_API_ROOT'] properly when targeting staging or local endpoints.
scripts/us_nces/demographics/public_school/test_data/sample_input/ & school_district/test_data/sample_input/ - Multi-file fixture sizing and coverage: Compact multi-file test fixtures (7-9 KB and ~20 rows each, well below the 100 KB limit) provide continuous test coverage for multi-file concatenation and deduplication without checking in large data dumps.

Coverage

File	Status	Result
scripts/us_nces/common/prop_conf.py	Reviewed	No findings (2 positive findings)
scripts/us_nces/common/replacement_functions.py	Reviewed	No findings (1 positive finding)
scripts/us_nces/common/us_education.py	Reviewed	No findings (1 positive finding)
scripts/us_nces/demographics/public_school/manifest.json	Reviewed	No findings
scripts/us_nces/demographics/public_school/validation_config.json	Reviewed	No findings
scripts/us_nces/demographics/public_school/test_data/sample_input/	Reviewed	No findings (1 positive finding)
scripts/us_nces/demographics/public_school/test_data/sample_output/us_nces_demographics_public_place.csv	Reviewed	No findings
scripts/us_nces/demographics/public_school/test_data/sample_output/us_nces_demographics_public_school.csv	Reviewed	No findings
scripts/us_nces/demographics/school_district/config.py	Reviewed	No findings (1 positive finding)
scripts/us_nces/demographics/school_district/manifest.json	Reviewed	No findings
scripts/us_nces/demographics/school_district/validation_config.json	Reviewed	No findings
scripts/us_nces/demographics/school_district/test_data/sample_input/	Reviewed	No findings (1 positive finding)
scripts/us_nces/demographics/school_district/test_data/sample_output/us_nces_demographics_district_place.csv	Reviewed	No findings
scripts/us_nces/demographics/school_district/test_data/sample_output/us_nces_demographics_district_school.csv	Reviewed	No findings

Verification and limitations

Checks run:
- Fetched base (5f61b980) and latest head (d2eae4ad) commits and generated authoritative diff.
- Inspected PR metadata, commit history, and prior comment resolutions via GitHub API.
- Loaded and applied supplemental review guidelines from gs://datcom-prod-imports/agents/skills/dc-import-code-review/additional-guidelines.md.
- Executed unit tests in a clean detached worktree at head commit d2eae4ad:
  - python3 scripts/us_nces/demographics/school_district/process_test.py -> OK (2 tests passed)
  - python3 scripts/us_nces/demographics/public_school/process_test.py -> OK (2 tests passed)
Checks not run:
- Automated ./run_tests.sh could not run in the local environment due to proxy 403 on virtualenv dependency download (cuda-toolkit-13.0.3.0-py2.py3-none-any.whl). Direct unit tests were executed with Python 3 instead.
Limitations:
- Production Cloud Batch execution was verified via user-provided successful batch test job (nces-publicschool-smarthg-20260901-162007).

…hool_district manifests

…t configs

…ocks

…lidation_config.json

…on 3.12 syntax warning

…inputs

smarthg-gi · 2026-10-05T07:38:51Z

Latest CRA findings: https://paste.googleplex.com/4900258908340224

…cron schedule, and lunch enum fix

…DME mode flag

…n place records

smarthg-gi

Review scope

Target: PR #2175 (ad9758f470f7d3cce4ba1374ed06f7d15e9a048e against 9546be84c1ca401cd500201f86844860e1cc021d)
Reviewed: All 38 changed files and associated GCS staging runs in gs://datcom-import-test/scripts/us_nces/demographics/
Skipped: None

Unanchored findings

[P1] Outdated `NCES_SchoolDistrictStats` & `NCES_PublicSchoolStats` staging runs and missing `LibrariansSpecialistsOrMediaSpecialist` schema definition

scripts/us_nces/common/replacement_functions.py / scripts/us_nces/demographics/school_district/test_data/sample_output/us_nces_demographics_district_school.mcf - Schema and staging sync
- Finding: Commit ad9758f4 maps "Librarians/media specialists" to "LibrariansSpecialistsOrMediaSpecialist", which generates Node: dcid:Count_Faculty_LibrariansSpecialistsOrMediaSpecialist (facultyType: dcs:LibrariansSpecialistsOrMediaSpecialist) in us_nces_demographics_district_school.mcf. However, LibrariansSpecialistsOrMediaSpecialist is not defined in edu.mcf (only MediaSupportStaff and SchoolOtherSupportServicesStaff are in the companion schema CL). In addition, after commit ad9758f4 (2026-10-05T00:53:46-07:00), only NCES_PublicSchool and NCES_SchoolDistrict were re-run in gs://datcom-import-test/; NCES_SchoolDistrictStats (2026_10_04T04_44_03_511494_07_00) and NCES_PublicSchoolStats (2026_10_04T04_58_27_031162_07_00) were not re-run.
- Impact: dcs:LibrariansSpecialistsOrMediaSpecialist is missing from edu.mcf, and the staging runs for NCES_SchoolDistrictStats and NCES_PublicSchoolStats do not reflect commit ad9758f4.
- Recommendation: Add Node: dcid:LibrariansSpecialistsOrMediaSpecialist (typeOf: dcs:FacultyTypeEnum) to edu.mcf in the companion schema CL, and re-run all 4 staging imports after addressing the findings in this review.

[P2] GCS validation links in PR description return 404 and `NCES_PublicSchoolStats` is missing `differ_summary.json`

PR #2175 description - Validation and differ artifacts
- Finding: All 4 gs://datcom-import-test/... paths in the PR description (2026_03_24T...) no longer exist in GCS and omit the /input0/validation/ subdirectory. Furthermore, in the latest NCES_PublicSchoolStats staging run (2026_10_04T04_58_27_031162_07_00/input0/validation/), differ_summary.json was not generated and check_deleted_records_percent was skipped in validation_output.csv.
- Impact: Reviewers cannot verify the validation outputs via the PR description links, and NCES_PublicSchoolStats (238,110,562 observations) has no differ comparison against the previous production baseline.
- Recommendation: Update the PR description with the latest gs://datcom-import-test/.../input0/validation/ paths after re-running all 4 jobs, and ensure NCES_PublicSchoolStats runs the differ against the previous production stats MCFs.

Positive findings

scripts/us_nces/common/us_education.py:153 - Dynamic ELSI footer detection in input_file_to_df() ✓
- Finding: Good - Inspecting trailing lines for NCES footer markers (Data Source, †, –, ‡) before slicing prevents truncating valid data rows in footerless sample CSVs.
scripts/us_nces/common/us_education.py:720 - Whitespace normalization on Physical_Address ✓
- Finding: Good - Normalizing internal whitespace and stripping leading/trailing spaces eliminates whitespace-only " " addresses when street/city/state/ZIP components are missing.
scripts/us_nces/demographics/public_school/manifest.json:23 - Staggered cron schedules for place and stats imports ✓
- Finding: Good - Scheduling the stats import 7 days after the place import (2nd vs 9th for public school; 3rd vs 10th for school district) ensures newly added school and district entities exist before observations are ingested.

Coverage

File	Status	Result
`scripts/us_nces/README.md`	Reviewed	One P2 finding
`scripts/us_nces/common/prop_conf.py`	Reviewed	One P2 finding
`scripts/us_nces/common/replacement_functions.py`	Reviewed	Two P1 findings, one P2 finding
`scripts/us_nces/common/us_education.py`	Reviewed	Two P1 findings
`scripts/us_nces/demographics/public_school/config.py`	Reviewed	One P2 finding
`scripts/us_nces/demographics/public_school/manifest.json`	Reviewed	No findings
`scripts/us_nces/demographics/public_school/process.py`	Reviewed	No findings
`scripts/us_nces/demographics/public_school/process_test.py`	Reviewed	One P2 finding
`scripts/us_nces/demographics/public_school/test_data/sample_input/ELSI_csv_export_*.csv` (10 files)	Reviewed	No findings
`scripts/us_nces/demographics/public_school/test_data/sample_output/us_nces_demographics_public_place.csv`	Reviewed	Affected by P1/P2 place & enum findings
`scripts/us_nces/demographics/public_school/test_data/sample_output/us_nces_demographics_public_school.csv`	Reviewed	No findings
`scripts/us_nces/demographics/public_school/validation_config.json`	Reviewed	No findings
`scripts/us_nces/demographics/school_district/config.py`	Reviewed	One P2 finding
`scripts/us_nces/demographics/school_district/manifest.json`	Reviewed	No findings
`scripts/us_nces/demographics/school_district/process.py`	Reviewed	One P2 finding (unsorted `os.listdir`)
`scripts/us_nces/demographics/school_district/process_test.py`	Reviewed	One P2 finding
`scripts/us_nces/demographics/school_district/test_data/sample_input/ELSI_csv_export_*.csv` (10 files)	Reviewed	One P1 finding (missing Chunk 1 & Chunk 8 in 2024-25 sample input)
`scripts/us_nces/demographics/school_district/test_data/sample_output/us_nces_demographics_district_place.csv`	Reviewed	One P1 finding (13/16 rows blank place metadata)
`scripts/us_nces/demographics/school_district/test_data/sample_output/us_nces_demographics_district_school.csv`	Reviewed	One P1 finding (missing teacher/faculty observations)
`scripts/us_nces/demographics/school_district/validation_config.json`	Reviewed	No findings

Verification and limitations

Checks run:
- Verified gh pr view 2175 -R datacommonsorg/data (headRefOid: ad9758f470f7d3cce4ba1374ed06f7d15e9a048e).
- Inspected all 38 changed files and diff hunks in a detached git worktree.
- Inspected staging outputs (import_summary.json, differ_summary.json, validation_output.csv, stat_var_schema.mcf, *.mcf, and full *_place.csv outputs) in gs://datcom-import-test/scripts/us_nces/demographics/.
Checks not run:
- Local unittest execution (python3 -m unittest discover -v -s scripts/us_nces/demographics/school_district -p '*_test.py') failed in the host environment with ModuleNotFoundError: No module named 'absl'.
Limitations: None.

smarthg-gi · 2026-10-06T03:25:19Z

Review scope

Target: PR Fix Pandas compatibility, DCID collisions, and validation config for NCES_SchoolDistrict and NCES_PublicSchool #2175 (ad9758f470f7d3cce4ba1374ed06f7d15e9a048e against 9546be84c1ca401cd500201f86844860e1cc021d)

Reviewed: All 38 changed files and associated GCS staging runs in gs://datcom-import-test/scripts/us_nces/demographics/

Skipped: None

Unanchored findings

[P1] Outdated NCES_SchoolDistrictStats & NCES_PublicSchoolStats staging runs and missing LibrariansSpecialistsOrMediaSpecialist schema definition

scripts/us_nces/common/replacement_functions.py / scripts/us_nces/demographics/school_district/test_data/sample_output/us_nces_demographics_district_school.mcf - Schema and staging sync

Finding: Commit ad9758f4 maps "Librarians/media specialists" to "LibrariansSpecialistsOrMediaSpecialist", which generates Node: dcid:Count_Faculty_LibrariansSpecialistsOrMediaSpecialist (facultyType: dcs:LibrariansSpecialistsOrMediaSpecialist) in us_nces_demographics_district_school.mcf. However, LibrariansSpecialistsOrMediaSpecialist is not defined in edu.mcf (only MediaSupportStaff and SchoolOtherSupportServicesStaff are in the companion schema CL). In addition, after commit ad9758f4 (2026-10-05T00:53:46-07:00), only NCES_PublicSchool and NCES_SchoolDistrict were re-run in gs://datcom-import-test/; NCES_SchoolDistrictStats (2026_10_04T04_44_03_511494_07_00) and NCES_PublicSchoolStats (2026_10_04T04_58_27_031162_07_00) were not re-run.

Impact: dcs:LibrariansSpecialistsOrMediaSpecialist is missing from edu.mcf, and the staging runs for NCES_SchoolDistrictStats and NCES_PublicSchoolStats do not reflect commit ad9758f4.

Recommendation: Add Node: dcid:LibrariansSpecialistsOrMediaSpecialist (typeOf: dcs:FacultyTypeEnum) to edu.mcf in the companion schema CL, and re-run all 4 staging imports after addressing the findings in this review.

[P2] GCS validation links in PR description return 404 and NCES_PublicSchoolStats is missing differ_summary.json

PR Fix Pandas compatibility, DCID collisions, and validation config for NCES_SchoolDistrict and NCES_PublicSchool #2175 description - Validation and differ artifacts

Finding: All 4 gs://datcom-import-test/... paths in the PR description (2026_03_24T...) no longer exist in GCS and omit the /input0/validation/ subdirectory. Furthermore, in the latest NCES_PublicSchoolStats staging run (2026_10_04T04_58_27_031162_07_00/input0/validation/), differ_summary.json was not generated and check_deleted_records_percent was skipped in validation_output.csv.

Impact: Reviewers cannot verify the validation outputs via the PR description links, and NCES_PublicSchoolStats (238,110,562 observations) has no differ comparison against the previous production baseline.

Recommendation: Update the PR description with the latest gs://datcom-import-test/.../input0/validation/ paths after re-running all 4 jobs, and ensure NCES_PublicSchoolStats runs the differ against the previous production stats MCFs.

Positive findings

scripts/us_nces/common/us_education.py:153 - Dynamic ELSI footer detection in input_file_to_df() ✓

Finding: Good - Inspecting trailing lines for NCES footer markers (Data Source, †, –, ‡) before slicing prevents truncating valid data rows in footerless sample CSVs.

scripts/us_nces/common/us_education.py:720 - Whitespace normalization on Physical_Address ✓

Finding: Good - Normalizing internal whitespace and stripping leading/trailing spaces eliminates whitespace-only " " addresses when street/city/state/ZIP components are missing.

scripts/us_nces/demographics/public_school/manifest.json:23 - Staggered cron schedules for place and stats imports ✓

Finding: Good - Scheduling the stats import 7 days after the place import (2nd vs 9th for public school; 3rd vs 10th for school district) ensures newly added school and district entities exist before observations are ingested.

Coverage

File Status Result
scripts/us_nces/README.md Reviewed One P2 finding
scripts/us_nces/common/prop_conf.py Reviewed One P2 finding
scripts/us_nces/common/replacement_functions.py Reviewed Two P1 findings, one P2 finding
scripts/us_nces/common/us_education.py Reviewed Two P1 findings
scripts/us_nces/demographics/public_school/config.py Reviewed One P2 finding
scripts/us_nces/demographics/public_school/manifest.json Reviewed No findings
scripts/us_nces/demographics/public_school/process.py Reviewed No findings
scripts/us_nces/demographics/public_school/process_test.py Reviewed One P2 finding
scripts/us_nces/demographics/public_school/test_data/sample_input/ELSI_csv_export_*.csv (10 files) Reviewed No findings
scripts/us_nces/demographics/public_school/test_data/sample_output/us_nces_demographics_public_place.csv Reviewed Affected by P1/P2 place & enum findings
scripts/us_nces/demographics/public_school/test_data/sample_output/us_nces_demographics_public_school.csv Reviewed No findings
scripts/us_nces/demographics/public_school/validation_config.json Reviewed No findings
scripts/us_nces/demographics/school_district/config.py Reviewed One P2 finding
scripts/us_nces/demographics/school_district/manifest.json Reviewed No findings
scripts/us_nces/demographics/school_district/process.py Reviewed One P2 finding (unsorted os.listdir)
scripts/us_nces/demographics/school_district/process_test.py Reviewed One P2 finding
scripts/us_nces/demographics/school_district/test_data/sample_input/ELSI_csv_export_*.csv (10 files) Reviewed One P1 finding (missing Chunk 1 & Chunk 8 in 2024-25 sample input)
scripts/us_nces/demographics/school_district/test_data/sample_output/us_nces_demographics_district_place.csv Reviewed One P1 finding (13/16 rows blank place metadata)
scripts/us_nces/demographics/school_district/test_data/sample_output/us_nces_demographics_district_school.csv Reviewed One P1 finding (missing teacher/faculty observations)
scripts/us_nces/demographics/school_district/validation_config.json Reviewed No findings

Verification and limitations

Checks run:

Verified gh pr view 2175 -R datacommonsorg/data (headRefOid: ad9758f470f7d3cce4ba1374ed06f7d15e9a048e).

Inspected all 38 changed files and diff hunks in a detached git worktree.

Inspected staging outputs (import_summary.json, differ_summary.json, validation_output.csv, stat_var_schema.mcf, *.mcf, and full *_place.csv outputs) in gs://datcom-import-test/scripts/us_nces/demographics/.

Checks not run:

Local unittest execution (python3 -m unittest discover -v -s scripts/us_nces/demographics/school_district -p '*_test.py') failed in the host environment with ModuleNotFoundError: No module named 'absl'.

Limitations: None.

[P1] Outdated NCES_SchoolDistrictStats & NCES_PublicSchoolStats staging runs and missing LibrariansSpecialistsOrMediaSpecialist schema definition
The schema definition has been added to the CL. Test runs will only be triggered once the code changes are made and complete to ensure correct comparison of the results.

[P2] GCS validation links in PR description return 404 and NCES_PublicSchoolStats is missing differ_summary.json
This is an incorrect comment. All the links in the PR description are fully functional

…patterns, and test coverage

smarthg-gi

Follow-up review comments on the latest updates (38ba5da7). Please see the inline comments below:

… deduplication

… validation

Updated code

0afe7b5

gemini-code-assist Bot reviewed Aug 23, 2026

View reviewed changes

smarthg-gi added 2 commits August 24, 2026 06:15

Replaced lambda with def _PV_FORMAT(pv)

a446adb

updated manifest with nodes.mcf

23cefa0

Adressed CRA comments

cf7777b

Made changes as per CRA review

c56f916

smarthg-gi commented Aug 24, 2026

View reviewed changes

Comment thread ...mographics/school_district/test_data/sample_input/ELSI_csv_export_6391562516627516691672.csv Outdated

Comment thread scripts/us_nces/demographics/school_district/validation_config.json

Comment thread scripts/us_nces/demographics/school_district/validation_config.json

smarthg-gi commented Aug 24, 2026

View reviewed changes

Comment thread scripts/us_nces/common/prop_conf.py

Updated test_data

bcdd014

smarthg-gi changed the title ~~Fix Pandas compatibility, DCID collisions, and validation config for NCES_SchoolDistrict~~ Fix Pandas compatibility, DCID collisions, and validation config for NCES_SchoolDistrict and NCES_PublicSchool Aug 30, 2026

smarthg-gi added 6 commits August 30, 2026 16:30

Adding files for PublicSchool

344e705

Merge branch 'master' into NCES_SchoolDistrict_fix

e3139ba

Fix Python formatting as per yapf style

6d4840e

Updated place merge logic

a2c33da

Resolve merge conflict: keep simplified multi-file place ingestion

132eccf

Fix Python formatting as per yapf style

a815f56

smarthg-gi commented Sep 2, 2026

View reviewed changes

Comment thread ...demographics/public_school/test_data/sample_input/ELSI_csv_export_6392166529530453281445.csv

Comment thread scripts/us_nces/common/us_education.py Outdated

updated test files

d2eae4a

smarthg-gi commented Sep 2, 2026

View reviewed changes

smarthg-gi added 7 commits September 2, 2026 09:24

Update provenance_url to https://nces.ed.gov/ in public_school and sc…

eed877d

…hool_district manifests

Merge branch 'datacommonsorg:master' into NCES_SchoolDistrict_fix

e40737b

updated files as per review comments

670cce9

Fix district ID padding and address review comments

9111c51

Fix review findings for error handling, test hermeticity, and manifes…

31bc976

…t configs

removing redundant files

de3ecf2

Regenerate test output files using generator scripts and align test m…

4f704c7

…ocks

smarthg-gi added 8 commits September 9, 2026 17:30

adding data freshness and max date consistency rules

b50e85e

use absl.app.run(main) in process.py

d968067

added validation rules for public school

46e77e2

Fix python formatting as per yapf style and trim extra newlines in va…

6604e27

…lidation_config.json

Used raw string literals for regex in us_education.py to resolve Pyth…

b48e4d4

…on 3.12 syntax warning

Fix duplicate function call in lambda and clean up unused imports

7cd4223

updated files with the split of imports

fd04f16

remove validation_config from place imports and fail fast on missing …

a7d9844

…inputs

smarthg-gi commented Oct 4, 2026

View reviewed changes

Comment thread scripts/us_nces/demographics/school_district/validation_config.json

smarthg-gi added 3 commits October 5, 2026 08:11

Resolve PR review comments: dynamic footer parsing, address cleanup, …

c17f846

…cron schedule, and lunch enum fix

Fix School_Type_Public educationalMethod mapping, dcs prefix, and REA…

c9b2301

…DME mode flag

fix cross-year attribute bleeding and preserve unreadable sentinels i…

ad9758f

…n place records

smarthg-gi commented Oct 6, 2026

View reviewed changes

smarthg-gi added 3 commits October 6, 2026 03:33

Address review feedback: place coalescing, sentinel mappings, config …

1c5b40c

…patterns, and test coverage

fix yapf formatting

58a5d76

fix yapf formatting

38ba5da

smarthg-gi commented Oct 6, 2026

View reviewed changes

smarthg-gi added 4 commits October 6, 2026 10:07

Resolve PR comments: fix place enum prefixes, sentinel filtering, and…

3477eae

… deduplication

Remove dcs prefix from School_Management to resolve missing reference…

776fffd

… validation

updated correct config

354f4aa

updated cron scheduls for timely execution

df7226f

Conversation

smarthg-gi commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Linked Issues & Reviews

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smarthg-gi commented Aug 24, 2026

[P1] Test Regression & Fixture Desynchronization

[P2] Import Validation Compliance

[P3] Code Hygiene and Formatting

Uh oh!

smarthg-gi commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smarthg-gi commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smarthg-gi commented Aug 24, 2026

[P1] Test Regression & Fixture Desynchronization

[P2] Import Validation Compliance

[P3] Code Hygiene and Formatting

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smarthg-gi left a comment

Choose a reason for hiding this comment

Review summary

Positive findings

Coverage

Verification and limitations

Uh oh!

Uh oh!

Uh oh!

smarthg-gi left a comment

Choose a reason for hiding this comment

Review summary

Findings

Positive findings

Coverage

Verification and limitations

Uh oh!

Uh oh!

smarthg-gi commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smarthg-gi left a comment

Choose a reason for hiding this comment

Review scope

Unanchored findings

[P1] Outdated NCES_SchoolDistrictStats & NCES_PublicSchoolStats staging runs and missing LibrariansSpecialistsOrMediaSpecialist schema definition

[P2] GCS validation links in PR description return 404 and NCES_PublicSchoolStats is missing differ_summary.json

Positive findings

Coverage

Verification and limitations

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smarthg-gi commented Oct 6, 2026

Review scope

Unanchored findings

[P1] Outdated NCES_SchoolDistrictStats & NCES_PublicSchoolStats staging runs and missing LibrariansSpecialistsOrMediaSpecialist schema definition

smarthg-gi commented Aug 23, 2026 •

edited

Loading

smarthg-gi commented Aug 24, 2026 •

edited

Loading

smarthg-gi commented Aug 24, 2026 •

edited

Loading

smarthg-gi commented Oct 5, 2026 •

edited

Loading

[P1] Outdated `NCES_SchoolDistrictStats` & `NCES_PublicSchoolStats` staging runs and missing `LibrariansSpecialistsOrMediaSpecialist` schema definition

[P2] GCS validation links in PR description return 404 and `NCES_PublicSchoolStats` is missing `differ_summary.json`

[P1] Outdated `NCES_SchoolDistrictStats` & `NCES_PublicSchoolStats` staging runs and missing `LibrariansSpecialistsOrMediaSpecialist` schema definition

[P2] GCS validation links in PR description return 404 and `NCES_PublicSchoolStats` is missing `differ_summary.json`