-
Notifications
You must be signed in to change notification settings - Fork 336
feat(python): generate invocation-only dynamic snippets #17402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devin/1786644600-invocation-only-dynamic-snippets
Are you sure you want to change the base?
Changes from all commits
cad1246
1355d36
15ea033
3051f49
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import { FernIr } from "@fern-api/dynamic-ir-sdk"; | ||
|
|
||
| /** | ||
| * The structured result of an invocation-only snippet. | ||
| * | ||
| * Unlike {@link FernIr.dynamic.EndpointSnippetResponse}, which returns a single fully-formed | ||
| * snippet string, this exposes the individual pieces a docs template needs to render (and | ||
| * keep in sync) an invocation on its own: the bare call, the imports the call requires, and | ||
| * the generated client class/type name. | ||
| */ | ||
| export interface InvocationSnippetResponse { | ||
| /** | ||
| * The bare invocation/call (e.g. `client.plants.update(...)`) with no imports, no client | ||
| * instantiation, and no trailing statement terminator. Honors `options.clientVariableName`. | ||
| */ | ||
| snippet: string; | ||
| /** | ||
| * The import block the call requires (e.g. an SDK namespace import for a branded string | ||
| * alias). Empty string when the call references no imports. | ||
| */ | ||
| imports: string; | ||
| /** | ||
| * The generated client class/type name (e.g. `AcmeClient`), so docs can render | ||
| * `new {{clientName}}(...)` and track renames of the SDK client. | ||
| */ | ||
| clientName: string; | ||
| errors: FernIr.dynamic.Error_[] | undefined; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ import { Class } from "./Class.js"; | |
| import { ClassInstantiation } from "./ClassInstantiation.js"; | ||
| import { CodeBlock } from "./CodeBlock.js"; | ||
| import { Comment } from "./Comment.js"; | ||
| import { AstNode } from "./core/AstNode.js"; | ||
| import { ModulePath } from "./core/types.js"; | ||
| import { Decorator } from "./Decorator.js"; | ||
| import { Field } from "./Field.js"; | ||
| import { Lambda } from "./Lambda.js"; | ||
|
|
@@ -115,3 +117,26 @@ export function methodArgument(args: MethodArgument.Args): MethodArgument { | |
| export function operator(args: Operator.Args): Operator { | ||
| return new Operator(args); | ||
| } | ||
|
|
||
| /** | ||
| * Renders a node separately from the imports it references, the Python analogue of the | ||
| * TypeScript AST's `toStringWithoutImports`. `code` is the node's body with no import lines, | ||
| * and `imports` is the rendered import block the body would otherwise need (empty string when | ||
| * none). This lets callers embed an invocation inside code they already own (e.g. a | ||
| * documentation code template) while surfacing the imports the call requires. | ||
| * | ||
| * `modulePath` is the module the code is imagined to live in; imports are relativized against it | ||
| * exactly as they would be in a generated file at that path, so the imports match what the full | ||
| * snippet would emit. | ||
| */ | ||
| export function renderNodeWithoutImports({ node, modulePath }: { node: AstNode; modulePath: ModulePath }): { | ||
| code: string; | ||
| imports: string; | ||
| } { | ||
| // A bare node's `toString()` only writes its own body — imports are emitted solely by | ||
| // PythonFile — so the body already excludes them. Wrapping the node in a file at the same | ||
| // path lets us compute just the import block the body references. | ||
| const code = node.toString(); | ||
| const file = new PythonFile({ path: modulePath, statements: [node] }); | ||
| return { code, imports: file.getImports() }; | ||
|
Comment on lines
+139
to
+141
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 warning
Suggest doing both in one pass, e.g. add a
Comment on lines
+139
to
+141
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Invocation-only snippets can name a type differently than the import line that provides it The call text is produced ( Name-override map is applied only when rendering the import block, not the body
Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,10 @@ | ||
| import { AbstractAstNode, Scope, Severity } from "@fern-api/browser-compatible-base-generator"; | ||
| import { | ||
| AbstractAstNode, | ||
| InvocationSnippetResponse, | ||
| Options, | ||
| Scope, | ||
| Severity | ||
| } from "@fern-api/browser-compatible-base-generator"; | ||
| import { assertNever } from "@fern-api/core-utils"; | ||
| import { FernIr } from "@fern-api/dynamic-ir-sdk"; | ||
| import { python } from "@fern-api/python-ast"; | ||
|
|
@@ -68,6 +74,43 @@ export class EndpointSnippetGenerator { | |
| return this.callMethod({ endpoint, snippet: request }); | ||
| } | ||
|
|
||
| /** | ||
| * Generates the structured pieces of an endpoint invocation for callers that render the | ||
| * invocation within code of their own (e.g. a documentation code template): the bare call | ||
| * (e.g. `client.plants.update(...)`), the imports the call requires, and the generated root | ||
| * client class name. | ||
| */ | ||
| public generateInvocationSnippetSync({ | ||
| endpoint, | ||
| request, | ||
| options | ||
| }: { | ||
| endpoint: FernIr.dynamic.Endpoint; | ||
| request: FernIr.dynamic.EndpointSnippetRequest; | ||
| options?: Options; | ||
| }): InvocationSnippetResponse { | ||
| const invocation = this.callMethod({ | ||
| endpoint, | ||
| snippet: request, | ||
| clientVariableName: options?.clientVariableName | ||
| }); | ||
| // The caller supplies the client and terminates the statement themselves, so the | ||
| // invocation is emitted as a bare expression. When the call references SDK types (e.g. | ||
| // an enum or an aliased request value) the imports it needs are surfaced separately so | ||
| // the caller can render them rather than falling back to the complete snippet. Python | ||
| // statements have no terminator, so no trailing character needs stripping. | ||
| const { code, imports } = python.renderNodeWithoutImports({ | ||
| node: invocation, | ||
| modulePath: SNIPPET_MODULE_PATH | ||
| }); | ||
| return { | ||
| snippet: code.trim(), | ||
| imports, | ||
| clientName: this.context.getRootClientClassName(), | ||
| errors: this.context.errors.empty() ? undefined : this.context.errors.toDynamicSnippetErrors() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 suggestion
|
||
| }; | ||
| } | ||
|
|
||
| private buildPythonFile({ | ||
| endpoint, | ||
| snippet | ||
|
|
@@ -443,13 +486,15 @@ export class EndpointSnippetGenerator { | |
|
|
||
| private callMethod({ | ||
| endpoint, | ||
| snippet | ||
| snippet, | ||
| clientVariableName | ||
| }: { | ||
| endpoint: FernIr.dynamic.Endpoint; | ||
| snippet: FernIr.dynamic.EndpointSnippetRequest; | ||
| clientVariableName?: string; | ||
| }): python.AstNode { | ||
| return python.invokeMethod({ | ||
| on: python.reference({ name: CLIENT_VAR_NAME }), | ||
| on: python.reference({ name: clientVariableName ?? CLIENT_VAR_NAME }), | ||
| method: this.getMethod({ endpoint }), | ||
| arguments_: this.getMethodArgs({ endpoint, snippet }) | ||
| .filter((arg) => !python.TypeInstantiation.isNop(arg.value)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,100 @@ | ||||||
| import { AbsoluteFilePath, join } from "@fern-api/path-utils"; | ||||||
|
|
||||||
| import { buildDynamicSnippetsGenerator } from "./utils/buildDynamicSnippetsGenerator.js"; | ||||||
| import { buildGeneratorConfig } from "./utils/buildGeneratorConfig.js"; | ||||||
|
|
||||||
| const DYNAMIC_IR_TEST_DEFINITIONS_DIRECTORY = AbsoluteFilePath.of( | ||||||
| `${__dirname}/../../../../../packages/cli/generation/ir-generator-tests/src/dynamic-snippets/__test__/test-definitions` | ||||||
| ); | ||||||
|
|
||||||
| // invocation-only snippets are rendered inside code the caller already owns (e.g. a | ||||||
| // documentation code template), so they must not include imports or client instantiation | ||||||
| describe("invocation-only snippets", () => { | ||||||
| const generator = buildDynamicSnippetsGenerator({ | ||||||
| irFilepath: AbsoluteFilePath.of(join(DYNAMIC_IR_TEST_DEFINITIONS_DIRECTORY, "exhaustive.json")), | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 suggestion
Suggested change
(adjust the import if you take this — otherwise just drop the |
||||||
| config: buildGeneratorConfig({}) | ||||||
| }); | ||||||
|
|
||||||
| const request = { | ||||||
| endpoint: { | ||||||
| method: "GET" as const, | ||||||
| path: "/http-methods/{id}" | ||||||
| }, | ||||||
| baseURL: undefined, | ||||||
| environment: undefined, | ||||||
| auth: { | ||||||
| type: "bearer" as const, | ||||||
| token: "<YOUR_API_KEY>" | ||||||
| }, | ||||||
| pathParameters: { | ||||||
| id: "id" | ||||||
| }, | ||||||
| queryParameters: undefined, | ||||||
| headers: undefined, | ||||||
| requestBody: undefined | ||||||
| }; | ||||||
|
|
||||||
| it("generates the invocation without imports or client instantiation", () => { | ||||||
| const response = generator.generateInvocationSync(request); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 warning The tests call |
||||||
|
|
||||||
| expect(response?.snippet).toBe('client.endpoints.http_methods.test_get(\n id="id",\n)'); | ||||||
| expect(response?.imports).toBe(""); | ||||||
| expect(response?.errors).toBeUndefined(); | ||||||
| }); | ||||||
|
|
||||||
| it("exposes the generated client class name so docs can render the client instantiation", () => { | ||||||
| const response = generator.generateInvocationSync(request); | ||||||
|
|
||||||
| expect(response?.clientName).toBe("Acme"); | ||||||
| }); | ||||||
|
|
||||||
| it("invokes the endpoint on the requested client variable", () => { | ||||||
| const response = generator.generateInvocationSync(request, { clientVariableName: "mailchimp" }); | ||||||
|
|
||||||
| expect(response?.snippet).toBe('mailchimp.endpoints.http_methods.test_get(\n id="id",\n)'); | ||||||
| }); | ||||||
|
|
||||||
| it("returns the imports the invocation references instead of falling back to the full snippet", () => { | ||||||
| // This body references stdlib types the call constructs inline (a datetime and a UUID), | ||||||
| // so the invocation must carry `import datetime` / `import uuid`. The previous | ||||||
| // invocation-only contract had no way to express this; now the imports are surfaced | ||||||
| // separately so docs can regenerate both the call and the imports it needs. | ||||||
| const response = generator.generateInvocationSync({ | ||||||
| endpoint: { | ||||||
| method: "POST" as const, | ||||||
| path: "/object/get-and-return-with-optional-field" | ||||||
| }, | ||||||
| baseURL: undefined, | ||||||
| environment: undefined, | ||||||
| auth: { | ||||||
| type: "bearer" as const, | ||||||
| token: "<YOUR_API_KEY>" | ||||||
| }, | ||||||
| pathParameters: undefined, | ||||||
| queryParameters: undefined, | ||||||
| headers: undefined, | ||||||
| requestBody: { | ||||||
| string: "string", | ||||||
| integer: 1, | ||||||
| long: 1000000, | ||||||
| double: 1.1, | ||||||
| bool: true, | ||||||
| datetime: "2024-01-15T09:30:00Z", | ||||||
| date: "2023-01-15", | ||||||
| uuid: "d5e9c84f-c2b2-4bf4-b4b0-7ffd7a9ffc32", | ||||||
| base64: "SGVsbG8gd29ybGQh", | ||||||
| list: ["list", "list"], | ||||||
| set: ["set"], | ||||||
| map: { 1: "map" }, | ||||||
| bigint: "1000000" | ||||||
| } | ||||||
| }); | ||||||
|
|
||||||
| expect(response).not.toBeUndefined(); | ||||||
| expect(response?.snippet).toContain("datetime.datetime.fromisoformat"); | ||||||
| expect(response?.snippet).toContain("uuid.UUID"); | ||||||
| expect(response?.imports).toContain("import datetime"); | ||||||
| expect(response?.imports).toContain("import uuid"); | ||||||
| expect(response?.errors).toBeUndefined(); | ||||||
| }); | ||||||
| }); | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 suggestion
This is a copy of the first half of
write(). If the dedupe/override/import logic ever changes in one place it'll drift in the other. Consider havingwrite()call a shared private helper (orgetImports()) so there's a single source of truth.