Skip to content

Commit c24366d

Browse files
maorlegerCopilot
andcommitted
fix(typespec-ts): emit PagedAsyncIterableIterator import once at source, remove dedup workaround
resolveReference(PagingHelpers.PagedAsyncIterableIterator) left the paging iterator type unpinned, so once the generated barrels re-exported the same name the emitter could pick it up through *index.js and then rely on src/codegen/pagingImports.ts to clean up the duplicate after the fact. This change pins PagedAsyncIterableIterator to static-helpers/pagingHelpers.js at the emission sites that write the public signatures (src/modular/helpers/operationHelpers.ts, src/codegen/operations.ts, src/codegen/classicalClient.ts, src/codegen/classicalOperations.ts) and makes the root barrel re-export point at that same concrete module (src/codegen/indexFiles.ts, src/modular/buildRootIndex.ts). With the source fixed, the post-hoc dedupe pass can be deleted entirely. Audit: .squad/decisions/inbox/ripley-symptom-audit.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6efd15c commit c24366d

11 files changed

Lines changed: 263 additions & 92 deletions

File tree

.squad/agents/lambert/history.md

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
# Project Context
2+
3+
- **Owner:** Maor Leger
4+
- **Project:** autorest.typescript — TypeSpec TS emitter refactor to align architecture with `typespec-rust` and `autorest.go` emitters.
5+
- **Primary reading list:**
6+
- `~/workspace/emitter-chain/typespec-rust` — TypeSpec Rust emitter (the closer cousin — same TypeSpec front-end)
7+
- `~/workspace/emitter-chain/autorest.go` — Go autorest generator
8+
- `/home/maorleger/workspace/emitter-chain/go-rust.md` — synthesized architecture doc comparing the two. START HERE.
9+
- **Target codebase:** `packages/typespec-ts/src/` — especially `codemodel/` (already follows a data/render split: types.ts, build-*.ts, render-*.ts), `modular/`, `rlc/`, `framework/`, `static-helpers/`.
10+
- **Stack note:** TS emitter is built on the TypeSpec compiler + TCGC (@azure-tools/typespec-client-generator-core) + ts-morph for code rendering. Whatever IR pattern exists in Rust/Go needs to fit into this pipeline.
11+
- **Maintenance-mode:** `packages/autorest.typescript/` — do not analyze for this refactor.
12+
- **Created:** 2026-05-15
13+
14+
## Learnings
15+
16+
<!-- Append new learnings below. Each entry is something lasting about the project. -->
17+
18+
### 2026-05-15 — TS Emitter vs Rust/Go Architecture Delta
19+
20+
**Pipeline shape confirmed:**
21+
- Entry: `src/index.ts` `$onEmit()` — 500+ lines, drives everything via nested async functions
22+
- RLC path: `src/transform/transform.ts:43` `transformRLCModel()` is a clean TCGC→IR adapter, producing `RLCModel` from `@azure-tools/rlc-common`. This is the model to copy for Modular.
23+
- Modular path: NO intermediate IR. `src/modular/build*.ts` functions receive `SdkClientType<SdkServiceOperation>` (TCGC type) directly. Rendering and TCGC consumption are fused in the same function.
24+
- `src/codemodel/` does NOT exist (despite history.md description). It is a target location, not existing code.
25+
- `ContextManager` singleton at `src/contextManager.ts:41` carries: `outputProject`, `tcgcContext`, `sdkTypes`, `binder`, `dependencies`, `rlcMetaTree`, `symbolMap`.
26+
27+
**Key citations to reuse:**
28+
- `src/index.ts:203` — is-modular-library branch point
29+
- `src/index.ts:357` — flat `for (subClient of clientMap)` loop (no recursive tree walk)
30+
- `src/index.ts:411–417` — final ts-morph file write loop
31+
- `src/transform/transform.ts:43``transformRLCModel()` — RLC adapter; use as pattern
32+
- `src/modular/buildOperations.ts:50``buildOperationFiles()` — imports SdkClientType directly
33+
- `src/modular/emitModels.ts:20–35` — imports ~10 TCGC types directly
34+
- `src/modular/helpers/namingHelpers.ts:13` — naming spread across helpers, no dedicated layer
35+
- `src/framework/hooks/sdkTypes.ts:79``provideSdkTypes()` — partial TCGC wrapper
36+
- `src/framework/hooks/binder.ts:34``Binder` interface — good pattern for explicit DI
37+
38+
**Rust/Go citations confirmed:**
39+
- Rust adapter: `typespec-rust/packages/typespec-rust/src/tcgcadapter/adapter.ts:58``class Adapter`
40+
- Rust IR root: `typespec-rust/packages/typespec-rust/src/codemodel/crate.ts:13``interface Crate`
41+
- Rust naming: `typespec-rust/packages/typespec-rust/src/tcgcadapter/naming.ts:18``getEscapedReservedName`
42+
- Go IR root: `autorest.go/packages/codemodel.go/src/codeModel.ts:9``interface CodeModel`
43+
- Go naming: `autorest.go/packages/naming.go/src/naming.ts:9``getEscapedReservedName`, `ensureNameCase`
44+
45+
**Top 3 deltas (filed to .squad/decisions/inbox/lambert-architecture-delta-v1.md):**
46+
1. No language-specific IR for Modular path (Rust: `Crate`, Go: `CodeModel`, TS Modular: nothing)
47+
2. No dedicated adapter/transform phase for Modular (TCGC leaks into every build* function)
48+
3. ContextManager ambient singleton vs explicit data flow / injectable DI
49+
50+
**Impedance mismatches noted:**
51+
- ts-morph AST vs string builders — ts-morph is *correct* for TS emitter; intent (separation) matters, not mechanism
52+
- Rust trait/impl vs TS structural typing — IR should use discriminated unions (kind: "model"|"enum"|...)
53+
- Go `FsFacilities` injectable FS — modular path actually close to this already (in-memory Project, single write loop)
54+
- Go `fixStutteringTypeNames()` named pass — TS has ad-hoc `normalizeName()` calls; consolidation needed
55+
56+
### 2026-05-15 — PR #3970 POC Findings (addendum)
57+
58+
**PR #3970 (`origin/poc-emitter-separation`, commit `4459962`) — confirmed details:**
59+
- 5 new files only. Zero existing files modified. `src/index.ts` and `src/modular/buildClientContext.ts` are identical to main.
60+
- New dirs: `src/codemodel/index.ts` (IR), `src/tcgcadapter/adapter.ts` (adapter), `src/codegen/{emitter,clients,index}.ts` (renderer)
61+
- The new pipeline is NOT yet wired into `$onEmit` — pure additive infrastructure
62+
- Unit tests 639 pass, smoke tests pass — validates the IR shape compiles and is logically sound
63+
- `TSCodeModel` shape: `{ clients: TSClient[], settings: TSGenerationSettings }` — correct root
64+
- `TSClient` has `id`, `name`, `modularName`, `contextTypeName`, `parameters: TSClientParameter[]`, `apiVersion`, `endpoint`, `credential`, `operationGroups`, `methods`, `children`, `path`
65+
- Models/enums/unions are NOT yet in the IR — only client context scope covered
66+
- Adapter at `adapter.ts:75``adaptToCodeModel(input)`, `adaptSingleClient(...)` at line 94
67+
- Adapter reuses `src/modular/helpers/` helpers (lines 29, 30, 40–43) — pragmatic, needs migration plan
68+
- Codegen at `codegen/clients.ts``emitClientContext(project, client, settings)` — zero TCGC imports ✓
69+
- `useDependencies()` / `resolveReference()` still called in codegen — acceptable (narrow framework hooks, not TCGC)
70+
- First-stage refactor: wire `adaptSingleClient` + `emitFromCodeModel` into `index.ts` `generateModularSources()` replacing `buildClientContext()` call — one-for-one swap, integration-test validated
71+
72+
**Key file citations:**
73+
- POC IR root: `packages/typespec-ts/src/codemodel/index.ts:26``interface TSCodeModel`
74+
- POC adapter entry: `packages/typespec-ts/src/tcgcadapter/adapter.ts:75``adaptToCodeModel()`
75+
- POC codegen entry: `packages/typespec-ts/src/codegen/emitter.ts:25``emitFromCodeModel()`
76+
- POC client renderer: `packages/typespec-ts/src/codegen/clients.ts:31``emitClientContext()`
77+
- Wiring target: `packages/typespec-ts/src/index.ts` around line 361 (`buildClientContext` call)
78+
79+
### 2026-05-15 — plan.md Reconciliation (addendum)
80+
81+
**plan.md is Maor's post-POC writeup (`poc-emitter-separation` + `poc-emitter-separation-complete`). Key takeaways:**
82+
83+
Doubly validated (plan + go-rust.md + source):
84+
- Three-layer pipeline, one-pass build, directory enforcement sufficient, smoke tests as oracle, getOperationFunction() as choke point
85+
86+
Divergences from Go/Rust (noted in pattern note §Reconciling):
87+
- PRD 2 self-contradiction: marked done locally, also advised against as standalone step → flag for Ripley
88+
- `CodeExpr = (string | SymbolRef)[]` (plan §2.2): TS-specific, correct divergence, must stay in codegen not codemodel
89+
- `TSTypeRef` deferred: OK but use typed discriminants (TSMethodKind, TSLroMetadata) as mitigation
90+
91+
Critical "aspirational vs shipped" finding:
92+
- ALL plan.md "✅ Done" items are local only. NOTHING from PRDs 1–13 is on main. Even the index.ts wiring (PRD 1 scope item) is not committed in PR #3970.
93+
94+
Missing from plan vs Go/Rust:
95+
1. Adapter type cache (Go: `Map<string, go.WireType>`) — add to PRD 7 design
96+
2. Named naming layer `src/tcgcadapter/naming.ts` — no PRD for this
97+
3. ESLint `no-restricted-imports` on `src/codegen/` and `src/codemodel/` — cheap layer guard
98+
4. `Readonly<TSCodeModel>` in codegen signatures — one-way flow enforcement
99+
100+
Key plan.md citations for future reference:
101+
- §2.1: operationHelpers.ts 3321 LOC choke point; extractOperationShape() as fix
102+
- §2.2: CodeExpr pattern; resolveReference side effects
103+
- §2.3: refkey is process-local, not serializable → stable semantic IDs in IR
104+
- §2.5: addDeclaration is renderer concern; store declarationRefkey in IR
105+
- §2.13: full TSCodeModel needs models/enums/unions/helpers at root
106+
- §PRD sequence: PRD 3 → PRD 4 → PRDs 5,6,8 parallel → PRD 7 → PRDs 9,10,11 → PRD 12 → PRD 13
107+
108+
### 2026-05-18T19:09:04.148+00:00 — PR #3981 description rewrite
109+
110+
- Rewrote the PR title/body in Maor's voice to explain the Rust/Go-inspired three-layer migration (`tcgcadapter/``codemodel/``codegen/`), call out stages 0-6 as the current in-place swap, and explicitly list the three deferred deltas.
111+
112+
## 2026-05-18T21:30Z — ARCHITECTURE.md for packages/typespec-ts
113+
Wrote `packages/typespec-ts/ARCHITECTURE.md` (~270 lines): entry-point summary,
114+
RLC vs Modular framing, ASCII diagram of the three-layer Modular pipeline,
115+
TCGC adapter / code model / codegen sections with file links, framework + static
116+
helpers section, two end-to-end flows (full $onEmit, single paged operation),
117+
testing pointer table, legacy notes, and deferred work (ContextManager singleton,
118+
adapter→modular/helpers coupling, missing naming.ts, codegen/models.ts still
119+
imports TCGC). Verified against src/index.ts, src/tcgcadapter/adapter.ts,
120+
src/codemodel/index.ts, src/codegen/*.ts, and CONTRIBUTING.md. Not committed —
121+
Maor will review and commit.
122+
123+
### 2026-05-18T21:01:22.901+00:00 — Paging helper import must stay module-pinned
124+
125+
- Root cause: `packages/typespec-ts/src/modular/helpers/operationHelpers.ts` emitted `PagedAsyncIterableIterator` as a free symbol in public signatures, while `packages/typespec-ts/src/codegen/indexFiles.ts` and `packages/typespec-ts/src/modular/buildRootIndex.ts` also exposed the same type through barrels. That left multiple plausible import sources and led to the post-hoc cleanup in `packages/typespec-ts/src/codegen/pagingImports.ts`.
126+
- Fix shape: pin the public type to `static-helpers/pagingHelpers.js` at the emission sites (`src/codegen/operations.ts`, `src/codegen/classicalClient.ts`, `src/codegen/classicalOperations.ts`) and pin barrel re-exports to the same concrete module (`src/codegen/indexFiles.ts`, `src/modular/buildRootIndex.ts`). The renderer now writes the import/export edge directly instead of asking a later sweep to guess.
127+
- Prior-art note: this matches the Rust/Go pattern where symbol provenance rides with the import edge rather than getting reconstructed from a barrel after the fact. In the TS emitter, the equivalent primitive is an explicit `moduleSpecifier` on the generated import/export.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# References must carry module identity
2+
3+
**Date:** 2026-05-18T21:01:22.901+00:00
4+
**Status:** Proposed
5+
6+
## Context
7+
8+
`packages/typespec-ts/src/codegen/pagingImports.ts` existed solely to strip `PagedAsyncIterableIterator` back out of `*index.js` imports after emission. The underlying problem was that paging signatures were emitted without a concrete module while root barrels also re-exported the same symbol, so the system had more than one valid-looking source for the same public type.
9+
10+
## Decision
11+
12+
References that cross files must carry module identity. If a helper or runtime symbol is meant to come from a concrete module, the emitter must write an explicit import/export edge for that module at the source emission site. Barrel resolution plus post-hoc dedupe is forbidden going forward.
13+
14+
## Consequences
15+
16+
When a helper type becomes part of public surface area, update the emitting renderer and any barrel re-exports together. If the current reference primitive cannot express the concrete module, extend the primitive first rather than adding another cleanup pass.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Module-aware references
2+
3+
Use this when a generated public type or helper can be imported from both a barrel and its concrete runtime/helper module.
4+
5+
## Rule
6+
7+
Keep symbol provenance on the import/export edge.
8+
9+
- Emit the public signature with the concrete symbol name.
10+
- Add an explicit import from the concrete module in the file that owns the signature.
11+
- Re-export that symbol from barrels with an explicit `moduleSpecifier` pointing at the same concrete module.
12+
- Do not add post-hoc dedupe or text-rewrite passes.
13+
14+
## Current example
15+
16+
`PagedAsyncIterableIterator` now stays pinned to `static-helpers/pagingHelpers.js` in:
17+
- `packages/typespec-ts/src/codegen/operations.ts`
18+
- `packages/typespec-ts/src/codegen/classicalClient.ts`
19+
- `packages/typespec-ts/src/codegen/classicalOperations.ts`
20+
- `packages/typespec-ts/src/codegen/indexFiles.ts`
21+
- `packages/typespec-ts/src/modular/buildRootIndex.ts`
22+
23+
## Checklist
24+
25+
1. Find the first renderer that writes the public signature.
26+
2. Find every barrel that re-exports the same symbol.
27+
3. Make the renderer and barrel agree on one concrete module.
28+
4. Delete any cleanup pass that only exists to paper over duplicate sources.
29+
5. Validate with build + unit tests + the relevant integration suite.

packages/typespec-ts/src/codegen/classicalClient.ts

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,10 @@ import type {
1515
TSMethod
1616
} from "../codemodel/index.js";
1717
import { resolveReference } from "../framework/reference.js";
18-
import { dedupePagedAsyncIterableIteratorImports } from "./pagingImports.js";
1918
import { refkey } from "../framework/refkey.js";
2019
import { useDependencies } from "../framework/hooks/useDependencies.js";
2120
import { AzurePollingDependencies } from "../modular/external-dependencies.js";
22-
import {
23-
PagingHelpers,
24-
SimplePollerHelpers
25-
} from "../modular/static-helpers-metadata.js";
21+
import { SimplePollerHelpers } from "../modular/static-helpers-metadata.js";
2622
import { getPagingLROMethodName } from "../modular/helpers/classicalOperationHelpers.js";
2723

2824
export function emitClassicalClient(
@@ -74,6 +70,14 @@ export function emitClassicalClient(
7470
});
7571
}
7672

73+
if (usesPagedAsyncIterableIterator(client.methods, settings)) {
74+
file.addImportDeclaration({
75+
isTypeOnly: true,
76+
namedImports: ["PagedAsyncIterableIterator"],
77+
moduleSpecifier: "./static-helpers/pagingHelpers.js"
78+
});
79+
}
80+
7781
const clientClass = file.addClass({
7882
isExported: true,
7983
name: client.name
@@ -206,11 +210,21 @@ export function emitClassicalClient(
206210
}
207211

208212
file.fixMissingImports({}, { importModuleSpecifierEnding: "js" });
209-
dedupePagedAsyncIterableIteratorImports(file);
210213
file.fixUnusedIdentifiers();
211214
return file;
212215
}
213216

217+
function usesPagedAsyncIterableIterator(
218+
methods: TSMethod[],
219+
settings: TSGenerationSettings
220+
): boolean {
221+
return methods.some(
222+
(method) =>
223+
method.kind === "paging" ||
224+
(settings.compatibilityLro && method.kind === "lroPaging")
225+
);
226+
}
227+
214228
function addConstructor(
215229
clientClass: any,
216230
client: TSClient,
@@ -378,7 +392,7 @@ function buildMethodDeclarations(
378392
docs: [`@deprecated use ${methodName} instead`],
379393
kind: StructureKind.Method,
380394
name: normalizeName(getPagingLROMethodName(methodName), NameType.Method),
381-
returnType: `${resolveReference(PagingHelpers.PagedAsyncIterableIterator)}<${method.compatibilityLroPagingReturnType ?? "void"}>`,
395+
returnType: `PagedAsyncIterableIterator<${method.compatibilityLroPagingReturnType ?? "void"}>`,
382396
parameters,
383397
statements: `return ${resolveReference(method.apiRefKey)}(${[
384398
"this._client",

packages/typespec-ts/src/codegen/classicalOperations.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,7 @@ import { resolveReference } from "../framework/reference.js";
2222
import { AzurePollingDependencies } from "../modular/external-dependencies.js";
2323
import { getPagingLROMethodName } from "../modular/helpers/classicalOperationHelpers.js";
2424
import { getClassicalLayerPrefix } from "../modular/helpers/namingHelpers.js";
25-
import {
26-
PagingHelpers,
27-
SimplePollerHelpers
28-
} from "../modular/static-helpers-metadata.js";
25+
import { SimplePollerHelpers } from "../modular/static-helpers-metadata.js";
2926

3027
interface ClassicalOperationNode {
3128
prefixes: string[];
@@ -56,6 +53,13 @@ export function emitClassicalOperationFiles(
5653
{ overwrite: true }
5754
);
5855
addContextImport(file, client, node);
56+
if (usesPagedAsyncIterableIterator(node.methods, settings)) {
57+
file.addImportDeclaration({
58+
isTypeOnly: true,
59+
namedImports: ["PagedAsyncIterableIterator"],
60+
moduleSpecifier: `${"../".repeat(node.prefixes.length + 1)}static-helpers/pagingHelpers.js`
61+
});
62+
}
5963
emitClassicalOperationFile(file, client, node, settings);
6064
file.fixMissingImports({}, { importModuleSpecifierEnding: "js" });
6165
file.fixUnusedIdentifiers();
@@ -65,6 +69,17 @@ export function emitClassicalOperationFiles(
6569
return files;
6670
}
6771

72+
function usesPagedAsyncIterableIterator(
73+
methods: TSMethod[],
74+
settings: TSGenerationSettings
75+
): boolean {
76+
return methods.some(
77+
(method) =>
78+
method.kind === "paging" ||
79+
(settings.compatibilityLro && method.kind === "lroPaging")
80+
);
81+
}
82+
6883
function buildOperationTree(
6984
groups: TSOperationGroup[]
7085
): ClassicalOperationNode {
@@ -270,9 +285,7 @@ function getMethodProperties(
270285
properties.push({
271286
kind: StructureKind.PropertySignature,
272287
name: normalizeName(getPagingLROMethodName(methodName), NameType.Method),
273-
type: `(${paramStr}) => ${resolveReference(
274-
PagingHelpers.PagedAsyncIterableIterator
275-
)}<${method.compatibilityLroPagingReturnType ?? "void"}>`,
288+
type: `(${paramStr}) => PagedAsyncIterableIterator<${method.compatibilityLroPagingReturnType ?? "void"}>`,
276289
docs: [`@deprecated use ${methodName} instead`]
277290
});
278291
}

packages/typespec-ts/src/codegen/indexFiles.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { resolveReference } from "../framework/reference.js";
99
import {
1010
CloudSettingHelpers,
1111
MultipartHelpers,
12-
PagingHelpers,
1312
PlatformTypeHelpers
1413
} from "../modular/static-helpers-metadata.js";
1514

@@ -298,15 +297,20 @@ function exportPagingTypes(client: TSClient, rootIndexFile: SourceFile): void {
298297
return;
299298
}
300299

301-
addExportsToIndex(
302-
rootIndexFile,
303-
[
304-
resolveReference(PagingHelpers.PageSettings),
305-
resolveReference(PagingHelpers.ContinuablePage),
306-
resolveReference(PagingHelpers.PagedAsyncIterableIterator)
307-
],
308-
true
309-
);
300+
const exported = new Set(rootIndexFile.getExportedDeclarations().keys());
301+
const namedExports = [
302+
"PageSettings",
303+
"ContinuablePage",
304+
"PagedAsyncIterableIterator"
305+
].filter((name) => !exported.has(name));
306+
307+
if (namedExports.length > 0) {
308+
rootIndexFile.addExportDeclaration({
309+
isTypeOnly: true,
310+
moduleSpecifier: "./static-helpers/pagingHelpers.js",
311+
namedExports
312+
});
313+
}
310314
}
311315

312316
function hasPaging(client: TSClient): boolean {

packages/typespec-ts/src/codegen/operations.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ import type {
1212
TSClient,
1313
TSFunctionDeclaration,
1414
TSGenerationSettings,
15-
TSOperationGroup
15+
TSOperationGroup,
16+
TSMethod
1617
} from "../codemodel/index.js";
1718
import { addDeclaration } from "../framework/declaration.js";
18-
import { dedupePagedAsyncIterableIteratorImports } from "./pagingImports.js";
1919

2020
/**
2121
* Emit modular operation source files from the TypeScript code model.
@@ -52,14 +52,31 @@ export function emitOperations(
5252
namedImports: [`${client.contextTypeName} as Client`],
5353
moduleSpecifier: `${indexPathPrefix}index.js`
5454
});
55+
if (usesPagedAsyncIterableIterator(group.methods, settings)) {
56+
file.addImportDeclaration({
57+
isTypeOnly: true,
58+
namedImports: ["PagedAsyncIterableIterator"],
59+
moduleSpecifier: `../${"../".repeat(group.prefixes.length)}static-helpers/pagingHelpers.js`
60+
});
61+
}
5562
file.fixMissingImports({}, { importModuleSpecifierEnding: "js" });
56-
dedupePagedAsyncIterableIteratorImports(file);
5763
file.fixUnusedIdentifiers();
5864

5965
return file;
6066
});
6167
}
6268

69+
function usesPagedAsyncIterableIterator(
70+
methods: TSMethod[],
71+
settings: TSGenerationSettings
72+
): boolean {
73+
return methods.some(
74+
(method) =>
75+
method.kind === "paging" ||
76+
(settings.compatibilityLro && method.kind === "lroPaging")
77+
);
78+
}
79+
6380
function getOperationGroups(client: TSClient): TSOperationGroup[] {
6481
const groups: TSOperationGroup[] = [];
6582

0 commit comments

Comments
 (0)