feat(qwp): stop resending the full symbol dictionary on every message

[glasstiger]

Tandem

This change lands together with its counterparts (merge as a set):

• OSS: feat(qwp): stop resending the full symbol dictionary on every message questdb#7374 -- bumps the java-questdb-client submodule, and adds one server-side change: the ingress decoder now rejects a delta symbol dictionary whose start id runs past the connection dictionary, atomically and with a dedicated retriable error, instead of null-padding the hole. See "Server-side gap rejection" below.
• Enterprise: questdb/questdb-enterprise#1122 -- bumps the client so the failover suite (SqlFailoverQwpClientLosslessTest, file-mode failover) runs end-to-end against this change.

Summary

Every QWP ingress message used to carry the entire symbol dictionary, so a connection that ingests many distinct symbols re-transmitted the whole dictionary on every message. This change makes the client register each symbol id with the server only once per connection and send only new ids (a delta) thereafter, re-registering the full dictionary when a connection is replaced.

The bandwidth saving grows with symbol cardinality and message count; for low-cardinality or short-lived connections it is negligible, and the change adds the costs described under Tradeoffs.

What changed

Memory mode

• The producer keeps a monotonic "sent" watermark; each frame's dictionary section carries only the ids above it instead of the full dictionary from id 0.
• On reconnect or failover the fresh server starts with an empty dictionary, so the I/O thread replays the whole dictionary as a catch-up frame before any post-reconnect traffic. The producer's monotonic baseline is deliberately preserved across the wire boundary rather than reset.

Store-and-forward (file mode)

• Each slot persists its dictionary to a dot-prefixed side-file (PersistedSymbolDict) so a recovered or orphan-drained slot on a fresh process -- which has no in-memory dictionary -- can rebuild what its (non-self-sufficient) delta frames reference.
• Write-ahead ordering: new symbols are appended to the side-file before the frame that references them is published to the ring.

Catch-up split

• The reconnect/recovery catch-up splits across as many frames as the server's advertised batch cap requires, so a dictionary larger than the cap is re-registered without any single frame exceeding it. The frames carry contiguous id ranges and reassemble on the server exactly as the original per-frame deltas would. When the server advertises no cap, or the whole dictionary fits, the behaviour is unchanged (a single frame).

Full-dictionary mode: the dictionary chunks when it outgrows the batch cap

A full-dictionary frame carries the whole dictionary from id 0, so its fixed overhead grows with lifetime symbol cardinality. Against the OSS default DEFAULT_MAX_BATCH_SIZE (16 MiB) that overhead reaches the cap at roughly 800k symbols of 20 bytes, ~165k of 100 bytes, or ~16k of 1 KB. Past that point every frame was oversized however the batch was split: the split pre-flight rejected it, reset() discards rows rather than the dictionary so the next batch failed identically, and the sender could not flush again until it was closed and rebuilt. Two paths reached it -- a mid-life degrade (disableDeltaDict) on a large delta-mode dictionary, and, with no fault at all, ordinary growth on a slot whose .symbol-dict never opened.

The producer now registers the dictionary up front as deferred, table-less frames, each carrying a contiguous id range sized under the cap -- the same chunking the reconnect catch-up already does -- and the batch's data frames follow with an empty delta.

That makes the data frames depend on the chunks, which full-dictionary mode otherwise avoids. The dependency is safe one level up: the server does not ack a deferred frame individually (QwpIngressUpgradeProcessor marks uncommitted deferred rows so the cumulative-ack watermark cannot pass them, and QwpIngressProcessorState clamps and logs critical if it ever tries), so a deferred group cannot be trimmed part-way. The group is self-sufficient even though its frames are not, and recovery replays it whole with the chunk deltas folded before the data frames.

Delta mode is deliberately excluded: there the section covers only the batch's new symbols, and publishing before persistNewSymbolsBeforePublish would break the write-ahead ordering -- a crash in between would leave frames referencing ids the .symbol-dict cannot describe. The pre-registration is also a no-op unless the dictionary section leaves no room for a table body, so behaviour below that threshold is unchanged.

Server-side gap rejection (OSS half)

Delta framing makes a non-zero start id reachable on the wire for the first time, so the decoder's handling of one now matters. QwpMessageCursor.parseDeltaSymbolDict grew the connection dictionary with nulls up to deltaStartId + deltaCount, which inflated size() -- the very bound QwpSymbolColumnCursor checks an incoming symbol index against. A row referencing a padded id therefore passed the bounds check, read back null, and landed a NULL symbol value with no error.

The decoder now rejects deltaStartId > size() with its own error code, DELTA_DICT_GAP, surfaced to the sender as a new wire status byte, STATUS_DICTIONARY_GAP (0x0D). The gap verdict depends on this connection's dictionary coverage -- server state, not the frame's bytes -- so unlike a parse error it is retriable: the server sends the NACK and keeps the connection open, and the sender recycles the wire and re-registers from an id the server actually holds. A contiguous append (deltaStartId == size()) and a lower start that re-registers or remaps existing ids both stay allowed. The parse is atomic on failure: a rejected delta restores every entry it overwrote and nulls the slots it grew into, so the connection dictionary is exactly what it was before the frame and can never hold a null.

Wire-compat note: the server now rejects a frame shape it previously (wrongly) accepted, and 0x0D is a status byte no earlier server emitted, under an unchanged protocol version. QWP is experimental and unreleased, and the bundled client moves in lockstep with the server, which is what the tandem labels assert; this client maps an unknown status byte to a retriable category, so an older bundled client against a newer server degrades to retry rather than failing. This client cannot emit a gapped frame -- its send loop refuses to -- so the guard exists for a client bug, a torn store-and-forward dictionary, or a third-party implementation.

Symbol dictionary capacity

The server caps a connection's symbol dictionary at 1,000,000 distinct values (MAX_SYMBOL_DICTIONARY_SIZE, pre-existing). Before this change the practical ceiling was far lower: every message re-shipped the dictionary prefix from id 0, so per-message cost grew with lifetime cardinality and a large dictionary outgrew the frame budget long before the cap. Delta encoding removes that per-message cost, which makes the protocol cap the binding constraint for the first time — and because the producer's baseline is lifetime-monotonic, the reconnect catch-up would trip the server's rejection on every reconnect, including recovered slots and orphan drainers, stranding an already-buffered store-and-forward backlog with no error ever reaching the producer.

The client therefore enforces the cap at registration: creating the 1,000,001st distinct symbol value throws a LineSenderException from symbol() naming the limit and the recovery, before the row is buffered. Rows using already-registered values are unaffected. Everything buffered stays deliverable — the server's check is >, so a dictionary of exactly the cap still catches up cleanly. To reset the id space, close the sender and build a new one: a fully drained close removes the slot's dictionary side-file, so the rebuilt sender starts fresh. Reaching a million distinct values in symbol columns usually means the data belongs in varchar.

The server-side rejection itself keeps its parse-error (terminal) classification: with the registration guard, this client cannot reach it, the same unreachability argument the gap status relies on for old clients.

Recovery-time side-file disposition

PersistedSymbolDict.open() — the recovery entry point — now mirrors the Rust client's open_recovered disposition matrix:

• A transient I/O failure against an existing side-file (stat error, failed open, mmap or short read, failed torn-tail truncate, late mmap fault) throws the retriable SfOperationalException instead of silently degrading to full-dictionary frames. Sender.build() aborts without quarantining and BackgroundDrainer leaves the slot for a later scan, so a transient can no longer permanently quarantine an intact backlog, and a degraded session can no longer write frames next to a stale populated side-file that a later recovery would trust — the silent cross-generation symbol-misattribution chain loses its only organic entry point.
• A provably absent or corrupt side-file (bad magic/version, sub-header stub) still degrades to full-dictionary frames, and the recovery path no longer fabricates a fresh empty side-file next to recovered segments. Both dispositions are sticky across restarts, so consecutive sessions cannot disagree about the slot's mode.
• openClean() (the fresh-slot truncate-or-refuse path) is unchanged.

Coverage equivalent to the earlier generation-stamp test (testRecoveryDiscardsADictionaryFromAnotherGeneration, removed with the stamp) is restored by testTransientDictFaultOnRecoveredSlotFailsLoudAndRetryRecoversInFull, which drives the three-session chain end-to-end and proves it now breaks at session B with the slot byte-identical, nothing quarantined, and a full recovery on retry.

Slot quarantine: deterministic recovery failures set the slot aside

A recovery failure that is deterministic -- a torn slot whose surviving frames cannot be replayed without corrupting data, an unreadable interior segment, a corrupt segment chain -- no longer aborts Sender.build() forever or spins the orphan drainer. Sender.build() and BackgroundDrainer catch the typed exceptions (UnreplayableSlotException, SfRecoveryException, MmapSegmentCorruptionException), rename the whole slot directory aside for operator attention, dispatch a synchronous SenderError, and continue on a fresh slot. Renaming the whole directory guarantees the replacement starts empty and cannot fail the same way twice. Operational failures -- e.g. a drained-slot leftover whose unlink fails -- deliberately stay plain aborts that retry, rather than quarantining data that is still deliverable.

Mmap faults on the dictionary path degrade instead of killing the sender

MmapSegment.isMmapAccessFault recognizes the InternalError HotSpot raises for an access to an unbacked page (delivered asynchronously before JDK 21, JDK-8283699). The dictionary-side consumers (persistNewSymbolsBeforePublish, healPersistedDictionary) treat a recognized fault as a persist failure and degrade the sender to full self-sufficient frames (disableDeltaDict) instead of propagating an untyped Error; an unrecognized InternalError still propagates. Segment recovery itself validates every page through positioned reads before mapping, so the recovery scan cannot hit a late-delivered fault on pages it has not already read.

Reconnect policy: post-connect endpoint rejections retry instead of killing the producer

Once a foreground sender has completed its first connection (including the dictionary catch-up), a later WebSocket upgrade rejection or durable-ack capability mismatch no longer latches a producer-fatal terminal: the send loop retries with backoff while store-and-forward keeps buffering, and the failure is reported through SenderError dispatch. At build/initialization time these failures still surface loudly. Auth failures on the orphan drainer, and initialization-time failures in all modes, keep their previous terminal behaviour. hasEverConnected latches only after the catch-up succeeds, so a first connection that fails inside the catch-up still counts as never-connected and keeps endpoint-policy failures terminal.

P-C8: .symbol-dict bytes count against sf_max_total_bytes

The provisioning cap check compared .sfa segment bytes only, while the
symbol dictionary's side-file grows monotonically over the sender's
lifetime -- so dictionaries could fill the SF filesystem while the cap
reported headroom. SegmentManager now reads a live per-slot gauge
(PersistedSymbolDict.occupiedDiskBytes(), wired at engine
registration) at every cap check, and the throttled disk-full warning
breaks the dictionary component out as sideFileBytes=. Memory mode and
degraded full-dict sessions are unaffected (no side-file, no gauge).

The gauge reports the side-file's real footprint, not its logical one.
ensureAppendMap rounds the append window up to APPEND_MAP_CAPACITY
and calls Files.allocate, which reserves real blocks, and close()
returns that tail only at the end of the session -- so a live slot
occupies up to 4 MiB more than its committed prefix throughout the run,
not just after a crash. appendedBytes() keeps the logical meaning,
which a reopen preserves; occupiedDiskBytes() is max(committed, reserved).

Counting a component no trim can reclaim needs a liveness floor, or the
cap deadlocks instead of backpressuring. totalBytes never falls below
the active segment and the dictionary never shrinks, so once side-file
bytes passed maxTotalBytes - 2 * segmentSizeBytes the manager could
never provision a hot spare again: the ring stalled at one segment, the
producer saw a 30 s appendBlocking timeout, and the state survived
restarts -- while the disk-full warning pointed at an ACK-driven trim
that cannot free dictionary bytes. The cap check now guarantees every
ring its minimum working set (the active segment plus one spare): when
the cap refuses and the ring is below that floor, the manager provisions
anyway and warns separately, naming the remedy the operator actually has
(raise sf_max_total_bytes, or reduce symbol cardinality). The ring
then cycles between one and two segments as acks arrive and ingestion
continues, overshooting the cap by what the dictionary needs rather than
stopping. Above the floor the cap governs unchanged, so segment bytes --
which trim does reclaim -- still produce ordinary backpressure.

Durability

The persisted dictionary intentionally does not fsync, matching the rest of store-and-forward: it is process-crash durable (the OS page cache survives a JVM crash) but not host-crash durable. Rather than fsync only the dictionary -- which would not make the frame data itself host-crash durable -- a host crash that tears the dictionary is caught rather than silently trusted. Each side-file chunk carries a CRC-32C over its header and batched entry bytes (the same checksum the SF segment frames use), so recovery stops at the first torn or mismatched chunk and trusts only the intact prefix; the send loop then detects any surviving delta frame whose start id exceeds that prefix and fails cleanly with a "resend required" error instead of transmitting a gapped frame that would corrupt the table.

Tradeoffs

• Each reconnect/failover now replays the full dictionary as a catch-up frame, so a reconnect on a very high-cardinality connection ships the whole dictionary once (previously every message did).

• File mode writes a per-slot dictionary side-file (extra disk I/O and one small file per slot).

• Without fsync, a host/power crash can still lose recently persisted symbols, and the affected data must be re-sent. Every detectable tear now fails clean rather than corrupting: the per-chunk CRC-32C catches an interior page lost out of order (or a stale chunk left by a failed best-effort truncate) that would otherwise shift the dense id->symbol mapping, so recovery trusts only the intact prefix and the send loop forces a "resend required" for the rest. A tail truncate that itself fails makes the file untrusted; recovery leaves it intact and falls back to full-dictionary frames rather than exposing stale bytes. The one residual is a tear that happens to leave a CRC-matching byte run -- a 1-in-2^32-per-chunk collision, no weaker than the SF frames' own checksum.

• On failover to a node advertising a smaller batch cap, a symbol accepted under a larger or absent cap can exceed the new cap during the catch-up. A foreground sender retries that indefinitely and recovers on its own once a larger-cap node returns, so store-and-forward contains the window instead of surfacing it to the producer. Only an orphan drainer gives up, and only after both 16 consecutive cap gaps and a minimum wall-clock dwell (catch_up_cap_gap_min_escalation_window_millis, 5 minutes by default); it then sets its slot aside for an operator and that slot's data must be re-sent. This cannot happen on a homogeneous cluster -- a symbol that fit inside a data frame under a given cap always fits the smaller catch-up frame under the same cap -- so it only affects heterogeneous/rolling-cap clusters or an operator lowering the cap below existing data.

• The server-side gap rejection turns a previously silent (and silently wrong) frame into a NACK. The rejection is retriable by design -- a gap is a statement about per-connection server state, and re-registering from a held id resolves it -- but a sender that persistently re-sends the same gapped frame escalates through the poison-frame detector to a terminal error rather than looping forever.

• Quarantine trades availability of one slot's data for the rest of the pipeline: a slot set aside must be re-sent (or inspected and restored by an operator), and the sender continues on a fresh slot instead of blocking.

• In full-dictionary mode a batch whose dictionary exceeds the cap now ships that dictionary as several extra frames per batch rather than failing. The bytes are what full-dictionary mode already paid -- the dictionary was always in every frame -- but they are spread over more frames, each carrying its own header and two varints. If a table body is still oversized after chunking, the split pre-flight throws with the dictionary chunks already published as deferred, row-less frames. They are harmless (a later commit over them is a no-op, and the next flush re-publishes) but they are a departure from the strict all-or-nothing the split otherwise gives.

• A single symbol value larger than the cap cannot be split across frames. It is now refused before any chunk is published, with a dedicated error naming the symbol id, rather than surfacing as an unexplained oversized batch.

Follow-ups (known, deliberately not in this PR)

• Sender.build()'s rollback closes the cursor engine without the failed-stop check the close-delegation protocol requires, and PersistedSymbolDict makes the send loop's mirror a borrower of the engine's native memory — so a throw landing in the narrow window after the I/O thread starts, combined with a thread that outlives the 30 s stop (in practice an OOM), could free memory a live I/O thread still reads. The reachable window is effectively theoretical, and the fix (move the rollback close into QwpWebSocketSender.connect's catch, which owns the engine and honours the protocol) touches teardown paths not worth destabilizing here. It must land together with narrowing ensureConnected's blanket exception wrap, which currently masks the worse variant of the same defect: fixing either alone makes the other worse.
• In full-dictionary fallback mode accumulateSentDict still runs per frame: it re-walks the already-held dictionary prefix varint-by-varint on the I/O thread and, when the loop was constructed with the delta dict already disabled, accumulates a native mirror nothing ever reads. Both are constant-factor costs on a mode that already re-sends the whole dictionary per frame, so the fix (carrying the encoder's entries-length as sideband on ring entries, plus offset arithmetic for the identical prefix) waits for profiling evidence rather than adding plumbing here.
• The Rust client (c-questdb-client) already enforces a producer-side dictionary cap (SymbolGlobalDict::intern errors at the cap), but its constant MAX_CONN_SYMBOL_DICT_SIZE = 8_388_608 was taken from the egress/result-batch direction, not the ingress server's 1,000,000 — so its guard cannot fire before the server rejection. One-line constant fix (plus comment correction) needed in that repo.
• Known perf debt, pre-existing and unchanged here: each flushed message is copied one extra full time on the producer thread (encoder buffer -> microbatch -> segment mapping; two copies would suffice on the non-split path), and both CRC-32C paths (frame append and recovery scan) run software slice-by-8 -- hardware CRC32 instructions behind runtime dispatch are a native-build change. Neither gets worse with this PR; both dominate their respective paths and are worth a dedicated pass.
• The oversized-single-entry residual of the catch-up cap fix: a single symbol whose solo frame exceeds the server's actual receive buffer still reconnect-loops when the server advertises no cap. Reachable only with a symbol value comparable to the receive buffer (default 128 KiB) on a no-cap server; the halve-and-retry probe is the planned fix. Until then the failure mode is a visible reconnect loop, not data corruption.
• The batch-too-large rejection names reset() as the non-destructive recovery, and (since the id reclaim above) that now holds for a delta-mode sender whatever the next batch references. The pre-flight is still whole-flush, though: one unsplittable table's batch blocks other tables' healthy batches behind the same exception until reset()/close(), and sendRow's per-row guard checks raw column bytes without the frame overhead (header, delta section, table name), so a batch can pass the row guard and still exceed the cap. Per-table pre-flight and an overhead-aware row guard are the follow-up.
• P-C8 (second half, deferred): size the dictionary append window from
  segmentSizeBytes instead of the fixed 4 MiB APPEND_MAP_CAPACITY.
  Until then ensureAppendMap preallocates in 4 MiB steps, so a crash
  can leave up to a 4 MiB allocated-but-unaccounted tail per slot; a
  clean close() truncates it back. Trigger for doing it: tightening
  small-cap configurations (cap comparable to a few segments), where a
  4 MiB tail is a material fraction of the budget.
• P-C8 liveness note (resolved, kept for the record): counting side-file
  bytes initially meant a configuration where sideFileBytes + 2 * segmentSize > sf_max_total_bytes could never provision a hot spare
  again, deadlocking ingestion across restarts. The cap check now
  guarantees each ring its minimum working set, so that configuration
  backpressures and recovers instead of stalling; see the P-C8 section.
  What remains deliberately unimplemented is a cap-vs-dictionary
  validation at construction -- the floor makes the condition survivable
  and diagnosable, not impossible -- and an operator whose dictionary
  outgrows the cap will see the throttled "provisioning past
  sf_max_total_bytes" warning rather than a rejected config.
• P-C8 residue note: a session that degrades to full-dict mode closes and
  discards its recovered .symbol-dict but leaves the file on disk with a
  null gauge, so its bytes sit outside the cap for that session. The
  residue is static (nothing appends to it) and is cleared by a fully
  drained close or the next fresh session's truncate; unlinking at
  discard time needs its own analysis before we do it.

Test plan

• DeltaDictCatchUpTest -- reconnect catch-up rebuilds the dictionary (memory mode); a large dictionary splits across multiple catch-up frames under a small advertised batch cap and reassembles gap-free.

• DeltaDictRecoveryTest -- a recovered file-mode slot replays its delta frames against a fresh server; a torn (host-crash) dictionary is caught by the per-chunk CRC and only the intact prefix is trusted.

• PersistedSymbolDictTest -- side-file append/read/orphan-removal round trips, and a multi-byte UTF-8 round trip across reopen (every other symbol in these suites is ASCII, where a symbol's UTF-8 byte length and its char count agree, so a confusion between the two would otherwise go unnoticed).

• GlobalSymbolDictionaryTest, DeltaDictCeilingTest -- the 1,000,000-entry protocol cap: the boundary entry is accepted, the next is refused without mutation, cancelRow() recovers the row, the sender keeps working with registered values, and the refused symbol never reaches the wire.

• CursorWebSocketSendLoopCatchUpAlignmentTest -- the split catch-up's chunks must tile [0, n) exactly: the captured frames are reassembled through the same decoder the end-to-end tests use and compared per id, so an overlap, a gap or a shift all fail. Also covers a reconnect with an empty dictionary (no catch-up frame at all) and a split over entries of differing widths.

• SelfSufficientFramesTest, ReconnectTest -- full-dict fallback and reconnect replay still hold.

• MmapFaultDegradesTest -- a recognized mmap access fault on the dictionary persist path degrades the sender to full-dict frames; an unrecognized InternalError still propagates.

• MmapSegmentRecoveryFaultTest -- single-segment recovery fault shapes: read errors, short reads, size changes and unbacked pages fail closed before mapping or skip the unbacked tail.

• SegmentSkipQuarantineTest, SegmentRecoveryIntegrityTest, BackgroundDrainerUnreplayableSlotQuarantineTest -- deterministic recovery failures quarantine the whole slot and the replacement starts empty; a drained-slot leftover whose unlink fails aborts and retries instead of quarantining.

• CursorWebSocketSendLoopForegroundReconnectPolicyTest -- post-connect endpoint rejections retry on a foreground sender; initialization-time failures stay terminal; a first connect that fails inside the catch-up does not latch hasEverConnected.

• SlotLockTest -- lock lifecycle, including the pid-sidecar-before-lock unlink order on retirement.

• OSS QwpSymbolDecoderTest -- a gapped delta is rejected with DELTA_DICT_GAP (routed to the DICTIONARY_GAP status), the deltaStartId == size() boundary is still accepted, a rejected delta restores every overwritten entry and leaves no nulls -- including when the rejected frame was the connection's first.

• The enterprise SqlFailoverQwpClientLosslessTest (file-mode failover) passes end-to-end against a real server, asserting per row that every surviving SYMBOL is the value its id implies.

• PersistedSymbolDictTest pins every disposition: each transient (stat, open, mmap, short read, truncate) throws SfOperationalException with the file byte-identical and a subsequent open recovering in full; absent/stub/bad-magic report null with nothing created or destroyed

• DeltaDictRecoveryTest#testTransientDictFaultOnRecoveredSlotFailsLoudAndRetryRecoversInFull drives the three-session misattribution chain: session B fails loudly, the slot stays intact and unquarantined, and the retry replays the backlog with every wire-reconstructed id resolving to the original string

• Both directions of the torn-dictionary defense are re-enabled: the fixtures now trim through the live SegmentManager (prefix-ACK, manifest-correct head trim), so CursorWebSocketSendLoopTornDictGuardTest proves the pre-send guard refuses a gapped frame and ships nothing, and DeltaDictRecoveryTest#testFullyAckedTornSlotResumesInPlaceWithoutQuarantine lands exactly on the ackedFsn == recoveredCommitBoundaryFsn boundary (a >= -> > mutation reddens it) and resumes in place without quarantine.

• The fixture-driven quarantine tests pin the dictionary-gap verdict via the .failed sentinel content, and a deliberate chain-boundary test keeps the missing-head-segment fail-closed path covered on purpose instead of by accident.

• DictionaryGapNackTest -- first end-to-end 0x0D: a real DICTIONARY_GAP NACK recycles the wire, replays from the ack watermark, materialises the server-side dictionary gap-free, and neither latches a terminal nor poison-escalates on a single gap.

• CloseDrainTest covers both branches of close()'s drain-timeout outage naming: a 401-after-upgrade produces "the wire is not draining: WebSocket upgrade rejected with HTTP 401", and the never-dropped wire keeps the generic guidance tail. The test-only writeAckWatermark helper now writes the real AckWatermark format (its legacy 16-byte stamps were silently reset on open, i.e. no-ops).

• SenderError gains a first-class classification for permanent data loss: Category.DATA_LOSS + Policy.ABANDONED, constructible only through the dataLoss() factory, with getQuarantinedPath() naming where the abandoned bytes remain. The previous PROTOCOL_VIOLATION/TERMINAL classification promised a throw that never comes after quarantine-and-continue; handlers can now discriminate data loss by category instead of message text (mirrors the Rust client's StoreResendRequired).

• SenderPool recovery builds now deliver quarantine notifications to the user's errorHandler: a provenance filter (DATA_LOSS, or a real server status byte) routed through a pool-owned SenderErrorDispatcher, so an unreplayable slot found during pool recovery is no longer announced to nobody. Pinned by SenderPoolDataLossNotificationTest -- delivery, suppression of environmental noise (mutation-verified), NACK passthrough, and a blocking-handler close() bound.

• BackgroundDrainer gains an error sink (pool-default, drainer-override) and all five .failed-sentinel abandonment sites dispatch SenderError.dataLoss, closing the paths where buffered data was abandoned with only an unbound slf4j logger as witness.

• The OK-ack path's wire sequence gains the lower clamp its NACK sibling already had, closing an overflow-wrap path (negative dict-catch-up baseline + corrupt/hostile negative sequence -> ack-and-trim of unsent frames); both paths now warn on any out-of-range sequence.

• Perf (review C5): sendRow() drops from two O(columns) walks per row to one — the batch-cap guard folds into QwpTableBuffer.nextRow(snapshotBytes, maxRowBytes)'s existing padding walk and throws before the commit motion, so rollback semantics are unchanged. Behavioural note: the guard now measures the row including padding-null bytes (they go into the wire frame), so a row whose values fit the cap but whose padding pushes it over is now rejected up front instead of producing an oversize frame the server closes with 1009.

• Perf (review C4, server side, in the OSS PR): QwpMessageCursor releases the delta-dict rollback scratch by prefix instead of ObjList.clear()'s whole-backing-array fill, removing full-capacity fills on catch-up/replay/full-dict frames.

• SelfSufficientFramesTest#testDictionaryLargerThanTheCapShipsAsChunkedDictionaryFrames -- 40 symbols against a 512-byte cap: the flush succeeds, every frame respects the cap, and the chunks reassemble through the same decoder the delta suites use, gap-free and in id order. #testSingleSymbolLargerThanTheCapThrowsWithNothingPublished -- an unshippable symbol is refused with nothing on the ring. #testSectionOverCapWithAnOversizedBodyPublishesNothingOnEveryRetry -- an over-cap section beside an over-cap body publishes nothing, on the first flush and on every retry (this replaces #testCloseCommitsDictionaryChunksStrandedByAnOversizedBody, which asserted the stranding the change removes). #testFullDictCommitFrameCarriesNoDictionaryAfterACancelledRow -- the commit frame registers no symbols after a cancelled row leaked an id.

• SelfSufficientFramesTest#testCloseStillDrainsWhenTheRetainedBatchIsOverCap -- close() discards an over-cap retained batch and still runs its commit, seal and drain steps, so rows an earlier successful flush published are not abandoned. Asserted through drainOnClose, because every close() site catches the parent LineSenderException and cannot otherwise distinguish caught-inside from escaping.

• CursorWebSocketSendLoopCatchUpAlignmentTest#testHostileNegativeAckSequenceCannotTrimUnsentFrames -- the OK-path ACK lower clamp: against the negative fsnAtZero a multi-frame catch-up produces, a corrupt or hostile negative wire sequence would wrap the sum positive and ack published-but-unsent frames.

• CursorWebSocketSendLoopCatchUpAlignmentTest#testConnectLoopEntryKeepsTheCapGapEpisodeForACapGapCause and #testConnectLoopEntryRestartsTheCapGapEpisodeForAnUnrelatedCause -- the reconnect loop's entry guard, both directions. Accrual inside a single connectLoop invocation is guarded separately, so neither direction was covered before.

Each of the four is verified by reverting the production line it guards: the tests fail without it and pass with it.

• reset() returns never-shipped symbol ids. Delta mode anchors every section at sentMaxSymbolId+1, and that watermark advances only on a publish, so symbols an abandoned batch registered sat permanently between the watermark and the dictionary tip. Whether reset() -- the recovery the over-cap rejection documents -- actually worked therefore depended on what the caller's next row happened to reference: reusing an already-registered symbol gave a tiny section, while any NEW symbol landed above the abandoned range and dragged all of it back in, throwing identically on every later batch. reset() now reclaims those ids through GlobalSymbolDictionary.truncateTo, stopping at the higher of sentMaxSymbolId+1 (in a frame on the ring and in the send loop's mirror) and the persisted dictionary's size (the write-ahead persist runs before the publish, so a persist that succeeded under a publish that failed leaves durable ids above the watermark) -- handing either to a different string is the silent misattribution the dense id space exists to prevent. Full-dict mode is excluded: its sections start at id 0, so there is nothing to shrink, and reclaiming would let a later frame redefine an id a frame on the ring already defines. Mutation-verified in both halves.
• Full-dict over-cap chunking: a full-dictionary frame carries the whole dictionary from id 0, so its overhead grows with lifetime symbol cardinality until the section -- alone, or beside a table body -- pushes every split frame over the cap, and reset() cannot shrink a dictionary. flushPendingRows encodes at the current baseline, and when the split would reject a batch whose bodies DO fit an empty delta, it publishes the section as deferred, table-less chunk frames through publishDictionaryChunks and re-encodes against that empty delta (the re-encode is forced: beginMessage resets the buffer the split's staged body slices live in). Publishing happens only behind that bodies-fit proof, so nothing reaches the ring for a batch that then throws -- on the first attempt or any retry. An earlier revision pre-registered the dictionary before sizing the bodies; because the split's throw retains the batch by design and its message invites a retry, every retry appended another whole dictionary to a ring whose ack watermark was frozen (the server withholds the ack for a deferred frame until its group commits), until sf_max_total_bytes filled. Pinned by wire-shape tests with deterministic 504-vs-512 sizing, a retry test asserting publishedFsn never advances for an unshippable batch, and a delta-mode test proving the fallback stays off where the write-ahead persist ordering forbids it (mutation-verified in both directions).
• Commit frames carry an empty symbol delta in both modes. sendCommitMessage bounded its delta with currentBatchMaxSymbolId in full-dict mode, on the premise -- stated in its own comment -- that the prior flush had reset it to -1. flushPendingRows returns early WITHOUT resetting it when pendingRowCount is 0 or every table is empty, and cancelRow leaves behind the id of a symbol it registered, so a commit reached through that window re-shipped the entire dictionary from id 0 -- in the one frame no cap check and no chunker covers, which the chunking above otherwise exists to prevent. Passing the baseline as both bounds makes the delta empty by construction, which is the only shape a row-less commit needs. Mutation-verified against the cancelled-row path.
• Known follow-up (introduced by this PR's chunker, not addressed here): a chunked full-dict group leans on the server to keep it atomic, and the client has no backstop of its own. hasReplayDictionaryDependency is false for a live full-dict slot, so no reconnect catch-up is sent, while the group's data frames now carry an empty delta that depends on separately-published chunk frames. Reading the tandem server, that group IS atomic with respect to trim -- markUncommittedDeferredRows fires for every deferred frame including a row-less one, and no ack is emitted for a deferred frame at all -- so the disconnect window this bullet previously described ("between a dict-chunk frame's ack and its data frames' acks") does not actually exist against it; the earlier wording was wrong. Process-crash recovery is likewise covered, because the chunks' non-zero delta start turns the catch-up back on. What remains is that a client invariant now rests entirely on a server behaviour: against any deferred-ack hole (an older server mid-rolling-upgrade, an intermediary) the data frame meets an empty dictionary and the outcome is a loud STATUS_DICTIONARY_GAP -> poison terminal and batch loss, never silent corruption. The cheap fix is to stop deriving the dependency from the engine mode: trySendOne already decodes each frame's delta start, so latching sawNonZeroDeltaStart there and folding it into the catch-up gate makes the chunked case self-healing on every reconnect. Untested either way today.

🤖 Generated with Claude Code

[glasstiger]

Tandem OSS PR (submodule bump): questdb/questdb#7374 — merge together.

[bluestreak01]

@glasstiger

Review of PR #66 — feat(qwp): stop resending the full symbol dictionary on every message

Reviewing at level 3 (full mission-critical pass: all steps, all reviewer dimensions, per-finding source verification). Note: the subagent tool is unavailable in this environment, so the parallel-reviewer passes and per-finding verification were run inline by the parent session using read/bash against the source and a local build+test run — not delegated. Every finding below was verified against the cited source lines; false positives are listed in Downgraded.

Build/test evidence: mvn -pl core compile clean on JDK 25; DeltaDictCatchUpTest, DeltaDictRecoveryTest, PersistedSymbolDictTest, SelfSufficientFramesTest, ReconnectTest → 15 tests, 0 failures.

Committed-binary gate: PASS — git diff --numstat shows no binary files; all 10 changed files are .java with numeric line counts.

---

Critical

C1 — Persisted .symbol-dict accumulates duplicate entries when appendBlocking fails and a later flush succeeds → silent symbol corruption on recovery (file mode, delta enabled). [in-diff]

File: core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java:3660-3676 (persistNewSymbolsBeforePublish), triggered via flushPendingRows (3491/3498) and flushPendingRowsSplit (3574/3582).

Code-path trace (verified):

flushPendingRows runs, in order:

persistNewSymbolsBeforePublish();   // 3491 — appends [sentMaxSymbolId+1 .. currentBatchMaxSymbolId] to .symbol-dict (Files.write, no fsync)
activeBuffer.write(...);            // 3494
sealAndSwapBuffer();               // 3495 — calls cursorEngine.appendBlocking(); CAN THROW
advanceSentMaxSymbolId();          // 3498 — SKIPPED on throw
...
resetTableBuffersAfterFlush(keys); // SKIPPED on throw → rows + currentBatchMaxSymbolId preserved

sealAndSwapBuffer → appendBlocking throws LineSenderException("cursor SF append failed", …) on the two documented conditions (QwpWebSocketSender.java:3768,3783-3785): backpressure deadline (the SF ring hit sf_max_total_bytes and did not drain — i.e. exactly the store-and-forward stress scenario, server slow/down) and PAYLOAD_TOO_LARGE. The I/O loop is not failed, so cursorSendLoop.checkError() passes and the sender stays open and usable.

On the throw: the frame's new symbols are already durably on disk (persist ran before sealAndSwapBuffer), but sentMaxSymbolId was not advanced (advanceSentMaxSymbolId at 3498 skipped) and the table buffers/currentBatchMaxSymbolId are not reset (resetTableBuffersAfterFlush skipped — verified: currentBatchMaxSymbolId is reset only at 3607, 3686, and inside resetTableBuffersAfterFlush, none of which run on this path).

The next successful flush() (a transient backpressure clears the moment the server catches up) re-enters persistNewSymbolsBeforePublish with the same from = sentMaxSymbolId + 1 (3668) and to = currentBatchMaxSymbolId (3669) — because pd.appendSymbol has no dedup (PersistedSymbolDict.java:appendSymbol) and nothing rolled back the earlier append, the failed frame's symbols are written to the file a second time. The file's positional invariant ("symbol id i is the i-th entry", PersistedSymbolDict.java class doc) is now broken.

Impact on recovery/orphan-drain (a fresh process reads the file):

• seedGlobalDictionaryFromPersisted (2243/3695) calls getOrAddSymbol, which de-dupes → producer globalSymbolDictionary.size() and sentMaxSymbolId are below the file's entry count.
• The send loop's constructor seeds the mirror directly from the raw file bytes with sentDictCount = pd.size() (CursorWebSocketSendLoop.java:515-522), i.e. including the duplicate.
• sendDictCatchUp re-registers the duplicated mirror on the fresh server, so every global id above the duplicate is shifted by +1.
• Symbol column cells are encoded as absolute global ids (QwpColumnWriter.writeSymbolColumnWithGlobalIds, line 277 buffer.putVarint(globalId)). The replayed frames carry the original ids, which now resolve against the shifted server dictionary → rows get the wrong symbol values, silently. The torn-dictionary guard does not catch this (deltaStart never exceeds the now-larger sentDictCount, so trySendOne at 2223-2238 passes).

This is a store-and-forward data-integrity violation triggered by an ordinary transient outage — the exact failure class SF exists to survive.

Suggested fix: base the append range on the true persist watermark, not the wire baseline. pd.size() already tracks how many symbols are durably persisted at contiguous ids 0..size-1:

int from = pd.size();          // instead of sentMaxSymbolId + 1
int to = currentBatchMaxSymbolId;
if (to < from) return;
for (int id = from; id <= to; id++) pd.appendSymbol(globalSymbolDictionary.getSymbol(id));

In the happy path pd.size() == sentMaxSymbolId + 1, so behavior is identical; after a failed append it skips the already-persisted ids, making the operation idempotent across retries. Add a regression test: file mode + delta, force an appendBlocking failure (small sf_max_bytes + silent server), then a successful flush, then assert .symbol-dict has no duplicate and a fresh-process recovery reconstructs the dictionary gap-free.

---

C2 — Required Enterprise failover tandem is missing/unlinked; the HA path this feature targets is UNTESTED in CI (Step 2.7 gate). [tandem]

Verification (commands recorded):

• OSS tandem: gh pr list --repo questdb/questdb --head qwp-delta-symbol-dict → #7374 present, matching branch, bidirectionally linked (body: "Tandem OSS half of #66"; a PR comment links back). It is a submodule bump only — "The OSS server already parses delta symbol-dictionary frames, so no server change is required." Its CI covers single-node QWP e2e.
• Enterprise tandem: gh pr list --repo questdb/questdb-enterprise --head qwp-delta-symbol-dict → empty. gh can reach the private enterprise repo (confirmed), and a scan of the 60 most-recent enterprise PRs shows no client-bump/qwp-symbol-dict PR. SqlFailoverQwpClientLosslessTest exists in enterprise (questdb-ent/src/test/java/com/questdb/lifecycle/), and the PR body claims it "passes end-to-end against a real server" — but with no enterprise PR bumping the client submodule, that test runs against the old client in enterprise CI, not this change.

Why this trips the gate: the change is squarely HA-facing — it rewrites the SF drainer's on-the-wire framing, adds reconnect/failover dictionary catch-up (swapClient → setWireBaselineWithCatchUp → sendDictCatchUp), and adds recovery/orphan-drain dictionary rebuild. The headline benefit (dictionary survives a reconnect/failover) is only proven end-to-end by the enterprise failover suite the PR itself names. Per Step 2.7, a required-but-missing tandem is Critical and every behavior it would cover is treated as UNTESTED. The client-local loopback tests (C-tier coverage below) are strong, but they cannot prove (a) a real server accepts and correctly registers a 0-table catch-up frame mid-stream, or (b) primary→replica failover preserves the dictionary.

Required action: open (or link) the enterprise tandem that bumps the client submodule to this SHA and runs SqlFailoverQwpClientLosslessTest (and, ideally, a kill-9 recovery variant in the enterprise e2e-python suite for the file-mode host-crash/torn-dict path, which the unit test only simulates by truncating the file). Also confirm OSS #7374's e2e actually drives a reconnect (so the catch-up frame is exercised against a real server), not just a single connected ingest.

---

Moderate

M1 — One Files.write syscall per new symbol on the producer thread. [in-diff]

persistNewSymbolsBeforePublish (3660-3676) loops pd.appendSymbol(...), and each appendSymbol (PersistedSymbolDict.java) issues its own Files.write(fd, …) (one pwrite). A frame that introduces K new symbols does K syscalls on the user/producer thread. This is per-new-symbol (not per-row), so it's bounded by dictionary growth, but a high-cardinality first batch will burst syscalls synchronously in the flush path. Batch the frame's whole new-symbol range into a single scratch buffer and one Files.write. Not zero-GC-blocking (no allocation), but avoidable syscall amplification on the ingestion path.

M2 — accumulateSentDict silently drops symbols on a partial-overlap delta. [in-diff]

CursorWebSocketSendLoop.java:1946-1960: the guard is if (deltaCount <= 0 || deltaStart != sentDictCount) return;. A delta with deltaStart < sentDictCount and deltaStart + deltaCount > sentDictCount (overlaps the tip and extends past it) is dropped entirely — the new tail symbols never enter the mirror, so a later catch-up would be incomplete (→ the same shifted-id corruption as C1). I verified this is currently unreachable: the producer emits strictly contiguous, non-overlapping deltas (beginMessage computes deltaStart = confirmedMaxId+1; advanceSentMaxSymbolId moves the baseline to exactly currentBatchMaxSymbolId), and recovery seeds sentDictCount from a superset, so deltaStart < sentDictCount ⇒ deltaStart+deltaCount ≤ sentDictCount. But it is load-bearing correctness resting on an invariant enforced elsewhere. Harden it: handle the partial overlap (accumulate only the [sentDictCount .. deltaStart+deltaCount) tail) or assert deltaStart + deltaCount <= sentDictCount so a future producer change fails loudly instead of silently corrupting the mirror.

---

Minor

m1 — Stale "self-sufficient / delta from id 0" comments now contradict delta mode.

QwpWebSocketSender.java:3392, 3398-3399, and 3777 still say cursor frames are "self-sufficient (every frame carries … a symbol-dict delta from id 0)". In delta mode frames are explicitly not self-sufficient (the whole point of the PR), and the 3777 comment ("next batch re-emits … symbol-dict delta from id 0") describes behavior that no longer happens. Update to match the new baseline semantics to avoid misleading a future reader on the recovery/retry path (which is exactly where C1 lives).

m2 — Memory-mode mirror double-stores the dictionary.

The I/O-thread mirror (sentDictBytes*) holds every symbol's UTF-8 bytes while globalSymbolDictionary already holds them as Java Strings. Bounded by distinct-symbol count (not per-row), so acceptable, but worth a comment that memory-mode steady-state native footprint is ~2× the dictionary size for the reconnect-catch-up capability.

---

Downgraded (false positives — verified against source)

• Negative fsnAtZero on fresh recovery (replayStart=0 ⇒ fsnAtZero = -catchUpFrames) corrupts ack accounting — dismissed. SegmentRing.acknowledge clamps to publishedFsn and no-ops when seq ≤ ackedFsn (339-349); the catch-up frame maps to an already-acked/nonexistent low FSN and its ack is a harmless no-op. DeltaDictRecoveryTest exercises exactly this (silent server, nothing acked) and passes.
• pd.size() read race in the send-loop constructor vs producer appendSymbol — dismissed. The loop is constructed during sender build/startCursorSendLoop (or on the drainer thread with no producer at all), which happens-before the first user send; no concurrent append occurs, so sentDictCount == loadedEntries count.
• Catch-up frame double-advances the durable-ack watermark — dismissed. The catch-up frame's OK enqueues a tableCount=0 (trivially durable) pending entry mapping to an ≤ackedFsn FSN; drainPendingDurable acks a no-op. Cumulative ack semantics make a missing catch-up OK harmless too.
• Catch-up (non-DEFER_COMMIT) frame prematurely commits deferred WAL on reconnect — dismissed. It is the first frame on a fresh server connection, which holds no pending WAL state; committing nothing is a no-op before the deferred replay frames arrive.
• positionCursorForStart re-sends a catch-up when retiring an orphan tail — dismissed. That branch is guarded by nextWireSeq == 0 (trySendOne 2166-2175), which cannot hold after sendDictCatchUp incremented nextWireSeq; when sentDictCount==0 there is nothing to re-send.
• A symbol larger than the batch cap breaks catch-up — dismissed. The original data frame carrying that symbol (plus row data) would already exceed the cap and fail; the catch-up (symbol only, less overhead) is strictly smaller, so sendDictCatchUp's entryBytes > budget terminal is consistent, not a new failure.
• Java 8 floor violations in new code — dismissed. No var, text blocks, instanceof patterns, List.of, etc. in the changed main files; the one -> is a pre-existing lambda. Compiles clean on JDK 25.
• PersistedSymbolDict uses slf4j instead of QuestDB Log — dismissed. Its sibling SF-cursor classes (AckWatermark, SegmentRing, CursorSendEngine, the send loop) all use slf4j; this is consistent.

---

Coverage map

#	Behavioral change	Test (local unless noted)	Failure link	Dimensions	Verdict
1	Memory-mode monotonic delta (symbolDeltaBaseline in beginMessage)	SelfSufficientFramesTest.testMemoryModeShipsMonotonicDelta	asserts batch-2 deltaStart=1,deltaCount=1 — fails if baseline reverts to -1	happy ✓; NULL N-A; boundary (2 symbols) ✓; concurrency N-A	TESTED
2	File-mode delta + write-ahead persist	SelfSufficientFramesTest.testFileModeShipsMonotonicDeltaAndPersistsDict	asserts monotonic delta + .symbol-dict retains both symbols	happy ✓; resource (dict file) ✓	TESTED
3	Reconnect catch-up (memory)	DeltaDictCatchUpTest.testReconnectCatchUpRebuildsDictionary	reconstructs conn-2 dict from wire; fails on null gap	happy ✓; reconnect ✓ (loopback)	TESTED
4	Split catch-up under batch cap	DeltaDictCatchUpTest.testReconnectCatchUpSplitsLargeDictionaryAcrossFrames	asserts ≥2 zero-table frames + gap-free reassembly	boundary (cap) ✓	TESTED
5	File-mode recovery replay to fresh server	DeltaDictRecoveryTest.testRecoveredSlotReplaysDeltaFramesAgainstFreshServer	asserts catch-up frame seen + gap-free dict	recovery ✓ (loopback); memory-leak N-A	TESTED
6	Torn-dictionary guard (simulated host crash)	DeltaDictRecoveryTest.testTornDictionaryFailsCleanlyInsteadOfCorrupting	asserts 0 frames replayed + terminal "incomplete" error	error path ✓	TESTED
7	PersistedSymbolDict open/append/reopen/torn-tail/bad-magic/removeOrphan	PersistedSymbolDictTest (5 tests, assertMemoryLeak)	round-trip + self-heal asserts	happy/boundary/empty-symbol/resource ✓	TESTED
8	appendBlocking failure → persist-then-retry dict duplication (file mode)	none (recorded search: no test references appendBlocking/backpressure/dup + persisted dict)	—	error+retry ✗; recovery-after-retry ✗	UNTESTED → Critical (C1)
9	Real-server 0-table catch-up acceptance + primary→replica failover	OSS tandem #7374 (single-node only); Enterprise tandem missing	—	real-server/failover ✗	UNTESTED → Critical (C2)
10	seedGlobalDictionaryFromPersisted id/baseline resume on recovery	indirect via DeltaDictRecoveryTest #5	dict reconstructed gap-free implies correct seed	happy ✓; retry-dup interaction ✗ (see C1)	TESTED (partial)

---

Summary

Verdict: REQUEST CHANGES.

The design is careful and the write-ahead/torn-dictionary reasoning is largely sound, but two blocking issues stand:

• C1 (data integrity): a transient appendBlocking backpressure failure followed by any successful flush duplicates the failed frame's symbols in the persisted .symbol-dict; a later recovery/orphan-drain then silently misattributes symbol values via shifted global ids. This is a store-and-forward correctness violation on the very outage class SF exists to survive, it has no regression test, and the fix is small (base the persist range on pd.size()).
• C2 (test gate): the HA failover behavior the feature targets has no linked, CI-running enterprise tandem; the OSS tandem #7374 covers single-node only.

Test & tandem gate: FAILS — one UNTESTED-Critical bug-fix-worthy path (C1, no regression test) and a required-but-missing Enterprise tandem (C2). Cannot approve.
Zero-GC gate: PASSES — no steady-state per-row/per-producer-call allocation on the ingestion path; producer-side additions (symbolDeltaBaseline, advanceSentMaxSymbolId, persistNewSymbolsBeforePublish) allocate nothing (M1 is syscall amplification, not GC). Catch-up/mirror allocations are I/O-thread, reconnect-only.
Coverage map: 10 behavioral-change groups — 8 tested locally (loopback), 2 UNTESTED (dict-dup-on-retry; HA-failover tandem).
Tandem status: OSS e2e tandem linked (#7374, single-node); Enterprise failover tandem required and missing; enterprise e2e-python kill-recovery coverage for the host-crash/torn-dict path recommended.
Findings: 6 verified (2 Critical, 2 Moderate, 2 Minor); 8 draft findings dropped as false positives after source verification.
In-diff vs out-of-diff: 4 in-diff (C1, M1, M2, m1), 1 tandem/process (C2), 1 cross-cutting (m2). The C1 mechanism spans the new persistNewSymbolsBeforePublish (in-diff) and the pre-existing sealAndSwapBuffer/appendBlocking failure path (out-of-diff) it now interacts with — the classic "diff quietly changed a contract at an unchanged callsite" case.

[glasstiger]

PR #66 review (round 2) — feat(qwp): stop resending the full symbol dictionary on every message

Reviewed at: 7238e193 · Base: main. The review body below was drafted against ddab88db; three commits have landed since (eb324888, d3219c53, 7238e193) and every finding they claim to close was re-verified at source before publishing. CI is green on 7238e193 (JDK 8 build+test, JDK 25 compile, GLIBC guards, gitleaks; coverage/javadoc legs still running).

The delta-dictionary core is genuinely well built. The id arithmetic was traced end-to-end — encode → write-ahead persist → recovery fold → mirror → catch-up — and could not be broken. The write-ahead resume from pd.size() (not sentMaxSymbolId+1) is correct and idempotent; RecoveredFrameAnalysis preserves the "unidentifiable frame is a retirement barrier" contract the removed findLastFsnWithoutPayloadFlag documented; Crc32c.updateUnsafe is bit-identical to the native twin; the deferred-group atomicity the chunker depends on is real (verified server-side). The defects are on the periphery.

---

Resolved since ddab88db — verified at source

C1 — .symbol-dict counted against sf_max_total_bytes created a permanent, restart-surviving stall. Fixed by eb324888. SegmentManager gains MIN_LIVE_SEGMENTS = 2 and a livenessFloorBytes; the provisioning gate (:969-1011) keeps the cap check but provisions anyway when the cap refuses and e.ring.totalSegmentBytes() < livenessFloorBytes, with its own throttled warning naming the operator's real remedy (raise the cap / reduce cardinality) instead of a trim. Traced: a ring at one segment is below the floor → provisions → two segments; at two it is not → refuses → the producer blocks until an ack trims the sealed one → back below the floor → provisions. That is ordinary ack-driven backpressure, not a deadlock, and it clears itself. The overflow clamp on segmentSizeBytes * MIN_LIVE_SEGMENTS is right — an unclamped product would wrap negative and make the floor test trivially false, silently restoring the deadlock.

C1b — the gauge under-counted real disk usage by up to 4 MiB per live slot. Fixed by eb324888. PersistedSymbolDict tracks reservedFileBytes, published in ensureAppendMap after ff.allocate succeeds and before the mmap that can still fail (correct: the blocks are committed either way), and exposes occupiedDiskBytes() = max(committed, reserved). close() lowers it only when the truncate actually succeeds. CursorSendEngine wires the manager gauge to occupiedDiskBytes instead of appendedBytes, and appendedBytes() keeps its logical meaning across a reopen. The wait-free two-volatile-read contract the manager needs is preserved.

M1 — a retried flush() after BatchTooLargeForCapException re-appended the whole dictionary to a ring with a frozen ack watermark. Fixed by d3219c53. The eager preRegisterDictionaryChunks is gone; flushPendingRows now encodes at the current baseline first and only chunks inside the over-cap fallback (:3942-3950), gated on !splitFramesFit(cap, deltaBaseline) && splitFramesFit(cap, currentBatchMaxSymbolId) — i.e. behind a proof that chunking makes the batch shippable. Nothing reaches the ring for a batch that then throws, on the first attempt or any retry. Both shapes the eager pass covered (section alone over cap; section over cap only beside a body) still reach the chunker. publishDictionaryChunks absorbed the per-entry validation pass.

M2 — reset() un-wedged a delta-mode sender only by luck. Fixed by 7238e193. reset() now calls reclaimUnsentSymbolIds(), which in delta mode only truncates globalSymbolDictionary down to max(sentMaxSymbolId + 1, pd.size()). The floor is the right one, and I checked the two ways it could have been wrong: sentMaxSymbolId is never reset (resetSymbolDictStateForNewConnection:4632-4641 deliberately leaves it alone, so a reconnect cannot drop the floor under ids the mirror holds), and on recovery seedGlobalDictionaryFromPersisted resumes the baseline at the tip seeded from both the persisted prefix and the surviving frames' own deltas — so the frame-recovered ids above pd.size() are covered by the sentMaxSymbolId half and truncateTo is a no-op there. GlobalSymbolDictionary has exactly two fields and truncateTo handles both; the ascending rebuild preserves addRecoveredSymbol's highest-id-wins rule.

M3 — full-dict sendCommitMessage could ship the whole dictionary unchunked and uncapped. Fixed by d3219c53. sendCommitMessage now passes symbolDeltaBaseline() as both bounds, making the delta empty by construction in either mode rather than relying on "the prior flush reset it to -1". The stale comment went with it.

M7 — preRegisterDictionaryChunks walked the whole dictionary on every full-dict flush. Fixed as a side effect of d3219c53. The per-entry dictionaryEntryWireBytes pass now lives in publishDictionaryChunks, reachable only once messageSize > cap, so the common path no longer pays a second full UTF-8 walk it discards.

Test coverage added for these looks proportionate and is claimed mutation-verified in both directions per commit body: SelfSufficientFramesTest#testSectionOverCapWithAnOversizedBodyPublishesNothingOnEveryRetry (replacing the test that asserted the stranding the fix removes), #testFullDictCommitFrameCarriesNoDictionaryAfterACancelledRow, SegmentManagerSideFileCapTest#testLivenessFloorProvisionsDespiteSideFileBytesOverTheCap, and the occupiedDiskBytes() > appendedBytes() assertion in CursorSendEngineTest.

Two items the fixes leave behind

R1 — three passages in the PR body now contradict the code (body lines 31, 35, 125). "The producer now registers the dictionary up front as deferred, table-less frames"; "The pre-registration is also a no-op unless the dictionary section leaves no room for a table body"; and, in Tradeoffs, "If a table body is still oversized after chunking, the split pre-flight throws with the dictionary chunks already published as deferred, row-less frames … they are a departure from the strict all-or-nothing the split otherwise gives." That last one advertises exactly the defect d3219c53 removed. Since the squash-merge makes this body the permanent commit message, it should describe the shipped behaviour.

R2 — sf_max_total_bytes is now a soft cap, and the constructor still does not say so. SegmentManager's validation is unchanged at maxTotalBytes >= segmentSizeBytes (:260-265), while the floor guarantees 2 * segmentSizeBytes plus the side file regardless of the configured value. A cap set to exactly one segment now yields two. That is the deliberate and correct trade — the alternative is the deadlock — but the guaranteed minimum footprint is a documented-config property now, not just a warning, and the body's P-C8 note only mentions the dictionary component. Minor; worth one sentence in the connect-string/config docs.

---

Still open

Moderate

M4 — NativeBufferWriter.writeVarint/putVarint: an assert guards a documented stream- and side-file-corrupting bug (in-diff, latent). Unchanged at :91 / :329. The javadoc at :85-88 states it plainly: a negative value emits a single truncated byte while varintSize returns 10. The guard is assert value >= 0, and this artifact ships to applications that run without -ea. Every caller today passes a non-negative id/length/count, so the trigger is unreachable — but writeVarint is now the shared writer for the durable .symbol-dict (14 sites) and the catch-up frame builder, and the fix is one token: while ((value & ~0x7FL) != 0). varintSize already returns 10 for a negative long, so the unsigned loop makes size and write agree for every input and retires the assert.

M5 — Dangling cachedTimestampColumn after rollbackRow() → NPE on the next row of the same table (out-of-diff, pre-existing, widened). rollbackRow() (:4775) calls rollbackUncommittedColumns(), which closes and removes columns created during the in-progress row (QwpTableBuffer.java:371-384) — including the designated-timestamp column atMicros cached at :2919. Neither rollbackRow() nor cancelRow() nulls the cache, and table()'s fast path returns at :2733, before the null-out at :2742. Next row on the same table: cachedTimestampColumn.addLong(...) → dataBuffer is null → NPE (clean failure, not a use-after-free — ColumnBuffer.close() nulls the buffers). Only reachable on a table's first row. This directly contradicts the contract the PR's own sendRow comment states ("the at()/atNow() error path can roll back… and prior committed rows stay intact"), and the padding-inclusive guard fires in strictly more cases. Fix: null both cached timestamp columns in rollbackRow() and cancelRow().

M6 — At the 1M cap, the error message's remedy is not performable through SenderPool (out-of-diff). getOrAddSymbol says "close this sender and build a new one", but PooledSender.close() (:153) gives the delegate back rather than closing it, and reset() does not clear globalSymbolDictionary. A delegate that hit the cap is handed to the next borrower still full, throwing on every new symbol value. Only incidental reapIdle recovers it.

M8 — Two decoders of the same wire format disagree on the max varint length (in-diff). CursorWebSocketSendLoop.readVarintAt:2828 bounds at bytes < 6; RecoveredFrameAnalysis.readVarint:382 at bytes < 5. They decode the identical deltaStart/deltaCount header and per-entry [len] prefix. A 6-byte encoding is accepted by the send loop and rejected as corrupt by recovery. readVarintAt's javadoc cites the other as "the shape it already uses" but copies the packing, not the bound.

M9 — SenderErrorHandler's threading contract changed (in-diff, public API). A build()-time DATA_LOSS is now dispatched synchronously on the caller's thread (Sender.java:3247). Existing implementors were told handlers always run on a dedicated daemon dispatcher. A throwing handler is caught, but a blocking one hangs build() indefinitely. Also: the dispatch is gated on errorHandler != null (default null), so handler-less users get only the LOG.error the surrounding comment says is insufficient for an embedder with no slf4j binding.

M10 — Drainer can report DATA_LOSS for data the server already has (in-diff). BackgroundDrainer.java:796-866 reads ackedFsn at :796, then loop.checkError() at :805; nothing re-reads the watermark in the catch. A final ack landing between them still writes .failed and fires SenderError.dataLoss(reason, slotPath) — then finally's engine.close() computes drained == true and unlinks everything the alarm named. seedGlobalDictionaryFromPersisted already applies exactly this guard for the build() path (QwpWebSocketSender.java:4732-4744); the drainer is the one site without it. One-line fix: re-check engine.ackedFsn() >= target at the top of the catch.

M11 — Drainer quarantine is sentinel-only, so the new DATA_LOSS notification can repeat forever (in-diff). OrphanScanner.markFailed returns silently when openRW fails (:317-324) — and a full or read-only disk is exactly the condition that produces the unreplayable verdict. isCandidateOrphan excludes a slot by the .failed sentinel or the .unreplayable- name, and only Sender.quarantineTornSlot produces the latter. So the next build() re-adopts the slot, re-runs recovery, re-hits the verdict, and re-fires DATA_LOSS — on every build. The sentinel-only quarantine is pre-existing; the notification storm is new.

M12 — DATA_LOSS from an abandoned drainer is silently dropped (in-diff). BackgroundDrainerPool.close() abandons drainers that miss the grace window (daemon threads, shutdownNow at :181); errorDispatcher.close() runs later in closeRemainingResources (:3488), and offer is a no-op once closed. Any quarantine performed afterwards leaves only the LOG.error — exactly the silence this sink was added to break.

M13 — quarantineTornSlot renames the slot while the torn engine may still own it (in-diff). Sender.java:3179 onward swallows torn.close(false) failures and renames unconditionally — but CursorSendEngine.close() can return normally without completing cleanup (:1003-1010, :1011-1027, :1048-1063), leaving the ring mapped, fds open and the flock held, with closeCompleted == false. quarantineTornSlot never consults isCloseCompleted() (the only two callers of it are in QwpWebSocketSender). On Windows the rename then fails and the method throws the "cannot start until … is moved by hand" error — a transient wedged worker reproducing the permanent brick this method exists to remove.

M14 — public static volatile boolean forceMirrorSeedFailureForTest (in-diff). CursorWebSocketSendLoop.java:247, read in the production constructor at :833, in a package exported from module-info.java. Any code on a user's classpath can make every recovered-slot sender in the JVM throw. Every other seam in this package is private static + a setter. Move the test into the package and drop the field to package-private.

M15 — Committed spec not updated. design/qwp-nack-policy-v2.md is the repo's authoritative NACK-policy record (with its own "Behavior changes (release notes)" section). This PR adds wire byte 0x0D, Category.DICTIONARY_GAP, Category.DATA_LOSS and Policy.ABANDONED and updates none of it — grep for all four still returns 0 hits. The doc's headline invariant, "No silent data loss. There is no drop policy", is materially amended by ABANDONED. The doc also states that 0x0C is deliberately reserved, not emitted, "until deployed client fleets classify it as retriable-with-rotation"; this PR emits 0x0D without that wait, and the body's justification ("QWP is experimental and unreleased") is contradicted by six released tags containing QWP, the most recent 1.3.6 on 2026-07-27.

M16 — Test-efficacy gaps. Verified individually; all still stand at 7238e193.

• Orphan-drainer DATA_LOSS wiring is untested end-to-end. Only two tests set the sink, both directly on a BackgroundDrainer (BackgroundDrainerDurableAckRetryTest:253, BackgroundDrainerUnreplayableSlotQuarantineTest:98). Deleting QwpWebSocketSender:2649-2654 and BackgroundDrainerPool:287-290 leaves the suite green. Three of the five dispatchDataLoss sites (:419, :720, :864) have no test at all.
• SegmentManagerSideFileCapTest:88-89 asserts a parallel path. getCapAccountedBytesForTesting() re-derives the sum independently of the gate, so deleting observedSideFileBytes from the gate leaves it green. The only real detector is the negative countSfaFiles == 2 assertion gated by a bare Thread.sleep(100) — while the sibling test at :117-124 already replaced that sleep with a bounded poll for exactly this reason. The new testLivenessFloorProvisionsDespiteSideFileBytesOverTheCap polls for the positive assertion but repeats the bare Thread.sleep(100) for its negative one.
• c5974217 survives its own mutation. The fix narrowed a throw from SfRecoveryException to MmapSegmentException — but SfRecoveryException extends MmapSegmentException, and SegmentRecoveryIntegrityTest:986 catches the parent. Reverting the type ships green, and the distinction it encodes (abort-and-retry vs. quarantine-the-slot) is asserted nowhere. This is the same class of mistake the PR correctly avoided for BatchTooLargeForCapException in close().
• testNextRowBudgetBoundaryIsExactIncludingPadding does not test padding. Its padded column is TYPE_VARCHAR with useNullBitmap = true; addNull() then only touches the null bitmap (QwpTableBuffer:1090-1093), which getBufferedBytes() does not count (:1342-1362). Padding contributes 0 bytes, so both arms evaluate identically with padding excluded. The behaviour the PR body advertises — "a row whose values fit the cap but whose padding pushes it over is now rejected" — holds only for non-nullable BOOLEAN/BYTE/SHORT/CHAR columns and is covered by no test.
• SenderPoolDataLossNotificationTest:154-158 asserts silence after Thread.sleep(2_000) with nothing proving a post-wall reconnect attempt ran; received starts empty, so zero attempts passes vacuously. Its sibling InitialConnectAsyncTest:438-450 added exactly the missing discriminator.
• TestWebSocketServer:512 swallows handler failures. handler.onBinaryMessage(...) runs in a read loop whose only catch is catch (IOException) (:723). Handlers in CloseDrainTest:778, CloseSafetyNetTest:174, SenderPoolDataLossNotificationTest:349,410 and DeltaDictCatchUpTest:463,547,646 throw AssertionError into a dead channel.
• Test resource leaks before the try in ~8 new tests (SegmentManagerSideFileCapTest:81/97, MmapSegmentTest:500, SegmentRecoveryIntegrityTest:898/923, EngineCloseSlotLockReleaseTest:231/252, SlotLockTest:142/148, PersistedSymbolDictTest:727/729, SenderPoolDataLossNotificationTest:200-215). Invisible to CI because TestUtils.assertMemoryLeak calls skipChecks() when the body throws (:136-139). The new testLivenessFloorProvisionsDespiteSideFileBytesOverTheCap has the same shape: SegmentRing.openExisting is assigned before the try, and ring.close() sits after it rather than in a finally.
• The "chunked full-dict groups are not reconnect-atomic" follow-up is still untested. The disclosure itself is now corrected in the body — it acknowledges the server emits no ack for a deferred frame at all, so the window it previously described does not exist — and correctly restates what remains: a client invariant resting entirely on a server behaviour, with sawNonZeroDeltaStart in trySendOne named as the cheap self-healing fix. Still no test in either direction.

M17 — PR metadata (house rules). Unchanged. Since PRs are squash-merged, the title is the permanent commit message, and feat(qwp): stop resending the full symbol dictionary on every message covers roughly one of eight shipped behaviours. Unmentioned: symbol() now throws at 1M distinct values; slot quarantine abandons buffered data; foreground auth/upgrade failures now retry instead of failing (a permanently-wrong credential is now silent); .symbol-dict counts against sf_max_total_bytes and that cap is now soft (C1/R2); a new connect-string key; nextRow counts padding. Fixes #69 is still missing — repo issue #69 (OPEN) is the JDK 8 MmapSegmentRecoveryFaultTest InternalError, and branch commit ece7817b fixes its root cause and says so in its own body. Labels are enhancement, tandem; bug is still missing.

M18 — SenderError.Policy javadoc documents a resolution chain that does not exist (in-diff). SenderError.java:300-318 describes errorPolicyResolver, per-category errorPolicy and connect-string on_*_error. No such builder methods exist (errorPolicyResolver appears nowhere but that javadoc), and all six on_*_error keys are accepted no-ops. This PR edits that block to add the DATA_LOSS sentence, extending a false public contract.

M19 — QwpWebSocketEncoder.splitMessageSize throws OutOfMemoryError for arithmetic overflow (:273). Nothing is allocated. OutOfMemoryError is a VirtualMachineError; supervisors kill processes on it, and no catch (LineSenderException) in the sender — including close()'s new catch (BatchTooLargeForCapException) — sees it. Should be BatchTooLargeForCapException.

M20 — Lazy-connect + recovered slot can stay "initializing" indefinitely (in-diff). hasEverConnected now latches at the end of swapClient, after the dictionary catch-up (:2570-2580) — deliberate and correct in isolation. But in ASYNC mode the constructor seeds it false, so a recovered slot whose first catch-up keeps failing never leaves initialization, and endpointPolicyFailureIsTerminal() keeps latching auth/upgrade rejections producer-fatal for a producer that is actively buffering. Narrow (lazy_connect + recovered slot + repeated catch-up failure), but the window is unbounded.

Minor

• Member ordering. The 28-method public @TestOnly block sits inside the private region of CursorWebSocketSendLoop (:3077-3211) and is internally unsorted (catchUpFrameGrowthCount before catchUpCapGapAttempts); lastReconnectError() (:1976) is far out of position. Plus ~30 alphabetical-ordering drifts across PersistedSymbolDict, RecoveredFrameAnalysis, QwpWebSocketSender, CursorSendEngine, SlotLock, Files, SegmentRing, MmapSegment, BackgroundDrainer, SegmentManager. Test classes are worse: DeltaDictRecoveryTest interleaves tests, helpers and five nested classes with no order at all; @Test alphabetical adjacency is 36-56% out-of-order in nine new classes vs. a 13% baseline.
• Dead 9-arg CursorWebSocketSendLoop constructor (:621) with zero callsites, still labelled "Master constructor" while the real one is the private 13-arg at :719.
• CATCHUP vs catch_up vs catchUp — three spellings for one feature (DEFAULT_CATCHUP_CAP_GAP_MIN_ESCALATION_WINDOW_MILLIS, MAX_CATCHUP_CAP_GAP_ATTEMPTS, UNCAPPED_CATCHUP_PACKING_LIMIT).
• DictionaryGapNackTest — the PR's headline end-to-end 0x0D test registers no errorHandler and asserts nothing about the category; deleting the classify arm leaves it green (DICTIONARY_GAP and UNKNOWN both map to RETRIABLE, and nothing in production branches on the category). The mapping is pinned by DictionaryGapPolicyTest:40-45, so this is a gap in the e2e test's stated claim, not in coverage.
• Test reuse. Three new classes hand-roll rmDirRec while this PR adds TestUtils.removeTmpDirRec (21 copies tree-wide, 0 migrated); CatchUpAlignmentTest:1576 re-implements the newly-public static NativeBufferWriter.writeVarint; four new one-off wait helpers plus 19 inline copies in DeltaDictRecoveryTest; reflection left in CursorWebSocketSendLoopDurableAckTest/DurableAckFuzzTest for seams this PR added and used elsewhere; SegmentRecoveryIntegrityTest:1067 re-declares a local facade instead of the new shared DelegatingFilesFacade; five temp-directory idioms across fifteen new classes.
• Stale citations. MmapFaultDegradesTest:66,67,137 cite QwpWebSocketSender.java:4028/:3944/:4242; those anchors were already wrong at ddab88db and have moved again since. :66-69 also claims a guard "is not independently exercised here" while :151, 70 lines below, exercises exactly it.
• Eighteen un-anchored M1/C2-style review-round identifiers in new test comments, only one of which is resolvable anywhere, with the letters colliding across files. Squash-merge makes "review round 5" unlocatable the day this lands.
• Assert-only guards in shipped code (users run without -ea): publishDictionaryChunks's assert !deltaDictEnabled — still the sole guard on the write-ahead ordering invariant, and now the only thing keeping the fallback out of delta mode, so it has become more load-bearing rather than less; make it a real throw. Also flushPendingRowsSplit's assert messageSize <= cap (add a LOG.error) and Crc32c.updateUnsafe's assert len >= 0 (silently returns the seed, i.e. fail-accept).
• FilesFacade.isMmapAllowed() defaults to this == INSTANCE — a production decorator would silently lose mmap. Prefer return true with test facades opting out. Relatedly, DelegatingFilesFacade's javadoc claims it "forwards every call" but overrides only the abstract methods, so isMmapAllowed() returns false for every subclass.
• 90000L without separators (WsSenderConfigHonoredTest:80); one unused import (CursorWebSocketSendLoopMirrorLeakTest:29); ~30 new booleans without is/has (against an 85% module-wide baseline, so not a regression); three test names describing mechanism rather than behaviour.
• Comment volume: 52% of added production lines and 23% of added test lines vs. a ~12% baseline. Almost none of it restates the code, but two blocks assert server internals this repo cannot pin, and sendCatchUpChunk:3055 sets FLAG_GORILLA on a row-less frame under an 11-line comment that explains only the neighbouring flag.

---

Downgraded (verified false positives)

• Constructor arity change silently re-binds stale callers — refuted. Every new overload puts boolean durableAckMode where a stale caller passes a long; there is no long→boolean conversion, so every missed callsite is a compile error. All 35 callsites were checked and the clean JDK 8 CI build is conclusive.
• Mid-enum Category insertion breaks consumers — refuted. Zero ordinal(), values()[i], EnumMap/EnumSet, or serialized use across all three repos; the single switch has explicit arms for both new constants.
• Two-slice sendBinary breaks WebSocket masking — refuted. Both slices are copied contiguously into the send buffer before framing, so endFrame masks [payloadStartOffset, writePos) in one pass and the length field covers both by construction.
• ensureConnected's rollback can leave two I/O loops on one engine — refuted. start() can only throw with ioThread == null, which makes close() skip the entire if (t != null) block containing all four of its throw sites.
• A released 1.3.5 client HALTs on the new 0x0D — refuted as reachable. A pre-delta client always sends confirmedMaxId = -1 → deltaStart = 0, and the server rejects only deltaStartId > size(), so an old client cannot provoke the new status byte. (The "unreleased" claim in the body is still inaccurate — see M15.)
• The chunked full-dict group is not reconnect-atomic as described — refuted. The server withholds the ack for every deferred frame regardless of row count, so a chunk can never be trimmed ahead of its data frames, and process-crash recovery flips the catch-up back on via recoveredMaxSymbolDeltaStart > 0. The body now says the same.
• preRegisterDictionaryChunks sizing is short — refuted, and now moot: HEADER_SIZE = 12 already contains the tableCount short and payloadLength int, so the arithmetic exactly matched what beginMessage writes.
• splitFramesFit(cap, currentBatchMaxSymbolId) trips splitDeltaEntriesLength's IllegalStateException — refuted. splitDeltaCount == 0 short-circuits before the mismatch check. (This one now guards the M1 fix, so it is load-bearing.)
• errorDispatcher is null when orphan drainers fire — refuted. It is created unconditionally in ensureConnected (:3776), which runs inside connect(), before Sender.build() calls startOrphanDrainers.
• A transport error resetting the capability-gap episode loses a safety property — refuted. The checklist requires that transient classes never burn the terminal budget; resetting is the compliant direction. (Worth confirming the intent.)
• nextRow's throw leaves hasInProgressRow() true and breaks callers — refuted. at()/atNow()/symbol() all wrap in catch (RuntimeException | Error) { rollbackRow(); throw; }, and cancelCurrentRow() clears both fields.
• The write-ahead persist has an ordering hole — refuted. Every sealAndSwapBuffer() callsite was enumerated; each delta-mode publish is preceded by persistNewSymbolsBeforePublish(), no beginMessage intervenes, and the fast/slow-path selector exactly matches the staged deltaStart.
• The mirror leaks / is double-freed / borrows past the engine's life — refuted across all teardown paths; Unsafe.realloc failure leaves (addr, capacity) a matched pair because the capacity is assigned after the try.

---

Summary

Verdict: the blocking issues are cleared. Both Criticals and the two Moderates flagged as "next" (M1, M3) are fixed and verified at source, along with M2 and M7. Nothing remaining is merge-blocking on its own; what is left is a tail of correctness hardening, a public-API contract that documents features that do not exist, and test-efficacy gaps.

Regressions and tradeoffs, stated plainly: delta mode is a large win (per-flush dictionary work drops from O(whole dictionary) to O(new symbols)). Full-dict fallback mode no longer carries the ~2× per-flush CPU regression M7 described, and the failure modes M1/M3 added are gone; what it does now is ship extra frames per over-cap batch, which is bytes full-dict mode already paid, re-spread. sf_max_total_bytes becomes best-effort with a hard floor of two segments per ring plus the side file — a real loosening of a disk budget, chosen over a deadlock, and the right call, but it should be documented as a config property rather than only warned about at runtime. Memory grows by a second native copy of the dictionary (~21 MB at the 1M cap, on top of ~88 MB of Java strings). The foreground reconnect-policy change makes a permanently-wrong credential retry silently, which is right for store-and-forward but is still not mentioned in the title.

Suggested sequencing for what remains:

1. R1 — the three PR-body passages that now describe the pre-fix behaviour, before the squash lands.
2. M10, M5 — both one-liners with data-integrity/usability impact.
3. M4, M19, M8 — cheap correctness hardening in the same sweep, plus promoting publishDictionaryChunks' assert !deltaDictEnabled to a throw now that it carries more weight.
4. M16 — the test gaps, of which the drainer-sink wiring and the c5974217 type-narrowing are the ones that would have caught real regressions.
5. M17, M15 — title/labels/Fixes #69 and the committed spec.

fixed and re-reviewed by Alex

[mtopolnik]

[PR Coverage check]

😍 pass : 1785 / 1943 (91.87%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SfOperationalException.java	0	2	00.00%
🔵	io/questdb/client/cutlass/qwp/client/WebSocketResponse.java	0	1	00.00%
🔵	io/questdb/client/std/Files.java	2	3	66.67%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/BackgroundDrainer.java	52	65	80.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java	18	21	85.71%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/OrphanScanner.java	6	7	85.71%
🔵	io/questdb/client/cutlass/qwp/client/GlobalSymbolDictionary.java	17	20	85.00%
🔵	io/questdb/client/Sender.java	84	97	86.60%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/PersistedSymbolDict.java	420	472	88.98%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java	262	288	90.97%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java	102	112	91.07%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java	399	424	94.10%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentManager.java	49	51	96.08%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/RecoveredFrameAnalysis.java	162	168	96.43%
🔵	io/questdb/client/cutlass/qwp/client/QwpWebSocketEncoder.java	54	54	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/BackgroundDrainerPool.java	5	5	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/MmapSegment.java	7	7	100.00%
🔵	io/questdb/client/cutlass/http/client/WebSocketClient.java	11	11	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/SlotLock.java	50	50	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/UnreplayableSlotException.java	2	2	100.00%
🔵	io/questdb/client/std/FilesFacade.java	2	2	100.00%
🔵	io/questdb/client/std/Crc32c.java	27	27	100.00%
🔵	io/questdb/client/cutlass/qwp/client/NativeBufferWriter.java	8	8	100.00%
🔵	io/questdb/client/cutlass/qwp/client/sf/cursor/DefaultSenderErrorHandler.java	6	6	100.00%
🔵	io/questdb/client/impl/ConfigSchema.java	1	1	100.00%
🔵	io/questdb/client/cutlass/qwp/client/BatchTooLargeForCapException.java	2	2	100.00%
🔵	io/questdb/client/impl/SenderPool.java	14	14	100.00%
🔵	io/questdb/client/SenderError.java	12	12	100.00%
🔵	io/questdb/client/cutlass/qwp/protocol/QwpTableBuffer.java	11	11	100.00%

[glasstiger]

Code review — feat(qwp): stop resending the full symbol dictionary on every message

Adversarial review pass over the full diff (94 files, +22,635/−1,382) at head 8ae795a4, merge base db25b543. Every finding below was verified against the source at the cited lines before being reported; findings that did not survive verification are listed under Downgraded so the reasoning can be checked.

Edited after posting: M4 (binary compatibility) has been withdrawn — QWP is not officially released, so its exported classes carry no compatibility guarantee and there is no caller to break. It is struck through in place below to preserve the numbering, and restated under Downgraded. Counts in the Summary have been corrected.

Gates and first-hand evidence

• Committed-binary gate: PASS — all 94 changed files are .java with numeric line counts; no binaries.
• Java 8 floor: clean — verified by compiling all 31 changed production files with javac --release 8, not by grep. The only unresolved symbols are sun.misc.Unsafe/FDBigInteger, which are ct.sym exclusions rather than violations.
• Build: green — mvn -pl core test-compile on JDK 11.
• Tests: 159/159 pass at PR head across the 12 core new suites (DeltaDictRecoveryTest, PersistedSymbolDictTest, CursorWebSocketSendLoopCatchUpAlignmentTest, SelfSufficientFramesTest, DeltaDictCatchUpTest, DictionaryGapNackTest, SegmentManagerSideFileCapTest, and others).

---

Critical

C1 — The tandem submodule chain points at commits that exist only on branches, and both tandems merged before this PR's last two fixes

Three verified facts:

1. OSS master → an open PR branch. questdb/questdb@master has its java-questdb-client submodule at 8f5ed4f91b6f. That commit is on qwp-delta-symbol-dict only — git merge-base --is-ancestor 8f5ed4f9 origin/main fails, and git branch -a --contains 8f5ed4f9 lists just the PR branch. Since this repo squash-merges, 8f5ed4f9 will never become an ancestor of client main; if the branch is deleted after merge, master's submodule pointer dangles.

2. Enterprise main → an orphaned OSS commit. questdb-enterprise@main has its questdb submodule at ce3b1409, which is OSS #7374's branch head, not its squash commit 9057e3800b. ce3b1409 is not an ancestor of OSS master. This is anomalous rather than the house workflow: the previous five enterprise pointer bumps all landed on OSS master commits, and only this one is orphaned.

3. Both tandems merged before the last two client fixes. OSS #7374 merged at 2026-08-05T11:35:04Z, enterprise #1122 at 10:43:40Z. Client commits 8615c55c ("Stop an unusable symbol dictionary wedging a slot") and 8ae795a4 ("Commit dictionary chunks a failed flush stranded") are dated 11:33:59Z and 11:34:14Z and are not in the pointer either tandem merged. Between them they change 722 lines across CursorSendEngine, CursorWebSocketSendLoop, PersistedSymbolDict and QwpWebSocketSender.

Consequence. The PR body's evidence — "The enterprise SqlFailoverQwpClientLosslessTest (file-mode failover) passes end-to-end against a real server" — holds for 8f5ed4f9, not for the head under review. The two newest production fixes have no end-to-end validation.

Suggested fix. Land this PR, then push follow-up bumps in both parent repos to the resulting client main SHA and the OSS master commit respectively, and re-run the enterprise failover suite against them.

---

Moderate

M1 — GlobalSymbolDictionary.truncateTo has zero test coverage, and the branch preventing silent symbol misattribution is never exercised

GlobalSymbolDictionary.java:218-241 (new, 42 lines) and QwpWebSocketSender.java:4698-4711. grep -rn truncateTo core/src/test/ returns nothing. The one reclaim test, SelfSufficientFramesTest:1117, runs in memory mode (ws::addr=, no sf_dir), so cursorEngine == null and the floor collapses to 0. Checked per-method: no test contains both reset() and sf_dir, so the floor = pd.size() branch at :4704-4709 never runs.

That branch is the sole guard preventing reset() from handing an already-persisted id to a different string — the "silent misattribution the dense id space exists to prevent" named in its own javadoc — and reset() is the documented user recovery from the over-cap rejection. Also untested: the !deltaDictEnabled early return, truncateTo's negative-arg guard, the newSize >= size no-op, and the symbolToId rebuild.

The floor logic itself traced sound on every constructible path, so this is a coverage gap on correct code rather than a live bug — but a future edit to either method ships green.

M2 — disableDeltaDict is never exercised mid-life; every degrade test arms before the first flush

QwpWebSocketSender.java:4369. MmapFaultDegradesTest:106 sets ff.armed = true before the first atNow() at :107; DeltaDictRecoveryTest:2126 is explicit ("Armed from the start"). Every degrade therefore fires at sentMaxSymbolId == -1 with a one-entry dictionary, while the method's javadoc is entirely about degrading mid-run on a full disk. Untested consequences at sentMaxSymbolId = K > 0: symbolDeltaBaseline() flips K → -1 so the next frame re-registers [0..currentBatchMaxSymbolId] on a connection already holding [0..K]; publishDictionaryChunks first becomes reachable from a degrade; advanceSentMaxSymbolId() freezes.

Suggested test. SF file mode: flush ~50 distinct symbols successfully, then arm the dictionary-mmap fault; assert the next flush throws with isDeltaDictEnabledForTest() == false, the retry ships a self-sufficient frame, and a forced reconnect tiles the catch-up gap-free.

M3 — PersistedSymbolDict has zero concurrency coverage and no append-after-close test

grep -cE "new Thread|CountDownLatch|ExecutorService|CyclicBarrier" PersistedSymbolDictTest.java returns 0. The class javadoc justifies its synchronized with a concrete corruption scenario (a close racing an in-flight append unmapping the append region and letting a write land on a reused fd), and the object is genuinely touched by the producer thread, the I/O send loop, the SegmentManager worker and close(). Removing synchronized from any one method, or any one if (closed) return; guard, is invisible to CI today.

M4 — Binary-incompatible public API changes in exported packages that shipped in 1.3.6 — WITHDRAWN

This finding is withdrawn; no action needed. See Downgraded below for the reasoning. Original text kept for the record:

module-info.java:59-61 exports qwp.protocol and qwp.client.sf.cursor. Verified against tag 1.3.6:

• QwpTableBuffer.nextRow() — public void → public long. The return type is part of the JVM method descriptor, so this is source-compatible but binary-incompatible.
• SegmentRing.findLastFsnWithoutPayloadFlag(int,int,int,int) — removed outright.
• MmapSegment.findLastFrameFsnWithoutPayloadFlag(int,int,int,int) — removed outright.

M5 — BackgroundDrainer's "unreplayable: " quarantine arm and the orphan-drainer DATA_LOSS wiring are untested

BackgroundDrainer.java:702-722. grep -rn 'unreplayable: ' core/src/test/ returns 0 hits; BackgroundDrainerUnreplayableSlotQuarantineTest:117 asserts the "setup: " prefix from a different catch arm. This arm converts the feature's own core failure — a torn .symbol-dict the surviving frames cannot rebuild — into a permanent quarantine plus a DATA_LOSS report. Mis-order it against the retryable catch (Exception) below and the drainer re-adopts the same slot on every scan with the user never told.

Separately, BackgroundDrainerPool.setErrorSink (:231) and its submit-time application (:287-289) have zero test references — the two tests asserting a drainer DATA_LOSS call drainer.setErrorSink(...) directly, bypassing every line of the wiring. This client ships slf4j-api with no binding, so that sink is the only thing making an abandoned orphan slot audible.

Three of five dispatchDataLoss sites have no test asserting the report reaches a sink: :419 (durable-ack persistently unavailable), :864 ("wire: "), :720 ("unreplayable: ").

M6 — SegmentManagerSideFileCapTest:80-96: the headline "must refuse to provision" assertion cannot fail

prepopulate(slotDir, 2) at :80; the assertion at :93-96 is assertEquals(..., 2, countSfaFiles(slotDir)) — exactly the pre-start() state. Nothing creates a .sfa synchronously, so if the 1 ms-tick worker is not scheduled inside Thread.sleep(100), the refusal is proven by nothing. Confirmed by deleting the sleep entirely: still passes. The sibling test at :117-128 documents this precise hazard ("a flat 100ms sleep flakes on a loaded CI box where the worker's 1ms tick gets delayed past the sleep window") and polls instead. The gauge arithmetic assertion at :88-89 is non-vacuous; only the refusal half is unproven.

M7 — A harness self-join burns ~5 s in five new suites, and one test passes on negative margin

DeltaDictRecoveryTest:1750-1758 exits its poll loop on timeout, not success: the condition first goes true at ~5024 ms against a 5000 ms deadline, and it passes only because the assertion at :1757 re-reads the counter after the loop gives up. Root cause is out-of-diff — TestWebSocketServer.ClientHandler.close() does readThread.join(5000) with no self-join guard, and the PR's new handlers call client.close() from that same read thread while holding the handler monitor.

Same tax in DeltaDictCatchUpTest:458,543,641 (all 5 tests, 25.8 s of a 25.8 s suite runtime), CursorWebSocketSendLoopForegroundReconnectPolicyTest:422, and SenderPoolDataLossNotificationTest:392. One guard — readThread != Thread.currentThread() — fixes all of them.

M8 — Test-only hooks shipped as JVM-global mutable public statics

Sender.java:3262-3270 adds public static setQuarantineAfterCloseHookForTest(Runnable) and setQuarantineFilesFacadeForTest(FilesFacade) to Sender.LineSenderBuilder — the flagship class in exported io.questdb.client — with no javadoc. CursorWebSocketSendLoop.java:247 adds public static volatile boolean forceMirrorSeedFailureForTest. Any caller can redirect every subsequent quarantine's filesystem access process-wide.

Every other seam this PR adds uses instance-level injection (PersistedSymbolDict.open(FilesFacade, …), SlotLock.acquireLogical(FilesFacade, …), SegmentManager(…, FilesFacade)), so the pattern already exists. Current tests do reset all three in finally and surefire is sequential, so contamination is contained today — the exposure is the API surface, not the tests.

M9 — Latent use-after-free the PR introduces and defers (no reachable trigger today)

Listed under Follow-ups in the PR body; the hazard is genuinely new here. CursorWebSocketSendLoop.java:798 makes the send loop's mirror a borrower of PersistedSymbolDict's native memory (sentDictBytesAddr = pd.loadedEntriesAddr(), sentDictBytesOwned = false), while Sender.build()'s rollback calls cursorEngine.close(false) without the failed-stop check the close-delegation protocol requires.

Reachability. ensureConnected's catch closes cursorSendLoop before rethrowing, so on the normal path the loop is down before the engine is freed. The window requires close() to return having failed to stop the I/O thread within 30 s — in practice an OOM. Filed as a latent invariant violation: no user-visible impact today, and what would make it live is any change letting build()'s rollback run while the loop is still up.

M10 — MmapFaultDegradesTest.MmapFaultDictFacade:258-275: unscoped fault plus a data race

mmap throws on the first armed call regardless of fd or flags, while its sibling HealMmapFaultFacade:233-234 correctly scopes on fd == dictFd && flags == MAP_RW. SegmentManager's worker also mmaps through this facade, so a provisioning tick after ff.armed = true (:106) eats the single-shot fault into its retry catch and the test fails at Assert.fail(...) (:110). boolean armed (:259) is non-volatile yet written by the test thread and read by the manager worker.

M11 — CloseDrainTest.GatedAckHandler:774-785: AssertionError thrown on the server read thread is swallowed

The read thread's only catch is catch (IOException e) (TestWebSocketServer.java:724), so the AssertionError never reaches JUnit. The test instead fails at :747 on "drain timed out" and the real diagnosis goes to stderr. DeltaDictRecoveryTest:314,339,360-362 already handles this correctly via an AtomicReference<Throwable>.

M12 — Deferred dictionary-chunk commit debt is tracked outside the region that discharges it

QwpWebSocketSender.java:3999-4044. publishDictionaryChunks(...), dictionaryChunksAwaitCommit = true, encodeCombinedFrame(...) and encoder.finishMessage() all sit outside the try whose catch calls commitOrphanedDictionaryChunks. A throw from the last two would leave FLAG_DEFER_COMMIT chunks on the ring with the debt un-discharged: ackedFsn freezes for the connection's life, trim stops and the ring fills — precisely the outcome that method's javadoc says it prevents.

Reachability. The only candidate thrower is NativeBufferWriter.ensureCapacity's OutOfMemoryError, and on this re-encode the buffer already holds a strictly larger message while beginMessage only calls reset(), so no growth is needed. No reachable trigger found. One-line hardening: move :4005-4008 inside the try (the flag assignment moves with them).

---

Minor

• Orphaned javadoc. QwpWebSocketSender.java:4380-4403 documents healPersistedDictionary but is immediately followed by a second /** for dictionaryEntryWireBytes. Javadoc keeps only the last block before a declaration, so this one is discarded and healPersistedDictionary (at :4417) ships undocumented — the one method whose write-ahead invariant a future reader most needs. Looks like alphabetical-sort fallout.
• CursorSendEngine.java:1290 calls PersistedSymbolDict.removeOrphan(sfDir) while :506, :558, :666, :743 all route through the dictFf seam. Production-identical (dictFf == FilesFacade.INSTANCE on all non-@TestOnly constructors), but a fault facade cannot reach the drained-close unlink path.
• SegmentManager.java:964-980 — the liveness floor fires on any cap shortfall, not just the un-reclaimable side-file kind its rationale at :87-99 describes. Benign in production (one ring per manager), but with the shared-manager constructors N rings each claim a 2-segment floor, so the shared cap can overshoot by N × segmentSizeBytes even when trim on a sibling ring would have cleared it. Narrow the predicate or document the multi-ring behaviour on livenessFloorBytes.
• Test residue. SelfSufficientFramesTest:1480 hasDeferCommit — added by this PR, zero callers. SenderPoolDataLossNotificationTest:119 cites task-3-report.md, which exists nowhere in the tree. CursorWebSocketSendLoopPoisonFrameTest:1200 javadoc says "frames are delivered by reflection" in a file with zero reflection left, and the helper keeps the reflection-era name invokeOnBinaryMessage. MmapFaultDegradesTest:73,75 cite a test and a method that exist nowhere; :66,67,137 cite stale line numbers (actual: 4436, 4520). WsSenderConfigHonoredTest:80 — 90000L wants 90_000L.
• PrReviewRedTests.java cleanup is half-done. The deletion is right and the coverage moved correctly — C2 landed stronger in SegmentRingTest:1146 (assertEquals(publishedFsn, ackedFsn) replacing assertTrue(<=)). But PrReviewRedTestsE2e.java (13 KB) is untouched and carries the same review-round naming, and testC7_strayBranchReviewMarkdownAbsent was dropped with no replacement (it guarded a file present in no branch head, so no real loss — SourceHygieneTest would be its natural home).
• Duplication the PR's own new helpers already solve. SelfSufficientFramesTest:1541 reimplements QwpWireTestUtils.buildAck(long) byte-for-byte, in the same package, in a file that already calls QwpWireTestUtils on three other lines. DeltaDictCatchUpTest:434 inlines QwpWireTestUtils.hasDelta and hardcodes 0x01 where QwpConstants.FLAG_DEFER_COMMIT is public. Four verbatim copies of a ~35-line WebSocket handler skeleton (including a six-line comment reproduced word-for-word); the DeltaDictRecoveryTest server prologue stamped 14 times; the SelfSufficientFramesTest engine block 8 times; four more hand-rolled poll-until-true helpers on top of seven existing ones, in a PR that already edits TestUtils.java.
• Member ordering. New files have no legacy excuse: RecoveredFrameAnalysis puts public close() after every package-private member; PersistedSymbolDict has appendMapGrowthCount/appendWriteCount/appendedBytes after close(). Regressions in edited files: SegmentManager.java:711 and SlotLock.java:217 insert privates into public runs that were clean pre-PR. Test files: 16 of 30 @Test methods out of order in CursorWebSocketSendLoopCatchUpAlignmentTest, 11 of 30 in DeltaDictRecoveryTest, 10 of 38 in PersistedSymbolDictTest.
• Boolean naming (is…/has…). committedGap, runningGap, runningUnackedGap, mappedAppend, mappedRecoveryInput, closed, sentDictBytesOwned, dataFrameSentThisConnection, and armed (recurring verbatim in three test files). RecoveredFrameAnalysis has hasRewoundSinceCommit immediately beside runningGap, so the rule is broken inside a single new file.
• Comment density. The 21 new test files total 10,433 lines, 22% comments excluding license headers; DeltaDictRecoveryTest is 30%, with 23 of 30 tests opening on six or more lines of prose (worst: 26 lines). Much of it argues why the test would go green without the fix — mutation rationale that belongs in the PR description rather than stamped into 30 method bodies, where it rots exactly as CursorWebSocketSendLoopPoisonFrameTest:1200 already has.
• Uncommitted working-tree change. SegmentManagerUnlinkFailureTest.java carries +17/−3 uncommitted on this branch — a race fix converting a one-shot unlinkFailuresRemaining into a volatile boolean failUnlink, with a comment proving the first manager's 1 ms tick could win. It is not in the PR as pushed. The test ran green 8/8 at PR head here, so the code comment is the evidence rather than a local reproduction; it should land on this branch rather than sit in a working tree. The new field also wants isUnlinkFailing.
• Labels. enhancement + tandem. Since the change is fundamentally a bandwidth/throughput win, Performance fits too. Title, Conventional Commits format, end-user framing and the Tradeoffs section are all fine.

---

Downgraded (raised during review, dismissed after verification)

• M4, binary-incompatible public API changes (withdrawn after posting) — the finding rested entirely on "exported package, present in tag 1.3.6". QWP has not been officially released, so its exported classes carry no compatibility guarantee, and a git tag is not a compatibility promise for a subsystem the PR body itself documents as experimental and moving in lockstep with the server. All three items are QWP classes (qwp.protocol, qwp.client.sf.cursor), so there is no caller outside these three repos to break; everything in-repo recompiles from source. The descriptor change is real (nextRow()V → nextRow()J, demonstrated with a NoSuchMethodError against a caller compiled to the 1.3.6 shape) — it simply has no one to affect.
  
  Optional cleanup only, no correctness argument: the no-arg nextRow() is a pure delegate (return nextRow(0, Long.MAX_VALUE);), and of 275 no-arg callsites across all three repos exactly one consumes the return value — QwpTableBufferTest.java:1261, a test added by this PR in 0b47d85c. Reverting it to void is one production line plus one test line. The two-arg nextRow(long, long) that sendRow uses for the byte budget is unaffected either way.
• Duplicate .symbol-dict entries after a failed publish — fixed. persistNewSymbolsBeforePublish resumes from pd.size() (:4480), not sentMaxSymbolId + 1, making the write-ahead idempotent across retries.
• Missing enterprise tandem — it exists (questdb-enterprise#1122, merged). The real problem is different and is C1 above.
• reclaimUnsentSymbolIds floor unsound — no state could be constructed where a persisted or shipped id is reclaimed. advanceSentMaxSymbolId runs strictly after sealAndSwapBuffer on both flush paths (:4028-4031, :4250-4253), appendBlocking either publishes or throws with no partial ring state, and PersistedSymbolDict advances size only after a full chunk lands.
• Transients burning the orphan drainer's capability-gap terminal budget — compliant. BackgroundDrainer.java:459-464 resets both the attempt counter and the wall clock on any transient, which is what "16 consecutive capability-gap sweeps" requires.
• SenderError enum ordinal shift from mid-enum insertion — no ordinal() usage anywhere in main, and SenderError.java is absent from tags 1.3.4/1.3.5/1.3.6, so it has never shipped.
• PersistedSymbolDict.occupiedDiskBytes() racy unsynchronized read — both appendOffset (:217) and reservedFileBytes (:239) are volatile with a documented single-writer contract. No tearing, bounded staleness.
• Java 8 floor violations — none, confirmed by --release 8 compilation of all 31 changed production files.
• PrReviewRedTests deletion as a coverage regression — C1 and C2 were genuinely re-homed into SegmentRingTest, and C2 landed strictly stronger.
• accumulateSentDict partial-overlap drop — the seam is covered end-to-end in DeltaDictCatchUpTest, and the producer emits strictly contiguous deltas by construction.

---

Summary

Verdict: request changes — on C1, which is a release-integrity problem rather than a code defect, and on the M1/M2/M3/M5 coverage gaps.

The production code came out of this review well. The correctness pass targeted the four highest-risk questions in the design — the id-reclaim floor, write-ahead duplicate entries, catch-up chunk tiling, and the cap-gap escalation counter — and found all four sound, with the reasoning recorded. The store-and-forward invariants hold: the reconnect loop is genuinely unbounded (connectLoop's while (running) with no deadline), DICTIONARY_GAP (0x0D) classifies retriable, an unknown status byte fails open, and no path advances the ack watermark past a NACK. Every Critical-severity candidate collapsed under verification except the submodule one.

The weak spot is test coverage and hygiene rather than test volume. Across 15,000 new lines of tests, the single method whose job is to prevent silent symbol misattribution has zero references; the mid-life degrade the feature's own javadoc is written about is never driven; and a 1,518-line synchronized class shared by four threads has no concurrency test at all.

Tradeoffs worth restating: each reconnect now replays the full dictionary; file mode adds a side-file per slot; and the deferred-borrow relationship in M9 is a new hazard this PR creates and defers.

Counts (corrected after the M4 withdrawal): 34 draft findings raised, 23 verified and reported, 11 dropped as false positives or withdrawn. Split: 21 in-diff, 2 out-of-diff (the submodule chain, and the TestWebSocketServer self-join root cause behind M7).

[glasstiger]

Follow-up: customer exposure of the Moderate findings

Companion to the review above, which ranked the 11 Moderate findings by certainty of defect. Overlaying customer exposure produces an almost inverse ordering, which is worth having on the record before triage.

No Moderate finding impacts a customer today. Every one is either test-only, or a coverage gap over production code that traced correct on inspection. So the useful question is not "impact today" but "what does a customer see if the behaviour this guards regresses, or if the latent trigger fires".

	Defect lives in	Impact today	If it regresses / fires, the customer sees
M1 truncateTo uncovered	Test gap (prod traced correct)	None	Silent symbol misattribution — wrong SYMBOL values in their tables, no error raised. Requires an edit to truncateTo / reclaimUnsentSymbolIds.
M2 mid-life degrade	Test gap (prod reasons benign, unverified)	None identified	Same class as M1, but layered on a disk-full incident and misattributed to it. The only item here where neither a test nor a trace has established correctness.
M3 dict concurrency	Test gap (prod traced correct)	None	Largest blast radius in the review: dictionary bytes written into an unrelated file via a reused fd, or SIGSEGV killing their JVM. Requires an edit to the class's synchronization.
M5 drainer notify	Test gap (wiring traced correct end-to-end)	None	Data abandoned on disk with nobody told, or a drainer livelock. Requires a catch-arm reorder, or the sink wiring breaking later.
M6 vacuous cap assertion	Test only	None	sf_max_total_bytes overshoot -> disk fills -> 30 s appendBlocking timeouts and producer stalls. Requires a cap-decision regression; the accounting is still guarded non-vacuously by :88-89.
M7 self-join / margin	Test only	None, ever	Nothing. Costs CI time and investigation hours, never a user.
M8 public static hooks	Shipped API surface	None realistically	The only finding visible to customers at all — it appears in javadoc and IDE autocomplete on Sender.LineSenderBuilder. Abuse would take deliberate effort.
M9 borrow / UAF	Production (latent)	None — trigger needs close() to fail stopping the I/O thread within 30 s	Use-after-free in the customer's JVM: SIGSEGV or silent native memory corruption. The most severe runtime consequence in the review.
M10 unscoped fault	Test only	None, ever	Nothing. CI flake.
M11 swallowed assert	Test only	None, ever	Nothing. A failing test reports the wrong cause.
M12 commit debt	Production (latent)	None — no reachable thrower found	ackedFsn freezes for the connection's life -> trim stops -> SF ring fills -> producer backpressure and stall.

What this reorders

• Four findings can never touch a customer under any circumstance: M6, M7, M10, M11. These are CI hygiene — a real cost, but paid in engineer-hours rather than user impact. Both of the "certain defect" items sit here.
• Two findings are in production code with customer-facing failure modes: M9 and M12. Both have a named unreachability barrier, which is why neither is Critical, but they are the only Moderates where the defect is in shipping code rather than in a test.
• Four are insurance on silent-failure paths: M1, M2, M3, M5. Nothing is wrong today; the exposure is that a future regression there produces data corruption or silent loss with no error to catch it.

Prioritised by customer risk rather than defect certainty, the order becomes M12 -> M9 -> M2 -> M3 / M1 / M5 -> M6 -> M8 -> M7 / M10 / M11. M12 in particular is a one-line change (move :4005-4008 inside the existing try) standing against a production stall, which makes it the best return in the set on that axis.

Two caveats on reading this table

1. "Traced correct" is weaker evidence than "proven by test." That gap is precisely what M1/M2/M3/M5 are: the code works when read, and nothing would tell you if that stopped being true. Reading the table as "nothing matters" inverts its point.
2. This review ran at level 1, which deliberately skips four reviewer dimensions: dedicated concurrency, resource management / leak-on-error-path, cross-context caller impact, and the adversarial fresh-context pass. For a change adding a 1,518-line mmap'd file format, a native mirror with borrow semantics, and new quarantine paths, those are the angles most likely to surface something the structured passes cannot see. Absence of found bugs here is not proof of absence, and a level-3 pass is the thing most likely to change this table.