Skip to content

fix(mock): strict faker schema mock types for enums, nested spreads, and binary - #3591

Closed
Hypenate wants to merge 9 commits into
orval-labs:masterfrom
Hypenate:fix/3590-strict-faker-schema-mock-types
Closed

fix(mock): strict faker schema mock types for enums, nested spreads, and binary#3591
Hypenate wants to merge 9 commits into
orval-labs:masterfrom
Hypenate:fix/3590-strict-faker-schema-mock-types

Conversation

@Hypenate

@Hypenate Hypenate commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Emit schema-aware strict {Schema}Mock aliases: mapped types for objects, type aliases for enums/scalars/arrays, and ArrayBuffer for binary faker schema mocks
  • Cast nested get*Mock() spreads to base mock types so MockWithNullableOverrides parent casts typecheck
  • Collect inline split-factory schema names for missing mock type declarations in schemas/index.faker.ts
  • Add petstore regression spec tests/specifications/issue-3590.yaml (follows issue-3525.yaml style)

Fixes #3590

Test plan

  • packages/mock unit tests (247 pass)
  • tests/generated/mock typecheck passes (node tests/scripts/typecheck-generated.mjs)
  • Snapshot test mock issue-3590 strict faker schema mocks typecheck and emit correct aliases

Summary by CodeRabbit

  • New Features

    • Generated mocks now include per-type schema "kinds" (object | alias | binary) and propagate this metadata through mock and MSW outputs.
    • Improved import recovery and de-duplication so consolidated schema factory imports are restored when needed.
  • Bug Fixes

    • Stricter mock typings and casts for delegated/array/binary schemas to produce correct aliasing and binary mappings.
  • Tests

    • New fixtures and tests covering kind classification, import recovery, and regression cases for strict mock generation.

…and binary

Classify strict {Schema}Mock aliases by schema shape.

Cast nested factory spreads to base mock types.

Add petstore regression spec for #3590.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds StrictMockSchemaKind classification, threads per-type kind maps through mock generators and writers, emits kind-aware strict mock type declarations and conditional casts in faker output, introduces import-recovery/merge utilities, and expands tests and fixtures for issue-3590 scenarios.

Changes

Strict Mock Schema Kinds: Classification, Propagation, and Type Emission

Layer / File(s) Summary
Core type contract for strict schema kinds
packages/core/src/types.ts
Introduces StrictMockSchemaKind = 'object' | 'alias' | 'binary' and adds optional strictMockSchemaKinds to generator and finalize option shapes.
Schema kind classification and type-aware emission
packages/mock/src/mock-types.ts, packages/mock/src/mock-types.test.ts
classifyStrictMockSchemaType derives kinds; declaration helpers accept kind maps to emit aliases/binary mappings/mapped types; getStrictMockSchemaKindsFromResponses and mergeStrictMockSchemaKinds added; tests validate classification and response-derived kinds.
MSW and Faker collect and expose kinds
packages/mock/src/msw/index.ts, packages/mock/src/faker/index.ts
Generators compute and return strictMockSchemaKinds, merging kinds derived from responses and inferred names from implementations; faker returns kinds alongside strictMockSchemaTypeNames.
Mock output aggregation and flattening
packages/core/src/writers/target.ts, packages/core/src/writers/target-tags.ts
Propagates and merges strictMockSchemaKinds when flattening and aggregating per-operation and per-tag mock outputs.
Finalize options and filtering
packages/core/src/writers/finalize-mock-implementation.ts, packages/core/src/writers/single-mode.ts, packages/core/src/writers/split-mode.ts, packages/core/src/writers/tags-mode.ts, packages/orval/src/write-specs.ts, packages/orval/src/client.ts
getFinalizeMockImplementationOptions aggregates strictMockSchemaKinds; writers compute finalize options once per mockOutput, filter local strict mock imports, merge recovered schema-factory imports, and forward kinds into builder.finalizeMockImplementation.
Recover and merge schema-factory imports
packages/core/src/writers/mock-imports.ts, packages/core/src/writers/mock-imports.test.ts
New utilities build known factory/type symbol sets, scan implementations for get<Schema>Mock() and as <Schema>Mock usages, recover missing imports, and merge generator import groups preferring value imports.
Import-delta helpers and usage
packages/mock/src/faker/imports.ts, packages/mock/src/faker/imports.test.ts, packages/mock/src/msw/mocks.ts
Adds appendImportsDelta, mergeReturnedMockImports, and collectSplitMockTypeImports; used to append only newly generated imports and avoid duplicate/exponential imports across combine/object/msw flows.
Strict casts in faker resolvers
packages/mock/src/faker/getters/array-item-factory.ts, packages/mock/src/faker/resolvers/value.ts, packages/mock/src/faker/getters/combine.ts, packages/mock/src/faker/getters/object.ts
Array-item factories and delegated object-like resolver output include conditional as <StrictType> casts when strict mocking is enabled; import snapshots and delta merges ensure imports remain correct.
Tests and fixtures for issue-3590
tests/specifications/issue-3590*.yaml, tests/configs/mock.config.ts, tests/api-generation.spec.ts, various mock/faker tests
New OpenAPI fixtures, config entries, and Vitest cases exercise enum aliasing, nested spreads, binary responses, wide-schema import behavior, and import recovery; tests updated to assert kind-aware outputs.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

mock

Suggested reviewers

  • melloware

🐰 I hopped through types by moonlit streams,
I sorted kinds and mended dreams,
Alias, object, binary too—
I cast the mocks to match what's true.
A little rabbit cheers the run—hooray for typed faux fun!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 26.19% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly summarizes the main changes: fixing strict faker schema mock types for enums, nested spreads (via casts), and binary responses.
Linked Issues check ✅ Passed The PR addresses all three bugs from #3590: enum mock aliases (via strictMockSchemaKinds), nested spread casting (via strict type casting in getters), and binary ArrayBuffer mismatch (via binary kind classification).
Out of Scope Changes check ✅ Passed All changes are directly related to fixing #3590 issues: type infrastructure, import recovery, and test coverage for the reported problems.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Co-authored-by: Cursor <cursoragent@cursor.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
packages/core/src/writers/target.ts (1)

83-87: ⚡ Quick win

Remove the redundant fallback in the strictMockSchemaKinds merge helper paths.

The same ...(x ?? {}) pattern appears in packages/core/src/writers/target.ts and packages/core/src/writers/target-tags.ts, and both sites are hitting unicorn/no-useless-fallback-in-spread in CI. A straight spread of the optional map is enough in each file.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/writers/target.ts` around lines 83 - 87, The merge uses a
redundant fallback spread like ...(acc.strictMockSchemaKinds ?? {}) which
triggers unicorn/no-useless-fallback-in-spread; update the helper merges (e.g.,
where acc.strictMockSchemaKinds and opMock.strictMockSchemaKinds are combined in
the target.ts and the analogous site in target-tags.ts) to simply spread the
optional maps without the ?? {} fallback (e.g., ...(acc.strictMockSchemaKinds)
and ...(opMock.strictMockSchemaKinds) or only spread
opMock.strictMockSchemaKinds into acc), removing the unnecessary
nullish-coalescing wrapper.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/mock/src/faker/index.test.ts`:
- Around line 15-19: Remove the unused imports to fix TypeScript/ESLint errors:
in the import that currently lists classifyStrictMockSchemaType,
collectStrictMockSchemaTypeNamesFromImplementation, and
dedupeStrictMockTypeDeclarations, delete the unused symbols
classifyStrictMockSchemaType and
collectStrictMockSchemaTypeNamesFromImplementation and only keep
dedupeStrictMockTypeDeclarations so the test file imports just the symbol it
actually uses.

In `@packages/mock/src/mock-types.test.ts`:
- Around line 357-366: The test currently force-casts the fixture to
ResReqTypesValue[] causing TS2352; change the cast to go through an unknown
intermediate (e.g., cast the literal to unknown and then to ResReqTypesValue[])
or replace the fixture with a fully typed object matching ResReqTypesValue so
the compiler accepts the shape; update the responses declaration (variable name:
responses) to use the safer "as unknown as ResReqTypesValue[]" pattern or
construct values that satisfy the ResReqTypesValue interface instead.

In `@packages/mock/src/msw/index.ts`:
- Around line 502-504: The object spread for strictMockSchemaKinds uses a
redundant nullish fallback; remove the "?? {}" so the spread only references
baseDefinition.strictMockSchemaKinds to satisfy
unicorn/no-useless-fallback-in-spread; update the const strictMockSchemaKinds
declaration to spread baseDefinition.strictMockSchemaKinds directly.

---

Nitpick comments:
In `@packages/core/src/writers/target.ts`:
- Around line 83-87: The merge uses a redundant fallback spread like
...(acc.strictMockSchemaKinds ?? {}) which triggers
unicorn/no-useless-fallback-in-spread; update the helper merges (e.g., where
acc.strictMockSchemaKinds and opMock.strictMockSchemaKinds are combined in the
target.ts and the analogous site in target-tags.ts) to simply spread the
optional maps without the ?? {} fallback (e.g., ...(acc.strictMockSchemaKinds)
and ...(opMock.strictMockSchemaKinds) or only spread
opMock.strictMockSchemaKinds into acc), removing the unnecessary
nullish-coalescing wrapper.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 21e08a84-5e98-4cce-90da-228625678410

📥 Commits

Reviewing files that changed from the base of the PR and between 62e3fe9 and dbc6b02.

⛔ Files ignored due to path filters (36)
  • tests/__snapshots__/mock/issue-3525-multi/endpoints.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3525/model/index.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/pets/index.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/pets/index.msw.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/pets/pets/pets.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/pets/pets/pets.msw.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/pets/pets/pets.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/schemas/index.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/schemas/index.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split-fetch/schemas/pet.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/pets/index.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/pets/index.msw.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/pets/pets/pets.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/pets/pets/pets.msw.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/pets/pets/pets.service.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/schemas/index.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3572-strict-mock-tags-split/schemas/pet.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3574-strict-mock-tags-split-fetch/pets/pets/pets.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3574-strict-mock-tags-split-fetch/pets/pets/pets.msw.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3574-strict-mock-tags-split-multi-fetch/store/store/store.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3574-strict-mock-tags-split-multi-fetch/store/store/store.msw.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3574-strict-mock-tags-split/pets/pets/pets.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3574-strict-mock-tags-split/pets/pets/pets.msw.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/endpoints.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/index.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/index.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/pet.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/petCategory.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/petDetail.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/petDetailSettingsItem.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/petList.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/petProfile.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/petSetting.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/photoUpload.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/score.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590/model/status.ts is excluded by !**/__snapshots__/**
📒 Files selected for processing (16)
  • packages/core/src/types.ts
  • packages/core/src/writers/finalize-mock-implementation.ts
  • packages/core/src/writers/target-tags.ts
  • packages/core/src/writers/target.ts
  • packages/mock/src/faker/getters/array-item-factory.ts
  • packages/mock/src/faker/index.test.ts
  • packages/mock/src/faker/index.ts
  • packages/mock/src/faker/resolvers/value.ts
  • packages/mock/src/mock-types.test.ts
  • packages/mock/src/mock-types.ts
  • packages/mock/src/msw/index.ts
  • packages/orval/src/client.ts
  • packages/orval/src/write-specs.ts
  • tests/api-generation.spec.ts
  • tests/configs/mock.config.ts
  • tests/specifications/issue-3590.yaml

Comment thread packages/mock/src/faker/index.test.ts Outdated
Comment thread packages/mock/src/mock-types.test.ts Outdated
Comment thread packages/mock/src/msw/index.ts
Co-authored-by: Cursor <cursoragent@cursor.com>
@Hypenate

Copy link
Copy Markdown
Contributor Author

@wadakatu We still have some issues wit the latest version of Orval.
But it's already in a much beter state, so hopefully this will be the nail in the coffin (famous last words).

Import {Schema}Mock from index.faker when operation fakers spread schema
factories with strict casts. Skip imports that duplicate local response mocks.

Co-authored-by: Cursor <cursoragent@cursor.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/core/src/writers/split-tags-mode.ts`:
- Around line 259-277: The import pruning is happening unconditionally by
calling filterLocalStrictMockTypeImports(...) before checking
builder.finalizeMockImplementation, which removes strict schema type imports
even when the builder will not emit local `{Schema}Mock` aliases; change the
code so you only call filterLocalStrictMockTypeImports and pass its result into
generateImportsForBuilder when builder.finalizeMockImplementation is truthy
(i.e., move the filter/filtered-imports logic behind the same
builder.finalizeMockImplementation check), using the same
getFinalizeMockImplementationOptions(...) /
finalizeMockOptions.strictSchemaTypeNames and mockOutput.imports symbols to
compute the filtered set only when finalizeMockImplementation will run.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 74b6468c-c206-4599-8aeb-d46d94b33c1d

📥 Commits

Reviewing files that changed from the base of the PR and between 351e27e and 9fc95da.

📒 Files selected for processing (5)
  • packages/core/src/writers/finalize-mock-implementation.ts
  • packages/core/src/writers/split-tags-mode.ts
  • packages/core/src/writers/tags-mode.ts
  • packages/mock/src/faker/index.ts
  • packages/mock/src/faker/resolvers/value.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/core/src/writers/finalize-mock-implementation.ts
  • packages/mock/src/faker/resolvers/value.ts
  • packages/mock/src/faker/index.ts

Comment on lines 259 to 277
const finalizeMockOptions = getFinalizeMockImplementationOptions(
output,
mockOutput,
);

const importsMockForBuilder = generateImportsForBuilder(
output,
mockOutput.imports,
filterLocalStrictMockTypeImports(
mockOutput.imports,
finalizeMockOptions.strictSchemaTypeNames,
),
mockRelativeSchemasPath,
);

const finalizedMockImplementation = builder.finalizeMockImplementation
? builder.finalizeMockImplementation(
mockOutput.implementation,
getFinalizeMockImplementationOptions(output, mockOutput),
finalizeMockOptions,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Only remove external strict mock type imports when local aliases will actually be emitted.

packages/core/src/writers/split-tags-mode.ts and packages/core/src/writers/tags-mode.ts now call filterLocalStrictMockTypeImports(...) before checking whether builder.finalizeMockImplementation exists. That couples import pruning to strictSchemaTypeNames, but not to the step that turns those names into local {Schema}Mock declarations. For builders that do not implement finalizeMockImplementation, the generated mock file keeps the strict casts but loses the imported type, producing an unresolved identifier. Gate the filtering behind the same builder.finalizeMockImplementation check in both files.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/writers/split-tags-mode.ts` around lines 259 - 277, The
import pruning is happening unconditionally by calling
filterLocalStrictMockTypeImports(...) before checking
builder.finalizeMockImplementation, which removes strict schema type imports
even when the builder will not emit local `{Schema}Mock` aliases; change the
code so you only call filterLocalStrictMockTypeImports and pass its result into
generateImportsForBuilder when builder.finalizeMockImplementation is truthy
(i.e., move the filter/filtered-imports logic behind the same
builder.finalizeMockImplementation check), using the same
getFinalizeMockImplementationOptions(...) /
finalizeMockOptions.strictSchemaTypeNames and mockOutput.imports symbols to
compute the filtered set only when finalizeMockImplementation will run.

@Hypenate
Hypenate marked this pull request as draft June 12, 2026 07:16
@Hypenate

Copy link
Copy Markdown
Contributor Author

I'm going for a full AI approach here...

I'll let it make a local build of Orval and check again against all our yaml files.
I'll let it itterate over this processes until all our errors are gone.

I'm not sure how we'll do the review process here...
I'll wait a bit for what it spits out.

Hypenate and others added 5 commits June 12, 2026 09:51
…ports (#3590)

Use appendImportsDelta to avoid stack overflows with schemas: true.

Recover missing get*Mock imports in tags-split faker files.

Guard MSW generation when response.imports is undefined.

Add pet-themed regression specs and tests for each failure mode.

Co-authored-by: Cursor <cursoragent@cursor.com>
Use OpenApiSchemaObject and createTestContextSpec override options.

Cast undefined response imports via unknown for the runtime guard test.

Co-authored-by: Cursor <cursoragent@cursor.com>
)

Merge returned mock imports only when the shared array was not mutated in place.

Propagate oneOf split helper types into shared import aggregation.

Co-authored-by: Cursor <cursoragent@cursor.com>
Merge getMockScalar result imports in generateFakerForSchemas.

Collect nested oneOf split helper types and filter local strict mock imports in single mode.

Co-authored-by: Cursor <cursoragent@cursor.com>
PetDetailSettingsItem is referenced by strict mock factories and must be imported in index.faker.ts.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Hypenate
Hypenate marked this pull request as ready for review June 12, 2026 08:39

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
packages/core/src/writers/split-tags-mode.ts (1)

289-299: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Only filter local strict mock type imports when local aliases will be emitted.

The call to filterLocalStrictMockTypeImports at line 291 is unconditional, but local {Schema}Mock type declarations are only emitted when builder.finalizeMockImplementation exists (lines 268-273). For builders without finalization support, this removes strict schema type imports while the generated mock file keeps strict casts that reference those types, producing unresolved identifier errors.

Gate the filtering behind the same builder.finalizeMockImplementation check:

🔒 Proposed fix to conditionally filter imports
-          const importsMockForBuilder = generateImportsForBuilder(
-            output,
-            filterLocalStrictMockTypeImports(
-              mergeGeneratorImports(
-                mockOutput.imports,
-                recoveredSchemaFactoryImports,
-              ),
-              finalizeMockOptions.strictSchemaTypeNames,
-            ),
-            mockRelativeSchemasPath,
-          );
+          const mergedImports = mergeGeneratorImports(
+            mockOutput.imports,
+            recoveredSchemaFactoryImports,
+          );
+          const filteredImports = builder.finalizeMockImplementation
+            ? filterLocalStrictMockTypeImports(
+                mergedImports,
+                finalizeMockOptions.strictSchemaTypeNames,
+              )
+            : mergedImports;
+          const importsMockForBuilder = generateImportsForBuilder(
+            output,
+            filteredImports,
+            mockRelativeSchemasPath,
+          );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/writers/split-tags-mode.ts` around lines 289 - 299, The
call to filterLocalStrictMockTypeImports is unconditional and removes strict
schema type imports even when local {Schema}Mock type aliases are not emitted;
update the generateImportsForBuilder invocation so that
filterLocalStrictMockTypeImports is only applied when
builder.finalizeMockImplementation is truthy: use
mergeGeneratorImports(mockOutput.imports, recoveredSchemaFactoryImports)
directly for the imports argument when builder.finalizeMockImplementation is
falsy, and wrap/filter with filterLocalStrictMockTypeImports(...,
finalizeMockOptions.strictSchemaTypeNames) only when
builder.finalizeMockImplementation exists (referencing
generateImportsForBuilder, filterLocalStrictMockTypeImports,
mergeGeneratorImports, mockOutput.imports, recoveredSchemaFactoryImports,
finalizeMockOptions.strictSchemaTypeNames, and
builder.finalizeMockImplementation).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@packages/core/src/writers/split-tags-mode.ts`:
- Around line 289-299: The call to filterLocalStrictMockTypeImports is
unconditional and removes strict schema type imports even when local
{Schema}Mock type aliases are not emitted; update the generateImportsForBuilder
invocation so that filterLocalStrictMockTypeImports is only applied when
builder.finalizeMockImplementation is truthy: use
mergeGeneratorImports(mockOutput.imports, recoveredSchemaFactoryImports)
directly for the imports argument when builder.finalizeMockImplementation is
falsy, and wrap/filter with filterLocalStrictMockTypeImports(...,
finalizeMockOptions.strictSchemaTypeNames) only when
builder.finalizeMockImplementation exists (referencing
generateImportsForBuilder, filterLocalStrictMockTypeImports,
mergeGeneratorImports, mockOutput.imports, recoveredSchemaFactoryImports,
finalizeMockOptions.strictSchemaTypeNames, and
builder.finalizeMockImplementation).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 398f0e31-c200-4f06-819a-c5fb203cf807

📥 Commits

Reviewing files that changed from the base of the PR and between 9fc95da and 09a69d0.

⛔ Files ignored due to path filters (45)
  • tests/__snapshots__/mock/issue-3590-binary-response-imports/endpoints.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-binary-response-imports/model/index.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-tags-split-schema-imports/pets/pets/pets.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-tags-split-schema-imports/pets/pets/pets.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-tags-split-schema-imports/schemas/healthStatus.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-tags-split-schema-imports/schemas/index.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-tags-split-schema-imports/schemas/index.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-tags-split-schema-imports/schemas/pet.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-tags-split-schema-imports/schemas/petDetailResponse.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-tags-split-schema-imports/schemas/petOwner.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-tags-split-schema-imports/schemas/responseBase.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/endpoints.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child0.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child1.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child10.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child11.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child12.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child13.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child14.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child15.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child16.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child17.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child18.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child19.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child2.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child20.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child21.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child22.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child23.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child24.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child25.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child26.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child27.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child28.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child29.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child3.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child4.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child5.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child6.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child7.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child8.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/child9.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/index.faker.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/index.ts is excluded by !**/__snapshots__/**
  • tests/__snapshots__/mock/issue-3590-wide-schema-imports/model/wideParent.ts is excluded by !**/__snapshots__/**
📒 Files selected for processing (21)
  • packages/core/src/writers/mock-imports.test.ts
  • packages/core/src/writers/mock-imports.ts
  • packages/core/src/writers/single-mode.ts
  • packages/core/src/writers/split-mode.ts
  • packages/core/src/writers/split-tags-mode.ts
  • packages/core/src/writers/tags-mode.ts
  • packages/mock/src/faker/getters/combine.ts
  • packages/mock/src/faker/getters/object.test.ts
  • packages/mock/src/faker/getters/object.ts
  • packages/mock/src/faker/imports.test.ts
  • packages/mock/src/faker/imports.ts
  • packages/mock/src/faker/index.test.ts
  • packages/mock/src/faker/index.ts
  • packages/mock/src/faker/resolvers/value.ts
  • packages/mock/src/msw/mocks.test.ts
  • packages/mock/src/msw/mocks.ts
  • tests/api-generation.spec.ts
  • tests/configs/mock.config.ts
  • tests/specifications/issue-3590-binary-response-imports.yaml
  • tests/specifications/issue-3590-tags-split-schema-imports.yaml
  • tests/specifications/issue-3590-wide-schema-imports.yaml
✅ Files skipped from review due to trivial changes (2)
  • packages/core/src/writers/mock-imports.test.ts
  • packages/mock/src/msw/mocks.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/core/src/writers/tags-mode.ts
  • packages/mock/src/faker/resolvers/value.ts
  • packages/mock/src/faker/index.ts

@Hypenate

Copy link
Copy Markdown
Contributor Author

@wadakatu & @melloware

The build is green, but we have almost 100 changed files...
How do we proceed?

Do we go yolo mode or do I split this up?

@melloware melloware added the mock Related to mock generation label Jun 12, 2026
@melloware

Copy link
Copy Markdown
Collaborator

@Hypenate a lot of the changes are because it added tests which I am OK with. I usually don't look too much at the tests but at the core code changed. It doesn't seem "too bad" but i will let @wadakatu review.

@Hypenate Hypenate closed this by deleting the head repository Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mock Related to mock generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Strict faker mocks (required+nonNullable): enum *Mock types, nested spreads, and Blob/ArrayBuffer mismatch

2 participants