Skip to content
This repository was archived by the owner on Jun 30, 2026. It is now read-only.

Commit 9a21858

Browse files
maorlegerCopilot
andcommitted
fix(codegen/models): emit array/dict serializer helpers to resolve B8 placeholders
The new filtered-IR renderer in src/codegen/models.ts only walked codeModel.models/enums/unions and skipped array/dict helper types from the emitQueue. The serializer builders (buildSerializerFunction.ts, buildDeserializerFunction.ts) still emit refkey(type, 'serializer') / refkey(type, 'deserializer') references for those helpers. With no matching declaration registered, the binder left __PLACEHOLDER_*__ tokens unresolved — causing 26 TS2304 errors in the SCVMM and NetworkAnalytics repros. Strategy A: walk emitQueue entries of kind 'array'/'dict' and call emitType() for each, mirroring what the legacy emitTypes() in emitModels.ts did. This registers the serializer/deserializer refkeys so the binder can substitute them. A TODO comment and .squad/decisions/inbox/dallas-models-helpers.md document the follow-up to migrate these helper types into TSCodeModel IR, removing the emitQueue side-channel dependency from the codegen layer. Validated: - pnpm build: clean - npm run unit-test (typespec-ts): 664/664 passed - NetworkAnalytics.Management regen: zero __PLACEHOLDER_ matches, zero TS2304 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 56aa9c5 commit 9a21858

4 files changed

Lines changed: 341 additions & 1 deletion

File tree

.squad/agents/dallas/history.md

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
# Project Context
2+
3+
- **Owner:** Maor Leger
4+
- **Project:** autorest.typescript — TypeSpec TS emitter refactor to align with Rust/Go emitter architecture.
5+
- **Stack:** TypeScript, pnpm, TypeSpec, ts-morph, vitest, TCGC.
6+
- **Key paths:**
7+
- Emitter source: `packages/typespec-ts/src/` (modular/, rlc/, codemodel/, framework/, static-helpers/)
8+
- Shared RLC: `packages/rlc-common/`
9+
- Smoke fixtures: `packages/typespec-test/test/`
10+
- Unit tests: `packages/typespec-ts/test/unit` (RLC), `packages/typespec-ts/test/modularUnit` (Modular)
11+
- Integration: `test/integration`, `test/modularIntegration`, `test/azureIntegration`, `test/azureModularIntegration`
12+
- **Build/test commands:** `pnpm install`, `pnpm build`, `pnpm format`, `npm run unit-test`, `npm run lint`, `npm run integration-test-ci:{rlc|modular|azure-rlc|azure-modular}`, `npm run smoke-test`.
13+
- **Reference architectures:** `~/workspace/emitter-chain/typespec-rust`, `~/workspace/emitter-chain/autorest.go`, doc at `/home/maorleger/workspace/emitter-chain/go-rust.md`.
14+
- **Hands-off:** `packages/autorest.typescript/` is in maintenance mode.
15+
- **Existing codemodel pattern:** `src/codemodel/` already uses types.ts + build-*.ts + render-*.ts separation — likely the seed for the broader refactor.
16+
- **Created:** 2026-05-15
17+
18+
## Learnings
19+
20+
<!-- Append new learnings below. Each entry is something lasting about the project. -->
21+
22+
### 2026-05-15 — Stage 1 client context pipeline swap
23+
24+
- Replaced the modular `$onEmit` client-context call in `packages/typespec-ts/src/index.ts` from `buildClientContext(dpgContext, subClient, modularEmitterOptions)` to `adaptSingleClient(subClient, dpgContext, modularEmitterOptions)` plus `emitClientContext(project, tsClient, generationSettings)`.
25+
- Kept `buildOperationFiles`, `buildClassicalClient`, and the rest of the modular source pipeline unchanged; only the client context path was swapped in-place.
26+
- Adjusted `packages/typespec-ts/src/codegen/clients.ts` to preserve prior client-context output semantics, including nested subfolder paths, api-version required/interface behavior, options typing, and passthrough of endpoint-assigned optional params.
27+
- Validation: `pnpm build` passed, `cd packages/typespec-ts && npm run unit-test` passed, and `cd packages/typespec-ts && npm run copy:typespec && npm run integration-test-ci:modular` passed. `npm run lint` currently fails with a pre-existing ESLint/@typescript-eslint rule loading error while linting `src/codegen/clients.ts`.
28+
29+
### 2026-05-15 — Ripley Staged Refactor Plan: Three-Layer Pipeline (appended by Scribe)
30+
31+
**Staged refactor plan approved and ready for implementation:**
32+
33+
Ripley completed a 9-stage refactor plan (`.squad/decisions/ripley-staged-refactor-plan.md`) to decouple TCGC from rendering:
34+
35+
1. **Three-layer architecture:**
36+
- **CodeModel (IR):** `TSCodeModel` capturing emitter intent, zero TCGC.
37+
- **TCGC Adapter:** Transforms TCGC → CodeModel; isolated in `src/tcgcadapter/`.
38+
- **CodeGen:** Renders CodeModel → ts-morph AST; consumed by `src/codegen/`.
39+
40+
2. **Stage structure (Stages 1–9; Stage 10 dropped):**
41+
- Stages 1–2: Adapter validation (TCGC→CodeModel only).
42+
- Stages 3–6: Codegen expansion (clients, operations, models, classicalClient).
43+
- Stages 7–9: Helper migration, cleanup, polish.
44+
45+
3. **Key directives applied:**
46+
- ✅ No feature flag (swap-in-place migration).
47+
- ✅ Adapter unit tests as primary validation surface.
48+
- ✅ Readability-first file organization (monolithic until needed).
49+
- ✅ Skip lint guard (trust patterns).
50+
- ✅ Skip Stage 10 (package separation).
51+
52+
**Your work (PRD 3 onwards) aligns with Stages 3–6.** See full plan for stage boundaries and test matrix.
53+
54+
### 2026-05-15 — Lambert Cross-Agent Summary: Architecture Analysis Findings (appended by Scribe)
55+
56+
**From Lambert's comparative analysis** (filed to decisions.md 2026-05-15):
57+
58+
Key findings for **Dallas** (codegen layer development):
59+
1. **Codegen target is well-defined.** The POC's `src/codegen/` provides a template. `emitFromCodeModel()` orchestrator + `emitClientContext()` renderer (for clients.ts category) show the pattern. Zero TCGC imports in this layer — verify via lint.
60+
2. **IR shape (`TSCodeModel`) needs extension.** Current POC covers client context files only. Add to IR for **your** work scope:
61+
- `TSOperationFile` for operations (PRD 3)
62+
- `TSModel`, `TSEnum`, `TSUnion` for types (PRD 7)
63+
- Model-scoped types: `TSProperty`, `TSPropertyConstraint`, `TSModelBase` (reference Rust's `codemodel/types.ts` pattern)
64+
3. **Adapter helpers reuse is pragmatic but temporary.** POC imports from old `src/modular/helpers/` (clientHelpers, operationHelpers). As each old `build*` migrates, its helpers either move into `src/tcgcadapter/` (if TCGC-aware) or into `src/codegen/` (if IR-only). Plan this as part of each adapter extension.
65+
4. **Rendering machinery is stable.** `resolveReference()` and `useDependencies()` from framework are acceptable as narrow hooks in codegen (not TCGC leakage). Equivalent to Go's `FsFacilities` injector — a contract for framework services.
66+
5. **Test all codegen outputs via integration suites.** Each category (operations, models, classicalClient, etc.) is tested by modular/azure-modular integration suites; smoke test catches compilation failures. Use these as your regression oracle.
67+
68+
### 2026-05-15 — Stage 0 Infrastructure Verification
69+
70+
- The `origin/poc-emitter-separation` POC commit `4459962` was already present on `squad-rewrite` under commit `3542d9e8c`, with the same additive `src/codemodel/`, `src/tcgcadapter/`, and `src/codegen/` files plus the fixture `.d.ts` deletions.
71+
- Verified the Stage 0 infrastructure remains unwired to `$onEmit`; no existing emitter entrypoints were changed as part of this slice.
72+
- `pnpm build` passed at repo root and `npm run unit-test` passed in `packages/typespec-ts/` without needing follow-up fixes.
73+
74+
### 2026-05-15 — Stage 2 operation IR expansion
75+
76+
- `packages/typespec-ts/src/codemodel/index.ts` now models operations with data-only shapes: `TSMethod`, `TSParameter`, `TSReturnType`, `TSRoute`, and `TSOperationGroup`.
77+
- `packages/typespec-ts/src/tcgcadapter/adapter.ts` now exports `adaptMethods()` and `adaptOperationGroups()` so operation extraction can be tested separately from full-client adaptation.
78+
- The operation-group IR keeps `prefixes` alongside `name` and `methods` so future rendering can recover nested group paths without reaching back into TCGC.
79+
- Validation for this slice was `pnpm build` at repo root and `npm run unit-test` in `packages/typespec-ts`.
80+
81+
### 2026-05-16 — Stage 4 operations codegen wiring
82+
83+
- `packages/typespec-ts/src/codegen/operations.ts` now serves as the modular operations renderer, consuming `TSClient`/`TSOperationGroup` IR and emitting stable `api/**/operations.ts` files directly through ts-morph.
84+
- The emitter path keeps the generated operation helpers deterministic by sorting operation files by their normalized path and running `fixMissingImports(..., { importModuleSpecifierEnding: "js" })` before trimming unused imports.
85+
- Validation for this slice was `pnpm build`, `cd packages/typespec-ts && npm run unit-test`, and `cd packages/typespec-ts && npm run copy:typespec && npm run integration-test-ci:modular`.
86+
87+
### 2026-05-19 — squad-rewrite regression fixes
88+
89+
- Restored top-level `api/**` recursion in `packages/typespec-ts/src/codegen/indexFiles.ts` so the root barrel once again reaches generated `./api/<resource>/index.js` subbarrels and their `*OptionalParams` exports.
90+
- Changed `packages/typespec-ts/src/codegen/clients.ts` to respect adapted `TSClientParameter.required` metadata for client contexts, and added a modular unit test that keeps defaulted client `apiVersion` optional in the generated `*Context` interface.
91+
- Switched `packages/typespec-ts/src/codegen/models.ts` to select raw model/enum/union declarations from filtered `TSCodeModel` IR lookups instead of the legacy global emit queue, which keeps paging `*ListResult` shapes internal unless the adapter actually exposes them.
92+
- Triaged user report regressions: 4 confirmed (indexFiles subpath barrel, clients apiVersion requiredness, models paging leak, dedupe workaround); 1 misdiagnosed (coreClient import text churn); 2 expected (import path normalization, beginX wrapper reappearance).
93+
- Commits: d24c6178d (indexFiles), e35c3244d (apiVersion), e8b5a8022 (models). Pushed origin/squad-rewrite tip 56aa9c54f.
94+
- Validation: `pnpm build` ✅, `npm run unit-test` in `packages/typespec-ts/` ✅.
95+
96+
---
97+
98+
## 2026-05-19T23:30:29.807+00:00 — B8 Fix: Array/dict serializer helper placeholders
99+
100+
**Task:** Fix P0 regression introduced by e8b5a8022 where `src/models/models.ts` contained
101+
unresolved `__PLACEHOLDER_*__` tokens for array/dict serializer helpers.
102+
103+
**Strategy chosen:** Strategy A (Renderer emits the missing helpers)
104+
105+
**Rationale:** Strategy B would require adding `helperTypes` to `TSCodeModel` and updating the
106+
tcgcadapter — a broader change. Strategy A is a targeted, correct fix: the renderer simply
107+
needs to walk the same `emitQueue` entries (array/dict kinds) that the legacy `emitTypes()`
108+
did, calling `emitType()` to register the serializer/deserializer refkeys with the binder.
109+
A TODO comment and follow-up note were left for Strategy B migration.
110+
111+
**Files touched:**
112+
- `packages/typespec-ts/src/codegen/models.ts` — import `emitQueue`; add loop for array/dict types
113+
- `packages/typespec-ts/test/modularUnit/models-helpers.spec.ts` — new regression-locking tests
114+
- `.squad/decisions/inbox/dallas-models-helpers.md` — follow-up note for IR migration
115+
116+
**Validation:**
117+
- `pnpm build` — passed
118+
- `npm run unit-test` (typespec-ts) — 664 tests passed, 0 failures
119+
- Regenerated `NetworkAnalytics.Management` (azure-modular tag) — zero `__PLACEHOLDER_` matches
120+
- `tsc --noEmit` on generated NetworkAnalytics package — only missing `@azure/identity` in samples (pre-existing, unrelated), zero TS2304 errors
121+
- B8 regression tests (array + dict) — both pass green
122+
123+
**Open follow-up:** `.squad/decisions/inbox/dallas-models-helpers.md` — migrate array/dict helper types into TSCodeModel IR to remove `emitQueue` side-channel dependency from the codegen layer.
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Follow-up: Migrate array/dict helper types into TSCodeModel IR
2+
3+
**Date:** 2026-05-19T23:30:29.807+00:00
4+
**Author:** Dallas (Refactor Engineer)
5+
**Status:** Open / Follow-up
6+
7+
## Context
8+
9+
B8 fix (Strategy A) in `src/codegen/models.ts` restores array/dict serializer helper
10+
registration by walking the global `emitQueue` side-channel (the same set that the
11+
legacy `emitTypes()` in `src/modular/emitModels.ts` used).
12+
13+
## Problem with current approach
14+
15+
`emitQueue` is a module-level `Set<SdkType>` populated by `visitPackageTypes()` (called
16+
via `provideSdkTypes()`). The new filtered-IR renderer in `src/codegen/models.ts` is
17+
supposed to work exclusively from `TSCodeModel` (pure IR, no TCGC) — but the B8 fix still
18+
reaches back into `emitQueue`, which is a TCGC-layer artifact.
19+
20+
This violates the layer boundary:
21+
22+
```
23+
src/tcgcadapter → src/codemodel (IR) → src/codegen
24+
25+
Should own array/dict helpers
26+
```
27+
28+
## Recommended follow-up
29+
30+
Add array and dictionary helper types explicitly to `TSCodeModel` so `emitModelFiles` can
31+
emit them purely from IR:
32+
33+
1. Add a `helperTypes` (or `arrayDictHelpers`) field to `TSCodeModel` in
34+
`src/codemodel/index.ts` containing the array/dict types that serializer builders
35+
will reference.
36+
2. Populate it in `src/tcgcadapter/adapter.ts` by walking the types reachable from
37+
`models`, `enums`, and `unions` and collecting all `SdkArrayType` / `SdkDictionaryType`
38+
that require serializer helpers.
39+
3. Update `src/codegen/models.ts` to iterate `codeModel.helperTypes` instead of `emitQueue`.
40+
4. Remove the `emitQueue` import from `src/codegen/models.ts`.
41+
42+
## Risk / priority
43+
44+
Low risk to defer — Strategy A is a correct and complete fix for B8.
45+
This is a cleanup task to keep the three-layer architecture clean.

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import { Project, SourceFile } from "ts-morph";
22
import type { TSCodeModel } from "../codemodel/index.js";
33
import type { SdkContext } from "../utils/interfaces.js";
4-
import { flattenPropertyModelMap } from "../framework/hooks/sdkTypes.js";
4+
import {
5+
emitQueue,
6+
flattenPropertyModelMap
7+
} from "../framework/hooks/sdkTypes.js";
58
import {
69
addSerializationFunctions,
710
emitType,
@@ -83,6 +86,27 @@ export function emitModelFiles(
8386
emitType(sdkContext, rawUnion, sourceFile);
8487
}
8588

89+
// Strategy A (B8 fix): emit array/dict serializer helpers that the serializer builders
90+
// still reference via refkey(type, "serializer"/"deserializer"). The legacy emitTypes()
91+
// in emitModels.ts handled this by walking the full emitQueue; the new filtered-IR
92+
// renderer only walks codeModel.models/enums/unions, so those helpers were never
93+
// registered with the binder — causing __PLACEHOLDER_*__ tokens to leak.
94+
//
95+
// TODO: migrate array/dict helper types into TSCodeModel IR so the renderer owns them
96+
// explicitly rather than relying on the global emitQueue side-channel.
97+
// See: .squad/decisions/inbox/dallas-models-helpers.md
98+
for (const type of emitQueue) {
99+
if (type.kind !== "array" && type.kind !== "dict") {
100+
continue;
101+
}
102+
const sourceFile = getOrCreateModelsFile(
103+
project,
104+
codeModel.settings.sourceRoot,
105+
getModelNamespaces(sdkContext, type)
106+
);
107+
emitType(sdkContext, type, sourceFile);
108+
}
109+
86110
for (const [property, baseModel] of flattenPropertyModelMap) {
87111
if (!includedModelKeys.has(getRawTypeKey(baseModel, sdkContext))) {
88112
continue;
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
/**
2+
* Regression test for B8: array/dict serializer helper refkeys must be registered
3+
* by src/codegen/models.ts so the binder can resolve them.
4+
*
5+
* Before the fix (commit e8b5a8022), emitModelFiles only walked codeModel.models/enums/unions
6+
* and skipped array/dict helper types, leaving __PLACEHOLDER_*__ tokens in the emitted file.
7+
*/
8+
import { describe, expect, it } from "vitest";
9+
import { Project } from "ts-morph";
10+
import { useContext } from "../../src/contextManager.js";
11+
import { useBinder } from "../../src/framework/hooks/binder.js";
12+
import { adaptToCodeModel } from "../../src/tcgcadapter/adapter.js";
13+
import { transformModularEmitterOptions } from "../../src/modular/buildModularOptions.js";
14+
import { emitModelFiles } from "../../src/codegen/models.js";
15+
import { renameClientName } from "../../src/index.js";
16+
import { createDpgContextTestHelper, rlcEmitterFor } from "../util/testUtil.js";
17+
18+
const PLACEHOLDER_PATTERN = /__PLACEHOLDER_/;
19+
20+
function buildAzureTypeSpec(body: string): string {
21+
return `
22+
import "@typespec/http";
23+
import "@typespec/rest";
24+
import "@typespec/versioning";
25+
import "@azure-tools/typespec-client-generator-core";
26+
import "@azure-tools/typespec-azure-core";
27+
28+
using Http;
29+
using Rest;
30+
using Versioning;
31+
using Azure.ClientGenerator.Core;
32+
using Azure.Core;
33+
using Azure.Core.Traits;
34+
35+
@service(#{ title: "Azure TypeScript Testing" })
36+
namespace Azure.TypeScript.Testing {
37+
${body}
38+
}
39+
`;
40+
}
41+
42+
describe("models-helpers (B8 regression lock)", () => {
43+
/**
44+
* Regression: array-of-complex-model serializer helpers must resolve cleanly.
45+
* If emitModelFiles doesn't emit the array helper declarations, the binder
46+
* leaves __PLACEHOLDER_*serializer__ / __PLACEHOLDER_*deserializer__ tokens.
47+
*/
48+
it("registers array serializer/deserializer refkeys — no placeholders after resolve", async () => {
49+
const typeSpec = buildAzureTypeSpec(`
50+
model Item {
51+
id: string;
52+
value: int32;
53+
}
54+
55+
model ItemCollection {
56+
items: Item[];
57+
}
58+
59+
@route("/items")
60+
@get
61+
op listItems(): ItemCollection;
62+
`);
63+
64+
const host = await rlcEmitterFor(typeSpec, { withRawContent: true });
65+
const sdkContext = await createDpgContextTestHelper(host.program, false, {
66+
isModularLibrary: true
67+
});
68+
sdkContext.rlcOptions!.isModularLibrary = true;
69+
70+
const emitterOptions = transformModularEmitterOptions(sdkContext, "", {
71+
casing: "camel"
72+
});
73+
for (const client of sdkContext.sdkPackage.clients) {
74+
await renameClientName(client, emitterOptions);
75+
}
76+
77+
const codeModel = adaptToCodeModel({ sdkContext, emitterOptions });
78+
const project = useContext("outputProject") as Project;
79+
const binder = useBinder();
80+
const sourceRoot = codeModel.settings.sourceRoot;
81+
82+
emitModelFiles(project, codeModel, sdkContext);
83+
binder.resolveAllReferences(sourceRoot);
84+
85+
const modelsFiles = project.getSourceFiles(`${sourceRoot}/models/**/*.ts`);
86+
expect(modelsFiles.length).toBeGreaterThan(0);
87+
88+
for (const file of modelsFiles) {
89+
const text = file.getFullText();
90+
expect(
91+
PLACEHOLDER_PATTERN.test(text),
92+
`Found unresolved __PLACEHOLDER__ in ${file.getFilePath()}:\n${text}`
93+
).toBe(false);
94+
}
95+
});
96+
97+
/**
98+
* Regression: dict-of-complex-model serializer helpers must resolve cleanly.
99+
*/
100+
it("registers dict serializer/deserializer refkeys — no placeholders after resolve", async () => {
101+
const typeSpec = buildAzureTypeSpec(`
102+
model Widget {
103+
name: string;
104+
weight: float32;
105+
}
106+
107+
model WidgetMap {
108+
byName: Record<Widget>;
109+
}
110+
111+
@route("/widgets")
112+
@get
113+
op getWidgets(): WidgetMap;
114+
`);
115+
116+
const host = await rlcEmitterFor(typeSpec, { withRawContent: true });
117+
const sdkContext = await createDpgContextTestHelper(host.program, false, {
118+
isModularLibrary: true
119+
});
120+
sdkContext.rlcOptions!.isModularLibrary = true;
121+
122+
const emitterOptions = transformModularEmitterOptions(sdkContext, "", {
123+
casing: "camel"
124+
});
125+
for (const client of sdkContext.sdkPackage.clients) {
126+
await renameClientName(client, emitterOptions);
127+
}
128+
129+
const codeModel = adaptToCodeModel({ sdkContext, emitterOptions });
130+
const project = useContext("outputProject") as Project;
131+
const binder = useBinder();
132+
const sourceRoot = codeModel.settings.sourceRoot;
133+
134+
emitModelFiles(project, codeModel, sdkContext);
135+
binder.resolveAllReferences(sourceRoot);
136+
137+
const modelsFiles = project.getSourceFiles(`${sourceRoot}/models/**/*.ts`);
138+
expect(modelsFiles.length).toBeGreaterThan(0);
139+
140+
for (const file of modelsFiles) {
141+
const text = file.getFullText();
142+
expect(
143+
PLACEHOLDER_PATTERN.test(text),
144+
`Found unresolved __PLACEHOLDER__ in ${file.getFilePath()}:\n${text}`
145+
).toBe(false);
146+
}
147+
});
148+
});

0 commit comments

Comments
 (0)