Conversation
🌱 Seed Test SelectorSelect languages to run seed tests for:
How to use: Click the ⋯ menu above → "Edit" → check the boxes you want → click "Update comment". Tests will run automatically and snapshots will be committed to this PR. |
jischein
changed the title
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
@claude review |
| function addEndpointNavigationTitleOverrides( | ||
| endpointTitlesById: Map<string, string>, | ||
| pkg: ReturnType<typeof convertIrToFdrApi>["rootPackage"], | ||
| subpackageId: string | ||
| ): void { | ||
| for (const endpoint of pkg.endpoints) { | ||
| if (endpoint.name != null) { | ||
| endpointTitlesById.set(getEndpointNavigationId(endpoint, subpackageId), endpoint.name); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Translated WebSocket and webhook navigation titles are silently dropped. addEndpointNavigationTitleOverrides (translatedApiOverrides.ts:414-424) only iterates pkg.endpoints, and applyTranslatedApiNavigationTitlesInObject (translatedApiOverrides.ts:326) only matches record.type === "endpoint", while the FDR ApiDefinitionPackage exposes parallel webhooks/websockets arrays and the nav tree produces type: "webSocket" / type: "webhook" nodes (ApiReferenceNodeConverter.ts:626/:668). Result: on a translated AsyncAPI/OpenAPI-3.1-webhook docs site, sibling HTTP endpoints in the same package translate while WebSocket and webhook H1/sidebar titles stay in the base language — silent partial translation. Fix: also walk pkg.websockets/pkg.webhooks in the harvest pass and add webSocket/webhook branches keyed by webSocketId/webhookId in the apply pass.
Extended reasoning...
What the bug is
This PR introduces translated endpoint navigation titles in two stages:
- Harvest (
addEndpointNavigationTitleOverrides, translatedApiOverrides.ts:414-424) — given the converted FDRapiDefinitionfor the translated workspace, populateendpointTitlesByIdkeyed by an endpoint nav id. - Apply (
applyTranslatedApiNavigationTitlesInObject, translatedApiOverrides.ts:311-342) — walk the locale-specific docs nav tree and, when the current nodetype === "endpoint", replacetitlewith the harvested translation for(apiDefinitionId, endpointId).
Both stages are HTTP-endpoint-only. But the FDR ApiDefinitionPackage returned by convertIrToFdrApi has parallel arrays for the other operation kinds, populated in packages/cli/register/src/ir-to-fdr-converter/convertPackage.ts:33-36:
return {
endpoints: restEndpoints,
webhooks: webhooks != null ? convertWebhookGroup(webhooks) : [],
websockets: websocket != null ? [convertWebSocketChannel(websocket, ir)] : [],
...
};Each entry carries a name derived from the localized source: webhook name: webhook.displayName ?? startCase(getOriginalName(webhook.name)) (convertPackage.ts:79), websocket name: channel.displayName ?? startCase(getOriginalName(channel.name)) (convertPackage.ts:379). Those are the same fields that produce the translated H1/sidebar text for HTTP endpoints — they exist for webhooks and websockets too, but the harvest pass never reads them.
Symmetrically, the docs nav tree produced by ApiReferenceNodeConverter creates dedicated node types for each operation kind:
type: "webSocket"withapiDefinitionId,webSocketId, and a translatabletitle(ApiReferenceNodeConverter.ts:624-646)type: "webhook"withapiDefinitionId,webhookId, and a translatabletitle(ApiReferenceNodeConverter.ts:666-688)
The apply pass only matches record.type === "endpoint" (translatedApiOverrides.ts:326), so even if titles were harvested for these node types, the walker would not apply them.
Why existing code does not prevent this
addEndpointNavigationTitleOverrides is the only consumer of the converted apiDefinition in this code path, and getApiNavigationTitleOverrides (translatedApiOverrides.ts:403-412) walks rootPackage.endpoints plus subpackages[*].endpoints. There is no fallback/default path that would translate webhook/websocket titles via a different mechanism — the entire translated-API feature is introduced by this PR, so there is nothing else covering it.
Step-by-step proof
Given a docs project with a translated AsyncAPI workspace at translations/zh/apis/Plant Store API/ containing one HTTP endpoint and one WebSocket channel (both with displayName set in the translated spec):
registerTranslatedApiOverridesenters its loop body for localezh, APIPlant Store API.loadTranslatedApiWorkspacereturnsirwith bothservices(HTTP) andwebsocketChannelspopulated from the translated spec.addTranslatedApiNavigationTitleOverridescallsconvertIrToFdrApi(...). The resultingapiDefinition.rootPackagehasendpoints: [{ name: "列出植物", ... }]andwebsockets: [{ name: "实时植物推送", ... }].getApiNavigationTitleOverridescallsaddEndpointNavigationTitleOverrides(endpointTitlesById, rootPackage, "__package__"). The function iteratespkg.endpointsonly —pkg.websocketsis never touched.endpointTitlesByIdends up with one entry:{ "endpoint_listPlants" -> "列出植物" }.registerApiDefinitionreturns a translatedapiDefinitionId, e.g.api-translated-zh.- Later, in
publishDocs.ts, the per-locale code path runsapplyTranslatedApiNavigationTitlesInObject(updatedRoot, overridesForZh)followed byreplaceApiDefinitionIdsInObject(...). - The walker descends into the nav tree. For the HTTP endpoint node
{ type: "endpoint", apiDefinitionId: "api-base", endpointId: "endpoint_listPlants", title: "List plants" }, therecord.type === "endpoint"branch fires, replacestitlewith"列出植物". ✓ - For the WebSocket node
{ type: "webSocket", apiDefinitionId: "api-base", webSocketId: "ws_realtimePlants", title: "Realtime plant updates" }, no branch matches. The title remains"Realtime plant updates"while the sibling HTTP endpoint shows"列出植物". ✗ - After
replaceApiDefinitionIdsInObject, the WebSocket page in/zh/api-reference/...references the translatedapiDefinitionId(so the body content is localized via the new translatedapiDefinition), but its sidebar entry and H1 stay in English. The same applies totype: "webhook"nodes for OpenAPI 3.1 specs that include translated webhookdisplayNamevalues.
The user-visible result is an inconsistent sidebar with mixed-language entries in the same package: HTTP endpoints in the localized language, WebSockets and webhooks in the base language.
Impact
Scope: any docs site that ships translations and includes either an AsyncAPI/WebSocket spec or OpenAPI 3.1 webhooks. OpenAPI 3.1 webhooks are common; AsyncAPI is less so. The failure is silent — no warning is emitted, no test fails (there is no fixture that exercises a translated WebSocket or webhook today). It is a correctness gap in the feature this PR introduces, not a regression in existing behavior, but it directly contradicts the spirit of the feature ("zh endpoint H1s/sidebar labels come from zh OpenAPI summaries" — that should hold for webhook and websocket H1s too, since their titles come from the same displayName field).
How to fix
Two small, symmetric extensions:
- In
addEndpointNavigationTitleOverrides, also walkpkg.websocketsandpkg.webhooks. Use the corresponding write-side ID off each entry (webSocketIdfor websockets,webhookIdfor webhooks) so the keys match what the nav tree uses. - In
applyTranslatedApiNavigationTitlesInObject, addrecord.type === "webSocket"andrecord.type === "webhook"branches that readrecord.webSocketId/record.webhookId(instead ofrecord.endpointId) and look the title up in the same map (or a kind-keyed sub-map if you prefer to avoid ID collisions across kinds).
The harvest map can keep a single namespace because the nav-tree keys (webSocketId / webhookId / endpoint nav id) are distinct, but a per-kind sub-map is cleaner and avoids any cross-kind collision risk for users with synthetic IDs.
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 |
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
| function addEndpointNavigationTitleOverrides( | ||
| endpointTitlesById: Map<string, string>, | ||
| pkg: ReturnType<typeof convertIrToFdrApi>["rootPackage"], | ||
| subpackageId: string | ||
| ): void { | ||
| for (const endpoint of pkg.endpoints) { | ||
| if (endpoint.name != null) { | ||
| endpointTitlesById.set(getEndpointNavigationId(endpoint, subpackageId), endpoint.name); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 The harvest pass at addEndpointNavigationTitleOverrides (translatedApiOverrides.ts:417-427) gates on if (endpoint.name != null), but the FDR converter at convertPackage.ts:211 synthesizes name = irEndpoint.displayName ?? startCase(getOriginalName(irEndpoint.name)) — so endpoint.name is always non-null and the guard is a no-op. When a translated OpenAPI omits summary for an endpoint (i.e., displayName is undefined), the harvested title becomes a startCase'd operationId like "Add Plant" and silently overrides the base spec's richer title "Add a new plant to the inventory" on the locale's H1 and sidebar — directly contradicting the PR's stated goal that zh titles come from zh summaries (it should be a no-op when no zh summary exists, not a downgrade). Fix by sourcing titles directly from HttpEndpoint.displayName in the IR (which IS undefined when no summary), or by skipping when the harvested name equals startCase(getOriginalName(endpoint.id)).
Extended reasoning...
What the bug is
addEndpointNavigationTitleOverrides (translatedApiOverrides.ts:417-427) is the harvest pass that builds endpointTitlesById from the converted FDR apiDefinition for the translated workspace:
for (const endpoint of pkg.endpoints) {
if (endpoint.name != null) {
endpointTitlesById.set(getEndpointNavigationId(endpoint, subpackageId), endpoint.name);
}
}The endpoint.name != null guard is always true. The FDR converter at packages/cli/register/src/ir-to-fdr-converter/convertPackage.ts:211 synthesizes:
name: irEndpoint.displayName ?? startCase(getOriginalName(irEndpoint.name)),getOriginalName always returns a non-null string, so name is unconditionally non-null. When the translated OpenAPI does not define a summary for an endpoint, irEndpoint.displayName is undefined (per buildEndpoint.ts:138-140, display-name is only set when endpoint.summary != null), and name falls back to a startCase'd operationId like "Add Plant".
That synthesized fallback then gets stored in endpointTitlesById and applied via applyTranslatedApiNavigationTitlesInObject (translatedApiOverrides.ts:326-339), which overwrites the locale's nav title with "Add Plant" — worse than the rich base "Add a new plant to the inventory" that came from the base OpenAPI summary.
Why existing safeguards do not prevent this
endpoint.namecarries no signal distinguishing a realdisplayNamefrom a synthesized startCase fallback, so the!= nullguard cannot detect this case.- The new
warnIfTranslatedApiIsMissingEndpoints(translatedApiOverrides.ts:436-461) only fires when an endpoint is absent from the translated spec (METHOD + PATH set diff). It does not detect endpoints that are present but missing a translated summary.
Step-by-step proof
Given:
- Base OpenAPI for
POST /plantshassummary: "Add a new plant to the inventory"andoperationId: addPlant. - Translated
translations/zh/apis/<api>/openapi.jsonincludes the same path/operationId but omitssummary(e.g. team has not translated it yet).
Trace:
- Base publish:
apiDefinition.rootPackage.endpoints[i].name = "Add a new plant to the inventory". The base nav node for that endpoint getstitle: "Add a new plant to the inventory". registerTranslatedApiOverridesenters its loop body forzh.loadTranslatedApiWorkspacereturns the IR;irEndpoint.displayNameisundefinedforaddPlantbecause the translated spec has no summary.addTranslatedApiNavigationTitleOverridescallsconvertIrToFdrApi(...). In the result,endpoint.nameforaddPlantisstartCase(getOriginalName("endpoint_.addPlant"))≈"Add Plant".addEndpointNavigationTitleOverrides:endpoint.name != nullis true, soendpointTitlesById.set(<navId>, "Add Plant").- Per-locale apply:
applyTranslatedApiNavigationTitlesInObjectfinds the matchingtype === "endpoint"node and replacestitlewith"Add Plant". zhH1 and sidebar entry render as"Add Plant"instead of the base"Add a new plant to the inventory"— strictly worse than the unmodified base, with no warning.
Impact
Silent regression scoped to the new translated-API feature: any endpoint present in a translated spec but lacking a summary silently downgrades the H1/sidebar title for that locale to a synthesized identifier. This directly contradicts the PR description ("zh endpoint H1s/sidebar labels come from zh OpenAPI summaries") — when no zh summary exists, the override should be a no-op, not a downgrade. Partial-translation workflows (a common adoption pattern, since translation is incremental) are most affected.
How to fix
Either:
- Source titles from the IR directly. Walk
ir.services[*].endpoints[*]readingHttpEndpoint.displayName(which is undefined when no summary), and only insert intoendpointTitlesByIdwhen it is non-null. This naturally skips the override and avoids the secondconvertIrToFdrApicall entirely. - Detect the synthesized fallback. Compare each harvested name against
startCase(getOriginalName(endpoint.id))(or the equivalent identifier for that endpoint) and skip when they match. This is more brittle (relies on an exact string match against a derivable value) but local to the harvest function.
Option 1 is cleaner and also addresses the duplicate-conversion concern raised earlier in this review.
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| async function loadTranslatedApiWorkspace({ | ||
| docsWorkspace, |
There was a problem hiding this comment.
🔴 A parse error in any single locale's translated API spec calls context.failAndThrow from loadTranslatedApiWorkspace (translatedApiOverrides.ts:202-206), which aborts the entire publish even though finishDocsRegister at publishDocs.ts:662 has already committed the base docs. As a result, every other locale's translated API and translated pages fail to register — even valid ones — because registerTranslatedApiOverrides runs before the per-locale translation-pages loop at publishDocs.ts:695, which is the only loop that wraps each locale in try/catch. Wrap the inner load+register call in try/catch with a logger.warn and continue, mirroring the toFernWorkspace try/catch 30 lines below in the same function and the per-locale page-registration loop.
Extended reasoning...
What is wrong
registerTranslatedApiOverrides (publishDocs.ts:684) runs after finishDocsRegister has already committed the base docs to FDR (publishDocs.ts:662) and before the translation-pages loop at publishDocs.ts:695-902. Inside the API loop, loadTranslatedApiWorkspace calls context.failAndThrow on !loadedWorkspace.didSucceed:
// translatedApiOverrides.ts:202-206
if (!loadedWorkspace.didSucceed) {
handleFailedWorkspaceParserResult(loadedWorkspace, context.logger);
return context.failAndThrow(`Failed to load translated API "${apiName}" for locale "${locale}".`, ...);
}The throw propagates up through registerTranslatedApiOverrides and out of the unprotected await at publishDocs.ts:684, taking the rest of the publish flow with it.
Step-by-step proof
Setup: docs project with translations: [en (default), zh, ja], base API "Plant Store API" published successfully, and a syntax error in translations/zh/apis/Plant Store API/openapi.json.
finishDocsRegistersucceeds at publishDocs.ts:662 → base docs are now live on FDR.registerTranslatedApiOverridesenters itsfor (const locale of locales)loop.zhis processed first (insertion order fromgetNonDefaultTranslationLocales).loadTranslatedApiWorkspacecallsloadAPIWorkspace({ ..., lenient: true }). The malformed JSON yieldsdidSucceed: false.handleFailedWorkspaceParserResultlogs the parse error, thenfailAndThrowthrows aConfigError.- The exception propagates out of
registerTranslatedApiOverrides→ out of theawaitat publishDocs.ts:684 → to the outer try/catch which lets it bubble out (see publishDocs.ts:doUnlock; the only catch on this path is for unlocking). - The translation-pages registration loop at publishDocs.ts:695 never runs.
ja's translated API andja's translated pages are never registered.zh's translated pages are also never registered, even thoughtranslations/zh/pages/*.mdxare perfectly valid. - Net effect: site is half-published — base docs live, all locale-specific pages and translated APIs missing — until the user fixes the one bad spec.
Why existing code does not prevent it
The translation-pages loop at publishDocs.ts:695-902 wraps each locale in try { ... } catch { context.logger.warn(Failed to register translations for locale "${locale}"...) }, so a parse failure in one locale's pages never blocks others. The translated-API loop is positioned before that loop and is itself unprotected, so it short-circuits the per-locale resilience.
The same function also already uses graceful degradation 30 lines below: when OSSWorkspace.toFernWorkspace throws (translatedApiOverrides.ts:233-247), it logs to context.logger.debug and continues with fernWorkspace = undefined (only dynamic snippets are skipped). So the file is internally inconsistent: malformed top-level workspace → fatal abort, malformed Fern conversion → silent debug.
Addressing the refutation
- "Consistency with base API handling." Base API failure aborting publish makes sense — without it there are no docs. Translated APIs are supplementary; the PR's own translation-pages loop and
toFernWorkspacecatch already accept this asymmetry. Treating one optional layer as fatal while the surrounding optional layers degrade gracefully is the actual inconsistency. - "Silent fallback is worse UX." The proposed fix is not silent —
handleFailedWorkspaceParserResultalready logs the parse errors at warn/error level, and alogger.warn(Skipping translated API ... due to parse errors)ensures CI logs surface the issue.logger.warnis exactly what the per-locale page registration uses on failure, and that is not considered "silent" there. - "loadTranslationPages also fails fast." Only on a missing directory; missing/malformed individual files emit warnings. The malformed-spec-inside-present-dir case maps to the per-file warn case, not the missing-dir abort case.
- "No actionable refactor." Wrapping the body of the inner
for (const [apiName, baseApiDefinitionId] of registeredApiIdsByName.entries())loop (translatedApiOverrides.ts:78-113) in try/catch withcontinueis one localized change. The base API definition is still wired up for that locale's docs nav viareplaceApiDefinitionIdsInObject, which only swaps IDs that are present intranslatedApiDefinitionIds; missing entries leave the base ID in place.
Impact
Any user of the new translated-API feature with multiple locales: a typo in one locale's spec disables translated docs for all locales until fixed. The base docs are already on FDR by the time the throw fires, so the site is in a mixed state. Severity is normal — clear error, partial-state surprise, blocks unrelated locales.
1 participant
Summary
Adds CLI support for translated OpenAPI/API specs in docs translations.
translations/<locale>.Context
Docs translations can override page content today, but API reference pages still pointed at the default API definition. This change lets users provide a locale-specific OpenAPI/API spec so API summaries, endpoint titles, descriptions, request/response descriptions, and schemas can be localized too.
Validation
pnpm --filter @fern-api/remote-workspace-runner compilepnpm exec vitest run src/__test__/translatedApiOverrides.test.ts src/__test__/publishDocsTranslatedOpenApi.e2e.test.tspnpm fern-dev:buildFERN_NO_VERSION_REDIRECTION=true node --enable-source-maps packages/cli/cli/dist/dev/cli.cjs generate --docs --log-level debug --no-prompt.添加一株新的植物,更新现有植物,按 ID 查找植物) and translated body text fromtranslations/zh/openapi.json.