[webthethird]

Fixes #48.

follower_count and following_count drift above the real sets because the counters move on every LinkAdd/LinkRemove, while the relationship writes beside them are keyed by the (target, source) pair and so are idempotent. Any path that processes a link message twice therefore leaves the sets correct and the counts permanently wrong.

Both process_event and process_batch funnel into process_link_message, and the block backfill re-reads messages the live path may already have indexed, so a replay is reachable in normal operation. On a public node today this shows up as counts close to double the lists the same node serves — measured in #48 across three FIDs, with the lists themselves clean.

The change

relationship_exists checks the pair key, taking a write pending in the current transaction into account the same way get_count_from_txn_or_db does — Some(Some(_)) is a put, Some(None) a delete, so a relationship removed earlier in the same batch correctly reads as absent.

The counters then move only when the relationship actually changes:

• LinkAdd increments only when the pair is not already indexed
• LinkRemove decrements only when it is
• the LinkCompactState target loop increments only for pairs not already present. Targets carried over from the previous state are decremented in the loop above it and so are counted again here, which keeps the existing behaviour; the absolute following_count write at the end is untouched

No storage format or key layout changes, so this is a behaviour-only fix to how the counters are maintained.

Rebuilding existing counters

This stops the drift but does not repair counters that have already drifted. Bumping SOCIAL_GRAPH_SCHEMA_VERSION does that through the existing path — clear_all_data sweeps the whole SOCIAL_GRAPH_PREFIX range, count keys included, and the backfill re-runs. I have deliberately not bumped it here, since that forces a re-backfill on every node and is your call to time. Say the word and I will add it.

Tests

Three added. I checked each one fails with the guards removed and the tests left in place, so they pin the behaviour rather than just passing alongside it:

test	with the fix	guards removed
test_backfill_after_live_indexing_does_not_inflate	3	6
test_replayed_follow_counts_once	1	3
test_replayed_unfollow_does_not_undercount	1	0

The first is the shape of the real bug: three follows indexed through process_event, then the same messages again through process_batch. Unguarded it returns exactly double, which is what the live node shows.

The existing social graph tests, including the compact-state and backfill ones, pass unchanged with and without the fix.

Cost

The guard adds one point lookup per LinkAdd. The counters were already read — increment_* is a read-modify-write — so it is a third read alongside two, not a read where there were none. A replayed link is cheaper than before: one probe, then both counter reads and both counter writes are skipped.

Measured on this branch against the real indexer and RocksDB, release build, 200k links across 2,000 source FIDs with scattered targets, four runs each:

path	before	after	delta
new links	3.88 us/link	4.89 us/link	+1.01 us (+26%)
replayed links	3.94 us/link	3.59 us/link	−0.35 us (−9%)

Run-to-run spread was under 0.1 us on either side.

Two caveats on that figure, both of which push the real cost up rather than down. A fresh temporary database keeps 200k small keys in the memtable and L0, which is the friendliest case for a point lookup; a node with deep SST levels pays more. And no bloom filter is configured — apply_block_cache_opts sets a block cache and cache_index_and_filter_blocks but no filter policy — so the negative lookup that a genuinely new link performs is not short-circuited, and it gets more expensive as levels grow. Treat +26% as a floor for the backfill path rather than a prediction.

I did not commit the benchmark; it was an #[ignore]d test in the module, happy to add it if you would find it useful.

If that overhead is unacceptable for a full backfill, the alternative is to leave the per-message path alone and instead stop the backfill re-indexing what the live path already covered, which removes the duplicate work rather than making it harmless. This PR takes the robust-by-construction route because it holds whichever path replays, but I am glad to go the other way if you prefer.

A note on running the suite

cargo fmt --all --check is clean. I could not validate against .github/workflows/verify.yml's RUSTFLAGS: "-Dwarnings -A mismatched_lifetime_syntaxes", because that currently fails on an unmodified checkout of this commit — 17 errors, all unused imports, dead code and non-camel-case variants across worker.rs, webhook_handler.rs, user_data_store.rs, link_store.rs, storage_lend_store.rs and others, none in social_graph.rs. I verified this by stashing my change and running the same command. The Verify workflow looks like it has not actually executed in a while (the run for this commit has been queued rather than running), which would explain it going unnoticed. Happy to open a separate PR clearing those lints if useful.

So the verification here is plain cargo test, plus the targeted runs above.

Unrelated, noticed in passing: building rewrites Cargo.lock's snapchain version from 0.15.0 to 0.16.0, so the lock is stale against Cargo.toml. I kept it out of this diff.

🤖 Generated with Claude Code