Skip to content

fix: apply decimal rule to value objects held by field.map in all five ports - #447

Merged
dmealing merged 4 commits into
mainfrom
fm/mo-1-1-0-rc7-cut
Oct 22, 2026
Merged

dmealing merged 4 commits into
mainfrom
fm/mo-1-1-0-rc7-cut

Conversation

@dmealing

@dmealing dmealing commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

What Changed

  • A field.decimal inside a value object held by a field.map now 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 shared jsonb-value-object-map-decimals api-contract scenario.
  • Generated Kotlin controllers now create, read and patch a field.map of value objects. The generated C# DbContext maps 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 a Map<String, VO>, and the TS and Python runtimes map decimals per value.
  • The codegen skill's eject --list wording 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.

  • Live validation: ⚠️ inconclusive - 4 of 9 scenarios driven live against the product
Scenario Result Live Evidence
TS top-level field.map of decimal value objects: string and JSON number in, canonical string out, 400 on excess scale (jsonb-value-object-map-decimals r1-r3) ✅ pass live bun test test/api-contract-jsonb.test.ts in server/typescript/packages/integration-tests (generated and reference lanes, Postgres container)
TS map nested inside a value object: each map value follows the decimal rule; 400 persists nothing (r4) ✅ pass live same TS run as above
Python top-level and nested map of decimal value objects matches the shared contract ✅ pass live uv run pytest tests/integration/test_api_contract_jsonb.py tests/integration/test_generated_decimal_wire.py (35 passed)
A map key named amount is treated as a key, not compared with the value object member name ✅ pass live TS and Python api-contract runs of jsonb-value-object-map-decimals r1 (key amount in tiers and pricing.tiers)
Java Spring generated code applies the decimal rule to map values (codegen plus compile-and-run) ⏸️ untested no Only Maven codegen unit and compile-run tests were run (SpringDecimalWireCompileRunTest, SpringMapFieldCodegenTest). The live HTTP lane (server/java/integration-tests) is outside the reactor and needs…
Kotlin generated controller applies the decimal rule to map values (codegen plus compile-and-run) ⏸️ untested no Only Maven codegen compile-run tests were run (KotlinDecimalWireTest). The live Kotlin api-contract lane (integration-tests-kotlin) was not driven; it needs its own Testcontainers setup.
C# generated DbContext and routes persist and read a map of decimal value objects ⏸️ untested no Only dotnet codegen tests were run (154 pass). The Postgres-backed generated PATCH test in MetaObjects.IntegrationTests was not driven live.
A scalar-valued field.map is unchanged by the fix (adversarial boundary) ⏸️ untested no No shared scenario checks a scalar map live, and none was built in this run.
Full regression: repo conventional run-everything command across all five ports ⏸️ untested no Full TS suite stopped at background time limit before printing a result. Java reactor, Kotlin, full Python and full C# suites were not run. Rerun with a longer background timeout to get an outcome.
  • Outcome: ⚠️ 1 warning across 1 run (30m14s)

Pipeline

Updates from git push no-mistakes

✅ **Intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 4 infos
  • ⚠️ server/python/tests/integration/test_generated_decimal_wire.py:217 - Four new test models put &#34;@storage&#34;: &#34;jsonb&#34; on a field.map. @storage is registered only on field.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) returns ERR_UNKNOWN_ATTR: Unknown attribute &#39;@storage&#39; on field.map &#39;tiers&#39;, 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 &#34;it[tiers] = dto.tiers&#34; in ctl and &#34;vTiers.values.any { validator.validate(it).isNotEmpty() }&#34; 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 decision kotlin-substring-test already ruled out this pattern in the same file. The same pattern is in the new C# test A_value_object_holding_a_map_is_converted_whole_not_owned (server/csharp/MetaObjects.Codegen.Tests/MapFieldCodegenTests.cs:350-353, Assert.Contains(&#34;Property(x =&gt; x.Pricing)&#34;, 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). The jsonb/ 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 a field.map of 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 new HoldsMap helper had been inserted between OwnedTypeConfig's doc comment and OwnedTypeConfig, so that comment described the wrong method. Moved HoldsMap above 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 new isVoMapColumn / voMapValueTypeFqn functions had been inserted between voElementTypeFqn's KDoc and its declaration, which orphaned that KDoc. Moved the KDoc back onto voElementTypeFqn. 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). The jsonb/ 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 a field.map of 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 new HoldsMap helper had been inserted between OwnedTypeConfig's doc comment and OwnedTypeConfig, so that comment described the wrong method. Moved HoldsMap above 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 new isVoMapColumn / voMapValueTypeFqn functions had been inserted between voElementTypeFqn's KDoc and its declaration, which orphaned that KDoc. Moved the KDoc back onto voElementTypeFqn. No code change.
⚠️ **Test** - 1 warning
  • ⚠️ live validation verdict: inconclusive (4 of 9 scenarios were driven live against the product); untested: Java Spring generated code applies the decimal rule to map values (codegen plus compile-and-run), Kotlin generated controller applies the decimal rule to map values (codegen plus compile-and-run), C# generated DbContext and routes persist and read a map of decimal value objects, A scalar-valued field.map is unchanged by the fix (adversarial boundary), Full regression: repo conventional run-everything command across all five ports
  • Live validation: ⚠️ inconclusive - 4 of 9 scenarios driven live against the product
Scenario Result Live Evidence
TS top-level field.map of decimal value objects: string and JSON number in, canonical string out, 400 on excess scale (jsonb-value-object-map-decimals r1-r3) ✅ pass live bun test test/api-contract-jsonb.test.ts in server/typescript/packages/integration-tests (generated and reference lanes, Postgres container)
TS map nested inside a value object: each map value follows the decimal rule; 400 persists nothing (r4) ✅ pass live same TS run as above
Python top-level and nested map of decimal value objects matches the shared contract ✅ pass live uv run pytest tests/integration/test_api_contract_jsonb.py tests/integration/test_generated_decimal_wire.py (35 passed)
A map key named amount is treated as a key, not compared with the value object member name ✅ pass live TS and Python api-contract runs of jsonb-value-object-map-decimals r1 (key amount in tiers and pricing.tiers)
Java Spring generated code applies the decimal rule to map values (codegen plus compile-and-run) ⏸️ untested no Only Maven codegen unit and compile-run tests were run (SpringDecimalWireCompileRunTest, SpringMapFieldCodegenTest). The live HTTP lane (server/java/integration-tests) is outside the reactor and needs…
Kotlin generated controller applies the decimal rule to map values (codegen plus compile-and-run) ⏸️ untested no Only Maven codegen compile-run tests were run (KotlinDecimalWireTest). The live Kotlin api-contract lane (integration-tests-kotlin) was not driven; it needs its own Testcontainers setup.
C# generated DbContext and routes persist and read a map of decimal value objects ⏸️ untested no Only dotnet codegen tests were run (154 pass). The Postgres-backed generated PATCH test in MetaObjects.IntegrationTests was not driven live.
A scalar-valued field.map is unchanged by the fix (adversarial boundary) ⏸️ untested no No shared scenario checks a scalar map live, and none was built in this run.
Full regression: repo conventional run-everything command across all five ports ⏸️ untested no Full TS suite stopped at background time limit before printing a result. Java reactor, Kotlin, full Python and full C# suites were not run. Rerun with a longer background timeout to get an outcome.
  • scripts/ci-local.sh --only ts-fast --only ts-unit --strict-toolchains
  • cd 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.

…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.
@dmealing
dmealing merged commit c59838e into main Oct 22, 2026
1 check passed
@dmealing
dmealing deleted the fm/mo-1-1-0-rc7-cut branch October 22, 2026 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant