Update from task f7a8005b-7feb-4793-a187-ec21725bd5e9

[tronpis]

This PR was created by qwen-chat coder for task f7a8005b-7feb-4793-a187-ec21725bd5e9.

[qodo-code-review]

Review Summary by Qodo

Multi-collection Overture Maps support and elevation merge strategy

✨ Enhancement

Walkthroughs

Description

• Generalized Overture Maps fetching to support multiple collections (buildings, roads, water, land
  use) with new OvertureCollection enum
• Implemented WKB parsing for LineString geometry to support road data extraction
• Enhanced elevation fetching with merge strategy combining regional high-res providers and AWS
  fallback for gap coverage
• Added Copernicus DEM GLO-30 global elevation provider as alternative to AWS Terrarium
• Improved deduplication logic to handle different Overture feature types against existing OSM data

Diagram

flowchart LR
  A["Overture Maps<br/>Multi-Collection"] -->|Buildings, Roads,<br/>Water, LandUse| B["OvertureCollection<br/>Enum"]
  B -->|Parse WKB| C["Feature Parsers<br/>LineString/Polygon"]
  C -->|Deduplicate| D["OSM Spatial Grid<br/>Deduplication"]
  D -->|ProcessedElements| E["Merged Dataset"]
  
  F["Regional Provider"] -->|High-res Data| G["Elevation Merge<br/>Strategy"]
  H["AWS Terrain"] -->|Fallback Data| G
  I["Copernicus DEM<br/>GLO-30"] -->|Global Coverage| G
  G -->|Gap-filled Grid| J["Final Elevation"]

Loading

File Changes

1. src/elevation/mod.rs ✨ Enhancement +73/-37

Elevation merge strategy with gap filling

• Refactored elevation fetching to use merge strategy combining regional provider with AWS fallback
• Added merge_elevation_grids() function to fill NaN gaps in primary elevation data with secondary
 source
• Improved progress reporting with detailed coverage statistics and gap-fill counts
• Changed variable naming from provider to regional_provider for clarity

src/elevation/mod.rs

---

2. src/elevation/providers/copernicus.rs ✨ Enhancement +225/-0

New Copernicus DEM GLO-30 global elevation provider

• New Copernicus DEM GLO-30 provider implementation for global 30m elevation coverage
• Supports 1°×1° GeoTIFF tile downloads from AWS S3 with caching
• Implements tile-based sampling with nearest-neighbor interpolation to output grid
• Handles missing tiles (ocean areas) gracefully by returning NaN values
• Includes comprehensive tests for provider metadata and resolution

src/elevation/providers/copernicus.rs

---

3. src/elevation/providers/mod.rs ⚙️ Configuration changes +1/-0

Export Copernicus provider module

• Added public module export for new copernicus provider

src/elevation/providers/mod.rs

---

View more (3)

4. src/elevation/selector.rs ✨ Enhancement +6/-4

Add Copernicus to elevation provider selection

• Integrated Copernicus DEM GLO-30 into provider selection list with 30m resolution
• Positioned after regional providers but before AWS fallback for better global coverage
• Updated comments to clarify provider ordering by resolution

src/elevation/selector.rs

---

5. src/main.rs ✨ Enhancement +14/-5

Update main pipeline for multi-collection Overture fetching

• Updated Overture Maps integration to use new multi-collection fetching API
• Changed from fetch_overture_buildings() to fetch_overture_features() with collection list
• Now fetches buildings, roads, water, and land use features in single call
• Updated user-facing messages to reflect multi-feature support

src/main.rs

---

6. src/overture.rs ✨ Enhancement +1490/-140

Generalized Overture Maps multi-collection support with WKB parsing

• Added OvertureCollection enum supporting Building, Road, Water, and LandUse variants
• Implemented new data structures for OvertureRoad, OvertureWater, and OvertureLandUse
• Added WKB LineString parser (parse_wkb_linestring()) for road geometry extraction
• Generalized list_partition_files() to filter by multiple collections instead of just buildings
• Refactored deduplication logic with spatial grids for each feature type (buildings, roads, water,
 landuse)
• Added proximity-based road deduplication using ways_are_close() function
• Implemented feature-type-specific conversion functions (road_to_processed_way(),
 water_to_processed_way(), landuse_to_processed_way())
• Added mapping functions for Overture-to-OSM tag conversion (highway, surface, water, landuse)
• Maintained backward compatibility with old fetch_overture_buildings() API
• Added comprehensive unit tests for WKB parsing, tag mapping, and spatial grid operations

src/overture.rs

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Copernicus never selected 🐞 Bug ≡ Correctness

Description

CopernicusDem30 returns coverage_bboxes() -> None, but select_provider() only considers
providers with Some(coverages), so Copernicus is never chosen and AWS remains the only global
fallback. This makes the new provider effectively dead code and the intended behavior change does
not occur.

Code

src/elevation/providers/copernicus.rs[R26-28]

+    fn coverage_bboxes(&self) -> Option<Vec<LLBBox>> {
+        None // Global coverage
+    }

Evidence

The ElevationProvider contract documents coverage_bboxes() == None as "global fallback providers";
however select_provider() ignores all None coverage providers by only checking `if let
Some(coverages) = .... Since Copernicus returns None`, it is skipped and the selector falls
through to the AWS fallback.

src/elevation/providers/copernicus.rs[21-28]
src/elevation/selector.rs[22-41]
src/elevation/provider.rs[18-23]
src/elevation/selector.rs[43-55]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CopernicusDem30` is added to the provider list but will never be selected because `select_provider()` ignores providers whose `coverage_bboxes()` returns `None`.
### Issue Context
The `ElevationProvider` trait explicitly uses `coverage_bboxes() == None` to represent global providers (e.g., AWS Terrain Tiles). The selector currently only checks overlap for `Some(coverages)` and otherwise skips.
### Fix Focus Areas
- src/elevation/selector.rs[22-41]
- src/elevation/providers/copernicus.rs[21-28]
- src/elevation/provider.rs[18-23]
### Suggested fix
Update `select_provider()` to treat `coverage_bboxes() == None` as "global coverage" candidates when no regional provider matches. For example:
- Iterate candidates; return first overlapping regional provider.
- Otherwise, if any candidate has `coverage_bboxes() == None`, return the best global candidate (e.g., lowest `native_resolution_m()` or first in list).
- Only if none exist, fall back to AWS.
(Alternatively, if you want Copernicus treated as regional-with-global-extent, return `Some(vec![global_bbox])`, but that contradicts the trait’s documented meaning of `None`.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Undefined scale in partition 🐞 Bug ≡ Correctness

Description

process_partition_file_by_collection() uses scale when clipping elements, but scale is not in
scope and not passed as a parameter, so the code does not compile. This blocks the new Overture
multi-collection fetch path used by main.rs.

Code

src/overture.rs[R663-666]

+    // Clip elements to bbox
+    let (coord_transformer, xzbbox) = CoordTransformer::llbbox_to_xzbbox(bbox, scale)?;
+
+    let clipped_elements: Vec<ProcessedElement> = elements

Evidence

The function signature of process_partition_file_by_collection does not include scale, but the
body calls CoordTransformer::llbbox_to_xzbbox(bbox, scale)?;, referencing an undefined identifier.
The call site also does not pass scale.

src/overture.rs[560-566]
src/overture.rs[663-666]
src/overture.rs[526-531]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`process_partition_file_by_collection()` references `scale` but does not accept it as a parameter, so the Overture pipeline will not compile.
### Issue Context
`scale` is available in `fetch_overture_features_inner()` and must be threaded into partition processing where clipping/coord transforms occur.
### Fix Focus Areas
- src/overture.rs[494-547]
- src/overture.rs[560-683]
### Suggested fix
- Add `scale: f64` parameter to `process_partition_file_by_collection(...)`.
- Pass `scale` from the caller in `fetch_overture_features_inner()`.
- Consider computing `CoordTransformer::llbbox_to_xzbbox(bbox, scale)` once per overall fetch (not per partition) and reusing it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Nonexistent transformer/bbox APIs 🐞 Bug ≡ Correctness

Description

parse_row_group_by_collection() calls CoordTransformer::identity() and
LLBBox::new_unchecked(...), but neither function exists in the codebase, so the new Overture
parsing path does not compile. This prevents any of the new collection types
(building/road/water/landuse) from being converted into ProcessedWays.

Code

src/overture.rs[R1138-1139]

+                    building_to_processed_way(&b, &CoordTransformer::identity(), &LLBBox::new_unchecked(target_min_lat, target_min_lng, target_max_lat, target_max_lng))
+                })

Evidence

CoordTransformer only exposes llbbox_to_xzbbox(...) and transform_point(...) (no
identity()), and LLBBox only has new(...) / from_str(...) (no new_unchecked(...)). The new
multi-collection row-group parser uses these missing APIs in multiple match arms.

src/overture.rs[1102-1216]
src/coordinate_system/transformation.rs[14-60]
src/coordinate_system/geographic/llbbox.rs[13-60]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`parse_row_group_by_collection()` uses `CoordTransformer::identity()` and `LLBBox::new_unchecked(...)`, which do not exist, causing compilation failure.
### Issue Context
The conversion helpers (`building_to_processed_way`, `road_to_processed_way`, etc.) expect a real `CoordTransformer` configured for the requested `bbox` + `scale`, and a valid `LLBBox`.
### Fix Focus Areas
- src/overture.rs[1102-1216]
- src/overture.rs[560-683]
- src/coordinate_system/transformation.rs[14-60]
- src/coordinate_system/geographic/llbbox.rs[13-60]
### Suggested fix
Refactor so that the conversion happens where `coord_transformer` and the real `bbox` are available:
- Compute `(coord_transformer, xzbbox)` in `process_partition_file_by_collection` (once `scale` is correctly passed in).
- Change `parse_row_group_by_collection(...)` to accept `&CoordTransformer` and `&LLBBox` (or perform conversion after parsing, in the caller), and call `*_to_processed_way(..., coord_transformer, bbox)`.
- Replace any need for `new_unchecked` with `LLBBox::new(...)` and proper error handling, or just reuse the already-validated `bbox` argument.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (2)

4. ProcessedWay id type mismatch ☑ 🐞 Bug ≡ Correctness

Description

road_to_processed_way, water_to_processed_way, and landuse_to_processed_way set
ProcessedWay.id using base_id as i64, but ProcessedWay.id is u64, so this does not compile.
It also risks lossy conversion if the cast were forced.

Code

src/overture.rs[R2269-2273]

+    Some(ProcessedWay {
+        id: base_id as i64,
+        tags,
+        nodes,
+    })

Evidence

ProcessedWay.id is defined as u64, but the new conversion functions assign an i64 value to
that field via as i64. This is a type mismatch in Rust struct initialization.

src/overture.rs[2212-2274]
src/osm_parser.rs[103-108]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New Overture conversion helpers assign `ProcessedWay.id` as `base_id as i64`, but `ProcessedWay.id` is `u64`.
### Issue Context
Overture IDs are already generated as `u64` (`gers_id_to_u64`) with the high bit set to avoid collisions.
### Fix Focus Areas
- src/overture.rs[2212-2420]
- src/osm_parser.rs[103-108]
### Suggested fix
Change:
- `id: base_id as i64` -> `id: base_id`
(and keep the rest of the struct literal consistent with `ProcessedWay { id: u64, nodes: Vec<ProcessedNode>, tags: HashMap<...> }`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Overture tests don't compile ☑ 🐞 Bug ≡ Correctness

Description

test_ways_are_close constructs ProcessedNode with fields {x, y, z} and omits required `{id,
tags}, but ProcessedNode is defined as {id, tags, x, z}`. This causes test compilation failure.

Code

src/overture.rs[R2796-2812]

+        let way1 = ProcessedWay {
+            id: 1,
+            nodes: vec![
+                ProcessedNode { x: 0, y: 0, z: 0 },
+                ProcessedNode { x: 10, y: 0, z: 0 },
+            ],
+            tags: HashMap::new(),
+        };
+
+        let way2 = ProcessedWay {
+            id: 2,
+            nodes: vec![
+                ProcessedNode { x: 3, y: 0, z: 0 },
+                ProcessedNode { x: 13, y: 0, z: 0 },
+            ],
+            tags: HashMap::new(),
+        };

Evidence

The test uses a non-existent y field and misses required fields id and tags, which are
mandatory in the ProcessedNode definition used throughout the codebase.

src/overture.rs[2791-2812]
src/osm_parser.rs[84-92]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Overture unit test `test_ways_are_close` builds `ProcessedNode` with incorrect fields, so tests will not compile.
### Issue Context
`ProcessedNode` requires `{ id: u64, tags: HashMap<String,String>, x: i32, z: i32 }`.
### Fix Focus Areas
- src/overture.rs[2791-2812]
- src/osm_parser.rs[84-92]
### Suggested fix
Update the test nodes to include `id` and `tags`, remove `y`, e.g.:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. Copernicus tile over-fetch ☑ 🐞 Bug ➹ Performance

Description

Copernicus tile enumeration uses inclusive floor(min)..=ceil(max) ranges, which over-fetches at
least one extra 1° tile for typical bboxes (non-integer max bounds), causing unnecessary large
downloads and slower elevation fetches. This can multiply network and memory cost because each 1°
GeoTIFF is large.

Code

src/elevation/providers/copernicus.rs[R66-71]

+        let mut tiles: Vec<(i32, i32)> = Vec::new();
+        for lat in (min_lat.floor() as i32)..=(max_lat.ceil() as i32) {
+            for lng in (min_lng.floor() as i32)..=(max_lng.ceil() as i32) {
+                tiles.push((lat, lng));
+            }
+        }

Evidence

The loop uses ceil(max) and an inclusive range, which includes the next integer tile whenever
max_lat/max_lng are not already integers (common). For a bbox contained within one degree, this
can still produce 2×2 = 4 tiles to download.

src/elevation/providers/copernicus.rs[59-71]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Copernicus tile enumeration over-fetches tiles due to `..=ceil(max)`.
### Issue Context
Copernicus tiles are 1°×1°; for most bboxes the max bounds are not integers, so `ceil(max)` advances to the next integer, and the inclusive range pulls in an extra tile.
### Fix Focus Areas
- src/elevation/providers/copernicus.rs[59-71]
### Suggested fix
Compute inclusive integer tile indices using `floor` for both ends, e.g.:
- `lat_start = min_lat.floor() as i32`
- `lat_end = max_lat.floor() as i32`
(and similarly for lng)
If you want precise boundary handling when `max_lat` is exactly an integer boundary, consider subtracting a tiny epsilon before flooring the max (or define bbox semantics clearly and implement accordingly).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[amazon-q-developer]

This PR adds Copernicus DEM 30m elevation provider and expands Overture Maps integration to support roads, water, and land use features alongside buildings. The elevation merge strategy improvements look solid.

Critical Issues Found (6):

1. Operator precedence bug in water feature detection (line 323) - missing parentheses cause incorrect boolean evaluation
2. Type mismatches in ProcessedWay ID fields for roads, water, and land use (lines 2270, 2348, 2416) - incorrect i64 cast instead of u64
3. Compilation failures in test code (lines 2799, 2808) - ProcessedNode struct uses invalid y field that doesn't exist

All issues have specific code suggestions attached for quick fixes. Once these are resolved, the implementation should work correctly.

---

You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

[gemini-code-assist]

Code Review

This pull request introduces the Copernicus DEM GLO-30 elevation provider and significantly expands the Overture Maps integration to include roads, water, and land use features. The elevation system now supports a merge strategy that fills gaps in high-resolution regional data with global fallback data. Review feedback identifies a critical bug in the Overture coordinate transformation logic and a potential resource exhaustion vulnerability in the WKB parser. Additionally, there are several performance concerns regarding the spatial deduplication logic and tile fetching range, as well as a formatting error in the updated .gitignore file.

[gemini-code-assist]

Using CoordTransformer::identity() here is incorrect. Overture data is in geographic coordinates (WGS84), but the pipeline expects Minecraft block coordinates (XZ). By using the identity transformer, you are passing raw lat/lng values as block coordinates, which will break the subsequent clipping logic and result in incorrect feature placement in the world. You should pass the actual coord_transformer (calculated from the target bbox and scale) into this function and use it here.

[gemini-code-assist]

The .gitignore file contains triple backticks at the beginning and end (line 85), which are likely a formatting error from the code generation process. These will cause Git to misinterpret the file. Please remove them.

# Dependencies

[gemini-code-assist]

Recomputing the grid bounds (grid_min_x, grid_min_z) by iterating over all bboxes in every call to overlaps_with_grid is highly inefficient ($O(N)$ per call). Since this function is called for every Overture feature, this results in $O(M \times N)$ complexity for deduplication, which will be extremely slow for large datasets. These bounds should be computed once in build_spatial_grid and stored or passed into this function.

[gemini-code-assist]

The num_points value is read directly from the input buffer and used to allocate a Vec with that capacity. This is a potential security vulnerability (Resource Exhaustion) as a malformed or malicious WKB payload could specify a massive number of points, leading to an Out-Of-Memory (OOM) panic. You should validate that num_points is reasonable and that the input buffer wkb actually contains enough data to satisfy the request.

[gemini-code-assist]

This road deduplication logic has $O(N \times M)$ complexity, where $N$ is the number of Overture roads and $M$ is the number of OSM roads. For each pair, ways_are_close also performs an $O(Nodes_1 \times Nodes_2)$ check. This will be extremely slow for large areas with many roads. Consider using a spatial index (like the grid used for buildings) to limit the number of OSM roads checked for each Overture road.

[gemini-code-assist]

The grid cell size is hardcoded as 64 here. It should use the CELL_SIZE constant defined in deduplicate_against_osm (or preferably a module-level constant) to ensure consistency with the grid construction.

[qodo-code-review]

1. Copernicus never selected 🐞 Bug ≡ Correctness

CopernicusDem30 returns coverage_bboxes() -> None, but select_provider() only considers
providers with Some(coverages), so Copernicus is never chosen and AWS remains the only global
fallback. This makes the new provider effectively dead code and the intended behavior change does
not occur.

Agent Prompt

### Issue description
`CopernicusDem30` is added to the provider list but will never be selected because `select_provider()` ignores providers whose `coverage_bboxes()` returns `None`.

### Issue Context
The `ElevationProvider` trait explicitly uses `coverage_bboxes() == None` to represent global providers (e.g., AWS Terrain Tiles). The selector currently only checks overlap for `Some(coverages)` and otherwise skips.

### Fix Focus Areas
- src/elevation/selector.rs[22-41]
- src/elevation/providers/copernicus.rs[21-28]
- src/elevation/provider.rs[18-23]

### Suggested fix
Update `select_provider()` to treat `coverage_bboxes() == None` as "global coverage" candidates when no regional provider matches. For example:
- Iterate candidates; return first overlapping regional provider.
- Otherwise, if any candidate has `coverage_bboxes() == None`, return the best global candidate (e.g., lowest `native_resolution_m()` or first in list).
- Only if none exist, fall back to AWS.

(Alternatively, if you want Copernicus treated as regional-with-global-extent, return `Some(vec![global_bbox])`, but that contradicts the trait’s documented meaning of `None`.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-code-review]

2. Undefined scale in partition 🐞 Bug ≡ Correctness

process_partition_file_by_collection() uses scale when clipping elements, but scale is not in
scope and not passed as a parameter, so the code does not compile. This blocks the new Overture
multi-collection fetch path used by main.rs.

Agent Prompt

### Issue description
`process_partition_file_by_collection()` references `scale` but does not accept it as a parameter, so the Overture pipeline will not compile.

### Issue Context
`scale` is available in `fetch_overture_features_inner()` and must be threaded into partition processing where clipping/coord transforms occur.

### Fix Focus Areas
- src/overture.rs[494-547]
- src/overture.rs[560-683]

### Suggested fix
- Add `scale: f64` parameter to `process_partition_file_by_collection(...)`.
- Pass `scale` from the caller in `fetch_overture_features_inner()`.
- Consider computing `CoordTransformer::llbbox_to_xzbbox(bbox, scale)` once per overall fetch (not per partition) and reusing it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-code-review]

3. Nonexistent transformer/bbox apis 🐞 Bug ≡ Correctness

parse_row_group_by_collection() calls CoordTransformer::identity() and
LLBBox::new_unchecked(...), but neither function exists in the codebase, so the new Overture
parsing path does not compile. This prevents any of the new collection types
(building/road/water/landuse) from being converted into ProcessedWays.

Agent Prompt

### Issue description
`parse_row_group_by_collection()` uses `CoordTransformer::identity()` and `LLBBox::new_unchecked(...)`, which do not exist, causing compilation failure.

### Issue Context
The conversion helpers (`building_to_processed_way`, `road_to_processed_way`, etc.) expect a real `CoordTransformer` configured for the requested `bbox` + `scale`, and a valid `LLBBox`.

### Fix Focus Areas
- src/overture.rs[1102-1216]
- src/overture.rs[560-683]
- src/coordinate_system/transformation.rs[14-60]
- src/coordinate_system/geographic/llbbox.rs[13-60]

### Suggested fix
Refactor so that the conversion happens where `coord_transformer` and the real `bbox` are available:
- Compute `(coord_transformer, xzbbox)` in `process_partition_file_by_collection` (once `scale` is correctly passed in).
- Change `parse_row_group_by_collection(...)` to accept `&CoordTransformer` and `&LLBBox` (or perform conversion after parsing, in the caller), and call `*_to_processed_way(..., coord_transformer, bbox)`.
- Replace any need for `new_unchecked` with `LLBBox::new(...)` and proper error handling, or just reuse the already-validated `bbox` argument.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools