Repository navigation
fix: apply decimal rule to value objects held by field.map in all five ports - #447
Merged
Merged
Conversation
…l rule in all five ports - Shared jsonb scenario jsonb-value-object-map-decimals (top-level and nested map, key named amount) - TypeScript runtime mapDecimals/validator, Python nested spec map form, Kotlin controller and OMDB map-of-VO read, C# map-holding value object stored as one jsonb property - Eject wording in the codegen skill corrected per port; exact Python assertion
…features/field-types.md` now says OMDB persists a `field.map @objectRef` as a jsonb column of value objects, and that the api-contract `jsonb-value-object-map-decimals` scenario gates it. `server/java/codegen-spring/.../KNOWN_GAPS.md` no longer says OMDB ignores maps. Earlier doc edits (CHANGELOG, SKILL.md eject wording and its 5 fixture copies, CONFORMANCE and api-contract counts, Kotlin KNOWN_GAPS) were already in the target commit and checked against source. Python has no `eject --list`, which the SKILL wording reflects. No code or tests changed.
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
field.decimalinside a value object held by afield.mapnow follows the top-level decimal rule in all five ports. The rule applies to each map value, and map keys are never matched against member names. Covers a top-level map and a map nested in a value object. Scalar-valued maps are unchanged. Gated by the sharedjsonb-value-object-map-decimalsapi-contract scenario.field.mapof value objects. The generated C#DbContextmaps a value object that holds a map as one converted jsonb property, because EF Core 8 cannot own a dictionary inside JSON. The Java OMDB driver deserializes that column into aMap<String, VO>, and the TS and Python runtimes map decimals per value.eject --listwording now matches what each port supports, and the five agent-context fixtures are regenerated. The CHANGELOG states the map rule.Validation
Risk Assessment
✅ Low: Per-value map-decimal handling matches the intent across all five ports and the shared scenario; tests execute real behavior, the eject --list wording matches each port's CLI, and no reachable defect was found. Tests were not run in this phase.
Testing
Live: map-of-value-object decimal scenarios driven in TS (reference and generated lanes, Postgres) and Python (generated and reference apps); both pass, including key-named-amount case and 400 on excess scale. Java, Kotlin and C# not driven live; only codegen and compile-run tests ran, and those pass. Full TS suite produced no result. Verdict inconclusive: intent covers all five ports, four not driven live.
Pipeline
Updates from git push no-mistakes
✅ **Intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
server/python/tests/integration/test_generated_decimal_wire.py:217- Four new test models put"@storage": "jsonb"on afield.map.@storageis registered only onfield.object(expected-registry.json, types[25] attrs). The tests load leniently, so the attribute is silently accepted. A strict load rejects it:MetaDataLoader.from_string(..., strict=True)returnsERR_UNKNOWN_ATTR: Unknown attribute '@storage' on field.map 'tiers', checked live. CLAUDE.md / ADR-0023 says the library's tests never permit an unregistered attr. The attribute also does nothing, because a map is always one jsonb column. Remove it at every site: server/python/tests/integration/test_generated_decimal_wire.py:217, server/java/codegen-spring/src/test/java/com/metaobjects/generator/spring/SpringDecimalWireCompileRunTest.java:62, server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinDecimalWireTest.kt:344, server/java/omdb/src/test/resources/meta.jsonbmap.json:12. The shared conformance meta.json correctly omits it.server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinDecimalWireTest.kt:389- The new map test ends with four substring checks on the generated controller source, for example"it[tiers] = dto.tiers" in ctland"vTiers.values.any { validator.validate(it).isNotEmpty() }" in ctl. Its comment claims these prove that the controller writes, reads and PATCHes the map, validating each value. Matching text cannot prove that, and the review-round-1 decisionkotlin-substring-testalready ruled out this pattern in the same file. The same pattern is in the new C# testA_value_object_holding_a_map_is_converted_whole_not_owned(server/csharp/MetaObjects.Codegen.Tests/MapFieldCodegenTests.cs:350-353,Assert.Contains("Property(x => x.Pricing)", db)), and in the changed asserts at server/csharp/MetaObjects.Codegen.Tests/DbContextCompileTests.cs:350,357. The behavior is already executed elsewhere: the Kotlin generated api-contract lane runs the new scenario, and C# JsonbMapDecimalPatchGeneratedTest boots the generated DbContext and routes against Postgres. Drop the substring asserts, or replace the Kotlin ones with a compiled-controller PATCH that rejects an excess-scale map value. A rename of the generated variable would break the current asserts with no behavior change, and a broken validation call with the text intact would still pass them.fixtures/api-contract-conformance/README.md:56- Fixed stale counts left by the new scenario. The sub-corpora total now reads "75 scenarios in all" (was 74; 75 + 31 core = 106). Thejsonb/table row now reads 4 scenarios (was 3).CHANGELOG.md:1535- Added a sentence to the 1.1.0 map-rule entry. It names the two generated-code changes an upgrading adopter will see in a diff: the Kotlin controller now creates, reads and patches afield.mapof value objects (it skipped that column before), and the C# DbContext maps a value object that holds a map as one converted jsonb property instead of OwnsOne.server/csharp/MetaObjects.Codegen/Generators/DbContextGenerator.cs:1083- The newHoldsMaphelper had been inserted between OwnedTypeConfig's doc comment and OwnedTypeConfig, so that comment described the wrong method. MovedHoldsMapabove the comment. Also corrected its one-line summary, which said "(non-flattened)" although the code does no flattened check. No code change.server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinSpringControllerGenerator.kt:1945- The newisVoMapColumn/voMapValueTypeFqnfunctions had been inserted betweenvoElementTypeFqn's KDoc and its declaration, which orphaned that KDoc. Moved the KDoc back ontovoElementTypeFqn. No code change.🔧 Fix applied.
4 infos still open:
fixtures/api-contract-conformance/README.md:56- Fixed stale counts left by the new scenario. The sub-corpora total now reads "75 scenarios in all" (was 74; 75 + 31 core = 106). Thejsonb/table row now reads 4 scenarios (was 3).CHANGELOG.md:1535- Added a sentence to the 1.1.0 map-rule entry. It names the two generated-code changes an upgrading adopter will see in a diff: the Kotlin controller now creates, reads and patches afield.mapof value objects (it skipped that column before), and the C# DbContext maps a value object that holds a map as one converted jsonb property instead of OwnsOne.server/csharp/MetaObjects.Codegen/Generators/DbContextGenerator.cs:1083- The newHoldsMaphelper had been inserted between OwnedTypeConfig's doc comment and OwnedTypeConfig, so that comment described the wrong method. MovedHoldsMapabove the comment. Also corrected its one-line summary, which said "(non-flattened)" although the code does no flattened check. No code change.server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinSpringControllerGenerator.kt:1945- The newisVoMapColumn/voMapValueTypeFqnfunctions had been inserted betweenvoElementTypeFqn's KDoc and its declaration, which orphaned that KDoc. Moved the KDoc back ontovoElementTypeFqn. No code change.scripts/ci-local.sh --only ts-fast --only ts-unit --strict-toolchainscd server/typescript/packages/integration-tests && bun test test/api-contract-jsonb.test.ts (8 pass: 4 scenarios x reference and generated lanes, Postgres via Testcontainers, includes jsonb-value-object-map-decimals)cd server/python && uv run --frozen --extra integration --extra dev pytest -q tests/integration/test_api_contract_jsonb.py tests/integration/test_generated_decimal_wire.py (35 pass; loads every jsonb/scenarios/*.yaml)cd server/java && mvn -B -o -pl codegen-spring,codegen-kotlin -am test -Dtest='*DecimalWire*,*Jsonb*,*Map*' (70 Kotlin and Spring codegen tests pass, incl. SpringDecimalWireCompileRunTest and KotlinDecimalWireTest)cd server/csharp && dotnet test MetaObjects.Codegen.Tests --filter 'Map|Decimal|Jsonb|DbContextCompile' (154 pass)cd server/typescript && bun test (full TS server suite; started in background, stopped at background time limit before any result; log /tmp/ts-full.log has only the header)Baseline already run by configured command: scripts/ci-local.sh --only ts-fast --only ts-unit --strict-toolchains✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
Validation: this branch took 2 no-mistakes validation runs in total (run 1 merged as the previous PR; run 2 produced this PR). CI step skipped; full scripts/ci-local.sh was green on the validated head.