feat(osm-equity): MappingEquityAtlas FFL — fan out over cities, one map#31
Conversation
A single parameterised FFL workflow that turns the study into: resolve a
sequence of regions + a min-population ("city size") floor into cities, run
each city in parallel (andThen foreach — fan-out by subregion), tile each at a
chosen size, and MERGE every tile into ONE combined map.
FFL (osm_equity.ffl): schema CityRef; event facets ResolveCities / CityTiles /
BuildAtlasMap; fan-out facet CollectCityTiles (foreach city -> CityTiles ->
aggregate records); workflow MappingEquityAtlas with input params regions,
min_population, tile_sqmi, acs_year, osm_kinds, with_footprints, metric,
half_deg, update, title. Validates clean under relative scoping.
Handlers (handlers/atlas/): ResolveCities (curated US city table filtered by
population + region), CityTiles (the fan-out unit — real: TIGER+ACS areal-
interpolated onto tiles, OSM by kinds, optional MS footprints; offline:
deterministic synthetic tiles), BuildAtlasMap (one combined MapLibre map by
`metric`). 2 offline atlas tests (fan-out + combine, population floor); 18
offline tests pass; ruff clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces the MappingEquityAtlas workflow, which fans out over multiple cities in parallel, tiles each city, and aggregates the results into a single combined MapLibre map. Feedback on the changes highlights a cache collision risk in handle_city_tiles when using different osm_kinds for the same city, a resource leak due to un-deleted temporary clipped GeoJSON files, and potential runtime crashes in parameter parsing if parameters are explicitly passed as None.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| osm_path = U.out_dir() / f"atlas_osm_{slug}.geojson" | ||
| if not osm_path.exists(): | ||
| feats, _snap = R.real_fetch_region_osm(bbox) | ||
| osm_path.write_text(json.dumps({"type": "FeatureCollection", "features": feats})) |
There was a problem hiding this comment.
The cached OSM file path (atlas_osm_{slug}.geojson) is determined solely by the city's slug name. However, the fetched OSM features depend on the osm_kinds parameter (e.g., "poi" vs. "building,road,poi"). If the workflow is run first with one set of osm_kinds and then with another for the same city, the second run will incorrectly reuse the cached file from the first run, leading to incomplete or incorrect analysis.
To fix this cache collision bug, include a hash of the osm_kinds parameter in the cached filename.
| osm_path = U.out_dir() / f"atlas_osm_{slug}.geojson" | |
| if not osm_path.exists(): | |
| feats, _snap = R.real_fetch_region_osm(bbox) | |
| osm_path.write_text(json.dumps({"type": "FeatureCollection", "features": feats})) | |
| kinds_hash = hashlib.sha256(osm_kinds.encode()).hexdigest()[:8] | |
| osm_path = U.out_dir() / f"atlas_osm_{slug}_{kinds_hash}.geojson" | |
| if not osm_path.exists(): | |
| feats, _snap = R.real_fetch_region_osm(bbox) | |
| osm_path.write_text(json.dumps({"type": "FeatureCollection", "features": feats})) |
| clip = U.clip_tract_osm(str(osm_path), tile["geometry_wkt"]) | ||
| intr = U.compute_attribute_quality(clip, tile["area_km2"], tile["geoid"]) | ||
| extr = U.compute_extrinsic_quality( | ||
| clip, tile["geometry_wkt"], fp_path, fp_ok, tile["geoid"] | ||
| ) |
There was a problem hiding this comment.
For each tile in the loop, U.clip_tract_osm writes a temporary GeoJSON file to disk. These files are never cleaned up, which can quickly lead to disk space or inode exhaustion when running the workflow over many cities and tiles.
To prevent this, wrap the quality computation in a try...finally block and delete the temporary clip file once it is no longer needed.
clip = U.clip_tract_osm(str(osm_path), tile["geometry_wkt"])
try:
intr = U.compute_attribute_quality(clip, tile["area_km2"], tile["geoid"])
extr = U.compute_extrinsic_quality(
clip, tile["geometry_wkt"], fp_path, fp_ok, tile["geoid"]
)
finally:
try:
os.remove(clip)
except OSError:
pass| regions = [r.strip().lower() for r in _d(params.get("regions", ["north-america"]))] | ||
| min_pop = int(params.get("min_population", 500000)) | ||
| half = float(params.get("half_deg", 0.12)) |
There was a problem hiding this comment.
Using params.get(key, default) can return None if the key is explicitly present in params but its value is None. This will bypass the default values and cause runtime crashes (e.g., TypeError when calling _d(None) or float(None)).
To make the parameter parsing robust and defensive, explicitly check for None before falling back to the default values.
raw_regions = params.get("regions")
regions = [r.strip().lower() for r in _d(raw_regions if raw_regions is not None else ["north-america"])]
raw_min_pop = params.get("min_population")
min_pop = int(raw_min_pop if raw_min_pop is not None else 500000)
raw_half = params.get("half_deg")
half = float(raw_half if raw_half is not None else 0.12)
A single parameterised FFL workflow: resolve a sequence of regions + a city-size floor (min population) into cities, run each city in parallel (
andThen foreach— fan-out by subregion), tile each at a chosen tile size, and merge every tile into ONE map.FFL (
osm_equity.ffl):schema CityRef; event facetsResolveCities/CityTiles/BuildAtlasMap; fan-out facetCollectCityTiles(foreach city → CityTiles → aggregate); workflowMappingEquityAtlaswith paramsregions, min_population, tile_sqmi, acs_year, osm_kinds, with_footprints, metric, half_deg, update, title. Validates clean.Handlers (
handlers/atlas/): ResolveCities (city table filtered by population + region), CityTiles (fan-out unit — real TIGER/ACS/OSM/footprints or offline synthetic), BuildAtlasMap (one MapLibre map bymetric). 2 offline atlas tests; 18 offline tests pass; ruff clean.🤖 Generated with Claude Code