finos / finos/morphir

Audit morphir-ui IR normalization and Insight/XRay parity

Open
#795 0 comments 0 reactions 0 assignees View on GitHub
ir-bindings javascript
Dominant language
Rust
Stars
209
Forks
65
Avg merge
8h 55m
Merged PRs (30d)
126

Description

## Parent

#792

## What to review

Run one deliberate, read-only audit of the `morphir-ui` side of IR ingestion. Build the full contract matrix before reporting findings. Do not review only the latest diff and do not stop at the first defect.

The audit starts at browser file/folder input and connected-session model payloads. It follows each input through format dispatch, V3/V4 decoding, name and access-wrapper normalization, the shared AST, definition selection, Insight, and XRay.

Cover these profiles:

- V3 single-file JSON and YAML
- V4 single-file JSON and YAML
- V4 JSON and YAML document trees/VFS
- browser file/folder and connected `morphir live` entry paths wherever applicable

For every schema-defined type, pattern, literal, and value expression, enumerate all accepted compact and expanded spellings. Include documentation, attributes, recursive nesting, malformed inputs, and unknown future tags. Equivalent inputs must normalize to equivalent ASTs and render the same selected definition in Insight and XRay.

Use the V3 and V4 schemas plus the JSON, YAML, and document-tree profile specifications as the contract. Treat a route that has no complete loader as a missing capability, not automatically as a decoder bug.

## Review history that must remain in scope

Re-prove these PR #790 findings against the current source. A prior fix is a regression anchor, not a reason to skip the case.

- Review `5103916579`: nested tuple patterns and subjects misaligned Insight decision-table columns. The fixes were morphir-ui commits `887d95c`, `050322f`, `e822422`, and `8b785e7`, vendored in finos/morphir commit `298e00ce`.
- Review `5104505026`: `formatVersion: "4.0.0"` and schema-valid flattened V4 access wrappers were rejected. The fixes were morphir-ui commits `d7487d3`, `bdab4a4`, and `28c5558`, vendored in finos/morphir commit `d121e9bd`.
- Review `5105072844`: compact V4 `Variable`, `Reference`, `Constructor`, `Tuple`, `List`, and `Literal` payloads degraded to unknown nodes. The fixes were morphir-ui commits `06da156` and `21116b5`, vendored in finos/morphir commit `b2eb5615`.

The last fix intentionally received no further external bot review. Re-evaluate the own-data-property checks, inherited properties and accessors, scalar literal inference, and non-finite-number rejection added in `21116b5`.

Also retain the cross-boundary regressions from finos/morphir PR #756 review `5071344516`: the connected client must accept legitimate host responses larger than its outbound request limit, and browser-local folder loading must permit the vendored WebAssembly module under CSP without adding general `unsafe-eval`.

Do not silently add support for `4.1.0`. Record any conflict with the exact `4.0.0` release policy as a contract decision. The retired `product-(i-d)` spelling in the books fixture belongs to #793 and must not weaken the canonical name parser.

## Acceptance criteria

- [ ] The report includes a complete row-by-row representation matrix for all schema alternatives and all applicable profile/entry-path combinations. Each cell says `supported`, `rejected with diagnostic`, `degrades to unknown`, `not implemented`, or `not applicable`, with evidence.
- [ ] Every valid semantic node is traced through the shared AST and both Insight and XRay. The report identifies omissions, `UnknownNode` fallbacks, crashes, data loss, selection-state errors, and rendering differences.
- [ ] JSON/YAML, V3/V4 semantic equivalents, and single-file/document-tree equivalents are compared for normalized AST and rendered-result parity.
- [ ] The report includes a disposition and fresh evidence for each review item listed above, including the final fixes that did not receive another bot review.
- [ ] Findings cite the contract, source location, a minimal reproduction, expected and observed behavior, affected matrix cells, and the smallest sensible fix boundary.
- [ ] The report separates code defects, missing product capabilities, fixture/spec inconsistencies, and test-only gaps. It groups proposed follow-up work into delivery blockers and optional hardening.
- [ ] The audit produces one consolidated report. It does not edit code, add tests, push commits, comment on a PR, or request another bot review.

## Blocked by

None. This can start immediately and can run in parallel with the Rust audit.

## Handoff

Compare results with the Rust-side audit under #792. A connected-path cell counts as end-to-end supported only when both audits agree on the payload and behavior at the protocol boundary.

Contributor guide

Open the contributing guide

Research direction

Start with the V3/V4 schemas and the JSON, YAML, and document-tree profile specifications, then trace browser file/folder and connected morphir live payloads through format dispatch, decoding, normalization, the shared AST, Insight, and XRay. Re-prove the listed review findings and cross-boundary regressions against the current source. Done means one consolidated report with a complete evidence-backed matrix, parity results, dispositions, and separated defect, capability, fixture, and test-gap findings.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, wasm
Domain
frontend
Issue type
Documentation
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.