Grandiras/ATProto.NET

Fork 0

fix(lexicon)!: wire fidelity against upstream lexicons, enforced by a drift test #159

Merged

Grandiras merged 1 commit from issue-123-lexicon-fidelity into main

2026-09-25 00:14:38 +00:00

Grandiras commented

2026-09-25 00:12:11 +00:00

Owner

Closes #123.

The hand-written Lexicon models had drifted from upstream. This PR adds a drift test against a pinned upstream snapshot and fixes everything it found: the issue's checklist and the wire bugs listed in its comment.

How to review this

The drift test is tests/ATProtoNet.Tests/Lexicon/Upstream/:
- LexiconDriftTests.cs: the checks.
- LexiconDriftTests.Allowlists.cs: every deliberate difference, each with a reason. A test fails when an entry no longer matches anything, so entries cannot go stale.
- UpstreamLexicons.cs: the snapshot loader.
- SdkSource.cs: finds the XRPC call sites and NSID literals in src/.
- lexicons/: the snapshot, 404 files, vendored unchanged. It holds bluesky-social/atproto 2e583a4 (2026-09-24, current main) for app.bsky, chat.bsky, com.atproto and tools.ozone, plus the 8 site.standard.* schema records the standard.site authority publishes, including its two permission sets.
- README.md: the commit, the record CIDs, the fetch date, the licenses (MIT/Apache-2.0 and MIT) and what each check does.
Refreshing the snapshot: run tests/ATProtoNet.Tests/Lexicon/Upstream/refresh.sh <atproto commit> (needs curl, tar and jq), update the README tables from its output, and run the unit tests. Each failure is an upstream change: model it, or add a commented allow-list entry.
Behavioural fixes. These change what goes over the wire or how responses are read, so review them closely. Everything else only adds fields or retypes them. Regression tests are in tests/ATProtoNet.Tests/Lexicon/WireFidelityTests.cs, the Ozone tests and TypedBskyClientTests:
- DeactivateAccountAsync and UpdateAllReadAsync now send their JSON body, {} when every field is empty.
- MuteConvoAsync, UnmuteConvoAsync and UpdateReadAsync unwrap {convo}. AddReactionAsync and RemoveReactionAsync unwrap {message}. Their return types are unchanged.
- ExportAccountDataAsync streams JSON Lines instead of parsing them as JSON.
- SearchAccountsAsync is now a GET.
- queryStatuses replaces querySubjects.
- listConvos sends readState.
- searchPosts sends one tag parameter per tag.
- listNotifications no longer sends seenAt or priority.
Unions are in tests/ATProtoNet.Tests/Serialization/LexiconUnionTests.cs. The fixtures are upstream-shaped, and several are captured from public.api.bsky.app, which puts $type last.

Drift test

Check	What it asserts
`XrpcCalls_ExistUpstreamWithTheirHttpMethod`	Every XRPC call (190 call sites) names an upstream query sent as GET or procedure sent as POST.
`NsidLiterals_NameUpstreamDefs`	Every `nsid` / `nsid#def` string literal in `src/` names an upstream def. This caught the invented permission sets and `querySubjects`.
`LexiconModels_EachResolveToAnUpstreamObject`	Every Lexicon model maps to one upstream object, found in this order: its `$type`, its union discriminator, its name (`FooResponse` → `foo#output`, `FooRequest` → `foo#input`, `FooRecord` → record `foo`, otherwise a unique def `foo` in the namespace), or an explicit `DefOf` entry.
`LexiconModels_JsonPropertyNamesExistOnTheirUpstreamDef`	Every `[JsonPropertyName]` exists on that def.
`LexiconModels_DeclareEveryPropertyOfTheirUpstreamDef`	Every property of the def is modeled. This goes beyond the issue, so "missing fields on existing models" cannot regress.
`PermissionSetConstants_ArePublishedPermissionSets` / `PublishedPermissionSets_EachHaveAConstant`	`AtProtoScopes.PermissionSets` is exactly the published set.
`KnownValueConstants_MatchUpstream`	17 constants classes match their knownValues.
`Allowlists_AllStillMatch`	No allow-list entry is stale.

Conventions/LexiconJsonNameTests requires an explicit camelCase [JsonPropertyName] on every public Lexicon model property. It and the drift test replace 54 DTO-only serializer tests that only serialized a model to check its property names:

ConvoModelsTests and LabelerModelsTests: deleted.
StandardSiteModelTests: deleted. Its one client test moved to StandardSiteClientTests.
Parts of CommonModelsTests, SyncModelsTests (non-firehose only), OzoneClientTests and ChatActorClientTests.

Tests that cover converters, unions or data-model shapes (BlobRef, signed labels, identifiers) stay.

Fixes the drift test demanded

Wrong names:
- canConvo → canChat, plus Convo.
- readOnly → readState.
- associatedChat → typed Associated (ProfileAssociated) on all three profile views.
- ConvoView.Opened removed.
- AcceptConvoResponse is {rev} only.
- RecordViewDetail.BlobCids → Blobs (BlobViews).
Methods:
- tools.ozone.moderation.queryStatuses replaces the invented querySubjects. It takes all ~35 filters as a SubjectStatusFilter, Takendown and Appealed are booleans, and it returns SubjectStatuses.
- tools.ozone.signature.searchAccounts is a GET with values[]. AccountResult is replaced by AccountInfo (admin.defs#accountView).
Bodies and outputs:
- deactivateAccount: deleteAfter.
- updateAllRead: status in, updatedCount out.
- exportAccountData: JSONL, returned as an XrpcStreamResponse.
- The five wrapped chat outputs.
Deprecations:
- listNotifications seenAt is removed.
- ListNotificationsResponse.Priority, IsFallback and SyncClient.NotifyOfUpdateAsync are [Obsolete].
- The priority parameter is removed, because a parameter cannot carry [Obsolete] (see Flagged).
Permission sets: the 7 invented NSIDs are removed. ManageNotifications is fixed to authManageNotifications. Added: DeleteContent, FullChatClient, StandardSiteFull and StandardSiteSocial.
Label values:
- Added: Hide, Warn and Bot.
- [Obsolete]: Gore, NotAvailable (duplicates NoUnauthenticated) and ContentWarning, which is neither a global value nor one mod.bsky.app applies.
- Spam, Impersonation and Misleading stay, allow-listed as mod.bsky.app labels.
Missing fields: every field the drift test found on existing models, listed in the CHANGELOG Added section. It covers posts, profiles, preferences, notifications, graph, labeler, video, chat, com.atproto, site.standard and Ozone, including the queryEvents / querySets / listMembers filters and verifierDid.
Typed open unions, each with an Unknown* fallback:
- feed reason (ReasonRepost / ReasonPin) and skeleton reason;
- threadgate and postgate rules;
- RecordView.Record (EmbeddedRecordView: #viewRecord, the not-found, blocked and detached placeholders, and GeneratorView / ListView / LabelerView / StarterPackViewBasic);
- getRelationships entries;
- the 16 preference types;
- Ozone subject hosting and blob details;
- Ozone ModerationSubject gains messageRef / convoRef.
Typed references:
- ThreadgateView
- BlockedAuthor
- list views on ViewerState
- starter-pack Feeds
- labeler Creator
- ChatMemberView.Associated
- ListRecord.Labels
- TeamMember.Profile (ProfileViewDetailed)

Breaking changes (migration)

ProfileViewDetailed.AssociatedChat → Associated?.Chat.
CanConvo → CanChat; readOnly: → readState: "unread"; drop ConvoView.Opened and AcceptConvoResponse.Convo.
UpdateAllReadAsync returns UpdateAllReadResponse (awaits still compile; recompile).
ExportAccountDataAsync returns XrpcStreamResponse: read Content line by line and dispose it.
QuerySubjectsAsync / EnumerateSubjectsAsync / QuerySubjectsResponse → QueryStatusesAsync / EnumerateStatusesAsync(SubjectStatusFilter) / QueryStatusesResponse.
SearchAccountsAsync(IEnumerable<SigDetail>) → SearchAccountsAsync(IEnumerable<string> values); AccountResult → AccountInfo.
ListNotificationsAsync / EnumerateNotificationsAsync lose priority and seenAt and gain reasons; GetUnreadCountAsync loses priority.
New Lexicon parameters shift positional arguments: SearchPostsAsync(tags), GetFollowers/FollowsAsync(sort), GetListsAsync(purposes), MuteActorAsync, CreateReportAsync(modTool), DeactivateAccountAsync(deleteAfter), the QueryEventsAsync filters, ListMembersAsync and QuerySetsAsync. Name the arguments.
AtProtoScopes.PermissionSets: the invented constants are removed and ManageNotifications has a new value. Use DeleteContent, ManageNotifications, ManageModeration or FullApp, and recompile.
Unions and references that were JsonElement are typed. Relationship.Type / NotFoundActor.Type are removed, and the four record-embed views now write $type. Match on the variant types; an unknown variant's Raw is the old JsonElement.
Ozone: BlobCids → Blobs; the ModEventLabel / ModEventTag lists are required.

Verification

Solution build: 0 warnings, 0 errors.
Unit tests: 3,214 passed, 0 failed, 5 skipped (3,219 total), rebased onto main with #157.
The drift suite is 25 tests (8 facts and 17 knownValues rows). Its allow-lists hold:
- 58 DefOf mappings;
- 7 not-a-def models: data-model types, the PLC service entry, and the firehose base and removed events;
- 2 unknown properties: InfoEvent seq / time, #126;
- 10 omitted properties: 3 deprecated upstream, 7 group-chat fields for #128;
- 3 extra and 1 missing knownValues pattern (Ozone report reasons, #131).
Fixtures: getProfile, getAuthorFeed (reasonPin / reasonRepost, a quote embed) and getRelationships were captured from public.api.bsky.app on 2026-09-25. The standard.site permission sets were fetched from the authority's PDS.
Integration tests still compile. They are skipped locally.

Deferred and flagged

#126 (firehose): removing HandleEvent / TombstoneEvent and the CommitEvent.TooBig / Rebase / Blobs obsoletions are left to #126. They are allow-listed with #126, and so is InfoEvent inheriting seq / time.
#128 (group chats): ConvoView.kind / lastReaction, ChatMemberView.kind, MessageView.reactions / replyTo, MessageInput.replyTo, the typed log entries, and listConvos kind / lockStatus are union-typed group-chat features. They are allow-listed with #128. The scalar chat fields (allowGroupInvites, relatedProfiles, member viewer / createdAt / verification) are added here. Overlap with #127: JobStatus.failureCode and the four new job states are added here, as this issue lists them.
#131: ReportReasons lacks the granular tools.ozone.report.defs#reason* values. They are allow-listed as a prefix.
priority parameters are removed, not [Obsolete]: C# cannot obsolete a parameter, and an obsolete overload would have been ambiguous with the new signatures. Upstream ignores the parameter.
createSession.allowTakendown is on the internal CreateSessionRequest, but no public method sets it. Exposing it means touching the session methods #124 owns.
SpaceRecordView.Path (spaces) is a computed property with no [JsonIgnore], even after #157, so it is serialized as "path". The convention test allow-lists it; the one-line fix ([JsonIgnore]) is left to the spaces owner.
GetLabelerServicesResponse.Views stays JsonElement. LabelerView can derive from only one union base, and it now belongs to the record-embed union.
FeedReplyRef.Root / Parent stay JsonElement. The issue does not list them, and NotFoundPost / BlockedPost already belong to the thread union.

Edits outside this issue's area

#124's files, surgical:
- ServerClient.DeactivateAccountAsync (the body).
- ServerModels.cs: CreateSessionRequest.AllowTakendown, DescribeServerResponse.BlobUploadLimit and a new internal DeactivateAccountRequest.
- Auth/OAuth/AtProtoScopes.cs: only PermissionSets and the Include doc example.
#119 part 4 (merged as #157): no source edits. SyncModelsTests lost only its non-firehose DTO tests; #157's typed firehose tests are kept unchanged. TypedIdentifierGuardTests.Exceptions gained entries for the new string members and lost the QuerySubjectsAsync ones.

🤖 Generated with Claude Code

Closes #123. The hand-written Lexicon models had drifted from upstream. This PR adds a drift test against a pinned upstream snapshot and fixes everything it found: the issue's checklist and the wire bugs listed in its comment. ## How to review this - **The drift test** is `tests/ATProtoNet.Tests/Lexicon/Upstream/`: - `LexiconDriftTests.cs`: the checks. - `LexiconDriftTests.Allowlists.cs`: every deliberate difference, each with a reason. A test fails when an entry no longer matches anything, so entries cannot go stale. - `UpstreamLexicons.cs`: the snapshot loader. - `SdkSource.cs`: finds the XRPC call sites and NSID literals in `src/`. - `lexicons/`: the snapshot, 404 files, vendored unchanged. It holds bluesky-social/atproto `2e583a4` (2026-09-24, current `main`) for `app.bsky`, `chat.bsky`, `com.atproto` and `tools.ozone`, plus the 8 `site.standard.*` schema records the standard.site authority publishes, including its two permission sets. - `README.md`: the commit, the record CIDs, the fetch date, the licenses (MIT/Apache-2.0 and MIT) and what each check does. - **Refreshing the snapshot:** run `tests/ATProtoNet.Tests/Lexicon/Upstream/refresh.sh <atproto commit>` (needs curl, tar and jq), update the README tables from its output, and run the unit tests. Each failure is an upstream change: model it, or add a commented allow-list entry. - **Behavioural fixes.** These change what goes over the wire or how responses are read, so review them closely. Everything else only adds fields or retypes them. Regression tests are in `tests/ATProtoNet.Tests/Lexicon/WireFidelityTests.cs`, the Ozone tests and `TypedBskyClientTests`: - `DeactivateAccountAsync` and `UpdateAllReadAsync` now send their JSON body, `{}` when every field is empty. - `MuteConvoAsync`, `UnmuteConvoAsync` and `UpdateReadAsync` unwrap `{convo}`. `AddReactionAsync` and `RemoveReactionAsync` unwrap `{message}`. Their return types are unchanged. - `ExportAccountDataAsync` streams JSON Lines instead of parsing them as JSON. - `SearchAccountsAsync` is now a GET. - `queryStatuses` replaces `querySubjects`. - `listConvos` sends `readState`. - `searchPosts` sends one `tag` parameter per tag. - `listNotifications` no longer sends `seenAt` or `priority`. - **Unions** are in `tests/ATProtoNet.Tests/Serialization/LexiconUnionTests.cs`. The fixtures are upstream-shaped, and several are captured from `public.api.bsky.app`, which puts `$type` last. ## Drift test | Check | What it asserts | |---|---| | `XrpcCalls_ExistUpstreamWithTheirHttpMethod` | Every XRPC call (190 call sites) names an upstream query sent as GET or procedure sent as POST. | | `NsidLiterals_NameUpstreamDefs` | Every `nsid` / `nsid#def` string literal in `src/` names an upstream def. This caught the invented permission sets and `querySubjects`. | | `LexiconModels_EachResolveToAnUpstreamObject` | Every Lexicon model maps to one upstream object, found in this order: its `$type`, its union discriminator, its name (`FooResponse` → `foo#output`, `FooRequest` → `foo#input`, `FooRecord` → record `foo`, otherwise a unique def `foo` in the namespace), or an explicit `DefOf` entry. | | `LexiconModels_JsonPropertyNamesExistOnTheirUpstreamDef` | Every `[JsonPropertyName]` exists on that def. | | `LexiconModels_DeclareEveryPropertyOfTheirUpstreamDef` | Every property of the def is modeled. This goes beyond the issue, so "missing fields on existing models" cannot regress. | | `PermissionSetConstants_ArePublishedPermissionSets` / `PublishedPermissionSets_EachHaveAConstant` | `AtProtoScopes.PermissionSets` is exactly the published set. | | `KnownValueConstants_MatchUpstream` | 17 constants classes match their knownValues. | | `Allowlists_AllStillMatch` | No allow-list entry is stale. | `Conventions/LexiconJsonNameTests` requires an explicit camelCase `[JsonPropertyName]` on every public Lexicon model property. It and the drift test replace **54 DTO-only serializer tests** that only serialized a model to check its property names: - `ConvoModelsTests` and `LabelerModelsTests`: deleted. - `StandardSiteModelTests`: deleted. Its one client test moved to `StandardSiteClientTests`. - Parts of `CommonModelsTests`, `SyncModelsTests` (non-firehose only), `OzoneClientTests` and `ChatActorClientTests`. Tests that cover converters, unions or data-model shapes (`BlobRef`, signed labels, identifiers) stay. ## Fixes the drift test demanded - **Wrong names:** - `canConvo` → `canChat`, plus `Convo`. - `readOnly` → `readState`. - `associatedChat` → typed `Associated` (`ProfileAssociated`) on all three profile views. - `ConvoView.Opened` removed. - `AcceptConvoResponse` is `{rev}` only. - `RecordViewDetail.BlobCids` → `Blobs` (`BlobView`s). - **Methods:** - `tools.ozone.moderation.queryStatuses` replaces the invented `querySubjects`. It takes all ~35 filters as a `SubjectStatusFilter`, `Takendown` and `Appealed` are booleans, and it returns `SubjectStatuses`. - `tools.ozone.signature.searchAccounts` is a GET with `values[]`. `AccountResult` is replaced by `AccountInfo` (`admin.defs#accountView`). - **Bodies and outputs:** - `deactivateAccount`: `deleteAfter`. - `updateAllRead`: `status` in, `updatedCount` out. - `exportAccountData`: JSONL, returned as an `XrpcStreamResponse`. - The five wrapped chat outputs. - **Deprecations:** - `listNotifications` `seenAt` is removed. - `ListNotificationsResponse.Priority`, `IsFallback` and `SyncClient.NotifyOfUpdateAsync` are `[Obsolete]`. - The `priority` parameter is removed, because a parameter cannot carry `[Obsolete]` (see Flagged). - **Permission sets:** the 7 invented NSIDs are removed. `ManageNotifications` is fixed to `authManageNotifications`. Added: `DeleteContent`, `FullChatClient`, `StandardSiteFull` and `StandardSiteSocial`. - **Label values:** - Added: `Hide`, `Warn` and `Bot`. - `[Obsolete]`: `Gore`, `NotAvailable` (duplicates `NoUnauthenticated`) and `ContentWarning`, which is neither a global value nor one mod.bsky.app applies. - `Spam`, `Impersonation` and `Misleading` stay, allow-listed as mod.bsky.app labels. - **Missing fields:** every field the drift test found on existing models, listed in the CHANGELOG `Added` section. It covers posts, profiles, preferences, notifications, graph, labeler, video, chat, `com.atproto`, `site.standard` and Ozone, including the `queryEvents` / `querySets` / `listMembers` filters and `verifierDid`. - **Typed open unions,** each with an `Unknown*` fallback: - feed reason (`ReasonRepost` / `ReasonPin`) and skeleton reason; - threadgate and postgate rules; - `RecordView.Record` (`EmbeddedRecordView`: `#viewRecord`, the not-found, blocked and detached placeholders, and `GeneratorView` / `ListView` / `LabelerView` / `StarterPackViewBasic`); - `getRelationships` entries; - the 16 preference types; - Ozone subject hosting and blob details; - Ozone `ModerationSubject` gains `messageRef` / `convoRef`. - **Typed references:** - `ThreadgateView` - `BlockedAuthor` - list views on `ViewerState` - starter-pack `Feeds` - labeler `Creator` - `ChatMemberView.Associated` - `ListRecord.Labels` - `TeamMember.Profile` (`ProfileViewDetailed`) ## Breaking changes (migration) - `ProfileViewDetailed.AssociatedChat` → `Associated?.Chat`. - `CanConvo` → `CanChat`; `readOnly:` → `readState: "unread"`; drop `ConvoView.Opened` and `AcceptConvoResponse.Convo`. - `UpdateAllReadAsync` returns `UpdateAllReadResponse` (awaits still compile; recompile). - `ExportAccountDataAsync` returns `XrpcStreamResponse`: read `Content` line by line and dispose it. - `QuerySubjectsAsync` / `EnumerateSubjectsAsync` / `QuerySubjectsResponse` → `QueryStatusesAsync` / `EnumerateStatusesAsync(SubjectStatusFilter)` / `QueryStatusesResponse`. - `SearchAccountsAsync(IEnumerable<SigDetail>)` → `SearchAccountsAsync(IEnumerable<string> values)`; `AccountResult` → `AccountInfo`. - `ListNotificationsAsync` / `EnumerateNotificationsAsync` lose `priority` and `seenAt` and gain `reasons`; `GetUnreadCountAsync` loses `priority`. - New Lexicon parameters shift positional arguments: `SearchPostsAsync(tags)`, `GetFollowers/FollowsAsync(sort)`, `GetListsAsync(purposes)`, `MuteActorAsync`, `CreateReportAsync(modTool)`, `DeactivateAccountAsync(deleteAfter)`, the `QueryEventsAsync` filters, `ListMembersAsync` and `QuerySetsAsync`. Name the arguments. - `AtProtoScopes.PermissionSets`: the invented constants are removed and `ManageNotifications` has a new value. Use `DeleteContent`, `ManageNotifications`, `ManageModeration` or `FullApp`, and recompile. - Unions and references that were `JsonElement` are typed. `Relationship.Type` / `NotFoundActor.Type` are removed, and the four record-embed views now write `$type`. Match on the variant types; an unknown variant's `Raw` is the old `JsonElement`. - Ozone: `BlobCids` → `Blobs`; the `ModEventLabel` / `ModEventTag` lists are `required`. ## Verification - Solution build: **0 warnings, 0 errors**. - Unit tests: **3,214 passed, 0 failed, 5 skipped** (3,219 total), rebased onto `main` with #157. - The drift suite is 25 tests (8 facts and 17 knownValues rows). Its allow-lists hold: - 58 `DefOf` mappings; - 7 not-a-def models: data-model types, the PLC service entry, and the firehose base and removed events; - 2 unknown properties: `InfoEvent` `seq` / `time`, #126; - 10 omitted properties: 3 deprecated upstream, 7 group-chat fields for #128; - 3 extra and 1 missing knownValues pattern (Ozone report reasons, #131). - Fixtures: `getProfile`, `getAuthorFeed` (`reasonPin` / `reasonRepost`, a quote embed) and `getRelationships` were captured from `public.api.bsky.app` on 2026-09-25. The standard.site permission sets were fetched from the authority's PDS. - Integration tests still compile. They are skipped locally. ## Deferred and flagged - **#126 (firehose):** removing `HandleEvent` / `TombstoneEvent` and the `CommitEvent.TooBig` / `Rebase` / `Blobs` obsoletions are left to #126. They are allow-listed with `#126`, and so is `InfoEvent` inheriting `seq` / `time`. - **#128 (group chats):** `ConvoView.kind` / `lastReaction`, `ChatMemberView.kind`, `MessageView.reactions` / `replyTo`, `MessageInput.replyTo`, the typed log entries, and `listConvos` `kind` / `lockStatus` are union-typed group-chat features. They are allow-listed with `#128`. The scalar chat fields (`allowGroupInvites`, `relatedProfiles`, member `viewer` / `createdAt` / `verification`) are added here. **Overlap with #127:** `JobStatus.failureCode` and the four new job states are added here, as this issue lists them. - **#131:** `ReportReasons` lacks the granular `tools.ozone.report.defs#reason*` values. They are allow-listed as a prefix. - **`priority` parameters are removed, not `[Obsolete]`:** C# cannot obsolete a parameter, and an obsolete overload would have been ambiguous with the new signatures. Upstream ignores the parameter. - **`createSession.allowTakendown`** is on the internal `CreateSessionRequest`, but no public method sets it. Exposing it means touching the session methods #124 owns. - **`SpaceRecordView.Path`** (spaces) is a computed property with no `[JsonIgnore]`, even after #157, so it is serialized as `"path"`. The convention test allow-lists it; the one-line fix (`[JsonIgnore]`) is left to the spaces owner. - **`GetLabelerServicesResponse.Views`** stays `JsonElement`. `LabelerView` can derive from only one union base, and it now belongs to the record-embed union. - **`FeedReplyRef.Root` / `Parent`** stay `JsonElement`. The issue does not list them, and `NotFoundPost` / `BlockedPost` already belong to the thread union. ## Edits outside this issue's area - **#124's files, surgical:** - `ServerClient.DeactivateAccountAsync` (the body). - `ServerModels.cs`: `CreateSessionRequest.AllowTakendown`, `DescribeServerResponse.BlobUploadLimit` and a new internal `DeactivateAccountRequest`. - `Auth/OAuth/AtProtoScopes.cs`: only `PermissionSets` and the `Include` doc example. - **#119 part 4 (merged as #157):** no source edits. `SyncModelsTests` lost only its non-firehose DTO tests; #157's typed firehose tests are kept unchanged. `TypedIdentifierGuardTests.Exceptions` gained entries for the new string members and lost the `QuerySubjectsAsync` ones. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit

2026-09-25 00:12:11 +00:00

fix(lexicon)!: wire fidelity against upstream lexicons, enforced by a drift test (closes #123 )

CI / pds-integration (pull_request) Successful in 26s

Details

CI / build-and-test (pull_request) Successful in 58s

Details

Sync Closures to GitHub / sync-closure (pull_request) Successful in 6s

Details

10afc59f6f

The hand-written Lexicon models had drifted from upstream: properties read
under names the services never send, a method that never existed, a query
sent as a POST, procedures posted without their JSON body, chat outputs read
in the wrong shape, and seven permission-set NSIDs nobody published.

The root fix is a drift test against a pinned snapshot of the upstream
Lexicons (bluesky-social/atproto at 2e583a4 plus standard.site's published
schemas, vendored under tests/ATProtoNet.Tests/Lexicon/Upstream with a
refresh script). It checks every XRPC call's NSID and HTTP method, every
NSID literal in src/, every model's JSON names against its upstream def in
both directions, the permission-set constants and the knownValues classes,
each with a commented allow-list whose entries must stay live. A convention
test for explicit camelCase JSON names replaces 54 DTO-only serializer tests.

Everything the drift test found is fixed: the wrong names and shapes, the
queryStatuses and searchAccounts methods, the request bodies, the missing
fields on existing models, the permission sets and label values, and the
raw JsonElement unions, now typed open unions with an Unknown* fallback.
Group-chat views are left to #128 and the firehose events to #126.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

opencode-bot commented

2026-09-25 00:12:13 +00:00

Collaborator

🤖 Reviewing this PR — the review will link the full session transcript.

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

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-159/run-2026-09-25T00-12-12-699Z.md) — updating every ~5 minutes while the review runs.

opencode-bot commented

2026-09-25 00:12:15 +00:00

Collaborator

⏳ Held on the OpenCode Go window — the 5-hour window is at 2% (the bot holds at 95%).
This continues automatically after the window resets (2026-09-25T03:54:33.092Z) — nothing to re-trigger.

⏳ **Held on the OpenCode Go window** — the 5-hour window is at 2% (the bot holds at 95%). This continues automatically after the window resets (2026-09-25T03:54:33.092Z) — nothing to re-trigger.

Grandiras commented

2026-09-25 00:12:17 +00:00

Author

Owner

@claude /review — the description has How to review this. The diff includes a 404-file vendored lexicon snapshot, which is data; skip it. Please read the drift tests in tests/ATProtoNet.Tests/Lexicon/Upstream/ and the behavioural fixes the description lists (wrapped chat outputs, queryStatuses, the GET searchAccounts, request bodies, JSONL export) in full.

@claude /review — the description has *How to review this*. The diff includes a 404-file vendored lexicon snapshot, which is data; skip it. Please read the drift tests in `tests/ATProtoNet.Tests/Lexicon/Upstream/` and the behavioural fixes the description lists (wrapped chat outputs, `queryStatuses`, the GET `searchAccounts`, request bodies, JSONL export) in full.

claude-bot commented

2026-09-25 00:12:22 +00:00

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/598); 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-159/run-598.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-09-25 00:13:59 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This is a large, well-scoped drift-fidelity PR, and it holds up under review. I spot-checked a representative cross-section of the claimed fixes against both the diff and the actual branch content (origin/issue-123-lexicon-fidelity), not just the PR description:

chat.bsky.convo unwrapping: MuteConvoAsync/UnmuteConvoAsync/UpdateReadAsync now correctly unwrap the internal ConvoOutput { convo }, and AddReactionAsync/RemoveReactionAsync unwrap MessageOutput { message } — matches the described wire shapes.
UpdateAllReadAsync/DeactivateAccountAsync now always send a JSON body via ProcedureAsync(..., body), which serializes the request DTO (producing {} when every field is null) rather than sending no body at all — consistent with the PDS rejecting bodyless requests.
ExportAccountDataAsync now returns XrpcStreamResponse via XrpcClient.DownloadAsync, which implements both IDisposable/IAsyncDisposable correctly, matching the JSONL streaming fix and the await using pattern shown in docs/chat.md.
GraphModels.Relationship's new Blocking/BlockedBy/BlockingByList/BlockedByList fields match the vendored upstream app.bsky.graph.defs#relationship schema field-for-field, including descriptions.
XrpcParams.AddAll (used for the new tags, purposes, reasons, values[] parameters) already exists in XrpcClient/XrpcParams, so the new call sites compile against real infra rather than something invented for this PR.
The new typed open unions (FeedReason, ThreadgateRule, PostgateEmbeddingRule, EmbeddedRecordView, RelationshipEntry, Preference) consistently follow the existing [AtProtoUnion(typeof(UnknownX))] / IUnknownUnionVariant pattern already used elsewhere in the codebase, with each UnknownX variant preserving the raw JsonElement and discriminator.

The scope creep into #124's files (DeactivateAccountAsync body, CreateSessionRequest.AllowTakendown, DescribeServerResponse.BlobUploadLimit, AtProtoScopes.PermissionSets) is explicitly called out and kept surgical, as described. The deferred items (#126 firehose, #128 group chat, #131 report reasons) are consistently allow-listed rather than silently skipped, and the drift test's own design (each allow-list entry must still match something, or the suite fails) is a solid anti-staleness mechanism.

I did not find a correctness, security, or omission issue worth blocking on. Given the size of the diff (the tool truncated it), I was not able to review every single line of every model file, but the sampled areas — including the ones most likely to hide bugs (unwrapping response shapes, body-sending fixes, union discriminators, and positional-argument reshuffles) — are all consistent with both the PR description and the actual upstream Lexicon snapshot vendored in the branch.

⏱ 1m 25s · 21 turns · tokens: 95.6k in / 8.0k out (+2238.3k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This is a large, well-scoped drift-fidelity PR, and it holds up under review. I spot-checked a representative cross-section of the claimed fixes against both the diff and the actual branch content (`origin/issue-123-lexicon-fidelity`), not just the PR description: - `chat.bsky.convo` unwrapping: `MuteConvoAsync`/`UnmuteConvoAsync`/`UpdateReadAsync` now correctly unwrap the internal `ConvoOutput { convo }`, and `AddReactionAsync`/`RemoveReactionAsync` unwrap `MessageOutput { message }` — matches the described wire shapes. - `UpdateAllReadAsync`/`DeactivateAccountAsync` now always send a JSON body via `ProcedureAsync(..., body)`, which serializes the request DTO (producing `{}` when every field is null) rather than sending no body at all — consistent with the PDS rejecting bodyless requests. - `ExportAccountDataAsync` now returns `XrpcStreamResponse` via `XrpcClient.DownloadAsync`, which implements both `IDisposable`/`IAsyncDisposable` correctly, matching the JSONL streaming fix and the `await using` pattern shown in `docs/chat.md`. - `GraphModels.Relationship`'s new `Blocking`/`BlockedBy`/`BlockingByList`/`BlockedByList` fields match the vendored upstream `app.bsky.graph.defs#relationship` schema field-for-field, including descriptions. - `XrpcParams.AddAll` (used for the new `tags`, `purposes`, `reasons`, `values[]` parameters) already exists in `XrpcClient`/`XrpcParams`, so the new call sites compile against real infra rather than something invented for this PR. - The new typed open unions (`FeedReason`, `ThreadgateRule`, `PostgateEmbeddingRule`, `EmbeddedRecordView`, `RelationshipEntry`, `Preference`) consistently follow the existing `[AtProtoUnion(typeof(UnknownX))]` / `IUnknownUnionVariant` pattern already used elsewhere in the codebase, with each `UnknownX` variant preserving the raw `JsonElement` and discriminator. The scope creep into `#124`'s files (`DeactivateAccountAsync` body, `CreateSessionRequest.AllowTakendown`, `DescribeServerResponse.BlobUploadLimit`, `AtProtoScopes.PermissionSets`) is explicitly called out and kept surgical, as described. The deferred items (#126 firehose, #128 group chat, #131 report reasons) are consistently allow-listed rather than silently skipped, and the drift test's own design (each allow-list entry must still match something, or the suite fails) is a solid anti-staleness mechanism. I did not find a correctness, security, or omission issue worth blocking on. Given the size of the diff (the tool truncated it), I was not able to review every single line of every model file, but the sampled areas — including the ones most likely to hide bugs (unwrapping response shapes, body-sending fixes, union discriminators, and positional-argument reshuffles) — are all consistent with both the PR description and the actual upstream Lexicon snapshot vendored in the branch. --- ⏱ 1m 25s · 21 turns · tokens: 95.6k in / 8.0k out (+2238.3k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-159/run-598.md)

Grandiras merged commit 3fb7938474 into main

2026-09-25 00:14:38 +00:00

Grandiras deleted branch issue-123-lexicon-fidelity

2026-09-25 00:14:38 +00:00

Grandiras referenced this pull request from a commit

2026-09-25 00:14:39 +00:00

fix(lexicon)!: wire fidelity against upstream lexicons, enforced by a drift test (#159)

Grandiras referenced this pull request

2026-09-25 00:19:34 +00:00

feat(auth)!: unified session model and a reliable session lifecycle #158