Location: deleting a Saved place has no Undo action

kayg commented

2026-10-02 13:20:44 +00:00

Owner

Source finding from #427 on origin/dev c4a61e8cf090170f35b1bed3350d9de20c83ecd5. No product changes or rendered proof in this source-only review.

DESIGN §47 L3–L4 defines Saved places and captured place labels. §34 and the owner UX completeness rule of 2026-10-02 require Undo for destructive actions.

Evidence: apps/web/src/routes/settings/account/LocationGroup.svelte:198–211 deletes the selected place, posts a plain Removed confirmation, drops the selected place and reloads. It retains no inverse action or successful-delete receipt. Existing Logs correctly keep their captured place names.

Observed: The confirmation prevents an accidental click, but a successful deletion has no immediate Undo. The User must manually rebuild the Saved place and cannot recover its original stable identity from this flow.

Expected: Offer the shared Undo toast after successful removal. Restore the same Saved place ID, name, coordinates and radius without changing existing Logs or another place. Make a failed Undo visible and retryable.

Test idea: Delete a Saved place and undo. Assert the same ID, exact coordinates, radius and name in the UI and API. Verify existing Logs retain their captured values. Test another simultaneous place edit and failed restoration.

Duplicate check: Searched all initial titles and the Saved-place Undo body search. Read #391 and its comments, which cover Location creation and deletion but do not identify this missing recovery action. #502 concerns the place-entry UI, and #722 concerns repaint after existing Files/Photos Undo.

Source finding from #427 on origin/dev `c4a61e8cf090170f35b1bed3350d9de20c83ecd5`. No product changes or rendered proof in this source-only review. DESIGN §47 L3–L4 defines Saved places and captured place labels. §34 and the owner UX completeness rule of 2026-10-02 require Undo for destructive actions. Evidence: `apps/web/src/routes/settings/account/LocationGroup.svelte:198–211` deletes the selected place, posts a plain Removed confirmation, drops the selected place and reloads. It retains no inverse action or successful-delete receipt. Existing Logs correctly keep their captured place names. Observed: The confirmation prevents an accidental click, but a successful deletion has no immediate Undo. The User must manually rebuild the Saved place and cannot recover its original stable identity from this flow. Expected: Offer the shared Undo toast after successful removal. Restore the same Saved place ID, name, coordinates and radius without changing existing Logs or another place. Make a failed Undo visible and retryable. Test idea: Delete a Saved place and undo. Assert the same ID, exact coordinates, radius and name in the UI and API. Verify existing Logs retain their captured values. Test another simultaneous place edit and failed restoration. Duplicate check: Searched all initial titles and the Saved-place Undo body search. Read #391 and its comments, which cover Location creation and deletion but do not identify this missing recovery action. #502 concerns the place-entry UI, and #722 concerns repaint after existing Files/Photos Undo.

kayg referenced this issue

2026-10-02 13:21:38 +00:00

Log attachments: deleted file still shown, attachments not live, batch send not live, Log order, raw camera names #427

kayg referenced this issue

2026-10-02 13:24:48 +00:00

Log attachments: deleted file still shown, attachments not live, batch send not live, Log order, raw camera names #427

kayg referenced this issue

2026-10-02 13:29:00 +00:00

Calendar: default Event calendar has no synced Settings choice #827

kayg commented

2026-10-02 13:29:01 +00:00

Author

Owner

Starting #827 and #828 on branch job/gaps-827, based on dev at c4a61e8cf0. I am tracing the shared User settings, Event creation paths and Saved place Undo patterns before implementation.

Starting #827 and #828 on branch job/gaps-827, based on dev at c4a61e8cf090170f35b1bed3350d9de20c83ecd5. I am tracing the shared User settings, Event creation paths and Saved place Undo patterns before implementation.

kayg commented

2026-10-02 13:32:03 +00:00

Author

Owner

Finding: Location DELETE /api/v1/location/places/{id} returns 204 and the Settings screen posts a plain Removed toast then reloads. There is no restore API. Places.md preserves a stable ID and ordered place entries, so the undo path needs a validated restore operation that accepts the deleted place and its former index.

kayg commented

2026-10-02 16:28:05 +00:00

Author

Owner

A new regression probe found that a Saved place list read started during DELETE could finish before the write and put the removed place back in the shared store. The same race during restore could hide a place after the server had accepted Undo. Both cases failed before the fix and pass now; the store reconciles after successful writes and invalidates reads that began while the request was pending.

kayg commented

2026-10-02 17:21:12 +00:00

Author

Owner

Implementation decisions where DESIGN is silent: Undo keeps the deleted stable ID, fields, and prior zero-based order in the short-lived client action. The server clamps that index if other places were added meanwhile. Repeating a restore with identical content is idempotent; an existing ID with different content returns a conflict. A failed Undo remains available as a retry action in the toast.

kayg commented

2026-10-02 17:41:00 +00:00

Author

Owner

Implemented and committed the #828 Saved place Undo slice on job/gaps-827.

Head: aefe09443caab758c15a6cf8ca08ecde89c3b176 (includes the single required merge from origin/dev). The feature commit is 875b8cd6d; the shared e2e and performance harness is bf4963f2f.

Built: optimistic Saved place removal through the shared Location store; an 8-second shared Undo toast; a server restore route that uses the same ID, fields, and former order; idempotent identical retries; a conflict when an ID is already used with different content; stale-read protections; regression tests; and adversarial probes/classifications.

Files: crates/calternal-server/src/location.rs, apps/web/src/lib/location/{location.svelte.ts,location.test.ts}, apps/web/src/routes/settings/account/LocationGroup.svelte, tests/adversarial/appearance_auto_scheme.mjs, tests/adversarial/{authz_matrix.py,xuser_matrix.py}, apps/web/e2e/{harness.mjs,gaps-827-828.mjs}, apps/web/package.json, and bench/gaps-827-828.mjs.

Gate output:

cargo fmt --check: exit 0, no output.

Focused web regressions:

Test Files  3 passed (3)
     Tests  42 passed (42)

bun run check before generated OpenAPI artifacts:
```
svelte-check found 16 errors and 0 warnings in 7 files
error: script "check" exited with code 1
```
The remaining diagnostics are generated-client mismatches, including the restore route.
cargo test -p calternal-plugin-calendar default_event_calendar_preference_round_trips did not finish. I stopped its cold dependency build at the four-hour job limit. The server crate gates and complete web gates are not run.

Known gaps: OpenAPI/client generation is not committed; production build and e2e are not run; no screenshots are attached; the real-server adversarial round and performance profile are not run, and docs/perf/baseline.json has no measurement for this profile. cargo clean ran and apps/web/build is absent.

UX gaps closed: removal is optimistic and reactive; Undo restores the same stable place ID, name, coordinates, radius, and former order; failed Undo stays available through a retry action. The e2e script covers pointer, keyboard, touch, all three widths, both themes, and macOS emulation, but its browser assertions and screenshots are not yet run.

UX gaps left: production browser behavior, screenshot review, screen-reader output, restore retry, and cross-screen updates remain unverified because production e2e did not run.

Decisions outside DESIGN: the restore request carries the prior index with the exact Saved place fields. The server clamps the index after concurrent additions, identical retries are idempotent, and an ID collision with different content returns 409. These decisions are also recorded in the earlier issue comment.

Implemented and committed the #828 Saved place Undo slice on `job/gaps-827`. **Head:** `aefe09443caab758c15a6cf8ca08ecde89c3b176` (includes the single required merge from `origin/dev`). The feature commit is `875b8cd6d`; the shared e2e and performance harness is `bf4963f2f`. **Built:** optimistic Saved place removal through the shared Location store; an 8-second shared Undo toast; a server restore route that uses the same ID, fields, and former order; idempotent identical retries; a conflict when an ID is already used with different content; stale-read protections; regression tests; and adversarial probes/classifications. **Files:** `crates/calternal-server/src/location.rs`, `apps/web/src/lib/location/{location.svelte.ts,location.test.ts}`, `apps/web/src/routes/settings/account/LocationGroup.svelte`, `tests/adversarial/appearance_auto_scheme.mjs`, `tests/adversarial/{authz_matrix.py,xuser_matrix.py}`, `apps/web/e2e/{harness.mjs,gaps-827-828.mjs}`, `apps/web/package.json`, and `bench/gaps-827-828.mjs`. **Gate output:** - `cargo fmt --check`: exit 0, no output. - Focused web regressions: ``` Test Files 3 passed (3) Tests 42 passed (42) ``` - `bun run check` before generated OpenAPI artifacts: ``` svelte-check found 16 errors and 0 warnings in 7 files error: script "check" exited with code 1 ``` The remaining diagnostics are generated-client mismatches, including the restore route. - `cargo test -p calternal-plugin-calendar default_event_calendar_preference_round_trips` did not finish. I stopped its cold dependency build at the four-hour job limit. The server crate gates and complete web gates are not run. **Known gaps:** OpenAPI/client generation is not committed; production build and e2e are not run; no screenshots are attached; the real-server adversarial round and performance profile are not run, and `docs/perf/baseline.json` has no measurement for this profile. `cargo clean` ran and `apps/web/build` is absent. **UX gaps closed:** removal is optimistic and reactive; Undo restores the same stable place ID, name, coordinates, radius, and former order; failed Undo stays available through a retry action. The e2e script covers pointer, keyboard, touch, all three widths, both themes, and macOS emulation, but its browser assertions and screenshots are not yet run. **UX gaps left:** production browser behavior, screenshot review, screen-reader output, restore retry, and cross-screen updates remain unverified because production e2e did not run. **Decisions outside DESIGN:** the restore request carries the prior index with the exact Saved place fields. The server clamps the index after concurrent additions, identical retries are idempotent, and an ID collision with different content returns 409. These decisions are also recorded in the earlier issue comment.

kayg commented

2026-10-02 17:42:46 +00:00

Author

Owner

Follow-up documentation correction: the e2e now says Composer delegates the synced default to the server, matching the request body. Latest branch head is e1797072f43292cd1a0121e8ba74210177325146; the earlier report's other gate and completion status is unchanged.

Follow-up documentation correction: the e2e now says Composer delegates the synced default to the server, matching the request body. Latest branch head is `e1797072f43292cd1a0121e8ba74210177325146`; the earlier report's other gate and completion status is unchanged.

kayg commented

2026-10-03 02:32:20 +00:00

Author

Owner

Fix for the failing server test location::tests::removed_saved_place_can_be_restored_at_its_previous_position, commit fb20188917 on job/gaps-827 (not pushed).

Cause: PlacesDocument::rewrite in crates/calternal-location/src/lib.rs appended every place that was not in Places.md yet. The restore endpoint inserted the place at its old index in memory, but after a reload it came back last.

Fix: a new place now goes directly in front of the heading of the next place that follows it in the saved order and is already in the file. If no such neighbour exists, it goes at the end. Edits are applied in one forward pass, so all other bytes stay as they were (frontmatter, prose, comments, other places, line endings). The write is still atomic through Root::write_checked.

New unit tests in calternal-location: restore into the middle, at the start (frontmatter and intro stay first), before a heading on line 1, at the end, with neighbours edited meanwhile, and with CRLF.

Gates:

cargo fmt --check: clean
cargo clippy -p calternal-location -p calternal-server --all-targets -- -D warnings: Finished, no warnings
cargo test -p calternal-location: unit test result: ok. 6 passed; 0 failed; tests/places.rs test result: ok. 11 passed; 0 failed
cargo test -p calternal-server: test result: ok. 108 passed; 0 failed; 3 ignored

Fix for the failing server test `location::tests::removed_saved_place_can_be_restored_at_its_previous_position`, commit fb20188917ae36546279224c61b36e1cb6430da6 on `job/gaps-827` (not pushed). Cause: `PlacesDocument::rewrite` in `crates/calternal-location/src/lib.rs` appended every place that was not in `Places.md` yet. The restore endpoint inserted the place at its old index in memory, but after a reload it came back last. Fix: a new place now goes directly in front of the heading of the next place that follows it in the saved order and is already in the file. If no such neighbour exists, it goes at the end. Edits are applied in one forward pass, so all other bytes stay as they were (frontmatter, prose, comments, other places, line endings). The write is still atomic through `Root::write_checked`. New unit tests in calternal-location: restore into the middle, at the start (frontmatter and intro stay first), before a heading on line 1, at the end, with neighbours edited meanwhile, and with CRLF. Gates: - `cargo fmt --check`: clean - `cargo clippy -p calternal-location -p calternal-server --all-targets -- -D warnings`: `Finished`, no warnings - `cargo test -p calternal-location`: unit `test result: ok. 6 passed; 0 failed`; tests/places.rs `test result: ok. 11 passed; 0 failed` - `cargo test -p calternal-server`: `test result: ok. 108 passed; 0 failed; 3 ignored`

kayg referenced this issue from a commit

2026-10-03 02:45:12 +00:00

fix(location): restore a Saved place at its previous position in Places.md (#828)

Rows
Columns

Location: deleting a Saved place has no Undo action #828