# nigig-site — Brutal Assessment & Execution Plan

Date: 2026-09-10. Scope: `crates/apps/nigig-site` (+ `matrix_client/src/nimanyatta.rs`,
`nigig_doc_scanner/src/scanner_core.rs` as consumed). Status at writing:
`cargo check --lib --bins --tests` clean, `cargo test -p nigig-site` 24/24 pass.
Verdict: **a solid single-device MVP with real PDF/Word output — not a field-ready
multi-site product.** What works is genuinely good; what is stubbed is load-bearing.

## 1. Architecture — C+

- Single JSON file as the database (`store.rs:57`). Atomic rename + fsync + `0o600`
  (`store.rs:100`) and versioned migration (`STORE_VERSION`, `store.rs:20`) are real,
  but it is still one pretty-printed file rewritten per mutation, with no schema
  versioning beyond an integer and no indexes. Fine for 5 sites; not for 50.
- Persistence race fixed structurally: one FIFO writer thread (`store.rs:143-170`,
  `enqueue_save`) replaced clone-and-forget `spawn_async` saves. Ordering guarantee
  rests on mpsc FIFO — correct, but untested under real concurrency (see Tests).
- Reads are cached by nanosecond mtime (`load_cached`, `store.rs:128`) with identical
  freshness to `load()`. Residual risk: two writes landing in the same nanosecond
  (coarse filesystems) serve one-tick-stale data; acceptable, documented here.
- God-store persists: sites, reports, workers, reminders, tasks, procurement,
  suppliers, meetings, directories, selection in one struct (`store.rs:22-38`).
  Every mutation serializes everything. Splitting bounded contexts is deferred.
- Chat is two stacks pretending to be one: Makepad `HttpRequest` in
  `nimanyatta_client.rs` vs reqwest `HttpClient` in `matrix_client/src/nimanyatta.rs`,
  sharing zero types with `../nimanyatta`. Vendoring is still a README paragraph.
- `crates/nimanyatta` in this repo is still a stub manifest. The server is not vendored.

## 2. Performance — B-

- UI thread is clean on paper: saves queued, OCR/PDF/D-Bus offloaded (`workers.rs`,
  `report_editor.rs`, `reports.rs:256`, `scheduler.rs:48`), preview parses off-thread
  via `ReportPreviewReady`. `PortalList` for workers (`workers.rs:72`) and tasks.
- Remaining UI-thread parses: `set_pdf_bytes` (`report_preview.rs:164`) still parses
  on the UI thread — and is now dead code (no callers). `load_cached` is a `stat`
  per call site per frame in draw guards; cheap but multiplied across 8 screens.
- 60s `Timer` (`main.rs:52`) wakes the app forever, even with zero reminders.
  Killed app = zero reminders (no `AlarmManager`/`UNUserNotificationCenter`).
- No virtualization for meetings/procurement lists (small N, acceptable for now).

## 3. Bugs — what is real vs fixed

Fixed and verified: label-on-View panics (approvals/meetings), dead More-hub buttons
(`nav_index` + test), site list never rendering, silent date drops (validated with
errors), month string-compare, hash-fake worker IDs, EOD reminder 3h late (now real
EAT→UTC math with tests), OCR symlink refusal, approval race on global last-meeting.
- `store.rs:260` `report_key` — dead, no callers. Delete.
- `nimanyatta_client.rs:201` `SiteChatClient` — dead stub generating fake `room_*`
  IDs. Actively misleading. Delete.
- `reports.rs:207` `refresh()` — dead. Delete.
- `report_preview.rs:164` `set_pdf_bytes` — dead since `ReportPreviewReady`. Delete
  or wire; deletion is correct until the worker lands.
- `update_task_list`/`update_meetings_list` 5-vs-10 row caps are now vestigial
  (PortalList renders all rows in approvals; meetings still uses a static label —
  inconsistent, see plan).
- `backup_corrupt` writes non-atomically and `prune` sorts lexicographically, which
  only orders chronologically because of the `%Y%m%d%H%M%S` format — fragile but true;
  a comment must say so.
- `check_and_fire_due` does N `mark_fired` calls → N queued full-file writes per tick
  when many reminders fire at once. Batch it.

## 4. Design — C+

- Rich text is half-honest: toolbar flags persist into `RichText`, lists keep markers,
  bold reaches PDF/Word (`export_runs`). But the `TextInput` box itself is plain —
  users cannot see bold while typing. Full WYSIWYG needs `doc-ui` `DocEditor`.
- Meetings list and directory are static labels; approvals got `PortalList` but
  meetings/procurement did not. Inconsistent virtualization story.
- Chat thread is optimistic-local only; incoming history needs server `sync`.
- Supplier linking by name/ID text + tap-to-fill works but is one step below a picker.
- AI refine is a pass-through (`refine_via_hub` returns `None`); the UI copy is honest
  about it. Real provider wiring is future work, not debt.

## 5. Security — B-

- `NIMANYATTA_BASE_URL` allowlist (`host_of`/`check_url`) is real and tested; localhost
  HTTP allowed for dev, `user@host` tricks rejected.
- OCR paths canonicalized against created dirs; traversal refused and tested.
- `store.json` is `0o600`, XML escapes control chars.
- Gaps: bearer token comes from the Matrix session with no refresh path in this crate
  (401s surface as banners — good — but no re-login flow); `store.json` is plaintext
  at rest (worker national IDs included) — encryption is deferred, must be explicit;
  `cargo deny` has never been run for the new `image`/`zip` surface.

## 6. Code quality — B-

- 24 tests, all passing, covering the fixes that hurt before (tz, selection, allowlist,
  docx shape, export markers, store round-trips). Missing: store-concurrency test,
  widget-level test for hub/nav and template/schedule flows, chat 401 test.
- Duplication reduced (`entry_body`/`pack_docx`, `selected_or_first`) but `display_`
  vs `export_runs` duality and per-screen store boilerplate remain.
- Dead code listed in §3 must go before it rots further.

## Execution plan (this file is the tracker — check off as landed)

### Phase 1 — Dead code + small correctness (1 day) — LANDED 2026-09-10
- [x] Deleted `report_key`, `SiteChatClient` stub, `reports.refresh()`, dead
      `set_pdf_bytes`/`set_report`/`pdf_host` (all had zero callers).
- [x] `mark_fired` batching: `check_and_fire_due` does one mutation + one queued
      write per tick (`scheduler.rs`).
- [x] Comment on lexicographic backup sort fragility (`store.rs`).
- [x] `cargo deny`: tool not installed and environment is offline, could not run.
      Pinned surface instead — `Cargo.lock`: `image 0.25.10`, `zip 8.6.0`,
      `ulid 1.2.1`. Re-run `cargo deny check` on a networked machine before release.
- Exit: `check` clean, 24/24 tests green, no dead `pub` items in `nigig-site`.

### Phase 2 — Lists consistency + chat depth (2–3 days) — LANDED 2026-09-11
- [x] Meetings list → `PortalList` (same pattern as approvals/workers); tapping
      a row prefills the reschedule form.
- [x] Supplier picker: directory is a `PortalList`; tapping a row fills the
      material form's supplier field directly ("Use match" remains as a
      keyboard-friendly fallback).
- [x] Chat thread persisted: `store.chat_threads` (room → lines, cap 50),
      written on every send via `mutate`, restored on first draw and room
      switch; thread header shows room + selected site
      (`chat::render_thread`, test `chat_threads_persist_capped_per_room`).
- [x] Token refresh path: on 401 with a pending request, one
      `refresh_access_token` attempt then exactly-one retry; otherwise
      "Session expired — log in again." No retry loops.
- Exit: every list virtualized, thread survives restart, 401s self-heal once.

### Phase 3 — Storage honesty (2 days) — LANDED 2026-09-11
- [x] Store-concurrency test: 50 OS threads × `mutate(push_task)` against
      `NIGIG_SITE_STORE_PATH`, `flush_writer_queue`, disk truth asserts all 50
      (`store::tests::concurrent_mutations_all_survive`). Required a real fix,
      not just a test: the FIFO queue alone does NOT save read-modify-write
      races (every thread snapshots the same base), so all UI mutations now go
      through `SiteStore::mutate()` on a global lock, with saveless `push_*`
      cores (`push_site/report/task_entry/worker_scan/task/procurement/meeting/
      supplier/reminder/contact`) and one queued write per mutation. Reads via
      `SiteStore::read()` (no disk I/O after init); `load_cached()` retained for
      compat/tests. `INFLIGHT_WRITES` + `flush_writer_queue()` added for the test.
- [x] At-rest encryption: DECISION — not implemented. `store.json` holds worker
      national IDs in plaintext (`0o600`). Design: envelope encryption with a
      keyring-backed DEK following `nigig-core::credential_store` (keyring crate:
      Secret Service / Credential Manager / Keychain), AES-256-GCM via a
      vetted crate, versioned header for migration, decrypt-on-`load()` only.
      Scheduled as its own work item; do not ship regulated-site data before it.
- [x] God-store reads: mitigated, not split. Screens read the canonical clone
      (`read()`) instead of parsing JSON per handler; per-domain query stores
      remain future work — the clone cost is bounded (KBs) and no handler parses.
- Exit: concurrency proven (25/25 tests green), encryption decision recorded.

### Formerly deferred — landed 2026-09-11 (45/45 tests green)
- At-rest encryption (`src/crypto.rs`): AES-256-GCM envelopes (`NIGIG1` magic,
  versioned alg byte, random 12B nonce; explicit-plaintext envelope when no
  keystore — fail-explicit, never fail-silent). DEK is a random 256-bit key in
  the OS keystore (`keyring`: Keychain / Secret Service / Credential Manager).
  `save_to` seals before the atomic rename; `load` opens envelopes (legacy raw
  JSON accepted once, then re-sealed). UI shows actual header state
  (🔒/⚠) in the More hub. Tests use synthetic keys + kill-switch env; a shared
  `TEST_ENV_LOCK` + drain-on-drop guard fixed the cross-test env flake that
  killed the race test in-suite while passing alone. `aes-gcm`/`keyring`/
  `getrandom` all resolved offline from cache.
- Incoming chat history: `POST /sync` fetch + defensive per-room timeline parse
  (grounded in the server's `JoinedRoomSync` shape), merged into the active
  thread with tolerant room-id matching, persisted via `chat_threads`.
  E2E still needs a live server; parsing + merge are tested on fixtures.
- GIF encoding (`src/gif.rs`): self-contained GIF89a encoder on `weezl` LZW
  (already locked+cached; `image`'s gif feature and the `gif` crate are NOT
  cached, so no new crate was possible offline — and none was needed: makepad
  only decodes/displays GIFs, never encodes). 216-color cube, infinite loop,
  burst photos → `.gif` clip from the report editor. MP4 remains out (no
  encoder offline).
- AI provider wiring (`ai_refine.rs`): real `makepad-ai-hub` path
  (`ClaudeApiChatProvider` blocking transport, `ANTHROPIC_API_KEY`, 60s
  deadline) with graceful `None`; editor shows the original immediately and
  upgrades on `RefineReady`, keeps original + reason on `RefineFailed`.
- On-device STT (`meetings.rs`): `makepad-system-speech` (`Stt::available` /
  `prepare` / `listen` → 0.5s poll pump → finals appended to the transcript).
  Manual typing remains the honest fallback with engine-state status lines.
- True pixel preview (`reports.rs`): background PDF bytes → UI-thread parse of
  page 0 into real `PageContent` → mounted `PdfPageWidget`; label fallback
  shows meanwhile. Same pipeline as `pdf-makepad`'s test host.
- Catch-up reminders (`main.rs`): `check_and_fire_due` on `Startup`/`Resume`.
- Multi-device sync (`sync.rs` + More-hub card): union-by-id merge with LWW on
  `updated_at` (union-only elsewhere; deletions don't propagate — documented),
  60KB-chunked snapshots over room messages, push/pull UI with ack counting.
  E2E still needs a live server; codec + merge are tested.
- WYSIWYG: assessed against `doc-ui` `DocEditor` (4291-line CRDT editor with no
  host embed API — a 3–5-day integration with regression risk). Instead: toolbar
  toggles now show real selected state (`NavigationBarButton`), and formatting
  demonstrably survives into PDF/Word. Full embed stays deferred, deliberately.

### Landed 2026-09-11, fourth pass (50/50 tests green)
- WYSIWYG, real: the "no host API" assessment was wrong — `DocEditor` exposes
  `set_controller` / `controller` / `toggle_inline_style` / `serialize`, and
  `Document::new` + `StyleSpan` map 1:1 to `RichTextSegment`. The report editor
  now mounts `mod.widgets.DocEditor` behind a 📝 toggle with text carried both
  ways (`richtext_to_document` / `document_to_richtext`, round-trip tested),
  toolbar B/I/U driving the editor's native styling in rich mode, and save
  persisting styled runs. (API discovery note: typed access is
  `WidgetRef::borrow_mut::<T>()`, not a generated `as_*` accessor.)
- Video clips, real: MJPEG-AVI (`src/video.rs`) — JPEG frames via the `image`
  crate inside a minimal RIFF shell, playable in VLC/WMP/QuickTime. No H.264
  exists offline, so this is the honest video format, labeled as such in the
  UI ("Video clip (MJPEG-AVI)"). Frame-level round-trip tests included.
- iOS scheduling: verified unwritable here — the `objc2-user-notifications`
  sources are not even in the local registry cache, and no iOS target is
  installed. Stays documented-unavailable per the crate's own rule.

### Still deferred (decisions, not gaps)
MP4/H.264 (no offline encoder) · iOS scheduled delivery (above) · incoming
history E2E (needs live server at test time; fixture-tested) · at-rest
encryption follow-through per Phase 3.

### Landed 2026-09-11, third pass — live server E2E (46/46 tests green)
- The rustc block is gone: a 1.99 nightly toolchain exists on this machine, and
  `nimanyatta` (`broadcast_server`, `b_server`) checks AND builds with it
  offline. `tests/sync_e2e.rs` runs the full loop against a live server —
  register → create room → send → `/sync` timeline → snapshot push → fetch →
  decode → merge — and PASSED FIRST TRY against `127.0.0.1:8080`, which
  simultaneously validates every endpoint shape, the 60KB chunk codec over the
  wire, and the merge policy end to end. Without `NIMANYATTA_E2E_URL` it
  passes trivially with a documented skip, so CI never needs a server.

### Landed 2026-09-11, second pass (45/45 site tests + 28 notification tests green)
- Killed-app alarms, where the platform allows: `robius-notification::schedule_at`
  (macOS `deliveryDate` — system-held, survives process death; past dates
  deliver immediately, matching catch-up semantics). Linux/Windows/Android/iOS
  return explicit `unsupported` instead of pretending. `nigig-site` registers
  every reminder with the OS at schedule time (tagged by reminder id, so a
  later in-foreground fire replaces rather than duplicates); the 60s timer
  remains the delivery path elsewhere. New `NSDate` feature on the
  `objc2-foundation` dep (was missing → compile error, fixed).
- On-device AI key settings (More hub): session-only `ANTHROPIC_API_KEY`
  set/clear with live status; never written to disk.
- Incoming chat history: `POST /sync` + defensive per-room timeline parse
  (grounded in the server's real `JoinedRoomSync`/`TimelineEvent` shapes),
  merged into the active thread behind a ⟳ Sync button; fixture-tested.

## Appendix A — makepad rev bump to b9a083c (2026-09-10)

HEAD `c901f1d` moved the fork past what compiles out of the box. Adaptations made,
all verified by `cargo check` + `cargo test -p nigig-site` (24/24):

1. `~/.cargo/.../b9a083c/widgets/src/lib.rs`: dropped `enter_isolate`,
   `leave_isolate`, `IsolateEntry` from the `widget_async` import (marked
   `NIGIG-LOCAL-PATCH`). The items exist nowhere in the rev and are referenced
   nowhere in this repo — leftover import from the removed isolate feature.
   **Real fix belongs upstream in the fork; this patch evaporates on cache clean
   or the next bump. Re-verify after either event.**
2. `pdf-makepad/src/renderer.rs`: the concurrent pdf WIP added
   `RenderCommand::SetOverprint`; the match needed its arm (landed alongside).
3. `nigig-site/src/site_frame/screens/chat.rs`: fixed an `E0515` in the Phase 2
   token-refresh future (borrowed temporary `Client`; now owned by the async block).
4. Pre-existing WIP breaks fixed along the way (untracked files, not ours):
   `pdf-graphics/src/cff.rs` (`&Vec<f64>` fn-pointer coercion),
   `pdf-graphics/src/truetype.rs` (`&u8 as u16` casts, C-style chained assignment),
   `nigig_doc_scanner` home-module rename.
5. No `nigig-site` widget-API rewrites were needed: PortalList/Modal/PageFlip/
   TextInput APIs used here are unchanged in the new rev.