A space type declaration is a Lexicon document whose `main` definition is
`"type": "space"` rather than `"type": "record"`. The generator did not know
about it in either direction: a space-type Lexicon emitted nothing and reported
nothing, which reads as "nothing to generate" rather than "not supported", and
there was no way to declare one from C# at all.

- JSON → C#: a `space` def emits a static holder (com.atmoboards.forum →
  ForumSpace) with the Nsid constant, a SpaceTypeDeclaration built from the
  schema, and Key/Name/LocalizedNames/Collections forwarders onto it. key,
  name, and collections are `required` on that model, so a declaration missing
  one still emits code that compiles and warns which field was substituted for.
- C# → JSON: `lexicon` emits a `space` def for every static
  SpaceTypeDeclaration an assembly exposes, taking the NSID from a sibling
  Nsid (or SpaceType) constant — the shape `csharp` generates, so a
  declaration round-trips. LexiconEmitter gains a Warnings list for a
  declaration with no NSID to attribute it to, and for two types claiming the
  same definition.
- diff: compares the declaration. Adding a collection is reported and
  non-breaking (it widens every bare space: grant already issued, since the
  default collection set is resolved at grant-evaluation time); removing one
  is breaking. Key changes are breaking, name and localizations are not.
- An unrecognized definition type is now a WARN naming the NSID and the type.
  query/procedure/subscription and the scalar defs stay silent — they are
  legitimate input that emits no standalone C#.

Adds LexiconSchema.Name/LocalizedNames/Collections, a "space" case to
TypeMapper.DefToClassName, and 17 tests across the three directions.

claude-bot referenced this pull request

2026-08-20 21:29:10 +00:00

atproto-lexgen does not understand "type": "space" Lexicon definitions #92

claude-bot commented

2026-08-20 21:29:12 +00:00

Author

Collaborator

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/323); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-96/run-323.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-08-20 21:31:18 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Solid implementation of space Lexicon definition support in both directions of atproto-lexgen, plus diffing.

JSON → C# (CSharpEmitter.EmitSpaceType): emits a static holder around SpaceTypeDeclaration with sensible fallback/warning behavior when key/name/collections are missing (matches the required members on the SDK's SpaceTypeDeclaration, verified in src/ATProtoNet/Spaces/SpaceTypeDeclaration.cs). The unknown-def-type warning correctly special-cases empty type vs an unrecognized one, and LexiconSchema.Type defaults to "" (not null), so no NRE risk there.
C# → JSON (LexiconEmitter.EmitSpaceDeclarations/GetStaticSpaceDeclarations): the space scan runs before the abstract/interface/enum filter, which is necessary since the generated holder classes are implicitly abstract sealed. NSID attribution via a sibling Nsid/SpaceType constant, with a .Contains('.') sanity check, is a reasonable heuristic. The def-collision guard (AddDefinition) deterministically keeps the first-seen definition and warns on the rest, addressing the same-NSID collision case the changelog calls out.
Diffing (LexiconDiffer.CompareSpaceDeclaration): correctly gated behind the existing type-equality check and record/space branches, so it can't be reached when types differ or after the record-specific comparison. Breaking/non-breaking classification (key = breaking, collection removal = breaking, name/localization/collection-addition = non-breaking) matches the documented "grant evaluated at read time" rationale, and tests cover each branch including type-swap-to-record.

Code, docs (docs/lexicon-codegen.md, docs/spaces.md), CHANGELOG, and the new/updated test suites (CSharpEmitterTests, new LexiconEmitterTests, LexiconDifferTests) are all internally consistent — the round-trip fixtures in the new test file are explicitly shaped to match what the csharp emitter produces, which is a nice touch for catching drift between the two directions.

One minor, non-blocking observation: EmitSpaceType warns whenever collections resolves to an empty list, whether the schema omitted the field or the author explicitly declared "collections": [] to mean "no default set." Not a bug, just a potential source of an unavoidable warning for an intentionally collection-less space type — worth knowing about but not worth blocking on.

⏱ 1m 50s · 6 turns · tokens: 48.1k in / 9.6k out (+361.1k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Solid implementation of `space` Lexicon definition support in both directions of `atproto-lexgen`, plus diffing. - **JSON → C#** (`CSharpEmitter.EmitSpaceType`): emits a static holder around `SpaceTypeDeclaration` with sensible fallback/warning behavior when `key`/`name`/`collections` are missing (matches the `required` members on the SDK's `SpaceTypeDeclaration`, verified in `src/ATProtoNet/Spaces/SpaceTypeDeclaration.cs`). The unknown-def-type warning correctly special-cases empty `type` vs an unrecognized one, and `LexiconSchema.Type` defaults to `""` (not null), so no NRE risk there. - **C# → JSON** (`LexiconEmitter.EmitSpaceDeclarations`/`GetStaticSpaceDeclarations`): the space scan runs before the abstract/interface/enum filter, which is necessary since the generated holder classes are implicitly `abstract sealed`. NSID attribution via a sibling `Nsid`/`SpaceType` constant, with a `.Contains('.')` sanity check, is a reasonable heuristic. The def-collision guard (`AddDefinition`) deterministically keeps the first-seen definition and warns on the rest, addressing the same-NSID collision case the changelog calls out. - **Diffing** (`LexiconDiffer.CompareSpaceDeclaration`): correctly gated behind the existing type-equality check and record/space branches, so it can't be reached when types differ or after the record-specific comparison. Breaking/non-breaking classification (key = breaking, collection removal = breaking, name/localization/collection-addition = non-breaking) matches the documented "grant evaluated at read time" rationale, and tests cover each branch including type-swap-to-record. Code, docs (`docs/lexicon-codegen.md`, `docs/spaces.md`), CHANGELOG, and the new/updated test suites (`CSharpEmitterTests`, new `LexiconEmitterTests`, `LexiconDifferTests`) are all internally consistent — the round-trip fixtures in the new test file are explicitly shaped to match what the `csharp` emitter produces, which is a nice touch for catching drift between the two directions. One minor, non-blocking observation: `EmitSpaceType` warns whenever `collections` resolves to an empty list, whether the schema omitted the field or the author explicitly declared `"collections": []` to mean "no default set." Not a bug, just a potential source of an unavoidable warning for an intentionally collection-less space type — worth knowing about but not worth blocking on. --- ⏱ 1m 50s · 6 turns · tokens: 48.1k in / 9.6k out (+361.1k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-96/run-323.md)