Claude: changes for issue #85 #86

Merged
Grandiras merged 2 commits from claude/issue-85 into main 2026-08-14 13:46:13 +00:00
Collaborator

Closes #85.

Closes #85.
feat: add Jetstream v2 archive replay and snapshot support
All checks were successful
CI / pds-integration (pull_request) Successful in 25s
CI / build-and-test (pull_request) Successful in 56s
c0019f45cb
Implements the second and third ways to consume Jetstream v2 — the HTTP
archive — alongside the live tail #84 landed. `JetstreamReplayConsumer`
backfills history and cuts over into the live socket in one sequence-ordered
`await foreach`, with no gap at the seam.

- `JetstreamSegmentReader` decodes the sealed `.jss` format: 256-byte fixed
  header, length-prefixed zstd block frames, and the columnar block body.
  Streams block by block rather than materializing a segment, exposes the raw
  columnar row (including untouched CBOR payloads) for mirrors, and projects
  to the existing `JetstreamEvent` model. Verified against Jetstream's own
  golden block and a real sealed segment header.
- `JetstreamArchiveClient` wraps planSnapshot / listSegments / getSegment /
  getBlock with bearer auth, ETags, `Range` resume from the exact byte offset,
  and `Retry-After`-aware 429 handling — the endpoints are metered in response
  bytes, so resume and backoff are first-class rather than an afterthought.
- The plan loop pins the first page's `sealedTipSeq` as its ceiling, downloads
  with bounded parallelism while decoding strictly in plan order, re-applies
  the exact DID/collection/kind filter to what it decoded, and cuts over at the
  pinned tip with sequence dedup. A tip that ages out of the socket's lookback
  window re-enters the plan loop instead of skipping the gap.
- Configured by `JetstreamConsumerOptions.Archive`, sharing the live tail's
  filters and cursor store since a replay cursor is the same v2 sequence
  number.

Adds unit tests (segment decoder, archive client, replay pipeline), gated
integration tests, `samples/JetstreamReplaySample`, and replaces the
*Historical Replay* placeholder in docs/jetstream.md.

closes #85
Author
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/199); 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-86/run-199.md) — updating every ~5 minutes while the task runs.
claude-reviewer requested changes 2026-08-14 02:47:20 +00:00
Dismissed
claude-reviewer left a comment

🛑 Verdict: request changes — blocking issues below.

This is a large, well-documented addition of Jetstream v2 archive replay/snapshot support (plan/download/decode/filter, then cutover to the live tail). The .jss segment decoder, retry/resume logic in JetstreamArchiveClient, and the docs/sample/tests are thorough and internally consistent. One correctness concern in the backfill loop undermines the PR's central "no gap" guarantee; everything else looks solid (byte-range resume handles the ignored-Range case, 401 vs 429 retry semantics are correct, cursor persistence on every exit path via finally, collection-filter-only-applies-to-commits semantics match the live tail).


⏱ 2m 52s · 8 turns · tokens: 90.7k in / 17.1k out (+760.5k cache-read)
📜 Full session transcript

🛑 **Verdict: request changes** — blocking issues below. This is a large, well-documented addition of Jetstream v2 archive replay/snapshot support (plan/download/decode/filter, then cutover to the live tail). The `.jss` segment decoder, retry/resume logic in `JetstreamArchiveClient`, and the docs/sample/tests are thorough and internally consistent. One correctness concern in the backfill loop undermines the PR's central "no gap" guarantee; everything else looks solid (byte-range resume handles the ignored-Range case, 401 vs 429 retry semantics are correct, cursor persistence on every exit path via `finally`, collection-filter-only-applies-to-commits semantics match the live tail). --- ⏱ 2m 52s · 8 turns · tokens: 90.7k in / 17.1k out (+760.5k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-86/run-199.md) <!-- claude-verdict:request_changes --> <!-- claude-reviewed-head:c0019f45cbe5d59c88559318fd316dd512bee7f0 -->
@ -0,0 +283,4 @@
// The server truncates a page at a whole work-unit boundary and always admits at least
// one, so plannedThroughSeq advances every page. Stopping when it does not is what
// keeps a server that disagrees from looping forever over the same page.
if (plan.PlannedThroughSeq <= planned)
Collaborator

BackfillAsync silently truncates the backfill when a plan page fails to advance past the previously planned sequence (plan.PlannedThroughSeq <= planned) — it just logs a warning and breaks, even though the loop is still below the pinned ceiling tip/PinnedTipSeq. Back in ReplayAsync, the cutover then connects the live socket at tip = PinnedTipSeq ?? … (line ~168), which is inclusive and drops everything at or below LastCursor. Any sequence range between the actual last delivered event and the pinned ceiling that was skipped by this early break is therefore never delivered by either phase — a permanent, silent gap, with nothing surfaced to the caller besides a log line (no exception, no flag on the consumer).

This directly contradicts the feature's advertised guarantee (CHANGELOG: "the cutover is inclusive and deduplicated... no gap"; docs: "nothing is lost at the handoff") and the code's own comment two lines above claiming paging "always progresses." The scenario isn't purely hypothetical: if plannedThroughSeq reflects what the server has actually sealed/served rather than a hard filter-derived count, a lag between when sealedTipSeq was reported and when the corresponding segments become fetchable would produce exactly this non-advancing response — a plausible race, not just server misbehavior.

Consider instead treating a non-advancing plan below the ceiling as a retryable condition (bounded retries/backoff) and only failing loudly (throwing, e.g. a JetstreamArchiveException) if it truly cannot make progress, rather than silently accepting data loss and proceeding to cut over.

`BackfillAsync` silently truncates the backfill when a plan page fails to advance past the previously planned sequence (`plan.PlannedThroughSeq <= planned`) — it just logs a warning and `break`s, even though the loop is still below the pinned ceiling `tip`/`PinnedTipSeq`. Back in `ReplayAsync`, the cutover then connects the live socket at `tip = PinnedTipSeq ?? …` (line ~168), which is inclusive and drops everything at or below `LastCursor`. Any sequence range between the actual last delivered event and the pinned ceiling that was skipped by this early break is therefore **never delivered by either phase** — a permanent, silent gap, with nothing surfaced to the caller besides a log line (no exception, no flag on the consumer). This directly contradicts the feature's advertised guarantee (CHANGELOG: "the cutover is inclusive and deduplicated... no gap"; docs: "nothing is lost at the handoff") and the code's own comment two lines above claiming paging "always progresses." The scenario isn't purely hypothetical: if `plannedThroughSeq` reflects what the server has actually sealed/served rather than a hard filter-derived count, a lag between when `sealedTipSeq` was reported and when the corresponding segments become fetchable would produce exactly this non-advancing response — a plausible race, not just server misbehavior. Consider instead treating a non-advancing plan below the ceiling as a retryable condition (bounded retries/backoff) and only failing loudly (throwing, e.g. a `JetstreamArchiveException`) if it truly cannot make progress, rather than silently accepting data loss and proceeding to cut over.
Owner
@claude-bot
Author
Collaborator

🤖 On it — follow along in the action run; the reply will link the full session transcript.

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

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/214); 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/issue-86/run-214.md) — updating every ~5 minutes while the task runs.
fix: fail loudly when the Jetstream backfill stops advancing
All checks were successful
CI / pds-integration (pull_request) Successful in 34s
CI / build-and-test (pull_request) Successful in 1m2s
Sync Closures to GitHub / sync-closure (pull_request) Successful in 7s
78357303f2
Addresses review feedback on #86. The plan loop treated a page whose
`plannedThroughSeq` did not advance as "the archive is done", logged a
warning, and broke out — even with the pinned ceiling still ahead. The
cutover then reconnected at that ceiling, which is inclusive, so the
range between the last delivered event and the pinned tip was delivered
by neither phase: a permanent, silent gap behind the feature's own
no-gap guarantee, with nothing surfaced to the caller.

A non-advancing page below the ceiling is now a stall, not a finish: the
consumer re-plans after an exponential wait (from a second, capped by
`MaxRetryDelay`) up to the new `MaxStalledPlanAttempts` (default 5), and
then throws a `JetstreamArchiveException` naming the sequence it got
through. The stall is detected before the download, so waiting out a lag
between a sealed tip being reported and its segments becoming servable
costs no re-fetched (metered) bytes. A page that does not advance while
at or above the ceiling is still a clean finish.

Also caps the pinned ceiling with `BeforeSeq`: it was only sent on the
first plan request, so from the second page a bounded snapshot adopted
`sealedTipSeq` as its ceiling and ran past the caller's bound.

Adds coverage for the stall-then-fail, stall-then-recover, resume-at-tip
and `BeforeSeq`-ceiling paths.
Author
Collaborator

Fixed the blocking review finding and pushed to claude/issue-85 (commit 7835730).

The gap

BackfillAsync treated a plan page whose plannedThroughSeq didn't advance as "the archive is exhausted" — warn and break — even with the pinned ceiling still ahead. ReplayAsync then connected the live socket at PinnedTipSeq, which is inclusive, so the range between the last delivered event and that ceiling was delivered by neither phase. The reviewer's read is right, and the lag case (a sealed tip reported before its segments are servable) makes it a plausible race rather than only server misbehaviour.

A non-advancing page below the ceiling is now a stall, not a finish:

  • re-plan after an exponential wait (from 1s, capped by MaxRetryDelay), up to the new JetstreamArchiveOptions.MaxStalledPlanAttempts (default 5);
  • then throw JetstreamArchiveException, naming the sequence it got through and the tip it couldn't reach, so the caller can resume from it instead of cutting over across a hole;
  • the stall is detected before the download, so waiting out the lag costs no re-fetched bytes — these endpoints are metered on the wire;
  • a page that doesn't advance while already at or above the ceiling (e.g. resuming exactly at the sealed tip) is still a clean, silent finish.

Adjacent bug found while there

BeforeSeq was only sent on the first plan request — from the second page on, beforeSeq = tip ?? _archive.BeforeSeq resolved to the pinned sealedTipSeq, and the row filter used that same value as its ceiling. A bounded snapshot therefore ran past the caller's bound all the way to the sealed tip. The ceiling is now min(BeforeSeq, sealedTipSeq). This also makes the "one segment's worth of history, so the test spends a bounded number of bytes" integration test actually bounded.

Docs & tests

CHANGELOG.md (amended in place — the feature is still unreleased) and the How it works section of docs/jetstream.md now state the retry-then-fail behaviour and the BeforeSeq cap, replacing the "paging always progresses" claim the old code leaned on. The APlanThatDoesNotAdvanceEndsTheBackfillInsteadOfLooping test asserted the buggy behaviour, so it's replaced by four: stall-then-fail (asserting bounded re-plans and that no segment is re-downloaded), stall-then-recover, resume-at-the-sealed-tip, and the BeforeSeq ceiling on later pages.

Verified: installed the .NET 10 SDK in the container (none was present); dotnet build clean across the solution with 0 warnings, dotnet test tests/ATProtoNet.Tests/1291 passed, 0 failed, 5 skipped (the Aspire manifest skips are pre-existing), and the integration test project compiles. Integration tests themselves weren't run — they need a live PDS/archive key.


⏱ 6m 28s · 39 turns · tokens: 71.0k in / 21.7k out (+2512.7k cache-read)
📜 Full session transcript

Fixed the blocking review finding and pushed to `claude/issue-85` (commit `7835730`). ## The gap `BackfillAsync` treated a plan page whose `plannedThroughSeq` didn't advance as *"the archive is exhausted"* — warn and `break` — even with the pinned ceiling still ahead. `ReplayAsync` then connected the live socket at `PinnedTipSeq`, which is inclusive, so the range between the last delivered event and that ceiling was delivered by **neither** phase. The reviewer's read is right, and the lag case (a sealed tip reported before its segments are servable) makes it a plausible race rather than only server misbehaviour. A non-advancing page below the ceiling is now a **stall, not a finish**: - re-plan after an exponential wait (from 1s, capped by `MaxRetryDelay`), up to the new `JetstreamArchiveOptions.MaxStalledPlanAttempts` (default 5); - then throw `JetstreamArchiveException`, naming the sequence it got through and the tip it couldn't reach, so the caller can resume from it instead of cutting over across a hole; - the stall is detected **before** the download, so waiting out the lag costs no re-fetched bytes — these endpoints are metered on the wire; - a page that doesn't advance while already *at or above* the ceiling (e.g. resuming exactly at the sealed tip) is still a clean, silent finish. ## Adjacent bug found while there `BeforeSeq` was only sent on the *first* plan request — from the second page on, `beforeSeq = tip ?? _archive.BeforeSeq` resolved to the pinned `sealedTipSeq`, and the row filter used that same value as its ceiling. A bounded snapshot therefore ran past the caller's bound all the way to the sealed tip. The ceiling is now `min(BeforeSeq, sealedTipSeq)`. This also makes the "one segment's worth of history, so the test spends a bounded number of bytes" integration test actually bounded. ## Docs & tests `CHANGELOG.md` (amended in place — the feature is still unreleased) and the *How it works* section of `docs/jetstream.md` now state the retry-then-fail behaviour and the `BeforeSeq` cap, replacing the "paging always progresses" claim the old code leaned on. The `APlanThatDoesNotAdvanceEndsTheBackfillInsteadOfLooping` test asserted the buggy behaviour, so it's replaced by four: stall-then-fail (asserting bounded re-plans *and* that no segment is re-downloaded), stall-then-recover, resume-at-the-sealed-tip, and the `BeforeSeq` ceiling on later pages. **Verified:** installed the .NET 10 SDK in the container (none was present); `dotnet build` clean across the solution with 0 warnings, `dotnet test tests/ATProtoNet.Tests/` → **1291 passed, 0 failed, 5 skipped** (the Aspire manifest skips are pre-existing), and the integration test project compiles. Integration tests themselves weren't run — they need a live PDS/archive key. --- ⏱ 6m 28s · 39 turns · tokens: 71.0k in / 21.7k out (+2512.7k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-86/run-214.md)
Owner
@claude-reviewer
Author
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/217); 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-86/run-217.md) — updating every ~5 minutes while the task runs.
claude-reviewer left a comment

Verdict: approve — no blocking issues found.

Re-review: the blocking issue from my previous pass is fixed. BackfillAsync in src/ATProtoNet/Streaming/JetstreamReplayConsumer.cs no longer silently breaks out of the backfill loop when a plan page fails to advance below the pinned ceiling. It now retries with exponential backoff (StallDelay, capped by MaxRetryDelay) up to JetstreamArchiveOptions.MaxStalledPlanAttempts (default 5), and if the plan genuinely cannot make progress it throws a JetstreamArchiveException instead of proceeding to cut over across the gap. This matches the documented guarantee (CHANGELOG, docs/jetstream.md) that a stalled plan either catches up or fails loudly rather than silently dropping a range that the inclusive cutover would never redeliver. The boundary conditions (planned < ceiling, plannedThroughSeq <= planned) look correct: a plan that starts already at or above the ceiling skips the stall check and proceeds straight to the terminal break, and a genuinely stalled plan resets stalls back to zero as soon as it advances.

I didn't spot other correctness issues in the areas I re-checked (cutover retry/re-plan flow, cursor tracking/dedup in TrackAsync, the bounded-channel download pipeline in DownloadAsync, and the .jss segment/block decoding math). The rest of the PR (docs, sample, archive client retry/resume semantics, segment reader) was already reviewed favorably last pass and is unchanged in substance. Good to merge.


⏱ 0m 48s · 7 turns · tokens: 78.6k in / 3.9k out (+627.0k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Re-review: the blocking issue from my previous pass is fixed. `BackfillAsync` in `src/ATProtoNet/Streaming/JetstreamReplayConsumer.cs` no longer silently `break`s out of the backfill loop when a plan page fails to advance below the pinned ceiling. It now retries with exponential backoff (`StallDelay`, capped by `MaxRetryDelay`) up to `JetstreamArchiveOptions.MaxStalledPlanAttempts` (default 5), and if the plan genuinely cannot make progress it throws a `JetstreamArchiveException` instead of proceeding to cut over across the gap. This matches the documented guarantee (CHANGELOG, docs/jetstream.md) that a stalled plan either catches up or fails loudly rather than silently dropping a range that the inclusive cutover would never redeliver. The boundary conditions (`planned < ceiling`, `plannedThroughSeq <= planned`) look correct: a plan that starts already at or above the ceiling skips the stall check and proceeds straight to the terminal break, and a genuinely stalled plan resets `stalls` back to zero as soon as it advances. I didn't spot other correctness issues in the areas I re-checked (cutover retry/re-plan flow, cursor tracking/dedup in `TrackAsync`, the bounded-channel download pipeline in `DownloadAsync`, and the `.jss` segment/block decoding math). The rest of the PR (docs, sample, archive client retry/resume semantics, segment reader) was already reviewed favorably last pass and is unchanged in substance. Good to merge. --- ⏱ 0m 48s · 7 turns · tokens: 78.6k in / 3.9k out (+627.0k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-86/run-217.md) <!-- claude-verdict:approve --> <!-- claude-reviewed-head:78357303f2126b80965c5a782d5f395f62750e2a -->
Grandiras deleted branch claude/issue-85 2026-08-14 13:46:13 +00:00
Grandiras referenced this pull request from a commit 2026-08-14 13:46:14 +00:00
Sign in to join this conversation.
No description provided.