ARCADIA-Studio/harness/review-v2-correctness.md

Independent V2 correctness review

Review date: 2026-09-05. Read-only review of current core, text/persistence and desktop state flow against PLAN and language.rst; no Computer Use (separate UI reviewer owns it). Implementation-owner conclusions were not treated as proof. Review remains open until the findings below are independently reverified.

Evidence actually run

• cargo test -p oa-core --locked --offline on Windows, D: target: 41 tests pass (12 language, 1 process diagnostics, 28 workflows). Existing tests do not cover the counterexamples below.
• Standalone executable probe outside the repository: D:/CodexBuilds/Capella2-v2-01a06e2e/correctness-review/src/main.rs, with Cargo manifest in its parent. Run using cargo run --manifest-path D:/CodexBuilds/Capella2-v2-01a06e2e/correctness-review/Cargo.toml --offline and the same D: target. It reproduces findings C1–C4 without modifying repository source.

Material findings

C1 — High: reviewed Keep Target value is overwritten on unchanged re-preview

Location: crates/oa-core/src/transitions.rs, field reconciliation around let safe (line 634 at review).

Reproduction: create OA Activity named Operational assessment and SA Function named System classification. Explicitly link them with Name = KeepTarget; apply. Without changing either object, preview the same OA source again with defaults.

Observed: no conflicts, action Update, Name before System classification -> Operational assessment. Apply overwrites the value the engineer just chose to preserve. The safe test checks only the destination against its baseline; it never compares the source against its own baseline. The existing conflict test stops immediately after KeepTarget and misses the second preview.

Expected: unchanged reviewed divergent values remain unchanged in either direction. Only a source change should trigger normal propagation; genuine subsequent competing changes should still conflict. Root assigned a worker fix and focused regressions; independent re-probe pending.

C2 — High: normal Save overwrites external edits without a conflict

Location: crates/oa-core/src/persistence.rs unconditional atomic replacement, and desktop save/load snapshot flow.

Reproduction: save/open a model; change the disk model name externally to External update to preserve; change only the loaded in-memory description; save loaded snapshot to the original path.

Observed before fix: save returns success; external name is reverted to Untitled mission. This is ordinary Git/text-editor/second-instance use of the requested text-first workflow. Atomic replacement prevents partial files, but does not prevent lost updates. The language spec originally excluded conflict resolution from the storage contract; PLAN nevertheless explicitly requires file-conflict checks. Root accepted this as an application safety requirement and is adding a checked save path. Independent verification must exercise the desktop guarded path; the explicit low-level overwrite helper may remain available.

C3 — Medium: same-parent Reparent changes behavior ordering

Location: crates/oa-core/src/session.rs, Command::Reparent (line 420 at review).

Reproduction: create a process with two distinct occurrences, then execute Reparent on its first occurrence with the exact existing parent ID.

Observed: [first, second] becomes [second, first]; revision 2 becomes 3. Unconditional detach/attach makes an idempotent ownership update silently alter sequence semantics. Fragment branches use the same mechanism and also require coverage.

Expected: unchanged ownership preserves order, history and revision. Root assigned worker fix; independent re-probe pending.

C4 — Medium: primary child-creation control cannot populate a fragment

Location: crates/oa-desktop/src/app.rs, add_element (line 1095 at review).

Reproduction: select an Alternative Fragment occurrence, select Occurrence kind, then use + In selected. The enabled state comes from can_parent, which returns true. Desktop constructs make_element followed by CreateElement, yielding the default Exchange occurrence under the fragment.

Observed via the exact core candidate path: Err(ModelError("occurrence_owner: Nested occurrences belong to branches; branches belong to fragments.")). The existing core AddElement command correctly creates Branch for this parent and succeeds. Thus the primary GUI workflow disagrees with its enabled state and shared command behavior; a multi-field manual workaround is not an adequate primary flow.

Expected: create an empty Branch under Fragment, and ordinary occurrences under Branch. Root notified; runtime confirmation and focused reducer regression remain required.

Extension of a previously known finding

Root's same-kind data-reset fix also applies to the constructor pickers in forms.rs, not only ElementKind/RelationKind. The picker determines its selected entry using same_variant, but emits a fresh default payload even when the selected constructor is unchanged. Re-selecting Enumeration erases literals; Named data type clears the referenced type to nil; Exchange occurrence clears exchange/sender/receiver. Root notified; treat this as part of the existing same-kind fix rather than a duplicate independent issue.

Review limits

No claims about native rendering, pointer behavior, Linux execution, service HTTP routes, or finished requirements coverage are made here. These belong to the parallel reviews. No additional blocking reference-cleanup or text round-trip defect was found in the reviewed paths; this does not supersede the findings above.

Focused re-probe during integration

The final standalone probe was rebuilt and run again after the workers' current changes became visible (exit 0). Observed results:

• C1: unchanged re-preview now reports Reuse, empty fields, no conflicts; the SA name remains System classification.
• C2: save_checked with the exact bytes originally read returns Err(Conflict) after an external name edit; an assertion verifies the external name survives. The explicit low-level save still overwrites by design. Desktop wiring and its user-facing conflict workflow still need runtime verification.
• C3: unchanged-parent Reparent now preserves [first, second] and revision remains 2 -> 2.
• C4: the exact make_element/CreateElement path now succeeds, as does core AddElement. GUI child creation still needs runtime verification.

These independent probes confirm the originally reported core counterexamples are fixed in this snapshot. The implementation worker's broader new regressions and the desktop constructor-picker fix still require the final focused review once source stabilizes. Per root request, this review turn is ending to free an implementation slot; it does not claim final application acceptance.