Grandiras/ATProto.NET

Fork 0

feat(records)!: record and custom-XRPC ergonomics, Bluesky helpers on client.Bsky #165

Merged

Grandiras merged 1 commit from issue-122-records into main

2026-09-26 00:14:33 +00:00

Grandiras commented

2026-09-26 00:12:08 +00:00

Owner

Closes #122.

Record and custom-XRPC ergonomics, a public transport for third-party Lexicon sub-clients, the Bluesky helpers moved to client.Bsky, one construction path for AtProtoClient, and the remaining duplicate defs merged. Breaking by design; every moved or renamed member has a migration note in CHANGELOG.md.

Records

IAtProtoRecord (static abstract Nsid Collection) and client.GetCollection<T>(). GetCollection<T>(Nsid) stays for run-time collections and throws ArgumentException when T declares a different collection (read through the interface map, so explicit implementations work too). The SDK's 25 record models (every app.bsky, chat.bsky and site.standard record in the upstream snapshot, including #127's status, contentVisibilityDeclaration, referencelistoptout, verification and notification.declaration) implement it, so client.GetCollection<PostRecord>() works; the client.Bsky helpers use those Collections. atproto-lexgen csharp now emits the interface and Collection on generated records (verified: generated app.bsky.feed.like compiles with warnings-as-errors and works with GetCollection<LikeRecord>()).
No fabricated createdAt. AtProtoRecord.CreatedAt has no initializer; CreateAsync stamps it only when null; PutAsync writes what it is given. Regression tests cover deserializing without createdAt and a read-then-put round trip.
One RecordRef(AtUri, Cid, CommitMeta?) (plus RecordKey, ValidationStatus, ToStrongRef()) and one RecordView<T>(AtUri, Cid?, T), both sealed records. Every write returns RecordRef (RecordCollection<T>, RepoClient.CreateRecordAsync/PutRecordAsync, client.Bsky helpers, StandardSiteClient); every typed read returns RecordView<T> (untyped GetRecordAsync returns RecordView<JsonElement>). CreateRecordResponse/PutRecordResponse (one internal RecordWriteResponse) and GetRecordResponse<T> are internal wire DTOs; the non-generic GetRecordResponse is gone.
Name clash removed: Embed.RecordView is renamed RecordEmbedView (the core type keeps the name the issue specifies).
Single deserialization (perf F24): get and list deserialize straight into T via internal GetRecordResponse<T> / ListRecordsResponse<T>, no JsonElement clone and second pass. A bad record still throws XrpcResponseFormatException, naming the record URI (get) or the collection and JSON path (list).
FindAsync / FindFromAsync return null on RecordNotFound only; ExistsAsync is FindAsync(...) is not null.

Custom XRPC and extensibility

QueryAsync<TOut>(Nsid, XrpcParams?, XrpcCallOptions?, ct), ProcedureAsync<TIn, TOut>, ProcedureAsync<TIn>, ProcedureAsync(Nsid, XrpcParams?, …). The anonymous-object overload is kept as QueryAsync<TOut>(Nsid, object, …) with [RequiresUnreferencedCode] (existing call sites keep compiling). The no-input ProcedureAsync carries [OverloadResolutionPriority(1)] so ProcedureAsync(nsid, xrpcParams) is not ambiguous with ProcedureAsync<TIn>.
Transport seam: public IXrpcTransport (QueryAsync, DownloadAsync, the three ProcedureAsync, UploadAsync with optional query parameters, matching #164's multipart upload) exposed as AtProtoClient.Transport. XrpcClient stays internal and implements it explicitly. XrpcParams is public (collection-initializer friendly). docs/custom-xrpc.md has a complete third-party sub-client example (models, sub-client, extension method, NSubstitute test).
EnsureAuthenticated is replaced by RequireSession()/RequireDid(), which read the session once and throw XrpcAuthenticationException(AuthenticationRequired, 401) with a message that fits password and OAuth sessions.

Facade

Bluesky helpers on client.Bsky (in a separate partial file, BlueskyClients.Helpers.cs; #164's new sub-client registrations stay in the main part in AtProtoClient.cs): PostAsync(RichText, PostOptions?), LikeAsync(StrongRef), RepostAsync(StrongRef), FollowAsync(Did), DeleteRecordAsync(AtUri) (validates collection + rkey before any request), UpdateProfileAsync. RichText is a sealed record with implicit conversion from string; RichTextBuilder.Build() returns it (still deconstructs).
Firehose construction removed from AtProtoClient (CreateFirehoseClient/CreateFirehoseConsumer/RelayUrl, and the Aspire AtProtoClientSettings.RelayUrl).
AtProtoClientBuilder removed; one constructor AtProtoClient(AtProtoClientOptions? = null, HttpClient? = null, IAtProtoSessionStore? = null, ILogger<AtProtoClient>? = null).
SetLabelers collapsed to one params IEnumerable<string> method.

Duplicate defs

CreatePdsAccountRequest → CreateAccountRequest (PdsAdminClient forwards every field; password still required).
BlobLink → CidLink.
Report subjects: one open union ModerationSubject in Com.AtProto.Moderation (repoRef, strongRef, the new RepoBlobSubject, messageRef, convoRef, unknown) replaces ReportSubject and the Ozone copy; CreateReportResponse.Subject and the admin subject-status Subjects are typed instead of JsonElement; PdsAdminClient.CreateRepoRef deleted.
Already done upstream: SessionResponse vs Session (#124 removed Session; SessionResponse is the only createSession/refreshSession wire DTO), TeamMemberProfile and AccountResult (#123).

Breaking changes (migration)

See the 16 Breaking changes bullets tagged (#122) in CHANGELOG.md; in short:

new AtProtoClientBuilder().WithInstanceUrl(u).Build() → new AtProtoClient(new AtProtoClientOptions { InstanceUrl = u }).
client.PostAsync(text, facets, embed: e) → client.Bsky.PostAsync(new RichText(text, facets), new PostOptions { Embed = e }); LikeAsync(uri, cid) → Bsky.LikeAsync(new StrongRef { Uri = uri, Cid = cid }); UnlikeAsync/UndoRepostAsync/UnfollowAsync/DeletePostAsync(uri) → Bsky.DeleteRecordAsync(uri); UpdateProfileAsync → Bsky.UpdateProfileAsync.
ProcedureAsync<Out>(nsid, body) → ProcedureAsync<In, Out>(nsid, body); pass XrpcCallOptions by name.
CreatedAt is null until set; set it before PutAsync/RepoClient writes of new records.
CreateRecordResponse/PutRecordResponse/GetRecordResponse<T> → RecordRef/RecordView<T>; Embed.RecordView → RecordEmbedView.
InvalidOperationException("Not authenticated…") → XrpcAuthenticationException (AuthenticationRequired).
CreatePdsAccountRequest → CreateAccountRequest; BlobLink → CidLink; ReportSubject → ModerationSubject (Ozone subject types move to ATProtoNet.Lexicon.Com.AtProto.Moderation).
CreateFirehoseClient() → new FirehoseClient(relayUrl, logger).

Verification

dotnet build (full solution): 0 warnings, 0 errors.
dotnet build --no-incremental: 0 warnings, 0 errors.
Unit tests: 3657 passed, 0 failed, 5 skipped (rebased onto origin/main at 4955389, after #161, #162, #163, #164).
Record coverage test (LexiconDriftTests.Records): every SDK type that is an AtProtoRecord or writes a $type that the upstream snapshot defines as a record must implement IAtProtoRecord with Collection equal to that $type (25 found; the SDK has no AtProtoRecord subclasses of its own, so the scan keys on upstream record kind), and every IAtProtoRecord must name an upstream record. Mutation-checked: dropping the interface from one model fails the test.
New tests: Http/CustomXrpcTests (12: query params incl. anonymous overload, the three procedure shapes, null input, format errors, a third-party sub-client over client.Transport following sign-in/sign-out, download/upload, faking IXrpcTransport), BlueskyHelpersTests (post/like/repost/follow/delete/validation/unauthenticated + the moved profile tests), AtProtoClientConstructionTests, and additions to RecordCollectionTests / RecordCollectionErrorTests (GetCollection checks, RecordRef/RecordView, createdAt stamping and non-fabrication, FindAsync, typed list, format errors, no-session behaviour), RichTextBuilderTests, LexiconUnionTests (RepoBlobSubject, typed admin subject), CSharpEmitterTests.
Drift test allowlist: RecordWriteResponse → createRecord#output, ListRecordsResponse<> → listRecords#output, BlobLink entry removed.
Integration tests updated and compiling (they skip locally).

Deliberately deferred / flagged

SetProxy / ClearProxy / SetLabelers / ClearLabelers kept. The issue removes forwarders "now covered by XrpcCallOptions", but the Lexicon sub-client methods (client.Bsky.Feed… etc.) don't take XrpcCallOptions, so these client-wide defaults are the only way to send labelers/proxy on those calls (docs/labeler.md relies on it). Only the duplicate SetLabelers overload was collapsed. LatestRepoRev/LatestRateLimitInfo are status, not forwarders, and stay.
com.atproto.lexicon.schema is the one upstream record the SDK doesn't model yet (lexicon-resolution issue); StandardSiteClient keeps its own private NSID fields (could switch to PublicationRecord.Collection and friends).
RichText lives in namespace ATProtoNet.Lexicon.App.Bsky.RichText (type and namespace share a name, as the issue names it); code inside a namespace with a RichText or Lexicon child may need an alias.

Edits outside this issue's area (unavoidable call sites)

Lexicon/App/Bsky, Lexicon/Chat/Bsky/Actor, Lexicon/Site/Standard/** models: IAtProtoRecord + static Nsid Collection on each record (and Type => Collection). Lexicon/App/Bsky: RichText.cs (new), RichTextBuilder.Build(), Embed.RecordView rename, one cref in ActorModels.cs. Lexicon/Tools/Ozone/Moderation/ModerationModels.cs: subject copies removed, one using. Lexicon/Com/AtProto/Admin/AdminModels.cs: subject types + using. Lexicon/Site/Standard/StandardSiteClient.cs: return types and the private list helper. RepoClient.cs: record methods only, importRepo untouched. BlueskyClients in AtProtoClient.cs: partial keyword only.

🤖 Generated with Claude Code

Closes #122. Record and custom-XRPC ergonomics, a public transport for third-party Lexicon sub-clients, the Bluesky helpers moved to `client.Bsky`, one construction path for `AtProtoClient`, and the remaining duplicate defs merged. Breaking by design; every moved or renamed member has a migration note in `CHANGELOG.md`. ## Records - **`IAtProtoRecord`** (`static abstract Nsid Collection`) and **`client.GetCollection<T>()`**. `GetCollection<T>(Nsid)` stays for run-time collections and throws `ArgumentException` when `T` declares a different collection (read through the interface map, so explicit implementations work too). The SDK's 25 record models (every `app.bsky`, `chat.bsky` and `site.standard` record in the upstream snapshot, including #127's `status`, `contentVisibilityDeclaration`, `referencelistoptout`, `verification` and `notification.declaration`) implement it, so `client.GetCollection<PostRecord>()` works; the `client.Bsky` helpers use those `Collection`s. `atproto-lexgen csharp` now emits the interface and `Collection` on generated records (verified: generated `app.bsky.feed.like` compiles with warnings-as-errors and works with `GetCollection<LikeRecord>()`). - **No fabricated `createdAt`.** `AtProtoRecord.CreatedAt` has no initializer; `CreateAsync` stamps it only when `null`; `PutAsync` writes what it is given. Regression tests cover deserializing without `createdAt` and a read-then-put round trip. - **One `RecordRef(AtUri, Cid, CommitMeta?)`** (plus `RecordKey`, `ValidationStatus`, `ToStrongRef()`) and **one `RecordView<T>(AtUri, Cid?, T)`**, both sealed records. Every write returns `RecordRef` (`RecordCollection<T>`, `RepoClient.CreateRecordAsync`/`PutRecordAsync`, `client.Bsky` helpers, `StandardSiteClient`); every typed read returns `RecordView<T>` (untyped `GetRecordAsync` returns `RecordView<JsonElement>`). `CreateRecordResponse`/`PutRecordResponse` (one internal `RecordWriteResponse`) and `GetRecordResponse<T>` are internal wire DTOs; the non-generic `GetRecordResponse` is gone. - **Name clash removed:** `Embed.RecordView` is renamed `RecordEmbedView` (the core type keeps the name the issue specifies). - **Single deserialization (perf F24):** get and list deserialize straight into `T` via internal `GetRecordResponse<T>` / `ListRecordsResponse<T>`, no `JsonElement` clone and second pass. A bad record still throws `XrpcResponseFormatException`, naming the record URI (get) or the collection and JSON path (list). - **`FindAsync` / `FindFromAsync`** return `null` on `RecordNotFound` only; `ExistsAsync` is `FindAsync(...) is not null`. ## Custom XRPC and extensibility - `QueryAsync<TOut>(Nsid, XrpcParams?, XrpcCallOptions?, ct)`, `ProcedureAsync<TIn, TOut>`, `ProcedureAsync<TIn>`, `ProcedureAsync(Nsid, XrpcParams?, …)`. The anonymous-object overload is kept as `QueryAsync<TOut>(Nsid, object, …)` with `[RequiresUnreferencedCode]` (existing call sites keep compiling). The no-input `ProcedureAsync` carries `[OverloadResolutionPriority(1)]` so `ProcedureAsync(nsid, xrpcParams)` is not ambiguous with `ProcedureAsync<TIn>`. - **Transport seam:** public `IXrpcTransport` (`QueryAsync`, `DownloadAsync`, the three `ProcedureAsync`, `UploadAsync` with optional query parameters, matching #164's multipart upload) exposed as `AtProtoClient.Transport`. `XrpcClient` stays internal and implements it explicitly. `XrpcParams` is public (collection-initializer friendly). `docs/custom-xrpc.md` has a complete third-party sub-client example (models, sub-client, extension method, NSubstitute test). - `EnsureAuthenticated` is replaced by `RequireSession()`/`RequireDid()`, which read the session once and throw `XrpcAuthenticationException(AuthenticationRequired, 401)` with a message that fits password and OAuth sessions. ## Facade - Bluesky helpers on `client.Bsky` (in a separate partial file, `BlueskyClients.Helpers.cs`; #164's new sub-client registrations stay in the main part in `AtProtoClient.cs`): `PostAsync(RichText, PostOptions?)`, `LikeAsync(StrongRef)`, `RepostAsync(StrongRef)`, `FollowAsync(Did)`, `DeleteRecordAsync(AtUri)` (validates collection + rkey before any request), `UpdateProfileAsync`. `RichText` is a sealed record with implicit conversion from `string`; `RichTextBuilder.Build()` returns it (still deconstructs). - Firehose construction removed from `AtProtoClient` (`CreateFirehoseClient`/`CreateFirehoseConsumer`/`RelayUrl`, and the Aspire `AtProtoClientSettings.RelayUrl`). - `AtProtoClientBuilder` removed; one constructor `AtProtoClient(AtProtoClientOptions? = null, HttpClient? = null, IAtProtoSessionStore? = null, ILogger<AtProtoClient>? = null)`. - `SetLabelers` collapsed to one `params IEnumerable<string>` method. ## Duplicate defs - `CreatePdsAccountRequest` → `CreateAccountRequest` (PdsAdminClient forwards every field; password still required). - `BlobLink` → `CidLink`. - Report subjects: one open union `ModerationSubject` in `Com.AtProto.Moderation` (repoRef, strongRef, the new `RepoBlobSubject`, messageRef, convoRef, unknown) replaces `ReportSubject` and the Ozone copy; `CreateReportResponse.Subject` and the admin subject-status `Subject`s are typed instead of `JsonElement`; `PdsAdminClient.CreateRepoRef` deleted. - Already done upstream: `SessionResponse` vs `Session` (#124 removed `Session`; `SessionResponse` is the only createSession/refreshSession wire DTO), `TeamMemberProfile` and `AccountResult` (#123). ## Breaking changes (migration) See the 16 `Breaking changes` bullets tagged (#122) in `CHANGELOG.md`; in short: - `new AtProtoClientBuilder().WithInstanceUrl(u).Build()` → `new AtProtoClient(new AtProtoClientOptions { InstanceUrl = u })`. - `client.PostAsync(text, facets, embed: e)` → `client.Bsky.PostAsync(new RichText(text, facets), new PostOptions { Embed = e })`; `LikeAsync(uri, cid)` → `Bsky.LikeAsync(new StrongRef { Uri = uri, Cid = cid })`; `UnlikeAsync`/`UndoRepostAsync`/`UnfollowAsync`/`DeletePostAsync(uri)` → `Bsky.DeleteRecordAsync(uri)`; `UpdateProfileAsync` → `Bsky.UpdateProfileAsync`. - `ProcedureAsync<Out>(nsid, body)` → `ProcedureAsync<In, Out>(nsid, body)`; pass `XrpcCallOptions` by name. - `CreatedAt` is `null` until set; set it before `PutAsync`/`RepoClient` writes of new records. - `CreateRecordResponse`/`PutRecordResponse`/`GetRecordResponse<T>` → `RecordRef`/`RecordView<T>`; `Embed.RecordView` → `RecordEmbedView`. - `InvalidOperationException("Not authenticated…")` → `XrpcAuthenticationException` (`AuthenticationRequired`). - `CreatePdsAccountRequest` → `CreateAccountRequest`; `BlobLink` → `CidLink`; `ReportSubject` → `ModerationSubject` (Ozone subject types move to `ATProtoNet.Lexicon.Com.AtProto.Moderation`). - `CreateFirehoseClient()` → `new FirehoseClient(relayUrl, logger)`. ## Verification - `dotnet build` (full solution): 0 warnings, 0 errors. - `dotnet build --no-incremental`: 0 warnings, 0 errors. - Unit tests: 3657 passed, 0 failed, 5 skipped (rebased onto `origin/main` at `4955389`, after #161, #162, #163, #164). - Record coverage test (`LexiconDriftTests.Records`): every SDK type that is an `AtProtoRecord` or writes a `$type` that the upstream snapshot defines as a record must implement `IAtProtoRecord` with `Collection` equal to that `$type` (25 found; the SDK has no `AtProtoRecord` subclasses of its own, so the scan keys on upstream record kind), and every `IAtProtoRecord` must name an upstream record. Mutation-checked: dropping the interface from one model fails the test. - New tests: `Http/CustomXrpcTests` (12: query params incl. anonymous overload, the three procedure shapes, null input, format errors, a third-party sub-client over `client.Transport` following sign-in/sign-out, download/upload, faking `IXrpcTransport`), `BlueskyHelpersTests` (post/like/repost/follow/delete/validation/unauthenticated + the moved profile tests), `AtProtoClientConstructionTests`, and additions to `RecordCollectionTests` / `RecordCollectionErrorTests` (GetCollection checks, RecordRef/RecordView, createdAt stamping and non-fabrication, FindAsync, typed list, format errors, no-session behaviour), `RichTextBuilderTests`, `LexiconUnionTests` (`RepoBlobSubject`, typed admin subject), `CSharpEmitterTests`. - Drift test allowlist: `RecordWriteResponse` → `createRecord#output`, `ListRecordsResponse<>` → `listRecords#output`, `BlobLink` entry removed. - Integration tests updated and compiling (they skip locally). ## Deliberately deferred / flagged - **`SetProxy` / `ClearProxy` / `SetLabelers` / `ClearLabelers` kept.** The issue removes forwarders "now covered by `XrpcCallOptions`", but the Lexicon sub-client methods (`client.Bsky.Feed…` etc.) don't take `XrpcCallOptions`, so these client-wide defaults are the only way to send labelers/proxy on those calls (docs/labeler.md relies on it). Only the duplicate `SetLabelers` overload was collapsed. `LatestRepoRev`/`LatestRateLimitInfo` are status, not forwarders, and stay. - `com.atproto.lexicon.schema` is the one upstream record the SDK doesn't model yet (lexicon-resolution issue); `StandardSiteClient` keeps its own private NSID fields (could switch to `PublicationRecord.Collection` and friends). - `RichText` lives in namespace `ATProtoNet.Lexicon.App.Bsky.RichText` (type and namespace share a name, as the issue names it); code inside a namespace with a `RichText` or `Lexicon` child may need an alias. ## Edits outside this issue's area (unavoidable call sites) `Lexicon/App/Bsky`, `Lexicon/Chat/Bsky/Actor`, `Lexicon/Site/Standard/**` models: `IAtProtoRecord` + `static Nsid Collection` on each record (and `Type => Collection`). `Lexicon/App/Bsky`: `RichText.cs` (new), `RichTextBuilder.Build()`, `Embed.RecordView` rename, one cref in `ActorModels.cs`. `Lexicon/Tools/Ozone/Moderation/ModerationModels.cs`: subject copies removed, one `using`. `Lexicon/Com/AtProto/Admin/AdminModels.cs`: subject types + `using`. `Lexicon/Site/Standard/StandardSiteClient.cs`: return types and the private list helper. `RepoClient.cs`: record methods only, `importRepo` untouched. `BlueskyClients` in `AtProtoClient.cs`: `partial` keyword only. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit

2026-09-26 00:12:09 +00:00

feat(records)!: record and custom-XRPC ergonomics, Bluesky helpers on client.Bsky (closes #122 )

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

Details

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

Details

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

Details

5514a0d98b

Record types now name their collection once (IAtProtoRecord, GetCollection<T>()),
reads no longer fabricate a createdAt that a later put would persist, and every
write and typed read returns one RecordRef / RecordView<T> shape, deserialized
once instead of through a JsonElement. FindAsync covers the "may be absent" read.

Custom XRPC takes XrpcParams and typed procedure inputs, and AtProtoClient.Transport
(IXrpcTransport) lets third-party Lexicon packages build sub-clients that share the
client's service and session while XrpcClient stays internal.

The facade shrinks to one construction path (the builder is gone), loses the
firehose factories, and moves the Bluesky helpers to client.Bsky with typed
arguments. Duplicate defs merge: CreatePdsAccountRequest into CreateAccountRequest,
BlobLink into CidLink, and the three moderation-subject copies into one union.

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

opencode-bot commented

2026-09-26 00:12:10 +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-165/run-2026-09-26T00-12-09-773Z.md) — updating every ~5 minutes while the review runs.

opencode-bot commented

2026-09-26 00:12:12 +00:00

Collaborator

⏳ Held on the OpenCode Go window — the 5-hour window is at 0% (the bot holds at 95%).
This continues automatically after the window resets (2026-09-26T05:12:10.267Z) — nothing to re-trigger.

⏳ **Held on the OpenCode Go window** — the 5-hour window is at 0% (the bot holds at 95%). This continues automatically after the window resets (2026-09-26T05:12:10.267Z) — nothing to re-trigger.

Grandiras commented

2026-09-26 00:12:14 +00:00

Author

Owner

@claude /review — large; the description has the migration notes. Please read these in full from the branch: IXrpcTransport (the public seam) and its implementation, RecordCollection<T> (single deserialization, FindAsync), the RecordRef/RecordView<T> types, and the GetCollection<T>() NSID consistency check.

@claude /review — large; the description has the migration notes. Please read these in full from the branch: `IXrpcTransport` (the public seam) and its implementation, `RecordCollection<T>` (single deserialization, `FindAsync`), the `RecordRef`/`RecordView<T>` types, and the `GetCollection<T>()` NSID consistency check.

claude-bot commented

2026-09-26 00:12:16 +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/610); 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-165/run-610.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-09-26 00:14:03 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This is a large, well-executed ergonomics/breaking-change PR, and the parts I inspected (both in the diff and by pulling the full files from origin/issue-122-records where the diff was truncated) hold up well.

Spot-checked in depth:

IAtProtoRecord / RecordPaths.DeclaredCollection<T>() — reads the static abstract Collection member through GetInterfaceMap, correctly guards against T being an interface or not implementing IAtProtoRecord, and caches the result per closed generic type. GetCollection<T>(Nsid) correctly cross-checks a declared collection against the explicit one and throws ArgumentException on mismatch; GetCollection<T>() reads T.Collection directly via the interface constraint.
RecordRef/RecordView<T> constructors validate the URI names a record via RecordPaths.PathOf, and ToStrongRef() is correct.
RecordCollection<T>.CreateAsync only stamps CreatedAt when null, matching the changelog's "no fabricated createdAt" claim; FindAsync/FindFromAsync correctly narrow the catch to RecordNotFound only.
AtProtoClient constructor ordering is correct: Bsky (and its constituent sub-clients) is fully built before Bsky.Bind(this) is called, and RequireSession/RequireDid throw XrpcAuthenticationException cleanly before any request is sent.
IXrpcTransport/XrpcClient explicit interface implementation forwards to the existing internal methods correctly, including the [OverloadResolutionPriority(1)] disambiguation between ProcedureAsync(nsid, XrpcParams?) and ProcedureAsync<TIn>(nsid, input) (a real ambiguity that this attribute resolves cleanly on .NET 10/C# 13+).
PdsAdminClient.CreateAccountAsync's invite-code minting path correctly copies the request rather than mutating a caller-owned object, and forwards every field including the new VerificationCode/VerificationPhone/PlcOp.
Docs are thorough and consistently updated to match every renamed/moved API surface described in the PR body.

I didn't find any correctness bugs, and the breaking changes are all documented with migration notes as required by CLAUDE.md. Given the scope, I did not re-derive every one of the ~25 record models or the full ModerationSubject union by hand, but the pattern used throughout is consistent and the PR author claims mutation-tested drift-test coverage for exactly that surface.

⏱ 1m 37s · 9 turns · tokens: 94.6k in / 6.4k out (+911.2k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This is a large, well-executed ergonomics/breaking-change PR, and the parts I inspected (both in the diff and by pulling the full files from `origin/issue-122-records` where the diff was truncated) hold up well. Spot-checked in depth: - `IAtProtoRecord` / `RecordPaths.DeclaredCollection<T>()` — reads the static abstract `Collection` member through `GetInterfaceMap`, correctly guards against `T` being an interface or not implementing `IAtProtoRecord`, and caches the result per closed generic type. `GetCollection<T>(Nsid)` correctly cross-checks a declared collection against the explicit one and throws `ArgumentException` on mismatch; `GetCollection<T>()` reads `T.Collection` directly via the interface constraint. - `RecordRef`/`RecordView<T>` constructors validate the URI names a record via `RecordPaths.PathOf`, and `ToStrongRef()` is correct. - `RecordCollection<T>.CreateAsync` only stamps `CreatedAt` when null, matching the changelog's "no fabricated createdAt" claim; `FindAsync`/`FindFromAsync` correctly narrow the catch to `RecordNotFound` only. - `AtProtoClient` constructor ordering is correct: `Bsky` (and its constituent sub-clients) is fully built before `Bsky.Bind(this)` is called, and `RequireSession`/`RequireDid` throw `XrpcAuthenticationException` cleanly before any request is sent. - `IXrpcTransport`/`XrpcClient` explicit interface implementation forwards to the existing internal methods correctly, including the `[OverloadResolutionPriority(1)]` disambiguation between `ProcedureAsync(nsid, XrpcParams?)` and `ProcedureAsync<TIn>(nsid, input)` (a real ambiguity that this attribute resolves cleanly on .NET 10/C# 13+). - `PdsAdminClient.CreateAccountAsync`'s invite-code minting path correctly copies the request rather than mutating a caller-owned object, and forwards every field including the new `VerificationCode`/`VerificationPhone`/`PlcOp`. - Docs are thorough and consistently updated to match every renamed/moved API surface described in the PR body. I didn't find any correctness bugs, and the breaking changes are all documented with migration notes as required by CLAUDE.md. Given the scope, I did not re-derive every one of the ~25 record models or the full `ModerationSubject` union by hand, but the pattern used throughout is consistent and the PR author claims mutation-tested drift-test coverage for exactly that surface. --- ⏱ 1m 37s · 9 turns · tokens: 94.6k in / 6.4k out (+911.2k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-165/run-610.md)

Grandiras merged commit 06a4285769 into main

2026-09-26 00:14:33 +00:00

Grandiras deleted branch issue-122-records

2026-09-26 00:14:34 +00:00

Grandiras referenced this pull request from a commit

2026-09-26 00:14:34 +00:00

feat(records)!: record and custom-XRPC ergonomics, Bluesky helpers on client.Bsky (#165)

Grandiras referenced this pull request

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

feat(identity)!: resolver abstraction, identity caching and SSRF-safe fetching #160