ARCADIA-Studio/harness/review-architecture.md

Independent architecture review — 2026-09-05

Scope: current integrated Rust core, language, persistence, desktop message/draft state, views, diagram, tests, architecture and execution documents. Read-only implementation review; no Computer Use or build/test execution by this reviewer.

Material finding

High impact: help navigation allowed two competing editable drafts. Resolved in source; runtime verification remains with the lead.

app.rs deliberately lets Page::Guide open without applying source so that syntax help is available for invalid drafts. Previously views.rs also displayed the graphical inspector on Guide. A user could edit valid source, open help, edit an inspector property, then Apply or Save. apply() calls apply_source() first; successful source replacement calls sync_selection(), overwriting that inspector draft before draft_dirty() examines it. This was a reachable silent-loss path, not merely hypothetical state complexity.

The lead changed views.rs:170 to omit the inspector on both Text and Guide. I reread the updated condition and confirmed the competing editor is no longer rendered on the exception route. Keep Guide read-only. Runtime regression: select an element, edit source without applying, open help, verify no inspector and unchanged draft, return to Text and apply; repeat with invalid source to verify help remains reachable and the draft survives. If a future guide adds editable controls, route them through the same apply boundary before exposing a second draft.

Architecture assessment

• The two-crate boundary is appropriate: no Iced dependency in oa-core; parsing, shared structural validation, transactional history and persistence operate on the same model. Desktop mutation paths inspected use Session::execute, and loaded candidates pass persistence validation before session adoption.
• Whole-project text replacement is one transaction. This avoids an unnecessary second model store and synchronization framework. Human-readable aliases are presentation; persisted UUIDs retain identity. Neither a dynamic plugin ABI nor an incremental parser framework is warranted for the current prototype.
• Separate source-draft history and committed-model history have different jobs and are presented separately. Bounded snapshots are a defensible modest-model tradeoff. Large-model profiling and delta history remain future work, not prerequisites for the MVP.
• App is large, but its current event loop and a few explicit apply/sync helpers are understandable. A broad state-machine rewrite would add risk during integration. The discovered loss path needed a local ownership fix, now made. Future additions should group draft ownership and file-operation state only when a concrete new transition requires it.
• Session::project remains publicly mutable and Session::new assumes a valid candidate. Thus the mutation boundary is enforced by call-site convention rather than Rust privacy. Current desktop call sites inspected honor it; this is a nonblocking library-hardening opportunity, not evidence of an existing application defect.
• No blocking unnecessary abstraction or speculative infrastructure found. Native-platform file selection and renderer limitations are explicit implementation/platform tradeoffs and require the lead's runtime evidence; this review does not certify that evidence.

Result: no unresolved high-impact architecture finding in the inspected source after the Guide fix. Runtime re-verification of that fix and final build/test evidence remain separate completion gates.

Lead integration closure: the read-only Guide and invalid-draft recovery were subsequently exercised in the actual Windows application by the lead/UI reviewer. Final Windows/Linux checks and native runtime evidence are recorded in docs/validation.md; no architecture finding remains open.