Repository navigation
fix: close 1.1.0-rc.7 re-check decimal and jsonb defects across all five ports - #446
Merged
Merged
Conversation
parse_json_object reads every non-integer number as a Decimal; only declared decimal fields should keep it. A field.double or a number inside a jsonb value object or array stayed a Decimal and json.dumps failed, answering 500 on create, update and PATCH.
- Python: decimal array element-wise writes, jsonb value-object decimals and floats, exact normalization beyond 28 digits - Kotlin: unbounded exact Exposed column for undeclared-precision decimals, decimal arrays, value-object decimal range check, Exposed 1.x array bind - TypeScript: refuse JSON-number decimals not exactly representable; decimals inside jsonb value objects follow the top-level rule; validator reports 400 - C#: decimal array nullable, value-object decimal converter, validation envelope, multi-package PATCH fix - Java: OMDB decimal arrays on Postgres; value-object nulls omitted - Corpus: decimal array / unbounded decimal / jsonb value-object numbers scenarios wired into every port - Fractional integral attr bound error names the edit - Docs, CHANGELOG, agent-context fixtures, RELEASING updates
…n, decimal wire cleanups)
…p recursion; fix tests
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 jsonb value object now follows the top-level decimal rule: canonical string out, string or number in, exact read, and 400 on excess scale. Decimal arrays are coerced element-wise and keep nulls. C# and Java omit null value-object members on the wire, and the multi-package PATCH is fixed.field.decimalstays a float, so a double inside a jsonb value object no longer returns 500.decimalWritehelper andObjectManager. The templates and the example copies are updated to match.decimal-array-exact,decimal-unbounded-exact,decimal-unbounded-exact-15,jsonb-value-object-numbers). The reference and generated servers in each port are updated to pass them.@min/@maxstays an error because the attrs are integralattr.int/attr.long. TheERR_BAD_ATTR_VALUEmessage now names the edit ("use an integer bound"). The CHANGELOG, 1.0-to-1.1 migration guide, api-contract, RELEASING and CONFORMANCE docs are corrected, and the agent-context skill, templates and conformance fixtures are regenerated.No version bump, tag or publish. Decimals inside
field.mapvalue objects and new metamodel vocabulary are out of scope.Validation
Risk Assessment
Testing
Drove decimal and jsonb api-contract lanes live on TypeScript, Python and C# against disposable Postgres/MySQL/SQLite, all passing, plus a new live C# regression test for malformed JSON bodies returning 400 (passes). Not driven: Java and Kotlin live lanes, the full five-port regression suite, and a pre-fix reproduction of the new C# test, so the verdict is inconclusive rather than go.
Pipeline
Updates from git push no-mistakes
✅ **Intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
server/csharp/MetaObjects.Codegen/Generators/EntityGenerator.cs:1502- The new condition(IsDecimalWireField(field) || !withAttributes) && !IsRequiredmakes EVERY non-required scalar array nullable with no initializer wheneverwithAttributesis false. That covers value objects (line 838), and also abstract shape classes (line 766). The comment above it says only a non-required decimal array becomes nullable. Example: an abstractBaseTagged { tags: field.string isArray }extended by a concretePost. Before, the base emittedIList<string> Tags = new List<string>(). Now it emitsIList<string>? Tags, andPostinherits that through the C# base chain (lines 282-292). Adopter codenew Post().Tags.Add(x)now throws NullReferenceException, and a POST that omitstagsstores NULL instead of[]. None of this is in the intent, which covers decimal arrays and value-object null omission. Fix: limit the!withAttributesarm to value objects, for example with a flag the VO caller at line 838 passes, so abstract shapes keep= new List<T>(). Also update the stale comment at line 1463, "Arrays are always non-nullable".server/python/src/metaobjects/codegen/generators/router_generator.py:232-_nested_decimal_specrecurses into afield.mapmember as if its value were one value-object instance. A map's value is{arbitraryKey: VO}, sodecimal_wire._nestedcompares the map's KEYS against the VO's member names. Example: VOPricing { tiers: field.map @objectRef Tier }, withTier { amount: field.decimal }, and body{"pricing": {"tiers": {"amount": {"amount": "1.5"}}}}. The map keyamountis treated as the decimal member, so_canonical(dict)raises and a valid body gets 400. For any other key the members fall through to_floats. The intent leaves out "decimals inside field.map value objects", so this recursion exceeds scope and is also wrong in shape. Remedy: removefc.FIELD_SUBTYPE_MAPfrom the tuple. A map value then falls through to_floats, which is the deliberately-unsupported behavior.server/typescript/packages/codegen-ts/test/decimal-write-schema.test.ts:57- The double15 loop decides whether each vector should be accepted or refused withNumber.isSafeInteger(n) || Number(n.toPrecision(15)) === n. That is the same predicate the generateddecimalWritehelper anddecimalFromJsonNumberuse. If the threshold were wrong (for example 17 digits instead of 15, or a dropped safe-integer arm), the expectation would move with it and the test would still pass. Use an independent oracle: mark the expected input verdict per vector infixtures/decimal-wire/vectors.json, or list concrete accepted and refused literals, as the D3 test below already does.server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinDecimalWireTest.kt:265- The new testa decimal with no precision is an unbounded column and the controller range-checks nothingproves its claim only with substring checks on generated source. Examples:assertFalse("MetaDecimalWire.fits(" in controller)and"portableDecimal(\"price\").nullable()" in table. A dead or renamed check passes the same way, so substring absence does not prove the controller accepts an out-of-range-looking value. The behavior is already covered by executing code: the next test runs the column type, and the Kotlin api-contract lane runsdecimal-unbounded-exact. Either drop these assertions or replace them with a compiled-controller call that accepts a 20-integer-digit value.🔧 Fix applied.
1 warning still open:
server/csharp/MetaObjects.Codegen/Generators/RoutesGenerator.cs:773- The newparseBody: HasDecimalField(entity)path routes every decimal-bearing entity with no @required keys through the hand-parsed POST handler. That handler callsJsonDocument.ParseAsync(http.Request.Body)at line 773 outside the try/catch that only wrapsDeserialize. Before this change those entities used the framework-bound({cls} input, ...)form, where a malformed or empty JSON body gave a bare 400. NowPOST /entrieswith body{not jsonor an empty body throws an unhandled JsonException, which is a 500. PATCH/PUT at line 318 and TPH at 1196 have the same unguarded parse. Remedy: wrap the ParseAsync call incatch (JsonException) { return Results.BadRequest(new { error = "validation" }); }at 773, and the same at 318 and 1196.🔧 Fix applied.
2 infos still open:
agent-context/skills/metaobjects-codegen/SKILL.md:66- Docs still tell agents to list owned generators withmetaobjects eject --list/dotnet meta eject, which this change's own upgrade guide now says do not exist. The guide (step 6) was corrected tometaobjects gen --list(Python has noeject --list; the eject parser at server/python/src/metaobjects/cli.py:2977 takes only names, --force and --root) anddotnet meta gen --list. The agent-context skill keeps the old wording: "meta eject --list(dotnet meta eject,metaobjects eject,mvn metaobjects:eject) reports each copy that differs". An agent following it in a Python project gets an argparse error. The same line is repeated in the five fixture copies of SKILL.md under fixtures/agent-context-conformance/*/expected/.claude/skills/metaobjects-codegen/SKILL.md, which must be regenerated together with the template.server/python/tests/integration/test_generated_decimal_wire.py:284- Weak assertion in the new Python test:assert body["fix"] == {"latitude": 1.5, "amount": "100.5", "readings": None} or body["fix"]["amount"] == "100.5". Theorarm is implied by the first arm, so the assertion reduces to "amount == '100.5'" and the other members (latitude kept as a JSON number, no spurious members) are never checked. A regression that turned latitude into a string or dropped it would still pass. Assert the exact expected object with one oracle instead.scripts/ci-local.sh --only ts-fast --only ts-unit --strict-toolchainsscripts/ci-local.sh --only ts-fast --only ts-unit --strict-toolchains(baseline, already green)bun test test/api-contract-decimal.test.ts test/api-contract-jsonb.test.tsin server/typescript/packages/integration-tests — 43 pass (live Fastify + Postgres via docker)uv run --extra dev pytest tests/integration/test_api_contract_decimal.py tests/integration/test_generated_decimal_wire.py tests/integration/test_api_contract_jsonb.pyin server/python — 60 passed, 11 skipped (live generated FastAPI app, postgres/sqlite/mysql)dotnet test MetaObjects.IntegrationTests --filter Decimal|Jsonb|MapValueObjectin server/csharp — 40 passed (live generated routes, testcontainers PG/MySQL)AddedMalformed_and_empty_body_are_400_not_500_on_sqlitein server/csharp/MetaObjects.IntegrationTests/Api/DecimalWireGeneratedApiTests.cs — POST{not jsonand empty body return 400 validation; passes (live HTTP against generated routes, sqlite)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
Validation: this change took 1 no-mistakes validation run (CI step skipped; full scripts/ci-local.sh green on the validated head). The test step was approved after the Kotlin, Java and five-port suites were run locally. Known gap: decimals inside field.map value objects are not covered.