feat: HarP-derived lake classification corrections (7,425 reaches)

[jameshgrn]

Summary

• Add scripts/maintenance/apply_harp_lake_corrections.py to correct lakeflag using HarP v1.1 (Sikder et al. 2024)
• 7,425 reaches corrected from lakeflag=0 to lakeflag=1, 200,201 nodes propagated
• Corrections tagged edit_flag='harp_lake'
• Fix FK constraint bug in load_from_duckdb.py (delete child tables before parents)

Data sources

• HarP v1.1 Intersected SWORD-PLD dataset (Zenodo) — identifies SWORD reaches geometrically related to PLD lake polygons
• UNC v16-to-v17b translation — maps HarP's v16 reach IDs to v17b/v17c IDs

Correction tiers

Tier	HarP Reach_type	Count	Description
T1	In-lake	3,335	Reach inside PLD lake polygon
T2	Through-lake	397	Lake runs entirely through reach
T3	Inflow/Outflow	3,693	Reach is both lake inlet and outlet

Exclusions

• type=4 (dam), type=5 (unreliable), type=6 (ghost) skipped
• Only reaches currently lakeflag=0 corrected
• 99.8% of HarP v16 reaches matched via translation (579 v16-only, no v17b equivalent)

Validation

• 495/496 existing lake_sandwich corrections confirmed by HarP (independent agreement)
• Before: lakeflag=0: 203,007 / lakeflag=1: 26,854
• After: lakeflag=0: 195,582 / lakeflag=1: 34,279

Flow verification rerun

Ran flow verification on all 6 regions after lakeflag corrections:

Region	Approved	Rejected	Low	Skipped
NA	0	17	52	733
SA	0	11	204	514
EU	0	4	53	384
AF	0	3	49	206
AS	0	18	308	1,004
OC	1	0	17	310

1 OC section (section 149: reaches 51180200781, 51180200101) approved and applied:

• All 4 signals agreed (HIGH tier, multi_signal_agreement)
• WSE was going uphill in current direction (13.8 → 17.8 → 19.2m)
• Original UNC error (topology identical in v17b)
• Derived attributes rebuilt for OC after flip
• run_id: f8c9b69f00e9, backup: reach_topology_backup_OC_f8c9b69f00e9

Exports synced

• DuckDB: corrections written directly
• GeoParquet: all 6 regions exported (38s)
• GeoPackage: all 6 regions exported (22min)
• PostgreSQL: all 6 regions loaded (FK delete-order bug fixed in this PR)

Test plan

• Dry run produces identical candidate count
• Post-apply lakeflag delta = exactly 7,425
• All corrected reaches tagged harp_lake
• Node propagation verified (200,201 nodes)
• Flow verification dry run on all regions — no unsafe flips
• OC section 149 flip investigated: 4/4 signals, WSE confirmed downhill after fix
• All exports regenerated and verified

[jameshgrn]

PR Review: Harp Lake Corrections & FK Load Fix

Summary

The PR introduces a new script for applying lake corrections from the HarP dataset and fixes a foreign key dependency issue in the DuckDB-to-PostgreSQL loader. While the architectural approach is sound and follows established patterns (RTREE index handling, provenance tagging), there is a critical runtime error in the new script and a major resilience issue regarding spatial index restoration.

---

Finding 1: Critical Runtime Error in (P1)

In the function, the code attempts to fetch a result from an [�[90m20:43:53�[39m] using file �[32m~/Users/jakegearon/.npm-global/lib/node_modules/update/lib/updatefile.js�[39m
[�[90m20:43:53�[39m] starting �[1m�[36mupdate.default�[39m�[22m �[31m�[39m
[�[90m20:43:53�[39m] �[32m✔�[39m �[32mrunning:�[39m �[1m�[34minit�[39m�[22m

Current updaters: �[36minit�[39m

no updaters were saved.
[�[90m20:43:53�[39m] finished �[1m�[36mupdate.default�[39m�[22m �[32m✔�[39m �[31m49ms�[39m statement:

nodes_updated = conn.execute("""UPDATE nodes ...""").fetchone()[0]

Issue: DuckDB's UPDATE statement does not return rows by default. Calling .fetchone() on the result of an UPDATE (which returns a connection or relation object without rows) will raise an error or return None, causing the script to crash when indexing [0].
Fix: Use conn.execute(...).rowcount to get the number of affected rows, or add a RETURNING * clause if the DuckDB version supports it and you need specific row data.

Finding 2: Missing for RTREE Index Restoration (P2)

The script correctly identifies and drops RTREE indexes before performing updates to avoid DuckDB segfaults. However, it lacks error handling around the update logic.
Issue: If any of the UPDATE statements fail (e.g., due to the P1 issue above, disk space, or lock contention), the script will exit before reaching the index recreation loop. This leaves the database in a broken state for spatial queries.
Fix: Wrap the UPDATE operations in a try...finally block to ensure RTREE indexes are always recreated, following the pattern used in src/sword_v17c_pipeline/stages/output.py.

Finding 3: Inconsistent Reach Type Skip Logic (P3)

The SKIP_TYPES constant is defined as {4, 6} (dam, ghost).
Issue: The filter_against_db function explicitly skips type=5 (unreliable) as well, but this is hardcoded in the function rather than being part of the constant.
Fix: Add 5 to the SKIP_TYPES set for better maintainability and consistency.

Finding 4: FK Delete Ordering in (P3)

Observation: The change to reverse the order of truncation/deletion in load_from_duckdb.py correctly addresses foreign key dependency issues when loading into PostgreSQL. This is a solid improvement.

Finding 5: Potential Duplication (P4)

Issue: The UPDATE logic for edit_flag appends ,harp_lake regardless of whether the flag is already present. While not functionally broken, it can lead to redundant tags (e.g., harp_lake,harp_lake) if the script is run multiple times.
Fix: Use a CASE statement or regex to check if harp_lake is already in the string before appending.

---

Recommendation

Fix the P1 and P2 issues before merging to avoid database corruption or runtime crashes. The FK ordering fix in load_from_duckdb.py is approved.

[jameshgrn]

PR Review: Harp Lake Corrections & FK Load Fix

Summary

The PR introduces a new script for applying lake corrections from the HarP dataset and fixes a foreign key dependency issue in the DuckDB-to-PostgreSQL loader. While the architectural approach is sound and follows established patterns (RTREE index handling, provenance tagging), there is a critical runtime error in the new script and a major resilience issue regarding spatial index restoration.

---

Finding 1: Critical Runtime Error in apply_harp_lake_corrections.py (P1)

In the apply_corrections function, the code attempts to fetch a result from an UPDATE statement:

nodes_updated = conn.execute("UPDATE nodes ...").fetchone()[0]

Issue: DuckDB's UPDATE statement does not return rows by default. Calling .fetchone() on the result of an UPDATE (which returns a connection or relation object without rows) will raise an error or return None, causing the script to crash when indexing [0].
Fix: Use conn.execute(...).rowcount to get the number of affected rows, or add a RETURNING * clause if the DuckDB version supports it and you need specific row data.

Finding 2: Missing try...finally for RTREE Index Restoration (P2)

The script correctly identifies and drops RTREE indexes before performing updates to avoid DuckDB segfaults. However, it lacks error handling around the update logic.
Issue: If any of the UPDATE statements fail (e.g., due to the P1 issue above, disk space, or lock contention), the script will exit before reaching the index recreation loop. This leaves the database in a broken state for spatial queries.
Fix: Wrap the UPDATE operations in a try...finally block to ensure RTREE indexes are always recreated, following the pattern used in src/sword_v17c_pipeline/stages/output.py.

Finding 3: Inconsistent Reach Type Skip Logic (P3)

The SKIP_TYPES constant is defined as {4, 6} (dam, ghost).
Issue: The filter_against_db function explicitly skips type=5 (unreliable) as well, but this is hardcoded in the function rather than being part of the constant.
Fix: Add 5 to the SKIP_TYPES set for better maintainability and consistency.

Finding 4: FK Delete Ordering in load_from_duckdb.py (P3)

Observation: The change to reverse the order of truncation/deletion in load_from_duckdb.py correctly addresses foreign key dependency issues when loading into PostgreSQL. This is a solid improvement.

Finding 5: Potential edit_flag Duplication (P4)

Issue: The UPDATE logic for edit_flag appends ,harp_lake regardless of whether the flag is already present. While not functionally broken, it can lead to redundant tags (e.g., harp_lake,harp_lake) if the script is run multiple times.
Fix: Use a CASE statement or regex to check if harp_lake is already in the string before appending.

---

Recommendation

Fix the P1 and P2 issues before merging to avoid database corruption or runtime crashes. The FK ordering fix in load_from_duckdb.py is approved.

[jameshgrn]

Review response

Gemini review produced 1 P1, 1 P2, 2 P3, 1 P4.

Fixed

• P2 — RTREE try/finally: Wrapped UPDATE operations in try/finally so RTREE indexes are always recreated even on failure. Valid finding.
• Lint cleanup: Removed unused imports (json, os, datetime, timezone), fixed ambiguous variable name (l → lyr), removed f-strings without placeholders.

Dismissed

• P1 — UPDATE...fetchone()[0] crash: False positive. DuckDB's execute('UPDATE ...') returns the affected row count via .fetchone() — this is the standard DuckDB pattern. Verified in a test.
• P3 — SKIP_TYPES inconsistency: type=5 is intentionally handled separately in the filter function to produce distinct skip stats (skip_type5 counter). Adding it to the constant would lose that granularity.
• P4 — edit_flag duplication on re-run: The script is idempotent — re-runs skip reaches where lakeflag != 0, so the edit_flag append never fires twice for the same reach.

Quality pipeline

• ruff check: passes (both files)
• ruff format: passes
• Pre-existing geom_idx unused variable in load_from_duckdb.py not in scope of this PR