Consumer deduplication guard is never invoked because ShapeLogCollector already guards against this particular case

[alco]

Problem

The deduplication guard in Shapes.Consumer.do_handle_txn/2 is dead code:

electric/packages/sync-service/lib/electric/shapes/consumer.ex

Lines 499 to 504 in e1dd7ef

	defp do_handle_txn(%Transaction{} = txn, state)
	when LogOffset.is_log_offset_lte(txn.last_log_offset, state.latest_offset) do
	Logger.debug(fn -> "Skipping already processed txn #{txn.xid}" end)
	{:cont, consider_flushed(state, txn)}
	end

Why it's dead code

All transaction fragments flow through the ShapeLogCollector before reaching any consumer. The ShapeLogCollector has its own deduplication guard (shape_log_collector.ex:471-484) that correctly compares txn_fragment.last_log_offset against last_processed_offset and drops duplicates before they are dispatched to consumers.

When a consumer process restarts, it repopulates state.latest_offset from storage (via Consumer.State.initialize/3 → Storage.fetch_latest_offset), which is guaranteed to be a transaction boundary offset by PureFileStorage. But the consumer doesn't independently re-receive fragments — the ShapeLogCollector is the sole dispatcher, and it tracks what's already been processed.

There is no code path where a duplicate transaction bypasses the ShapeLogCollector and reaches the consumer directly.

Why the guard is also incorrect

Even if the guard weren't dead code, it would fail to detect replayed transaction fragments when the last change in the fragment doesn't match the shape:

1. state.latest_offset is set to the log offset of the last matching change (line 586), not to txn.last_log_offset.
2. A transaction fragment with changes at offsets [{lsn, 0}, {lsn, 1}, {lsn, 2}] where only {lsn, 0} matches the shape would set state.latest_offset = {lsn, 0}.
3. On replay, the guard checks {lsn, 2} <= {lsn, 0} → false, so the fragment would be re-processed.

state.latest_offset can't simply be changed to store txn.last_log_offset because it's also used in handle_call({:subscribe_materializer, ...}) (line 216) as an upper bound for reading the log stream from storage. That value must correspond to an actual offset written to the log.

Suggested fix

Remove the dead code guard entirely. If consumer-level deduplication is desired in the future (e.g. as a defense-in-depth measure), it would need a separate field like latest_txn_fragment_offset that stores the last_log_offset of the most recently processed transaction fragment regardless of whether any changes matched the shape.

[alco]

Closing this as superseded by #4668 ("Skip already-processed transactions on restart").

The core premise here was that the consumer-level dedup guard is dead code — "there is no code path where a duplicate transaction bypasses the ShapeLogCollector and reaches the consumer directly" — and the recommendation was to remove it.

#4668 found the path that premise missed. On a StackSupervisor restart the ShapeLogCollector restarts too, coming back with a fresh (empty) last_processed_offset. The persistent replication slot then replays from its confirmed LSN, which can lag the shape's on-disk offset. The amnesiac collector doesn't drop that replayed transaction, so it reaches the consumer — where the storage-restored latest_offset is the only thing that can catch it. Left unchecked it re-writes duplicate ops to the shape log and re-notifies dependent subquery materializers, which re-apply and crash with Key ... already exists (bug 2 of the #4648 restart-oracle triage). So the guard isn't dead — it's the load-bearing dedup on restart.

Rather than remove it, #4668 consolidates it: a single handle_txn_fragment/2 clause (guarded by is_log_offset_lte(offset, state.latest_offset)) placed ahead of the complete-transaction fast paths, replacing the old fragment_already_processed?/2 and covering the single- and multi-fragment paths uniformly. That's the same last_log_offset vs latest_offset comparison analyzed above, now with regression tests that replay an already-applied txn straight past a fresh collector.

One caveat from this issue that #4668 does not explicitly close out: the observation that latest_offset tracks the last matching change rather than the transaction's last_log_offset, so a partial-match fragment could compare above it on replay. #4668 leans on the "fragments are entirely at-or-below or entirely above latest_offset" boundary assumption and its tests exercise the all-matching case. If the partial-match scenario is still a real concern, it's worth a fresh, focused issue against the new consolidated clause rather than keeping this (now largely obsolete) analysis open.