diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..1fe4ca2 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,108 @@ +# 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/requirements.md`; this file is +about habits, not features. + +## Catalog reads: work is proportional to what changed, never to library size + +`docs/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.