Establish OA/SA prototype baseline and first-release work
This commit is contained in:
@@ -0,0 +1,67 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user