drk-blutspende-orte/docs/improvements.md

# Improvement roadmap

Review of the current code, prioritized for the actual deployment: **one
non-technical user, Linux Mint laptop, offline some of the time, no one else to
fix things when they break.** That ordering puts *data safety* and *robustness*
above features.

Priorities: 🔴 do first · 🟡 worth doing · 🟢 nice to have

**Status (2026-09-07):** items 1, 2 and 3 are implemented. Items 6 and 10 are
partly done (logging added; shared autocomplete helpers extracted; map now
`fit_bounds`; region list is now a constant). See
[changelog.md](changelog.md).

---

## 1. Data safety 🔴 — ✅ done

Implemented: atomic writes (`tmp` + `fsync` + `os.replace`), rotating backups in
`data/backups/` (startup + before every change, keep 20), `EmptyDataError` /
corrupt-file fallback to an empty frame or the newest backup, and all writes
(including append) routed through `_save_csv`. A "restore from backup" button in
the UI is still open; for now it is a manual file copy.

<details><summary>Original finding</summary>

`data/orte.csv` is the only copy of the data and there is no safety net.

- **No backups.** A bad edit, a botched delete, or a disk hiccup loses history
  permanently. Delete is explicitly irreversible and only guarded by a yes/no
  dialog.
- **Non-atomic full rewrites.** `update_row` / `delete_rows` do
  `df.to_csv(CSV_PATH)` directly. A crash or power loss mid-write can leave a
  truncated file.
- **External-edit clobber.** The CSV is read once at startup. If it is opened in
  a spreadsheet and changed while the app runs, the next save silently
  overwrites those changes.
- **Empty-file crash.** If `orte.csv` exists but is 0 bytes (e.g. an interrupted
  write), `pd.read_csv` raises `EmptyDataError` and the app won't start.

**Suggested changes**

- Write every full rewrite to a temp file in the same directory, then
  `os.replace()` it into place (atomic on POSIX).
- On startup, and before each destructive write, copy the current file to
  `data/backups/orte-YYYYMMDD-HHMMSS.csv`; keep the last ~20 and prune the rest.
- Add a "Backup jetzt erstellen" / "Aus Backup wiederherstellen" pair in Tab 2,
  or at least open the backups folder from a menu.
- Handle `EmptyDataError` / missing columns by falling back to an empty frame
  and logging a warning.

</details>

## 2. Geocoder thread robustness 🔴 — ✅ done

Implemented: every queue item is wrapped in `try/except Exception` (both the
geocode call and the callback), failures are logged and the loop continues.
`geocode_city` now raises `GeocodingUnavailable` when every attempt hit a
service/network error, and after 3 consecutive such failures the app shows a
one-time "Standortdienst nicht erreichbar" dialog. `RateLimiter` was not
adopted; the manual `sleep(1)` stays.

<details><summary>Original finding</summary>

`GeocoderWorker.run` has no error boundary around the per-item work. If
`geocode_city` or a callback raises anything other than
`GeocoderTimedOut` / `GeocoderServiceError` (a network stack error, a bug in a
callback, `GeocoderQuotaExceeded`), the **worker thread dies** and every
subsequent entry stays stuck on `⏳ wird gesucht…` with no error shown. The user
has no way to know geocoding stopped working.

**Suggested changes**

- Wrap each item's processing in `try/except Exception`, log the traceback, mark
  that row as failed, and keep the loop alive.
- Consider `geopy.extra.rate_limiter.RateLimiter` (with `swallow_exceptions`)
  instead of the manual `sleep(1)`.
- Surface a clear one-time status message when geocoding fails repeatedly
  ("Standortdienst nicht erreichbar – bitte Internetverbindung prüfen").

</details>

## 3. Easier, safer input 🔴 — ✅ done

Implemented: the date field is now `ttkbootstrap.DateEntry` (calendar picker) in
both the entry form and the edit dialog, with `parse_date()` validation on save
(a bad date is rejected with a dialog). The edit dialog gained manual
**Breitengrad / Längengrad** fields. Tab 2 shows rows without coordinates in red,
has a "Nur ohne Koordinaten" filter, a hint line with the count, and a
right-click **Koordinaten suchen** action.

<details><summary>Original finding</summary>

For the target user the free-text date field is the biggest usability risk.

- **Date field is unvalidated free text** in both the entry form and
  `EditDialog`. A typo like `2026-13-05` or `05.02.2026` is stored verbatim,
  breaks date sorting, and never appears correctly on the timeline.
- **No recourse when geocoding fails.** A row that comes back
  `⚠ nicht gefunden` saves with blank coordinates and then just silently never
  shows on the map. There is no way to type coordinates by hand or retry from
  Tab 2.

**Suggested changes**

- Replace the date `Entry` with `ttkbootstrap.DateEntry` (calendar picker) in
  both places. If keeping free text, validate on add/save and refuse invalid
  dates with a clear message.
- In `EditDialog`, add optional `lat` / `lon` fields so a location can be fixed
  manually.
- In Tab 2, highlight rows with missing coordinates (e.g. red text) and add a
  filter / right-click "Koordinaten suchen" so failed geocodes are visible and
  fixable after the fact.

</details>

## 4. Put it under version control 🟡 — partly done

`git init` done and `.gitignore` added (covers `.venv/`, `__pycache__/`,
`data/karte.html`, `data/backups/`, `data/app.log`). Still open: the initial
commit (left to Patrick), and the decision on committing `data/orte.csv` vs. an
example file.
- Decide on `data/orte.csv`: for a personal tool it's reasonable to commit it,
  or commit a small `data/orte.example.csv` and ignore the real one.
- This `docs/` folder.

Gives you history, a rollback path, and a clean way to push updates to the
laptop (`git pull`).

## 5. Packaging & distribution 🟡

Today install is: `apt install python3-tk`, create a venv, `pip install`. That's
a one-time terminal session, which is acceptable but fragile (loose version
pins mean a future `pip install` could pull an incompatible pandas/folium).

**Suggested changes**

- Pin all four dependencies to exact versions and regenerate deliberately.
- Ship the `run.sh` + `.desktop` launcher from [setup.md](setup.md) in the repo.
- Optional: build a [PyInstaller](https://pyinstaller.org/) one-file bundle on a
  matching Linux box. It bundles Python, Tk, and all deps, so install becomes
  "copy one file + double-click" with no apt/venv step. Trade-off: you build it,
  and rebuild on dependency updates.

## 6. Diagnostics / logging 🟡 — ✅ done

`_setup_logging()` writes a `RotatingFileHandler` to `data/app.log`
(512 KB × 3). Saves, map generation, geocode failures and uncaught exceptions
are logged. `_on_open_map` now logs and narrows its handling. Open: no in-app
"open log folder" shortcut yet.

## 7. Stable row identity 🟡

Rows are keyed by the pandas index, which is renumbered after every delete. An
in-flight geocode callback for one row can land on another row if the user
deletes something in between. `_apply_edit_geocode` guards against a *deleted*
index but not a *reused* one.

**Suggested change:** add an immutable `id` column (uuid or incrementing int)
written to the CSV, and match callbacks on that instead of the positional index.

## 8. Duplicate handling 🟡

`_on_save_all` detects likely duplicates but saves them anyway with only a
transient status-bar line the user will probably miss. Consider a modal
"Diese Einträge existieren schon – trotzdem speichern?" with a per-row choice,
or at least skip exact duplicates by default.

## 9. Offline map 🟢

`karte.html` loads Leaflet/jQuery/Bootstrap from CDNs and tiles from CartoDB, so
the map is blank without internet even though all the data is local. If offline
use matters, vendor the Leaflet JS/CSS into `data/` and post-process the Folium
output to point at local files (tiles would still need caching or an offline
tile pack — larger effort).

## 10. Small code-quality items 🟢 — partly done

- ✅ Shared autocomplete helpers (`filter_locations`, `split_city_plz`)
  extracted; `EditDialog` and `App` both use them.
- ✅ Region list is now the `GEOCODE_REGIONS` constant; map centre/zoom are
  `MAP_DEFAULT_CENTER` / `MAP_DEFAULT_ZOOM`.
- ✅ `generate_map` now `fit_bounds` to the markers.
- ⬜ Add a `tests/` folder with pytest coverage for `DataStore` (append / update
  / delete / `find_duplicates`) and the input helpers. A non-GUI smoke script
  exists (used during development) but is not in the repo — worth formalizing.
- ⬜ Add `ruff` / `mypy` config.

---

## Suggested order of remaining work

1. Initial git commit (item 4), then decide on committing `orte.csv`.
2. Pinned deps + `run.sh` / `.desktop` launcher in the repo (item 5).
3. Stable `id` column (item 7) and a modal duplicate prompt (item 8).
4. Formalize tests (item 10); offline map (item 9) only if offline use becomes real.