Skip to content

Hearth based derivation - WIP - #5514

Draft
Kamil-Lontkowski wants to merge 14 commits into
masterfrom
hearth-based-derivation
Draft

Kamil-Lontkowski wants to merge 14 commits into
masterfrom
hearth-based-derivation

Conversation

@Kamil-Lontkowski

@Kamil-Lontkowski Kamil-Lontkowski commented Sep 9, 2026 •

Copy link
Copy Markdown

Rewrites tapir-json-pickler on Hearth + jsoniter-scala: Schema and JsonValueCodec are derived in one macro expansion from one PicklerConfiguration, with every name computed once and spliced into both halves, so the documentation and the wire format cannot drift. A single Shape classification drives both derivations; SchemaCodecAgreementTest checks the property for generated values of every fixture under every configuration.

Behaviour changes vs. the µPickle-based module: untagged Either; @default is documentation-only(jsoniter does not support default params through config); picklerForMap takes a key parser; wrapper givens removed; PicklerConfiguration must be compile-time evaluable; oneOfUsingField must be complete; no in SNames; recursive types supported; jsoniter-scala-macros becomes a transitive compile dependency. See “Migrating from the µPickle-based pickler” in the docs.

@adamw

adamw commented Sep 18, 2026

Copy link
Copy Markdown
Member

Review

The approach is right and the compile-time goal is met. There are four correctness bugs I'd fix before merging, three of which are schema/codec disagreements — the thing the module exists to prevent.

Tests pass: 189 succeeded, 0 failed, 168s (sbt picklerJson3/test).

Goals

Faster derivation — yes, measured. 27 types, derives Pickler on each, 3 runs:

master (µPickle) this PR
compile time 26.8 / 27.3 / 27.5 s 6.8 / 7.2 / 7.4 s
compiler max RSS ~2.55 GB ~0.56 GB
bytecode 38.1 MB, 967 classes 0.52 MB, 84 classes

Scaling is linear in fields, types and call sites. JsonCodecMaker.make runs once per unique type per expansion, never nested inside another make. Runtime speed is not lost: derived codecs benchmark the same as a plain JsonCodecMaker.make[Order] (~150k ops/s, within noise), and generated code for a normal ADT contains no CodecCombinators calls at all.

Reusing jsoniter — yes for the codec. CodecDerivation never writes reader/writer code; it walks the type graph and builds configs. Each hand-written codec matches a real jsoniter gap: Either does not exist in jsoniter 2.40.1, java.math.BigDecimal/BigInteger have no codec, and the rest are runtime facade wrappers that a compile-time macro cannot produce. One make per type is forced by the library, since withFieldNameMapper is keyed on the bare field name.

The schema half is where duplication sits: about 209 of 477 lines overlap tapir core. Roughly half of that is unavoidable (core's Configuration has three fields and cannot express enum case naming or oneOfUsingField per-leaf overrides). But SchemaUtils.enrichSchema is now the third copy of the annotation fold in this repo, and it has already drifted — it handles @hidden, core's magnolia copy does not.

Single configuration point — done for names, not for shape. Field names, type names, discriminator field and discriminator values really are computed once and spliced into both halves. Shape is where it leaks; see below.

Customisation coverage — the weak spot. Of jsoniter's 31 CodecMakerConfig options, 4 are exposed, 5 are hardcoded, 22 cannot be reached.

Bugs to fix before merge

1. Array[T] field with a validator throws ClassCastException when decoding. SchemaUtils.scala:131 uses _.asInstanceOf[Iterable[E]] for every collection. Core uses _.toIterable for arrays and identity only for Iterable (Schema.scala:85 vs :97), because a Scala Array is not an Iterable. Schema.applyValidation calls SArray.toIterable whenever hasRuntimeValidation is true, so case class Post(tags: Array[Tag]) with any validator under Tag crashes instead of returning a decode failure. The existing Array fixtures have no validators, and SArray equality ignores the function, so no test sees it.

2. A user given Pickler[Leaf] for a coproduct leaf produces JSON that cannot be read back. CodecDerivation.scala:230 and SchemaDerivation.scala:156 accept a user pickler for any nested type. For a leaf that is wrong: jsoniter delegates encoding to the leaf codec, but the parent's decoder and its SCoproduct mapping still come from the parent's DerivationEnv. Measured — a leaf pickler derived with withFullDiscriminatorValues writes {"$type":"...LeafA","v":1} while the parent documents Set("LeafA","LeafA2") and cannot decode its own output. rejectBareCodec guards this for a bare JsonValueCodec, and oneOfUsingField guards it through implicitLookupExclusions. Structural derivation does not. Fix: add the hierarchy's leaves to implicitLookupExclusions, or reject with a PicklerDerivationError.

3. A leaf derived on its own writes a $type its schema does not document. alwaysEmitDiscriminator(true) (CodecDerivation.scala:317) applies to every Shape.Product, but deriveCaseClassSchema (SchemaDerivation.scala:275) builds the plain SProduct — the discriminator is only added by the parent (SchemaUtils.scala:97). So jsonBody[Leaf1] documents {a} and writes {"$type":"Leaf1","a":7}. CodecDerivationTest.scala:194 pins the JSON but never the schema. It also disagrees with case-object leaves, which go through Shape.Singleton and write no tag.

4. Two leaves with the same simple name: one disappears from the schema, and validation throws. leavesOf (TypeShape.scala:147) is keyed by simple name, so Ns1.Same and Ns2.Same collapse into one. Pickler.schemaFor[Dup] compiles and documents only one subtype, and the generated subtypeIndex match has no default for the dropped leaf, so SchemaUtils.scala:111 throws MatchError inside Schema.applyValidation — on every decoded body. Pickler.derived[Dup] does fail, but with a raw jsoniter Can't evaluate compile-time expression message. Same class of problem one level down: duplicate discriminator values collapse silently at SchemaUtils.scala:103 (.toMap keeps the last), so one type becomes undecodable. deriveOneOfUsingField already checks for this.

Two more disagreements, lower impact:

  • Array[Byte] is classified BuiltInScalar (TypeShape.scala:98), so its schema is SBinary() (OpenAPI type: string, format: binary) while jsoniter writes [1,2,3] — jsoniter 2.40.1 has no base64 path at all. Either classify it as Collection(Byte), which gives an agreeing SArray(SInteger), or add a base64 leaf codec.
  • @default without a Scala default parameter documents the field as optional with a default (Schema.default sets isOptional = true) while the decoder rejects it as missing. So the docs tell a client the field can be omitted and the server then returns 400. The opposite case — a Scala default with no @default — documents the field as required while the decoder accepts it missing.

That @default split is also the clearest break of the single-config-point goal: to get both the documented default and the decode behaviour you have to write @default("x") and = "x", and Fixtures.scala:33 has the two disagreeing. It is a regression too — master's Readers.scala:14-15 / macros.scala:67-94 did fill defaults from the schema. The fix that collapses both into one place: read the Scala default parameter in deriveCaseClassSchema and emit Schema.default, which makes @default unnecessary.

Derivation timeout

The 5s macro timeout is a build failure that depends on the machine. DerivationTimeout.scala:59. Measured: a chain of 60 types compiles, 90 compiles, 110 fails with Macro 'Pickler.derived' timed out after 5000ms. It is wall clock, so the threshold moves with CPU load, JIT warmth and GC — a project that builds locally can fail on a busy CI runner with no code change. It is not hiding a performance problem, since scaling is linear. Runaway recursion is already caught by the inProgress/visited guards, so a 30–60s default costs nothing. The comment is also wrong: hearth's default is 2s, not 5s.

The documented way out of that does not work. DerivationTimeout.scala:11 documents bare seconds. Measured: -Xmacro-settings:tapirPickler.timeout=120 is ignored with no warning and still fails at 5000ms. Only 120s, 120000ms and 2m work. The bare form is the first thing anyone will try.

Customisation gaps

Schema annotations are fully covered — all 11 are honoured, and @hidden is handled where core's derivation does not. The gaps are on the jsoniter side and in PicklerConfiguration.

  • Every with* on PicklerConfiguration silently resets transientNone. All of them except withTransientNone rebuild with the one-arg constructor (PicklerConfiguration.scala:19-41), so default.withTransientNone(false).withSnakeCaseMemberNames.transientNone is true (measured). Order-dependent and silent. Pre-existing on master, but this PR rewrites the file. Fix is copy(genericDerivationConfig = ...).
  • Ten limit and strictness options are plain pass-throughs with no schema meaning and nothing blocking them: mapMaxInsertNumber, setMaxInsertNumber, bigDecimalDigitsLimit/ScaleLimit/Precision, bigIntDigitsLimit, bitSetValueLimit, checkFieldDuplication, requireDiscriminatorFirst, skipUnexpectedFields. The first two matter most: a Map or Set field with more than 1024 entries fails to decode today and there is no way to raise the limit. That shows up in production, not at compile time.
  • No discriminator = None. Core supports untagged coproducts; PicklerConfiguration narrows Option[String] to String. circeLikeObjectEncoding ({"Dog": {...}}) is also unreachable, though core has wrapWithSingleFieldProduct for it. Circe users on either encoding have no path over.
  • No withToDiscriminatorValue, although withToEncodedName exists. Only reachable through .copy.
  • Discriminator-value config does nothing for all-singleton hierarchies (PlatformSupport.scala:44-49). Correct by design, but a global withSnakeCaseDiscriminatorValues that visibly has no effect is confusing.
  • ReaderConfig is hardcoded and private (PicklerUtils.scala:17), WriterConfig is never used. tapir-jsoniter-scala at least exposes readerConfig as an overridable lazy val.
  • Enumeration schemaType and default were dropped. Master's CreateDerivedEnumerationPickler.apply(encode, schemaType, default) let you document an enum as an integer and set a default. Dropping schemaType makes sense on its own (an integer schema with a string codec is drift), but then the answer is a paired customIntBased that changes both halves, not removal. default is documentation only, no drift risk, and is a plain loss.

Also inconsistent: a non-literal @encodedName on a field is a compile error (AnnotationSupport.scala:41-58), but on a type it is silently ignored (:114-117).

One asymmetry to decide on

A bare given JsonValueCodec[Foo] for a structural type is a compile error, but a bare given Schema[Foo] is silently ignored — StructuralRule always wins, and useImplicitSchema is only reachable for scalars, Java numbers and opaque types (SchemaDerivation.scala:180). Someone coming from circe with given Schema[Foo] = Schema.derived[Foo].description(...) in scope loses it with no message. It should be the same error, pointing at Pickler.fromSchemaAndCodec.

For opaque and leaf types both givens are accepted independently and nothing checks them against each other: opaque type Cents = Int with given Schema[Cents] = Schema.string documents a string and writes a number.

Performance follow-ups

CodecCombinators.either (:110-121) decodes Left about 6x slower than Right. Some of that is inherent — raw bytes copy, re-parse, an exception per Left. But the fallback reads use the default ReaderConfig, which builds a hex dump into an exception message that is then thrown away. Passing ReaderConfig.withAppendHexDumpToParseException(false), which PicklerUtils.scala:17 already builds, measured about 2x faster on the Left path. One line.

Smaller ones: SchemaUtils.scala:111 uses Seq.lift, which allocates a Lifted plus a Some per validated coproduct value — that is per request body when validators exist. Log.namedScope is strict while Log.info is by-name, so SchemaDerivation.scala:110 and CodecDerivation.scala:84 pretty-print a type on every visit even with logging off (~0.6% of compile time). TypeShape.scala:98 identifies Array[Byte] by string-comparing plainPrint, which is both slow and fragile.

With generic.auto.* each call site re-derives the whole reachable graph: 10 wrappers over one 27-type graph cost 19.3s and 1.91 MB, against 7.2s and 435 KB for one. Linear, not a blowup, but the docs should point people at derives Pickler the way the Schema docs do. Leaving auto in scope when every type already has a given still costs ~9%, because each nested type expands the candidate down to derivePicklerImpl just to hit the depth > 0 abort.

Code that can go — around 250 lines

  • Dead branch in the rule pipeline. StructuralRule covers every Shape and ends in .map(Rule.matched) (SchemaDerivation.scala:196), so the Left(reasons) branch at :119-126 and all three Rule.yielded strings can never run, and UnsupportedType.reasons is dead. A plain chain would do.
  • LoadStandardExtensionsOnce.scala — 30 lines and a var guarding a double load that cannot happen; one call site (PicklerMacrosImpl.scala:271).
  • DerivationTimeout.scala — 63 lines, eight unit spellings, two warning branches, no tests, and the documented format does not work. Keeping only the seconds path is about 15 lines.
  • PicklerDerivationError — 88 lines of sealed ADT that is never pattern-matched. A message catalogue is fine; the ADT adds nothing.
  • Scala 2 scaffolding — PlatformSupport, PicklerCompanionCompat and PicklerMacros each justify themselves with a Scala 2.13 port that does not exist, and PicklerCompanionCompat is not even in src/main/scala-3/ where its own comment says it would have to go. Either commit to it or drop it.
  • PicklerUtils.toTapirCodec is a verbatim copy of TapirJsonJsoniter.jsoniterCodec. The stated reason (not depending on tapir-jsoniter-scala) does not hold, since that module only depends on core and jsoniter-scala-core. Its implicit jsoniterCodec would clash though, so a shared non-implicit helper is the fix, not dependsOn.
  • Test duplication. PicklerCoproductTest (8 tests) and PicklerEnumTest (7) are the µPickle-era suites pointed at the new code; every test is already covered by CodecDerivationTest/PicklerFacadeTest against a near-duplicate fixture set. PicklerBasicTest has six unique tests out of about 15. PicklerScaffoldingTest is five should not be null checks the compiler already proves; only :47-52 (the hoisting check) tests something real.

Visibility: nothing in internal/compiletime is private[pickler] — the whole 16-case Shape ADT and every derivation trait are public API. Every macro in core/src/main/scala-3/sttp/tapir/internal/ is private[tapir]. (internal/runtime has to stay public, and that is correctly explained in the code.)

Tests

SchemaCodecAgreementTest is the right idea but weaker than its comment says. It walks the JSON, so it only catches under-documentation — an extra discriminator mapping key, an extra subtype, or a field the schema marks optional but the codec requires all pass. SInteger|SNumber and SBinary|SDate|SDateTime are collapsed, so the number or string kind cannot be checked. Untagged coproducts pass if any subtype matches, which is always true for Either[String, String]. And the decoder only ever sees JSON the encoder just wrote, so the OpenAPI contract direction is untested.

The generators never produce Array, any java.time type, Byte/Short/Float/Double/BigDecimal, a generic case class, or a standalone leaf — which is why bug 3 and the Array[Byte] mismatch stay green. Adding an Array[Byte] field and a standalone leaf to the agreement list turns both red straight away. The config matrix (:49-53) also misses the one combination that is broken (withTransientNone plus any other with*).

Four compile-failure tests use bare assertDoesNotCompile with no message check (CodecDerivationTest.scala:294, :308, :364, PicklerFacadeTest.scala:132), so they pass on any compile error, including a typo in the snippet. Their neighbours use typeCheckErrors(...) should include(...).

One thing was dropped with no replacement: master asserted Pickler.derived[RichColorEnum.Cyan.type] compiles. It no longer does, and nothing pins either the new behaviour or a readable error, so a user hitting it gets a raw jsoniter failure.

SchemaRecursionTest is the best file in the diff — :69-77 isolates the exact SName identity property applyValidation needs, with a comment saying why a near-miss would pass everything else.

Docs

generated-doc/out/endpoint/pickler.md was not regenerated. The whole "Migrating from the µPickle-based pickler" section is missing from the published docs, along with the jsoniter-scala-macros dependency note and the nested-override paragraph. The main deliverable of this PR is not in the docs.

Missing from the migration list:

  • CreateDerivedEnumerationPickler.apply is gone, and derivedEnumeration is listed as "largely unchanged".
  • Out-of-the-box picklers for primitives, AnyVal and java.math.* are gone; master's docs promised them.
  • generic.auto.picklerForCaseClass renamed to picklerForType.
  • Tuples no longer derive.
  • The new unconstrained given picklerToCodec[T] (package.scala:9), which will clash with sttp.tapir.json.circe.* if both are imported.

CreateDerivedEnumerationPickler.scala:19-21 contradicts the code: it says cases are rendered by toDiscriminatorValue, and PlatformSupport.scala:45-47 says plainly that they are not. Two examples at doc/endpoint/pickler.md:114,129 still show µPickle-era output the code cannot produce.

build.sbt:989-992 — the commented-out debug option cannot be uncommented as written: the ) above has no comma and the commented line has one.

What's good

Comment quality is unusually high for macro code — the Shape classification, the Hearth idioms and the jsoniter coupling are all explained at the right level, and the reasoning behind decisions is nearly always there. One make per type is the right call and the benchmark backs it up. The compute-names-once invariant is real and enforced by construction. rejectBareCodec and the oneOfUsingField exclusion show the drift problem was thought about carefully — bugs 2 and 3 are gaps in that work, not an absence of it. CI now runs picklerJsonJS3/test, which master did not run at all.

One coupling worth guarding: jsoniterLeafName (PicklerMacros.scala:114-123) reimplements JsonCodecMaker.discriminatorValue's naming rule by hand, and its own comment admits a jsoniter change breaks the leaf mapper silently. A test that fails when jsoniter's leaf naming changes would make a Versions.jsoniter bump safe.

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.

2 participants