feat(stats): hand transactions to the tracker over a channel, not a lock

[bdchatham]

TOT-010, TOT-014, TOT-018. Seventh in the outcome-tracking stack. Closes User Story 3.

The stall

The sender called Register on the goroutine that had just completed a send, and Register took the registry lock. The reap loop holds that same lock while it walks every in-flight transaction. So a reap sweep landed directly in the latency this package reports, and the measurement slowed the thing it measures.

The hand-off

Submit is a non-blocking send into a channel. A dedicated goroutine drains it and does the admission.

func (t *InclusionTracker) Submit(ctx context.Context, tx *types.LoadTx) {
	select {
	case t.submit <- tx:
	default:
		// count the drop, then report it
	}
}

A full channel drops and counts. Both halves are required: a hand-off that blocks brings back the stall this removes, and one that drops in silence leaves an accepted transaction with no terminal state, which breaks the conservation identity.

Why the drain does no chain read

This is the requirement that makes the depth derivable rather than guessed (TOT-018).

A drain sharing the head loop inherits that loop's block read. At a few thousand transactions per second no depth absorbs a multi-second stall, so the number would be a guess defended by nothing. A drain that only admits waits as long as the registry lock is held, which a reap sweep bounds at microseconds.

The depth then follows from the configured send rate: one second of headroom, floored for a profile that sets no rate. That is four orders of magnitude above what the drain actually stalls for, and shallow enough that a drain which genuinely cannot keep up says so through dropped_at_handoff rather than hiding in a deep queue.

dropped_at_handoff stops being the count with no producer that #78 flagged.

D-1 and D-2

D-1, the queue depth, is answered here: the clarification said the depth follows once the drain's goroutine is settled, and TOT-018 settles it.

D-2, whether a drop voids the run, stays deferred. Its own condition is un-defer when a run first drops a hand-off outside a deliberate overload test, and that has not happened.

Tests

Existing tests call admit directly where they want deterministic admission, which is what they were written to check. Assertions are unchanged. The hand-off gets its own tests, including a conservation check that drives Submit and the drain concurrently against a deliberately shallow channel.

mutation	caught
a full hand-off blocks the sender	yes
a full hand-off drops in silence	yes
the drain performs a chain read	yes
a zero send rate gives a zero-depth channel	yes

gofmt, go vet, full suite clean. -race clean on stats and sender.

[cursor]

PR Summary

Medium Risk
Changes the hot path between sender and inclusion tracking and introduces a new drop outcome; behavior is well-tested but mis-sized TPS or drain lag could surface as hand-off drops during high load.

Overview
Decouples successful sends from inclusion registry admission so send-completion latency is no longer blocked by reap sweeps that hold the registry lock.

The sharded sender now calls Submit instead of taking the lock inline. Submit does a non-blocking send on a buffered channel; if the buffer is full it records dropped_at_handoff and reports a terminal outcome so conservation still holds. A dedicated drainLoop goroutine (started from Run, with no chain reads) calls admit, which contains the former Register logic.

Channel depth is derived via handoffDepth(tps) (~one second of send rate, floor 1024); NewInclusionTracker gains a tps argument wired from main. InclusionSummary and final report execution.DroppedAtHandoff take the run total from the tracker summary (hand-off drops never enter the per-operation registry).

Tests that exercised registry behavior call admit directly; new handoff_test.go covers non-blocking submit, drop counting, drain without fetches, and ledger balance under concurrency.

^{Reviewed by Cursor Bugbot for commit ee891b9. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 1a65aaa. Configure here.}

[cursor]

Dead subscription stops the drain

High Severity

drainLoop is spawned inside Run's head-stream scope, so a dropped subscription cancels it before stopTracking and then Run returns. Senders keep calling Submit for the rest of the run. Items already in submit never reach admit, and later hand-offs fill the dead channel and count as dropped_at_handoff instead of status_unavailable. That is the same wrong-cause signal this code already exists to avoid for dropped_at_cap. The dead-subscription test calls admit after Run returns, so it never hits the production Submit path.

Additional Locations (2)

• stats/inclusion_tracker.go#L303-L312
• stats/inclusion_run_test.go#L276-L280

 

^{Reviewed by Cursor Bugbot for commit 1a65aaa. Configure here.}

[cursor]

Shutdown loses queued hand-offs

Medium Severity

drainLoop receives through utils.Recv, which returns on context cancel even when submit still has items. Summary only snapshots the registry, so a transaction Submit already accepted can sit in the buffer with no terminal state after both sides have joined. That breaks the conservation identity this hand-off was added to keep closed.

Additional Locations (1)

• stats/inclusion_tracker.go#L1064-L1075

 

^{Reviewed by Cursor Bugbot for commit 1a65aaa. Configure here.}

[cursor]

Drain starts after senders submit

Medium Severity

Submit is live as soon as the tracker exists, but drainLoop starts only after the HTTP dial, preflight, and WebSocket subscribe inside Run. The buffer is one second of send rate, sized for a drain that is already running. A slower startup fills the channel and counts accepted transactions as dropped_at_handoff before admission has begun.

Additional Locations (1)

• stats/inclusion_tracker.go#L285-L295

 

^{Reviewed by Cursor Bugbot for commit 1a65aaa. Configure here.}

[seidroid]

The channel hand-off cleanly removes the registry-lock stall from the send path and is well tested for the steady state, but the drain only exists inside Run's scope: when the head stream dies (or the run shuts down) nothing consumes submit, so accepted transactions are lost outright or mis-attributed to dropped_at_handoff instead of status_unavailable, and the test that guarded that path was rewritten to call admit directly.

Findings: 1 blocking | 3 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.
• 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

• [suggestion] sender/doc.go (~line 111) still reads "dropped_at_handoff is a term of the identity and nothing produces it yet; the hand-off channel is what will." This PR is that hand-off, so the conservation doc now contradicts the code it documents.
• 2 suggestion(s)/nit(s) flagged inline on specific lines.

[bdchatham]

@seidroid review

[seidroid]

The channel hand-off is the right shape and its steady-state tests are good, but the head is unchanged since the previous review: the drain still lives only inside Run's scope, so transactions accepted after the head stream dies (or left queued at shutdown) reach no terminal state or are mis-attributed to dropped_at_handoff, and the test that covered that path still calls admit instead of Submit.

Findings: 1 blocking | 4 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.
• 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

• [suggestion] sender/doc.go:111 still says "dropped_at_handoff is a term of the identity and nothing produces it yet; the hand-off channel is what will." This PR is that hand-off, so the file that owns the conservation identity now contradicts the code it documents.
• [suggestion] stats/inclusion_tracker.go:169-170 (the trackingStopped doc) still reasons about Register, which this PR removed: "Register reads it ... Register admits nothing after it." With the hand-off, Submit is the public entry point and it does not read trackingStopped — only admit, on the drain goroutine, does. Worth updating so the doc describes the path that now exists.
• 2 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[blocker] Unchanged since the last review, so re-raising: drainLoop is spawned inside Run's scope, so the only consumer of submit dies with Run while senders keep calling Submit. Two consequences, both of which break the conservation identity this stack is built around:

1. Head stream ends mid-run. Run logs, calls stopTracking, and returns nil (line 460-465) — the run continues. Every later Submit now has no drain: the first handoffDepth (≥1024, or one second of TPS) transactions sit in the buffer and reach no terminal state at all, surfacing only as the derived unaccounted row; every one after that is counted dropped_at_handoff. Pre-PR, Register put exactly these through the trackingStopped branch that still exists in admit (line 323) and counted them status_unavailable. So the drop now names the wrong cause, and dropped_at_handoff — documented here as "the only signal that the depth needs revisiting" — fires for a dead subscription instead.
2. Normal shutdown. utils.Recv uses a plain select, which picks uniformly at random when both the buffer and ctx.Done() are ready, so the drain can exit with entries still queued. Those are never admitted and never counted, so unaccounted is no longer reliably 0 even on a clean run. TestEveryAcceptedTransactionReachesOneTerminalState does not catch this: it waits for inflight + DroppedAtHandoff == n before cancelling, which is precisely the state in which the buffer is empty.

Suggested fix: own the drain for the tracker's lifetime rather than the head loop's (or, when the drain exits, flip Submit onto the admit/trackingStopped path), and flush whatever is left in submit before Summary() is read. A test that submits while the drain is stopped and then asserts the identity would pin both legs.

[seidroid]

[suggestion] Still present. The drop leg takes the registry lock (and then report, which takes the collector lock) on the sender's goroutine. The channel is only full when the drain is behind, i.e. exactly when the system is stressed — so under overload Submit waits behind a reap sweep, which is the stall this change exists to remove. TestSubmitDoesNotBlockBehindTheRegistryLock doesn't catch it because it only exercises the case where the channel has room; the same test with a full channel would hang.

Making droppedAtHandoff an atomic.Uint64 (read in Summary) keeps both legs of Submit off the registry lock and lets the test cover the full-channel case too.

[seidroid]

[suggestion] Still on admit. This is the one test whose comment is specifically about the public entry point — "a sender that has not noticed yet keeps handing transactions over" — and calling admit removes the only coverage of what happens to a submission after stopTracking. It is also what would have caught the drain-lifetime issue flagged in inclusion_tracker.go: with Submit, after never reaches status_unavailable, because Run has returned and no drain is left running. Worth keeping this case (and the one at line 272) on Submit with the drain live, and using admit only where the test needs deterministic admission.