ARCADIA-Studio/harness/review-requirements.md

Independent requirements review

Reviewed 2026-09-05 against the complete attached user objective, docs/language.rst, current Rust implementation/tests, README, scope, acceptance, plan and state. No implementation edits or Computer Use were performed by this reviewer. Independent focused test execution is recorded below; parent-reported runtime results are distinguished from inspected code.

Material finding

R1 — identity edits are lost outside the dedicated button (fixed; focused re-review).

App::draft_dirty in crates/oa-desktop/src/app.rs checks names, descriptions, kinds, endpoints and positions, but not identity. Message::Identity changes the input. From a clean project, entering a different UUID leaves dirty() false, so Request(New) resets the project without an unsaved-work guard. Selecting another object calls apply(), which sees no property draft, then sync_selection() overwrites the entered UUID. Even ordinary Apply discards the ID input without applying it. Only the separate Change ID button uses the entered value.

This contradicts specification section 6: edited property drafts contribute to unsaved work, failed edits preserve drafts, and Save first applies valid drafts. The identity field edits an existing persistent model field, unlike preparatory controls for creating a new relationship. Integrate identity into validated property application, or preserve and guard its pending state explicitly. Verify valid/invalid identity edits, Save, selection changes, New/Cancel and reference updates; avoid partially applying companion properties before an invalid identity is rejected.

The integrated fix adds identity to draft_dirty, parses it before any property command, and combines identity plus companion properties through EditElement(old, value), EditRelation(old, value), and EditProject(id, name). Their shared reidentification helper rewrites references on a candidate before validation/adoption, making collision failures atomic and successful property/identity edits one history entry. Ordinary Apply, navigation and Save use this path; Change ID delegates to it. Inspected new desktop regressions cover all three object categories, guards, collision/invalid input, reference integrity and one-step Undo. Parent reports 49 full-workspace tests passing. This reviewer independently built the desktop tests with the D: target directory and ran the resulting test binary with identity_ --nocapture: all three identity/source-identity tests passed, exit 0. Valid Save captures the post-apply session.project, so the saved snapshot contains the new identity. R1 is closed by focused code/test review; GUI verification remains a separate gate.

R2 — an action can retain the old target across identity auto-apply (P2, fixed).

Select an object, edit its UUID, then click its Delete control. The rendered button carries Command::Delete(old). Message::Command first applies the identity draft, then executes that old command. Delete is now a successful no-op, leaving the object present with a new identity. Process Remove/Up/Down controls likewise embed the old process ID and report a missing element after committing the new ID. Connect snapshots source/target choices before applying drafts and can submit an obsolete endpoint when that selected endpoint is being reidentified. Selecting the same canvas/explorer object first applies the ID then selects the obsolete ID, clearing selection.

These are concrete reachable single-click workflows, not asynchronous hypothetical races. They do not corrupt committed model data and can be recovered by repeating the action; severity is P2. Apply must either remap action targets after a successful identity change or resolve the user's target after application. Include both command targets and relation endpoint choices; preserve the intended selection as well.

Focused re-review: the fix remaps selected command targets across Apply, preserves same-item selection, remaps existing builder choices only for identity commands and resolves Connect choices after Apply. Independently ran cargo test --locked -p oa-desktop identity_: all four tests passed, including the new process reorder/remove, selection, delete and connection regression. These single-message paths are resolved.

The first fix left a residual drag path: ButtonPressed retained an old ID in State.drag, while Select auto-applied the new identity before ButtonReleased submitted Move(old). The final fix preserves a draft when reselecting the current object outside Text; that selection is a no-op, so drag identity remains valid until release. Move then applies the draft and remaps its old ID in the same message. Selection changes and leaving Text still apply drafts as required. The expanded regression sends Select(current), asserts the ID draft remains pending, then sends Move(old) and checks the surviving new-ID object's position. Independently reran cargo test --locked -p oa-desktop identity_ after this fix: all four focused tests passed, exit 0. R2 is closed by code/test review; actual drag/identity operation remains part of the root's separate native GUI retest.

Implementation coverage

• The Rust/Iced two-crate application implements the specified OA MVP: five element kinds, four typed directed relations, cardinality checks, ordered/repeated process interactions, history, model checks, diagram/explorer/narrative views, and durable files.
• The graphical and textual representations share Project, the validator and transactional session. Every persistent field has both a source representation and a graphical route, including exact strings, coordinates, kind/endpoint changes and identity. R1 affects the draft contract, rather than the existence of an identity operation.
• The normative reST document covers abstract values, lexical syntax/EBNF, structural/warning semantics, forward resolution, canonicalization, equivalence, graphical editing, transaction/draft guards, persistence, SVG export and examples. The language is explicitly an OA prototype language, with the same domain boundary in both representations.
• Inspected tests cover complete model round-trip, canonical idempotence, aliases/Unicode/coordinates, strict malformed input, process order/repetition, shared validation, identity reference updates, atomic replacement and JSON/text equivalence. Application state tests now include the R1 transition and action-after-identity paths described in R2, including the press/release message sequence.
• cargo metadata --offline --locked --format-version 1 found no dependency declaring a Rust minimum above the workspace's stated 1.90. Actual 1.90 build verification is not claimed.
• Scope documents candidly list unsupported Capella concepts and remaining usability/performance tradeoffs. The objective requests an MVP and asks to try to address known downsides; the implementation makes concrete improvements without claiming complete Capella or SysML conformance. I found no additional explicit required OA feature silently available only in one representation.

Open completion gates (expected pending verification)

1. Complete the native GUI retest of identity/drag changes and close material findings from the other independent reviewers. R1 and R2 focused code/test verification is complete.
2. Record real-app editing of existing relation kind/endpoints, process append/reorder/remove, and IDs/reference preservation. Source-created objects plus graphical rename alone do not establish these newly exposed workflows. Root reports Windows source authoring, graph synchronization/save/error guards and 1100×700/1440×900/maximized inspection; these are useful but narrower evidence.
3. Exercise the current Linux editor/file workflow on the actual native build, including text-to-graph-to-text and save/reopen. The 44-test native pass predates the latest focused fixes; run final checks after integration.
4. Complete the clean documented build, final full-workspace checks, refreshed Windows/Linux artifacts and reproducible startup guidance. Existing historical artifacts and CI configuration do not prove the final revision.
5. Reconcile harness/STATE.md, docs/validation.md and docs/acceptance.md with final commands/runtime/review outcomes. The inspected validation record still describes the earlier 18-test MVP; the updated README and scope correctly defer final acceptance.

The reported R1/R2 findings are closed by independent focused review. No unresolved blocking or high-impact requirement defect remains from this review. Overall completion remains unproven until the runtime/build/documentation gates have current evidence. No claim of actual rendered-UI quality is made by this reviewer.

Lead integration closure — after the independent review

All five completion gates listed above now have final evidence in docs/validation.md and docs/acceptance.md. The independent UI reviewer exercised existing relation/process operations and the final release's pending-ID drag, closing all material UI findings. The lead completed the native Linux text Apply/save/reopen and process-view workflow. Windows and Linux each passed the final 50-test suite, strict Clippy, formatting and optimized builds; both refreshed archives were checked against executable hashes. Documentation is reconciled. This note records integration evidence and does not imply this reviewer personally performed the lead's GUI checks.