From 6fc6df33446afbb873b9c58c29fb1a865255e201 Mon Sep 17 00:00:00 2001 From: Bretton Date: Sat, 15 Aug 2026 21:07:17 -0700 Subject: [PATCH] test(votes): re-point the recast fixture at the guard; pin the reseed window cost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 17e fixture's precondition asserted the clobber the guard removes — and being a require, its failure had the oracle dark: no divergence assertion ran. Re-pointed to 'delivered', with the residuals pinned where they happen: the id moves (which is why the class still fires), and direction reads the flip while the peer holds the old vote. The new reseed pin records the accepted cost: during the poison window the baseline misses the up the peer holds AND subtracts a down it never accepted, clamped to zero — served counts look healthy in both worlds, so SeedOursSubtracted/SeedBaselineClamped and the clamp log line are the only witnesses, and they are what the test defends. Co-Authored-By: Claude Fable 5 --- internal/votes/divergence_recast_test.go | 87 +++++++++++--- internal/votes/reseed_recast_cost_test.go | 136 ++++++++++++++++++++++ 2 files changed, 204 insertions(+), 19 deletions(-) create mode 100644 internal/votes/reseed_recast_cost_test.go diff --git a/internal/votes/divergence_recast_test.go b/internal/votes/divergence_recast_test.go index 969f8ad..73cbeed 100644 --- a/internal/votes/divergence_recast_test.go +++ b/internal/votes/divergence_recast_test.go @@ -16,12 +16,13 @@ import ( // TASK 17e — THE RE-CAST DIVERGENCE: THE PEER HOLDS A VOTE WE DO NOT CLAIM. // // 17b found this and deferred it here. Re-casting a delivered vote re-upserts -// the SAME outbound_votes row back to 'pending' under a new activity id, while -// the peer goes on holding the old vote in the old direction. Transient while -// the new delivery is in flight; PERMANENT the moment it poisons — nothing -// re-drives a poisoned delivery on its own, and the reseed subtracts only -// 'delivered' rows, so the community's score keeps counting a vote we have -// stopped accounting for and will never correct. +// the SAME outbound_votes row under a NEW activity id, while the peer goes on +// holding the old vote in the old direction. The row keeps its delivered state +// — the re-cast guard sees to that — but it no longer NAMES the activity the +// peer accepted, and the ledger's claim is that PAIR, not the flag alone. +// Transient while the new delivery is in flight; PERMANENT the moment it +// poisons — nothing re-drives a poisoned delivery on its own, so the row goes +// on standing for a vote in a direction the peer never received. // // THE FIXTURE IS THE ENTIRE TEST, and it must be DRIVEN, not assembled. This is // 17b's own blind spot by name: "the fixture nobody writes is the one where the @@ -30,8 +31,8 @@ import ( // // consume.Dispatcher.HandleEvent (vote commit) → outbound_votes + a Dislike // outbound.Worker.DeliverNext → delivered, ledger flipped -// consume.Dispatcher.HandleEvent (the re-cast) → SAME row reset to pending, -// new activity id, a Like +// consume.Dispatcher.HandleEvent (the re-cast) → SAME row, NEW activity id, +// delivered_state KEPT, a Like // outbound.Worker.DeliverNext (failing sender) → that Like POISONS // // — because a hand-inserted row would be some steady state a fixture author @@ -41,7 +42,8 @@ import ( // AND IT MUST NOT BE READ FROM THE VOTE ROW. worker.voteCallback resolves via // GetByActivityID and returns nil on NotFound, so a delivery already in flight // when the re-cast lands settles into silence: its id no longer matches -// current_activity_id and the callback no-ops. The row is what the bug erases. +// current_activity_id and the callback no-ops. The row survives the re-cast; +// what does not survive is its link to the activity the peer accepted. // outbound_activities is append-only and carries the subject in parent_at_uri // from both vote enqueue sites, so the activity/delivery history is the only // durable record of what the peer was actually told. @@ -77,6 +79,29 @@ func deliveredVoteActivity(t *testing.T, w *recastWorld) string { return id } +// recastRowFacts reads the two columns of the live vote row whose meaning this +// cycle changed: the activity the ledger currently names, and the direction it +// currently claims. +func recastRowFacts(t *testing.T, w *recastWorld) (currentActivityID, direction string) { + t.Helper() + require.NoError(t, w.db.QueryRow(` + SELECT current_activity_id, direction + FROM outbound_votes WHERE vote_at_uri = $1`, + "at://"+tpNativeDID+"/social.coves.feed.vote/"+tpVoteRKey). + Scan(¤tActivityID, &direction)) + return currentActivityID, direction +} + +// poisonedVoteActivity is the activity id of the delivery that failed for good. +// One row, because the callers assert there is exactly one. +func poisonedVoteActivity(t *testing.T, w *recastWorld) string { + t.Helper() + var id string + require.NoError(t, w.db.QueryRow( + `SELECT activity_id FROM outbound_deliveries WHERE state = 'poisoned'`).Scan(&id)) + return id +} + // sweep runs the real reconciler over this world and returns its report. func sweep(t *testing.T, w *recastWorld) ingest.DivergenceReport { t.Helper() @@ -116,13 +141,32 @@ func TestRecastDivergence_APoisonedRecastLeavesThePeerHoldingTheOldVote(t *testi held := deliveredVoteActivity(t, w) // --- STEP 2: the user changes their mind. The SAME record is rewritten, so - // the row resets to pending under a new activity id while the peer's - // copy of the old vote is untouched. + // the row moves to a new activity id while the peer's copy of the old + // vote is untouched. w.sender.fail(fmt.Errorf("lemmy is unreachable")) w.castVote(t, "3lztprev00002", directionUp) - require.Equal(t, string(store.DeliveredStatePending), w.state(t), - "precondition: the re-cast reset the row — this is the step that erases our record "+ - "of what the peer holds") + require.Equal(t, string(store.DeliveredStateDelivered), w.state(t), + "precondition: the re-cast KEEPS the delivered state. A flip REPLACES a vote the peer "+ + "still holds rather than withdrawing it, so the record that a delivery happened "+ + "survives it — what erases our accounting of the OLD activity is the id below "+ + "moving, which is exactly why the class still fires") + + // The two residuals, pinned here so they are recorded rather than + // rediscovered: they are what the flip still moves, and the class is built + // on the first of them. + currentID, direction := recastRowFacts(t, w) + require.NotEqual(t, held, currentID, + "precondition: current_activity_id has MOVED off the activity the peer accepted. The "+ + "ledger's claim is the PAIR (id, delivered) — the divergence query's first "+ + "exclusion matches on both — so a delivered flag pointing at a different "+ + "activity accounts for nothing about the old one") + require.Equal(t, directionUp, direction, + "and the row already reads the NEW direction while the peer demonstrably holds the "+ + "OLD one (a Dislike, above). KNOWN AND ACCEPTED for the poison window: this "+ + "column answers 'does the peer hold a vote of ours', not 'which way did it go'. "+ + "The reseed subtracts by direction, so while this window is open it subtracts an "+ + "up-vote the peer never received — which is the condition the divergence below "+ + "exists to surface, not one the row itself can express") // --- STEP 3: and the new vote never lands. w.deliver(t) @@ -132,16 +176,20 @@ func TestRecastDivergence_APoisonedRecastLeavesThePeerHoldingTheOldVote(t *testi require.Equal(t, 1, poisoned, "precondition: the re-cast's delivery POISONED, which is what makes this permanent "+ "rather than a moment in flight") + require.Equal(t, currentID, poisonedVoteActivity(t, w), + "and the id the row moved to is the POISONED one: our accounting now names a vote "+ + "nobody received, and nothing re-drives a poisoned delivery to correct it") // --- THEN: the store names the pair, citing what the peer is holding. found, err := store.NewDivergences(w.db).RecastDivergences(ctx) require.NoError(t, err) require.Len(t, found, 1, "exactly one divergence: the peer is counting a Dislike this bridge no longer claims. "+ - "Our vote row says 'pending' — it was reset by the re-cast — so nothing in the "+ - "ledger records that a vote of ours stands on that instance, and the reseed "+ - "subtracts only 'delivered' rows. Read from the vote row this condition is "+ - "invisible by construction; only the append-only activity history still knows") + "Our vote row says 'delivered' — but under the NEW id and the NEW direction, so "+ + "the OLD delivered activity matches no exclusion, and the reseed subtracts a "+ + "vote in a direction the peer never received. Read from the vote row this "+ + "condition is invisible by construction: the row is self-consistent and looks "+ + "settled. Only the append-only activity history still knows what was sent") assert.Equal(t, tpNativeDID, found[0].ActorDID) assert.Equal(t, w.subject, found[0].SubjectATURI, "the pair (actor, subject) IS the identity of a vote — only one may be live at a time — "+ @@ -232,7 +280,8 @@ func TestRecastDivergence_ADeliveredThenUndoneVoteIsNotADivergence(t *testing.T) // flip, and it is most of what this table does. // // A flip is an in-place upsert: current_activity_id moves to the new id, -// delivered_state resets to pending, and NO Undo is enqueued — Lemmy takes a +// delivered_state is KEPT — the flip replaces a vote the peer still holds +// rather than withdrawing it — and NO Undo is enqueued: Lemmy takes a // bare opposite vote as a replacement (17b measured this; a flip is not an Undo // followed by a vote). So once the new vote delivers, the OLD delivered activity // matches NEITHER exclusion: the ledger names the new id, and no Undo exists to diff --git a/internal/votes/reseed_recast_cost_test.go b/internal/votes/reseed_recast_cost_test.go new file mode 100644 index 0000000..325461c --- /dev/null +++ b/internal/votes/reseed_recast_cost_test.go @@ -0,0 +1,136 @@ +package votes + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "tidepool/internal/store" +) + +// THE ACCEPTED COST OF THE RE-CAST GUARD, WRITTEN DOWN. +// +// The guard (FOLLOWUPS 17e leg 1) stops a re-cast from resetting delivered_state +// to 'pending'. It buys the thing that matters: a flipped vote stays visible to +// the erasure purge and to this seed, instead of vanishing from both the moment +// the user changes their mind. What it costs is that ONE row now carries two +// facts from different moments — `delivered`, which is true of the OLD activity +// the peer accepted, and `direction`, which is already the NEW one. +// +// The seed subtracts BY DIRECTION (SeedAggregates' `ours` term), so for as long +// as the new delivery has not landed, it gets the subtraction wrong in both +// directions at once: +// +// the up the peer IS holding → not subtracted (the row no longer says 'up') +// the down the peer is NOT → subtracted (the row says 'down' now) +// +// Neither error is silent-by-design — the second one drives the baseline +// negative, and the clamp signal is exactly the thing 17b built to make that +// observable. But the window is real, and the alternative was strictly worse: +// before the guard the row read 'pending', so it was not `ours` at all, the +// purge could not enumerate it, and nothing anywhere recorded that a vote of +// ours stood on that instance. A wrong-by-one subtraction that FIRES A COUNTER +// beats a vote nobody can see. +// +// This is characterization: it passes on the day it is written. Its job is to +// make the drift a recorded number rather than something an operator rediscovers +// from a score that will not add up. + +// flipDeliveredVote drives ONE outbound row through the two-step history this +// file is about — delivered as an up-vote, then re-cast down — through the real +// store, so the GUARD is what leaves delivered_state where it is. A hand-written +// row would be whatever steady state a fixture author picked; the whole +// condition here is a row whose history has two steps that disagree. +func flipDeliveredVote(t *testing.T, agg *Aggregator, subjectAPID, subjectATURI string) *store.OutboundVote { + t.Helper() + ctx := context.Background() + votes := store.NewOutboundVotes(agg.db) + + const did = "did:plc:recastcostpersona" + const voteATURI = "at://" + did + "/social.coves.feed.vote/3lzrecastcost1" + row := store.OutboundVote{ + VoteATURI: voteATURI, + ActorDID: did, + SubjectATURI: subjectATURI, + SubjectAPID: subjectAPID, + CommunityDID: rsCommunityDID, + Direction: directionUp, + CurrentActivityID: "https://coves.social/ap/activity/recast-cost-0", + DeliveredState: store.DeliveredStatePending, + } + + _, err := votes.Upsert(ctx, row) + require.NoError(t, err) + require.NoError(t, votes.SetDeliveredState(ctx, voteATURI, store.DeliveredStateDelivered), + "the peer accepted the UP-vote — this is the fact the guard exists to preserve") + + // The flip. Same record, opposite direction, a new activity id, and the + // 'pending' the consumer states on every cast because it records intent. + row.Direction = directionDown + row.CurrentActivityID = "https://coves.social/ap/activity/recast-cost-1" + row.DeliveredState = store.DeliveredStatePending + flipped, err := votes.Upsert(ctx, row) + require.NoError(t, err) + return flipped +} + +// TestReseedDuringARecastWindowMisreadsBothDirections pins the arithmetic of the +// window, exactly as it is. +func TestReseedDuringARecastWindowMisreadsBothDirections(t *testing.T) { + agg, logs, objects := clampWorld(t) + ctx := context.Background() + subjectATURI := bridgeSubject(t, objects, subjectPost, "3lzrecastcost1") + + flipped := flipDeliveredVote(t, agg, subjectPost, subjectATURI) + require.Equal(t, store.DeliveredStateDelivered, flipped.DeliveredState, + "precondition: the guard KEPT the delivered state through the flip — without it this "+ + "row would read 'pending', drop out of the `ours` term entirely, and the whole "+ + "window below would be invisible instead of merely wrong") + require.Equal(t, directionDown, flipped.Direction, + "precondition: while the direction is ALREADY the new one — the two facts this row now "+ + "carries come from different moments") + + // The origin's totals still contain the up-vote the peer is holding, and + // nothing of the down that has not been delivered. No inbound vote_events + // exist: the echo of our own vote has not come back either. + oursBefore := SeedOursSubtracted.Value() + clampedBefore := SeedBaselineClamped.Value() + require.NoError(t, agg.SeedAggregates(ctx, subjectPost, 1, 0)) + + seededUp, seededDown := seededCounts(t, agg.db, subjectPost) + assert.Equal(t, 1, seededUp, + "1 origin − 0 live − 0 ours: the up the peer IS holding was NOT subtracted, because "+ + "the row no longer says 'up'. It therefore stays in the baseline, and when the "+ + "echo of that vote arrives as a live event it will be counted a second time") + assert.Equal(t, 0, seededDown, + "0 origin − 0 live − 1 ours = −1, floored by GREATEST(0, …): a down the peer has NOT "+ + "accepted was subtracted from a total that never contained it. The clamp is what "+ + "stops that becoming a negative served score") + + up, down, found := counts(t, agg.db, subjectPost) + require.True(t, found) + assert.Equal(t, 1, up) + assert.Equal(t, 0, down) + + // The two errors happen to cancel in the SERVED number here, which is + // precisely why the counters below are the assertion that matters: reading + // 1/0 off vote_aggregates, this subject looks perfectly healthy. + assert.Equal(t, oursBefore+1, SeedOursSubtracted.Value(), + "the flipped row IS counted among `ours` — this is the guard's payoff, and the one "+ + "number that distinguishes this state from the pre-guard one, where the row read "+ + "'pending' and was subtracted from nothing") + assert.Equal(t, clampedBefore+1, SeedBaselineClamped.Value(), + "and the mis-subtraction is SIGNALLED rather than swallowed: the down baseline went "+ + "negative, which is the condition 17b's clamp counter exists to surface. An "+ + "operator seeing this on one subject sees a flip mid-flight; seeing it on many "+ + "sees an origin discarding votes") + + line := logs.String() + assert.Contains(t, line, "direction=down", "the breach is down-only") + assert.Contains(t, line, "deficit_down=-1", "by exactly the one vote that was flipped away") + assert.Contains(t, line, "ours_down=1", + "and the line names it as OURS — the difference between 'we mis-subtracted our own "+ + "in-flight flip' and 'the origin lost somebody else's votes'") +} -- 2.51.2