fix(openapi): collapse dotted operationIds into valid endpoint names - #17310
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![nitpickybot[bot]](https://avatars.githubusercontent.com/in/2901896?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review Summary
Small, targeted fix: dotted operationIds are collapsed via camelCase so they don't get parsed as cross-file references (import.endpoint). Logic is duplicated verbatim in the v2 and v3 importers, and the collapse can silently collide with sibling endpoint names. Fixtures/snapshots look consistent with the described behavior.
- 🟡 1 warning(s)
- 🔵 2 suggestion(s)
| // A dot in an endpoint id is parsed as a reference to another file (`import.endpoint`), | ||
| // so dotted operation ids must be collapsed into a single name. | ||
| function sanitizeEndpointId(operationId: string): string { | ||
| return operationId.includes(".") ? camelCase(operationId) : operationId; | ||
| } |
There was a problem hiding this comment.
🟡 warning
Collapsing foo.bar → fooBar can silently collide with an existing fooBar operationId under the same tag, which would overwrite one endpoint in the generated file rather than erroring. Worth at least documenting, and ideally detecting the duplicate at build time (the endpoint map presumably just gets clobbered).
Also note this is a user-visible rename for anyone who already has dotted operationIds and isn't using pagination — their SDK method names change. Make sure the changelog entry calls that out.
There was a problem hiding this comment.
Changelog updated in 07d3cca to call out the rename explicitly (dotted ids are now imported as e.g. actionsIndex; x-fern-sdk-method-name pins a specific name).
On the collision: agreed it's theoretically possible (foo.bar + fooBar under the same tag), but note the collapse only runs on ids that contain a dot, and an id like foo.bar is currently unusable for anything that resolves endpoints by key — so the realistic blast radius is small. Proper duplicate detection would need to live in the endpoint-map construction (both importers), which is beyond this fix; flagging it to the requester as a possible follow-up rather than expanding scope here.
| // A dot in a method name is parsed as a reference to another file (`import.endpoint`), | ||
| // so dotted operation ids must be collapsed into a single name. | ||
| return operationId.includes(".") ? camelCase(operationId) : operationId; |
There was a problem hiding this comment.
🔵 suggestion
This is the exact same predicate + comment as sanitizeEndpointId in openapi-ir-to-fern/src/utils/getEndpointLocation.ts. Since the two importers must stay in agreement (v2/v3 snapshot parity), consider exporting a single shared helper (e.g. from a commons package) so a future tweak doesn't drift between them.
There was a problem hiding this comment.
Fair, though after e3f6dc2 the two are no longer identical in placement: v2 sanitizes the endpoint id in getEndpointLocation, v3 sanitizes the method name after tokenization in computeGroupNameFromTagAndOperationId. The shared part is a one-line predicate, and the v2 package (openapi-ir-to-fern) doesn't currently depend on importer-commons, so extracting it would mean a new cross-package dependency. Leaving as-is for this bugfix; the new x-fern-pagination-tag-mismatch fixture exists in both suites specifically to catch drift between them.
| // A dot in an endpoint id is parsed as a reference to another file (`import.endpoint`), | ||
| // so dotted operation ids must be collapsed into a single name. | ||
| function sanitizeEndpointId(operationId: string): string { | ||
| return operationId.includes(".") ? camelCase(operationId) : operationId; |
There was a problem hiding this comment.
🔵 suggestion
Only . is handled here, but other characters are equally hostile to Fern names (spaces, /, :, leading digits). Not required for this fix, but if the intent is "produce a valid endpoint name", a general validity check (/^[A-Za-z_][A-Za-z0-9_]*$/) would be more robust than a dot sniff — otherwise the next report is actions index.
There was a problem hiding this comment.
Agreed that . isn't the only hostile character, but it is the only one that changes parsing semantics (import.endpoint), which is what causes the reported failure — spaces/:// produce ugly names, not resolution errors. Broadening to a full validity check would rename endpoints for specs that work today, so it's a behavior change I don't want to slip into a bugfix. Raising it with the requester as a separate item.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // A dot in a method name is parsed as a reference to another file (`import.endpoint`), | ||
| // so dotted operation ids must be collapsed into a single name. | ||
| return operationId.includes(".") ? camelCase(operationId) : operationId; |
There was a problem hiding this comment.
🟡 Some imported endpoints get a redundant group prefix in their generated name
The operation name is rewritten before it is split into words for grouping (camelCase(operationId) at packages/cli/api-importers/openapi-to-ir/src/3.1/paths/operations/AbstractOperationConverter.ts:334), so for names that mix dots with digits the shared prefix with the tag is no longer detected and stays in the final name.
Impact: Affected endpoints are generated as e.g. users.usersListV2 instead of users.listV2, and differ from what the older importer produces for the same spec.
Tokenization changes when camelCase merges digit boundaries
computeGroupNameFromTagAndOperationId (packages/cli/api-importers/openapi-to-ir/src/3.1/paths/operations/AbstractOperationConverter.ts:337-357) tokenizes the value returned by evaluateMethodNameFromOperation. tokenizeString only splits on capital letters when the input matches /^[a-z]+(?:[A-Z][a-z]+)*$/ (...:406), which fails as soon as a digit is present.
Example: tag Users, operationId users.list_v2.
- Before:
tokenizeString("users.list_v2")→["users","list","v2"]; tag is a prefix, so method =camelCase("list_v2")=listV2, group[Users]. - After:
camelCase("users.list_v2")=usersListV2;tokenizeString("usersListV2")→["userslistv2"](single token, because the digit disables the camel-case split), so the tag is no longer a prefix andcomputeGroupAndMethodFromTokens(...:359-384) returns methodusersListV2under group[Users].
The v2 importer avoids this because it collapses the dots after tokenizing (packages/cli/api-importers/openapi/openapi-ir-to-fern/src/utils/getEndpointLocation.ts:78-110), so the two importers now disagree for these operation ids.
Prompt for agents
In packages/cli/api-importers/openapi-to-ir/src/3.1/paths/operations/AbstractOperationConverter.ts, evaluateMethodNameFromOperation now returns camelCase(operationId) when the operationId contains a dot. That value is consumed by computeGroupNameFromTagAndOperationId, which tokenizes it via tokenizeString before comparing against the tag tokens. tokenizeString only splits on capital letters when the string matches /^[a-z]+(?:[A-Z][a-z]+)*$/, so any camelCased name containing a digit (e.g. camelCase('users.list_v2') === 'usersListV2') collapses to a single token, and the tag prefix is no longer stripped — producing method names like usersListV2 under group Users instead of listV2. The v2 importer (openapi-ir-to-fern/src/utils/getEndpointLocation.ts) avoids this by tokenizing the raw operationId and only collapsing dots on the branches that emit the operationId verbatim. Consider mirroring that: keep tokenizing the raw operationId for grouping and apply the dot collapse only where the final method name is produced (the tag == null branch and the non-prefix branch), or make tokenizeString digit-aware.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch — confirmed: tokenizeString("users.list_v2") → ["users","list","v2"] but tokenizeString(camelCase("users.list_v2")) = tokenizeString("usersListV2") → ["userslistv2"], so the tag prefix stopped being stripped for ids with digits.
Fixed in e3f6dc2 by mirroring the v2 importer: evaluateMethodNameFromOperation returns the raw operationId again, and the dot collapse moved into a sanitizeMethodName helper applied only where the final method name is emitted (the tag == null branch and the non-prefix branch), i.e. after tokenization.
Added Users + users.list_v2 to the x-fern-pagination-tag-mismatch fixture; both importers now produce users.listV2.
…mporter Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
3 participants
Description
An OpenAPI operation whose
operationIdcontains a dot and whose leading token does not match its tag (e.g.actions.indexunder tagCustom Actions) is imported as an endpoint literally namedactions.indexincustomActions.yml. A dot in a Fern endpoint name is parsed as a cross-file reference (import.endpoint), sox-fern-pagination— the one path that resolves an endpoint by its own key (convertPagination→PropertyResolver→EndpointResolver) — fails with:comments.indexunder tagCommentsworks today only because the tag/operationId prefixes overlap, which makes the importer camelCase the remaining tokens.Changes Made
getEndpointLocation.ts(v2 importer): the two branches that passoperationIdstraight through as the endpoint id (no tag, and tag/operationId mismatch) now run it throughactions.index→actionsIndex. Non-dotted ids are untouched, so no existing snapshot changed.AbstractOperationConverter.evaluateMethodNameFromOperation(v3 importer): same collapse, so the v3 path no longer emits a dotted method name into the IR. Tokenization is unaffected (tokenizeString("actionsIndex")===tokenizeString("actions.index")), so grouping is unchanged.x-fern-pagination-tag-mismatchfixture (Comments/comments.indexvsCustom Actions/actions.index, both paginated) in both importer test suites, plus a CLI changelog entry.Testing
Unit tests added/updated — new fixture + snapshots in
openapi-ir-to-fern-testsandv3-importer-tests;pnpm test:updateproduced no diffs on existing snapshots.Manual testing completed — built the CLI and ran
fern iragainst the fixture project:before:
[api]: Cannot resolve endpoint: actions.index in file customActions.yml(exit 1)after:
[api]: ✓ All checks passed/Wrote IR to /tmp/ir.json, with the endpoint namedactionsIndexunder servicecustomActions.Link to Devin session: https://app.devin.ai/sessions/8fd92fc8964840f4a6b9e314385d65c9
Requested by: @fern-support