Grandiras/ATProto.NET
1
1
Fork 0
Code Issues Pull requests Projects Releases 9 Packages 8 Wiki Activity Actions

refactor!: pre-release debloat pass #180

New issue
Closed
opened 2026-09-27 11:52:44 +00:00 by Grandiras · 1 comment
Grandiras commented 2026-09-27 11:52:44 +00:00
Owner
Copy link

Part of the v0.7.0 update cycle (tracking #141). A pre-release debloat pass: the cycle added a lot of protocol surface the owner asked for, and v0.6.0 code got leaner (about −4,100 code lines of old src/ code removed), but three read-only audits found duplication and over-structure inside the new code: about 3,700 code, 2,500 XML-doc and 5,900 test lines removable without losing a feature.

Where the bloat is

  • Duplicated plumbing:
    • Six copies of the "resolve key → verify → refetch once → retry" loop (firehose, sync, labels, service auth, two in Spaces).
    • Jetstream and Tap keep their own reconnect loops beside the shared stream core.
    • Commit blocks are parsed three times, and three MST walkers each decompress keys.
    • Four bounded readers, five hand-rolled caches, five ws↔http converters and two base32 decoders.
  • Repeated structure in the Lexicon models:
    • 72 responses each redeclare Cursor.
    • 40 Unknown* union classes run to 20 lines each.
    • 106 internal request DTOs are written as fully documented classes.
    • 77 Enumerate* helpers, where the issues asked for about 13.
  • Surface without a consumer: option knobs, hooks, public members only tests use, and single-implementation interfaces.
  • Tests: about 270 per-endpoint "hits NSID X with param Y" tests that a table-driven test plus a drift check of parameter names would cover. There are also duplicate fakes, and copies of JWS/identifier cases.

Owner decisions

  • Drop FirehoseVerifier (use SyncVerifier).
  • Trim Enumerate* to the requested ones and make the paginator public.
  • Fold FirehoseClient/JetstreamClient into the consumers.
  • Keep CS1591 everywhere.

Passes (one PR each)

  • A: (#182) streaming, repo and core wire primitives.
  • B: (#181) auth, hosting and Spaces.
  • C: (#183) Lexicon surface and the lexgen tool.
  • D: (#185) shared primitives: the verify-with-refresh helper, one bounded reader, one cache, the JSON helpers and the XrpcClient send path.
  • E: (#186, #189) tests: table-driven endpoint tests plus the drift parameter check, the shared test kit, and removing duplicate tests.
  • F: (#184, D-owned files follow in pass D) docs: each rationale once.

Every breaking change gets a Breaking changes CHANGELOG bullet with a migration note, and an entry in docs/migrating-to-0.7.md. Each PR reports the before/after line counts for its area.

Part of the v0.7.0 update cycle (tracking #141). A **pre-release debloat pass**: the cycle added a lot of protocol surface the owner asked for, and v0.6.0 code got leaner (about −4,100 code lines of old `src/` code removed), but three read-only audits found duplication and over-structure *inside* the new code: about **3,700 code, 2,500 XML-doc and 5,900 test lines** removable without losing a feature. ## Where the bloat is - **Duplicated plumbing:** - Six copies of the "resolve key → verify → refetch once → retry" loop (firehose, sync, labels, service auth, two in Spaces). - Jetstream and Tap keep their own reconnect loops beside the shared stream core. - Commit blocks are parsed three times, and three MST walkers each decompress keys. - Four bounded readers, five hand-rolled caches, five ws↔http converters and two base32 decoders. - **Repeated structure in the Lexicon models:** - 72 responses each redeclare `Cursor`. - 40 `Unknown*` union classes run to 20 lines each. - 106 internal request DTOs are written as fully documented classes. - 77 `Enumerate*` helpers, where the issues asked for about 13. - **Surface without a consumer:** option knobs, hooks, public members only tests use, and single-implementation interfaces. - **Tests:** about 270 per-endpoint "hits NSID X with param Y" tests that a table-driven test plus a drift check of parameter names would cover. There are also duplicate fakes, and copies of JWS/identifier cases. ## Owner decisions - Drop `FirehoseVerifier` (use `SyncVerifier`). - Trim `Enumerate*` to the requested ones and make the paginator public. - Fold `FirehoseClient`/`JetstreamClient` into the consumers. - Keep CS1591 everywhere. ## Passes (one PR each) - [x] **A:** (#182) streaming, repo and core wire primitives. - [x] **B:** (#181) auth, hosting and Spaces. - [x] **C:** (#183) Lexicon surface and the lexgen tool. - [x] **D:** (#185) shared primitives: the verify-with-refresh helper, one bounded reader, one cache, the JSON helpers and the XrpcClient send path. - [x] **E:** (#186, #189) tests: table-driven endpoint tests plus the drift parameter check, the shared test kit, and removing duplicate tests. - [x] **F:** (#184, D-owned files follow in pass D) docs: each rationale once. Every breaking change gets a `Breaking changes` CHANGELOG bullet with a migration note, and an entry in `docs/migrating-to-0.7.md`. Each PR reports the before/after line counts for its area.
Grandiras commented 2026-09-27 21:27:32 +00:00
Author
Owner
Copy link

All six passes have merged: A #182, B #181, C #183, D #185, E #186 and #189, F #184.

Totals, from main before the pass (573747c) to now:

before after Δ
src code 41,345 38,369 -2,976
tests code 50,883 46,191 -4,692
tools code 3,203 3,200 -3
src XML doc 22,477 15,306 -7,171
src // comments 1,379 3,876 +2,497

About a third of the removed XML doc was rationale on internal members, and it became plain // comments rather than being dropped. The net doc-plus-comment reduction is about 4,700 lines.

What went:

  • the six copies of refetch-once verification, the duplicate reconnect loops, the three commit-block parsers and three MST key walkers, four bounded readers, nine hand-rolled caches, and five ws↔http converters;
  • FirehoseVerifier, FirehoseClient and JetstreamClient, per the owner's decision;
  • 64 of the 77 Enumerate* helpers; the paginator is now public instead;
  • about 270 per-endpoint tests, now covered by a 161-row table plus a query-parameter drift check over all 143 call sites, which adds coverage;
  • single-implementation interfaces, knobs without a consumer, and test-only public members.

Bugs the adversarial reviews caught before merge:

  • a startup crash with a scoped replay store;
  • two Jetstream regressions;
  • a consumer crash on a signed commit with a malformed data link;
  • commit bytes after the map;
  • a lexicon-fetch deadlock on a caller's context;
  • a lost-update race;
  • a cache capacity overshoot.

The performance follow-up is #190.

All six passes have merged: A #182, B #181, C #183, D #185, E #186 and #189, F #184. **Totals**, from main before the pass (`573747c`) to now: | | before | after | Δ | |---|---:|---:|---:| | src code | 41,345 | 38,369 | -2,976 | | tests code | 50,883 | 46,191 | -4,692 | | tools code | 3,203 | 3,200 | -3 | | src XML doc | 22,477 | 15,306 | -7,171 | | src // comments | 1,379 | 3,876 | +2,497 | About a third of the removed XML doc was rationale on internal members, and it became plain `//` comments rather than being dropped. The net doc-plus-comment reduction is about 4,700 lines. **What went:** - the six copies of refetch-once verification, the duplicate reconnect loops, the three commit-block parsers and three MST key walkers, four bounded readers, nine hand-rolled caches, and five ws↔http converters; - `FirehoseVerifier`, `FirehoseClient` and `JetstreamClient`, per the owner's decision; - 64 of the 77 `Enumerate*` helpers; the paginator is now public instead; - about 270 per-endpoint tests, now covered by a 161-row table plus a query-parameter drift check over all 143 call sites, which adds coverage; - single-implementation interfaces, knobs without a consumer, and test-only public members. **Bugs the adversarial reviews caught before merge:** - a startup crash with a scoped replay store; - two Jetstream regressions; - a consumer crash on a signed commit with a malformed `data` link; - commit bytes after the map; - a lexicon-fetch deadlock on a caller's context; - a lost-update race; - a cache capacity overshoot. The performance follow-up is #190.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
v0.8.0
v0.7.0
v0.6.0
v0.5.0
v0.4.0
v0.3.0
v0.2.0
v0.1.1
v0.1.0
Labels
Clear labels   
breaking-change
Introduces a breaking API change

  
bug
Something is not working correctly

  
documentation
Documentation improvements

  
duplicate
This issue or PR already exists

  
enhancement
New feature or improvement

  
good first issue
Good for newcomers to the project

  
help wanted
Extra attention is needed

  
performance
Performance-related issue or improvement

  
question
Further information is requested

  
wontfix
This will not be worked on

No labels
breaking-change
bug
documentation
duplicate
enhancement
good first issue
help wanted
performance
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Grandiras/ATProto.NET#180
No description provided.