Add LST Super-Resolution (SR2D4) plugin - #48
Conversation
|
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:
📝 WalkthroughWalkthroughAdds the ChangesLST super-resolution plugin
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BaseDecoder
participant RasterDecoder
participant CodecRuntime
BaseDecoder->>RasterDecoder: decode(buffer)
RasterDecoder->>CodecRuntime: decodeBlock(buffer)
CodecRuntime-->>RasterDecoder: return decoded bytes
RasterDecoder-->>BaseDecoder: apply predictor and return buffer
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@plugin-registry.json`:
- Line 106: Correct the Portuguese description value in the plugin registry by
replacing the misspelled “espcial” term with “espacial”; leave the rest of the
description unchanged.
In `@plugins/lst-super-resolution/index.js`:
- Around line 202-244: Replace the remaining GeoLibre template implementation
with the LST Super-Resolution SR2D4 workflow: update openFiles/loadFromUrl to
accept and process LST inputs and render the result, replace the demo panels and
Template toolbar menu with LST SR2D4 controls and result views, set the control
title to the manifest’s LST plugin name, and update plugin identity fields to
the manifest values. In plugins/lst-super-resolution/index.js lines 202-244,
implement the input/processing/map-render flow; lines 464-593, replace the demo
UI; lines 637-645, update the title; and lines 657-661, set plugin.id,
plugin.name, and plugin.version and namespace host registration IDs under
lst-super-resolution.
- Around line 123-129: Update setState() to render the merged state on the
mounted panel: synchronize its width and expanded class after assigning _state,
and recalculate the panel position when the restored state is expanded. Preserve
the existing statechange emission and use the panel’s established
rendering/positioning methods.
- Around line 690-698: Update setMapControlPosition so the existing position is
preserved until app.addMapControl succeeds; when reattachment fails, restore or
retain that previous position while preserving the pending-state cleanup, and
commit nextPosition only after a successful add.
In `@plugins/lst-super-resolution/style.css`:
- Around line 196-213: Increase the text contrast for the .plugin-control-action
button by updating its color or accent token in the source stylesheet
src/lib/styles/plugin-control.css, not the generated plugin stylesheet; ensure
the normal button state meets a 4.5:1 contrast ratio and regenerate the bundle
afterward.
- Around line 77-90: Add a visible :focus-visible focus indicator for
.plugin-control-toggle in src/lib/styles/plugin-control.css, replacing the
removed outline for keyboard users while preserving the existing control
styling. Regenerate the Vite bundle so the corresponding generated
plugins/*/style.css output is updated.
🪄 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: f412b7d5-cdf1-4721-a300-5ed80ed35fa9
📒 Files selected for processing (4)
plugin-registry.jsonplugins/lst-super-resolution/index.jsplugins/lst-super-resolution/plugin.jsonplugins/lst-super-resolution/style.css
| { | ||
| "id": "lst-super-resolution", | ||
| "name": "LST Super-Resolution (SR2D4)", | ||
| "description": "Downscaling espacial avançado (TsHARP) da temperatura de superfície Landsat (100m para 1m) utilizando Sentinel SR2D4 com análise estatística de Ilhas de Calor (UHI).", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the Portuguese description.
Replace espcial with espacial.
🤖 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 `@plugin-registry.json` at line 106, Correct the Portuguese description value
in the plugin registry by replacing the misspelled “espcial” term with
“espacial”; leave the rest of the description unchanged.
| setState(newState) { | ||
| this._state = { | ||
| ...this._state, | ||
| ...newState | ||
| }; | ||
| this._emit("statechange"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Render accepted state changes.
applyProjectState() calls setState() for an active control at lines 703-706. setState() only changes _state. The mounted panel keeps its old width and expanded state.
Update the panel width and expanded class after merging state. Recalculate the panel position when the restored state is expanded.
Proposed fix
setState(newState) {
this._state = {
...this._state,
...newState
};
+ if (this._panel) {
+ this._panel.style.width = `${this._state.panelWidth}px`;
+ this._panel.classList.toggle("expanded", !this._state.collapsed);
+ if (!this._state.collapsed) this._updatePanelPosition();
+ }
this._emit("statechange");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setState(newState) { | |
| this._state = { | |
| ...this._state, | |
| ...newState | |
| }; | |
| this._emit("statechange"); | |
| } | |
| setState(newState) { | |
| this._state = { | |
| ...this._state, | |
| ...newState | |
| }; | |
| if (this._panel) { | |
| this._panel.style.width = `${this._state.panelWidth}px`; | |
| this._panel.classList.toggle("expanded", !this._state.collapsed); | |
| if (!this._state.collapsed) this._updatePanelPosition(); | |
| } | |
| this._emit("statechange"); | |
| } |
🤖 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 `@plugins/lst-super-resolution/index.js` around lines 123 - 129, Update
setState() to render the merged state on the mounted panel: synchronize its
width and expanded class after assigning _state, and recalculate the panel
position when the restored state is expanded. Preserve the existing statechange
emission and use the panel’s established rendering/positioning methods.
| setMapControlPosition(app, nextPosition) { | ||
| position = nextPosition; | ||
| if (!control) return; | ||
| app.removeMapControl(control); | ||
| if (!app.addMapControl(control, position)) { | ||
| pendingState = control.getState(); | ||
| control = null; | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore the previous position when reattachment fails.
Line 691 stores nextPosition before app.addMapControl() succeeds. If line 694 fails, getMapControlPosition() and the next activation keep the rejected position. The plugin can remain unavailable until another caller changes it.
Commit position only after a successful add, or restore the previous value on failure.
Proposed fix
setMapControlPosition(app, nextPosition) {
- position = nextPosition;
if (!control) return;
+ const previousPosition = position;
app.removeMapControl(control);
- if (!app.addMapControl(control, position)) {
+ if (!app.addMapControl(control, nextPosition)) {
+ position = previousPosition;
pendingState = control.getState();
control = null;
return false;
}
+ position = nextPosition;
}🤖 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 `@plugins/lst-super-resolution/index.js` around lines 690 - 698, Update
setMapControlPosition so the existing position is preserved until
app.addMapControl succeeds; when reattachment fails, restore or retain that
previous position while preserving the pending-state cleanup, and commit
nextPosition only after a successful add.
| /* Toggle button - 29x29 to match navigation control */ | ||
| .plugin-control-toggle { | ||
| background: none; | ||
| border: none; | ||
| padding: 0; | ||
| width: 29px; | ||
| height: 29px; | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| cursor: pointer; | ||
| outline: none; | ||
| color: var(--pc-icon); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore a visible keyboard-focus indicator.
Line 88 removes the browser focus outline. The toggle button has no :focus or :focus-visible replacement. Keyboard users cannot identify the focused control.
Add a visible :focus-visible style in the template stylesheet, then regenerate this bundle. Based on learnings, plugins/*/style.css is generated Vite output, so update src/lib/styles/plugin-control.css instead of editing this file directly.
🤖 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 `@plugins/lst-super-resolution/style.css` around lines 77 - 90, Add a visible
:focus-visible focus indicator for .plugin-control-toggle in
src/lib/styles/plugin-control.css, replacing the removed outline for keyboard
users while preserving the existing control styling. Regenerate the Vite bundle
so the corresponding generated plugins/*/style.css output is updated.
Source: Learnings
| .plugin-control-action { | ||
| display: inline-flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| padding: 8px 16px; | ||
| font-size: 14px; | ||
| font-weight: 500; | ||
| color: white; | ||
| background: var(--pc-accent); | ||
| border: none; | ||
| border-radius: 4px; | ||
| cursor: pointer; | ||
| transition: background-color 0.15s; | ||
| } | ||
|
|
||
| .plugin-control-action:hover { | ||
| background: var(--pc-accent-hover); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Increase action-button text contrast.
White text on --pc-accent: #4a90d9`` has about 3.3:1 contrast. The button text is 14px, so it does not meet the common 4.5:1 minimum. Use an accessible accent token or a darker text color.
Make the change in the template stylesheet and regenerate this bundle. Based on learnings, plugins/*/style.css is generated Vite output, so update src/lib/styles/plugin-control.css instead of editing this file directly.
🤖 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 `@plugins/lst-super-resolution/style.css` around lines 196 - 213, Increase the
text contrast for the .plugin-control-action button by updating its color or
accent token in the source stylesheet src/lib/styles/plugin-control.css, not the
generated plugin stylesheet; ensure the normal button state meets a 4.5:1
contrast ratio and regenerate the bundle afterward.
Source: Learnings
|
/claude-review |
|
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 `@plugins/lst-super-resolution/zstd-MVmsxGT0.js`:
- Around line 134-145: Document the provenance and licenses for all nine
committed generated chunks: in
plugins/lst-super-resolution/zstd-MVmsxGT0.js:134-145 record the exact zstddec
version, preserve/surface the Zstandard BSD license in plugin distribution
metadata, and either generate this output during the build or retain the
documented vendoring; in
plugins/lst-super-resolution/basedecoder-BEbFOz03.js:1-115 record the geotiff
version for predictor.js and basedecoder.js; in
plugins/lst-super-resolution/globals-B9FiI9ma.js:1-23 record the bundler and
geotiff versions; and in plugins/lst-super-resolution/jpeg-Cld8JONX.js:1-2,
lzw-qEVgi16o.js:1-2, packbits-BkVksIAf.js:1-2, raw-CSK_7147.js:1-2, and
webimage-JdOPiB5x.js:1-2 record their geotiff origin. For
plugins/lst-super-resolution/deflate-jqWMVTm5.js:1-2 also record the bundled
pako license. If source builds are available, replace committed artifacts with
build-generated chunks.
🪄 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: 92113681-6089-4abe-9eef-40827f7ec6c2
📒 Files selected for processing (12)
plugins/lst-super-resolution/basedecoder-BEbFOz03.jsplugins/lst-super-resolution/deflate-jqWMVTm5.jsplugins/lst-super-resolution/globals-B9FiI9ma.jsplugins/lst-super-resolution/index.jsplugins/lst-super-resolution/jpeg-Cld8JONX.jsplugins/lst-super-resolution/lerc-DB9wtJoF.jsplugins/lst-super-resolution/lzw-qEVgi16o.jsplugins/lst-super-resolution/packbits-BkVksIAf.jsplugins/lst-super-resolution/pako.esm-Bh1yUGpp.jsplugins/lst-super-resolution/raw-CSK_7147.jsplugins/lst-super-resolution/webimage-JdOPiB5x.jsplugins/lst-super-resolution/zstd-MVmsxGT0.js
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@plugins/lst-super-resolution/THIRD_PARTY_NOTICES.md`:
- Line 10: Add the required Zlib attribution to THIRD_PARTY_NOTICES.md alongside
the pako dependency entry, including the full Zlib license text and copyright
notice for Jean-loup Gailly and Mark Adler.
- Around line 7-11: Update the bundled dependency inventory in
THIRD_PARTY_NOTICES.md using build metadata’s resolved versions, including a new
lerc entry and the bundled pako@2.2.0 and Zstandard versions rather than
package-lock resolutions. Change pako’s license to “MIT AND Zlib,” and add the
LERC Apache-2.0 notice plus the complete BSD 3-Clause Zstandard license text
with all required copyright notices.
- Around line 15-25: Add the missing bundled third-party license notices and
attributions to THIRD_PARTY_NOTICES.md: include copyright notices for geotiff,
pako, and zstddec; pako’s required Zlib license and attribution; the complete
zstddec BSD-3-Clause text with clause 3 and the Zstandard copyright notice; and
the Apache 2.0 notice for bundled Esri LERC code. Preserve the existing notice
formatting.
🪄 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: 1e2e37e5-3bfd-4e0a-a8ba-9682b1c1f814
📒 Files selected for processing (1)
plugins/lst-super-resolution/THIRD_PARTY_NOTICES.md
Removed extensive comments and styles for plugin control, and added third-party notices and license information.
Updated the version of Vite and added new bundled files to the third-party notices. Expanded the Apache License section with detailed definitions and terms.
|
The plugin failed CI tests. Can you fix them? |
2 participants
Summary by CodeRabbit
New Features
lst-super-resolutionplugin for enhanced land-surface temperature imagery.Documentation