Sightline: editable top times, cancellable sweeps, multi-DEM, native-resolution sizing

[devin-ai-integration]

Purpose

Four improvements to the Sightline Tool: make the shared top-section Start/End times directly editable, allow cancelling a running sweep, support selecting from multiple DEMs, and drive the working resolution from each DEM's real dataset resolution (meters-per-pixel from its GeoTransform) instead of screen-relative multipliers.

Proposed Changes

1. Editable top-section Start/End times

• [CHANGE] SightlinePanel.jsx: replaced the read-only Start/End <span>s with editable UTC <input>s, synced from sweepStart/sweepEnd. On blur/Enter they commit through the same shared helpers the per-item observer-local inputs use.
• [ADD] SightlineTool.js: normalizeUTCTime(), applySweepStartTime(), applySweepEndTime() — shared validation + global timeline/sweep-time update logic. Zoneless input is treated as UTC; invalid input reverts.

2. Cancel sweeps

• [ADD] Client-side AbortController per element (_sweepAbortControllers); the sweep fetch passes signal, and abort is handled in catch/finally.
• [ADD] SightlineTool.cancelSweepElement(elmId) / _abortSweepRequests(elmId); cancelSweep() now aborts in-flight requests first.
• [CHANGE] SightlineElement.jsx: while a sweep runs the button becomes a red Cancel (n%) button.
• [FIX] sightmap.py: per-frame streaming loop now catches BrokenPipeError/IOError on stdout/stderr writes and stops cleanly when the client aborts.

3. Multiple DEMs

• [ADD] plugin.json: new dems objectarray (name, path, optional resolution) alongside the legacy single dem field (mirrors MeasureTool.layerDems).
• [ADD] store.js buildDemsList(vars): builds the DEM list with backward-compatible fallback to the legacy dem.
• [ADD] SightlineElement.jsx: per-item DEM selector (shown only when >1 DEM configured).
• [ADD] SightlineTool.js helpers: getElementDem, getElementDemUrl, getElementNativeResolution, fetchElementDemInfo; the selected DEM URL is threaded into the sightmap call and (via SightlineTool_Horizon.js) the horizonprofile call.
• [ADD] Backend routes/deminfo.js + scripts/deminfo.py (POST /api/sightline/deminfo): reads a DEM's GeoTransform and returns its native meters-per-pixel; registered in plugin.js; client call added in calls.js.

4. Native resolution = dataset resolution

• [CHANGE] SightlineTool._resolutionToMaxDim(): computes maxOutputDim from target meters-per-pixel ÷ viewport ground extent (new _getViewportGroundExtentMeters()), clamps requests finer than native to native, and falls back to the legacy relative scales when the native resolution is unknown.
• [CHANGE] SightlineElement.jsx: Resolution options now show real m/px (Native, 2×, 4×, 8×) derived from the DEM's native resolution; legacy relative scales remain as fallback.

Docs

• [CHANGE] public/helps/SightlineTool.md and docs/pages/Tools/Sightline/Sightline.md: documented editable times, cancel, multi-DEM config (dem/dems), the meters-per-pixel resolution behavior, and the new deminfo endpoint.

Issues

• N/A

Testing

• Extended tests/unit/sightlineTool.spec.js with 21 unit tests (buildDemsList backward-compat + multi-DEM, normalizeUTCTime, native pixel-scale for projected/geographic DEMs, resolution-option generation, and maxOutputDim-from-dataset sizing incl. clamping/fallback). Full suite: 775 passed via npm run test:unit.
• npx eslint on the changed JS/JSX: no new errors (only pre-existing no-undef for the Leaflet L global and pre-existing unused-var warnings).
• Verified all three reference mission configs (Mars, Lunar-SouthPole, Reference-Mission) load through buildDemsList for both the as-configured single-dem shape and an injected multi-dems shape.

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/83f45496202e4e8c8119ee82aeafadee
Requested by: @tariqksoliman

[devin-ai-integration]

Original prompt from tariq.k.soliman

Work in the JPL-Devin/MMGIS repo (branch development) on the Sightline Tool. The relevant source files are NOT in the Ask index, so first locate them on disk. Start by finding the plugin implementation (registered as "js": "SightlineTool" in the mission configs) and its algorithm module SightlineTool_Algorithm — search under src/essence/Tools/ (and check tests/unit/sightlineTool.spec.js which inlines algorithm logic and references the real module path). Also locate the backend route for /api/sightline/sightmap (search server routes for sightmap/sightline; src/pre/calls.js has the client call definitions). Read public/helps/SightlineTool.md and docs/pages/Tools/Sightline/Sightline.md for the current documented behavior.

Implement these four improvements:

1. Edit start/end time in the top section: Currently the shared Time section at the top shows Start Time, End Time, and Step Size (connected to the MMGIS timeline), but editable Start/End inputs only appear inside a per-sightline-item once an observer is selected (observer-local, with UTC↔local round-trip conversion on blur). Make the top-section Start Time and End Time directly editable, reusing the same validation/conversion and global sweep-time update logic that the per-item observer-local inputs already use.

2. Cancel sweeps: A sweep runs the time-range analysis across all timesteps via the batch params (startTime, endTime, stepSeconds) with per-frame progress streaming. There is currently no cancel mechanism. Add client-side cancellation via an AbortController (or equivalent) so the Sweep button toggles to a Cancel state while running and aborts the in-flight request(s); ensure the backend per-frame streaming loop stops cleanly when the request is aborted.

3. Support multiple DEMs: The config exposes a single dem string and the sightmap dem API param is a single path. Extend the config schema to accept a list of DEMs (each with name + path, and ideally native resolution), sur... (1357 chars truncated...)

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR that start with 'DevinAI' or '@devin'.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[github-actions]

✅ Version Already Updated

This PR includes a manual version update to 5.2.18-20260709

No automatic version bump needed.

[devin-ai-integration]

Devin Review found 2 new potential issues.

[devin-ai-integration]

🔍 Blueprint configs reuse the same DEM file path for entries with different resolutions

In all three blueprint config files, the dems array contains two entries that point to the same physical DEM file but declare different resolution values. For example, config.reference-mission-lunar-southpole.json:249-256 lists LRO_LOLA_DEM_4000m.tif twice — once at 4000 m/px and once at 8000 m/px. Similarly config.reference-mission-mars.json:171-178 and config.reference-mission.json:4175-4182. The resolution field is a metadata hint (the DEM's native ground sample distance) used to cap the output grid and display the effective working resolution. Declaring a different resolution for the same file is misleading — the DEM's actual pixel spacing doesn't change. If these are placeholders to demonstrate the multi-DEM dropdown, a comment or distinct file paths would be clearer. If they're meant to model different downsampled views of the same terrain, the current implementation doesn't actually downsample the DEM read — it only caps the output grid dimension.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Intentional — these are placeholder entries to demonstrate the multi-DEM dropdown. The repo owner explicitly asked for each reference mission to have two DEM entries both pointing at the same file with distinct resolution values ("Both dem paths in each ref mission should just point to the same dem since its easier"). As the comment notes, resolution only caps the output grid dimension / drives the effective-resolution readout; it does not resample the DEM read, so the second entry simply models a coarser working cap over the same terrain. Leaving as-is per that request.

[devin-ai-integration]

🔍 Editable time inputs strip seconds, potentially losing precision on round-trip

The stripSeconds helper in SightlinePanel.jsx:17-18 removes seconds from the displayed time (e.g., 2025-01-02T03:04:05Z → 2025-01-02T03:04Z). When the user commits the input without editing, applySweepStartTime at SightlineTool.js:2058 calls normalizeUTCTime which appends Z if missing and parses with new Date(). new Date('2025-01-02T03:04Z') is valid and sets seconds to 00. So a time originally at :30 seconds would silently become :00 if the user simply clicks into and out of the field. The onBlur handler always fires handleStartCommit/handleEndCommit, but normalizeUTCTime returns the stripped string (not the original store value), so the store gets overwritten with the seconds-truncated version. This is a minor precision loss that only matters if the user interacts with the field without intending to change it.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Intentional trade-off per the repo owner's request to make the top times "look exactly as they did before this PR, just be editable" and to drop seconds so the inputs are less wide. The minute-granularity display matches the pre-PR read-only rendering; sweep step sizes here are on the order of minutes, so committing without editing snapping :30 → :00 is an acceptable minor precision loss for this tool. If sub-minute precision on untouched blur becomes a concern, the fix would be to only re-commit when the field value actually changed — happy to add that if desired.

[github-actions]

🤖 Version Auto-Bumped

The version has been automatically incremented to 5.2.18-20260709

This commit was added to your PR branch. When you merge this PR, the new version will be included.

---

_{If you want a different version, update package.json manually and push to this PR.}

[devin-ai-integration]

Devin Review found 1 new potential issue.

[devin-ai-integration]

🟡 Reference mission demo configs list two DEMs that point to the same file but claim different resolutions

The second DEM entry in each reference mission config reuses the same file path as the first but declares a different (coarser) native resolution (config.reference-mission-lunar-southpole.json:254), so selecting it produces an artificially degraded sightline map and a misleading "Native" readout in the UI.

Impact: Users choosing the second DEM option in any reference mission get unnecessarily coarse results and incorrect resolution labels.

Copy-paste error across all three reference mission configs

All three configs exhibit the same pattern — the second dems entry duplicates the first entry's path but changes only name and resolution:

• blueprints/Missions/Reference-Mission-Lunar-SouthPole/config.reference-mission-lunar-southpole.json:248-256: both entries use Data/DEMs/LRO_LOLA_DEM_4000m.tif, but the second claims resolution: 8000.
• blueprints/Missions/Reference-Mission-Mars/config.reference-mission-mars.json:170-178: both entries use Data/DEMs/missionDEM_20m_COG.tif, but the second claims resolution: 40.
• blueprints/Missions/Reference-Mission/config.reference-mission.json:4174-4182: both entries use Data/DEMs/USGS_13_n38w123_20250826_SFHill.tif, but the second claims resolution: 20.

The resolution field is consumed by SightlineTool.getElementNativeResolution() (SightlineTool.js:2128-2133) and used to cap _resolutionToMaxDim() (SightlineTool.js:2345-2352). A falsely coarse resolution value caps the output grid to fewer pixels than the DEM actually supports, producing a blurrier sightline map than necessary. The UI also displays this wrong value as "Native: 8000 m/px" via SightlineElement.jsx:677.

The second entries should either point to genuinely different DEM files or, if only one file exists, be removed.

Prompt for agents

All three reference mission configs (Lunar SouthPole, Mars, and Reference-Mission) have a second DEM entry in the new `dems` array that is a copy-paste of the first entry with only the name and resolution changed — the `path` still points to the same file. This means the second option is functionally identical to the first but with a wrong resolution cap.

Either:
1. Point the second entry's `path` to a genuinely different DEM file (if one exists in the mission data), or
2. Remove the second entry entirely if only one DEM file is available for the demo.

Affected files:
- blueprints/Missions/Reference-Mission-Lunar-SouthPole/config.reference-mission-lunar-southpole.json (lines 253-256)
- blueprints/Missions/Reference-Mission-Mars/config.reference-mission-mars.json (lines 175-178)
- blueprints/Missions/Reference-Mission/config.reference-mission.json (lines 4179-4182)

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

This is intentional, not a copy-paste error. The maintainer explicitly requested each reference mission's Sightline have two DEM entries pointing at the same file with different resolution values ("Both dem paths in each ref mission should just point to the same dem since its easier") — the goal is to exercise/demo the multi-DEM selector and native-resolution readout without shipping a second large DEM file. Leaving as-is.