Skip to content

fix: cross-field comparison rule across ports and C# concrete-parent properties - #531

Merged
dmealing merged 10 commits into
mainfrom
fm/mo-found-csharp-extends-flakes
Nov 4, 2026
Merged

dmealing merged 10 commits into
mainfrom
fm/mo-found-csharp-extends-flakes

Conversation

@dmealing

@dmealing dmealing commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

What Changed

  • C# codegen: a child entity of a concrete parent now declares the parent's properties itself and names no C# base class, so each mapped entity keeps all its columns. A child of an abstract parent (with EmitAbstractShapes) emits class Child : Base and declares only its own members. Removed the dead special-case guard in the cross-field Validate override. Added EntityExtendsParentTests.
  • Cross-field validator.comparison now follows one rule in all five ports. Numerics compare with each other. Strings and temporals compare only with the same subtype (temporals also need matching @localTime). Boolean, enum, uuid, uri and inet fields test only eq/ne. Any other pair is skipped, never rejected. Java/Kotlin no longer cast to Comparable (create returned 500 instead of 400), TypeScript no longer compares as strings, and Python no longer raises TypeError. Ordering on boolean and enum is no longer enforced by generated validators. Added shared conformance cases in fixtures/validation-conformance/, documented in its README, and re-synced the owned zod-validators template copies.
  • Test stability and coverage: the C# DependencyCliTests now joins the serial OwnedRunnerCollection, fixing a working-directory race. The codegen-ts compile tests share a 30s timeout from test/compile-timeout.ts. The Java Patch cross-field test executes the generated Dto and Patch classes instead of comparing source text. A TS React regression test covers a TPH subtype form binding the refined body schema. Adds a test-scope jackson-datatype-jsr310 to codegen-spring for the conformance runner.
  • CHANGELOG entries under Unreleased. No version bump.

Flaky tests fixed (at the cause, no retries)

  • C# Cli GenCommandTests.Gen_writes_the_hash_manifest_beside_the_metadata_not_in_cwd: DependencyCliTests runs gen in process, which changes the process-wide working directory, without joining the serial OwnedRunnerCollection; the two classes ran in parallel and one read the other's directory as the cwd (about one run in five). Now serial; 12 of 12 looped runs passed after the fix.
  • codegen-ts tests that type-check generated output with the real TypeScript compiler (29 files, including tph-cross-field.test.ts): bun's 5s default timeout against a slowest measured 3.3s case under load. Each file now calls setDefaultTimeout(COMPILE_TEST_TIMEOUT_MS) (30s, defined once in test/compile-timeout.ts; a side-effect import would only reach the first file per process). The original timeouts were not reproduced on demand, so this one is justified by measurement.

Gate runs

Two no-mistakes runs for this PR. Run 1 failed at the test step before any push because GitHub SSH auth was down during a GitHub incident (infrastructure, not code); its review fixes were carried into run 2, which produced this PR. The test step of run 2 was approved past an "inconclusive" live-evidence verdict after the maintainer-requested serial local suites all passed (C# Codegen 1128, Cli 364, Conformance 2222; Java/Kotlin cross-field and conformance; Python 1304; TS cross-field conformance 25, codegen-ts 71, codegen-ts-react 62).

Validation

Risk Assessment

✅ Low: Codegen and test-only changes, bounded and consistent across all five ports; the prior-round fixes (@localTime comparability, C# eq/ne CLR-type guard, per-file timeouts) check out against the code, and I found no regression I could substantiate.

Testing

Live-driven: per-file compile timeout probe on the real bun runner (pass). Suite-only, not live: TS validation conformance (85 pass), C# codegen (1128 pass, incl. EqAcrossClrTypes), C# conformance (2222 pass), C# CLI (364 pass). Not driven: Java, Kotlin and Python conformance and unit suites, java-reactor build, integration-tests lanes, full ci-local.sh run. Verdict inconclusive: live-exercisable surface exists but only one scenario was driven live.

  • Live validation: ⚠️ inconclusive - 1 of 6 scenarios driven live against the product
Scenario Result Live Evidence
TS validator skips cross-kind comparison (date vs timestamp, instant vs wall-clock) and enforces uuid ne ⏸️ untested no Prior payload recorded only a generated-validator test suite run, not a live product run. Live drive not established.
Per-file 30s compile timeout applies in plain bun test regardless of file order ✅ pass live Temporary probe files run with bun test in codegen-ts, both orders; probes removed
C# eq/ne across differing CLR types (instant vs wall-clock) is skipped, not rejected ⏸️ untested no Prior payload recorded a generator test suite run, which compiles and inspects output; no live product run was established.
C# child of concrete parent declares inherited properties with no base clause (inheritance fix) ⏸️ untested no Covered only by the codegen suite pass; no live run of the inheritance case was established.
Java/Kotlin/Python cross-kind comparison skipped instead of ClassCastException/TypeError ⏸️ untested no Java-reactor, Kotlin and Python conformance lanes were not executed. Would need mvn and uv test runs of the validation corpus in those ports.
C# CLI GenCommandTests hash-manifest flake fixed by serial collection ⏸️ untested no Prior payload recorded one suite run, not a live reproduction or repeated runs to measure flake rate.
  • Outcome: ⚠️ 2 warnings across 1 run (36m53s)

Pipeline

Updates from git push no-mistakes

✅ **Intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed (2) ✅
  • ⚠️ server/typescript/packages/codegen-ts/test/tph-cross-field.test.ts:164 - The intent says every codegen-ts test that type-checks generated output with the real TypeScript compiler runs under the shared 30s timeout. tph-cross-field.test.ts does this, but it is not one of the 28 files. It calls ts.createProgram at line 123, and its test "the generated entity, queries and routes compile" (line 164) runs that compile. The file never calls setDefaultTimeout(COMPILE_TEST_TIMEOUT_MS). Bun scopes that call to the file that makes it, so this test keeps bun's 5s default. On a loaded machine it can still report the timeout flake this change says it fixes. CI passes --timeout 30000, so CI does not hit it. A plain bun test in the package can. Fix: import setDefaultTimeout and COMPILE_TEST_TIMEOUT_MS from ./compile-timeout.js, and call it at the file's top level, as the other 28 files do. The file came from feat: enforce cross-field validators in every port's generated create models #530 and is not in this diff. The gap is in the coverage this change claims. Every other createProgram user under test/ already makes the call.

🔧 Fix applied.
1 warning still open:

  • ⚠️ server/csharp/MetaObjects.Codegen/Generators/EntityGenerator.cs:1159 - The eq/ne fallback in the C# comparison emitter requires only the same field subtype, not the same CLR type. loggedAt eq wallClock (field.timestamp vs field.timestamp @localTime) misses the temporal branch because ScalarForField differs (DateTimeOffset vs DateTime). It then reaches !ordering && l.SubType == r.SubType && lk != Other and emits Equals(__l, __r). That compares a boxed DateTimeOffset with a boxed DateTime, which is always false, so eq rejects every body where both fields are set. ne never fires. Java, Kotlin, TypeScript and Python skip this pair, and the intent says an unsound pair is 'skipped, never rejected'. The new conformance case only exercises lte for this pair, so it does not catch it. The same hole exists for two enum fields with different generated enum types (EnumPropertyTypeName differs, so Equals is always false) and for a @lenient uri/inet (string) against a strict uri/inet (Uri/IPAddress). The latest fix round (a25bd2b) added the @localTime check to the temporal ordering branch only and left this equality fallback behind. Fix: in the fallback also require CSharpNaming.ScalarForField(l) == CSharpNaming.ScalarForField(r), and for enums require equal EnumPropertyTypeName. Add an eq conformance variant for the instant vs wall-clock pair.

🔧 Fix applied.
✅ Re-checked - no issues remain.

⚠️ **Test** - 2 warnings
  • ⚠️ server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringGeneratorSupport.java:679 - Java/Kotlin/Python ports' soundness fixes lack live evidence in this run.
  • ⚠️ live validation verdict: inconclusive (1 of 6 scenarios were driven live against the product); untested: TS validator skips cross-kind comparison (date vs timestamp, instant vs wall-clock) and enforces uuid ne, C# eq/ne across differing CLR types (instant vs wall-clock) is skipped, not rejected, C# child of concrete parent declares inherited properties with no base clause (inheritance fix), Java/Kotlin/Python cross-kind comparison skipped instead of ClassCastException/TypeError, C# CLI GenCommandTests hash-manifest flake fixed by serial collection
  • Live validation: ⚠️ inconclusive - 1 of 6 scenarios driven live against the product
Scenario Result Live Evidence
TS validator skips cross-kind comparison (date vs timestamp, instant vs wall-clock) and enforces uuid ne ⏸️ untested no Prior payload recorded only a generated-validator test suite run, not a live product run. Live drive not established.
Per-file 30s compile timeout applies in plain bun test regardless of file order ✅ pass live Temporary probe files run with bun test in codegen-ts, both orders; probes removed
C# eq/ne across differing CLR types (instant vs wall-clock) is skipped, not rejected ⏸️ untested no Prior payload recorded a generator test suite run, which compiles and inspects output; no live product run was established.
C# child of concrete parent declares inherited properties with no base clause (inheritance fix) ⏸️ untested no Covered only by the codegen suite pass; no live run of the inheritance case was established.
Java/Kotlin/Python cross-kind comparison skipped instead of ClassCastException/TypeError ⏸️ untested no Java-reactor, Kotlin and Python conformance lanes were not executed. Would need mvn and uv test runs of the validation corpus in those ports.
C# CLI GenCommandTests hash-manifest flake fixed by serial collection ⏸️ untested no Prior payload recorded one suite run, not a live reproduction or repeated runs to measure flake rate.
  • scripts/ci-local.sh --only ts-fast --only ts-unit --strict-toolchains
  • cd server/typescript/packages/integration-tests && bun test test/validation-conformance.test.ts test/validation-conformance-runtime.test.ts (85 pass, 0 fail; suite run, not live)
  • Probe in server/typescript/packages/codegen-ts: two temporary test files, one calling setDefaultTimeout(COMPILE_TEST_TIMEOUT_MS) and one not, 6s test each, run via plain bun test in both file orders. Calling file passes at 30s; non-calling file fails at 5000ms as designed. Probes deleted afterward.
  • dotnet test server/csharp/MetaObjects.Codegen.Tests (1128 pass, 0 fail, 1 skipped; suite run, not live)
  • dotnet test server/csharp/MetaObjects.Conformance.Tests (2222 pass, 0 fail; suite run, not live)
  • dotnet test server/csharp/MetaObjects.Cli.Tests (364 pass, 0 fail; suite run, single run, not live)
  • Inspected codegen-ts test/ for setDefaultTimeout coverage: 30 files call setDefaultTimeout; no createProgram user without it
  • Baseline ts-fast/ts-unit from the no-mistakes run, not repeated
🔧 **Document** - 1 issue found → auto-fixed ✅
  • ⚠️ agent-context/skills/metaobjects-authoring/SKILL.md:437 - Authoring skill comparison row omits the kind rule (only numerics, own-subtype strings/temporals, eq/ne on boolean/enum/uuid/uri/inet; other pairs skipped). The file is mirrored into six fixtures/agent-context-conformance/*/expected copies, so editing it needs those regenerated. Also spec/metamodel/validator.json rules text is schema-backed and generated into fixtures/metamodel-docs; it is not wrong, so left alone.

🔧 Fix applied.
✅ Re-checked - no issues remain.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

…ies; fix test timing flakes

A C# entity that extends another skipped the inherited fields but its class
declaration never named the base, so the parent's properties appeared nowhere.
A concrete parent is flattened onto the child (each concrete entity owns a table
with all its columns; class inheritance between mapped entities would be an EF
TPH/TPT hierarchy). An emitted abstract shape is named as the base.

Flakes fixed at the cause: DependencyCliTests changed the process-wide cwd outside
the serial collection, racing GenCommandTests; codegen-ts compile tests run under
a 30s timeout set in one helper instead of bun's 5s default.
…executed Patch check; TPH form rule test

A validator.comparison between fields of different kinds (e.g. date vs timestamp) is now
skipped in Java, Kotlin, TypeScript and Python as C# already did, instead of a 500
(ClassCastException), a string compare, or a TypeError. Adds the shared
comparison-across-kinds-is-skipped conformance case, replaces the Java Patch source-text
comparison with an executed check, and adds a TPH subtype form test for a cross-field rule.
…ays enforced; review fixes

Code review of the cross-kind skip found that equality on identifier-like fields was dropped
along with ordering. Restores it in every port, adds shared uuid cases, corrects stale C#
comments, and documents the EmitClassDeclarationLine seam change.
…nd-based comparison rule; one follow-up left.
…Mapper

An ejected copy of KotlinCrossFieldRules compiles standalone, so it cannot
reference KotlinTypeMapper. Decide the native shape from @localTime and
@lenient directly.
@dmealing
dmealing merged commit b7a031a into main Nov 4, 2026
9 checks passed
@dmealing
dmealing deleted the fm/mo-found-csharp-extends-flakes branch November 4, 2026 13:24
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