feat(processing): add DGGS processing - #1710
Merged
Merged
Conversation
- DGGS Generator - DGGS Binning - DGGS Compact
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change replaces H3-specific vector tools with unified DGGS grid, binning, and compaction tools. It adds H3, S2, A5, DGGRID, and DGGAL processing, DuckDB loaders, desktop integration, legacy rerun migration, localization updates, and extensive tests. ChangesDGGS processing
Desktop integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant VectorToolsDialog
participant DggsTools
participant DuckDBOrDggal
participant ResultLayer
User->>VectorToolsDialog: Select DGGS tool and parameters
VectorToolsDialog->>DggsTools: Execute grid, bin, or compact operation
DggsTools->>DuckDBOrDggal: Run backend operation
DuckDBOrDggal-->>DggsTools: Return cell features
DggsTools->>ResultLayer: Create result layer
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🔍 GitHub Pages PR preview
|
thangqd
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/geolibre-desktop/src/components/processing/VectorToolsDialog.tsx`:
- Around line 497-501: Update the dynamic label in the VectorToolsDialog
parameter mapping to use the translation function with a new English catalog key
that accepts the max interpolation value. Add the corresponding key to the
English catalog and preserve the rendered “Resolution (0-max)” text through
localized interpolation.
In `@apps/geolibre-desktop/src/lib/duckdb-vector-loader.ts`:
- Around line 287-306: Replace the duplicated memoized loader implementations
for ensureH3Extension, ensureA5Extension, and ensureDuckDggsExtension with one
module-internal createCommunityExtensionLoader factory that installs and loads
the provided literal extension name, clears its cached promise on failure, and
rethrows the error. Export the three ensure*Extension functions from factory
calls using fixed internal names; do not expose caller-supplied extension names.
In `@packages/core/src/store.ts`:
- Around line 126-128: Add deserialization migration handling for legacy
"h3-grid" and "h3-bin-points" values in ui.vectorToolOpen and Processing
History, mapping them to "dggs-grid" and "dggs-bin" respectively before they are
opened or used. Update the relevant persistence/loading logic near
VectorToolKind so these removed kinds cannot remain as unsupported runtime
values.
In `@packages/processing/src/a5-tools.ts`:
- Around line 51-57: Centralize the SQL escaping helpers by exporting sqlStr and
sqlIdent from the existing definitions in packages/processing/src/h3-tools.ts.
In packages/processing/src/a5-tools.ts at lines 51-57, remove the local
definitions and import both helpers from ./h3-tools; make the same removal and
import change in packages/processing/src/dggrid-tools.ts at lines 326-332.
Preserve all existing call sites and escaping behavior.
In `@packages/processing/src/antimeridian.ts`:
- Around line 17-27: Update unwrapAntimeridianRing to preserve every coordinate
component after longitude normalization: copy each source vertex’s remaining
members while replacing only its longitude, and retain those components when
adding the closing vertex. Keep the existing latitude, unwrapping, and
closed-ring behavior unchanged.
In `@packages/processing/src/dggal-tools.ts`:
- Around line 228-288: Update dggalGridFromBbox so that after clamping south and
north to [-90, 90], it swaps them when south exceeds north. Ensure
engine.listZones receives ll.lat <= ur.lat, matching the normalization behavior
used by normalizeLonLatBbox.
- Around line 169-196: Apply antimeridian unwrapping to both conversion paths:
in zoneRing, return the ring through unwrapAntimeridianRing; in
a5RowsToFeatureCollection, add the fixAntimeridian parameter matching
rowsToFeatureCollection and conditionally apply unwrapAntimeridianGeometry to
parsed geometry. Update FIX_ANTIMERIDIAN_PARAM.visibleWhen in
packages/processing/src/dggs-tools.ts to include both a5 and dggal; make the
corresponding changes in packages/processing/src/dggal-tools.ts lines 169-196
and packages/processing/src/a5-tools.ts lines 243-277.
In `@packages/processing/src/dggrid-tools.ts`:
- Around line 388-412: Refactor buildDggridGridFromSourceSql to reuse the shared
CTE chain from cellsCteFromGeom instead of duplicating _dggs_area, _dggs_meta,
_dggs_grid, _dggs_pts, and cells. Extend cellsCteFromGeom to accept the
pre-built source-specific _dggs_area SELECT, then pass the merged source SQL
through it from both builders while preserving the existing gridSelect output.
- Around line 344-362: Update cellsCteFromGeom so the computed step scales up
when either geometry axis would require more than DGGRID_SAMPLE_AXIS_CAP
samples, ensuring the generated series reaches the full bounding box without
LEAST truncating coverage. Preserve the existing minimum step and
cell-generation flow, and document that large areas may be under-sampled rather
than clipped.
- Around line 364-370: Update gridSelect and buildDggridBinSql so their
ST_AsGeoJSON(...) projections cast the resulting geojson value to VARCHAR,
matching the existing A5 cell SQL strategy and allowing
dggridRowsToFeatureCollection to process the rows.
In `@packages/processing/src/dggs-tools.ts`:
- Around line 467-472: Extract the repeated DGGS backend label ternary into a
shared helper such as dggsLabel(type, dggridType, dggalType), preserving the
existing dggrid, dggal, and DGGS_TYPE_LABEL behavior. Replace the duplicated
logic in all three tool locations, including the flows around the current label
computations at lines 467, 667, and 879.
- Around line 538-583: Ensure extension setup failures are handled consistently
with other backends. In packages/processing/src/dggs-tools.ts lines 538-583,
move ensureExtensions inside the try in createDggsGridTool and add a catch
before the existing finally that logs the error message through ctx.log. Apply
the same change in packages/processing/src/dggs-tools.ts lines 641-739 for
dggsBinPointsTool, moving its ensureExtensions call into the try and adding the
matching catch; preserve both existing cleanup finally blocks.
In `@packages/processing/src/h3-tools.ts`:
- Around line 80-97: Update normalizeLonLatBbox to handle dateline-crossing
inputs such as west = -190 and east = -10 without dropping requested coverage,
preferably by normalizing such wrapped boxes to [-180, 180] through the
wide-span path; alternatively, document the function’s constraint that wrapping
viewports are unsupported. Keep the existing DGGS caller behavior unchanged.
In `@packages/processing/src/index.ts`:
- Around line 82-83: Remove the deprecated unwrapDggridGeometry and
unwrapDggridRing aliases from dggrid-tools.ts, then remove both names from the
barrel exports in index.ts; retain the canonical unwrapAntimeridianGeometry and
unwrapAntimeridianRing exports.
In `@packages/processing/src/s2-tools.ts`:
- Around line 336-355: Guard sentinel min/max values in both binners: update
binPointsToS2 in packages/processing/src/s2-tools.ts (lines 336-355) and
binPointsToDggal in packages/processing/src/dggal-tools.ts (lines 518-528) to
emit null when acc.min or acc.max is non-finite, otherwise retain the numeric
value.
In `@packages/processing/src/vector-tools.ts`:
- Around line 2694-2697: Update the rerun-history handling used by
VectorToolsDialog to translate legacy H3 tool IDs to the current DGGS tools
before getVectorTool(rerun.toolId) evaluates them. Add aliases for the removed
H3 grid, bin, and compact IDs, mapping them to createDggsGridTool,
dggsBinPointsTool, and dggsCompactTool respectively, while supplying dggsType:
"h3", so those reruns are recognized and cleared normally.
In `@tests/dggs-tools.test.ts`:
- Around line 362-365: Replace the disjunctive ISEA3H assertion in
tests/dggs-tools.test.ts lines 362-365 with an assertion for the single expected
outcome at resolution 30 on a 1° viewport, covering whether one layer is added
or the hard cell cap aborts as determined by the implementation. In
tests/s2-tools.test.ts line 36, split the combined assertion into separate
checks that estimateS2CellCount(big, 30) exceeds S2_HARD_CAP and
estimateS2CellCount(big, 20) is finite.
- Around line 90-92: Simplify the condition in the SQL mock branch by removing
the redundant “a5_lonlat” operand from the check in the test setup. Keep the
existing “a5_” match and returned fixture unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e8f5348b-930f-47dd-acb6-b582b636561a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (43)
apps/geolibre-desktop/src/components/layout/toolbar/ProcessingMenu.tsxapps/geolibre-desktop/src/components/layout/toolbar/constants.tsapps/geolibre-desktop/src/components/processing/ParameterField.tsxapps/geolibre-desktop/src/components/processing/VectorToolsDialog.tsxapps/geolibre-desktop/src/i18n/locales/ar.jsonapps/geolibre-desktop/src/i18n/locales/de.jsonapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/i18n/locales/es.jsonapps/geolibre-desktop/src/i18n/locales/fr.jsonapps/geolibre-desktop/src/i18n/locales/hi.jsonapps/geolibre-desktop/src/i18n/locales/id.jsonapps/geolibre-desktop/src/i18n/locales/it.jsonapps/geolibre-desktop/src/i18n/locales/ja.jsonapps/geolibre-desktop/src/i18n/locales/ka.jsonapps/geolibre-desktop/src/i18n/locales/ko.jsonapps/geolibre-desktop/src/i18n/locales/nl.jsonapps/geolibre-desktop/src/i18n/locales/pt.jsonapps/geolibre-desktop/src/i18n/locales/ru.jsonapps/geolibre-desktop/src/i18n/locales/tr.jsonapps/geolibre-desktop/src/i18n/locales/zh.jsonapps/geolibre-desktop/src/lib/duckdb-processing.tsapps/geolibre-desktop/src/lib/duckdb-vector-loader.tspackages/core/src/store.tspackages/plugins/src/plugins/maplibre-geohash.tspackages/plugins/src/plugins/maplibre-olc.tspackages/plugins/src/plugins/maplibre-tilecode.tspackages/processing/package.jsonpackages/processing/src/a5-tools.tspackages/processing/src/antimeridian.tspackages/processing/src/dggal-tools.tspackages/processing/src/dggrid-tools.tspackages/processing/src/dggs-tools.tspackages/processing/src/h3-tools.tspackages/processing/src/index.tspackages/processing/src/s2-tools.tspackages/processing/src/vector-tools.tstests/a5-tools.test.tstests/dggal-tools.test.tstests/dggrid-tools.test.tstests/dggs-compact.test.tstests/dggs-tools.test.tstests/h3-tools.test.tstests/s2-tools.test.ts
💤 Files with no reviewable changes (15)
- apps/geolibre-desktop/src/i18n/locales/es.json
- apps/geolibre-desktop/src/i18n/locales/ka.json
- apps/geolibre-desktop/src/i18n/locales/zh.json
- apps/geolibre-desktop/src/i18n/locales/ru.json
- apps/geolibre-desktop/src/i18n/locales/ko.json
- apps/geolibre-desktop/src/i18n/locales/nl.json
- apps/geolibre-desktop/src/i18n/locales/hi.json
- apps/geolibre-desktop/src/i18n/locales/tr.json
- apps/geolibre-desktop/src/i18n/locales/ar.json
- apps/geolibre-desktop/src/i18n/locales/ja.json
- apps/geolibre-desktop/src/i18n/locales/de.json
- apps/geolibre-desktop/src/i18n/locales/it.json
- apps/geolibre-desktop/src/i18n/locales/fr.json
- apps/geolibre-desktop/src/i18n/locales/pt.json
- apps/geolibre-desktop/src/i18n/locales/id.json
Contributor
🔍 Cloudflare PR preview
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/processing/src/index.ts (1)
50-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove unused
H3_TOOLSandgetH3Toolexports.
H3_TOOLSandgetH3Toolhave no consumers in the repository. Remove both names frompackages/processing/src/index.tsandpackages/processing/src/h3-tools.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/processing/src/index.ts` around lines 50 - 51, Remove the unused H3_TOOLS and getH3Tool exports from the processing package, including their declarations or export wiring in h3-tools.ts and their re-exports in index.ts. Preserve the remaining H3 functionality and exports.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/processing/src/index.ts`:
- Around line 50-51: Remove the unused H3_TOOLS and getH3Tool exports from the
processing package, including their declarations or export wiring in h3-tools.ts
and their re-exports in index.ts. Preserve the remaining H3 functionality and
exports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 94012a7e-8d2a-49b9-9765-83d9eeeb10a7
📒 Files selected for processing (21)
apps/geolibre-desktop/src/components/processing/ProcessingHistoryDialog.tsxapps/geolibre-desktop/src/components/processing/VectorToolsDialog.tsxapps/geolibre-desktop/src/i18n/locales/en.jsonapps/geolibre-desktop/src/lib/duckdb-vector-loader.tspackages/core/src/project.tspackages/core/src/store.tspackages/processing/src/a5-tools.tspackages/processing/src/antimeridian.tspackages/processing/src/dggal-tools.tspackages/processing/src/dggrid-tools.tspackages/processing/src/dggs-tools.tspackages/processing/src/h3-tools.tspackages/processing/src/index.tspackages/processing/src/s2-tools.tspackages/processing/src/vector-tools.tstests/dggrid-tools.test.tstests/dggs-compact.test.tstests/dggs-tools.test.tstests/h3-tools.test.tstests/processing-history.test.tstests/s2-tools.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/dggal-tools.test.ts`:
- Around line 117-119: Update the ring validation in the feature loop to first
assert that each ring contains enough coordinates for a closed polygon ring,
then perform the existing longitude-span check. Ensure empty or undersized rings
fail explicitly instead of passing via the Math.max/Math.min result.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 75d69c78-55e8-4a11-8e3f-96defbfe32aa
📒 Files selected for processing (7)
packages/processing/src/a5-tools.tspackages/processing/src/dggal-tools.tspackages/processing/src/dggs-tools.tstests/a5-tools.test.tstests/dggal-tools.test.tstests/dggs-compact.test.tstests/dggs-tools.test.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/dggal-tools.test.ts (1)
70-78: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winVerify that bbox compaction preserves coverage.
The current assertions accept a no-op because equal feature counts pass. They also accept dropped cells because a smaller count passes. Expand the compacted DGGAL tokens to resolution 5 and compare them with the plain grid tokens. Use a fixture that is known to contain sibling groups if the test must also prove that compaction reduces the count.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/dggal-tools.test.ts` around lines 70 - 78, Update the test around dggalGridFromBbox to expand compacted tokens back to resolution 5 and assert their token set matches the plain grid token set, proving coverage is preserved. If verifying reduction, use a bbox fixture known to contain sibling groups and assert compacted.features.length is strictly less than plain.features.length rather than allowing equality.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/dggal-tools.test.ts`:
- Around line 117-120: Update the ring assertions in the feature loop to compare
the first and last coordinate pairs and require them to match, confirming the
polygon ring is closed before validating longitude span. Keep the existing
minimum vertex-count assertion and longitude-span check unchanged.
---
Outside diff comments:
In `@tests/dggal-tools.test.ts`:
- Around line 70-78: Update the test around dggalGridFromBbox to expand
compacted tokens back to resolution 5 and assert their token set matches the
plain grid token set, proving coverage is preserved. If verifying reduction, use
a bbox fixture known to contain sibling groups and assert
compacted.features.length is strictly less than plain.features.length rather
than allowing equality.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f125dbdf-803a-45d4-9169-90bf34658765
📒 Files selected for processing (1)
tests/dggal-tools.test.ts
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Member
|
@thangqd The CI failure here is a small merge-time i18n conflict — could you fix it up? What fails: one frontend test, Everything else is green (lint warnings are pre-existing, coverage floors are met — the coverage table just happens to be the last thing printed, so the log tail looks clean). Why: the Thai catalog Fix: merge (or rebase onto) "h3Grid": "สร้างกริด H3",
"h3BinPoints": "จัดกลุ่มจุดลงกริด H3",I verified this locally on a merge of your branch with One note while you're in there: no locale (including the 16 you touched) has the new |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add DGGS Processing into Processing-GeoLibre-Vector, supporting H3, S2, A5, DGGRID, and DGGAL.
Summary by CodeRabbit
Summary by CodeRabbit