Files
ARCADIA-Studio/harness/review-v2-requirements-architecture.md

12 KiB
Raw Permalink Blame History

V2 independent requirements and architecture review

Date: 2026-09-05. Reviewer: fresh v2_requirements_arch_review agent. Scope: user objective, PLAN/STATE, normative docs/language.rst, current core/desktop/service implementation, research and deployment docs. Source was read-only; no Computer Use was performed because the lead owns the desktop. This is not a substitute for the separate runtime acceptance pass.

Latest disposition after integration

R1–R7 are addressed in the current implementation/contract. Independently re-read the changed code: conflict cards now identify the object pair/field and both values, keep resolution controls after selection, accept merged text and display warning messages; the source caption explains comment loss; the role rule explicitly describes its advisory warning; and root StateMachine choices agree with incomplete-model validation. The core now exports counterpart-kind choices rather than requiring a divergent desktop policy.

An independent executable rerun confirms the offered Capability→Mission mapping has no conflicts. cargo test -p oa-desktop --locked forms:: passed all six focused form tests, including placement relocation plus UUID change with one-step undo, shared perspective movement preserving payload, incompatible-dependency rejection and filtered references. The README V2 source example was also parsed successfully. The Branch constructor fix remains independently verified.

No unresolved source-level blocking requirements or architecture finding remains from this review. Native interaction/visual acceptance of the updated conflict and membership controls belongs to the lead's runtime pass; this reviewer does not infer it from passing reducer tests. The original findings below remain as the audit trail, with their initial pending language superseded by this disposition. Manual desktop/API transfer is now documented and was exercised over real HTTP; the lead confirmed the exported changes in the native GUI, recorded in review-v2-api-transfer.md.

Findings requiring disposition

R1 — P1: counterpart conflicts cannot be meaningfully reviewed in the dialog

crates/oa-desktop/src/workflow.rs, conflict rendering around line 450, displays only the generic conflict message plus Keep target / Take source. It does not identify the source, target, field, or either value. Conflicted fields are omitted from plan.changes, so the separate change list does not supply the missing comparison. Two simultaneous Name/Description conflicts are indistinguishable. This undermines the primary reviewed bidirectional update workflow and can lead an engineer to overwrite the wrong property. Show labeled source/target values and field identity, with the existing core merged-value decision if practical. Also display actual warning messages; the present dialog shows only their count.

Verification: independently traced plan_transition conflict generation and Workflow::view; reported promptly to lead. Pending implementation and actual GUI verification.

R2 — P1: advertised capability/mission transitions are rejected by core

workflow::counterpart_kinds offers OA Capability -> SA Mission and SA Mission -> OA Capability. transitions::compatible_kinds only supports Capability <-> SystemCapability, participant choices, and inferred same-kind shared mappings. Choosing Mission therefore yields an unrecoverable automatic-mapping conflict. Implement the offered method mapping (a Manual trace can express the special correspondence), or remove unsupported menu choices and disclose scope consistently.

Verification: external executable probe constructed an OA Capability and an explicit Mission mapping; plan_transition returned Counterpart kind is not valid for this automatic transition. Probe is at D:/CodexBuilds/Capella2-v2-01a06e2e/requirements-review, outside repository sources. Pending fix/reverification.

R3 — P2: two stored membership choices lack direct graphical editing

  • forms::Draft::from_project stores a Node's containing diagram in RecordKind::Node(diagram) out of band. Draft::command always edits the node within that diagram. There is no graphical destination-diagram property or move-placement operation. In source, changing the node's diagram @alias preserves node UUID and artifact references while moving its membership.
  • Shared elements derive perspective from the Oa/Sa body wrapper. The inspector offers kinds only in the current layer and hides those wrappers. A source edit can move an isolated shared element to the opposite layer while preserving its identity, but no corresponding graphical operation is exposed.

The normative contract explicitly promises every persistent property in both editing forms. Add explicit validated move operations, preserving references and identity. This is about stored membership parity, not automatic cross-layer realization or silent drag semantics. Pending disposition.

R4 — P2: source editor does not explain comment loss

docs/language.rst section 7 requires the editor to explain that canonical Apply discards comments/whitespace. Neither the Text workspace caption nor the Engineering guide currently mentions this. A short source caption is sufficient; comments are especially plausible for the promised human/AI/Git authoring workflow. Pending UI copy fix.

R5 — P2: normative role-allocation rule disagrees with diagnostic severity

Section 4 says a direct allocation must not contradict role-realizing performers. validation::validate_relation reports role_allocation_conflict as a Warning, allowing the candidate to commit/save. Advisory diagnosis can be a reasonable authoring decision, but specify it as such instead of an unconditional prohibition. Pending contract or behavior alignment.

R6 — P2: state-machine parent-choice predicate excludes a valid incomplete model

validation::can_parent(... StateMachine, None) returns false, disabling At root and potentially hiding a valid parent choice from filtered forms. Normative section 4 and validate permit an unowned machine with a warning. Align the creation/reference predicate with that explicit policy. Pending fix.

Finding already fixed during review

R7 — Initial Branch creation under a Fragment previously used make_element's default Exchange body in the desktop, while only Command::AddElement converted it to Branch. The UI enabled In selected, then validation rejected creation before the user could change the variant. The lead/core worker moved the context-sensitive construction into Project::make_element during this review. The independent executable probe now confirms both the desktop-style CreateElement(make_element(...)) and AddElement accept a Branch under a Fragment. Actual scenario UI authoring must still be exercised after integration.

This fix is also a concrete architecture improvement: one context-aware constructor avoids desktop/core command divergence.

Coverage and scope assessment

  • Core exposes OA participants/roles/work/goals/processes and SA System/Actors/functions/missions/capabilities/chains, with typed relations, decomposition, leaf allocation, independent diagram placements and traces. Shared payloads cover data/items/interfaces/ports, rich scenario occurrences/fragments, state-machine regions/transitions/actions/availability and constraints. These are substantive OA/SA semantics, beyond renamed V1 enums.
  • Text has a closed versioned language, stable aliases/UUIDs, typed constructors, reference resolution, canonical ordering, tolerant tokens/lint, strict validation and explicit deterministic V1 migration. The normative contract is detailed enough to assess supported field parity, but the inconsistencies above must be corrected before calling it satisfied.
  • Desktop source exposes creation/editing for element/relation/diagram/placement/trace/artifact record families, ordered occurrence lists and typed variant controls. Source editing and graphical edits share the Session boundary. Behavior graphics are actively being replaced by the lead; old flat lists are a known pending milestone, not an independently verified completed behavior view.
  • REST uses the same Project/Command/language/validator, with resource identity separate from editable model identity, ETag preconditions, atomic envelopes, bounded requests and a single-owner local store. Query, source, validation, transaction, transition and artifact routes exist. Deployment recipes are concrete; Docker and installed remote service execution remain unverified, as documented.
  • Desktop hosted-model editing is absent: no API endpoint/open/save client exists. This is not automatically a demand for collaboration. The literal backend requirement can be met by the independent hosted model service, but final limitations must say that desktop and hosted models transfer by import/export. Acceptance should exercise desktop export -> REST import/edit/export -> desktop reopen so the claimed integration is demonstrated.
  • Transition scope is a reference/ownership closure, not all related content. An independent probe confirmed Whole subtree with two selected activities does not include their connecting interaction. Allocation relationships and allocated work are likewise not implicit ownership children. Explain this in user-facing scope/help rather than promising a complete architectural copy. Existing-target updates deliberately compare names/descriptions only, not structural synchronization; the normative spec discloses this boundary.
  • LA/PA components/deployment, simulation, native .capella/.aird interchange, complete SysML/Capella conformance, advanced capability include/extend/generalization, distributed collaboration and retained service history are legitimate declared prototype boundaries. This review does not request those systems as speculative extensions.

Architecture and simplicity assessment

The three-crate boundary is appropriate: one renderer-independent typed core, an Iced interaction/draft layer, and a headless transport/persistence layer. Candidate validation before adoption is shared by all editing paths. UUID references and persisted aliases serve separate clear purposes. Diagrams do not own semantics. Projection reuse for canvas/Fit/SVG avoids duplicate layout meaning. Full snapshot history and whole-envelope persistence are reasonable for the explicit bounded prototype; no speculative plugin ABI/database/event framework is needed.

The material maintainability risk is duplicated domain-choice policy in desktop helpers, evidenced by R2 and R7. Expose a small core query for permitted transition target kinds, analogous to can_parent/can_connect, rather than maintaining a separately diverging match. Keep generic JSON only as a closed temporary form adapter; future additions need a graphical reachability test, not just a lossless no-op serialization test. The existing every_demo_record_has_a_lossless_graphical_draft proves preservation but cannot prove that every control/action exists.

No additional architecture blocker was found. Avoid broad module rearrangement solely for line-count/style preferences. Final README/architecture/scope/acceptance documents are still V1 and must be rewritten as the lead already plans; they currently contain false V2 statements about one diagram, transient aliases, process exchange IDs and absent roles/hierarchies.

Verification performed by this reviewer

  • Read the user objective and authoritative plan/contract, inspected core model/session/references/projection/transitions/validation plus graphical drafts/builders/views and service routes/store/deployment boundaries.
  • Ran a standalone Rust executable against the current oa-core: reproduced R2; verified the integrated R7 constructor fix; established actual transition scope behavior for connected work endpoints.
  • Did not run the native GUI, Linux application, full regression suite, service HTTP tests or packaging. Those are separate lead/runtime/QA responsibilities and remain required completion evidence.

Initial review status was incomplete; the latest disposition above records the subsequent fixes and independent focused verification. Overall product completion still depends on the lead's final runtime/platform/package evidence.