Skip to content

fix(downgrader): stop nested multi-type arrays from doubling 3.1 to 3.0 output - #21

Merged
dinwwwh merged 3 commits into
mainfrom
claude/objective-clarke-8d824f
Sep 26, 2026
Merged

dinwwwh merged 3 commits into
mainfrom
claude/objective-clarke-8d824f

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Sep 26, 2026

Copy link
Copy Markdown
Member

The 3.1 → 3.0 downgrader no longer doubles its output at each nesting level of a multi-type array schema such as { type: ['array', 'object'], items: ... }. A 20-level chain took about 5.6 s and produced 85M characters of JSON. It now takes 0.3 ms and produces 1,117 characters.

Fixes

  • items next to a multi-type type now moves into the generated array variant. Before, it stayed at the top level and a copy went into the variant. Validation is unchanged: top-level items only applies to arrays, and only the array variant accepts arrays.
  • A cyclic multi-type array schema now has its array variant point back at the converted schema. Before, it pointed at a partial copy that had no anyOf.

Performance

Nesting depth Before After
16 304 ms, 5.3M chars 1 ms, 897 chars
20 5.6 s, 85M chars 0.3 ms, 1,117 chars
200 out of memory 1.7 ms, 11K chars

Pointing both places at the same converted object would fix conversion time but not size. JSON.stringify writes shared objects out in full, so the serialized document stayed at 85M characters.

Testing

  • Added a regression test that converts a 10-level chain and checks the exact output. It fails on the old code.
  • Added cases showing that items stays at the top level when the union has no array variant, and when an invalid allOf makes the converter drop the union.
  • pnpm vitest run (355 tests), pnpm lint and pnpm type:check pass. The e2e and corpus tests validate the output against the 3.0 schema.

….0 output

A schema like `{ type: ['array', 'object'], items: ... }` kept `items` at the
top level and cloned it into the synthesized array variant, so every nesting
level doubled the output. `items` now moves into the array variant instead.
# Conflicts:
#	packages/downgrader/README.md
@codecov

codecov Bot commented Sep 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Minor suggestion inline — the fix itself looks correct.

Reviewed changes

  • applyTypes moves sibling items — in packages/downgrader/src/v3.1-to-v3.0.ts, the synthesized array variant now reuses out.items and deletes the top-level copy instead of deep-cloning it while leaving one in place, so nested multi-type arrays grow linearly instead of doubling per level.
  • Malformed-allOf guard — an early return preserves top-level items when an existing anyOf plus a non-array allOf makes the type union get dropped.
  • Tests and docs — items-placement cases (moved / retained when no array variant / retained when the union is dropped), a 10-level regression test asserting exact output, and the README mapping row.

I re-ran pnpm test (355 passing across 5 files) and traced the move against every applyTypes branch: removing top-level items is safe because it only constrains arrays, and every array instance must still satisfy the array variant (directly as anyOf, or nested under allOf when anyOf already exists).

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (default — pick a model for stronger reviews) | 𝕏

Comment thread packages/downgrader/src/v3.1-to-v3.0.ts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Regression test for the cyclic multi-type array variant — 8c7c7b1 adds a case converting a self-referential { type: ['array', 'object'], items: <self> } schema, asserting the top-level items is gone, the array variant's items points at the converted schema, and the input is left unmutated. I reverted only the items move locally and confirmed the new test fails on the pre-fix code (the result keeps an items self-reference), then re-ran the file against the restored source (141 passing).

This directly addresses the prior pullfrog inline suggestion; that thread has been resolved. No further concerns.

Pullfrog  | View workflow run | Using DeepSeek Flash (default — pick a model for stronger reviews) | 𝕏

@dinwwwh
dinwwwh merged commit 29e5cf4 into main Sep 26, 2026
7 checks passed
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