Skip to content

fix(mock): resolve circular suffix matching and combine recursion - #3448

Merged
melloware merged 3 commits into
orval-labs:masterfrom
Hypenate:fix/issue-2802-recursion-suffix-mismatch
May 26, 2026
Merged

fix(mock): resolve circular suffix matching and combine recursion#3448
melloware merged 3 commits into
orval-labs:masterfrom
Hypenate:fix/issue-2802-recursion-suffix-mismatch

Conversation

@Hypenate

@Hypenate Hypenate commented May 26, 2026

Copy link
Copy Markdown
Contributor

Fixes #2802

Summary by CodeRabbit

  • Refactor
    • Mock generation now resolves schema references with contextual awareness, improving accuracy for nested and referenced models.
    • Recursion and cycle-prevention checks were enhanced to use context-resolved reference names, reducing incorrect or duplicate mock types.
    • More reliable mock outputs for multi-level schema hierarchies, especially for referenced properties and array items.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 680a7ab4-712a-4366-a52d-e2ef9265abb7

📥 Commits

Reviewing files that changed from the base of the PR and between 8eeb247 and c9b469c.

📒 Files selected for processing (3)
  • packages/mock/src/faker/getters/combine.ts
  • packages/mock/src/faker/getters/object.ts
  • packages/mock/src/faker/getters/scalar.ts

📝 Walkthrough

Walkthrough

Three mock faker getter modules now derive reference names via context-aware getRefInfo(ref, context).name instead of computing PascalCase from $ref paths; call sites were updated to pass context for recursion/skip guards in combine.ts, object.ts, and scalar.ts.

Changes

Reference name resolution refactoring

Layer / File(s) Summary
getReferenceName implementations using getRefInfo
packages/mock/src/faker/getters/combine.ts, packages/mock/src/faker/getters/object.ts, packages/mock/src/faker/getters/scalar.ts
Local getReferenceName helpers and import lists are updated to use getRefInfo(ref, context).name and to accept a context parameter instead of deriving names from the final $ref path segment.
Call sites passing context to getReferenceName
packages/mock/src/faker/getters/combine.ts, packages/mock/src/faker/getters/object.ts, packages/mock/src/faker/getters/scalar.ts
All recursion-prevention and skip logic sites (combine-schemas skip, referenced object properties, additionalProperties, and array items $ref guards) are updated to pass context so the resolved name comes from getRefInfo(..., context).name.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • orval-labs/orval#3035: Both PRs adjust mock generation naming to use @orval/core’s getRefInfo(...).name instead of deriving/transforming the reference name via pascal(...)/pascalCase(...).
  • orval-labs/orval#3421: Related changes to packages/mock/src/faker/getters/scalar.ts's $ref/array-items recursion-prevention handling.

Suggested labels

mock

Suggested reviewers

  • melloware

Poem

A rabbit hops through schema trees,
Sniffing refs and context keys,
From path-hacked names to ones refined,
getRefInfo helps loops unbind,
Hooray — no stack overflow today! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 title 'fix(mock): resolve circular suffix matching and combine recursion' directly addresses the main change: resolving circular reference handling and recursion issues in the mock package.
Linked Issues check ✅ Passed The PR successfully addresses issue #2802 by updating reference name resolution to use context-aware getRefInfo, which prevents the stack overflow from recursive self-referential schemas like AssetItem.
Out of Scope Changes check ✅ Passed All changes are scoped to the mock package's reference resolution logic across three getter files, directly supporting the fix for recursive schema handling.

✏️ 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.

@melloware

Copy link
Copy Markdown
Collaborator

@aqeelat your new code is causing this PR test to fail in DynamicRef can you investigate?

@melloware
melloware force-pushed the fix/issue-2802-recursion-suffix-mismatch branch from 8eeb247 to c9b469c Compare May 26, 2026 15:22
@melloware
melloware merged commit 06e2135 into orval-labs:master May 26, 2026
5 checks passed
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.

Orval 8: Maximum call stack size exceeded when referencing same schema inside schema

2 participants