Fix Pandas compatibility, DCID collisions, and validation config for NCES_SchoolDistrict and NCES_PublicSchool

[smarthg-gi]

Import: NCES_SchoolDistrict
Test job run: nces-schooldistrict-smarthg-20260901-091725
Stats:
validation_output.csv
differ_summary.json
Places:
validation_output.csv
differ_summary.json

Import: NCES_PublicSchool
Test job run: nces-publicschool-smarthg-20260901-162007
Stats:
validation_output.csv
differ_summary.json
Places:
validation_output.csv
differ_summary.json

Summary of Changes

• 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 grade-level StatVars in Pandas 3.0+.
• DCID Collision Prevention: Added word boundaries to _GENDER regex and configured "Total Staff" population type to avoid duplicate StatVar collisions.
• Dynamic DC API Root: Removed hardcoded 'dc_api_root': None to dynamically inherit os.environ['DC_API_ROOT'].
• Validation: Configured validation_config.json with historical deletion threshold.
• Multi-File Place Merging: Used pd.concat() and .groupby("school_state_code").first() to coalesce place attributes across multi-year files without dropping places.
• Provenance URL Alignment: Updated provenance_url to https://nces.ed.gov/ in both public_school/manifest.json and school_district/manifest.json to align with the SOPs.

Linked Issues & Reviews

• Bugs: b/530491127 (NCES_SchoolDistrictStats), b/530486992 (NCES_PublicSchoolStats)
• 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]

[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]

[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]

[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]

[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:

1. 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.
2. 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.
3. 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).