feat(python): generate invocation-only dynamic snippets - #17402
Conversation
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>
…snippets The invocation-only dynamic-snippets hook now returns a structured InvocationSnippetResponse (snippet + imports + clientName + errors) instead of a bare call string. Callers such as documentation code templates can now regenerate the imports and client instantiation alongside the call and keep them in sync across SDK renames. The branded-string-alias case, which previously returned undefined (dropping the call because it referenced an SDK import the caller couldn't supply), now returns the call plus the required import block. generateInvocationSync still returns undefined only for the "generator doesn't implement the hook" capability check the multi-language fan-out relies on. Co-Authored-By: Claude <noreply@anthropic.com>
Mirrors the finalized TypeScript structured contract: alongside the full
snippet, the generator now returns InvocationSnippetResponse = { snippet,
imports, clientName, errors } for callers (e.g. docs code templates) that
render the invocation inside code they already own.
- snippet: the bare call (honoring options.clientVariableName), no client
construction, no imports, no terminator.
- imports: the Python import block the call references, captured separately
via a new python.renderNodeWithoutImports helper + PythonFile.getImports()
(the Python analogue of the TS AST's toStringWithoutImports). Empty string
when the call needs no imports.
- clientName: the generated root client class name (getRootClientClassName).
- errors: preserved from the existing error reporter.
Co-Authored-By: Claude <noreply@anthropic.com>
![nitpickybot[bot]](https://avatars.githubusercontent.com/in/2901896?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review Summary
Adds a structured invocation-only snippet path for the Python generator: PythonFile.getImports(), a python.renderNodeWithoutImports helper, a clientVariableName-parameterized callMethod, and generateInvocationSnippetSync. The main concern is that the body and the import block are rendered with two different writers, so import name-overrides (aliasing on collision) can desync from the rendered code. Also worth confirming the DynamicSnippetsGenerator wiring that the new tests exercise.
- 🟡 2 warning(s)
- 🔵 3 suggestion(s)
| const code = node.toString(); | ||
| const file = new PythonFile({ path: modulePath, statements: [node] }); | ||
| return { code, imports: file.getImports() }; |
There was a problem hiding this comment.
🟡 warning
code is rendered with a fresh writer that has no ref-name overrides, while imports is rendered inside PythonFile.getImports() after updateWriterRefNameOverrides runs. In PythonFile.write both the imports and the statements are written under the same overrides, so when two references collide the import block emits an alias (import x as x_1) and the body uses it consistently. Here they're independent: the import block can be aliased while node.toString() still writes the original name — silently broken output.
Suggest doing both in one pass, e.g. add a PythonFile.renderImportsAndBody(): { imports, body } that sets the overrides once, writes imports, writes statements, then unsets — and have renderNodeWithoutImports delegate to it instead of calling node.toString() separately.
| public getImports(): string { | ||
| const writer = new Writer(); | ||
| const uniqueReferences = this.deduplicateReferences(); | ||
| this.updateWriterRefNameOverrides({ writer, uniqueReferences }); | ||
| this.writeImports({ writer, uniqueReferences }); | ||
| writer.unsetRefNameOverrides(); | ||
| return writer.toString().trimEnd(); | ||
| } |
There was a problem hiding this comment.
🔵 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 having write() call a shared private helper (or getImports()) so there's a single source of truth.
| }; | ||
|
|
||
| it("generates the invocation without imports or client instantiation", () => { | ||
| const response = generator.generateInvocationSync(request); |
There was a problem hiding this comment.
🟡 warning
The tests call generator.generateInvocationSync(...), but the only new public method in the diff is EndpointSnippetGenerator.generateInvocationSnippetSync. No change to DynamicSnippetsGenerator is included — is that wiring inherited from the base class added in #17393, or is it missing from this PR? Worth confirming, otherwise these tests can't be green on main.
| 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.
🔵 suggestion
this.context.errors is shared state on the generator context; if the caller reuses the same context across endpoints this will report errors accumulated from previous calls. Check that the surrounding generateInvocationSync wrapper scopes/clears errors the same way generateSnippetSync does.
| // 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.
🔵 suggestion
join() already returns an AbsoluteFilePath when given one; the extra AbsoluteFilePath.of(...) wrapper is redundant.
| irFilepath: AbsoluteFilePath.of(join(DYNAMIC_IR_TEST_DEFINITIONS_DIRECTORY, "exhaustive.json")), | |
| irFilepath: join(DYNAMIC_IR_TEST_DEFINITIONS_DIRECTORY, RelativeFilePath.of("exhaustive.json")), |
(adjust the import if you take this — otherwise just drop the AbsoluteFilePath.of wrapper.)
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const code = node.toString(); | ||
| const file = new PythonFile({ path: modulePath, statements: [node] }); | ||
| return { code, imports: file.getImports() }; |
There was a problem hiding this comment.
🟡 Invocation-only snippets can name a type differently than the import line that provides it
The call text is produced (node.toString() at generators/python-v2/ast/src/python.ts:139) without the renaming rules that the accompanying import block is built with, so when two same-named types from different modules appear in one call the snippet uses a name the imports never define.
Impact: A documentation snippet can end up referring to a name that was renamed in its import line, producing code that fails to run.
Name-override map is applied only when rendering the import block, not the body
PythonFile.write (generators/python-v2/ast/src/PythonFile.ts:57-73) first computes deduplicated references and installs a name-override map on the writer (updateWriterRefNameOverrides, generators/python-v2/ast/src/PythonFile.ts:79-151), and only then writes both the imports and the statements with that same writer, so body and imports agree.
renderNodeWithoutImports instead renders the body with a fresh Writer that has no overrides (AstNode.toString, generators/python-v2/ast/src/core/AstNode.ts:31-35), while PythonFile.getImports() (generators/python-v2/ast/src/PythonFile.ts:82-89) installs overrides for the import block. When updateWriterRefNameOverrides hits a name collision (two references with the same name but different modulePath, e.g. Metadata from two subpackages of the SDK) it aliases the second one, so getImports() emits from seed.b import Metadata as BMetadata while the body written without overrides emits Metadata(...) for both references. The invocation returned by EndpointSnippetGenerator.generateInvocationSnippetSync (generators/python-v2/dynamic-snippets/src/EndpointSnippetGenerator.ts:108-114) then references an undefined/incorrect name.
Prompt for agents
In generators/python-v2/ast/src/python.ts, renderNodeWithoutImports renders the body via node.toString(), which uses a fresh Writer with no reference-name overrides, while the imports are rendered by PythonFile.getImports(), which does install overrides computed by PythonFile.updateWriterRefNameOverrides. When two references share a name but come from different module paths, the override logic aliases one of them in the import line (e.g. `import Metadata as BMetadata`) but the body still writes the un-aliased name, so the emitted invocation references a name the imports do not define.
Fix by rendering both pieces from the same writer/override state. A natural approach is to add a method on PythonFile (e.g. `getBodyAndImports()` or extend getImports to also return the statement body) that deduplicates references, installs the overrides once, writes the imports into one buffer and the statements into another (or writes both and splits), then unsets the overrides — and have renderNodeWithoutImports delegate to it instead of calling node.toString() separately.
Was this helpful? React with 👍 or 👎 to provide feedback.
741c4f6 to
c85a00c
Compare
1 participant
Stacks on #17393. Mirrors that PR's finalized structured contract for the Python generator.
What
EndpointSnippetGenerator.generateInvocationSnippetSyncnow returns the structuredInvocationSnippetResponse={ snippet, imports, clientName, errors }, so callers (e.g. a docs code template) can render an invocation inside code they already own and keep it in sync:client.endpoints.http_methods.test_get(id="id"), honoringoptions.clientVariableName(defaultclient). No client construction, no imports, no terminator (Python statements have none).importblock the call references (""when none).getRootClientClassName()), so docs can renderclient = {{clientName}}(...)and track renames.How imports are captured (the non-mechanical part)
Python imports are emitted only by
PythonFile(computed from thereferencescollected on AST nodes); a bare node'stoString()already excludes them. To surface them separately — the Python analogue of the TS AST'stoStringWithoutImports() -> { code, imports }— this PR adds:PythonFile.getImports()— renders just the import block for the file's statements (dedup, name-override, and relativization logic reused unchanged), trimming the trailing blank line.python.renderNodeWithoutImports({ node, modulePath })— returns{ code: node.toString(), imports: <PythonFile at modulePath>.getImports() }. Relativizing against the snippet module path keeps imports identical to what the full snippet would emit.callMethodwas parameterized withclientVariableName(mirroring the TScallMethod({ clientVariableName })).Before / after
snippet = client.endpoints.http_methods.test_get(id="id"),imports = "",clientName = "Acme".snippetcontainingdatetime.datetime.fromisoformat(...)/uuid.UUID(...)andimports = "import datetime\nimport uuid"— no fallback to the full snippet.Tests
src/__test__/InvocationSnippet.test.ts(all green):(a) bare call for a normal endpoint (exact string,
imports === "", no errors); (b)clientNamepopulated; (c) customclientVariableName; (d) import-referencing invocation asserting theimport datetime/import uuidblock. Existing dynamic-snippets snapshots and python-ast tests remain green.Verify
pnpm turbo run compile --filter @fern-api/python-dynamic-snippets— clean (includes@fern-api/python-ast).vitest runin python-dynamic-snippets (70 passed) and python-ast (156 passed).npx biome check --writeon changed.ts— clean.generators/python/sdk/changes/unreleased/invocation-only-dynamic-snippets.yml(type: feat).Auto-retargets to
mainonce #17393 merges. Do not merge before it.Generated with Claude Code