Repository navigation
feat(ir): realign the v4 model with upstream and drop per-input attributes on request - #35
Conversation
Plan the realignment of the Python v4 model and draft codec with upstream finos/morphir v4 at the pinned commit (morphir-ukvn.8.14) and an explicit, reported drop of per-input value attributes (morphir-ukvn.8.15).
v4 JSON spells a value-definition input as a bare type (kit case definitions-0027; morphir-rust classic_to_v4 drops v3 input attributes), so the morphir-elm reference model, whose typed inputs carry resolved types, failed v4 encoding with no_v4_spelling. Add drop_input_attributes for any v4 distribution, a drop_input_attributes keyword on v3_to_v4, and ir convert --drop-input-attributes. Each input that loses information reports input_attributes_dropped. The default migration stays lossless. Refs: morphir-ukvn.8.15
Add morphir.ir.v4.incompleteness (UnresolvedReference, DeletedDuringRefactor, TypeMismatch; Hole and Draft incompleteness) after the finos/morphir v4 schema at efa36060 and the morphir-rust v4 model. Hole.reason is now a typed HoleReason. Add IncompleteTypeDefinition with its expanded spelling (kit definitions-0014, -0015, -0024, -0026), and make reverse migration refuse it with unsupported_v4_downgrade. Refs: morphir-ukvn.8.14.2
… to v4 Add NativeBody (with NativeInfo and the five native hints), ExternalBody (platform bindings and an optional fallback) and IncompleteBody beside the expression body, after the finos/morphir v4 schema at efa36060. Module values and let bindings hold any of the four (ValueDefinitionBody). The codec reads and writes them in expanded form with the kit refusals (definitions-0007 to -0009, -0016, -0025, -0030); reverse migration refuses them with unsupported_v4_downgrade, and drop_input_attributes walks their expressions. Refs: morphir-ukvn.8.14.1
Add morphir.ir.v4.annotation (compact and structured annotations, positional and named arguments) after the finos/morphir v4 schema at efa36060 and the morphir-rust v4 model. Specifications get an annotations field with an empty default. The codec writes it first and only when non-empty, omits empty arguments, and keeps refusing annotations on definitions (kit definitions-0020 to -0023). Reverse migration refuses annotations with unsupported_v4_downgrade. Refs: morphir-ukvn.8.14.3
Add type.Union (two or more alternatives, order, duplicates and nesting kept)
after finos/morphir docs/spec/draft/types.md and the morphir-moonbit reference
implementation (morphir-ukvn.7.9.1). The codec spells it
{"Union": {"attributes"?, "alternatives": [...]}} and refuses fewer than two
alternatives; map_attributes and the morphir.ir.type shim cover it; reverse
migration refuses it with unsupported_v4_downgrade.
Refs: morphir-ukvn.8.14.4
…aligned model Mark the Native and External value nodes and TypeConstraints as deprecated, naming their replacements; update the codec's gap list and the package README for the new bodies, incompleteness, annotations and Union. Follow-up removal is morphir-ukvn.8.18. Refs: morphir-ukvn.8.14.5
Merge the change's deltas into openspec/specs (ir/v4, ir/migrate, tools/ir-interchange) and fix the archived design's asset paths. Refs: morphir-ukvn.8.14, morphir-ukvn.8.15
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14b63098ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case dist4.SpecsDistribution(): | ||
| return Ok(dist) | ||
| case dist4.LibraryDistribution(package, definition, dependencies): | ||
| result = dist4.LibraryDistribution( | ||
| package, job.package_def(definition, loc), dependencies |
There was a problem hiding this comment.
Traverse annotation arguments when dropping input attributes
When a structured annotation argument contains a LetDefinition or LetRecursion, its expression-body inputs can carry attributes that prevent v4 encoding. Specifications now support arbitrary value expressions as annotation arguments, so returning a SpecsDistribution unchanged skips these definitions; leaving Library/Application dependencies untouched has the same problem. A Specs distribution with such an argument returns no warnings from drop_input_attributes, but encoding the result still raises no_v4_spelling and recommends calling that same helper. Traverse module, type, and value specification annotations and apply the existing value walker to their arguments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2299f95. drop_input_attributes now walks structured annotation arguments (positional and named) on module, type and value specifications, the Specs distribution's specification, Library dependency specifications and Application dependency definitions. Tests: TestInputDropInSpecifications in tests/unit/ir/test_v4_upstream.py (each case fails to encode before the drop and encodes after it).
Annotation arguments are expressions, so a let binding in a module, type or value specification annotation can carry input attributes that block v4 encoding. drop_input_attributes now walks those arguments, the Specs distribution, Library dependency specifications and Application dependency definitions, instead of returning them unchanged. Refs: morphir-ukvn.8.15
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Realigns the Python IR v4 model and the draft v4 codec with upstream finos/morphir v4 at the pinned commit
efa36060(still finos/morphir main), and lets the morphir-elm reference model reach v4 JSON. OpenSpec changealign-ir-v4-with-upstream, archived in this PR.Per-input attributes (
morphir-ukvn.8.15). Upstream v4 has no slot for them: the reference model spells an input as a bare type, kit casedefinitions-0027refuses an attributed input, and morphir-rustclassic_to_v4.rsdrops the v3 input attributes. So no upstream proposal is needed. This PR adds an explicit, reported drop:morphir.ir.migrate.drop_input_attributes(dist)for any v4 distribution (module-level, let-bound and fallback/partial expressions);v3_to_v4(..., drop_input_attributes=True);morphir-tools ir convert --to v4 --drop-input-attributes.Each input that loses information reports the new warning
input_attributes_dropped. The reference model now converts with 10 warnings (8 module-level, 2 let-bound inputs). Without the opt-in nothing changes: the in-memory v3→v4→v3 round trip stays lossless and encoding still fails withno_v4_spelling, whose message now names the flag.Model realignment (
morphir-ukvn.8.14). The shapes follow the pinned schema, the kit cases and the morphir-rust v4 model:NativeBody(withNativeInfoand the five native hints),ExternalBody(platform bindings, unique by platform, plus an optional fallback) andIncompleteBodybeside the expression body. Module values and let bindings hold any of the four (ValueDefinitionBody).morphir.ir.v4.incompleteness: typed hole reasons and theHole/Draftkinds;IncompleteTypeDefinition. Breaking (draft API):Hole.reasonis now a typedHoleReason, not raw JSON.morphir.ir.v4.annotation: compact and structured annotations on module, type and value specifications, written first and only when non-empty.Uniontype (two or more alternatives) fromdocs/spec/draft/types.md, implemented upstream in finos/morphir-moonbit #155.unsupported_v4_downgradeat its location.Native/Externalvalue nodes andTypeConstraintsare kept but deprecated. Their removal is filed asmorphir-ukvn.8.18.The codec still reads and writes only the expanded spelling. Compact and legacy spellings wait for the re-sync in
morphir-ukvn.8.16.Test plan
mise run check: lint, mypy and pyright strict, 1863 testsmise run coverage: 98%mise run test-bdd: 23 scenariosdefinitions-0007–0009,0014–0016,0020–0026,0030covered in expanded form (tests/unit/ir/test_v4_upstream.py)openspec validate --specs --strict