dtourolle
6b1aac477d
docs/ had 26 developer documents flat beside the manual, and the two audiences are very differently sized: most readers want the manual and the gesture reference, a few want the register, the designs and the measurements. The manual and gestures.md stay at the top; everything for someone changing the code moves to docs/dev/, and the two documents that name their own successors — the v0.1 milestone and the UI-refinement plan — go to docs/dev/archive/ rather than being deleted, since both are still cited. docs/README.md is the index, users first. Every reference follows: code comments, Cargo manifests, the workflows, the pre-commit hook, the bench and traceability tools (which locate the repo root by docs/dev/requirements.md now), packaging, the Docker READMEs, CLAUDE.md, CONTRIBUTING.md and the README. The matrix links one level deeper and is regenerated. Links out of the moved documents into the tree gain a level; a link checker over every Markdown file finds none broken.
109 lines
5.8 KiB
Markdown
109 lines
5.8 KiB
Markdown
# Working in this repository
|
|
|
|
Notes for anyone — person or agent — changing this code. They record what
|
|
went wrong once and what the fix looked like, so the same shape is not
|
|
written again. Requirements live in `docs/dev/requirements.md`; this file is
|
|
about habits, not features.
|
|
|
|
## Catalog reads: work is proportional to what changed, never to library size
|
|
|
|
`docs/dev/catalog.md §1` states the rule. These are the ways it was broken on
|
|
the Identity screen, found when every confirm click cost half a second on a
|
|
24k-image library (2026-09-19), and what each fix looked like.
|
|
|
|
**A redraw must know what changed.** A click handler that calls "refresh
|
|
everything" pays for everything. `identity_ui::refresh` takes a `Changed`:
|
|
a confirm re-reads the rail and the grid and *not* the coverage line,
|
|
because moving a face between people cannot alter how many images are
|
|
indexed. Before adding a read to a shared refresh, ask which events can
|
|
change its answer, and gate it on those.
|
|
|
|
**Count with `COUNT(*)`, never with `.len()` on a list you then drop.**
|
|
`repairs::counts` used to build every repair's work list — a `Target` with
|
|
its path per row, sorted into visiting order — to report its length. Six
|
|
repairs, 350 ms, nothing kept. If the caller wants a number, the query
|
|
returns a number.
|
|
|
|
**One query, not one per row.** `ThumbStore::contains` in a filter over
|
|
5,000 rows is 5,000 prepared statements; `ThumbStore::held(size)` reads the
|
|
index once into a set. The same applies to any `query_row` inside a loop
|
|
over a result set — including `deep_count` per sidebar row, which is fine
|
|
at sidebar scale and would not be at grid scale. Aggregate in one
|
|
statement and look up in memory.
|
|
|
|
**Filter and aggregate in SQL, and aggregate the small side first.**
|
|
`faces::people` read 19,000 rows, grouped, sorted them by name, and the
|
|
screen threw 17,000 away (empty unnamed groups). `people_in_use` filters in
|
|
the `WHERE`, and joins `people` to a pre-aggregated `face_person` (2,000
|
|
groups) rather than grouping after a `LEFT JOIN` over every person. The
|
|
sort then sees only the rows that will be drawn.
|
|
|
|
**Wide rows make "just check one column" a table scan.** A `faces` row is
|
|
~8 KB (a 1 KB embedding and a ~5 KB crop, then the columns added later).
|
|
Any predicate that reads `quality`, `crop` or an eye column for every face
|
|
reads every row. V17 learned this for the eye filter; V19 applies it to the
|
|
repair counts with partial indexes (`faces_owed_*`) that hold only the rows
|
|
still owing, keyed on what the predicate joins on and carrying `model_id`
|
|
because the predicate reads it. Two things to know about them:
|
|
|
|
- **Drive the count from the small side.** SQLite uses a partial index
|
|
when the query starts from `faces` (`repairs::count`, `Needs::Face`) and
|
|
ignores it inside a correlated `EXISTS (... WHERE f.image_id = i.id ...)`.
|
|
That is why `Needs::Face` carries the per-face fragment and spells it two
|
|
ways.
|
|
- **Spell the predicate as the index's `WHERE` is spelled.** `NEEDS_EYES`
|
|
is `(f.eye_right IS NULL OR f.landmarks_dense IS NULL)` because
|
|
`faces_owed_eyes` is `WHERE eye_right IS NULL OR landmarks_dense IS NULL`.
|
|
Change one, change both, and `counts_are_the_sizes_of_the_lists` will
|
|
tell you if they drift.
|
|
|
|
Check a query's plan with `EXPLAIN QUERY PLAN` against a copy of a real
|
|
catalog before trusting an index exists for it: "SEARCH ... USING COVERING
|
|
INDEX" is the answer you want, "SEARCH f USING INDEX faces_image" on a wide
|
|
table means every probe opens a row.
|
|
|
|
## Catalog writes: one transaction per user action
|
|
|
|
`faces::confirm` opens a transaction. Calling it in a loop over a group is
|
|
a commit per face; `faces::confirm_all` is two statements and one commit,
|
|
`faces::reassign` one transaction for a whole split. When a UI action
|
|
touches N rows, give the catalog a function that takes the N, not a loop
|
|
that calls the one-row function N times — `unchecked_transaction` cannot
|
|
nest, so this has to be designed in at the catalog layer, not wrapped
|
|
from above.
|
|
|
|
## Screens: keep what is already decoded
|
|
|
|
`identity::load_faces` takes the crops the grid is currently showing and
|
|
hands them back into the new cells. Before that, a click re-read 4 MB of
|
|
crop blobs and decoded 700 JPEGs to produce the pixels already on screen.
|
|
When a redraw replaces a model, the expensive parts of the old model — a
|
|
decoded image, a cut portrait — are the first thing to reuse; only the row
|
|
that changed needs new work. Drain the old cells rather than cloning them.
|
|
|
|
## Remote calls: one round trip per file, not one per ancestor
|
|
|
|
`NextcloudBackend::move_to` guaranteed its destination's parent with a
|
|
`MKCOL` per ancestor from the account root, on every file of a batch —
|
|
three `405`s before each `MOVE`. The backend now remembers the collections
|
|
it has confirmed (`known_dirs`) for its lifetime, which is one job. When a
|
|
per-file operation has a per-batch precondition, satisfy it once.
|
|
|
|
## Measuring
|
|
|
|
`cargo run --release -p dr-ui --example identity_bench -- CATALOG THUMBS`
|
|
times what one click on the Identity screen reads and what the batch
|
|
operations write. Run it against a **copy** of a real catalog (it writes),
|
|
never the library's own file; `sqlite3 catalog.sqlite ".backup copy.sqlite"`
|
|
takes a consistent one while the app runs. Compare the `cpu` column when
|
|
other builds are running on the machine — the wall clock doubles under
|
|
load, the CPU figure does not. Keep the binary from before the change and
|
|
run both back to back rather than trusting numbers taken an hour apart.
|
|
|
|
Reference figures from the 2026-09-19 fixes, largest person (754 faces),
|
|
24k images, 19k faces, before → after. What one click read: `load_people`
|
|
22 ms → 12 ms, `load_faces` 316 ms → 2.4 ms, `audit` 190 ms → not run
|
|
(66 ms when it is, on open and at the end of a sweep). What one click
|
|
wrote: `confirm_all` 16 ms → 2 ms, `split_off` 23 ms → 4.5 ms. A click on
|
|
the face grid went from ~530 ms of catalog work to ~15 ms.
|