Skip to content

Commit 6139ad4

Browse files
committed
Run 11 prep: ban Duplicate classes, monkey-patching, duplicate fields; add TS export name extraction
Key changes for the next generation run: Verify script improvements: - Add _dedup_anyof normalization: deduplicate structurally identical anyOf variants so the agent can reuse error classes instead of creating *Duplicate/*Triplicate copies - Ban *Duplicate/*Triplicate class name suffixes - Ban monkey-patching patterns: .json_schema =, _bind_reference, _load_reference, frozen_schema, adapter.json_schema - Add duplicate field declaration detection (catches e.g. ParseError declaring extras: twice) Naming improvements: - Extract TypeBox export names from TS source into naming_hints.json (tsExportNames per service, tsSharedExportNames for lib/ dirs) - Agent now gets pre-extracted names like ExitInfo, MonitorResponse, ScreenshotAction, FilesystemError mapped to their service directories Prompt improvements (Pass 1 + Pass 2): - Document anyOf dedup normalization -- agent should reuse error classes - Ban _schema_map.py monkey-patching patterns explicitly - Reference tsExportNames/tsSharedExportNames from naming_hints.json - Add Run 10 failure patterns (Duplicate classes, monkey-patching, duplicate fields, missing TS names) - Rewrite _schema_map.py instructions to be explicit about what's banned
1 parent f868d2d commit 6139ad4

File tree

3 files changed

+348
-16
lines changed

3 files changed

+348
-16
lines changed

codegen-llm/src/codegen.ts

Lines changed: 146 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,17 @@ interface NamingHints {
391391
standardErrorCodes: string[];
392392
/** Maps $kind literal values to suggested variant class name prefixes. */
393393
kindValueToClassName: Record<string, string>;
394+
/**
395+
* TypeScript export names extracted from the server source.
396+
* Maps service directory name → list of exported TypeBox schema names.
397+
* e.g. { "shellExec": ["ExitInfo", "OutputChunk", "MonitorResponse", "ShellOutput"] }
398+
*/
399+
tsExportNames: Record<string, string[]>;
400+
/**
401+
* Shared TypeScript export names from lib/ directories.
402+
* e.g. { "lib/fs/errors": ["FilesystemError"], "lib/shell/schemas": ["ShellOutput"] }
403+
*/
404+
tsSharedExportNames: Record<string, string[]>;
394405
}
395406

396407
/**
@@ -438,7 +449,17 @@ function extractNamingHints(schemaPath: string): NamingHints {
438449
"CANCEL",
439450
];
440451

441-
return { errorCodeToClassName, standardErrorCodes, kindValueToClassName };
452+
// Extract TS export names (empty if no server source provided)
453+
const tsExportNames: Record<string, string[]> = {};
454+
const tsSharedExportNames: Record<string, string[]> = {};
455+
456+
return {
457+
errorCodeToClassName,
458+
standardErrorCodes,
459+
kindValueToClassName,
460+
tsExportNames,
461+
tsSharedExportNames,
462+
};
442463
}
443464

444465
/** Recursively find all `const` values under `properties.code` in error schemas. */
@@ -517,6 +538,116 @@ function kindValueToPascal(kind: string): string {
517538
return kind.charAt(0).toUpperCase() + kind.slice(1);
518539
}
519540

541+
/**
542+
* Regex to match TypeBox schema exports in TypeScript source files.
543+
*
544+
* Matches patterns like:
545+
* export const FooSchema = Type.Object({
546+
* export const MonitorResponse = Type.Union([
547+
* const ExitInfo = Type.Object({
548+
*/
549+
const TS_SCHEMA_EXPORT_RE =
550+
/(?:export\s+)?const\s+(\w+)\s*=\s*Type\.\w+\s*[<([\{]/g;
551+
552+
/**
553+
* Scan TypeScript source files for exported TypeBox schema names.
554+
*
555+
* Returns a map of service directory name → list of schema names found,
556+
* and a separate map for shared lib/ directory exports.
557+
*/
558+
function extractTsExportNames(
559+
serverSrcPath: string,
560+
): { services: Record<string, string[]>; shared: Record<string, string[]> } {
561+
const services: Record<string, string[]> = {};
562+
const shared: Record<string, string[]> = {};
563+
564+
if (!fs.existsSync(serverSrcPath)) {
565+
return { services, shared };
566+
}
567+
568+
// Scan service directories
569+
const entries = fs.readdirSync(serverSrcPath, { withFileTypes: true });
570+
for (const entry of entries) {
571+
if (!entry.isDirectory()) continue;
572+
573+
const dirPath = path.join(serverSrcPath, entry.name);
574+
const names = scanDirForTypeBoxExports(dirPath);
575+
if (names.length > 0) {
576+
if (entry.name === "lib") {
577+
// Recurse into lib subdirectories
578+
const libEntries = fs.readdirSync(dirPath, { withFileTypes: true });
579+
for (const libEntry of libEntries) {
580+
if (!libEntry.isDirectory()) continue;
581+
const libDirPath = path.join(dirPath, libEntry.name);
582+
const libNames = scanDirForTypeBoxExports(libDirPath);
583+
if (libNames.length > 0) {
584+
shared[`lib/${libEntry.name}`] = libNames;
585+
}
586+
}
587+
} else {
588+
services[entry.name] = names;
589+
}
590+
}
591+
}
592+
593+
// Also scan lib/ one level up from serverSrcPath (common pattern)
594+
const parentLib = path.join(serverSrcPath, "..", "lib");
595+
if (fs.existsSync(parentLib)) {
596+
const libEntries = fs.readdirSync(parentLib, { withFileTypes: true });
597+
for (const libEntry of libEntries) {
598+
if (!libEntry.isDirectory()) continue;
599+
const libDirPath = path.join(parentLib, libEntry.name);
600+
const libNames = scanDirForTypeBoxExports(libDirPath);
601+
if (libNames.length > 0) {
602+
const key = `lib/${libEntry.name}`;
603+
shared[key] = [...(shared[key] ?? []), ...libNames];
604+
}
605+
}
606+
}
607+
608+
return { services, shared };
609+
}
610+
611+
/**
612+
* Scan a directory's .ts files for TypeBox export names.
613+
* Returns deduplicated list of names, dropping "Schema" suffix.
614+
*/
615+
function scanDirForTypeBoxExports(dirPath: string): string[] {
616+
const names = new Set<string>();
617+
618+
let files: string[];
619+
try {
620+
files = fs.readdirSync(dirPath).filter((f) => f.endsWith(".ts"));
621+
} catch {
622+
return [];
623+
}
624+
625+
for (const file of files) {
626+
const filePath = path.join(dirPath, file);
627+
let content: string;
628+
try {
629+
content = fs.readFileSync(filePath, "utf8");
630+
} catch {
631+
continue;
632+
}
633+
634+
TS_SCHEMA_EXPORT_RE.lastIndex = 0;
635+
let match: RegExpExecArray | null;
636+
while ((match = TS_SCHEMA_EXPORT_RE.exec(content)) !== null) {
637+
const name = match[1]!;
638+
// Drop "Schema" suffix for the Python class name suggestion
639+
const pythonName = name.replace(/Schema$/, "");
640+
// Skip if the name looks like a local variable (starts with lowercase
641+
// single letter) or is too short to be meaningful
642+
if (pythonName.length >= 3 && /^[A-Z]/.test(pythonName)) {
643+
names.add(pythonName);
644+
}
645+
}
646+
}
647+
648+
return [...names].sort();
649+
}
650+
520651
function setupWorkspace(workDir: string, opts: CodegenOptions): void {
521652
// Copy the serialised schema
522653
fs.copyFileSync(opts.schemaPath, path.join(workDir, "schema.json"));
@@ -525,14 +656,27 @@ function setupWorkspace(workDir: string, opts: CodegenOptions): void {
525656
// implement PascalCase conversion (which it keeps getting wrong).
526657
log(opts, "Extracting naming hints from schema...");
527658
const namingHints = extractNamingHints(opts.schemaPath);
659+
660+
// Scan TypeScript source for exported schema names
661+
log(opts, "Scanning TypeScript source for export names...");
662+
const tsNames = extractTsExportNames(opts.serverSrcPath);
663+
namingHints.tsExportNames = tsNames.services;
664+
namingHints.tsSharedExportNames = tsNames.shared;
665+
666+
const totalTsNames = Object.values(tsNames.services).reduce(
667+
(s, names) => s + names.length,
668+
0,
669+
) + Object.values(tsNames.shared).reduce((s, names) => s + names.length, 0);
670+
528671
fs.writeFileSync(
529672
path.join(workDir, "naming_hints.json"),
530673
JSON.stringify(namingHints, null, 2),
531674
);
532675
log(
533676
opts,
534677
`Extracted ${Object.keys(namingHints.errorCodeToClassName).length} error names, ` +
535-
`${Object.keys(namingHints.kindValueToClassName).length} $kind variant names`,
678+
`${Object.keys(namingHints.kindValueToClassName).length} $kind variant names, ` +
679+
`${totalTsNames} TS export names`,
536680
);
537681

538682
// Write the verification script OUTSIDE the workspace so the agent

codegen-llm/src/prompts.ts

Lines changed: 112 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,30 @@ code and every \`$kind\` value found in schema.json:
8585
"output": "Output",
8686
"result": "Result",
8787
...
88+
},
89+
"tsExportNames": {
90+
"shellExec": ["ExitInfo", "OutputChunk", "MonitorResponse", "ShellOutput", ...],
91+
"artifact": ["Artifact", "CreateArtifactOptions", ...],
92+
...
93+
},
94+
"tsSharedExportNames": {
95+
"lib/fs": ["FilesystemError"],
96+
"lib/shell": ["ShellOutput", "ShellError"],
97+
...
8898
}
8999
}
90100
\`\`\`
91101
92102
**You MUST use the exact class names from this file** for error classes and
93103
\`$kind\` variants. Do NOT invent your own PascalCase conversion.
94104
105+
**For data types**, check \`tsExportNames\` and \`tsSharedExportNames\`. These
106+
are names extracted from the TypeScript source (with the "Schema" suffix
107+
removed). When you see a TypeBox schema in a service's \`schemas.ts\` named
108+
e.g. \`ExitInfo\`, the matching entry will appear in
109+
\`tsExportNames["shellExec"]\` as \`"ExitInfo"\`. **Use these names for your
110+
Python classes** instead of mechanical path-derived names.
111+
95112
The verifier rejects class names with 4+ consecutive uppercase letters
96113
(e.g. \`NOTFOUNDError\`, \`CGROUPCLEANUPERRORError\` — these are BANNED).
97114
@@ -112,10 +129,14 @@ verification fails immediately.
112129
- \`WithJsonSchema\` — do NOT attach raw JSON schemas to \`Any\` via annotations
113130
- \`json.loads(\` — do NOT embed raw JSON schema strings in generated code
114131
- Loading \`schema.json\` at runtime in \`_schema_map.py\` or any module
132+
- Monkey-patching \`.json_schema\` on TypeAdapter instances (\`.json_schema = \`)
133+
- \`_bind_reference\`, \`_load_reference\`, \`frozen_schema\` — do NOT replace
134+
adapter schemas with reference data from schema.json
115135
- Raw JSON Schema dicts embedded as Python dict literals or JSON strings
116136
- Any helper/utility that builds models from schema dicts at runtime
117137
118138
**Banned naming patterns (regex-enforced):**
139+
- \`Duplicate\` or \`Triplicate\` in a class name — reuse the same class instead
119140
- \`Variant\\d+\` anywhere in a class name — ALL banned
120141
- \`Input2\`, \`Output2\`, \`Errors3\` — numbered suffixes
121142
- 4+ consecutive uppercase letters: \`NOTFOUNDError\`, \`PTYERRORError\` — banned
@@ -406,7 +427,10 @@ generated/
406427
8. **Intersections.** Flatten all properties into a single BaseModel.
407428
408429
9. **Error deduplication.** Same \`code\` literal + same fields = same class.
409-
Define once at the top of the service file.
430+
Define once at the top of the service file. The verifier deduplicates
431+
structurally identical \`anyOf\` variants, so if the same error appears
432+
multiple times in a composed union, **reuse the same class** — do NOT
433+
create \`FooDuplicate\` or \`FooTriplicate\` copies.
410434
411435
412436
### The _schema_map.py module
@@ -430,6 +454,16 @@ SCHEMA_MAP: dict = {
430454
431455
Every service and procedure from schema.json must be represented.
432456
457+
**CRITICAL:** \`_schema_map.py\` must ONLY import TypeAdapter instances from
458+
the service modules and assemble them into the dict. It must NOT:
459+
- Load \`schema.json\` at runtime
460+
- Monkey-patch \`.json_schema\` methods on adapters
461+
- Use \`copy.deepcopy\` to cache reference schemas
462+
- Contain any function named \`_bind_reference_schemas\` or \`_load_reference_services\`
463+
464+
The adapters' \`.json_schema()\` output must come entirely from the Pydantic
465+
models they wrap. If verification fails, fix the **models**, not the schema map.
466+
433467
434468
## Verification
435469
@@ -452,6 +486,13 @@ Exit codes: 0 = success, 1 = mismatches, 2 = import error or banned patterns.
452486
- \`additionalProperties\` — stripped
453487
- \`enum\` → normalised to \`anyOf\` with \`const\` entries
454488
- \`allOf\` → flattened into merged object
489+
- \`anyOf\` / \`oneOf\` → structurally identical variants deduplicated
490+
491+
The verifier deduplicates \`anyOf\` variants, so if the same error type
492+
(e.g. \`UncaughtError\`) appears multiple times in a composed error union,
493+
**reuse the same Python class** — do NOT create \`UncaughtErrorDuplicate\`
494+
or similar copies. Use a plain union (not discriminated) when the same
495+
discriminator value appears more than once.
455496
456497
Use \`bytes\` for Uint8Array fields. The verifier handles the rest.
457498
@@ -721,6 +762,27 @@ actual models are never tested through the schema map. After refactoring,
721762
Many files import \`StringConstraints\`, \`ConfigDict\`, etc. without using them.
722763
Clean up.
723764
765+
### 7. Duplicate/Triplicate error classes
766+
Classes like \`UncaughtErrorDuplicate\`, \`NotFoundErrorDuplicate\`,
767+
\`UncaughtErrorTriplicate\` exist because the same error code appears
768+
multiple times in composed \`anyOf\` unions, and Pydantic discriminated
769+
unions require unique types. The verifier now **deduplicates structurally
770+
identical anyOf variants**, so you can reuse the same error class.
771+
**Delete all \`*Duplicate\` and \`*Triplicate\` classes.** Use the original
772+
class in a plain union (without \`Field(discriminator=...)\`) when the same
773+
discriminator value appears more than once.
774+
775+
### 8. Duplicate field declarations
776+
Some classes have the same field declared twice (e.g. \`extras: Foo\` on two
777+
consecutive lines). Python silently shadows the first. The verifier now
778+
catches this. **Remove duplicate field declarations.**
779+
780+
### 9. Data type names not from TypeScript source
781+
Top-level schema names like \`ExitInfo\`, \`MonitorResponse\`, \`ShellOutput\`
782+
from the TypeScript source don't appear — everything has path-derived names.
783+
Check \`naming_hints.json\` → \`tsExportNames\` for pre-extracted names per
784+
service, and \`tsSharedExportNames\` for shared lib/ types.
785+
724786
725787
## File access scope
726788
@@ -739,25 +801,35 @@ Python is \`./verify\`. Do NOT write or run Python scripts.
739801
740802
## How to approach the refactoring
741803
742-
### Phase 1: Study the TypeScript source to build a naming map
804+
### Phase 1: Study naming_hints.json and TypeScript source
805+
806+
Before changing any Python code:
743807
744-
Before changing any Python code, read the TypeScript source systematically:
808+
1. **Read \`naming_hints.json\`** — it contains:
809+
- \`errorCodeToClassName\`: error code → Python class name mapping
810+
- \`kindValueToClassName\`: \`$kind\` value → class name prefix
811+
- \`tsExportNames\`: per-service list of TypeBox schema names from TS source
812+
(e.g. \`"shellExec": ["ExitInfo", "OutputChunk", "MonitorResponse"]\`)
813+
- \`tsSharedExportNames\`: shared lib/ TypeBox schema names
814+
(e.g. \`"lib/fs": ["FilesystemError"]\`)
745815
746-
1. Read \`${opts.serverSrcPath}/index.ts\` to get the service list.
816+
**Use these names** to rename Python classes. The \`tsExportNames\` entries
817+
are the authoritative TypeScript names with "Schema" suffix already removed.
747818
748-
2. For each service, read its \`schemas.ts\` and \`index.ts\`. Build a mental
749-
map of:
750-
- **Named exports**: \`export const ExitInfo = Type.Object({...})\` →
751-
the Python class should be called \`ExitInfo\`
819+
2. Read \`${opts.serverSrcPath}/index.ts\` to get the service list.
820+
821+
3. For each service, read its \`schemas.ts\` and \`index.ts\`. Cross-reference
822+
with \`tsExportNames[serviceName]\` to confirm which TS names map to which
823+
Python classes. Look for:
824+
- **Named exports**: match to entries in \`tsExportNames\`
752825
- **Union variant names**: if TS has \`const MonitorResponse = Type.Union([
753826
NotStartedState, RunningState, FinishedState])\`, those variant names
754827
should appear in Python
755-
- **Shared types in lib/**: \`lib/fs/errors.ts\`, \`lib/shell/schemas.ts\`
756-
— these define types used across many services
828+
- **Shared types in lib/**: match to entries in \`tsSharedExportNames\`
757829
758-
3. Read the shared \`lib/\` directories:
830+
4. Read the shared \`lib/\` directories:
759831
- \`${opts.serverSrcPath}/../lib/\` or \`${opts.serverSrcPath}/lib/\` if it exists
760-
- Note which error types and data types are shared
832+
- Cross-reference with \`tsSharedExportNames\`
761833
762834
### Phase 2: Refactor shared types
763835
@@ -806,8 +878,15 @@ For **each** service file:
806878
807879
### Phase 4: Rebuild _schema_map.py and verify
808880
809-
14. **Rewrite \`_schema_map.py\`** to import TypeAdapters from the refactored
810-
service modules. Do NOT use monkey-patched adapters or raw dicts.
881+
14. **Rewrite \`_schema_map.py\`** to be a simple import-and-assemble module:
882+
- Import TypeAdapter instances from each service module
883+
- Assemble them into the \`SCHEMA_MAP\` dict
884+
- Do NOT load \`schema.json\` at runtime
885+
- Do NOT use AST parsing, \`inspect\`, or runtime introspection
886+
- Do NOT monkey-patch \`.json_schema\` on any adapter
887+
- Do NOT use \`_bind_reference_schemas\`, \`_load_reference_services\`,
888+
\`frozen_schema\`, \`copy.deepcopy\`, or any schema override mechanism
889+
- The \`_schema_map.py\` should be ~200 lines of straightforward imports
811890
812891
15. **Update \`__init__.py\`** if any service class names changed.
813892
@@ -855,6 +934,25 @@ Exit 0 = pass. Must pass when you're done.
855934
- Don't skip services. Every service file must be reviewed and improved.
856935
- Don't break imports. If you move a class from \`service.py\` to
857936
\`_errors.py\`, update every file that used the local definition.
937+
938+
### Run 10 failures (most recent):
939+
940+
- **\`*Duplicate\` / \`*Triplicate\` error classes** — \`UncaughtErrorDuplicate\`,
941+
\`NotFoundErrorDuplicate\`, etc. These are BANNED. The verifier now
942+
deduplicates \`anyOf\` variants, so just reuse the same class. If the same
943+
discriminator value appears more than once in a union, use a plain union
944+
(no \`Field(discriminator=...)\`).
945+
946+
- **Monkey-patched \`_schema_map.py\`** — \`_bind_reference_schemas()\` loaded
947+
\`schema.json\` and replaced each adapter's \`.json_schema\` with a lambda.
948+
This is BANNED. Write a simple import-and-assemble schema map.
949+
950+
- **Duplicate field declarations** — \`ParseError\` had \`extras: ParseErrorExtras\`
951+
on two consecutive lines. This is now caught by the verifier.
952+
953+
- **TS export names still missing** — Classes were still named with path-derived
954+
mechanical names instead of \`ExitInfo\`, \`MonitorResponse\`, etc. Use
955+
\`naming_hints.json\` → \`tsExportNames\` for the correct names.
858956
`.trim();
859957
}
860958

0 commit comments

Comments
 (0)