AtProtoRecord subclasses serialize a stray 'type' property — $type JsonPropertyName is not inherited through overrides #49

New issue

Closed

opened 2026-07-05 18:13:39 +00:00 by Grandiras · 2 comments

Grandiras commented

2026-07-05 18:13:39 +00:00

Owner

AtProtoRecord declares the discriminator as an abstract property with [JsonPropertyName("$type")] on the BASE member. System.Text.Json does not inherit attributes through property overrides, so every consumer subclass following the documented pattern (public override string Type => "com.example.todo.item"; — the README's headline example) serializes as "type": "…" (camelCase policy) instead of "$type": "…".

Observed in the wild with Mise: records created via RecordCollection<T>.CreateAsync carried BOTH "$type" (injected by the LexiconTypeRegistry path) and a stray "type" property — polluting records other apps read. Consumers not registered in the registry presumably get only "type" and produce records with no valid $type at all.

Fix options: make Type non-abstract (virtual with a backing NSID passed via ctor), or document that subclasses must re-declare [JsonPropertyName("$type")] on the override, or add a contract modifier that renames the member on AtProtoRecord-derived types. Mise works around it by re-declaring the attribute on each override.

`AtProtoRecord` declares the discriminator as an abstract property with `[JsonPropertyName("$type")]` on the BASE member. System.Text.Json does not inherit attributes through property overrides, so every consumer subclass following the documented pattern (`public override string Type => "com.example.todo.item";` — the README's headline example) serializes as `"type": "…"` (camelCase policy) instead of `"$type": "…"`. Observed in the wild with Mise: records created via `RecordCollection<T>.CreateAsync` carried BOTH `"$type"` (injected by the LexiconTypeRegistry path) and a stray `"type"` property — polluting records other apps read. Consumers not registered in the registry presumably get only `"type"` and produce records with no valid `$type` at all. Fix options: make `Type` non-abstract (virtual with a backing NSID passed via ctor), or document that subclasses must re-declare `[JsonPropertyName("$type")]` on the override, or add a contract modifier that renames the member on `AtProtoRecord`-derived types. Mise works around it by re-declaring the attribute on each override.

claude-bot was assigned by Grandiras

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

claude-bot commented

2026-07-25 02:10:16 +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/34); the reply will link the full session transcript.

claude-bot referenced this issue from a commit

2026-07-25 02:18:04 +00:00

fix: collapse AtProtoRecord discriminator to a single $type (closes #49)

claude-bot referenced this issue

2026-07-25 02:18:22 +00:00

Claude: changes for issue #49 #57

claude-bot commented

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

Collaborator

Fixed and pushed to claude/issue-49.

What I found

Reproduced it first — and the cause is a bit worse than the issue describes. It isn't just that the [JsonPropertyName("$type")] fails to reach the override; System.Text.Json also doesn't collapse the abstract base member and the override into one contract property. Both survive, so a record following the README pattern serializes as:

{"type":"com.example.todo.item","title":"x","$type":"com.example.todo.item","createdAt":"…"}

So the stray "type" and the correct "$type" both come from AtProtoRecord itself — the LexiconTypeRegistry path wasn't injecting anything. That also means the "consumers not in the registry get no valid $type" worry doesn't apply: they always got a valid $type plus the stray sibling.

The existing AtProtoRecord_SerializesWithDollarType test missed this because it only asserted Contains("$type") and used raw JsonSerializer.Serialize(todo) (no camelCase policy).

The fix

Went with the third option in the issue — a contract modifier — since it repairs already-compiled consumer code with no source change on their side.

AtProtoJsonDefaults.ApplyRecordTypeDiscriminator(JsonTypeInfo) (new public API) collapses every contract property mapping to the AtProtoRecord.Type virtual slot down to one, renamed $type and kept first in the object.
Wired into AtProtoJsonDefaults.Options and LexiconTypeRegistry.CreateOptions() — so RecordCollection<T>, RepoClient, and everything through XrpcClient are fixed automatically. Public so apps that serialize records with hand-built JsonSerializerOptions can add it to DefaultJsonTypeInfoResolver.Modifiers; documented in docs/custom-records.md.
Matches on the virtual slot (getter.GetBaseDefinition().DeclaringType) rather than the member name, so an unrelated new-shadowing Type member is left alone. Mise's workaround (re-declaring the attribute on the override) stays safe — it's deduped rather than colliding with the base member.

One implementation note worth flagging: my first cut of the slot check matched every property declared on AtProtoRecord, silently dropping createdAt. Caught by the repro, fixed, and covered by Serialize_PlainOverride_PreservesOtherRecordProperties.

Verified

Standalone repro program confirms all paths now emit a single leading $type: direct, boxed as object (the actual CreateRecordRequest.Record path), through the registry options, the attribute workaround, and unrelated Type-named members.
New tests/ATProtoNet.Tests/Serialization/RecordTypeDiscriminatorTests.cs (10 tests) + tightened the existing test to use AtProtoJsonDefaults.Options and assert the stray "type" is absent. I confirmed these are real regressions by short-circuiting the modifier — 6 fail without the fix, pass with it.
Full suite green: 1004 passed, 0 failed in both Debug and Release (dotnet test tests/ATProtoNet.Tests/ --configuration Release), and dotnet build -c Release succeeds with no new warnings on any touched file.

CHANGELOG.md updated under [Unreleased] (Added for the new public modifier, Fixed for the bug).

One thing I did not change: JsonSerializer.Serialize(record) with stock options still emits both properties, since the modifier lives in the options. Closing that completely would need option 1 (non-abstract Type with a ctor-supplied NSID), which trades away the compile-time error you currently get for a subclass that forgets to declare its type — I left that call to you rather than making it unilaterally.

⏱ 7m 57s · 45 turns · tokens: 69.2k in / 27.7k out (+2556.1k cache-read)
📜 Full session transcript
→ Opened PR: #57

Fixed and pushed to **`claude/issue-49`**. ## What I found Reproduced it first — and the cause is a bit worse than the issue describes. It isn't just that the `[JsonPropertyName("$type")]` fails to reach the override; System.Text.Json also **doesn't collapse the abstract base member and the override into one contract property**. Both survive, so a record following the README pattern serializes as: ```json {"type":"com.example.todo.item","title":"x","$type":"com.example.todo.item","createdAt":"…"} ``` So the stray `"type"` and the correct `"$type"` both come from `AtProtoRecord` itself — the `LexiconTypeRegistry` path wasn't injecting anything. That also means the "consumers not in the registry get no valid `$type`" worry doesn't apply: they always got a valid `$type` plus the stray sibling. The existing `AtProtoRecord_SerializesWithDollarType` test missed this because it only asserted `Contains("$type")` and used raw `JsonSerializer.Serialize(todo)` (no camelCase policy). ## The fix Went with the third option in the issue — a contract modifier — since it repairs already-compiled consumer code with no source change on their side. - **`AtProtoJsonDefaults.ApplyRecordTypeDiscriminator(JsonTypeInfo)`** (new public API) collapses every contract property mapping to the `AtProtoRecord.Type` virtual slot down to one, renamed `$type` and kept first in the object. - Wired into `AtProtoJsonDefaults.Options` and `LexiconTypeRegistry.CreateOptions()` — so `RecordCollection<T>`, `RepoClient`, and everything through `XrpcClient` are fixed automatically. Public so apps that serialize records with hand-built `JsonSerializerOptions` can add it to `DefaultJsonTypeInfoResolver.Modifiers`; documented in `docs/custom-records.md`. - Matches on the **virtual slot** (`getter.GetBaseDefinition().DeclaringType`) rather than the member name, so an unrelated `new`-shadowing `Type` member is left alone. Mise's workaround (re-declaring the attribute on the override) stays safe — it's deduped rather than colliding with the base member. One implementation note worth flagging: my first cut of the slot check matched *every* property declared on `AtProtoRecord`, silently dropping `createdAt`. Caught by the repro, fixed, and covered by `Serialize_PlainOverride_PreservesOtherRecordProperties`. ## Verified - Standalone repro program confirms all paths now emit a single leading `$type`: direct, boxed as `object` (the actual `CreateRecordRequest.Record` path), through the registry options, the attribute workaround, and unrelated `Type`-named members. - New `tests/ATProtoNet.Tests/Serialization/RecordTypeDiscriminatorTests.cs` (10 tests) + tightened the existing test to use `AtProtoJsonDefaults.Options` and assert the stray `"type"` is absent. I confirmed these are real regressions by short-circuiting the modifier — 6 fail without the fix, pass with it. - Full suite green: **1004 passed, 0 failed** in both Debug and Release (`dotnet test tests/ATProtoNet.Tests/ --configuration Release`), and `dotnet build -c Release` succeeds with no new warnings on any touched file. `CHANGELOG.md` updated under `[Unreleased]` (`Added` for the new public modifier, `Fixed` for the bug). One thing I did **not** change: `JsonSerializer.Serialize(record)` with stock options still emits both properties, since the modifier lives in the options. Closing that completely would need option 1 (non-abstract `Type` with a ctor-supplied NSID), which trades away the compile-time error you currently get for a subclass that forgets to declare its type — I left that call to you rather than making it unilaterally. --- ⏱ 7m 57s · 45 turns · tokens: 69.2k in / 27.7k out (+2556.1k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-49/run-34.md) → Opened PR: https://git.grandiras.net/Grandiras/ATProto.NET/pulls/57

claude-bot referenced this issue

2026-07-25 02:26:33 +00:00

Claude: changes for issue #49 #57

Grandiras referenced this issue from a commit

2026-07-25 02:27:51 +00:00

Claude: changes for issue #49 (#57)

Grandiras closed this issue

2026-07-25 02:27:57 +00:00

claude-bot referenced this issue from a commit

2026-07-25 14:39:15 +00:00

Merge branch 'main' into claude/issue-45

claude-bot referenced this issue