From d017d90392794ad6cfb1165aab06018f11c06476 Mon Sep 17 00:00:00 2001 From: dfguerrerom Date: Mon, 22 Jun 2026 20:01:14 +0200 Subject: [PATCH 1/4] fix: prevent OOM from dense custom AOIs and rework sub-AOI handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dense AOI (e.g. an Indonesia-scale asset, ~2.45M edges) was materialised client-side, growing the solara process to ~2.5GB and OOM-killing it on the 4GB SEPAL box. Root cause: importing/selecting an AOI pulled the full FeatureCollection geometry into Python (get_info -> GeoDataFrame -> __geo_interface__ -> ee.serializer). Geometry handling - aoi_geometry: add `simplify_fc` (per-feature server-side simplification, no dissolve) and `fc_from_source` (rebuild the EXACT geometry server-side from a small descriptor: asset id / admin code). Nothing dense reaches the client. - import/admin AOI: async via create_task; pull only a SIMPLIFIED outline for display, store a `source` descriptor on each feature for analysis + tiles. - Decouple display from analysis: `get_ee_features` rebuilds the exact geometry from `source` (asset/admin) so dashboard stats stay exact while display stays cheap. Custom sub-AOI rendering - Draw the exact outline as a single merged EE tile layer (FeatureCollection.style); no client-side GeoJSON layer. The simplified geometry collapsed small features into Point/Line/GeometryCollection parts that ipyleaflet rendered as stray markers and that broke hover. - Hover name label is driven from the cursor against a cached geometry, so it no longer depends on a transparent GeoJSON hit-area. - Containment check uses the exact geometry (via `source`) and a relative tolerance (1% of area) instead of a fixed 1 m^2 — independent datasets never align perfectly, which produced false "outside" verdicts. UX - Remove the "Vector file" (SHAPE) upload option from the main AOI and the import dialog; users supply geometry as a GEE asset. - Zoom to a freshly added sub-AOI (async, exact bounds). - Add an eye/zoom action to the custom-geometries table (zoom + close). Diagnostics - Add opt-in memory diagnostics (RSS / tracemalloc / session counts), DISABLED by default; enable with SEPLAN_MEM_DIAG=1. Requires pysepal>=3.6.2 (the column dropdown now reads propertyNames instead of pulling the whole first feature with its geometry). --- component/frontend/icons.py | 5 +- component/model/aoi_model.py | 7 +- component/scripts/aoi_geometry.py | 85 +++- component/scripts/mem_diagnostics.py | 427 +++++++++++++++++++ component/widget/admin_aoi_dialog.py | 16 +- component/widget/custom_aoi_dialog.py | 80 +++- component/widget/custom_aoi_view.py | 4 +- component/widget/custom_geometries_dialog.py | 31 +- component/widget/import_aoi_dialog.py | 132 ++++-- component/widget/map.py | 235 +++++----- pyproject.toml | 2 +- solara_app.py | 28 +- 12 files changed, 870 insertions(+), 182 deletions(-) create mode 100644 component/scripts/mem_diagnostics.py diff --git a/component/frontend/icons.py b/component/frontend/icons.py index ce24a609..106ced6d 100644 --- a/component/frontend/icons.py +++ b/component/frontend/icons.py @@ -51,6 +51,10 @@ "fa": "fa-solid fa-trash", "mdi": "mdi-trash-can", }, + "eye": { + "fa": "fa-solid fa-eye", + "mdi": "mdi-eye", + }, "draw": { "fa": "fa-solid fa-draw-polygon", "mdi": "mdi-draw", @@ -96,5 +100,4 @@ def icon(icon: str, lib: str = "mdi") -> str: """Return the icon class.""" - return icons[icon][lib] diff --git a/component/model/aoi_model.py b/component/model/aoi_model.py index 97fa332f..37fb21c5 100644 --- a/component/model/aoi_model.py +++ b/component/model/aoi_model.py @@ -18,7 +18,7 @@ from traitlets import Any, Bool, Dict, Int import component.parameter as cp -from component.scripts.aoi_geometry import _aoi_bbox +from component.scripts.aoi_geometry import _aoi_bbox, fc_from_source logger = logging.getLogger("SEPLAN") @@ -507,9 +507,12 @@ def get_ee_features(self) -> Tuple[DictType[str, AnyType], DictType[str, AnyType } } + # Analysis uses the EXACT geometry, rebuilt server-side from each + # feature's ``source`` descriptor (asset id / admin code). The geometry + # stored in ``custom_layers`` is only a simplified display copy. custom_aois = { feat["properties"]["name"]: { - "ee_feature": su.geojson_to_ee(feat), + "ee_feature": fc_from_source(feat["properties"].get("source"), feat), "color": feat["properties"]["style"]["color"], } for feat in self.custom_layers["features"] diff --git a/component/scripts/aoi_geometry.py b/component/scripts/aoi_geometry.py index b67729d8..3d2dca4b 100644 --- a/component/scripts/aoi_geometry.py +++ b/component/scripts/aoi_geometry.py @@ -6,7 +6,7 @@ (only ``ee``) so every module can import them at the top level. """ -from typing import Union +from typing import Optional, Union import ee @@ -21,3 +21,86 @@ def _aoi_bbox(aoi: Union[ee.FeatureCollection, ee.Geometry]) -> ee.Geometry: """ fc = ee.FeatureCollection(aoi) return fc.map(lambda feat: ee.Feature(feat.geometry().bounds())).geometry().bounds() + + +# Display simplification tolerance (meters). Dense AOIs (millions of vertices) +# are simplified server-side to a low-vertex outline so only a tiny geometry is +# ever pulled client-side for the map + hover label. The analysis still runs on +# the full-resolution server-side ``ee.FeatureCollection``. +DISPLAY_SIMPLIFY_MAX_ERROR = 1000.0 + + +def simplify_fc( + aoi: Union[ee.FeatureCollection, ee.Geometry], + max_error: float = DISPLAY_SIMPLIFY_MAX_ERROR, + dissolve: bool = False, +) -> ee.FeatureCollection: + """Per-feature, server-side geometry simplification for client display. + + Materialising a dense AOI client-side (``get_info`` -> GeoDataFrame -> + ``__geo_interface__`` -> ``ee.serializer``) pulls millions of vertices into + native + Python memory and OOM-kills the process. Simplifying each feature on + the server first means ``get_info`` only ever transfers a low-vertex outline. + + Per-feature simplification (not ``Collection.geometry().simplify``) avoids + EE's 2M-edge dissolve limit on dense collections. + + Args: + aoi: source collection / geometry (kept server-side, never downloaded). + max_error: simplification tolerance in meters. + dissolve: merge the already-simplified (low-edge) features into a single + geometry — safe because simplification runs first, so the union stays + well under EE's 2M-edge limit. + + Returns: + An ``ee.FeatureCollection`` with simplified geometry, safe to materialise + via ``get_info``. + """ + fc = ee.FeatureCollection(aoi) + simplified = fc.map( + lambda feat: feat.setGeometry(feat.geometry().simplify(maxError=max_error)) + ) + if dissolve: + merged = simplified.geometry(maxError=max_error) + return ee.FeatureCollection([ee.Feature(merged)]) + return simplified + + +def fc_from_source(source: Optional[dict], feat: dict) -> ee.FeatureCollection: + """Rebuild the EXACT server-side FeatureCollection for a custom sub-AOI. + + ``custom_layers`` stores a *simplified* display geometry (so dense AOIs never + sit in client memory). Analysis and tile rendering need the full-resolution + geometry, so we reconstruct it server-side from a small ``source`` descriptor + instead of from the simplified geojson: + + * ``{"type": "asset", "id", "column", "value"}`` -> ``ee.FeatureCollection`` + (optionally filtered to one feature) — nothing is downloaded. + * ``{"type": "admin", "code"}`` -> ``pygaul.AdmItems`` — nothing is downloaded. + * no/unknown descriptor -> ``geojson_to_ee(feat)``, the stored geojson. DRAW + sub-AOIs are exact and tiny, so this is correct for them; any other path + degrades gracefully to the (possibly simplified) displayed geometry. + + Args: + source: the descriptor stored on the feature's ``properties["source"]``. + feat: the GeoJSON feature (fallback geometry source). + + Returns: + A server-side ``ee.FeatureCollection`` at full resolution. + """ + if source: + kind = source.get("type") + if kind == "asset" and source.get("id"): + fc = ee.FeatureCollection(source["id"]) + column, value = source.get("column", "ALL"), source.get("value") + if column and column != "ALL" and value is not None: + fc = fc.filter(ee.Filter.eq(column, value)) + return fc + if kind == "admin" and source.get("code"): + import pygaul + + return pygaul.AdmItems(admin=source["code"]) + + from sepal_ui.scripts import utils as su + + return ee.FeatureCollection(su.geojson_to_ee(feat)) diff --git a/component/scripts/mem_diagnostics.py b/component/scripts/mem_diagnostics.py new file mode 100644 index 00000000..413bb810 --- /dev/null +++ b/component/scripts/mem_diagnostics.py @@ -0,0 +1,427 @@ +"""Non-invasive memory diagnostics for the se.plan Solara process. + +Purpose +------- +The ``solara`` process was OOM-killed twice on a 4 GiB box after its +anonymous heap reached ~2.5 GB (see ``journal-crash.txt.gz``). This module +adds lightweight, behaviour-preserving instrumentation to confirm *which* +mechanism drives the climb: + +* a periodic one-line sample of resident memory and live-object counts + (Solara kernels, pysepal sessions, ipywidgets), so the growth can be + correlated with user actions; +* ``tracemalloc`` peak tracking so transient spikes that are freed between + samples are still caught; +* an automatic detailed top-N allocation dump whenever RSS jumps sharply or + crosses a danger threshold — this captures the smoking gun (e.g. a dense + AOI import materialising full geometry client-side) without anyone having + to be watching; +* an on-demand dump via ``SIGUSR1`` (``kill -USR1 `` inside the + container). + +Everything is guarded so a failure in a probe can never crash the app. + +**Disabled by default**: the whole thing (including ``probe()``) is a no-op +unless ``SEPLAN_MEM_DIAG=1`` is set in the environment (e.g. via ``.env``). + +Configuration (environment variables) +------------------------------------- +==================================== ========= ====================================== +Variable Default Meaning +==================================== ========= ====================================== +``SEPLAN_MEM_DIAG`` ``0`` master switch — set ``1`` to enable +``SEPLAN_MEM_INTERVAL`` ``30`` seconds between periodic samples +``SEPLAN_TRACEMALLOC`` ``1`` enable ``tracemalloc`` (small overhead) +``SEPLAN_TRACEMALLOC_FRAMES`` ``10`` stack depth kept per allocation +``SEPLAN_MEM_JUMP_MB`` ``150`` RSS jump (MB) that triggers a dump +``SEPLAN_MEM_DANGER_MB`` ``1500`` RSS (MB) above which we WARN + dump +``SEPLAN_MEM_TOP`` ``30`` allocations listed in a detailed dump +``SEPLAN_MEM_LOG`` *(auto)* log file path override +==================================== ========= ====================================== + +Output goes to ``~/module_results/se.plan/mem_diagnostics.log`` (rotating) +*and* to the ``SEPLAN`` logger (stdout / journald), prefixed ``MEMDIAG`` for +easy grepping. +""" + +from __future__ import annotations + +import logging +import os +import threading +import tracemalloc +from logging.handlers import RotatingFileHandler +from pathlib import Path +from typing import Optional + +logger = logging.getLogger("SEPLAN") + +# --------------------------------------------------------------------------- +# module-global state (one diagnostics thread per process) +# --------------------------------------------------------------------------- +_started = False +_enabled = False # set True only when SEPLAN_MEM_DIAG=1 (opt-in); gates probe() +_lock = threading.Lock() +_wake = threading.Event() +_dump_requested = False +_diag_logger: Optional[logging.Logger] = None +_proc = None # cached psutil.Process if available + +# Registry references resolved ONCE in the main thread at start-up. The hot +# loop must not import solara/pysepal/ipywidgets itself: cold-importing them +# from the daemon thread while tracemalloc is active is pathologically slow +# (every allocation during import gets traced), which would stall sampling. +_solara_contexts = None # solara.server.kernel_context.contexts (dict) +_pysepal_sm = None # the SessionManager class (live dict is on its singleton) +_ipw_instances = None # ipywidgets.widgets.widget._instances (dict) + + +def _env_flag(name: str, default: str = "1") -> bool: + return os.environ.get(name, default).strip().lower() not in ("0", "false", "no", "") + + +def _env_int(name: str, default: int) -> int: + try: + return int(os.environ.get(name, str(default))) + except (TypeError, ValueError): + return default + + +def _log_path() -> Path: + """Resolve the diagnostics log file path, falling back gracefully.""" + override = os.environ.get("SEPLAN_MEM_LOG") + if override: + return Path(override).expanduser() + try: + from component.parameter.directory import result_dir + + return Path(result_dir) / "mem_diagnostics.log" + except Exception: + return Path( + "~", "module_results", "se.plan", "mem_diagnostics.log" + ).expanduser() + + +def _build_diag_logger() -> logging.Logger: + """Dedicated rotating-file logger so the samples survive a restart.""" + diag = logging.getLogger("SEPLAN.memdiag") + diag.setLevel(logging.INFO) + diag.propagate = False # don't double-print through the root SEPLAN logger + if not diag.handlers: + try: + path = _log_path() + path.parent.mkdir(parents=True, exist_ok=True) + handler = RotatingFileHandler( + path, maxBytes=5_000_000, backupCount=3, encoding="utf-8" + ) + handler.setFormatter( + logging.Formatter("%(asctime)s %(levelname)s %(message)s") + ) + diag.addHandler(handler) + logger.info("MEMDIAG writing to %s", path) + except Exception as e: # pragma: no cover - filesystem dependent + logger.warning("MEMDIAG could not open log file: %s", e) + return diag + + +def _emit(msg: str, level: int = logging.INFO) -> None: + """Send a line to both the dedicated file and stdout/journald.""" + if _diag_logger is not None: + _diag_logger.log(level, msg) + logger.log(level, "MEMDIAG %s", msg) + + +# --------------------------------------------------------------------------- +# metric collectors (each fully guarded; -1 means "could not read") +# --------------------------------------------------------------------------- +def _rss_mb() -> float: + """Resident set size in MB via psutil, /proc, then getrusage.""" + global _proc + try: + import psutil + + if _proc is None: + _proc = psutil.Process() + return _proc.memory_info().rss / 1024 / 1024 + except Exception: + pass + try: + for line in open("/proc/self/status"): + if line.startswith("VmRSS:"): + return int(line.split()[1]) / 1024 # value is in kB + except Exception: + pass + try: + import resource + + # ru_maxrss is kB on Linux (high-water mark, not current) + return resource.getrusage(resource.RUSAGE_SELF).ru_maxrss / 1024 + except Exception: + return -1.0 + + +def _vms_mb() -> float: + try: + import psutil + + global _proc + if _proc is None: + _proc = psutil.Process() + return _proc.memory_info().vms / 1024 / 1024 + except Exception: + return -1.0 + + +def _resolve_registries() -> None: + """Resolve live-object registries once, in the main thread, at start-up. + + By the time this runs (from ``solara_app.py`` after ``setup_solara_server``) + solara, pysepal and ipywidgets are already imported, so this is cheap; it + just caches the dict/class objects the hot loop counts. + """ + global _solara_contexts, _pysepal_sm, _ipw_instances + try: + from solara.server import kernel_context + + _solara_contexts = kernel_context.contexts + except Exception as e: + logger.warning("MEMDIAG could not resolve solara contexts: %s", e) + try: + from pysepal.solara.session_manager import SessionManager + + _pysepal_sm = SessionManager + except Exception as e: + logger.warning("MEMDIAG could not resolve SessionManager: %s", e) + try: + from ipywidgets.widgets import widget as _w + + # module-level backing dict (the non-deprecated registry in ipywidgets 8.1) + _ipw_instances = _w._instances + except Exception as e: + logger.warning("MEMDIAG could not resolve ipywidgets registry: %s", e) + + +def _count_solara_kernels() -> int: + return len(_solara_contexts) if _solara_contexts is not None else -1 + + +def _count_pysepal_sessions() -> int: + # The live dict is the instance attr on the singleton (set in __init__); + # the class attr is a stale empty {} once initialised. Read the instance, + # falling back to the class attr before the singleton exists. + sm = _pysepal_sm + if sm is None: + return -1 + try: + inst = getattr(sm, "_instance", None) + sessions = getattr(inst, "_sessions", None) if inst is not None else None + if sessions is None: + sessions = getattr(sm, "_sessions", None) + return len(sessions) if sessions is not None else -1 + except Exception: + return -1 + + +def _count_ipywidgets() -> int: + return len(_ipw_instances) if _ipw_instances is not None else -1 + + +# --------------------------------------------------------------------------- +# detailed tracemalloc dump +# --------------------------------------------------------------------------- +def _dump_top(reason: str, rss: float) -> None: + """Write the top allocators (by line, plus the single biggest traceback).""" + if not tracemalloc.is_tracing(): + _emit(f"SNAPSHOT skipped (tracemalloc off) reason={reason} rss={rss:.0f}MB") + return + top = _env_int("SEPLAN_MEM_TOP", 30) + try: + snapshot = tracemalloc.take_snapshot() + except Exception as e: + _emit(f"SNAPSHOT failed: {e}", logging.WARNING) + return + + lines = [f"==== SNAPSHOT reason={reason} rss={rss:.0f}MB top={top} ===="] + for i, stat in enumerate(snapshot.statistics("lineno")[:top], 1): + frame = stat.traceback[0] + lines.append( + f"#{i:>2} {stat.size / 1024 / 1024:8.2f} MB " + f"{stat.count:>7} blocks {frame.filename}:{frame.lineno}" + ) + + # full call chain of the single biggest allocation site — this is what + # reveals e.g. import_aoi_dialog -> get_ipygeojson -> .gdf -> _load_gdf + big = snapshot.statistics("traceback") + if big: + lines.append("---- biggest allocation traceback ----") + lines.append(f" ({big[0].size / 1024 / 1024:.2f} MB, {big[0].count} blocks)") + lines.extend(" " + ln for ln in big[0].traceback.format()) + _emit("\n".join(lines)) + + +# --------------------------------------------------------------------------- +# the sampling loop +# --------------------------------------------------------------------------- +def _loop(interval: int, jump_mb: int, danger_mb: int) -> None: + global _dump_requested + last_rss = _rss_mb() + # high-water band so the long climb to the OOM also gets snapshots + next_band = ((int(last_rss) // 500) + 1) * 500 + + _emit(f"started interval={interval}s rss={last_rss:.0f}MB") + + while True: + # wakes early on SIGUSR1 (handler sets the event), else ticks on timeout + _wake.wait(timeout=interval) + _wake.clear() + + rss = _rss_mb() + tm_cur = tm_peak = -1.0 + if tracemalloc.is_tracing(): + try: + cur, peak = tracemalloc.get_traced_memory() + tm_cur, tm_peak = cur / 1024 / 1024, peak / 1024 / 1024 + tracemalloc.reset_peak() # so peak is per-interval, catches spikes + except Exception: + pass + + delta = rss - last_rss if (rss >= 0 and last_rss >= 0) else 0.0 + level = logging.WARNING if (danger_mb and rss >= danger_mb) else logging.INFO + _emit( + f"rss={rss:.0f}MB d={delta:+.0f}MB vms={_vms_mb():.0f}MB " + f"tm_cur={tm_cur:.0f}MB tm_peak={tm_peak:.0f}MB " + f"kernels={_count_solara_kernels()} " + f"sessions={_count_pysepal_sessions()} widgets={_count_ipywidgets()}", + level, + ) + + # automatic detailed dumps ------------------------------------------------- + if _dump_requested: + _dump_requested = False + _dump_top("SIGUSR1", rss) + if jump_mb and delta >= jump_mb: + _dump_top(f"RSS_JUMP {delta:+.0f}MB", rss) + if danger_mb and rss >= danger_mb: + _dump_top("DANGER", rss) + elif rss >= next_band: + _dump_top(f"BAND_{next_band}MB", rss) + next_band += 500 + + if rss >= 0: + last_rss = rss + + +def _handle_sigusr1(signum, frame) -> None: # pragma: no cover - signal path + global _dump_requested + _dump_requested = True + _wake.set() + + +# --------------------------------------------------------------------------- +# public entry point +# --------------------------------------------------------------------------- +def start_memory_diagnostics() -> None: + """Start the background memory-diagnostics thread (idempotent, safe). + + Call once at process start (from ``solara_app.py``). Honours the + ``SEPLAN_MEM_*`` environment variables documented in the module docstring. + **Disabled by default** — a no-op unless ``SEPLAN_MEM_DIAG=1`` is set (or if + already started). + """ + global _started, _enabled, _diag_logger + with _lock: + if _started: + return + if not _env_flag("SEPLAN_MEM_DIAG", "0"): + return + _started = True + _enabled = True + + _diag_logger = _build_diag_logger() + + # Resolve registries in the main thread BEFORE tracemalloc starts. + _resolve_registries() + + if _env_flag("SEPLAN_TRACEMALLOC"): + try: + if not tracemalloc.is_tracing(): + tracemalloc.start(_env_int("SEPLAN_TRACEMALLOC_FRAMES", 10)) + logger.info( + "MEMDIAG tracemalloc on (frames=%s)", + _env_int("SEPLAN_TRACEMALLOC_FRAMES", 10), + ) + except Exception as e: + logger.warning("MEMDIAG tracemalloc failed to start: %s", e) + + # SIGUSR1 -> on-demand dump (must register on the main thread) + try: + import signal + + signal.signal(signal.SIGUSR1, _handle_sigusr1) + logger.info("MEMDIAG SIGUSR1 dump handler installed (kill -USR1 )") + except Exception as e: # not the main thread / unsupported platform + logger.warning("MEMDIAG could not install SIGUSR1 handler: %s", e) + + interval = _env_int("SEPLAN_MEM_INTERVAL", 30) + jump_mb = _env_int("SEPLAN_MEM_JUMP_MB", 150) + danger_mb = _env_int("SEPLAN_MEM_DANGER_MB", 1500) + + thread = threading.Thread( + target=_loop, + args=(interval, jump_mb, danger_mb), + name="seplan-memdiag", + daemon=True, + ) + thread.start() + logger.info("MEMDIAG sampler thread started (interval=%ss)", interval) + + +# --------------------------------------------------------------------------- +# targeted probe — wrap a suspect block to log RSS / peak before & after +# --------------------------------------------------------------------------- +class probe: + """Context manager logging RSS and tracemalloc peak around a block. + + Use it to get an unambiguous per-operation delta on a suspect path, e.g.:: + + with probe("aoi-import-extract-geometry"): + feature_collection = self.model.gdf.__geo_interface__ + + Cheap and exception-safe; logs even if the block raises. + """ + + def __init__(self, label: str): + self.label = label + self._rss0 = -1.0 + + def __enter__(self) -> "probe": + """Record RSS and reset the tracemalloc peak before the block runs.""" + if not _enabled: # diagnostics off -> no-op + return self + self._rss0 = _rss_mb() + if tracemalloc.is_tracing(): + try: + tracemalloc.reset_peak() + except Exception: + pass + _emit(f"PROBE start [{self.label}] rss={self._rss0:.0f}MB") + return self + + def __exit__(self, exc_type, exc, tb) -> bool: + """Log the RSS delta and tracemalloc peak; never swallow exceptions.""" + if not _enabled: # diagnostics off -> no-op + return False + rss1 = _rss_mb() + peak = -1.0 + if tracemalloc.is_tracing(): + try: + _, peak_b = tracemalloc.get_traced_memory() + peak = peak_b / 1024 / 1024 + except Exception: + pass + status = "raised " + exc_type.__name__ if exc_type else "ok" + _emit( + f"PROBE end [{self.label}] rss={rss1:.0f}MB " + f"d={rss1 - self._rss0:+.0f}MB tm_peak={peak:.0f}MB ({status})" + ) + return False # never swallow exceptions diff --git a/component/widget/admin_aoi_dialog.py b/component/widget/admin_aoi_dialog.py index 17132c83..feafd5b0 100644 --- a/component/widget/admin_aoi_dialog.py +++ b/component/widget/admin_aoi_dialog.py @@ -14,6 +14,7 @@ from sepal_ui.scripts.gee_interface import GEEInterface from component.frontend.icons import icon +from component.scripts.aoi_geometry import simplify_fc from component.widget.base_dialog import BaseDialog from component.widget.buttons import TextBtn @@ -232,8 +233,7 @@ def on_submit(self, *_): if not field.v_model: level = self._parent_level + 1 + idx self.alert.add_msg( - f"Please pick an admin level {level} unit before " - "submitting.", + f"Please pick an admin level {level} unit before " "submitting.", type_="error", ) return @@ -272,7 +272,10 @@ async def _resolve_admin_async(self): ``getInfo`` round-trip is slow. """ fc = pygaul.AdmItems(admin=self._admin_code) - return await self.gee_interface.get_info_async(fc) + # Simplify server-side: a dense admin unit (e.g. a country with millions + # of vertices) would otherwise be pulled in full and OOM the kernel. The + # full-resolution geometry stays server-side for analysis. + return await self.gee_interface.get_info_async(simplify_fc(fc)) def _on_resolved(self, geo_json: dict): """Forward the materialized geometry to ``CustomAoiDialog``.""" @@ -294,6 +297,9 @@ def _on_resolved(self, geo_json: dict): feature_collection=geo_json, name=self._admin_text or self._admin_code, skip_containment_check=True, + # rebuild the EXACT geometry from the admin code for analysis + tiles + # (geo_json above is simplified for display only) + source={"type": "admin", "code": self._admin_code}, ) def _on_resolve_error(self, exc: Exception): @@ -307,6 +313,4 @@ def _on_resolve_error(self, exc: Exception): def _show_message(self, text: str): """Render a plain message in place of the selector.""" - self.body_card.children = [ - sw.Html(tag="p", class_="ma-2", children=[text]) - ] + self.body_card.children = [sw.Html(tag="p", class_="ma-2", children=[text])] diff --git a/component/widget/custom_aoi_dialog.py b/component/widget/custom_aoi_dialog.py index 84aa45a2..fd123d46 100644 --- a/component/widget/custom_aoi_dialog.py +++ b/component/widget/custom_aoi_dialog.py @@ -1,5 +1,6 @@ import logging from copy import deepcopy +from typing import Optional import ee from matplotlib import pyplot as plt @@ -9,6 +10,7 @@ import component.parameter as cp from component.frontend.icons import icon from component.message import cm +from component.scripts.aoi_geometry import fc_from_source from component.widget.base_dialog import BaseDialog from component.widget.buttons import TextBtn @@ -17,10 +19,13 @@ logger = logging.getLogger("SEPLAN") -# Tolerance (in m²) for the "outside primary AOI" check. Geoman polygons can -# carry sub-meter rounding error vs. the primary AOI boundary, so anything -# under 1 m² of leakage is treated as inside. -_CONTAINMENT_TOLERANCE_M2 = 1.0 +# A sub-AOI is flagged "outside" only when more than this FRACTION of its area +# falls outside the primary AOI. Independent vector datasets never align exactly +# (coastlines / borders from different sources), so a small relative mismatch is +# expected and tolerated; a geometry that is genuinely (largely) outside still +# trips it. Replaces the old fixed 1 m² tolerance, which an imported asset's +# boundary mismatch (~0.005 % of area) would already exceed. +_CONTAINMENT_FRACTION = 0.01 def _outside_area(child: ee.Geometry, primary_fc: ee.FeatureCollection) -> ee.Number: @@ -42,6 +47,14 @@ def _outside_area(child: ee.Geometry, primary_fc: ee.FeatureCollection) -> ee.Nu return child.area(maxError=1).subtract(covered) +def _outside_fraction( + child: ee.Geometry, primary_fc: ee.FeatureCollection +) -> ee.Number: + """Fraction (0-1) of ``child``'s area lying outside the primary AOI.""" + area = child.area(maxError=1) + return _outside_area(child, primary_fc).divide(area.max(1)) + + class CustomAoiDialog(BaseDialog): feature: dict = None "feature collection of new geometry imported from ImportAoiDialog." @@ -84,6 +97,10 @@ def __init__(self, map_: SeplanMap): # honors it by bypassing the GEE check. self._skip_containment_check = False + # Descriptor (asset id / admin code) carried from on_new_geom to + # on_save_geom so analysis + tiles can rebuild the exact geometry. + self._pending_source = None + # add js behavior btn_cancel.on_event("click", self.on_cancel) self.btn.on_event("click", self.on_save_geom) @@ -126,6 +143,10 @@ def on_save_geom(self, *_): "fillOpacity": 0.4, "weight": 2, } + # Descriptor to rebuild the EXACT geometry server-side for analysis + # + tile rendering (None for drawn geometries — their geojson is + # already exact). The stored geometry itself stays simplified. + feature["properties"]["source"] = getattr(self, "_pending_source", None) self._candidate_features = features self.alert.reset() @@ -136,11 +157,7 @@ def on_save_geom(self, *_): # Skip the GEE round-trip in any of: # - No GEE session / no primary AOI (defensive fallback). # - The admin-sub path opted out (containment is structural). - if ( - self._skip_containment_check - or gee_interface is None - or primary_fc is None - ): + if self._skip_containment_check or gee_interface is None or primary_fc is None: self._commit_save() return @@ -155,17 +172,36 @@ def on_save_geom(self, *_): self._validate_task.start() async def _validate_async(self): - """Return the count of staged features that fall outside the primary AOI.""" + """Return the count of staged sub-AOIs that fall (largely) outside the primary. + + Uses the EXACT geometry, rebuilt server-side from the ``source`` + descriptor (asset id / admin code). The stored geojson is *simplified* + and its distortion produced false "outside" verdicts on near-boundary + features. Drawn geometries have no source, so their exact geojson is used + directly. A relative threshold (``_CONTAINMENT_FRACTION``) tolerates + boundary mismatches between independent datasets. + """ primary_fc = self.map_.aoi_model.feature_collection gee_interface = self.map_.gee_interface + feats = self._candidate_features or [] + + # Import/admin: every candidate feature shares one source, so rebuild and + # check the exact reconstructed FC once. Draw: each exact geojson feature. + source = feats[0]["properties"].get("source") if feats else None + if source: + child = fc_from_source(source, feats[0]).geometry(maxError=1) + frac = await gee_interface.get_info_async( + _outside_fraction(child, primary_fc) + ) + return 1 if (frac is not None and frac > _CONTAINMENT_FRACTION) else 0 outside_count = 0 - for feat in self._candidate_features: + for feat in feats: child = ee.Geometry(feat["geometry"]) - outside_area = await gee_interface.get_info_async( - _outside_area(child, primary_fc) + frac = await gee_interface.get_info_async( + _outside_fraction(child, primary_fc) ) - if outside_area is not None and outside_area > _CONTAINMENT_TOLERANCE_M2: + if frac is not None and frac > _CONTAINMENT_FRACTION: outside_count += 1 return outside_count @@ -202,6 +238,8 @@ def _commit_save(self): current_feats = deepcopy(self.map_.custom_layers) current_feats["features"] += features self.map_.custom_layers = current_feats + # zoom to the freshly added sub-AOI (async, exact geometry) + self.map_.zoom_to_custom(features) self._candidate_features = None self.on_cancel() @@ -220,6 +258,7 @@ def on_cancel(self, *_): self.feature = None self._candidate_features = None self._skip_containment_check = False + self._pending_source = None # Reset transient validation UI self.btn.disabled = False @@ -243,9 +282,10 @@ def open_dialog( def on_new_geom( self, *_, - feature_collection: dict = None, - name: str = None, + feature_collection: Optional[dict] = None, + name: Optional[str] = None, skip_containment_check: bool = False, + source: Optional[dict] = None, ): """Read the aoi and give a default name. @@ -255,11 +295,16 @@ def on_new_geom( Args: feature_collection: Optional GeoJSON ``FeatureCollection`` dict - from the import / admin paths. + from the import / admin paths. Its geometry is SIMPLIFIED (for + display / hover only); analysis uses ``source`` instead. name: Suggested name when ``feature_collection`` is provided. skip_containment_check: If True, ``on_save_geom`` will skip the GEE containment check. Used by the admin-sub path where hierarchy guarantees the geometry sits inside the primary AOI. + source: Descriptor used to rebuild the EXACT geometry server-side + for analysis + tile rendering (``{"type": "asset"|"admin", ...}``). + ``None`` for drawn geometries (their geojson is already exact). + Stored on each feature's ``properties["source"]``. """ # Count the number of geometries in map_.custom_layers index = len(self.map_.custom_layers["features"]) + 1 @@ -271,5 +316,6 @@ def on_new_geom( self.feature = feature_collection self._skip_containment_check = skip_containment_check + self._pending_source = source self.w_name.v_model = aoi_name self.open_dialog(new_geom=True) diff --git a/component/widget/custom_aoi_view.py b/component/widget/custom_aoi_view.py index e86eda79..40064266 100644 --- a/component/widget/custom_aoi_view.py +++ b/component/widget/custom_aoi_view.py @@ -29,7 +29,9 @@ def __init__(self, model: SeplanAoi, app_model=None, **kwargs: dict): self.seplan_aoi = model kwargs.update( { - "methods": ["-POINTS"], + # SHAPE (local vector-file upload) and POINTS are excluded — users + # upload geometries as a GEE asset instead. + "methods": ["-POINTS", "-SHAPE"], "class_": "d-block pa-2 py-4", "model": model.aoi_model, "elevation": 0, diff --git a/component/widget/custom_geometries_dialog.py b/component/widget/custom_geometries_dialog.py index 6e3eac41..015f4809 100644 --- a/component/widget/custom_geometries_dialog.py +++ b/component/widget/custom_geometries_dialog.py @@ -42,9 +42,7 @@ def __init__(self, map_): # new → Geoman on the map, import → ImportAoiDialog). Admin first # because it's the recommended path for admin-based primaries. self.item_admin = self._make_list_item("admin", "Admin sub-area") - self.item_new = self._make_list_item( - "new", cm.map.toolbar.draw_menu["new"] - ) + self.item_new = self._make_list_item("new", cm.map.toolbar.draw_menu["new"]) self.item_import = self._make_list_item( "import", cm.map.toolbar.draw_menu["import"] ) @@ -81,9 +79,7 @@ def _make_list_item(self, action_id: str, label: str) -> v.ListItem: link=True, ripple=True, children=[ - v.ListItemContent( - children=[v.ListItemTitle(children=[label])] - ), + v.ListItemContent(children=[v.ListItemTitle(children=[label])]), ], ) item.on_event("click", self.map_.on_draw) @@ -186,18 +182,39 @@ def __init__(self, map_, layer_id, **kwargs) -> None: name = layer["properties"]["name"] break + self.zoom_btn = cw.TableIcon(icon("eye"), self.layer_id, class_="mr-2") self.delete_btn = cw.TableIcon(icon("trash-can"), self.layer_id) td_list = [ - sw.Html(tag="td", children=[self.delete_btn]), + sw.Html(tag="td", children=[self.zoom_btn, self.delete_btn]), sw.Html(tag="td", children=[name]), ] super().__init__(tag="tr", children=td_list) # add js behaviour + self.zoom_btn.on_event("click", self.on_zoom) self.delete_btn.on_event("click", self.on_delete) + def on_zoom(self, widget, data, event): + """Zoom to this geometry (async, exact) and close the dialog.""" + feature = next( + ( + feat + for feat in self.map_.custom_layers["features"] + if feat["properties"]["id"] == self.layer_id + ), + None, + ) + if feature is None: + return + self.map_.zoom_to_custom([feature]) + # close whichever dialog hosts this table (idempotent on a closed one) + for attr in ("custom_geometries_dialog", "custom_aoi_dialog"): + dialog = getattr(self.map_, attr, None) + if dialog is not None: + dialog.close_dialog() + def on_delete(self, widget, data, event): """Remove the line from the model and trigger table update.""" self.map_.remove_custom_layer(self.layer_id) diff --git a/component/widget/import_aoi_dialog.py b/component/widget/import_aoi_dialog.py index 43504163..faa4e8fd 100644 --- a/component/widget/import_aoi_dialog.py +++ b/component/widget/import_aoi_dialog.py @@ -1,19 +1,22 @@ -from typing_extensions import Self +import logging +from functools import partial -from sepal_ui.aoi.aoi_view import AoiView -from sepal_ui.scripts import decorator as sd from sepal_ui import sepalwidgets as sw +from sepal_ui.aoi.aoi_view import AoiView from sepal_ui.message import ms +from typing_extensions import Self from component.message import cm +from component.scripts.aoi_geometry import simplify_fc +from component.scripts.mem_diagnostics import probe from component.widget.base_dialog import BaseDialog -from sepal_ui.scripts.gee_interface import GEEInterface from component.widget.buttons import TextBtn +logger = logging.getLogger("SEPLAN") + class ImportAoiDialog(BaseDialog): - """Dialog wrapper for AoiView used on the map to import a custom AOI from - the user's assets.""" + """Dialog wrapper for AoiView to import a custom AOI from the user's assets.""" def __init__(self, custom_aoi_dialog, gee_interface=None): super().__init__() @@ -72,53 +75,106 @@ def open_dialog(self, *_, return_to=None): class ImportAoiView(AoiView): - """This class is a wrapper of the AoiModel that aims to not generate - the client geometry when the aoi is selected""" + """Wrap AoiModel so importing an AOI doesn't materialise client geometry.""" def __init__(self, custom_aoi_dialog, gee_interface, **kwargs): - # Admin sub-areas have their own dedicated entry (AdminAoiDialog) — - # exclude ADMIN0/1/2 here to keep the import dialog focused on file - # uploads (SHAPE) and asset references (ASSET). POINTS is also off. - methods = ["-POINTS", "-ADMIN0", "-ADMIN1", "-ADMIN2"] + # Admin sub-areas have their own dedicated entry (AdminAoiDialog) — exclude + # ADMIN0/1/2 here. SHAPE (local vector-file upload) and POINTS are also + # off: users upload via a GEE asset instead, so the import dialog is + # focused on ASSET references. + methods = ["-POINTS", "-ADMIN0", "-ADMIN1", "-ADMIN2", "-SHAPE"] self.elevation = False super().__init__(methods=methods, gee_interface=gee_interface, **kwargs) self.custom_aoi_dialog = custom_aoi_dialog - @sd.loading_button() - def _update_aoi(self, *_) -> Self: - """Load the object in the model & update the map (if possible).""" - # read the information from the geojson data - if self.map_: - self.model.geo_json = self.aoi_dc.to_json() + def _update_aoi(self, *args) -> Self: + """Import the AOI without materialising full geometry client-side. - # update the model - self.model.set_object() + The dense-geometry OOM (and UI freeze) came from pulling the entire + FeatureCollection down — ``get_info`` -> GeoDataFrame -> + ``__geo_interface__`` -> ``ee.serializer``. Instead we keep the AOI + server-side and only pull a SIMPLIFIED outline (via ``simplify_fc``) for + the hover label, off the kernel thread. - # update the map + This view is always GEE-backed and has no ``map_`` of its own — the + imported AOI is rendered through ``map_.custom_layers`` after + ``on_new_geom`` — so there is no synchronous / non-GEE branch. + """ if self.map_: - self.map_.remove_layer("aoi", none_ok=True) - self.map_.zoom_bounds(self.model.total_bounds()) - self.map_.add_layer(self.model.get_ipygeojson(self.map_style)) - - self.aoi_dc.hide() + self.model.geo_json = self.aoi_dc.to_json() - # tell the rest of the apps that the aoi have been updated - self.alert.add_msg(ms.aoi_sel.complete, "success") - self.updated += 1 + self.model.set_object() # builds the server-side ee.FeatureCollection - # Extract the geometry from the model + fc = self.model.feature_collection + if fc is None: + return self + # ASSET: keep the geometry server-side and store a descriptor so analysis + # + tiles rebuild the EXACT FC; only a SIMPLIFIED outline is pulled for + # display/hover. SHAPE: a (bounded) local upload with no server-side + # source — keep its exact geojson so analysis stays exact. if self.model.method == "ASSET": - if self.model.asset_json.get("column", "") == "ALL": - # Dissolve the geometries - self.model.gdf = self.model.gdf.dissolve() + aj = self.model.asset_json or {} + source = { + "type": "asset", + "id": aj.get("pathname"), + "column": aj.get("column", "ALL"), + "value": aj.get("value"), + } + dissolve = aj.get("column", "") == "ALL" + do_simplify = True + else: + source = None + dissolve = False + do_simplify = False + + self._set_loading(True) + # hold a ref so the task isn't GC'd before it runs + self._import_task = self.model.gee_interface.create_task( + func=partial( + self._build_async, fc, self.model.name, dissolve, do_simplify, source + ), + key="import_aoi_build", + on_error=self._on_build_error, + ) + self._import_task.start() + return self - feature_collection = self.model.gdf.__geo_interface__ - name = self.model.name + async def _build_async(self, fc, name, dissolve, do_simplify, source): + """Pull the display outline server-side, then hand off to the rename dialog. - self.custom_aoi_dialog.on_new_geom( - feature_collection=feature_collection, name=name - ) + Runs on the GEE event loop (via ``create_task``) so the kernel thread + stays responsive. For an ASSET the outline is SIMPLIFIED (tiny transfer); + the exact geometry is rebuilt from ``source`` for analysis + tiles. A + SHAPE keeps its exact (bounded) geojson. + """ + try: + display_fc = simplify_fc(fc, dissolve=dissolve) if do_simplify else fc + with probe("aoi-import-display-geojson"): + feature_collection = await self.model.gee_interface.get_info_async( + display_fc + ) + + self.alert.add_msg(ms.aoi_sel.complete, "success") + self.updated += 1 + self.custom_aoi_dialog.on_new_geom( + feature_collection=feature_collection, name=name, source=source + ) + finally: + self._set_loading(False) + + def _on_build_error(self, exc: Exception): + """Surface import failures in the dialog alert without silently saving.""" + self._set_loading(False) + logger.exception("AOI import failed", exc_info=exc) + self.alert.add_msg(str(exc), type_="error") + + def _set_loading(self, loading: bool) -> None: + """Spin the validate button across the async import (best-effort).""" + btn = getattr(self, "btn", None) + if btn is not None: + btn.loading = loading + btn.disabled = loading diff --git a/component/widget/map.py b/component/widget/map.py index 78d58945..bcf9a687 100644 --- a/component/widget/map.py +++ b/component/widget/map.py @@ -1,23 +1,26 @@ +import logging import time from copy import deepcopy +from functools import partial -from component.frontend.icons import icon -from component.widget.buttons import TextBtn +import ee import sepal_ui.sepalwidgets as sw -from ipyleaflet import GeoJSON, WidgetControl -from shapely.geometry import Point, shape +from ipyleaflet import SplitMapControl, WidgetControl from sepal_ui import mapping as sm -from traitlets import Dict, Int, link -from sepal_ui.scripts.gee_interface import GEEInterface from sepal_ui.mapping.map_btn import MapBtn -from component.widget.admin_aoi_dialog import AdminAoiDialog -from component.widget.custom_geometries_dialog import CustomGeometriesDialog -from component.message import cm -from component import widget as cw +from sepal_ui.scripts.gee_interface import GEEInterface +from shapely.geometry import Point, shape +from traitlets import Dict, Int, link +from component import widget as cw +from component.frontend.icons import icon from component.model.aoi_model import SeplanAoi -from component.widget.admin_aoi_dialog import _is_admin_eligible -from ipyleaflet import SplitMapControl +from component.scripts.aoi_geometry import _aoi_bbox, fc_from_source +from component.widget.admin_aoi_dialog import AdminAoiDialog, _is_admin_eligible +from component.widget.buttons import TextBtn +from component.widget.custom_geometries_dialog import CustomGeometriesDialog + +logger = logging.getLogger("SEPLAN") class SeplanMap(sm.SepalMap): @@ -135,7 +138,6 @@ def _sync_custom_geom_buttons(self, *_): ``ADMIN2`` is the finest grain so no further subdivision is offered, and non-admin primaries (DRAW / SHAPE / ASSET) skip it. """ - has_aoi = self.aoi_model.feature_collection is not None primary_method = getattr(self.aoi_model.aoi_model, "method", "") or "" admin_eligible = has_aoi and _is_admin_eligible(primary_method) @@ -172,17 +174,12 @@ def on_draw(self, widget, event, data): def clean_map(self, *args, keep_aoi: bool = True): """Remove computed result layers but keep the AOI and user geometries. - Custom sub-AOIs the user drew or imported live in - ``self.custom_layers``; their on-map ipyleaflet ``GeoJSON`` layers - carry the user's chosen names. We preserve those by name so that - re-running compute / compare-scenarios doesn't wipe them. + Custom sub-AOIs the user drew or imported are drawn as one merged EE + tile layer (``_OUTLINE_KEY``); we preserve it (and the primary ``aoi``) + so re-running compute / compare-scenarios doesn't wipe them. """ keep = ["aoi"] if keep_aoi else [] - keep += [ - feat["properties"]["name"] - for feat in self.custom_layers["features"] - if feat.get("properties", {}).get("name") - ] + keep.append(self._OUTLINE_KEY) self.remove_all(keep_names=keep) self.controls = [ control @@ -190,50 +187,117 @@ def clean_map(self, *args, keep_aoi: bool = True): if not isinstance(control, SplitMapControl) ] + # Key of the single EE tile layer that draws all sub-AOI exact outlines. + _OUTLINE_KEY = "custom_outlines" + def on_custom_layers(self, *_): - """Event triggered when there are new custom layers (created by user). + """Render custom sub-AOIs as pixel-perfect EE-tile outlines. + + There is deliberately NO client-side GeoJSON layer: the simplified + display geometry can contain Point / LineString / GeometryCollection + parts (small features collapse under simplification), which ipyleaflet + renders as stray markers and which also break hover. Instead the exact + outline is an EE tile layer and the name label is driven by + ``_on_map_interaction`` against the per-sub-AOI geometry cache below. + """ + features = self.custom_layers["features"] - Create GeoJSON layers and add them to the map if they're not. + # exact outline tiles (single merged EE layer, rebuilt async) + self._refresh_outline_tiles(features) - """ - geojson_layers = [layer for layer in self.layers if isinstance(layer, GeoJSON)] - # Check if there are new geometries in the custom_layers - # If there are new geometries that are not in the map, add them - - # Add layers to the map - for feat in self.custom_layers["features"]: - if feat["properties"]["name"] not in [lyr.name for lyr in geojson_layers]: - # Add the layer to the map - layer = GeoJSON( - data=feat, - hover_style=feat["properties"]["hover_style"], - name=feat["properties"]["name"], - style=feat["properties"]["style"], - ) - layer.on_hover(self._display_name) - - # Add the layer to the map_layers list - self.add_layer(layer) - - # Remove layers from the map - for layer in geojson_layers: - if layer.name not in [ - feat["properties"]["name"] for feat in self.custom_layers["features"] - ]: - self.remove_layer(layer) - - # Refresh the cached bbox / shapely geoms used by the hover-leave - # detector. Built once here, not on every mousemove. + # (bbox, shapely geom, name) per sub-AOI — drives the hover label. + # ``shape`` handles every geometry type (incl. GeometryCollection); a + # collapsed point/line part simply never ``contains`` the cursor. cache = [] - for feat in self.custom_layers["features"]: + for feat in features: geom_dict = feat.get("geometry") - if not geom_dict: + name = feat.get("properties", {}).get("name") + if not geom_dict or not name: continue - geom = shape(geom_dict) - minx, miny, maxx, maxy = geom.bounds - cache.append((minx, miny, maxx, maxy, geom)) + try: + geom = shape(geom_dict) + minx, miny, maxx, maxy = geom.bounds + except Exception: + continue + cache.append((minx, miny, maxx, maxy, geom, name)) self._hover_bbox_cache = cache + def _refresh_outline_tiles(self, features): + """(Re)build the single EE tile layer with every sub-AOI's exact outline. + + The exact geometry is reconstructed server-side from each feature's + ``source`` descriptor; the ``getMapId`` runs on the GEE event loop so the + kernel thread stays responsive. Nothing dense is pulled to the client. + """ + if not features: + self.remove_layer(self._OUTLINE_KEY, none_ok=True) + return + + gee_interface = getattr(self, "gee_interface", None) + if gee_interface is None: + return + + # snapshot (the trait can change before the task runs) + snapshot = [dict(f) for f in features] + self._outline_task = gee_interface.create_task( + func=partial(self._build_outline_tiles, snapshot), + key="custom_outline_tiles", + on_error=lambda exc: logger.exception( + "Custom outline tiles failed", exc_info=exc + ), + ) + self._outline_task.start() + + async def _build_outline_tiles(self, features): + """Merge each sub-AOI's exact FC, style per-feature, add as one tile layer.""" + styled = [] + for feat in features: + fc = fc_from_source(feat["properties"].get("source"), feat) + color = feat["properties"]["style"]["color"] + # per-feature style dict consumed by FeatureCollection.style(styleProperty) + style_dict = {"color": color, "fillColor": "#00000000", "width": 2} + styled.append(fc.map(lambda f, s=style_dict: f.set("_seplan_style", s))) + + merged = ee.FeatureCollection(styled).flatten() + image = merged.style(styleProperty="_seplan_style") + + self.remove_layer(self._OUTLINE_KEY, none_ok=True) + await self.add_ee_layer_async( + image, {}, self._OUTLINE_KEY, key=self._OUTLINE_KEY + ) + + def zoom_to_custom(self, features: list) -> None: + """Zoom to just-added custom sub-AOI feature(s) — async, off the kernel thread. + + Uses each feature's EXACT geometry (rebuilt from its ``source`` + descriptor) and per-feature bounding boxes (no dissolve), so it stays + cheap and safe on dense AOIs. No-op without a GEE session or features. + """ + gee_interface = getattr(self, "gee_interface", None) + if gee_interface is None or not features: + return + snapshot = [dict(f) for f in features] + self._zoom_custom_task = gee_interface.create_task( + func=partial(self._zoom_to_custom_async, snapshot), + key="custom_zoom", + on_error=lambda exc: logger.exception("Custom zoom failed", exc_info=exc), + ) + self._zoom_custom_task.start() + + async def _zoom_to_custom_async(self, features: list) -> None: + """Resolve the added sub-AOI extent off the kernel thread and zoom to it.""" + fcs = [ + fc_from_source(feat.get("properties", {}).get("source"), feat) + for feat in features + ] + merged = ee.FeatureCollection(fcs).flatten() + # per-feature bbox (see _aoi_bbox) -> overall extent ring -> [minx,miny,maxx,maxy] + coords = await self.gee_interface.get_info_async( + _aoi_bbox(merged).coordinates().get(0) + ) + bounds = [coords[0][0], coords[0][1], coords[2][0], coords[2][1]] + self.zoom_bounds(bounds) + def remove_custom_layer(self, layer_id): """Remove custom layer from the custom_layers dict.""" # Create a copy of the current custom_layers dict @@ -248,7 +312,7 @@ def remove_custom_layer(self, layer_id): self.custom_layers = current_feats def _handle_draw(self, target, action, geo_json): - """handle the draw on map event. + """Handle the draw on map event. Accepts both action conventions — legacy ipl.DrawControl emits ``created``/``deleted``/``edited`` with ``geo_json`` as a single @@ -264,25 +328,16 @@ def _handle_draw(self, target, action, geo_json): self.aoi_model.updated += 1 def _on_map_interaction(self, **kwargs): - """Clear the hover label when the cursor isn't over a custom layer. - - Mousemove fires constantly, so this runs on a multi-user server — - we keep it cheap with three gates: - - 1. ``self.html.children`` is empty → return immediately (no label - to clear). This is the common case: the label only exists for - a few seconds after a hover. - 2. Throttle to ~10 Hz via ``_hover_check_last``. - 3. Bounding-box quick-reject against the cached - ``_hover_bbox_cache`` before paying for ``shapely.contains``. - - ``mouseout`` would be the natural signal but ipyleaflet's GeoJSON - only registers ``mouseover``/``click`` on the JS side - (``onEachFeature`` in jupyter-leaflet's index.js), so the comm - never sees it. + """Show/clear the sub-AOI name label from the cursor position. + + The custom outlines are EE tiles (raster, no per-feature events), so the + label is driven here: on each (throttled) mousemove we find the sub-AOI + whose cached geometry contains the cursor and show its name, clearing the + label otherwise. A bbox quick-reject keeps the common case cheap. + + Mousemove fires constantly (multi-user server), so we gate on a ~10 Hz + throttle and a bounding-box reject before paying for ``shapely.contains``. """ - if not self.html.children: - return if kwargs.get("type") != "mousemove": return now = time.monotonic() @@ -294,24 +349,14 @@ def _on_map_interaction(self, **kwargs): return # Leaflet gives ``[lat, lng]``; shapely uses ``(x=lng, y=lat)``. x, y = coords[1], coords[0] - for minx, miny, maxx, maxy, geom in self._hover_bbox_cache: - if minx <= x <= maxx and miny <= y <= maxy: - if geom.contains(Point(x, y)): - return - self.html.children = [] - - def _display_name(self, feature, **kwargs): - """update the AOI in the html viewver widget.""" - # if the feature is a aoi it has no name so I display only the sub AOI name - # it will be solved with: https://github.com/12rambau/sepal_ui/issues/390 - name = ( - feature["properties"]["name"] - if "name" in feature["properties"] - else "Main AOI" - ) - self.html.children = [name] - - return self + point = Point(x, y) + for minx, miny, maxx, maxy, geom, name in self._hover_bbox_cache: + if minx <= x <= maxx and miny <= y <= maxy and geom.contains(point): + if self.html.children != [name]: + self.html.children = [name] + return + if self.html.children: + self.html.children = [] def reset(self, change): """Reset the map view (remove all the layers).""" diff --git a/pyproject.toml b/pyproject.toml index 4b445627..f88c4368 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,7 +14,7 @@ dependencies = [ "ee-client>=2.4.0", "toml", "ipecharts>=1.2.0", - "pysepal>=3.6.1", + "pysepal>=3.6.2", "rasterio", # custom ipyleaflet build (fork) shipped as direct wheel URLs "jupyter_leaflet @ https://github.com/dfguerrerom/ipyleaflet/raw/634bcf22c09034a917819437358d5840fa72ac41/wheels/jupyter_leaflet-0.20.0-py3-none-any.whl", diff --git a/solara_app.py b/solara_app.py index 4ca3ae9d..c6c1a4eb 100644 --- a/solara_app.py +++ b/solara_app.py @@ -5,36 +5,34 @@ import logging from pathlib import Path -from traitlets import Float, HasTraits, List, link - -import solara -from solara.lab.components.theming import theme import sepal_ui.sepalwidgets as sw +import solara from sepal_ui.scripts.utils import init_ee from sepal_ui.sepalwidgets.vue_app import MapApp, ThemeToggle - from sepal_ui.solara import ( - setup_sessions, - with_sepal_sessions, get_current_gee_interface, get_current_sepal_client, - setup_theme_colors, + setup_sessions, setup_solara_server, + setup_theme_colors, + with_sepal_sessions, ) +from solara.lab.components.theming import theme +from traitlets import Float, HasTraits, List, link from component.frontend.icons import icon +from component.message import cm +from component.model.app_model import AppModel from component.model.recipe import Recipe +from component.scripts.mem_diagnostics import start_memory_diagnostics from component.tile.custom_aoi_tile import AoiView -from component.widget.map import SeplanMap -from component.widget.seplan_legend import SuitabilityLegendOverlay from component.tile.questionnaire_tile import QuestionnaireTile from component.tile.recipe_tile import RecipeView from component.tile.right_panel import get_right_panel_content -from component.model.app_model import AppModel -from component.message import cm from component.widget.custom_widgets import CustomAppBar, CustomTileAbout - +from component.widget.map import SeplanMap +from component.widget.seplan_legend import SuitabilityLegendOverlay logging.getLogger("httpx").setLevel(logging.WARNING) logging.getLogger("httpcore").setLevel(logging.WARNING) @@ -43,6 +41,10 @@ init_ee() setup_solara_server(extra_asset_locations=[str(Path(__file__).parent / "assets")]) +# Background memory diagnostics (RSS / tracemalloc / session counts). +# Runs once per process; no-op when SEPLAN_MEM_DIAG=0. See the module docstring. +start_memory_diagnostics() + @solara.lab.on_kernel_start def init_gee(): From 0af46d1ebed1e50c9981a6cc599d13eddef3f5fe Mon Sep 17 00:00:00 2001 From: dfguerrerom Date: Mon, 22 Jun 2026 21:37:55 +0200 Subject: [PATCH 2/4] fix(aoi): guard async task completions against cancel/supersede races MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Async AOI tasks could apply results after the user cancelled or made a newer selection. Add a monotonic generation token per path (captured at task start, re-checked before any state mutation) and carry per-selection data in the task result instead of reading mutable self.* fields on completion; cancel the prior task as a courtesy. - admin_aoi_dialog: _resolve_admin_async now takes/returns code+text+gen; _on_resolved rejects a stale gen and reads code/text from the result (was reading self._admin_code on completion -> could pair selection A's geometry with selection B's source). _cancel bumps the token + cancels. - custom_aoi_dialog: _validate_async returns {gen, outside_count}; _on_validate_done / _on_validate_error reject a stale gen (no commit/alert after cancel or a superseding save). on_cancel bumps + cancels. - import_aoi_dialog: _build_async takes a gen and skips the on_new_geom tail when superseded (the tail has no await, so cancel() alone can't stop it); dialog _cancel/open_dialog invalidate via cancel_import(). - map: outline-tile refresh is single-flight — bump token + cancel prior before any layer mutation (incl. the empty/delete-all remove), drop the pre-add remove (add_layer self-removes), and add a delete-all guard so a late getMapId can't resurrect deleted outlines. zoom_to_custom is left as-is (last-writer-wins viewport, captures its features, no resurrection/clobber). --- component/widget/admin_aoi_dialog.py | 55 ++++++++++++++++++++------- component/widget/custom_aoi_dialog.py | 36 ++++++++++++++---- component/widget/import_aoi_dialog.py | 30 ++++++++++++++- component/widget/map.py | 31 +++++++++++++-- 4 files changed, 126 insertions(+), 26 deletions(-) diff --git a/component/widget/admin_aoi_dialog.py b/component/widget/admin_aoi_dialog.py index feafd5b0..90359507 100644 --- a/component/widget/admin_aoi_dialog.py +++ b/component/widget/admin_aoi_dialog.py @@ -63,6 +63,10 @@ def __init__( self._primary_admin: str = "" self._parent_level: int = -1 self._admin_task = None + # Monotonic token: bumped on every submit/cancel so a task that settles + # after the user moved on (cancel, or a newer pick) is rejected at + # completion instead of committing a stale/wrong-AOI geometry. + self._admin_gen = 0 # Caller-supplied dialog to re-open if the user cancels out of this # one — used to restore the consolidated Custom Geometries picker # after a back-out. @@ -93,6 +97,11 @@ def __init__( def _cancel(self, *_): """Close the dialog and re-open the caller dialog if any.""" + # Invalidate + cancel any in-flight resolution so it can't commit after + # the user backed out (the cancel button stays enabled during the await). + self._admin_gen += 1 + if self._admin_task is not None: + self._admin_task.cancel() return_to = self._return_to self._return_to = None self.close_dialog() @@ -253,32 +262,50 @@ def on_submit(self, *_): self.btn.loading = True self.alert.reset() - self._admin_code = admin_code - self._admin_text = admin_text + # Supersede any in-flight resolution and capture a fresh token; the + # per-selection code/text travel WITH the task (not via mutable self.*), + # so the completion can't pair selection A's geometry with code B. + if self._admin_task is not None: + self._admin_task.cancel() + self._admin_gen += 1 + gen = self._admin_gen self._admin_task = self.gee_interface.create_task( - func=self._resolve_admin_async, + func=lambda: self._resolve_admin_async(admin_code, admin_text, gen), key="admin_subaoi_resolve", on_done=self._on_resolved, on_error=self._on_resolve_error, ) self._admin_task.start() - async def _resolve_admin_async(self): + async def _resolve_admin_async(self, admin_code, admin_text, gen): """Materialize the picked admin unit as a GeoJSON FeatureCollection. - Returns the GeoJSON dict ready to feed into - ``CustomAoiDialog.on_new_geom``. We use ``get_info_async`` so the - resolution doesn't block the kernel — pygaul is lazy and only the - ``getInfo`` round-trip is slow. + Returns ``{gen, code, text, geo_json}`` so the completion handler reads + the per-selection data from the result, never from mutable ``self.*``. + ``get_info_async`` keeps the kernel responsive — pygaul is lazy and only + the ``getInfo`` round-trip is slow. """ - fc = pygaul.AdmItems(admin=self._admin_code) + fc = pygaul.AdmItems(admin=admin_code) # Simplify server-side: a dense admin unit (e.g. a country with millions # of vertices) would otherwise be pulled in full and OOM the kernel. The # full-resolution geometry stays server-side for analysis. - return await self.gee_interface.get_info_async(simplify_fc(fc)) - - def _on_resolved(self, geo_json: dict): + geo_json = await self.gee_interface.get_info_async(simplify_fc(fc)) + return { + "gen": gen, + "code": admin_code, + "text": admin_text, + "geo_json": geo_json, + } + + def _on_resolved(self, result: dict): """Forward the materialized geometry to ``CustomAoiDialog``.""" + # Reject a result whose selection the user has cancelled or replaced. + if not result or result["gen"] != self._admin_gen: + return + geo_json = result["geo_json"] + code = result["code"] + text = result["text"] + self.btn.disabled = False self.btn.loading = False if not geo_json or not geo_json.get("features"): @@ -295,11 +322,11 @@ def _on_resolved(self, geo_json: dict): self.close_dialog() self.custom_aoi_dialog.on_new_geom( feature_collection=geo_json, - name=self._admin_text or self._admin_code, + name=text or code, skip_containment_check=True, # rebuild the EXACT geometry from the admin code for analysis + tiles # (geo_json above is simplified for display only) - source={"type": "admin", "code": self._admin_code}, + source={"type": "admin", "code": code}, ) def _on_resolve_error(self, exc: Exception): diff --git a/component/widget/custom_aoi_dialog.py b/component/widget/custom_aoi_dialog.py index fd123d46..46941b6c 100644 --- a/component/widget/custom_aoi_dialog.py +++ b/component/widget/custom_aoi_dialog.py @@ -91,6 +91,9 @@ def __init__(self, map_: SeplanMap): # Holds the in-flight validation task between kick-off and completion self._validate_task = None + # Monotonic token: rejects a validation verdict that settles after the + # user cancelled or started a newer save. + self._validate_gen = 0 # Set by ``on_new_geom`` when the admin-sub flow forwards a feature # whose containment is structurally guaranteed; ``on_save_geom`` @@ -163,15 +166,21 @@ def on_save_geom(self, *_): self.btn.disabled = True self.btn.loading = True + # Supersede any in-flight validation; the token lets us reject a verdict + # that settles after the user cancelled (the cancel button stays enabled). + if self._validate_task is not None: + self._validate_task.cancel() + self._validate_gen += 1 + gen = self._validate_gen self._validate_task = gee_interface.create_task( - func=self._validate_async, + func=lambda: self._validate_async(gen), key="custom_geom_validate", on_done=self._on_validate_done, - on_error=self._on_validate_error, + on_error=lambda exc: self._on_validate_error(exc, gen), ) self._validate_task.start() - async def _validate_async(self): + async def _validate_async(self, gen): """Return the count of staged sub-AOIs that fall (largely) outside the primary. Uses the EXACT geometry, rebuilt server-side from the ``source`` @@ -193,7 +202,8 @@ async def _validate_async(self): frac = await gee_interface.get_info_async( _outside_fraction(child, primary_fc) ) - return 1 if (frac is not None and frac > _CONTAINMENT_FRACTION) else 0 + outside = 1 if (frac is not None and frac > _CONTAINMENT_FRACTION) else 0 + return {"gen": gen, "outside_count": outside} outside_count = 0 for feat in feats: @@ -203,10 +213,14 @@ async def _validate_async(self): ) if frac is not None and frac > _CONTAINMENT_FRACTION: outside_count += 1 - return outside_count + return {"gen": gen, "outside_count": outside_count} - def _on_validate_done(self, outside_count: int): + def _on_validate_done(self, result: dict): """Commit on full containment, otherwise surface the failure.""" + # Reject a verdict whose save the user cancelled or superseded. + if not result or result["gen"] != self._validate_gen: + return + outside_count = result["outside_count"] self.btn.disabled = False self.btn.loading = False if outside_count > 0: @@ -221,8 +235,10 @@ def _on_validate_done(self, outside_count: int): return self._commit_save() - def _on_validate_error(self, exc: Exception): + def _on_validate_error(self, exc: Exception, gen: int): """Surface the GEE error in the dialog without silently saving.""" + if gen != self._validate_gen: # superseded/cancelled — drop stale error + return self.btn.disabled = False self.btn.loading = False logger.exception("Custom geometry validation failed", exc_info=exc) @@ -254,6 +270,12 @@ def on_cancel(self, *_): self.map_.dc.clear() self.map_.dc.hide() + # Invalidate + cancel any in-flight validation so it can't commit/alert + # after the user cancelled. + self._validate_gen += 1 + if self._validate_task is not None: + self._validate_task.cancel() + # Clear any feature that was selected self.feature = None self._candidate_features = None diff --git a/component/widget/import_aoi_dialog.py b/component/widget/import_aoi_dialog.py index faa4e8fd..519eeda7 100644 --- a/component/widget/import_aoi_dialog.py +++ b/component/widget/import_aoi_dialog.py @@ -56,6 +56,7 @@ def _on_success(self): def _cancel(self, *_): """Close and re-open the caller dialog if any.""" + self.aoi_view.cancel_import() # reject any in-flight build that settles late return_to = self._return_to self._return_to = None self.close_dialog() @@ -70,6 +71,7 @@ def open_dialog(self, *_, return_to=None): """ if return_to is not None: self._return_to = return_to + self.aoi_view.cancel_import() # invalidate any leftover in-flight build self.aoi_view.reset() super().open_dialog() @@ -89,6 +91,16 @@ def __init__(self, custom_aoi_dialog, gee_interface, **kwargs): super().__init__(methods=methods, gee_interface=gee_interface, **kwargs) self.custom_aoi_dialog = custom_aoi_dialog + self._import_task = None + # Monotonic token: bumped on each validate / cancel / reopen so a build + # that settles after the user moved on doesn't pop the rename dialog. + self._import_gen = 0 + + def cancel_import(self) -> None: + """Invalidate + cancel any in-flight import build (call on cancel/reopen).""" + self._import_gen += 1 + if self._import_task is not None: + self._import_task.cancel() def _update_aoi(self, *args) -> Self: """Import the AOI without materialising full geometry client-side. @@ -131,11 +143,20 @@ def _update_aoi(self, *args) -> Self: dissolve = False do_simplify = False + # Supersede any in-flight build and capture a fresh token. + self.cancel_import() + gen = self._import_gen self._set_loading(True) # hold a ref so the task isn't GC'd before it runs self._import_task = self.model.gee_interface.create_task( func=partial( - self._build_async, fc, self.model.name, dissolve, do_simplify, source + self._build_async, + gen, + fc, + self.model.name, + dissolve, + do_simplify, + source, ), key="import_aoi_build", on_error=self._on_build_error, @@ -143,7 +164,7 @@ def _update_aoi(self, *args) -> Self: self._import_task.start() return self - async def _build_async(self, fc, name, dissolve, do_simplify, source): + async def _build_async(self, gen, fc, name, dissolve, do_simplify, source): """Pull the display outline server-side, then hand off to the rename dialog. Runs on the GEE event loop (via ``create_task``) so the kernel thread @@ -158,6 +179,11 @@ async def _build_async(self, fc, name, dissolve, do_simplify, source): display_fc ) + # Drop the result if the user cancelled/reopened or re-validated; the + # completion tail has no further await, so cancel() alone can't stop it. + if gen != self._import_gen: + return + self.alert.add_msg(ms.aoi_sel.complete, "success") self.updated += 1 self.custom_aoi_dialog.on_new_geom( diff --git a/component/widget/map.py b/component/widget/map.py index bcf9a687..6dca75e8 100644 --- a/component/widget/map.py +++ b/component/widget/map.py @@ -103,6 +103,11 @@ def __init__( self._hover_bbox_cache: list = [] self._hover_check_last: float = 0.0 + # Custom-outline tile task + monotonic token (single-flight; a superseded + # getMapId must not resurrect deleted outlines — see _refresh_outline_tiles). + self._outline_task = None + self._outline_gen = 0 + self.dc.on_draw(self._handle_draw) self.observe(self.on_custom_layers, "custom_layers") self.on_interaction(self._on_map_interaction) @@ -228,7 +233,16 @@ def _refresh_outline_tiles(self, features): The exact geometry is reconstructed server-side from each feature's ``source`` descriptor; the ``getMapId`` runs on the GEE event loop so the kernel thread stays responsive. Nothing dense is pulled to the client. + + Concurrency: bump a token and cancel the prior task BEFORE any layer + mutation (incl. the empty/delete-all remove), so a slow ``getMapId`` from + a superseded refresh can't resurrect deleted outlines. """ + if self._outline_task is not None: + self._outline_task.cancel() + self._outline_gen += 1 + my_gen = self._outline_gen + if not features: self.remove_layer(self._OUTLINE_KEY, none_ok=True) return @@ -240,7 +254,7 @@ def _refresh_outline_tiles(self, features): # snapshot (the trait can change before the task runs) snapshot = [dict(f) for f in features] self._outline_task = gee_interface.create_task( - func=partial(self._build_outline_tiles, snapshot), + func=partial(self._build_outline_tiles, snapshot, my_gen), key="custom_outline_tiles", on_error=lambda exc: logger.exception( "Custom outline tiles failed", exc_info=exc @@ -248,8 +262,10 @@ def _refresh_outline_tiles(self, features): ) self._outline_task.start() - async def _build_outline_tiles(self, features): + async def _build_outline_tiles(self, features, my_gen): """Merge each sub-AOI's exact FC, style per-feature, add as one tile layer.""" + if my_gen != self._outline_gen: # superseded before we started + return styled = [] for feat in features: fc = fc_from_source(feat["properties"].get("source"), feat) @@ -261,11 +277,20 @@ async def _build_outline_tiles(self, features): merged = ee.FeatureCollection(styled).flatten() image = merged.style(styleProperty="_seplan_style") - self.remove_layer(self._OUTLINE_KEY, none_ok=True) + if my_gen != self._outline_gen: # superseded while building the request + return + # add_layer self-removes the existing _OUTLINE_KEY layer; we deliberately + # do NOT remove first (a stale task removing could blank a newer layer). await self.add_ee_layer_async( image, {}, self._OUTLINE_KEY, key=self._OUTLINE_KEY ) + # Delete-all guard: if every sub-AOI was removed while our getMapId was in + # flight, drop the outline we just added (no clobber — only fires when the + # map is genuinely empty). + if not self.custom_layers["features"]: + self.remove_layer(self._OUTLINE_KEY, none_ok=True) + def zoom_to_custom(self, features: list) -> None: """Zoom to just-added custom sub-AOI feature(s) — async, off the kernel thread. From 51cc2eee4979bafb6796df0ef10d333b0fb5f8d1 Mon Sep 17 00:00:00 2001 From: dfguerrerom Date: Mon, 22 Jun 2026 21:45:05 +0200 Subject: [PATCH 3/4] test: cover stale-generation rejection in async AOI completions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unit-tests the sync completion handlers (no GEE): a result whose generation token was superseded by a cancel or newer selection is dropped — admin _on_resolved does not forward a geometry, custom-geometry _on_validate_done does not commit, and a stale _on_validate_error does not reset the UI. Also asserts the fresh-token paths still act, and that admin reads source/name from the result (not mutable self.*). --- tests/test_async_race_guards.py | 106 ++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 tests/test_async_race_guards.py diff --git a/tests/test_async_race_guards.py b/tests/test_async_race_guards.py new file mode 100644 index 00000000..23dd050a --- /dev/null +++ b/tests/test_async_race_guards.py @@ -0,0 +1,106 @@ +"""Regression: async task completions must be rejected after cancel/supersede. + +Admin resolution and custom-geometry containment run as background GEE tasks. +If the user cancels or makes a newer selection while a task is in flight, the +stale completion must NOT commit/forward anything. Each path carries a monotonic +generation token (and, for admin, the per-selection data) in the task result; +the ``on_done``/``on_error`` handlers drop a result whose token no longer matches. + +These exercise the sync completion handlers directly (no GEE needed) — the +load-bearing guard, since the completion tail has no further ``await`` for +``cancel()`` to interrupt. +""" + +import types + +from component.widget.admin_aoi_dialog import AdminAoiDialog +from component.widget.custom_aoi_dialog import CustomAoiDialog + + +def _admin_stub(gen: int): + """An AdminAoiDialog with only the fields ``_on_resolved`` touches. + + Deliberately does NOT set ``_admin_code``/``_admin_text`` — if the handler + regressed to reading those mutable fields (the original bug) it would raise + AttributeError instead of using the per-result data. + """ + dlg = AdminAoiDialog.__new__(AdminAoiDialog) + dlg._admin_gen = gen + dlg.btn = types.SimpleNamespace(disabled=True, loading=True) + dlg.alert = types.SimpleNamespace(add_msg=lambda *a, **k: None) + dlg.close_dialog = lambda *a, **k: None + calls = [] + dlg.custom_aoi_dialog = types.SimpleNamespace( + on_new_geom=lambda **kw: calls.append(kw) + ) + return dlg, calls + + +def test_admin_on_resolved_rejects_stale_gen(): + """A resolution whose token was superseded must not forward a geometry.""" + dlg, calls = _admin_stub(gen=2) # current generation is 2 + dlg._on_resolved( + {"gen": 1, "code": "A", "text": "Area A", "geo_json": {"features": [{}]}} + ) + assert calls == [] # stale (gen 1 != 2) -> dropped + + +def test_admin_on_resolved_uses_result_not_mutable_state(): + """A fresh resolution forwards the geometry with source from the RESULT.""" + dlg, calls = _admin_stub(gen=1) + dlg._on_resolved( + {"gen": 1, "code": "123", "text": "Java", "geo_json": {"features": [{}]}} + ) + assert len(calls) == 1 + kw = calls[0] + assert kw["name"] == "Java" + assert kw["source"] == {"type": "admin", "code": "123"} + assert kw["skip_containment_check"] is True + + +def _validate_stub(gen: int): + """A CustomAoiDialog with only the fields the validate handlers touch.""" + dlg = CustomAoiDialog.__new__(CustomAoiDialog) + dlg._validate_gen = gen + dlg.btn = types.SimpleNamespace(disabled=True, loading=True) + dlg.alert = types.SimpleNamespace(add_msg=lambda *a, **k: None) + committed = [] + dlg._commit_save = lambda: committed.append(True) + return dlg, committed + + +def test_validate_done_rejects_stale_gen(): + """A verdict from a cancelled/superseded save must not commit.""" + dlg, committed = _validate_stub(gen=2) + dlg._on_validate_done({"gen": 1, "outside_count": 0}) + assert committed == [] # stale -> no commit + + +def test_validate_done_commits_when_inside(): + """A fresh verdict with nothing outside commits the sub-AOI.""" + dlg, committed = _validate_stub(gen=1) + dlg._on_validate_done({"gen": 1, "outside_count": 0}) + assert committed == [True] + + +def test_validate_done_blocks_when_outside(): + """A fresh verdict with geometry outside the primary AOI does not commit.""" + dlg, committed = _validate_stub(gen=1) + dlg._on_validate_done({"gen": 1, "outside_count": 2}) + assert committed == [] # surfaced as an error, no commit + + +def test_validate_error_rejects_stale_gen(): + """A stale validation error must not reset the (reused) dialog UI.""" + dlg, _ = _validate_stub(gen=2) + dlg.btn.disabled = True + dlg._on_validate_error(RuntimeError("boom"), gen=1) + assert dlg.btn.disabled is True # stale -> handler returned before resetting + + +def test_validate_error_resets_on_fresh_gen(): + """A current validation error resets the button so the user can retry.""" + dlg, _ = _validate_stub(gen=1) + dlg.btn.disabled = True + dlg._on_validate_error(RuntimeError("boom"), gen=1) + assert dlg.btn.disabled is False From b3732493e303a12d5f39f8d23b83a38b8d3666be Mon Sep 17 00:00:00 2001 From: dfguerrerom Date: Mon, 22 Jun 2026 22:16:27 +0200 Subject: [PATCH 4/4] perf(map): make hover-label throttle a tunable constant (0.2s) _on_map_interaction runs on the shared kernel for every mousemove; hoist the hardcoded 0.1s into _HOVER_THROTTLE_S and raise to 0.2s (~5 Hz) to halve the per-user hover work ahead of multi-user load. Label stays responsive. --- component/widget/map.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/component/widget/map.py b/component/widget/map.py index 6dca75e8..1d1be0f1 100644 --- a/component/widget/map.py +++ b/component/widget/map.py @@ -30,6 +30,11 @@ class SeplanMap(sm.SepalMap): new_geom = Int(0).tag(sync=True) """int: either a new geometry has been drawn on the map""" + # Minimum seconds between hover-label recomputes in ``_on_map_interaction``. + # mousemove fires constantly and this runs on the shared kernel; throttling + # caps the per-user work. Higher = cheaper but laggier label (see ~5 Hz here). + _HOVER_THROTTLE_S = 0.2 + def __init__( self, seplan_aoi: SeplanAoi = None, @@ -360,13 +365,14 @@ def _on_map_interaction(self, **kwargs): whose cached geometry contains the cursor and show its name, clearing the label otherwise. A bbox quick-reject keeps the common case cheap. - Mousemove fires constantly (multi-user server), so we gate on a ~10 Hz - throttle and a bounding-box reject before paying for ``shapely.contains``. + Mousemove fires constantly (multi-user server), so we gate on the + ``_HOVER_THROTTLE_S`` throttle and a bounding-box reject before paying for + ``shapely.contains``. """ if kwargs.get("type") != "mousemove": return now = time.monotonic() - if now - self._hover_check_last < 0.1: + if now - self._hover_check_last < self._HOVER_THROTTLE_S: return self._hover_check_last = now coords = kwargs.get("coordinates")