Repository navigation
feat(ir): write the compact canonical v4 spelling by default - #48
Conversation
The writer half of morphir-ukvn.8.16: write the compact canonical v4 spelling the profile prefers, flatten documentation per decision 0010, and keep the expanded spelling selectable.
The v4 encoders now write a node without attributes in its shortest
schema-valid form, as morphir-core 0.2.0 writes it, and keep the
expanded form for nodes with attributes. Compiler output decodes and
re-encodes byte for byte. Documentation is flattened with doc first
instead of the nested {doc, value} wrapper. Spelling.EXPANDED, passed
as spelling= or selected with use_spelling, writes every node expanded,
and ir convert gains --v4-spelling. Golden files prove the expanded
output is otherwise unchanged; tests that quote expanded upstream or
reader cases now select it explicitly.
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. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 726ac9d800
ℹ️ 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 "Variable": | ||
| return members["name"] |
There was a problem hiding this comment.
When a type variable's canonical name contains an initialism or starts with a digit, returning its name as a bare string produces output that the decoder rejects. For example, decode_type({'Variable': {'name': 'USD'}}) succeeds, but encoding that model now yields 'USD', which fails in _decode_type_string because _TYPE_VARIABLE_STRING permits only lowercase-leading names with lowercase/digit segments. The same regression occurs for valid names such as item-USD and 123, so default serialization can make previously readable distributions unreadable. Use the bare string only when it matches the type-variable string grammar, and otherwise retain a tagged variable spelling.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified: decode_type({"Variable": {"name": "USD"}}) re-encoded to "USD", which the bare type-variable grammar rejects. Fixed in e66a00d: the writer uses the bare string only when it matches _TYPE_VARIABLE_STRING, and otherwise writes {"Variable": name}, which is still compact. Added test_a_variable_outside_the_bare_string_grammar_keeps_its_tag for USD, item-USD and 123; the full suite passes, 2501 tests.
…ammar
The schema's bare type-variable string is lowercase only, so names such
as USD, item-USD or 123 were written in a form the reader rejects. Write
them as {"Variable": name}, which is still compact and reads back.
OpenSpec change
write-compact-ir-v4, the writer half of beadmorphir-ukvn.8.16(groupmorphir-ukvn.8.16.1).The pinned finos/morphir v4 JSON profile (
efa36060, unchanged upstream) says writers SHOULD emit the compact canonical vocabulary. This repository's writer emitted only the expanded spelling and the pre-decision nested{doc, value}wrapper, 30–35% larger than morphir-core's output.Compact by default. A node without attributes takes its shortest form, following morphir-core 0.2.0's
serde_v4.rs:{"Reference": [fqn, args...]}, a tuple as{"Tuple": [...]};Literal,Constructor,Reference,Variable,FieldFunction,TupleandListwith the payload directly under the tag;TuplePatternandLiteralPatternthe same way.A node with attributes keeps the expanded spelling,
attributesfirst.Flattened documentation.
doccomes first beside documented types and values (decision 0010). Module definitions keepdocaftertypesandvalues, as morphir-core writes them.Expanded on request.
Spelling.EXPANDED, passed asspelling=or selected with theuse_spellingcontext manager (task-local), andmorphir-python ir convert --v4-spelling expanded.BREAKING for consumers that accept only the expanded spelling. Every schema-valid reader, including this library and morphir-core, reads both.
Out of scope, still deferred on upstream: legacy-spelling warnings, kit diagnostic renames,
Unionand 4.1.0 annotations, and the morphir-diwy downgrade codes.Verification
mise run check: lint, format, mypy and pyright strict, 2498 tests, 34 behave scenarios.stubtestand theuse_spellingdoctest pass.