Add Bangladesh city corporation and sub-district admin boundaries - #33
Conversation
…mpling create_replacement_pixels could silently lose a candidate to a concurrent claim (INSERT ... ON CONFLICT ... DO UPDATE ... WHERE no-op) while still reporting success; it now retries remaining candidates and only counts an actual claim. Building-target rounds previously had no replacement-pixel logic at all - a new activity anchors each sampled building's quadkey to a coverage_pixel row so the existing neighbor-selection logic can be reused. Both round-creation UIs get an explicit "generate replacement pixels" toggle and hints for the building/population count check, and the map legend now explains the primary/replacement pixel colors.
…ies spec Verified geopandas (already a dependency) reads .shp natively, so no new pyshp dependency is needed. Also found the map-click add-area flow is backed by separate static PMTiles infrastructure, not the admin_boundaries table this spec extends - documented as out of scope, with the drill-down picker as the actual extension point.
Union names are only unique within an upazila, not district-wide, so grouping solely by UNINAME could merge two different upazilas' same-named unions into one group and attach one upazila's wards/blocks under the wrong upazila's matched union row. Group by (THANAME, UNINAME) instead, and key the union_id_cache the same way.
…rd under existing district rows)
TestImportCityCorporation relied on the real district name 'Dhaka', which now exists in the shared dev database after the CLI import ran against real shapefiles. import_city_corporation's idempotency check found the existing DNCC row and returned 0 created instead of 1, breaking test_creates_city_corporation_zones_and_wards. Add a district_fixture that inserts a fresh, uniquely-named district per test (mirroring TestImportRuralDistrict's union_fixture), and point synthetic_dncc_gdf's DISTNAME at it so these tests no longer depend on real seeded data existing or not existing in the database.
Districts can have both a pcode-reachable upazila set and a parent_id-linked city corporation (e.g. Chittagong, Dhaka, Gazipur, Khulna, Narayanganj, Rangpur, Sylhet) - these are siblings, not alternatives. The children endpoint returned early when parent_id children existed, silently dropping the real upazilas for those districts. Now both lookups always run and their results are merged. Also fixes a latent bug in the pcode fallback: the parent's own pcode was picked as the first non-null adm*_pcode column rather than the one matching its actual level, so districts with populated ancestor codes (all real imported districts) resolved the wrong pcode and silently returned zero pcode-based children. Replaced the fragile dash-counting UUID heuristic with an actual uuid.UUID() check, and dropped the unused boundary_type column from the parent_id children query.
…ssage The merge test's district fixture only had adm2_pcode set, which happened to produce the same result under both the old "first non-null pcode column" heuristic and the fixed parent_row[1 + parent_level] lookup, so it couldn't catch a regression back to the old heuristic. Updated the fixture to populate adm0/adm1/adm2_pcode together, matching real district rows, and verified (via a temporary local revert) that the old heuristic now fails this test. Also restored the 'No child level exists' message for level-4 boundaries with neither parent_id nor pcode-based children, which was silently dropped when the parent_id and pcode lookups were merged.
The existing tests only replayed the by-id and by-pcode SQL by hand, never calling add_campaign_area itself, so a typo in its if/else branching (checking the wrong variable, reversed branches) would pass unnoticed. Add tests that call add_campaign_area.__wrapped__ directly with a real Flask request context, a committed campaign, and a real admin boundary with no pcode (mirroring city corporations), verifying both the admin_boundary_id and pcode paths create the correct campaign_areas row. Confirmed the new tests fail when the branch condition is broken.
…rporation/zone/ward/block
Add optional boundary_ids parameter to populate_pixels_for_leaf_boundaries and roll_up_pixels_to_ancestors so callers can target specific rows instead of always processing every matching row in admin_boundaries. Defaults to None (process everything), preserving existing full-run behavior. Scope test calls to their own synthetic boundary ids so tests don't trigger a spatial join against the ~21,000+ real committed leaf boundaries and the full pixels table already present in the shared database. Also fix a test assertion that assumed a single spatial-join match; real committed pixel data in the same region means multiple rows can legitimately intersect, so check membership instead of exact equality.
…ed as campaign areas
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d65c150d14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """, (admin_boundary_pcode, admin_boundary_pcode, admin_boundary_pcode, | ||
| admin_boundary_pcode, admin_boundary_pcode)) | ||
| # Get geometry, name, id, and pre-computed stats from admin_boundaries table by id or pcode | ||
| if admin_boundary_id_param: |
There was a problem hiding this comment.
When admin_boundary_id_param is used for the new city-corporation/zone/ward/block rows, those imported rows do not have adm*_pcode values, but they can now be saved as campaign areas. The pixel-sampling workflow still calls sample_pixels_for_campaign_area, which returns No admin pcode found for campaign area before sampling from pixel_area, so an area added through the new drill-down path cannot be adaptively pixel-sampled even after its pixels are populated.
Useful? React with 👍 / 👎.
| quadkey, campaign_id, indicator_id, version, n_trials, n_covered, rounds | ||
| ) | ||
| VALUES (%s, %s, %s, 0, 0, 0, ARRAY[%s]) | ||
| ON CONFLICT (quadkey, indicator_id, campaign_id) DO UPDATE SET |
There was a problem hiding this comment.
For building-target sampling with replacements enabled, this new upsert runs before replacement pixels are created. The checked schema defines the coverage-pixel uniqueness with version included (UNIQUE (quadkey, indicator_id, campaign_id, version) in migrations_campaign_refactor.py), so PostgreSQL rejects ON CONFLICT (quadkey, indicator_id, campaign_id) with no matching unique constraint, causing the default replacement-enabled building sampling path to fail.
Useful? React with 👍 / 👎.
| if generate_replacements: | ||
| self.status = "creating_replacements" | ||
| primary_pixel_ids = await workflow.execute_activity( | ||
| ensure_coverage_pixels_for_locations, | ||
| args=[campaign_id, indicator_id, selected_ids, self.round_number], |
There was a problem hiding this comment.
When generate_replacements is true for building-target sampling, the workflow now creates coverage_pixel rows for the selected buildings' quadkeys and replacement rows, but the resample branch above only calls clear_round_from_buildings. Resampling a building round therefore leaves the old pixel round assignments/replacements behind, so maps and later replacement selection can still treat the previous building pixels as sampled even after the building sample has been replaced.
Useful? React with 👍 / 👎.
1 participant
Summary
admin_boundarieswith an additive, self-referential tree (parent_id/boundary_type/source_code) so city corporations, zones, wards, and rural blocks can attach as children of existing division/district/upazila/union rows without touching any existing column, row, or query path.data/new/(21 districts, 8 city corporations), resolving parents by name with a small alias table and a spatial overlap sanity check.get_admin_boundary_children,add_campaign_area, andlist_campaign_areasto work with the new boundary types alongside the legacy pcode-based ones.AdminBoundaryDrillPicker) so a city corporation (or any new boundary type) can be picked directly as a campaign area's starting point, alongside the existing map-click flow.Design doc:
docs/superpowers/specs/2026-07-03-bd-city-corporation-boundaries-design.mdImplementation plan:
docs/superpowers/plans/2026-07-03-bd-city-corporation-boundaries.mdExplicitly out of scope (documented, not touched): Stratified Cluster Sampling generalization for city corporations (separate follow-up plan), regenerating the static PMTiles map-click layer,
District.json.Test plan
data/new/(21 districts, 8 city corporations) into local dev DBnpx tsc --noEmitclean across the frontendscripts/populate_boundary_pixels_for_new_levels.pyagainst production data before relying onadmin_boundary_pixelsfor the new levels (pixel computation for campaign areas already works without this - it recomputes directly from geometry)