Repository navigation
fix: canonicalize decimal wire and close 1.1.0 release review findings - #445
Merged
Merged
Conversation
Fixes everything the 1.1.0 release review found, plus the adopter-found `eject --list` defect, and closes the test gaps it named. - Decimals: one canonical string on the wire in every port (W1-W4), a shared decimal-wire corpus and an api-contract decimal/ sub-corpus, exact filter operands, and 400 validation past @precision/@scale. The stronger assertions found the TypeScript TPH polymorphic read sending a subtype-only decimal uncanonicalized; fixed in the generator. - Report lanes: every port runs the report scenarios on SQLite and MySQL (Java OMDB SQLiteDriver, Python SQLite/MySQL drivers), the Java report lane is database-backed, and the Cube lane queries every exporter shape. - Loader: four more report shapes refused (D5, M10, R10, R11), an integral float accepted where an int is declared, the requirement package and ERR_UNRESOLVED_SUPER hint fixed. - eject --list reports owned companion files; JVM and Python naming discovery; TS forms step/inputMode; config unknown keys warn. - Breaking (recorded in CHANGELOG and the 1.0-to-1.1 upgrade guide): C# scalar and enum arrays are IList<T>; a Java POJO redeclaring an inherited field with another subtype is standalone; Kotlin gains per-package MetaDecimalWire.kt and MetaPortableColumnTypes.kt.
…nt fractional validator bounds
… Python and Kotlin
…ew-2 CHANGELOG and upgrade-guide fixes were already present in the diff. Edited the reporting skill decimal line (source plus five fixture copies, which mirror it) to the canonical-string rule, and added the companion-file clause to the own-your-codegen eject --list bullet.
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
@scale, or more integer digits than@precision - @scale, is rejected with 400 and never rounded. Decimal filter operands compare exactly, and a malformed operand returns 400invalid_filter_value. Each port implements this idiomatically: TSdecimal-wireand generated routes, C#DecimalWire.g.cs, JavaCanonicalDecimal*andDeclaredDecimal, KotlinMetaDecimalWire.kt, and Pythondecimal_wire.SQLiteDriver, and Python gains SQLite and MySQL drivers. The Java in-memory report lane is replaced by database-backed OMDB lanes. TS routes give a TPH base the decimal columns that only a subtype declares.docs/features/migrations/upgrading-1.0-to-1.1.md. C# scalar and enum arrays becomeIList<T>. Kotlin gains per-package decimal and portable-type helper files.eject --listreports owned companion files. Thepublish-*.ymlworkflows gain the release-tag gate fromscripts/check-release-tag.mjs.Validation
Risk Assessment
✅ Low: The latest fix round makes Python coerce_decimal_fields and the Kotlin controller (create range check and PATCH bind) handle decimal arrays element by element, matching C#, Java and the TS runtime, and I found no defect in them.
Testing
Live-driven: Python list decimal coerce accepts and canonicalizes elements, and refuses an element past @scale (live=true); eject --list lists owned companion files (live=true). Not live-driven, so untested under the contract: Kotlin per-element W3 range check (harness only), TS decimal array canonicalization (direct function calls, not a running server), TS full regression (12017 pass in log, not driven live), Python full non-integration run (3 failures from missing 'metaobjects' install, not live-driven), fractional validator bound, and Python integration tests (pg8000 missing). Verdict inconclusive.
Evidence: Python decimal array live transcript
Evidence: eject --list owned copies live output
Evidence: TS full suite output (EXIT=0)
~/.no-mistakes/evidence/01M5FP64V387HRG1Y9N1EMNNDE/py-suite.txt)Pipeline
Updates from git push no-mistakes
✅ **Intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (2) ✅
server/typescript/packages/runtime-ts/src/type-coercer.ts:314- The TS ObjectManager write path now throws on anyfield.decimaldeclared@isArray(for examplenumeric[]on Postgres).bindDecimalsloops over every decimal child with noresolvedIsArray()skip. It passes the whole array toparseDecimalInput, which only accepts a string or a number and returnsundefinedfor an array, so the function throws "field.decimal '<name>' cannot be written ... not a decimal number". Decimal arrays are a supported shape (migrate-tsemits anumeric[]column for them), and this write used to pass through. Sibling inconsistencies of the same class: (1)canonicalizeDecimals(type-coercer.ts:292) also ignores arrays, so array elements are never canonicalized on read; (2)decimalFieldNames(codegen-ts/src/api-surface.ts:127) explicitly excludes arrays, so generated TS routes send decimal arrays as the driver returns them (Postgres"100.5000"). C# (DecimalListJsonConverter), Python (to_wireover lists) and Java (contentUsing) all emit canonical strings for array elements, so W1 is not cross-port for arrays. The smallest fix is to skip array fields inbindDecimalsand map the element-wise rule over arrays in both coercers and in the route mount.CHANGELOG.md:863- Rule A tightensattr.intandattr.longin TS, C# and Python, so a fractional value such as@min: 0.5is nowERR_BAD_ATTR_VALUE.validator.numericdeclares@minand@maxasattr.int, so a fractional bound on a decimal or double field no longer loads. Java already rejected it, so the ports now agree. This contradicts the CHANGELOG sentence at CHANGELOG.md:863, still in the 1.1.0 section: "No port changed its generalattr.intcheck, so a fraction in a validator's@minor@maxloads where it did."docs/features/migrations/upgrading-1.0-to-1.1.mddoes not mention it. Decide one of two things: (a) the rejection is intended, in which case fix the stale CHANGELOG sentence and add an upgrade-guide entry (fractionalvalidator.numericbounds now fail to load); or (b) makevalidator.numeric @min/@maxaccept fractions, since a bound like0.01is legitimate on a decimal field and is now inexpressible in every port. The choice is a product decision.🔧 Fix applied.
1 warning still open:
server/python/src/metaobjects/codegen/runtime/decimal_wire.py:127- The decimal-array fix is incomplete for Python. The fix round made the TS write path, the TS read path and the TS route mount element-wise, but the generated Python routers still reject anyfield.decimal @isArrayon write.router_generator._decimal_field_names(router_generator.py:184) lists every decimal field, arrays included, in_DECIMAL_FIELDS.coerce_decimal_fieldsthen callsparse_decimal(list), which raisesValueError("list is not a decimal"), so the route answers400 {"error":"validation"}. I generated a router for an entity withfield.decimal prices isArray @precision 12 @scale 4: it emitsdto = coerce_decimal_fields(dto, _DECIMAL_FIELDS, _DECIMAL_LIMITS)and_DECIMAL_LIMITS = {"prices": (12, 4)}. Callingcoerce_decimal_fields({'amounts':['1.5','2']}, frozenset({'amounts'}), {'amounts':(10,2)})raisesValueError: amounts: list is not a decimal. Before this change that write passed. Only the read side (to_wireover lists) is element-wise. This is the same defect class as review-1, left behind in a sibling port. Sibling site: Kotlin skips the W3 range check for array elements (KotlinSpringControllerGenerator.kt:1239,appendDecimalRangeCheckreturns early forisArrayType), so an array element with more fraction digits than@scaleis not refused there, although the TS fix and Java'sbindDecimalListapply W3 per element. Fix: incoerce_decimal_fields, when the value is a list, parse each element (null stays None) and applycheck_decimal_rangeto each. Add a Python behavioural test, and a Kotlin per-element range check, with canonical-string assertions.🔧 Fix applied.
✅ Re-checked - no issues remain.
server/python/tests/codegen/test_cli_surface.py- Python non-integration suite: 3 failed, 3748 passed. The 3 failures are tests/codegen/test_cli_surface.py::test_a_version_flag_prints_the_bare_version[--version|-v|-V] raising PackageNotFoundError for 'metaobjects'. The package is not installed in this environment (no editable install). Environment setup gap, not a code defect. Not re-run in an editable venv.scripts/ci-local.sh --only ts-fast --only ts-unit --strict-toolchainsbun test runtime-ts test/type-coercer.test.ts (12 pass)bun test codegen-ts test/decimal-wire-e2e.test.ts (8 pass)pytest server/python/tests/runtime/test_decimal_wire.py (79 pass)live: python3 -I script calling coerce_decimal_fields with list and out-of-scale element (evidence py-decimal-array.txt)mvn -o test -Dtest=KotlinDecimalWireTest in server/java/codegen-kotlin (13 pass)live: bun server/typescript/packages/cli/bin/meta.ts init, eject entity, eject --list --format text in temp project (evidence eject-list-owned.txt)bun test full server/typescript: 12017 pass, 50 skip, 0 fail, EXIT=0 (ts-full-suite.txt)pytest server/python --ignore=tests/integration: 3748 pass, 3 fail (PackageNotFoundError: metaobjects in test_cli_surface version tests), EXIT=1 (py-suite.txt)pytest server/python tests/integration blocked by missing pg8000✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
Validation
scripts/ci-local.sh(every port, the 5-port Testcontainers integration matrix and the Cube live check) was green on06adeb34, so the gate's CI step was skipped as local CI.field.decimal @isArray): TS writes threw and elements were not canonicalized on read or in generated routes; the Python router refused array writes and Kotlin skipped the per-element@precision/@scalecheck. It also corrected the stale CHANGELOG sentence on fractional validator bounds (which now fail to load in TS, C# and Python, as in Java) and added the upgrade-guide entry.532c123b: TS runtime-ts 819 and codegen-ts 2752 pass, including the decimal-array cases; Python 4115 pass including integration; Kotlin codegen 549, integration 361 and Exposed 1.x 4 pass, includingKotlinDecimalWireTest; a fractionalvalidator.numeric @mingivesERR_BAD_ATTR_VALUE.