Harden for run 4: ban VariantN names, RiverTypeAdapter, standard error redefs; add allOf flattening to verifier

Author: Monkatraz

codegen-llm/src/prompts.ts

@@ -65,27 +65,44 @@ If ANY are found, verification fails immediately and you must rewrite.
 - \`SchemaAdapter\` or \`make_schema_adapter()\` wrapper classes
 - \`create_model()\` from pydantic — no dynamic model creation
 - Any "helper" or "utility" that builds models from schema dicts at runtime
-
-**Banned naming patterns:**
-- \`Input2\`, \`Output2\`, \`Init2\` — meaningless suffixed names
-- \`ErrorVariant1\`, \`ErrorVariant2\`, ... — numbered error classes
+- \`RiverTypeAdapter\` or any custom TypeAdapter subclass — use plain
+  \`TypeAdapter\` only.  Do NOT override \`json_schema()\` in any way.
+- \`schema_override_json\`, \`schema_override\`, or \`_schema_json\` — do NOT
+  embed or cache raw JSON schemas to return from \`json_schema()\`
+- Redefining \`UncaughtError\`, \`UnexpectedDisconnectError\`,
+  \`InvalidRequestError\`, or \`CancelError\` outside of \`_errors.py\` — these
+  standard River errors must be defined ONCE and imported everywhere
+
+**Banned naming patterns (the verifier enforces these with a regex):**
+- \`Variant\\d+\` anywhere in a class name — \`ErrorsVariant1\`, \`OutputVariant2\`,
+  \`AgentExecErrorsVariant3\`, \`ConnectErrorsVariant9\` are ALL banned
+- \`Input2\`, \`Output2\`, \`Init2\`, \`Errors3\` — numbered type suffixes
 - \`OutputVariant1Variant2\` — nested numbering
 - \`Input2ArtifactServicesItemDevelopmentRunVariant1\` — path-based names
   derived from JSON Schema structure
 - Any class name that a developer cannot understand without looking at
   the schema
 
+The verifier scans every class definition with a regex.  If ANY class
+name contains \`Variant\` followed by a digit, verification fails.
+Name error classes after their \`code\` literal, \`$kind\` variants after
+their kind value, and input/output types after the procedure or TS schema.
+
 **Required:**
 - Every Input, Output, and Error type MUST be a concrete \`BaseModel\` subclass
   with explicitly declared, typed fields
 - \`TypeAdapter(MyModel).json_schema()\` must produce correct schemas through
-  Pydantic's own schema generation — NOT through a hardcoded override
+  Pydantic's own native schema generation — NOT through hardcoded overrides
+  or JSON embedding
 - Every class MUST have a meaningful name derived from reading the TypeScript
   source code.  Examples:
   - Error with \`code: Literal['NOT_FOUND']\` → \`NotFoundError\`
   - Error with \`code: Literal['DISK_QUOTA_EXCEEDED']\` → \`DiskQuotaExceededError\`
   - Output with \`$kind: 'finished'\` → \`FinishedOutput\` or \`ExitInfo\` (from TS)
   - A ping procedure's output → \`PingOutput\`, not \`Output2\`
+- The four standard River error classes (UncaughtError, UnexpectedDisconnectError,
+  InvalidRequestError, CancelError) must be defined ONLY in \`_errors.py\` and
+  imported from there in every service module.  Do NOT redefine them.
 
 
 ## File access scope
@@ -442,6 +459,17 @@ utility functions, schema helpers, or dynamic class factories.
 
 8. **Intersections.** For \`Type.Intersect([A, B])\`, flatten all properties
    into a single BaseModel. Do NOT try to represent \`allOf\` in Pydantic.
+   The verifier normalises \`allOf\` by merging schemas, so a flat model
+   is correct.  Do NOT subclass TypeAdapter or embed raw JSON to handle
+   \`allOf\` — just merge all properties into one model.
+
+9. **Error deduplication within a service.** Many procedures in a service
+   share the same error types (e.g. all filesystem operations share
+   \`NotFoundError\`, \`PermissionDeniedError\`, etc.).  Define each unique
+   error class ONCE at the top of the service file, then reference it in
+   every procedure's error union.  Do NOT create separate copies like
+   \`ReadNotFoundError\`, \`WriteNotFoundError\`, \`MkdirNotFoundError\` — if
+   they have the same \`code\` literal and fields, they are the same class.
 
 
 ### The _schema_map.py module
@@ -519,6 +547,17 @@ Repeat until verification passes.**
 Because of these normalisations, use \`bytes\` for Uint8Array fields and your
 preferred Literal style for string unions. The verifier handles the rest.
 
+### How the verifier handles allOf (intersections)
+
+When comparing schemas with \`allOf\`, the verifier flattens/merges the
+\`allOf\` entries into a single object schema, then compares.  This means
+if you flatten an \`allOf\` into a single BaseModel (as recommended), the
+schemas will match.  You do NOT need to make Pydantic produce \`allOf\` —
+the verifier normalises both sides.
+
+Do NOT create custom TypeAdapter subclasses or embed raw JSON schemas
+to handle \`allOf\` cases.  Just flatten the properties into one model.
+
 
 ## How to approach this
 
@@ -540,10 +579,29 @@ This is a reasonable way to handle 50+ services efficiently.
   within a service (e.g. filesystem errors) should be defined ONCE at the
   top of the service file and reused.
 - Shared error types across ALL services (the four standard River errors)
-  must come from \`_errors.py\`.
+  must come from \`_errors.py\` — do NOT redefine them locally.
 
 If your final output still has numbered names like \`ErrorVariant1\`,
-\`Input2\`, \`OutputVariant1Variant2\`, it will be **discarded**.
+\`Input2\`, \`OutputVariant1Variant2\`, **the verifier will reject it**.
+The verifier enforces this with a regex — any class name containing
+\`Variant\` followed by a digit will fail.
+
+### Critical: the verifier WILL catch mechanical names
+
+Previous attempts failed because a scaffolding script generated all files
+with numbered names (e.g. \`ReadErrorsVariant1\` through \`ReadErrorsVariant16\`)
+and then never renamed them using the TypeScript source.
+
+The verifier now enforces:
+- **No \`Variant\\d+\` in any class name** (regex enforced)
+- **No redefinition of standard River errors** outside \`_errors.py\`
+- **No \`RiverTypeAdapter\` or \`schema_override\` patterns**
+
+If you write a scaffolding script, it MUST produce clean names from the
+start.  The easiest way: for error classes, read the \`code\` literal from
+the JSON Schema \`const\` field and convert it to PascalCase + "Error"
+(e.g. \`NOT_FOUND\` → \`NotFoundError\`).  For \`$kind\` variants, use the
+kind value.  For input/output, use \`<ProcedureName>Input/Output\`.
 
 ### Where names come from
 

codegen-llm/src/verify-script.ts

@@ -51,8 +51,31 @@ _BANNED_PATTERNS: list[tuple[str, str]] = [
     ('SchemaAdapter', 'Do not use SchemaAdapter wrappers — use TypeAdapter directly'),
     ('make_schema_adapter', 'Do not use make_schema_adapter() — use TypeAdapter directly'),
     ('create_model(', 'Do not use create_model() — define BaseModel classes statically'),
+    ('schema_override_json', 'Do not use schema_override_json — models must produce correct schemas natively'),
+    ('schema_override', 'Do not override json_schema() output — models must produce correct schemas natively'),
+    ('_schema_json', 'Do not cache/embed raw JSON schemas — models must produce correct schemas natively'),
+    ('RiverTypeAdapter', 'Do not subclass TypeAdapter — use TypeAdapter directly with correct models'),
+    ('json.loads(self._', 'Do not return embedded JSON from json_schema() — fix the model instead'),
 ]
 
+# Standard River error class names that must ONLY be defined in _errors.py.
+_STANDARD_ERROR_CLASSES = frozenset({
+    'UncaughtError', 'UnexpectedDisconnectError', 'InvalidRequestError', 'CancelError',
+})
+
+# Regex for banned naming: class names ending in Variant + digits, or
+# names like Input2, Output2, ErrorsVariant3, etc.
+_BANNED_NAME_RE = re.compile(
+    r'^class\\s+'
+    r'('
+    r'\\w*Variant\\d+\\w*'        # any name with VariantN in it
+    r'|\\w*Errors?Variant\\d+'   # ErrorsVariant1, ErrorVariant2, ...
+    r'|(?:Input|Output|Init|Errors?)\\d+'  # Input2, Output3, Errors4
+    r')'
+    r'\\s*\\(',
+    re.MULTILINE,
+)
+
 
 def check_code_quality(generated_dir: Path) -> list[str]:
     """Scan generated Python files for banned patterns."""
@@ -68,6 +91,27 @@ def check_code_quality(generated_dir: Path) -> list[str]:
         for pattern, msg in _BANNED_PATTERNS:
             if pattern in content:
                 errors.append(f'[{rel}] BANNED: {msg} (found \\"{pattern}\\")')
+
+        # Check for numbered variant names
+        for m in _BANNED_NAME_RE.finditer(content):
+            class_name = m.group(1)
+            errors.append(
+                f'[{rel}] BANNED NAME: "{class_name}" — class names must be '
+                f'meaningful, not numbered. Name error classes after their '
+                f'code literal (e.g. NotFoundError), $kind variants after '
+                f'their kind value (e.g. FinishedOutput), and types after '
+                f'their TypeScript schema name.'
+            )
+
+        # Check for standard error classes redefined outside _errors.py
+        if rel.name != '_errors.py':
+            for line in content.splitlines():
+                class_match = re.match(r'^class\\s+(\\w+)\\s*\\(', line)
+                if class_match and class_match.group(1) in _STANDARD_ERROR_CLASSES:
+                    errors.append(
+                        f'[{rel}] BANNED: class {class_match.group(1)} must not '
+                        f'be redefined — import it from _errors.py instead'
+                    )
     return errors
 
 
@@ -276,6 +320,69 @@ def _strip_null_variant(node: Any) -> Any:
     return node
 
 
+def _flatten_allof(node: Any) -> Any:
+    """
+    Flatten allOf into a single merged object schema.
+
+    TypeBox Type.Intersect produces allOf in JSON Schema.  Pydantic generates
+    a flat object with all properties merged.  Normalise both to the merged
+    form so they compare equal.
+    """
+    if isinstance(node, dict):
+        out = {}
+        for k, v in node.items():
+            out[k] = _flatten_allof(v)
+
+        if 'allOf' in out and isinstance(out['allOf'], list):
+            merged_props: dict[str, Any] = {}
+            merged_required: list[str] = []
+            merged_pattern_props: dict[str, Any] = {}
+            remaining: dict[str, Any] = {}
+
+            for sub in out['allOf']:
+                if not isinstance(sub, dict):
+                    continue
+                if 'properties' in sub:
+                    merged_props.update(sub['properties'])
+                if 'patternProperties' in sub:
+                    merged_pattern_props.update(sub['patternProperties'])
+                if 'required' in sub and isinstance(sub['required'], list):
+                    merged_required.extend(sub['required'])
+                for sk, sv in sub.items():
+                    if sk not in ('properties', 'patternProperties', 'required', 'type'):
+                        remaining[sk] = sv
+
+            # Carry over keys from the parent that aren't allOf
+            for pk, pv in out.items():
+                if pk == 'allOf':
+                    continue
+                if pk == 'properties':
+                    merged_props.update(pv)
+                elif pk == 'patternProperties':
+                    merged_pattern_props.update(pv)
+                elif pk == 'required' and isinstance(pv, list):
+                    merged_required.extend(pv)
+                elif pk != 'type':
+                    remaining[pk] = pv
+
+            result: dict[str, Any] = {'type': 'object'}
+            if merged_props:
+                result['properties'] = merged_props
+            if merged_pattern_props:
+                result['patternProperties'] = merged_pattern_props
+            if merged_required:
+                result['required'] = list(dict.fromkeys(merged_required))
+            result.update(remaining)
+            return result
+
+        return out
+
+    if isinstance(node, list):
+        return [_flatten_allof(item) for item in node]
+
+    return node
+
+
 def _sort_canonical(node: Any) -> Any:
     """Sort dict keys and union variant lists for stable comparison."""
     if isinstance(node, dict):
@@ -307,6 +414,7 @@ def prepare(schema: Any) -> Any:
     s = _normalise_enum_to_anyof(s)
     s = _normalise_nullable(s)
     s = _strip_null_variant(s)
+    s = _flatten_allof(s)
     s = _sort_canonical(s)
     return s