feat: expose read-only layer queries to external plugins - #1784
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe plugin API now provides read-only layer, feature, drawing, and selection queries. It also supports selection-change subscriptions. Documentation and tests define and verify the new behavior. ChangesPlugin query API
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Plugin
participant createAppAPI
participant AppStore
participant readPluginSelection
Plugin->>createAppAPI: Register onSelectionChange callback
createAppAPI->>AppStore: Subscribe to selected layer and feature IDs
AppStore-->>createAppAPI: Report selection change
createAppAPI->>readPluginSelection: Resolve selected features
readPluginSelection-->>createAppAPI: Return layer ID and features
createAppAPI-->>Plugin: Invoke callback
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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 |
Jihkk
marked this pull request as ready for review
August 11, 2026 17:10
![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: 2
🤖 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/hooks/usePlugins.ts`:
- Around line 834-844: The plugin query APIs must return detached feature data
rather than Zustand-owned Feature objects. Update getLayerFeatures,
getSelectedFeatures, getDrawnFeatures, and selection callbacks such as
readPluginSelection to deep-copy features, including nested properties and
geometry coordinates; add a regression test that mutates a returned feature and
verifies the store state is unchanged.
In `@packages/plugins/src/types.ts`:
- Around line 342-355: Update GeoLibreSelection.features and every feature-query
method in packages/plugins/src/types.ts to use Feature<Geometry | null>[],
importing Geometry as needed; mirror the same nullable geometry types for all
corresponding APIs in docs/plugin-api.md and add the Geometry import there.
Ensure the public contracts match runtime features that may have geometry: null.
🪄 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: bfdd86d9-2922-4e54-8282-5f5225dc35f5
📒 Files selected for processing (4)
apps/geolibre-desktop/src/hooks/usePlugins.tsdocs/plugin-api.mdpackages/plugins/src/types.tstests/plugin-query-api.test.ts
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)
apps/geolibre-desktop/src/hooks/usePlugins.ts (1)
860-867: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInclude detached
metadatain each layer summary.
GeoLibreLayerSummaryincludes layer metadata, butlistLayers()omits it. Plugins that use the declaredmetadatafield receiveundefined.Return
metadata: structuredClone(metadata)with each summary. This also preserves the read-only API boundary.Proposed fix
- useAppStore.getState().layers.map(({ id, name, type, visible, opacity }) => ({ + useAppStore.getState().layers.map(({ id, name, type, visible, opacity, metadata }) => ({ id, name, type, visible, opacity, + metadata: structuredClone(metadata), })),🤖 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 `@apps/geolibre-desktop/src/hooks/usePlugins.ts` around lines 860 - 867, Update listLayers in usePlugins.ts to destructure each layer’s metadata and include metadata: structuredClone(metadata) in every returned layer summary, preserving the read-only API boundary and the existing summary fields.
🤖 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 `@apps/geolibre-desktop/src/hooks/usePlugins.ts`:
- Around line 860-867: Update listLayers in usePlugins.ts to destructure each
layer’s metadata and include metadata: structuredClone(metadata) in every
returned layer summary, preserving the read-only API boundary and the existing
summary fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0f975e57-8375-4547-9a6a-e0a783df0681
📒 Files selected for processing (4)
apps/geolibre-desktop/src/hooks/usePlugins.tsdocs/plugin-api.mdpackages/plugins/src/types.tstests/plugin-query-api.test.ts
Contributor
🔍 Cloudflare PR preview
|
giswqs
approved these changes
Aug 14, 2026
giswqs
left a comment
giswqs
left a comment
Member
There was a problem hiding this comment.
LGTM. Thank you for your contribution.
Contributor
🔍 GitHub Pages PR preview
Note GitHub Pages built this preview successfully, but its serving edge returned HTTP 403 when checked. The links may still be propagating. |
giswqs
added a commit
that referenced
this pull request
Aug 14, 2026
) The read-only query API added in #1784 is exercised through `createAppAPI`, which lives in `usePlugins.ts`. That module imports the whole built-in plugin registry, so loading it in a Node test pulled in MapCanvas, CesiumCanvas, and every `maplibre-*` plugin: 39 browser-only modules, none of them meaningfully exercised, all of them newly counted by the coverage reporter. Coverage is reported only over files a test actually imports, so those 39 files landed in the denominator at 1-30% function coverage and dropped the total from 72.90% to 60.36%, under the 63% floor. CI has been red on `main` since, and the report is measuring module reachability rather than how well the code is tested. Move the six query methods and `readPluginSelection` into `lib/plugin-layer-queries.ts`, which needs only the store, and have `createAppAPI` spread them in. The test imports that module directly and no longer stubs `maplibre-gl`, `window`, `sessionStorage`, or `localStorage` to get off the ground. Same reasoning, and the same shape, as `geo-editor-geometry.ts` in `@geolibre/plugins`, which is already kept free of the Geoman/MapLibre runtime so it can be unit-tested under Node. `SKETCHES_SOURCE_KIND` gets a subpath export alongside the five already in that package, so the test can share the constant instead of repeating the string literal as it did before. The wiring into the plugin-facing API is now a typed spread rather than something this test asserts; `tsc` covers it on every build. Coverage returns to 82.84% lines / 84.45% branches / 72.55% functions (baseline before #1784: 82.89 / 84.46 / 72.90), with the counted file set back from 444 to 406. The one addition is the extracted module itself, at 100% lines and 95.45% functions. All 8 tests still pass.
giswqs
added a commit
that referenced
this pull request
Aug 14, 2026
) * fix(ci): re-measure a line-only coverage shortfall before failing Line coverage is nondeterministic on CI. Two runs over byte-identical sources reported 81.82% and 76.47%, with 114 of 444 files differing on lines and zero differing on branches or functions, the same 5935 tests passing in both. The low run failed a floor the identical tree had cleared minutes earlier, which is how a release PR touching only version strings and docs came to fail CI. It does not reproduce locally. Eleven runs across Node 26 and CI's exact Node 22.23.2, at 24 cores and pinned to 4 with taskset, and at both default and serial test concurrency, all landed within 0.05 points. Serial execution produced identical numbers to parallel at 2.3x the wall time, so pinning concurrency buys nothing measurable. Since the trigger is unknown but the signature is specific, mitigate it narrowly instead of lowering the floor. `test:frontend:coverage` now runs through `scripts/coverage-check.mjs`, following the `audit-check.mjs` precedent of wrapping a tool that cannot express the policy we want. Node still enforces all three floors; the wrapper re-measures once when lines alone come up short with every test passing, and fails if the second run is short too. Branch and function shortfalls, and any test failure, fail immediately with no retry, so a real regression still fails fast and a line regression fails on the second run rather than being retried away. `classify()` is exported and covered by tests/coverage-check.test.ts: a retry that swallowed a genuine regression would be worse than the flake it works around, so which failures earn a second run is pinned by tests rather than trusted. Verified end to end with impossible floors: a line-only shortfall runs the suite twice and fails; a function shortfall runs it once and fails. CLAUDE.md documents the wrapper, and separately the denominator trap that caused the real regression in #1784, since "first test for a big module looks like a regression" will recur. * fix(ci): stream the wrapped suite instead of buffering it The first version of the wrapper captured the run with spawnSync and wrote it out afterwards, then called process.exit. `process.stdout` is asynchronous when it is a pipe, which is what CI gives it, so the exit discarded everything still queued: the CI run for this branch lost about 42,000 lines of test output and the entire coverage summary, cut off mid-line, while still reporting success. Redirecting to a file locally made stdout synchronous, which is why it passed here first. Stream the child's stdout and stderr through as they arrive, and return an exit code for the caller to assign to process.exitCode rather than calling process.exit anywhere, so Node exits only once the output has drained. Classification is unchanged; it reads the accumulated string either way. Verified with stdout on a pipe, the CI shape rather than the one that hid it: 45,384 lines through the wrapper ending in "end of coverage report" with the summary intact, against 2,044 truncated lines before. The retry paths still behave, checked with impossible floors: line-only short runs the suite twice and exits 1, function short runs it once and exits 1. Pins the regression at the source, since a truncated log looks green and no assertion downstream would notice.
This was referenced Aug 16, 2026
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.
Summary
Why
External plugins can add layers, but they currently cannot inspect GeoLibre's current layer list or feature selection through the public plugin API. Plugins that need to run an analysis for a user-selected feature therefore have no supported read-only path to obtain that context.
The new methods remain optional so existing plugins can continue to run against older GeoLibre hosts. The query surface does not mutate the application store, and subscription callers receive an unsubscribe function for plugin cleanup.
API
listLayers()getLayerFeatures(layerId)getSelectedFeatures()getSelectedLayerId()getDrawnFeatures()onSelectionChange(callback)Validation
node --import tsx --test tests/plugin-query-api.test.ts— 7 passednpm run lint— 0 errors (40 existing warnings)npm run build— passedThe API was also exercised from an external hydrology plugin in the Tauri desktop during a synthetic end-to-end workflow.
Summary by CodeRabbit
New Features
Tests