lexgen csharp: codegen defects found generating exchange.recipe.* (real-world third-party lexicon) #45

New issue

Closed

opened 2026-07-05 17:22:06 +00:00 by Grandiras · 2 comments

Grandiras commented

2026-07-05 17:22:06 +00:00

Owner

Dogfooding atproto-lexgen csharp against the published recipe.exchange lexicons (exchange.recipe.recipe/.collection/.profile/.defs) for the Mise appview surfaced these defects in the generated C#:

Unbalanced braces — Recipe.g.cs ends with a stray extra } (file does not compile).
CS0542 — the image blob property inside the image object def generates public required BlobRef Image inside class Image (member named same as enclosing type; compile error). Needs a rename strategy (e.g. Blob or ImageBlob).
BlobRef unqualified — no using ATProtoNet;/qualification for BlobRef, so the file doesn't resolve it.
Non-nullable JsonElement fallbacks — optional union fields and inline object schemas (e.g. attribution, nutrition) generate public JsonElement X { get; init; }. Serializing default(JsonElement) (ValueKind.Undefined) throws — should at minimum be JsonElement?, ideally a generated union base + [JsonPolymorphic] variants and a nested class for inline objects.
Cross-namespace refs land in the consumer namespace — app.bsky.embed.defs#aspectRatio generates Mise.Core.Lexicon.App.Bsky.Embed.AspectRatio (nonexistent) instead of mapping to the SDK's existing app.bsky model types.
Records don't extend AtProtoRecord — generated record classes re-declare $type/createdAt instead of subclassing the SDK base, so they miss the SDK's record ergonomics.
(nice-to-have) Token families (knownValues sets like cookingMethod*) generate ~100 separate static classes; grouping per family (one static class with const members) would be far more usable.

Repro: atproto-lexgen csharp --input <the four exchange.recipe jsons> --output out --namespace Mise.Core.Lexicon. Mise is hand-maintaining patched copies for now (see Mise repo, src/Mise.Core/Lexicon/Exchange/Recipe/) — that folder is the reference for what the generator ideally emits.

Dogfooding `atproto-lexgen csharp` against the published recipe.exchange lexicons (exchange.recipe.recipe/.collection/.profile/.defs) for the Mise appview surfaced these defects in the generated C#: 1. **Unbalanced braces** — `Recipe.g.cs` ends with a stray extra `}` (file does not compile). 2. **CS0542** — the `image` blob property inside the `image` object def generates `public required BlobRef Image` inside `class Image` (member named same as enclosing type; compile error). Needs a rename strategy (e.g. `Blob` or `ImageBlob`). 3. **`BlobRef` unqualified** — no `using ATProtoNet;`/qualification for `BlobRef`, so the file doesn't resolve it. 4. **Non-nullable `JsonElement` fallbacks** — optional `union` fields and inline `object` schemas (e.g. `attribution`, `nutrition`) generate `public JsonElement X { get; init; }`. Serializing `default(JsonElement)` (ValueKind.Undefined) throws — should at minimum be `JsonElement?`, ideally a generated union base + `[JsonPolymorphic]` variants and a nested class for inline objects. 5. **Cross-namespace refs land in the consumer namespace** — `app.bsky.embed.defs#aspectRatio` generates `Mise.Core.Lexicon.App.Bsky.Embed.AspectRatio` (nonexistent) instead of mapping to the SDK's existing `app.bsky` model types. 6. **Records don't extend `AtProtoRecord`** — generated record classes re-declare `$type`/`createdAt` instead of subclassing the SDK base, so they miss the SDK's record ergonomics. 7. *(nice-to-have)* Token families (`knownValues` sets like cookingMethod*) generate ~100 separate static classes; grouping per family (one static class with const members) would be far more usable. Repro: `atproto-lexgen csharp --input <the four exchange.recipe jsons> --output out --namespace Mise.Core.Lexicon`. Mise is hand-maintaining patched copies for now (see Mise repo, src/Mise.Core/Lexicon/Exchange/Recipe/) — that folder is the reference for what the generator ideally emits.

claude-bot was assigned by Grandiras

2026-07-25 02:10:42 +00:00

claude-bot commented

2026-07-25 02:10:47 +00:00

Collaborator

🤖 On it — follow along in the action run; the reply will link the full session transcript.

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/36); the reply will link the full session transcript.

claude-bot referenced this issue from a commit

2026-07-25 02:36:56 +00:00

fix(lexgen): make generated C# compile and type unions/tokens (closes #45)

claude-bot referenced this issue

2026-07-25 02:37:23 +00:00

Claude: changes for issue #45 #60

claude-bot commented

2026-07-25 02:37:23 +00:00

Collaborator

Pushed branch claude/issue-45 (compare).

I fetched the real schemas rather than approximating them — resolved _lexicon.recipe.exchange → did:plc:4cx7ts7lqgjtsfquo53qo3sz → its PDS, and pulled the four com.atproto.lexicon.schema records — then fixed the generator against them.

All seven items

Unbalanced braces — the emitter appended } after a file-scoped namespace X;. Removed.
CS0542 — members that collide with their enclosing type, another member, or AtProtoRecord's Type/CreatedAt are renamed (image blob in def image → ImageBlob; also …Ref/…List/…Value). [JsonPropertyName] keeps the wire format identical. Illegal identifiers (2fa, kebab-case, keywords) are sanitized.
BlobRef unqualified — using ATProtoNet; / using ATProtoNet.Models; / using System.Text.Json; are emitted only when actually used, plus #nullable enable and using System.Collections.Generic;.
JsonElement fallbacks — optional members are always nullable, and unions/inline objects are now typed properly: a union of object defs emits abstract class AttributionUnion with [JsonPolymorphic(TypeDiscriminatorPropertyName = "$type")] + [JsonDerivedType] variants (the variants subclass it); a union of tokens (profileType, businessType in the profile schema) is string; inline objects become nested classes (RecipeRecord.NutritionInfo, List<Ingredient>). Overlapping unions can't both use inheritance, so those still degrade to JsonElement? — and now say so as a WARN.
Cross-namespace refs — SdkTypeMap maps well-known com.atproto.*/app.bsky.* defs onto the SDK's models (app.bsky.embed.defs#aspectRatio → ATProtoNet.Lexicon.App.Bsky.Embed.AspectRatio). Refs that resolve to nothing become JsonElement? + a WARN instead of a dangling type name; a unit test reflects over the whole table to prove every mapped type exists.
Records extend AtProtoRecord — public sealed class RecipeRecord : AtProtoRecord, createdAt inherited from the base.
Token families — one static class of constants per family plus an All list. The recipe defs document went from 101 generated classes to 9 (CookingMethod.Baking, Diet.Vegan, License.CreativeCommonsByNcSa, …); the family name is the longest prefix all members share, so license* groups under License rather than fragmenting.

Found while verifying (also fixed)

CS0101 — sibling documents share a C# namespace, so the two #appPassword defs under com.atproto.server collided; later ones are prefixed with their document name.
Name resolution — a generated namespace like Bsky.Generated.Chat.Bsky.Actor shadowed qualified references to Bsky.Generated.App.…; cross-namespace names are now rooted at global::.
Spurious "type" key — System.Text.Json does not inherit [JsonPropertyName] onto an override, so the pattern in RecordCollection's XML docs, README, and docs/ (public override string Type => … with no attribute) wrote both "type" and "$type" into every record. The attribute is now repeated on the override in generated code, all doc examples, and the test fixtures, with a regression test.
atproto NSID segment now cases as AtProto (matching the SDK layout and NsidToNamespace's own documented behaviour), and "type": "number" → double.

Verification

dotnet test tests/ATProtoNet.Tests/ — 1010 passing, including 15 new CSharpEmitterTests and the new AtProtoRecord serialization test.
Generated output for the four exchange.recipe schemas compiles against the SDK with TreatWarningsAsErrors=true, zero warnings, and round-trips: {"$type":"exchange.recipe.recipe",…,"attribution":{"$type":"exchange.recipe.defs#attributionWebsite",…}} deserializes back to AttributionWebsite. Zero WARN lines for this corpus.
Same for all 395 official atproto Lexicons → 103 files, 0 errors (20 WARNs, all honest reports of overlapping unions and def-name collisions).
docs/lexicon-codegen.md example output was regenerated from the tool, so it matches byte-for-byte; CHANGELOG updated under [Unreleased].

One thing left as-is: a Lexicon def used by two different unions can only inherit one base, so the second union stays JsonElement?. Modelling that would need generated wrapper types per union instead of inheritance — worth a follow-up issue if Mise hits it.

Sources: DNS lexicon authority, atproto lexicons, Lexicon spec

⏱ 26m 27s · 119 turns · tokens: 204.7k in / 106.2k out (+17077.1k cache-read)
📜 Full session transcript
→ Opened PR: #60

Pushed branch **`claude/issue-45`** ([compare](https://git.grandiras.net/Grandiras/ATProto.NET/compare/main...claude/issue-45)). I fetched the real schemas rather than approximating them — resolved `_lexicon.recipe.exchange` → `did:plc:4cx7ts7lqgjtsfquo53qo3sz` → its PDS, and pulled the four `com.atproto.lexicon.schema` records — then fixed the generator against them. ## All seven items 1. **Unbalanced braces** — the emitter appended `}` after a *file-scoped* `namespace X;`. Removed. 2. **CS0542** — members that collide with their enclosing type, another member, or `AtProtoRecord`'s `Type`/`CreatedAt` are renamed (`image` blob in def `image` → `ImageBlob`; also `…Ref`/`…List`/`…Value`). `[JsonPropertyName]` keeps the wire format identical. Illegal identifiers (`2fa`, `kebab-case`, keywords) are sanitized. 3. **`BlobRef` unqualified** — `using ATProtoNet;` / `using ATProtoNet.Models;` / `using System.Text.Json;` are emitted only when actually used, plus `#nullable enable` and `using System.Collections.Generic;`. 4. **`JsonElement` fallbacks** — optional members are always nullable, and unions/inline objects are now typed properly: a union of object defs emits `abstract class AttributionUnion` with `[JsonPolymorphic(TypeDiscriminatorPropertyName = "$type")]` + `[JsonDerivedType]` variants (the variants subclass it); a union of *tokens* (`profileType`, `businessType` in the profile schema) is `string`; inline objects become nested classes (`RecipeRecord.NutritionInfo`, `List<Ingredient>`). Overlapping unions can't both use inheritance, so those still degrade to `JsonElement?` — and now say so as a `WARN`. 5. **Cross-namespace refs** — `SdkTypeMap` maps well-known `com.atproto.*`/`app.bsky.*` defs onto the SDK's models (`app.bsky.embed.defs#aspectRatio` → `ATProtoNet.Lexicon.App.Bsky.Embed.AspectRatio`). Refs that resolve to nothing become `JsonElement?` + a `WARN` instead of a dangling type name; a unit test reflects over the whole table to prove every mapped type exists. 6. **Records extend `AtProtoRecord`** — `public sealed class RecipeRecord : AtProtoRecord`, `createdAt` inherited from the base. 7. **Token families** — one static class of constants per family plus an `All` list. The recipe `defs` document went from 101 generated classes to 9 (`CookingMethod.Baking`, `Diet.Vegan`, `License.CreativeCommonsByNcSa`, …); the family name is the longest prefix all members share, so `license*` groups under `License` rather than fragmenting. ## Found while verifying (also fixed) - **CS0101** — sibling documents share a C# namespace, so the two `#appPassword` defs under `com.atproto.server` collided; later ones are prefixed with their document name. - **Name resolution** — a generated namespace like `Bsky.Generated.Chat.Bsky.Actor` shadowed qualified references to `Bsky.Generated.App.…`; cross-namespace names are now rooted at `global::`. - **Spurious `"type"` key** — `System.Text.Json` does not inherit `[JsonPropertyName]` onto an override, so the pattern in `RecordCollection`'s XML docs, README, and `docs/` (`public override string Type => …` with no attribute) wrote **both** `"type"` and `"$type"` into every record. The attribute is now repeated on the override in generated code, all doc examples, and the test fixtures, with a regression test. - `atproto` NSID segment now cases as `AtProto` (matching the SDK layout and `NsidToNamespace`'s own documented behaviour), and `"type": "number"` → `double`. ## Verification - `dotnet test tests/ATProtoNet.Tests/` — **1010 passing**, including 15 new `CSharpEmitterTests` and the new `AtProtoRecord` serialization test. - Generated output for the four `exchange.recipe` schemas **compiles against the SDK with `TreatWarningsAsErrors=true`, zero warnings**, and round-trips: `{"$type":"exchange.recipe.recipe",…,"attribution":{"$type":"exchange.recipe.defs#attributionWebsite",…}}` deserializes back to `AttributionWebsite`. Zero `WARN` lines for this corpus. - Same for **all 395 official atproto Lexicons** → 103 files, 0 errors (20 `WARN`s, all honest reports of overlapping unions and def-name collisions). - `docs/lexicon-codegen.md` example output was regenerated from the tool, so it matches byte-for-byte; CHANGELOG updated under `[Unreleased]`. One thing left as-is: a Lexicon def used by two different unions can only inherit one base, so the second union stays `JsonElement?`. Modelling that would need generated wrapper types per union instead of inheritance — worth a follow-up issue if Mise hits it. Sources: [DNS lexicon authority](https://dns.google/resolve?name=_lexicon.recipe.exchange&type=TXT), [atproto lexicons](https://github.com/bluesky-social/atproto), [Lexicon spec](https://atproto.com/specs/lexicon) --- ⏱ 26m 27s · 119 turns · tokens: 204.7k in / 106.2k out (+17077.1k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-45/run-36.md) → Opened PR: https://git.grandiras.net/Grandiras/ATProto.NET/pulls/60

claude-bot referenced this issue

2026-07-25 14:35:20 +00:00

Claude: changes for issue #45 #60

Grandiras referenced this issue from a commit

2026-07-25 14:53:53 +00:00

Claude: changes for issue #45 (#60)

claude-bot referenced this issue

2026-07-25 14:55:20 +00:00

Claude: changes for issue #45 #60

Grandiras closed this issue

2026-07-25 15:00:40 +00:00

claude-bot referenced this issue