nigig-org/crates/apps/nigig-site/NIGIG_SITE_ASSESSMENT_AND_PLAN.md

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

• Deleted report_key, SiteChatClient stub, reports.refresh(), dead set_pdf_bytes/set_report/pdf_host (all had zero callers).
• mark_fired batching: check_and_fire_due does one mutation + one queued write per tick (scheduler.rs).
• Comment on lexicographic backup sort fragility (store.rs).
• 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

• Meetings list → PortalList (same pattern as approvals/workers); tapping a row prefills the reschedule form.
• 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).
• 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).
• 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

• 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.
• 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.
• 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.