From cad9782ca1ce67b2bf19d7267f88ec56579ac02b Mon Sep 17 00:00:00 2001 From: Andrey Vyrodov Date: Sat, 10 Oct 2026 09:31:32 +0200 Subject: [PATCH] Issue #345: long-running tasks use the activity stack (rule and guide) CLAUDE.md gains the rule (a TaskGuard for every job that runs off the UI thread and can take more than a moment; no progress window, panel or toast of its own), and docs/background-tasks.md says how: kind, label, progress, units, detail, the three ways to cancel, examples and tests. Photo Merge's and the face scan's docs no longer describe the old progress toast. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 4 +++ docs/background-tasks.md | 65 ++++++++++++++++++++++++++++++++++++++++ docs/faces.md | 2 +- docs/merge.md | 2 +- docs/parity.md | 2 +- 5 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 docs/background-tasks.md diff --git a/CLAUDE.md b/CLAUDE.md index 577458965..814bc5cf8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -55,6 +55,10 @@ of it. Full standard: `../craftrules/standards/never-crash.md` - **Pure Rust** in the product. No C/C++ dependencies. - **Layering** (`plan/architecture.md` §3, enforced by `cargo xtask layers`): nothing below L5 depends on egui/eframe/winit/rfd. - **Everything is a command** (`crates/engine`): id, label, menu path, shortcut, params, enabled(), run(). UI, CLI, control channel and MCP all dispatch by id. Every slider is a `develop` control spec. +- **Long-running work shows in the activity stack** ([`docs/background-tasks.md`](docs/background-tasks.md)): anything + that runs off the UI thread and can take more than a moment holds a `TaskGuard` from `session.activity.start(kind, + label, cancel)` for its whole run. It adopts the cancel flag the job already checks (`Cancel::Flag`), or uses + `Cancel::No` when the job can't stop. No progress window, panel or progress toast of its own; its tests assert the row. - **Resolution independence:** settings use normalized image coordinates and relative radii; previews and exports must match. - **Quality gates** before every commit: `cargo xtask ci` (fmt, clippy -D warnings, tests, layers, assets, wasm). - **Commits:** one task id per commit (`M2.3: local Laplacian highlights/shadows`). Only green states. End messages with the attribution line required by the environment. diff --git a/docs/background-tasks.md b/docs/background-tasks.md new file mode 100644 index 000000000..576b3868f --- /dev/null +++ b/docs/background-tasks.md @@ -0,0 +1,65 @@ +# Long-running tasks and the activity stack + +Everything that takes a while runs off the UI thread and shows up in **one** place: the activity stack in the +top-left corner under the top bar (issue #345). Each task has a row with its name, a progress bar (a sweep while +the amount of work isn't known), the count and what it's working on, and ✕ if it can stop. Quitting asks first while +a task that can stop is running. Agents see the same list through `activity.list` / `activity.cancel`, and +`ui.inspect` reports it as `activity`. + +The registry is `lightcraft_engine::activity` (`crates/engine/src/activity.rs`, `Session::activity`). The stack is +`crates/ui-egui/src/panels/activity.rs`. Every job that can last more than a moment uses it; **don't add a progress +window, panel or progress toast of your own**. A dialog may still show progress inline (the Synchronize Folder +dialog counts files while it scans), and a toast at the end saying what happened is fine. + +## Adding a task + +```rust +use lightcraft_engine::activity::{Cancel, Unit}; + +// when the job starts, with the cancel flag the worker already checks between files +let guard = app.session.activity.start("export", "Exporting", Cancel::Flag(cancel.clone())); +guard.progress(0, total as u64); + +// every frame (or from the worker, through `guard.handle()`) +guard.progress(done as u64, total as u64); +guard.detail(&file_name); +``` + +- **Keep the guard for exactly as long as the job runs.** Put it in the struct the UI polls (`ExportTask`, `ScanTask`, + `SyncRun`…), or move it into the worker thread. Dropping it removes the row, also when the worker panics, so a + dead job never leaves a stuck bar. A worker that only reports progress takes a `TaskHandle` (`guard.handle()`, + `Clone + Send`). +- **`kind`** is a stable camelCase id that agents filter by (`export`, `import`, `scan`, `sync`, `previews`, + `download`, `faces`…). **`label`** is a short English name in sentence case ("Building previews"). The stack shows it + translated, so add it to every catalog in `crates/ui-egui/locales/`. +- **Progress:** `progress(done, total)`. A `total` of 0 means the amount of work isn't known yet, and the bar sweeps. + `set_unit(Unit::Bytes)` makes the count read "12 of 340 MB"; `Unit::Percent` reads "40 %". The default `Unit::Count` + reads "3 of 25". +- **Detail:** what the job is on (a file name, a model's name). Detail set by engine code is a name, never a sentence. + UI code may set translated text. +- **Cancel:** + - `Cancel::Flag(Arc)` adopts the flag the job already checks. The row's ✕, `activity.cancel` and the + job's own Cancel then all set the same atomic. + - `Cancel::Yes`: the job polls `guard.is_cancelled()` itself. + - `Cancel::No`: the job can't stop; it gets no ✕ and doesn't make Quit ask. + - `set_cancellable(false)` for a phase that has to finish once started (a Lightroom import adding its photos). +- **Quiet periodic work** shows no row: the auto-import folder listing runs every few seconds through + `tasks::spawn(.., None, ..)`. + +## Examples in the code + +| Job | Where | Shows how to | +|---|---|---| +| Export, contact sheet | `crates/ui-egui/src/export_task.rs` | adopt the job's flag; update the row each frame | +| Build Previews | `crates/engine/src/cmd/previews.rs` | move the guard into the worker thread | +| Model downloads | `crates/engine/src/face_download.rs` | report progress through a `TaskHandle` in shared state | +| Lightroom catalog import | `crates/ui-egui/src/lightroom_import.rs` | stop being cancellable for a phase that must finish | +| Face scan, AI Denoise | `crates/engine/src/cmd/face_recognize.rs`, `crates/engine/src/denoise.rs` | a continuous job with no ✕, kept in session state | +| Find Missing Photos | `crates/ui-egui/src/tasks.rs` | a generic background task with a row (`kind: Some(..)`) | + +## Testing + +Assert the row (`session.activity.list()`: kind, label, total, `cancellable`), that `activity.cancel(id)` stops the +job, and that the row is gone once the job ends. `crates/ui-egui/src/tests_activity.rs` has the helpers, and the +job-specific tests show the pattern (`tests_sync.rs`, `export_task.rs`, `tests_denoise.rs`…). A row only appears +on screen once its task is 0.5 s old, so tests that click its ✕ wait for `activity:cancel:` first. diff --git a/docs/faces.md b/docs/faces.md index 3b1692c6a..996c82ca5 100644 --- a/docs/faces.md +++ b/docs/faces.md @@ -248,7 +248,7 @@ Turning recognition on starts a scan of the whole library, once, in the backgrou the running ones, so a worker that finishes has its next photo at once. - **Earlier scan speed (a different runner).** A mixed raw and JPEG library of 184 photos (the raws read through their embedded previews, 73 photos searched for faces) took about 29 seconds on that machine at full pace: roughly 6 photos a second, so the first scan of 10,000 photos - is a matter of half an hour; after that only new photos are looked at. Settings ▸ Faces shows how many are left, with a progress bar. After you accept a model's terms + is a matter of half an hour; after that only new photos are looked at. Settings ▸ Faces shows how many are left, with a progress bar, and the activity stack shows "Finding faces" meanwhile. After you accept a model's terms you are returned to that tab, so the download and then the scan can be watched there; the main window has no status bar. - Opening another library starts a fresh scan state; nothing learned about one library is used in another. diff --git a/docs/merge.md b/docs/merge.md index 600eb840f..e78c356fb 100644 --- a/docs/merge.md +++ b/docs/merge.md @@ -9,7 +9,7 @@ All algorithms are our own implementations from published papers (cited in the m - **App:** select the photos, then *Photo Merge ▸ HDR…* (`Ctrl+H`), *Panorama…* (`Ctrl+M`) or *HDR Panorama…* from the photo context menu (or the `dialog.merge*` commands). The dialog previews the merge at ≤ 1024 px and re-runs the preview when an option changes; *Merge* runs the full merge - in the background (progress in a toast). The result is written next to the first photo as + in the background (a row in the activity stack shows its progress, with ✕ to stop it). The result is written next to the first photo as `-HDR.dng`, `-Pano.dng` or `-HDR-Pano.dng`, imported and selected. - **Commands** (CLI, MCP, control channel): - `merge.hdr {ids?, align=true, deghost=none|low|medium|high, autoSettings=true, stack=false, preview=false, showOverlay=false, previewPath?}` diff --git a/docs/parity.md b/docs/parity.md index 4c05ae08a..d767ea2d6 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -516,7 +516,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-BEHAV-BATCH | Batch apply to selection | P0 | ✅ | `cmd:photo.rate`, `cmd:develop.paste`, `cmd:preset.apply`, `cmd:app.export` | | | LR-BEHAV-PREVIEW-HOVER | Hover previews | P1 | ✅ | `crates/ui-egui/src/panels/presets.rs`, `crates/ui-egui/src/panels/profiles.rs`, `crates/ui-egui/src/panels/right.rs` (`versions`), `crates/ui-egui/src/panels/detail.rs` | presets, profile menu, profile browser and versions preview in the loupe | | LR-BEHAV-PROGRESSIVE | Progressive rendering | P0 | ✅ | `crates/engine/src/media.rs`, `crates/preview/src/lib.rs` | | -| LR-BEHAV-BG-TASKS | Background tasks | P0 | ✅ | `cmd:activity.list`, `cmd:activity.cancel`, `crates/engine/src/activity.rs`, `crates/ui-egui/src/panels/activity.rs`, `crates/preview/src/lib.rs` (`JobPool`) | renders off the UI thread; exports run several photos of a batch side by side (`export_parallelism`, memory-gated; files, names and order as one at a time, #496); one activity stack in the top-left corner under the top bar, as in Lightroom Classic (issue #345): imports, folder scans, exports (and the contact sheet PDF), Synchronize Folder (its scan and its run), Build / Smart Previews, Lightroom catalog inspect / import, Photo Merge, model downloads (SAM 3, face, AI Denoise), the face scan, AI Denoise and Find Missing Photos each show a row (name, progress bar or a sweep while the amount isn't known, count and current item) once 0.5 s old, three rows then "+N more"; ✕ cancels the jobs that can stop ("Stopping…" until they do); quitting asks while a cancellable task runs; agents and `ui.inspect` (`activity`) see the same list. No pause (Lightroom can pause some tasks) | +| LR-BEHAV-BG-TASKS | Background tasks | P0 | ✅ | `cmd:activity.list`, `cmd:activity.cancel`, `crates/engine/src/activity.rs`, `crates/ui-egui/src/panels/activity.rs`, `docs/background-tasks.md`, `crates/preview/src/lib.rs` (`JobPool`) | renders off the UI thread; exports run several photos of a batch side by side (`export_parallelism`, memory-gated; files, names and order as one at a time, #496); one activity stack in the top-left corner under the top bar, as in Lightroom Classic (issue #345): imports, folder scans, exports (and the contact sheet PDF), Synchronize Folder (its scan and its run), Build / Smart Previews, Lightroom catalog inspect / import, Photo Merge, model downloads (SAM 3, face, AI Denoise), the face scan, AI Denoise and Find Missing Photos each show a row (name, progress bar or a sweep while the amount isn't known, count and current item) once 0.5 s old, three rows then "+N more"; ✕ cancels the jobs that can stop ("Stopping…" until they do); quitting asks while a cancellable task runs; agents and `ui.inspect` (`activity`) see the same list. No pause (Lightroom can pause some tasks) | | LR-BEHAV-OFFLINE | Offline editing | P1 | ✅ | | local-first: everything works offline | | LR-BEHAV-WEB-SAFETY | Library safety in the browser build | P1 | 🟡 | `cmd:file.backupLibrary`, `cmd:file.restoreLibrary`, `apps/lightcraft-web/src/safety.rs`, `apps/lightcraft-web/src/backup.rs`, `apps/lightcraft-web/src/files.rs` | web only (experimental): Back Up Library (zip of the catalog and every stored original) and a non-destructive Restore; failed saves show as unsaved and are retried; a photo that can't be stored isn't added; one tab per library (Web Locks); notices for storage that isn't persistent and for a library that can't open; `?reset` asks first; a panic shows a message. Missing: `?safe` start, deleting originals of removed photos, zip64 (backups > 4 GB) | | LR-BEHAV-GPU | GPU acceleration | P0 | ✅ | `crates/gpu/src/render.rs`, `cmd:app.gpu`, `docs/gpu-pipeline.md` | CPU fallback per render on device limits, out of memory, device errors / loss, incomplete or black results (`perf.gpuReason`, `perf.gpuFallback`); short submissions for slow iGPUs; backend choice (DX12 only on Windows, `LIGHTCRAFT_GPU_BACKEND` / `WGPU_BACKEND`), device created after the window opens, crash sentinel (issue #136); quitting waits (up to 2 s) for renders in flight and starts no new GPU work, so none is inside the driver when the process exits (`crates/gpu/src/exit.rs`, issue #620; seen crashing only on Linux + NVIDIA, other drivers unverified); no tiling beyond the buffer limit (CPU) |