Skip to content

fix(infra): lenient store JSON decode so jitM can migrate legacy documents - #894

Closed
patroza wants to merge 2 commits into
mainfrom
fix/validate-sample-json-codec
Closed

patroza wants to merge 2 commits into
mainfrom
fix/validate-sample-json-codec

Conversation

@patroza

@patroza patroza commented Sep 16, 2026 •

Copy link
Copy Markdown
Member

Problem

makeJsonDocumentCodec.decode ran the full schema over every stored document:

const codec = S.toCodecJson(S.toEncoded(schema))
decode: (doc) => S.decodeSync(codec)(rest)

It is called by Cosmos (fromStored), SQL (parseRow) and Memory (decodeDoc) on every read — find, filter, all, validateSample — and it runs before the repository's jitM migration (mapFrom). So a document of an older shape, which jitM exists precisely to migrate, now fails to read at all:

SchemaError: Missing key
  at ["vatRate"]

Reproduced directly: makeJsonDocumentCodec(Shop).decode({ id, name, createdAt }) (no vatRate) throws.

This is not limited to validateSample — it breaks normal reads of real production documents. Found via macs-holding/configurator, where all 7 DB Validation jobs (demo + prod) fail on Shop.vatRate, User.permissions and Configurator.conditionGroups[].conditions[].rules[].groupId. Introduced by #874 ("native Date/Map/Set Encoded"), released in beta.323+.

Fix

Decode at the store boundary is now lenient: it only lifts JSON back to native Encoded values (Date/Map/Set and app-native declarations) for the keys that are present.

decodeWithSchema / decodeJson in Store/utils.ts is the decode counterpart of the existing value-driven encodeJson walker and reuses its helpers (unwrapAst, astAtPath, elementAst, isPlainObject):

  • iterates the document's own keys, never the schema's, so an absent key stays absent
  • tagged unions resolve by _tag literal, else by the first member that decodes
  • arrays recurse per element; ReadonlyMap/ReadonlySet declarations reconstruct via toCodecJson
  • a leaf that cannot be decoded passes through unchanged (runSyncExit, so defects can't escape either), leaving the repository's own strict decode — which runs after jitM — to report it with full path context
  • no required keys, refinements or checks are enforced here

encode still uses the strict whole-document codec: writes always carry a complete document.

Tests

  • packages/infra/test/json-document-lenient.test.ts (8): complete document lowers Date/Set/Map/app-native declaration and preserves _etag; missing top-level key; missing key inside array[].struct[].field with siblings still lowered; tagged-union member resolution; unparseable leaf (null, "not-a-date") passes through; refinements not enforced; encodeKeys-renamed fields lower and missing renamed keys stay absent; encode round-trip.
  • packages/infra/test/repository-legacy-document.test.ts (3): repo.all, repo.find and repo.validateSample over a legacy document missing vatRate that jitM fills.

packages/infra: 284 passed / 26 skipped, 0 failures. pnpm check and pnpm lint clean.

Worth a follow-up (not in this PR)

  1. Type jitM as a JSON value and reorder the pipeline to jitM → toCodecJson → schema, i.e. the store returns raw JSON documents and the repository decodes once after migrating. jitM is currently typed (pm: Encoded) => Encoded, which since feat: native Date/Map/Set Encoded; query adapters convert to JSON #874 promises native Date/Map/Set while jitMs are actually written against stored JSON. That would also make this lenient walker unnecessary for documents.
  2. encode is strict, so a legacy-shaped document cannot be stored or re-saved at all — which makes Memory/Disk unusable for legacy fixtures (the repository test here needs its own store harness). Worth deciding whether writes should fail loudly or lower leniently.
  3. Memory and Disk never apply config.defaultValues on read, while Cosmos and SQL merge them on every read — so a defaultValues migration behaves differently in tests than in production.

Note defaultValues only ever fills absent keys, and a stored null correctly wins over a default — null is a value, not a missing key. Configurator's Preconfiguration.updatedAt failure is therefore a genuine data/schema mismatch on that side, not something this PR should hide.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

patroza and others added 2 commits September 16, 2026 07:48
…ments

`makeJsonDocumentCodec.decode` ran the full schema over every stored document,
so reads of an older-shaped document failed with `Missing key` before the
repository's `jitM` could add the key -- breaking `find`, `filter`, `all` and
`validateSample` on real production data.

Decode now walks the stored document with `decodeWithSchema`, the decode
counterpart of `encodeJson`: it iterates the document's own keys and only lifts
JSON to native Encoded values (Date/Map/Set and app-native declarations). Keys
that are absent stay absent, refinements and checks are not enforced, and a leaf
that cannot be decoded passes through unchanged, so the repository's own decode
-- which runs after `jitM` -- reports it with full path context. Writes keep
using the strict whole-document codec.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@pkg-pr-new

pkg-pr-new Bot commented Sep 16, 2026

Copy link
Copy Markdown

Open in StackBlitz

@effect-app/cli

npm i https://pkg.pr.new/effect-app/libs/@effect-app/cli@894

effect-app

npm i https://pkg.pr.new/effect-app/libs/effect-app@894

@effect-app/eslint-codegen-model

npm i https://pkg.pr.new/effect-app/libs/@effect-app/eslint-codegen-model@894

@effect-app/eslint-shared-config

npm i https://pkg.pr.new/effect-app/libs/@effect-app/eslint-shared-config@894

@effect-app/infra

npm i https://pkg.pr.new/effect-app/libs/@effect-app/infra@894

@effect-app/vue

npm i https://pkg.pr.new/effect-app/libs/@effect-app/vue@894

@effect-app/vue-components

npm i https://pkg.pr.new/effect-app/libs/@effect-app/vue-components@894

commit: 872f698

@patroza

patroza commented Sep 16, 2026

Copy link
Copy Markdown
Member Author

Superseded by #896.

Leniency is out: the owner's call is that nothing should be lenient. #896 rebuilds the fix on main as the jitM JSON → JSON store-boundary pipeline with a strict decode throughout — packages/infra/src/Store/utils.ts is byte-identical to main there, and a document jitM does not repair fails loudly (pinned by a test).

The root cause analysis in this PR still stands and is restated in #896.

@patroza patroza closed this Sep 16, 2026
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