25 lines
3.8 KiB
Markdown
25 lines
3.8 KiB
Markdown
# 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.
|