Doc: mark Phase 0 done; record single-owner refactor and vision status

This commit is contained in:
Greg Pomerantz 2026-08-16 01:33:39 -04:00
parent 58725a5e6c
commit 21bd9fe02e

View File

@ -49,14 +49,15 @@ Status: v2, 2026-08-16. Written against the **live** repo `/home/gmp/pad`
## 3. Current state of the live repo (post housekeeping, 2026-08-16) ## 3. Current state of the live repo (post housekeeping, 2026-08-16)
- `go build ./...` green. **`go test ./...` now green** (was red: `internal/editor` - `go build ./...` green. **`go test ./...` and `go test -race ./...` green.**
and `internal/test/e2e` failed to build after `NewLogic` gained a `path` and - `03fb638` fixed the red build (harness + 5 call sites after `NewLogic` gained
`openfunc` parameter). Commit `03fb638` fixed the harness + 5 call sites; the e2e a `path` and `openfunc` parameter); the e2e harness uses a no-op `openfunc`
harness now uses a no-op `openfunc` (matching `impl_other.go`, whose real (matching `impl_other.go`, whose real `OpenFile` is a no-op off-Android).
`OpenFile` is a no-op off-Android). - `7240b62` committed the JNI/Termux open-file bridge + word-wrap-aware
- Uncommitted (yours, left untouched): JNI/Termux bridge, chunk-buffer + logic viewport/scroll WIP as the working-tree baseline.
changes. **Recommend committing these** so the working tree is a clean baseline - `58725a5` made state single-owner and turned the race detector green
before the IME work. (see §13).
- The working tree is a clean, race-clean baseline. Ready for the IME work.
## 4. The IME wiring gap (verified, testable) — THE central work item ## 4. The IME wiring gap (verified, testable) — THE central work item
@ -94,10 +95,10 @@ autocorrect produce clean, non-duplicated text.
## 5. Phases (Path B) ## 5. Phases (Path B)
### Phase 0 — clean baseline (small) ### Phase 0 — clean baseline (small) — DONE (`58725a5`)
1. Commit the uncommitted JNI/chunk-buffer work. 1. Commit the uncommitted JNI/chunk-buffer work.`7240b62`.
2. `go test -race ./...` green (add `-race`; the channel model may surface races — 2. `go test -race ./...` green (add `-race`; the channel model may surface races —
fix any found). fix any found).`58725a5`. Details in §13.
### Phase 1 — complete the IME (§4) ### Phase 1 — complete the IME (§4)
Implement items 14, with **Go unit tests** for item 3 (range-replace on the Implement items 14, with **Go unit tests** for item 3 (range-replace on the
@ -186,9 +187,12 @@ The in-app selftest drives the **real state machine through the real draw path**
(browser entries → open file → edit → autosave deadline → reload-prompt decision), (browser entries → open file → edit → autosave deadline → reload-prompt decision),
logging PASS/FAIL to a file the host `adb pull`s and asserts. logging PASS/FAIL to a file the host `adb pull`s and asserts.
### 9.3 Emulator observation layer (this 16 GB VM, KVM verified; agent has NO vision) ### 9.3 Emulator observation layer (this 16 GB VM, KVM verified)
Gio renders into one GL surface, so Android's view hierarchy exposes nothing about Gio renders into one GL surface, so Android's view hierarchy exposes nothing about
our UI. The debug loop is data-based — *more* precise than pixels for this codebase: our UI. The debug loop is data-based — *more* precise than pixels for this codebase.
Vision is now enabled and verified in-session (screenshots can be read), but it is
only an **auxiliary** check: state dumps, logcat, gfxinfo, and file diffs remain
the authoritative correctness signals.
1. **State-dump debug flag** (add in Phase 1): debug builds write `app.State` 1. **State-dump debug flag** (add in Phase 1): debug builds write `app.State`
(browser entries, editor text length, cursor px, scroll, dirty, row geometry) (browser entries, editor text length, cursor px, scroll, dirty, row geometry)
as JSON to the app data dir on a magic tap or 2 s interval; host `adb pull`s as JSON to the app data dir on a magic tap or 2 s interval; host `adb pull`s
@ -214,8 +218,8 @@ within ~1 s. Repo: `go test -race ./...` green.
- **§4.1 caret plumbing** is coupled to the cursor-positioning geometry; if that - **§4.1 caret plumbing** is coupled to the cursor-positioning geometry; if that
geometry is itself buggy, the IME will inherit it. Mitigation: the state-dump geometry is itself buggy, the IME will inherit it. Mitigation: the state-dump
reports caret px, so we can assert it against expected values in tests. reports caret px, so we can assert it against expected values in tests.
- **Channel model + `-race`** (Phase 0): the existing goroutine/channel design may - **Channel model + `-race`** (Phase 0)**resolved** in `58725a5` (§13);
have latent races; running `-race` before IME work avoids conflating them. `-race` is green and is now a standing regression gate.
- **Chunked buffer at 10 MB** (Phase 3): unproven on-device; the measurement is - **Chunked buffer at 10 MB** (Phase 3): unproven on-device; the measurement is
the gate for the size claim. the gate for the size claim.
- **AOSP keyboard** is a proxy for real IMEs (Gboard, etc.); final swipe/autocorrect - **AOSP keyboard** is a proxy for real IMEs (Gboard, etc.); final swipe/autocorrect
@ -232,3 +236,45 @@ that motivated the rebuild is actually ~4 small wirings away on the existing cod
fragile on-device** (e.g., the caret/snippet plumbing drags in a long tail of fragile on-device** (e.g., the caret/snippet plumbing drags in a long tail of
cursor-positioning bugs that are cheaper to not own). The emulator (Phase 2) is the cursor-positioning bugs that are cheaper to not own). The emulator (Phase 2) is the
instrument that makes that call. instrument that makes that call.
## 13. Single-owner state refactor (`58725a5`)
`go test -race ./...` was red. The races were architectural, not incidental —
several violated `doc/architecture.md §1` (logic goroutine = sole owner of
mutable state). Fixes, in order of impact:
1. **Frame is the only cross-goroutine state carrier.** `editor.Frame` now
carries `Elems`, `Scale`, `FocusedElementID`, and `Query`. The main goroutine
reads *only* the frame-receiver-stored snapshot (under its mutex); it no
longer calls `logic.State()` for scale/focus/search. `Renderer.Draw` takes
the scale as a parameter; the `ScaleProvider` indirection is gone.
2. **Gio-mutable widgets are main-owned.** `browser.BrowserState.SearchEditor`
(a `widget.Editor`) was removed. Main owns the search `widget.Editor` and
forwards its text via `SearchQueryChan`; the logic stores the result in
`Browser.Query`. More generally, `Renderer` now owns all `widget.Editor`
instances (registered by element ID) because Gio mutates them during draw.
3. **Autosave is owner-mediated.** The 1 s debounce timer goroutine no longer
reads editor state; it sends a `struct{}` token on `autosaveChan` and the
owner reconstructs content and dispatches the write.
4. **Shutdown is ordered and waitable.** `Shutdown()` = `Done()`
`WaitForExit()``FlushAll()``workerPool.Stop()`, so `FlushAll` (lock-free)
only ever runs after the owner has exited or from the owner itself.
5. **Tests inspect via the owner.** `Harness.Inspect/WithState/FileLoaded/
FullContent/CursorPosition` (and the in-package `withState`/`l.Inspect`
helpers) execute callbacks on the logic goroutine. `Harness.Run` now panics
if called twice — `TestTypeAtStartOfBuffer` and
`TestEditorClickToMoveCursorWithScroll` had a *double `Run()`* (two logic
loops on one state) that was the largest race source.
6. **Test-only flakes fixed:** `TestWorkerPool_PriorityPreemption` was rewritten
deterministically (gate task holds the worker while both priorities queue —
the old version raced the worker's task pickup; Go's `select` is random when
both channels are ready, so the old ordering guarantee was unimplementable as
written). `TestLazyLoadingLargeDirectory` timeout raised 2 s → 15 s (a 10k
entry index build under `-race` exceeds 2 s).
Residual (accepted) invariants:
- `editor.TheState` is a global used by package-level mutators (`GoToEditor`,
`OpenFile`, `HandleInsert`, …); it is only safe to call them from the owner
goroutine (input-handler closures run on the owner via `SendInput`).
- `Logic.FlushAll` is lock-free by design; it must not run concurrently with
`Run` (guaranteed by the `Shutdown` order).