Grandiras/ATProto.NET

Fork 0

refactor!: debloat the Lexicon surface and lexgen #183

Merged

Grandiras merged 10 commits from debloat-c-lexicon into main

2026-09-27 15:55:44 +00:00

Grandiras commented

2026-09-27 13:11:09 +00:00

Owner

Part of #180 (pass C: Lexicon surface and lexgen).

What changed

L3 — Enumerate* trimmed to 13, Http.Pagination made public. Of the ~77 Enumerate*/ListAll*
wrappers, only the 13 the tracking issues (#119, #127) named or that 0.6.0 already had survive:
RecordCollection<T>.EnumerateAsync/EnumerateFromAsync, RepoClient.EnumerateRecordsAsync,
SimpleSpaceClient.EnumerateMembersAsync, SpaceClient.EnumerateReposAsync/EnumerateRecordsAsync,
JetstreamArchiveClient.EnumerateSegmentsAsync (Pass A's file, untouched — already on the keep list),
FeedClient.EnumerateTimelineAsync/EnumerateAuthorFeedAsync/EnumerateActorLikesAsync,
GraphClient.EnumerateFollowersAsync/EnumerateFollowsAsync/EnumerateListMembersAsync,
NotificationClient.EnumerateNotificationsAsync, ConvoClient.EnumerateMessagesAsync,
BookmarkClient.EnumerateBookmarksAsync. Every other one is gone. ATProtoNet.Http.Pagination, the
cursor loop every one of them used, is now public with an expanded doc, so any other endpoint's
GetXAsync(..., cursor, cancellationToken) walks with one line:
Pagination.EnumerateAsync<TPage, T>((cursor, ct) => client.X.GetYAsync(..., cursor, ct)). Updated
every doc snippet, sample and test that used a removed enumerator.

L5 — one Models.CursorPage<T> base for the 72 cursored responses. Each used to implement
ICursorPage<T> by hand and redeclare an identical Cursor property with the same doc comment.
They now derive from CursorPage<T> (a sealed record, since records can only derive from other
records — was sealed class) and override Items publicly ([JsonIgnore], so the wire shape is
unchanged) instead of implementing it as an explicit interface member.

L9 — view inheritance and two type merges.
ProfileViewBasic ⊂ ProfileView ⊂ ProfileViewDetailed now inherit instead of each redeclaring
the fields they share (12, then +2, then +7 own fields); GetSessionResponse is the base of
SessionResponse the same way. Tools.Ozone.Report.LiveStats and Tools.Ozone.Queue.QueueStats
were byte-identical shapes for two different upstream defs; GetLiveStatsResponse.Stats is now
QueueStats and LiveStats is gone.

L4 — Unknown* union variants use the primary-constructor form. The 38 straightforward ones
(everywhere but the two — UnknownConvoLogEntry, UnknownChatModerationEvent — that also read
fields from their base type) collapse an 8-line constructor plus two properties into one line, using
a new UnknownUnionVariant.RequireType alongside RequireObject. OpenUnionTests' hand-picked
12-entry theory is now a reflection scan over every [AtProtoUnion(typeof(Unknown...))] base (all
40), replacing ConvoModelsTests' separate 9-case duplicate of the same behavior.

L1 — internal request DTOs. 22 structurally-identical DTOs across 9 groups merge into one
shared positional record each (the 10 endpoints across chat.bsky.convo.*/group.* that take only
a convoId, addReaction/removeReaction, addMembers/removeMembers,
approveJoinRequest/rejectJoinRequest, tools.ozone.set add/deleteValues,
tools.ozone.queue assign/unassignModerator, com.atproto.admin disable/enableAccountInvites,
app.bsky.video finish/abortUpload, com.atproto.sync notifyOfUpdate/requestCrawl); each merge adds
a LexiconDriftTests.Allowlists DefOf entry, since the merged type's name no longer resolves to
either upstream def by convention. The remaining 80 single-purpose DTOs become one-line positional
records with no XML docs (internal, CS1591-exempt); 11 needed an explicit JsonPropertyOrder per
field to keep the wire order when a required field's Lexicon position came after an optional one
(C#'s primary-constructor parameters can't do that). All internal — no public API change.

L11 — atproto-lexgen's 6 commands share one option-parsing helper instead of each hand-rolling
a for/switch loop. The migrate stub and publish's legacy-option messages
(--output/--baseline/--assembly/--no-bump), which only explained an already-shipped removal,
are gone — both now fail like any other unrecognized command/option.

L12 — reverted. Reusing the SDK's LexiconPermission (whose Resource is required) traded
the linter's lenient "'resource' is required" diagnostic for a hard JsonException on load when
a permission-set entry omits it — a worse tool for the ~35 lines it saved. The tool keeps its own
tolerant LexiconPermission in Schema/LexiconDocument.cs.

L10/L13 — test duplicates. Removed the pagination-mechanics tests left orphaned by the L3
removals (each endpoint's own query-parameter wiring stayed where an enumerator was kept; two
endpoints whose cursor travels in the request body, not the query string, got their coverage
rewritten against Pagination.EnumerateAsync directly rather than deleted). Dropped
Emit_AnyDocument_ProducesBalancedBraces: CSharpEmitterCompileTests actually compiles the
emitter's output with Roslyn, a strictly stronger check.

Filter objects for the very long parameter lists (ModerationClient, the FeedClient search) —
left as is; they didn't clearly read better as a filter object than as named optional parameters.

Breaking changes

All documented in CHANGELOG.md under [Unreleased] → Breaking changes and in
docs/migrating-to-0.7.md:

Enumerate* trimmed to 13 (migration: Pagination.EnumerateAsync).
Cursored responses derive from Models.CursorPage<T> (records now; Items is a public override).
View inheritance (is ProfileView now also matches ProfileViewDetailed) and the session-response
base.
LiveStats → QueueStats.
atproto-lexgen's migrate/legacy-publish-option messages become generic "Unknown" errors.

Verification

dotnet build (whole solution, including doc snippets and samples): 0 warnings, 0 errors.
dotnet test tests/ATProtoNet.Tests/: 5067 passed, 5 skipped (pre-existing, Aspire manifest tests
needing a manifest fixture), 0 failed — run twice, identical both times.
python3 <scratchpad>/loc.py origin/main HEAD for this pass's area (src/ATProtoNet/{Models,Http,
Admin,Serialization,Lexicon/*}, tools/, tests/ATProtoNet.Tests):
- code: 19,216 → 17,695 (‑1,521)
- xmldoc: 9,678 → 8,801 (‑877)
- tests: 49,571 → 49,131 (‑440)
- PublicAPI.Shipped.txt: 9,983 → 10,355 (+372 — expected: the compiler-synthesized members
  (Equals, GetHashCode, ==/!=, ToString, the copy constructor) of the 72 new
  CursorPage<T>-derived records)
- docs: 12,818 → 12,926 (+108 — migration-guide entries plus the doc-snippet rewrites the
  Enumerate* trim forced)

Deliberately left

The Enumerate* list settled at exactly 13 by treating RecordCollection<T>'s two generic
helpers and JetstreamArchiveClient.EnumerateSegmentsAsync (Pass A's file) as pre-existing
infrastructure rather than part of the "77 to trim," per the plan's own arithmetic.
Two Unknown* variants (UnknownConvoLogEntry, UnknownChatModerationEvent) keep their explicit
constructors — they also read fields from their base type via [SetsRequiredMembers], which a
primary constructor can't express cleanly.
Filter objects for ModerationClient/FeedClient's long parameter lists: left alone.
L12 (reuse the SDK's LexiconPermission in the tool): tried, then reverted — it turned a lint
diagnostic for a malformed permission set into a load-time JsonException, which is a worse tool
for the line count it saved.

Unavoidable edit outside this pass's area

src/ATProtoNet.Server/Spaces/SpaceWriteNotifier.cs (Pass B's file): one call site,
new NotifySpaceDeletedRequest { Space = space } → new NotifySpaceDeletedRequest(space), forced
by that type's L1 conversion to a positional record.

🤖 Generated with Claude Code

Part of #180 (pass C: Lexicon surface and lexgen). ## What changed **L3 — `Enumerate*` trimmed to 13, `Http.Pagination` made public.** Of the ~77 `Enumerate*`/`ListAll*` wrappers, only the 13 the tracking issues (#119, #127) named or that 0.6.0 already had survive: `RecordCollection<T>.EnumerateAsync`/`EnumerateFromAsync`, `RepoClient.EnumerateRecordsAsync`, `SimpleSpaceClient.EnumerateMembersAsync`, `SpaceClient.EnumerateReposAsync`/`EnumerateRecordsAsync`, `JetstreamArchiveClient.EnumerateSegmentsAsync` (Pass A's file, untouched — already on the keep list), `FeedClient.EnumerateTimelineAsync`/`EnumerateAuthorFeedAsync`/`EnumerateActorLikesAsync`, `GraphClient.EnumerateFollowersAsync`/`EnumerateFollowsAsync`/`EnumerateListMembersAsync`, `NotificationClient.EnumerateNotificationsAsync`, `ConvoClient.EnumerateMessagesAsync`, `BookmarkClient.EnumerateBookmarksAsync`. Every other one is gone. `ATProtoNet.Http.Pagination`, the cursor loop every one of them used, is now public with an expanded doc, so any other endpoint's `GetXAsync(..., cursor, cancellationToken)` walks with one line: `Pagination.EnumerateAsync<TPage, T>((cursor, ct) => client.X.GetYAsync(..., cursor, ct))`. Updated every doc snippet, sample and test that used a removed enumerator. **L5 — one `Models.CursorPage<T>` base for the 72 cursored responses.** Each used to implement `ICursorPage<T>` by hand and redeclare an identical `Cursor` property with the same doc comment. They now derive from `CursorPage<T>` (a `sealed record`, since records can only derive from other records — was `sealed class`) and override `Items` publicly (`[JsonIgnore]`, so the wire shape is unchanged) instead of implementing it as an explicit interface member. **L9 — view inheritance and two type merges.** `ProfileViewBasic` ⊂ `ProfileView` ⊂ `ProfileViewDetailed` now inherit instead of each redeclaring the fields they share (12, then +2, then +7 own fields); `GetSessionResponse` is the base of `SessionResponse` the same way. `Tools.Ozone.Report.LiveStats` and `Tools.Ozone.Queue.QueueStats` were byte-identical shapes for two different upstream defs; `GetLiveStatsResponse.Stats` is now `QueueStats` and `LiveStats` is gone. **L4 — `Unknown*` union variants use the primary-constructor form.** The 38 straightforward ones (everywhere but the two — `UnknownConvoLogEntry`, `UnknownChatModerationEvent` — that also read fields from their base type) collapse an 8-line constructor plus two properties into one line, using a new `UnknownUnionVariant.RequireType` alongside `RequireObject`. `OpenUnionTests`' hand-picked 12-entry theory is now a reflection scan over every `[AtProtoUnion(typeof(Unknown...))]` base (all 40), replacing `ConvoModelsTests`' separate 9-case duplicate of the same behavior. **L1 — internal request DTOs.** 22 structurally-identical DTOs across 9 groups merge into one shared positional record each (the 10 endpoints across `chat.bsky.convo.*`/`group.*` that take only a `convoId`, `addReaction`/`removeReaction`, `addMembers`/`removeMembers`, `approveJoinRequest`/`rejectJoinRequest`, `tools.ozone.set` add/deleteValues, `tools.ozone.queue` assign/unassignModerator, `com.atproto.admin` disable/enableAccountInvites, `app.bsky.video` finish/abortUpload, `com.atproto.sync` notifyOfUpdate/requestCrawl); each merge adds a `LexiconDriftTests.Allowlists` `DefOf` entry, since the merged type's name no longer resolves to either upstream def by convention. The remaining 80 single-purpose DTOs become one-line positional records with no XML docs (internal, CS1591-exempt); 11 needed an explicit `JsonPropertyOrder` per field to keep the wire order when a required field's Lexicon position came after an optional one (C#'s primary-constructor parameters can't do that). All internal — no public API change. **L11 — `atproto-lexgen`'s 6 commands share one option-parsing helper** instead of each hand-rolling a `for`/`switch` loop. The `migrate` stub and `publish`'s legacy-option messages (`--output`/`--baseline`/`--assembly`/`--no-bump`), which only explained an already-shipped removal, are gone — both now fail like any other unrecognized command/option. **L12 — reverted.** Reusing the SDK's `LexiconPermission` (whose `Resource` is `required`) traded the linter's lenient "`'resource' is required`" diagnostic for a hard `JsonException` on load when a permission-set entry omits it — a worse tool for the ~35 lines it saved. The tool keeps its own tolerant `LexiconPermission` in `Schema/LexiconDocument.cs`. **L10/L13 — test duplicates.** Removed the pagination-mechanics tests left orphaned by the L3 removals (each endpoint's own query-parameter wiring stayed where an enumerator was kept; two endpoints whose cursor travels in the request body, not the query string, got their coverage rewritten against `Pagination.EnumerateAsync` directly rather than deleted). Dropped `Emit_AnyDocument_ProducesBalancedBraces`: `CSharpEmitterCompileTests` actually compiles the emitter's output with Roslyn, a strictly stronger check. **Filter objects** for the very long parameter lists (`ModerationClient`, the `FeedClient` search) — left as is; they didn't clearly read better as a filter object than as named optional parameters. ## Breaking changes All documented in `CHANGELOG.md` under `[Unreleased] → Breaking changes` and in `docs/migrating-to-0.7.md`: - `Enumerate*` trimmed to 13 (migration: `Pagination.EnumerateAsync`). - Cursored responses derive from `Models.CursorPage<T>` (records now; `Items` is a public override). - View inheritance (`is ProfileView` now also matches `ProfileViewDetailed`) and the session-response base. - `LiveStats` → `QueueStats`. - `atproto-lexgen`'s `migrate`/legacy-`publish`-option messages become generic "Unknown" errors. ## Verification - `dotnet build` (whole solution, including doc snippets and samples): 0 warnings, 0 errors. - `dotnet test tests/ATProtoNet.Tests/`: 5067 passed, 5 skipped (pre-existing, Aspire manifest tests needing a manifest fixture), 0 failed — run twice, identical both times. - `python3 <scratchpad>/loc.py origin/main HEAD` for this pass's area (src/ATProtoNet/{Models,Http, Admin,Serialization,Lexicon/*}, tools/, tests/ATProtoNet.Tests): - code: 19,216 → 17,695 (‑1,521) - xmldoc: 9,678 → 8,801 (‑877) - tests: 49,571 → 49,131 (‑440) - `PublicAPI.Shipped.txt`: 9,983 → 10,355 (+372 — expected: the compiler-synthesized members (`Equals`, `GetHashCode`, `==`/`!=`, `ToString`, the copy constructor) of the 72 new `CursorPage<T>`-derived records) - docs: 12,818 → 12,926 (+108 — migration-guide entries plus the doc-snippet rewrites the `Enumerate*` trim forced) ## Deliberately left - The `Enumerate*` list settled at exactly 13 by treating `RecordCollection<T>`'s two generic helpers and `JetstreamArchiveClient.EnumerateSegmentsAsync` (Pass A's file) as pre-existing infrastructure rather than part of the "77 to trim," per the plan's own arithmetic. - Two `Unknown*` variants (`UnknownConvoLogEntry`, `UnknownChatModerationEvent`) keep their explicit constructors — they also read fields from their base type via `[SetsRequiredMembers]`, which a primary constructor can't express cleanly. - Filter objects for `ModerationClient`/`FeedClient`'s long parameter lists: left alone. - L12 (reuse the SDK's `LexiconPermission` in the tool): tried, then reverted — it turned a lint diagnostic for a malformed permission set into a load-time `JsonException`, which is a worse tool for the line count it saved. ## Unavoidable edit outside this pass's area - `src/ATProtoNet.Server/Spaces/SpaceWriteNotifier.cs` (Pass B's file): one call site, `new NotifySpaceDeletedRequest { Space = space }` → `new NotifySpaceDeletedRequest(space)`, forced by that type's L1 conversion to a positional record. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 10 commits

2026-09-27 13:11:09 +00:00

refactor(lexicon)!: trim Enumerate* helpers to 13, make Pagination public (#180 ) d9a7fd8361

L3: 77 Enumerate*/ListAll* wrappers shrink to the 13 the tracking issues asked
for or 0.6.0 already had; the rest are gone. Http.Pagination, the cursor loop
behind all of them, is now public, so any other endpoint's GetXAsync/ListXAsync
walks with Pagination.EnumerateAsync<TPage, T>(...) in one line instead of a
dedicated wrapper.

Updates every doc snippet, sample and test that used a removed enumerator, and
drops the pagination tests that only re-verified the shared cursor loop (kept
where the endpoint's own wiring is bespoke, e.g. safelink/scheduled-actions
cursors traveling in a JSON body instead of the query string).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

refactor(lexicon)!: one CursorPage<T> base for the 72 cursored responses (#180 ) 158426f43b

L5: every com.atproto.*, Bluesky, chat and Ozone response with a Lexicon
cursor now derives from Models.CursorPage<T> instead of implementing
ICursorPage<T> by hand and redeclaring Cursor with the same doc comment 72
times. Items becomes a public override (was an explicit interface
implementation), marked [JsonIgnore] so the wire shape is unchanged.

The 72 responses become sealed records (records can only derive from other
records); GraphClient.GetListAsync's response renames its Items property to
Members since the Lexicon's own "items" field collided with the new Items
override. PublicAPI.Shipped.txt picks up the synthesized record members
(Equals, GetHashCode, ==, !=, ToString, the copy constructor) for each.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

refactor(lexicon): Unknown* union variants use primary constructors, one reflection theory (#180 ) 7c7f78b5c6

L4: the 38 straightforward Unknown* variants (everywhere but the two that also
read fields from their base type) collapse their 8-line constructor and
duplicated properties into one primary-constructor line, using the new
UnknownUnionVariant.RequireType helper alongside RequireObject.

OpenUnionTests' hand-picked 12-entry OpenUnionBases theory is now a reflection
scan for every [AtProtoUnion(typeof(Unknown...))] base (all 40), replacing
ConvoModelsTests' separate 9-case duplicate of the same "unknown $type reads
back byte-for-byte" behavior.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

refactor(lexicon)!: view inheritance, session response and stats merge (#180 ) f09b584835

L9: ProfileViewBasic <- ProfileView <- ProfileViewDetailed now inherit instead
of each redeclaring the fields they share (12, then +2, then +7 own fields).
GetSessionResponse is the base of SessionResponse the same way. Breaking: `is
ProfileView` now also matches ProfileViewDetailed.

Tools.Ozone.Report.LiveStats and Tools.Ozone.Queue.QueueStats were
byte-identical shapes for two different upstream defs (liveStats vs
queueStats); GetLiveStatsResponse.Stats is now QueueStats and LiveStats is
gone.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

refactor(lexicon): merge structurally-identical internal request DTOs (#180 ) 5efd7065c4

L1 (merges): 22 internal request DTOs collapse into 9 shared positional
records where two or more endpoints send the exact same body shape:

- ConvoIdRequest: the 10 chat.bsky.convo.*/group.* endpoints (leaveConvo,
  muteConvo, unmuteConvo, lockConvo, unlockConvo, acceptConvo,
  enableJoinLink, disableJoinLink, withdrawJoinRequest,
  updateJoinRequestsRead) that take only a convoId.
- ConvoReactionRequest: addReaction/removeReaction.
- GroupMembersRequest: addMembers/removeMembers.
- GroupMemberRequest: approveJoinRequest/rejectJoinRequest.
- SetValuesRequest: tools.ozone.set addValues/deleteValues.
- QueueModeratorRequest: tools.ozone.queue assignModerator/unassignModerator.
- AccountInvitesRequest: com.atproto.admin disable/enableAccountInvites.
- JobIdRequest: app.bsky.video finishUpload/abortUpload.
- HostnameRequest: com.atproto.sync notifyOfUpdate/requestCrawl.

Adds LexiconDriftTests.Allowlists DefOf entries for each, since a merged
type's name no longer resolves to either of its two upstream defs by
convention; both defs are identical, so checking against either validates
the shape. Wire format and NSIDs are unchanged.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

refactor(lexicon): remaining internal request DTOs become positional records (#180 ) 9d6807e5cf

L1 (positional records): the 80 remaining single-purpose internal request
DTOs collapse from an 8+ line class (doc comment, attribute, property, times
N fields) to one positional record line, with [property: JsonPropertyName]
per parameter and no XML docs (internal, CS1591-exempt).

11 of the 80 had a field order that isn't valid C# primary-constructor order
(a required field listed after an optional one); those get an explicit
[property: JsonPropertyOrder] per field so serialization keeps the original
Lexicon field order despite the constructor parameter order moving required
fields first. Verified against SendInteractionsRequest (feed before the
required interactions list) and spot-checked others against origin/main.

Updates every call site from object-initializer to (named-argument)
construction; one unavoidable edit outside this pass's area,
ATProtoNet.Server/Spaces/SpaceWriteNotifier.cs, for its NotifySpaceDeletedRequest
call.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

refactor(lexgen)!: one option-parsing helper, reuse SDK's LexiconPermission (#180 ) c2a0fbb04a

L11: atproto-lexgen's 6 commands (csharp, lexicon, lint, diff, publish,
resolve) shared no code for parsing their `args`; each hand-rolled a
for/switch loop. They now go through one ParseOptions helper (value options,
flags, --help, and an optional positional handler for resolve's NSIDs), with
identical behavior and error messages. Drops the `migrate` stub and
publish's legacy-option explanations (`--output`/`--baseline`/`--assembly`/
`--no-bump`), which only explained a removal that already shipped; both now
fail like any other unrecognized command/option, plus their tests.

L12: the tool's own LexiconPermission (a near-duplicate of the SDK's) is
gone; LexiconDocument.Permissions and the linter use
ATProtoNet.Lexicon.Com.AtProto.Lexicon.LexiconPermission directly. Its
`Resource` is required, so a permission-set entry missing it now fails to
parse (JsonException) instead of reaching the linter as a diagnostic; the
`type` discriminator and any field the SDK model doesn't name are read from
the inherited LexObject.ExtensionData instead of a duplicate property/field.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test(lexicon): drop the balanced-braces emitter test (#180 ) acdeefb8fe

L13: Emit_AnyDocument_ProducesBalancedBraces string-counted braces in
generated C# as a syntax sanity check; CSharpEmitterCompileTests actually
compiles the emitter's output with Roslyn, a strictly stronger check the
brace count could never catch anything that check would not.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

docs: add the missing CHANGELOG bullet for the DTO merges (#180 ) 04130ff6b2

The 5efd706 commit merging 22 structurally-identical internal request DTOs
into 9 shared positional records landed without its CHANGELOG bullet.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

revert(lexgen): keep the lenient permission type so lint reports a missing resource (#180 )

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

Details

CI / build-and-test (pull_request) Successful in 1m5s

Details

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

Details

e970b75c17

L12 (reusing the SDK's LexiconPermission, whose Resource is required) traded
a lenient lint diagnostic for a hard JsonException on load: a permission-set
entry missing "resource" no longer reached the linter at all. That is a
worse tool for the ~35 lines it saved, so the tool's own tolerant
LexiconPermission (Schema/LexiconDocument.cs) is back, and LexiconLinter
reads .Type/.OtherParameters from it directly again instead of ExtensionData.
Restores Lint_MalformedPermission_IsAnError's "'resource' is required" case
and drops the JsonException fact it had replaced.

The L11 option-parsing helper (ParseOptions in Program.cs) is unrelated and
untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

opencode-bot commented

2026-09-27 13:11:11 +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-183/run-2026-09-27T13-11-11-176Z.md) — updating every ~5 minutes while the review runs.

opencode-bot commented

2026-09-27 13:11:14 +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-27T18:11:11.686Z) — 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-27T18:11:11.686Z) — nothing to re-trigger.

Grandiras commented

2026-09-27 13:11:17 +00:00

Author

Owner

CursorPage<T>, the base record for the 72 cursor responses. JSON shape unchanged? ICursorPage preserved?
The Unknown* union variants in their primary-constructor form, and the RequireType helper. The new reflection theory must cover all 40.
The view inheritance (ProfileViewBasic ⊂ ProfileView ⊂ ProfileViewDetailed) and the SessionResponse/GetSessionResponse and QueueStats/LiveStats merges. Any serialization or polymorphism surprise? The description names one deliberate breaking change: is ProfileView now matches Detailed.
The internal request DTOs as positional records, the merged convoId-only DTOs, and the DefOf drift entries.
The public Http.Pagination and the 13 Enumerate* helpers kept. Do the docs and migration guide show the paginator one-liner for everything removed?
The lexgen ParseOptions helper.

@claude /review, please. This is debloat pass C of #180: the Lexicon surface and lexgen. The drift and convention tests guard the wire format. The diff will be truncated, so please check these parts on the branch: 1. `CursorPage<T>`, the base record for the 72 cursor responses. JSON shape unchanged? `ICursorPage` preserved? 2. The `Unknown*` union variants in their primary-constructor form, and the `RequireType` helper. The new reflection theory must cover all 40. 3. The view inheritance (`ProfileViewBasic` ⊂ `ProfileView` ⊂ `ProfileViewDetailed`) and the `SessionResponse`/`GetSessionResponse` and `QueueStats`/`LiveStats` merges. Any serialization or polymorphism surprise? The description names one deliberate breaking change: `is ProfileView` now matches Detailed. 4. The internal request DTOs as positional records, the merged convoId-only DTOs, and the `DefOf` drift entries. 5. The public `Http.Pagination` and the 13 `Enumerate*` helpers kept. Do the docs and migration guide show the paginator one-liner for everything removed? 6. The lexgen `ParseOptions` helper.

claude-bot commented

2026-09-27 13:11:20 +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/635); 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-183/run-635.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-09-27 15:54:38 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Large, mechanical debloat pass (L1, L3, L4, L5, L9, L11) with unusually good internal consistency for its size. I spot-checked the mechanics rather than trusting the description outright:

ProfileViewBasic / ProfileView / ProfileViewDetailed inheritance: manually recounted every field across the three original classes against the new base/derived split — all 21 fields of ProfileViewDetailed are accounted for with none dropped or duplicated (12 base + 2 ProfileView + 7 own). Matches the PR's own field counts exactly.
Models.CursorPage<T>: verified the actual definition in CommonModels.cs — an abstract record with Cursor and an abstract Items ([JsonIgnore]), exactly as described, and every converted response (GetFollowersResponse, GetListResponse, FeedResponse, etc.) overrides Items correctly, preserving the original wire property names.
GetLiveStatsResponse.Stats → QueueStats: confirmed in ReportModels.cs/QueueModels.cs — LiveStats is gone, GetLiveStatsResponse.Stats is now Tools.Ozone.Queue.QueueStats.
GetSessionResponse/SessionResponse base: confirmed SessionResponse : GetSessionResponse in ServerModels.cs.
UnknownUnionVariant.RequireType: confirmed in AtProtoUnionConverter.cs — same null/empty check as the old inline ArgumentException.ThrowIfNullOrEmpty, so the ~40 Unknown* primary-constructor variants preserve prior validation behavior exactly, including field-initializer order (Type then Raw, same as the original constructor body order).
SendInteractionsRequest: the JsonPropertyOrder pair (Feed=0, Interactions=1) correctly reproduces the original class's declaration order (Feed then Interactions) despite the primary-constructor parameter order having to put the non-default Interactions first.
GetListResponse.Items → Members: the rename keeps [JsonPropertyName("items")] on Members while Items becomes the CursorPage<T> override, so the wire shape is genuinely unchanged; this is called out correctly in both the CHANGELOG and migration guide.

I wasn't able to do a from-scratch dotnet build/test run in this session (worktree/build commands kept requiring interactive approval I couldn't grant), so I'm relying on code reading rather than a green build, but nothing I traced showed a mismatch between the description and the actual diff. Docs samples were also spot-checked (e.g. the ozone.md fully-qualified QueryEventsResponse/QueueStats types where two Ozone sub-namespaces would otherwise collide) and look correct.

This is a good, faithful, well-documented refactor.

⏱ 163m 6s · 21 turns · tokens: 106.5k in / 10.1k out (+2359.7k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Large, mechanical debloat pass (L1, L3, L4, L5, L9, L11) with unusually good internal consistency for its size. I spot-checked the mechanics rather than trusting the description outright: - **`ProfileViewBasic` / `ProfileView` / `ProfileViewDetailed` inheritance**: manually recounted every field across the three original classes against the new base/derived split — all 21 fields of `ProfileViewDetailed` are accounted for with none dropped or duplicated (12 base + 2 `ProfileView` + 7 own). Matches the PR's own field counts exactly. - **`Models.CursorPage<T>`**: verified the actual definition in `CommonModels.cs` — an abstract record with `Cursor` and an abstract `Items` (`[JsonIgnore]`), exactly as described, and every converted response (`GetFollowersResponse`, `GetListResponse`, `FeedResponse`, etc.) overrides `Items` correctly, preserving the original wire property names. - **`GetLiveStatsResponse.Stats` → `QueueStats`**: confirmed in `ReportModels.cs`/`QueueModels.cs` — `LiveStats` is gone, `GetLiveStatsResponse.Stats` is now `Tools.Ozone.Queue.QueueStats`. - **`GetSessionResponse`/`SessionResponse` base**: confirmed `SessionResponse : GetSessionResponse` in `ServerModels.cs`. - **`UnknownUnionVariant.RequireType`**: confirmed in `AtProtoUnionConverter.cs` — same null/empty check as the old inline `ArgumentException.ThrowIfNullOrEmpty`, so the ~40 `Unknown*` primary-constructor variants preserve prior validation behavior exactly, including field-initializer order (`Type` then `Raw`, same as the original constructor body order). - **`SendInteractionsRequest`**: the `JsonPropertyOrder` pair (`Feed`=0, `Interactions`=1) correctly reproduces the original class's declaration order (`Feed` then `Interactions`) despite the primary-constructor parameter order having to put the non-default `Interactions` first. - **`GetListResponse.Items` → `Members`**: the rename keeps `[JsonPropertyName("items")]` on `Members` while `Items` becomes the `CursorPage<T>` override, so the wire shape is genuinely unchanged; this is called out correctly in both the CHANGELOG and migration guide. I wasn't able to do a from-scratch `dotnet build`/`test` run in this session (worktree/build commands kept requiring interactive approval I couldn't grant), so I'm relying on code reading rather than a green build, but nothing I traced showed a mismatch between the description and the actual diff. Docs samples were also spot-checked (e.g. the ozone.md fully-qualified `QueryEventsResponse`/`QueueStats` types where two Ozone sub-namespaces would otherwise collide) and look correct. This is a good, faithful, well-documented refactor. --- ⏱ 163m 6s · 21 turns · tokens: 106.5k in / 10.1k out (+2359.7k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-183/run-635.md)

Grandiras merged commit 7058df77d9 into main

2026-09-27 15:55:44 +00:00

Grandiras deleted branch debloat-c-lexicon

2026-09-27 15:55:45 +00:00

Grandiras referenced this pull request from a commit

2026-09-27 15:55:45 +00:00

refactor!: debloat the Lexicon surface and lexgen (#183)

Grandiras referenced this pull request

2026-09-27 15:55:54 +00:00

refactor!: pre-release debloat pass #180

Grandiras referenced this pull request

2026-09-27 16:00:21 +00:00