Repository navigation
feat: shared enum through multi-hop extends, list -last accessor, value-object rules - #426
Merged
Merged
Conversation
…-object rules constant - Shared-enum resolution walks the extends chain (TypeScript, C#, Java, Python), so an enum field that extends a field which extends the shared abstract references the shared type instead of inlining its own. C# EffectiveEnumValues reads through every hop. Kotlin keeps its per-object projection enum (#259). - Templates: every map-shaped collection element carries derived -first/-last booleans, so a list separator is {{^-last}}, {{/-last}} with no payload flag. All five ports render it identically; verify --templates accepts it inside a container section and flags it elsewhere. New render-list-separator and verify-positional-accessor-* fixtures. - TypeScript: a value object's module exports a field-rules constant, like an entity's <Entity>.<field>.rules, without the table and path keys.
…red enums Assisted-by: no-mistakes:claude:claude-sonnet-5-5
…yload views Assisted-by: no-mistakes:claude:claude-opus-5-5
…lver, C# cycle guard Assisted-by: no-mistakes:claude:claude-opus-5-5
…ted generators Assisted-by: no-mistakes:claude:claude-sonnet-5-5
…bject gate; Java re-eject note Assisted-by: no-mistakes:claude:claude-opus-5-5
…ad CHANGELOG wording Assisted-by: no-mistakes:claude:claude-opus-5-5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Changed
extendshops in all five ports. One resolver inserver/typescript/packages/metadata/src/core/field/shared-enum.tsis called by the loader and every codegen site. A field that redeclares@valuesor@intValueMapon the chain still gets its own inline enum. Java value objects and Kotlin generators now reference shared and@providedenums instead of nesting copies, which changes some generated names and needs the build settings and re-eject steps listed in the CHANGELOG.-lastaccessor on every object element of a collection, so{{^-last}}, {{/-last}}separators need no payload flag.meta verify --templatesaccepts-lastonly inside a section over a list of objects, and the render path (payload-accessorsin TypeScript, C#, Java, Kotlin and Python) addshas<Field>and-lastwithout changing rendered bytes for templates that do not use them.object.valuemodules now export aconst <ValueObject>with per-fieldrules(andname,label,view), matching the entity constant minus$tableand$path.meta genadds it to existing value-object modules, and three-way merge keeps hand edits.Gate runs
This PR took 2 gate runs.
main, applied two more review fix rounds and one test-step fix round, and passed.Two notes on the validation details below:
object.valuegetting the field-rules constant) does not reproduce on the final commit. The composer invalue-object-file.tsexcludes an abstract value object, a regression test covers it, and the test step was approved on that evidence.scripts/ci-local.sh --quick. The Docker integration lanes were not run.Validation
Risk Assessment
Testing
Eight TypeScript scenarios driven live against the CLI and render library from worktree source in disposable projects, with a base-commit comparison. All intended behaviors pass. One open question: abstract object.value gets the constant, contradicting the CHANGELOG (review ask-user item). Per-package regression all green: render 382, codegen-ts 2557, cli 1470, metadata 3061. Non-TS ports untested. Evidence in ~/.no-mistakes/evidence/01M4NBD22X9F5TYV6EYEHZZHDV. Worktree clean.
~/.no-mistakes/evidence/01M4NBD22X9F5TYV6EYEHZZHDV/gen-fixed)~/.no-mistakes/evidence/01M4NBD22X9F5TYV6EYEHZZHDV/gen-base)Evidence: Render scenario results
Pipeline
Updates from git push no-mistakes
✅ **Intent** - passed
✅ No issues found.
🔧 **Rebase** - 2 issues found → auto-fixed ✅
CHANGELOG.md- merge conflict rebasing onto origin/maindocs/CONFORMANCE.md- merge conflict rebasing onto origin/main🔧 Fix applied.
✅ Re-checked - no issues remain.
server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinTypeMapper.kt:204- Fix round 3 (8165ab4) changed the signatures of Kotlin helper functions that ejected generators call, and left no source-compatible form.mvn metaobjects:ejectcopies a generator's source verbatim into the adopter'scodegen/module (codegen-kotlin/pom.xmlship-eject-reference-source), and that copy keeps importing the helpers from themetaobjects-codegen-kotlinjar. Concrete sequence: an adopter ejectedKotlinEntityGeneratorbefore this change. The copy callsFr019SharedEnum.ProvidedEnumConfig.of(getArg(...), getArg(...)),KotlinEnumEmitter.emitEnumFile(obj, field, outRoot, emittedEnumFqns),Fr019SharedEnum.providedClassName(field, fr019Config)?.let {...},KotlinTypeMapper.enumTypeName(field, owner)andKotlinGenUtil.reportEnumClasses(obj, loader). After the upgrade none of these resolve, so the adopter's codegen module stops compiling, on a model with no@providedenum at all. Changed or removed signatures:KotlinTypeMapper.enumTypeName(KotlinTypeMapper.kt:204, two-argument form removed),ProvidedEnumConfig.of(Fr019SharedEnum.kt:41, removed),providedClassName(Fr019SharedEnum.kt, parameter and nullability changed),KotlinEnumEmitter.emitEnumFile(KotlinEnumEmitter.kt:35),KotlinM2mSupport.resolveandtphQueryFnNames(KotlinM2mSupport.kt:99, 223),KotlinGenUtil.reportEnumClasses(KotlinGenUtil.kt:699, 725),KotlinExtractSchemaEmitter.emitMirrorFilesandmirrorSource(KotlinExtractSchemaEmitter.kt:66, 84), and theKotlinApiModelBuilderconstructor. Ejectable generators whose old copies call them:KotlinEntityGenerator,KotlinExposedTableGenerator,KotlinSpringControllerGenerator,KotlinRelationsGenerator,KotlinOutputParserGenerator(GeneratorRegistry.kt:121-206). This is a sibling of the round-2 findingjava-payloadfield-ctor-break, which was fixed with a compatibility constructor; round 3 left the Kotlin helpers behind. The CHANGELOG [1.1.0] entry tells TypeScript adopters that an ejectedentitygenerator needs no resync and says nothing for Kotlin.docs/compatibility-policy.mdallows generator internals to change in a minor, so the remedy needs a decision, not the defect: either keep deprecated overloads with the old signatures that delegate with an empty provided config, or state in the CHANGELOG upgrade notes that an ejected Kotlin generator that types an enum must be re-ejected or updated to the new signatures.🔧 Fix applied.
3 issues (2 warnings, 1 info) still open:
server/java/render/src/main/java/com/metaobjects/render/PayloadAccessors.java:199-isViewabletreats a primitive array as an application object, so the Java renderer wraps it in aTypedViewand stops iterating it. This code came from the previous run's fix rounds (20ce9ed addedisViewable; 34f61b0 kept it when it made the view an overlay). Trace: a payload valueint[] {1,2,3}(a map value, a record component, a bean getter, or a Kotlin data classIntArray/DoubleArrayproperty, which uses the same path).withDerivedAccessorsskips it at line 185 because that branch requires a non-primitive component type. Line 191 then callsisViewable: the value is not a Number, CharSequence or Iterable, and its class name is[I, which matches none of thejava./javax./jdk./kotlin.prefixes, so it returns true. The engine receives aTypedView, which is aMap. Mustache.java 0.9.14AbstractObjectHandler.iterateiterates a value only when it is a List, Iterable, Iterator or array, so{{#scores}}{{.}},{{/scores}}renders the body once with[I@<hash>in place of1,2,3,.falseytreats any other object as truthy, so{{^scores}}none{{/scores}}over an empty primitive array renders nothing. At the base commit the array reached the engine untouched and rendered per element. No error is raised, and it breaks the standing constraint that existing templates render identical bytes. The new typed-payload tests useListmembers only. Fix: return false fromisViewablewhenpayload.getClass().isArray(). No sibling in the other ports: C# arrays take theIEnumerablebranch, TypeScript arrays takeArray.isArray, and Python sequences are handled before any view.server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringOutputParserGenerator.java:399- A Java adopter who owns (ejected) exactly one ofvalue-objectandoutput-parsergets generated code that no longer compiles after upgrading, and the CHANGELOG does not say so. Both generators are ejectable (codegen-spring/pom.xmlship-eject-reference-source). This is the Java sibling of the Kotlin owned-generator finding fixed in this run's round 1, which covered Kotlin only. Sequence: a response value objectTicketPayloadhaspriority extends Priority, a shared root abstract enum, one hop. Before this change the value-object generator nestedTicketPayload.Priorityand the parser emittedTicketPayload.Priority.valueOf(s). (1) Owned old parser copy with the upgraded package value-object generator: the record no longer nests the enum (SpringValueObjectGenerator.java:226 skips it, :202 types the component by the top-levelPriority), while the owned parser still emitspayloadClass + "." + SpringTypeMapper.enumTypeName(owner, ef), which isTicketPayload.Priority, a class that no longer exists. (2) Owned old value-object copy with the upgraded package parser: the record still nestsTicketPayload.Priority, and the parser now passesacme.shop.Priority.valueOf(s)(line 399,qualifiedTypeReference) to that constructor, which is an incompatible type. Both builds compiled before. An owneddtocopy is not affected, because it resolves through the jar'sFr019SharedEnum. CHANGELOG.md:579-598 lists the Java value-object change and CHANGELOG.md:610 gives Kotlin a re-eject note; Java has none. No compatibility overload can repair a disagreement in emitted shape, so the remedy needs a decision, not the defect: for example a Java upgrade note in the CHANGELOG and docs/ports/java.md saying an ownedvalue-objectoroutput-parsercopy must be re-ejected together with the other (mvn metaobjects:eject -Dnames=value-object,output-parser -Dforce, or the manual diff procedure for an edited copy).server/typescript/packages/codegen-ts/src/templates/value-object-file.ts:92- The field-rules constant is also emitted for an ABSTRACTobject.value, which the recorded decision and the CHANGELOG say does not happen. The decision reads: "Emit the field-rules constant only for the value-object subtype (use the named subtype constant). No constant for abstract objects, for an entity emitted to a contract-only target, for a TPH subtype or for a sourceless entity." The gate isobj.subType === OBJECT_SUBTYPE_VALUEand has no abstract check. An abstract value object such as a shared payload base reaches this composer through templates/entity-file.ts:75 whenemitAbstractShapesis on, which is the TypeScript default, soBaseAddress.tsnow exportsconst BaseAddress = { $entity, ... }. The comment at line 89 and CHANGELOG.md:120 ("Only the value subtype takes it: an abstract object ... are unchanged") both state the opposite, and the absence test in test/generators/entity-file.test.ts uses an abstract entity only. The two sentences of the decision conflict for this one case, so it needs a ruling: either add&& !obj.isAbstractto the gate with a test for an abstract value object, or keep the constant (the limits are real and concrete subtypes inherit them) and correct the comment and the CHANGELOG sentence.🔧 Fix applied.
2 infos still open:
CHANGELOG.md:100- The sentence "A typed object is overlaid, never copied: its own members (methods and fields included) resolve as they always did and only when the template names them" follows a list that includes a Java record, a C# object with[JsonPropertyName]and a Pydantic model. Those three shapes are not overlaid. They are copied into a map with every member read up front: server/java/render/src/main/java/com/metaobjects/render/PayloadAccessors.java:151-162 invokes every record accessor and recurses into each value; server/csharp/MetaObjects.Render/PayloadAccessors.cs:154-159 callsGetValueon every wire-named property; server/python/src/metaobjects/render/payload_accessors.py:113-115 callsmodel_dump(mode="json"). These are the generated payload types of the Java, C# and Python ports, and a method on a Java record that is not a component accessor does not resolve at all. The behavior is unchanged from the base commit, so nothing regressed; only the release note is wrong. The overlay (lazy, own members and string form kept) applies to a TypeScript class instance, a C# object without wire names, a Java getter-based bean and a Kotlin data class. Fix the wording: limit the overlay sentence to those four shapes and say that a record, a wire-named C# object and a Pydantic model are viewed as the map of their fields, as before. This sentence was written by the earlier fix round that made the C# and Java views an overlay.server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinTypedPayloadRenderTest.kt:122- The two JVM laziness tests assert only the rendered output, so they pass on an eager view that swallows read failures. Both were added in the previous run's fix round 2 (34f61b0), whose decision required lazy resolution and tests that fail on round 1's code. Round 1's Java view (20ce9ed,readersForloop) read every getter up front insidecatch (ReflectiveOperationException | RuntimeException e). Trace the Kotlin test on that code:Line.getExploding()throws and is swallowed,getSku()is read,-lastis injected, and the output isa,b. The test passes with the eager walk present. Sibling: server/java/render/src/test/java/com/metaobjects/render/TypedPayloadRenderTest.java:194 (aMemberTheTemplateNeverNamesIsNeverRead) fails on round 1's code only becauseskuis a package-private field, not because a member was read. The currentTypedView.resolveis lazy, so no product defect exists today. Fix in both tests: count reads instead of throwing only (a counter incremented in the unused getter, asserted 0 after render), so an eager view fails whether or not it swallows.cd server/typescript && bun test) timed out at the 1500-second wrapper (exit 124) with no output. No pass or fail result exists. Run per package, or with a longer timeout, and confirm the outcome before relying on the verdict.-laston a scalar list element: no-lastkey is set, so{{^-last}}is true for every element and a separator follows each one (["p","q"]renders ", , "). This matches the documented contract, since scalars carry no positional accessor. Template authors over scalar lists should use a different separator pattern or an object wrapper.Address.<field>.rules.maxLengthvalue 60scripts/ci-local.sh --only ts-fast --only ts-unit --strict-toolchainsBuilt disposable project under /tmp; ranbun server/typescript/packages/cli/bin/meta.ts genagainst it (live CLI)Inspected generated enums.ts, Refund.ts, Address.ts, BaseAddress.tsRan render() via /tmp render-check.ts with{{#items}}{{name}}{{^-last}}, {{/-last}}{{/items}}and{{^items}}casesRan verify() and render() with verify option via /tmp verify-check.tsBackgroundtimeout 1500 bun testin server/typescript (log /tmp/mo-ts-full-test.log): killed at 1500s, EXIT=124, no output🔧 Fix applied.
1 warning still open:
server/typescript/packages/codegen-ts/src/templates/value-object-file.ts:92- Abstract object.value gets a field-rules constant, contradicting the CHANGELOG sentence that abstract objects are unchanged. Needs a ruling.scripts/ci-local.sh --only ts-fast --only ts-unit --strict-toolchainsmeta init, eject, gen, verify --codegen from worktree source in /tmp/mo-val/projBase comparison via git archive 86bbadb in /tmp/mo-val/base-projbun /tmp/mo-val/drive/render-drive.ts (live render and verify)bun test per package: render 382 pass, codegen-ts 2557 pass, cli 1470 pass, metadata 3061 pass✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.