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:
|
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…WireValue() Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| @@ -1,3 +1,4 @@ | |||
| import { getWireValue } from "@fern-api/base-generator"; | |||
There was a problem hiding this comment.
Import inconsistency detected. This file imports getWireValue from @fern-api/base-generator, but all other files in the dynamic-snippets module import from @fern-api/browser-compatible-base-generator (see EndpointSnippetGenerator.ts, DynamicSnippetsGeneratorContext.ts, and DynamicTypeLiteralMapper.ts). This will cause a runtime import error if getWireValue is not exported from @fern-api/base-generator or if the browser-compatible variant is required.
Fix:
import { getWireValue } from "@fern-api/browser-compatible-base-generator";| import { getWireValue } from "@fern-api/base-generator"; | |
| import { getWireValue } from "@fern-api/browser-compatible-base-generator"; |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
…onflict Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
| @@ -1,3 +1,4 @@ | |||
| import { getWireValue } from "@fern-api/base-generator"; | |||
| import { assertNever } from "@fern-api/core-utils"; | |||
There was a problem hiding this comment.
🟡 FilePropertyMapper imports getWireValue from non-browser-compatible package
In FilePropertyMapper.ts, getWireValue is imported from @fern-api/base-generator, while all other files in the same dynamic-snippets package (DynamicTypeLiteralMapper.ts, EndpointSnippetGenerator.ts, DynamicSnippetsGeneratorContext.ts) import from @fern-api/browser-compatible-base-generator. The dynamic-snippets package is designed to be browser-compatible, and importing from @fern-api/base-generator could introduce Node.js-specific dependencies into the browser bundle, potentially breaking browser builds or bloating bundle size.
| import { assertNever } from "@fern-api/core-utils"; | |
| import { getWireValue } from "@fern-api/browser-compatible-base-generator"; |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged — however @fern-api/browser-compatible-base-generator doesn't currently export getWireValue. The dynamic-snippets package.json lists @fern-api/base-generator as a dependency, so the import works. If browser compatibility is a concern, getWireValue would need to be re-exported from the browser-compatible package first.
…e compressed names Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…cannot deserialize v66 compressed names Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…on-ir-v66-upgrade
…h Python IR SDK Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…on-ir-v66-upgrade Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…on-ir-v66-upgrade
…on-ir-v66-upgrade Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against 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 |
…on-ir-v66-upgrade Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
SDK Generation Benchmark ResultsComparing PR branch against 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 |
Description
Implements full IR v66 support for the Python SDK, Pydantic, and FastAPI generators. IR v66 compresses the
Nametype into aNameOrStringunion (plain string or fullNameobject), reducing IR wire size and improving performance.This PR adds
CaseConverterandgetWireValue/getOriginalNamehelpers throughout the Python generators so they can transparently handle both compressed (string) and full (Name/NameAndWireValue) forms.Changes Made
Generator code updates
AbstractPythonGeneratorContext: AddedCaseConverterinstance; replaced direct.snakeCase.unsafeName,.pascalCase.safeName,.snakeCase.safeNameaccesses withcaseConverter.snakeUnsafe(),caseConverter.pascalSafe(),caseConverter.snakeSafe()DynamicSnippetsGeneratorContext: Same CaseConverter pattern forgetClassName,getPropertyName,getMethodName,getEnvironmentEnumName, andfernFilepath.allPartsmappingDynamicTypeLiteralMapper: Replaced.wireValuedirect accesses withgetWireValue()for discriminant values, property names, and enum valuesEndpointSnippetGenerator: Replaced.wireValuewithgetWireValue()for auth parameter and header lookupsFilePropertyMapper: AddedgetWireValue()for file property wire value fallbacksReadmeSnippetBuilder: CaseConverter for environment names, auth scheme names, service paths, endpoint names; null-safety guards for environment name accessbuildReference.ts: CaseConverter for endpoint names, service paths, parameter names, type names, section titles; null-safety guards for environment namesWireTestGenerator: CaseConverter for service/endpoint names,getWireValue()for query params, headers, path params, and file upload keys;getOriginalName()for path parameter namesWireTestSetupGenerator: CaseConverter for base URL names, auth scheme names, global header names;getWireValue()for query parameter datetime detectionEnumGenerator: CaseConverter for enum member names (screamingSnakeSafe,snakeSafe);getWireValue()for enum wire valuesObjectGenerator:getWireValue()for property wire value comparison and alias detectionWrappedAliasGenerator: CaseConverter for named type visitor method names (snakeUnsafe)Version & config updates
versions.ymlfor SDK (5.3.0-rc.0), Pydantic (1.12.0-rc.0), FastAPI (2.2.0-rc.0) withirVersion: 66migrateFromV66ToV65.tsto register all three generatorsseed.ymlirVersiontov66forpython-sdk,pydantic,pydantic-v2, andfastapiTest fix
test_ir_deserialization.py: Pinned--version v65when generating IR via the CLI, because the published CLI now produces v66 IR with compressed names that the Python IR SDK (fern_fern_ir_v65Pydantic models) cannot deserialize. This is a targeted fix — when a v66 Python IR SDK is published, this version pin should be updated to match.Testing
Human Review Checklist
test_ir_deserialization.pyversion pin: The test now passes--version v65to generate IR the Python SDK can parse. This is correct for now but creates a maintenance item — whenfern_fern_ir_v66is published, this pin and the pyproject.toml dependency should both be updated.CaseConverterinitialized withkeywords: undefinedin multiple files — confirm this is intentional for Python generators (the TypeScript reference impl also passesundefinedfor keywords in some cases)ReadmeSnippetBuilderandbuildReference: Environment name access now usesif (defaultEnv?.name != null)guard before callingcaseConverter.screamingSnakeUnsafe()— this changes prior behavior wheredefaultEnv?.name.screamingSnakeCase.unsafeNamewould have thrown on a compressed string. Verify the guard logic is correct.caseConverterinstances with identical config across files — not a bug, but could be centralized if desiredLink to Devin session: https://app.devin.ai/sessions/1947204928ba4bfe9b663dfe828ac3d7
Requested by: @Swimburger