From 0b1c178a5003e352b78c6efe9ec78dfef5304707 Mon Sep 17 00:00:00 2001 From: Bretton Date: Fri, 18 Sep 2026 19:21:09 -0700 Subject: [PATCH] feat(discover): boost hosted-community posts with a keyed deterministic bonus Fresh zero-vote posts in Coves-hosted communities ranked below much older bridged posts carrying hundreds of imported votes. A flat boost was rejected because it would stack every fresh hosted post at the top. Discover Hot now adds a per-post numerator bonus of 3.0 x u for posts in hosted communities. u comes from HMAC-SHA256 keyed with the existing CURSOR_SECRET over a domain label plus the post URI, so it is stable across snapshot rebuilds for a post's lifetime and cannot be ground offline by an author choosing rkeys (postv2 URIs are author-chosen). 3.0 was chosen from a read-only production simulation (2026-09-19T00:41Z) recorded in the plan. Changes: - discover: HostedCommunityBonus(secret, uri); DiscoverHotRank takes a trailing hostedBonus, applied only when score >= 0, clamped to 3.0, NaN/Inf/negative treated as 0; bonus 0 is bit-identical to the old rank - postgres: the candidate query selects communities.pds_refresh_token_encrypted IS NOT NULL (presence only, never decrypted, hosted_by_did ignored); the snapshot build passes the bonus keyed with the repository's cursor secret for hosted candidates - discoverHotAlgorithmVersion 1 -> 2: version 1 snapshots are not reused and their cursors return InvalidCursor - tests: keyed reference vectors computed independently, proof the unkeyed digest no longer passes, frozen pre-change ranks, clamp/negative/ monotonicity, the 15h-vs-48h scenario pin, the deliberate downvote cutoff pin; T1 persisted base_rank across snapshots for every hosted-predicate case and across two secrets, version supersession with controls, HTTP acceptance ordering - docs: DISCOVER_HOT_RANKING_PLAN.md amended (no longer source-neutral) with section 3.5, the simulation table, and the post-deploy stored-candidate headroom note The first downvote on a hosted post removes the whole bonus; this hard cutoff is deliberate. Rotating CURSOR_SECRET reshuffles bonuses. Timeline and community Hot and Discover New/Top are unchanged. No new configuration. Reviewed with /second-opinion mini (codex gpt-6-astra + fable); make ci passed on the branch tip. Co-Authored-By: Claude Fable 5.1 --- docs/DISCOVER_HOT_RANKING_PLAN.md | 38 +- .../core/discover/hot_hosted_bonus_test.go | 80 ++++ .../discover/hot_hosted_bonus_unit_test.go | 306 ++++++++++++++ internal/core/discover/hot_ranking.go | 47 ++- internal/core/discover/hot_ranking_test.go | 18 +- .../discover_hot_algorithm_version_test.go | 230 +++++++++++ .../discover_hot_hosted_bonus_test.go | 380 ++++++++++++++++++ .../discover_hot_rank_sql_equivalence_test.go | 2 +- internal/db/postgres/discover_hot_repo.go | 20 +- 9 files changed, 1097 insertions(+), 24 deletions(-) create mode 100644 internal/core/discover/hot_hosted_bonus_test.go create mode 100644 internal/core/discover/hot_hosted_bonus_unit_test.go create mode 100644 internal/db/postgres/discover_hot_algorithm_version_test.go create mode 100644 internal/db/postgres/discover_hot_hosted_bonus_test.go diff --git a/docs/DISCOVER_HOT_RANKING_PLAN.md b/docs/DISCOVER_HOT_RANKING_PLAN.md index a3ea90f..a3d734c 100644 --- a/docs/DISCOVER_HOT_RANKING_PLAN.md +++ b/docs/DISCOVER_HOT_RANKING_PLAN.md @@ -14,7 +14,9 @@ The user also selected **stable snapshot pagination**: scrolling preserves a ran Apply this to **Discover Hot only**, including requests that default to Hot. Timeline Hot, community Hot, and Discover New/Top retain their existing ranking behavior. Do not change the shared Hot formula to implement Discover's new policy. -Native-specific slots, native-community multipliers, subscriber weighting, author penalties, and an additional age taper are outside this version. Do not infer human authorship from community hosting or imported vote counts. This ranking is source-neutral. +Native-specific slots, native-community multipliers, subscriber weighting, author penalties, and an additional age taper are outside this version. Do not infer human authorship from community hosting or imported vote counts. + +**Amended 2026-09-18 (algorithm version 2):** the ranking is no longer source-neutral. Posts in Coves-hosted communities receive the deterministic numerator bonus specified in §3.5. Every other statement in this section stands. ### Intended outcome @@ -123,6 +125,40 @@ Choose the highest adjusted rank, with `created_at DESC, uri DESC` as determinis The sign-safe branches ensure repetition never improves a negative rank. The adjustment preserves base-rank order within a community. Therefore only the best remaining candidate from each community can win the next selection. +### 3.5 Hosted-community bonus (algorithm version 2) + +Approved 2026-09-18. Bridged communities import vote counts in the hundreds while posts in Coves-hosted communities sit at zero or one vote, so fresh hosted posts ranked below much older bridged posts. A flat boost was rejected because it would stack every fresh hosted post at the top of the feed. + +```text +hosted = communities.pds_refresh_token_encrypted IS NOT NULL +mac = HMAC-SHA256(key = CURSOR_SECRET, message = "discover-hot-hosted-bonus\0" + post URI) +u = (first 8 bytes of mac, big-endian) >> 11, divided by 2^53 # [0, 1) +bonus = 3.0 × u when hosted and score >= 0 +bonus = 0 otherwise +numerator = (numerator from §3.3) + bonus +``` + +- `hosted` is the same fact as `internal/db/postgres/community_hosted.go`: the presence of the stored credential. It is never decrypted, and `hosted_by_did` is a claim anyone can write, so it is not used. +- The bonus depends only on the post URI and the server's `CURSOR_SECRET`. It has no clock and no per-process randomness, so it is identical across the 30-second snapshot rebuilds for the post's lifetime and the feed does not reshuffle on refresh. +- It is keyed because the author chooses the URI: a `postv2` record lives at `at:///social.coves.community.postv2/`. An unkeyed digest could be ground offline, about 1,000 rkeys for a near-maximum bonus on every post. The label prefix separates the bonus from cursor signatures, which use the same key. Rotating `CURSOR_SECRET` reshuffles every bonus; rotation already invalidates every cursor. +- It is additive and independent of score, so an upvote never lowers a post's rank. Negative-score posts receive no bonus. This is a deliberate hard cutoff: the first downvote on a zero-vote hosted post removes the whole bonus, taking the numerator from as much as 4.0 to `1 - ln 2` (about 13x), against about 3.3x for a non-hosted post. Hosted posts sit at 0-1 votes, so one account can do this. A taper over the first few negative scores is the alternative if it is abused. +- A post with `u` near zero ranks where it did under version 1. Freshness decay, partial normalization and the repetition penalty are unchanged, and the repetition penalty remains the guard against one hosted community filling a page. +- With a bonus of zero the Go rank still equals the legacy `hotRankSQL` expression at neutral normalization. +- The algorithm version moved from 1 to 2. Version 1 snapshots are not reused and their cursors return `InvalidCursor`, which clients already recover from (§7.2). Version 1 candidate rows keep counting against the stored-candidate limit until they expire, so stored-candidate headroom is reduced for up to one snapshot lifetime (one hour) after the deploy. +- Eligibility is every post in a hosted community, aggregator-authored or human. Revisit this as native human posting grows. + +Read-only production simulation (run with the unkeyed digest; the keyed value has the same uniform distribution), snapshot `2026-09-19T00:41Z`: 1,217 publicly visible posts from the preceding 14 days. Posts under 24 hours old: 44 hosted (all aggregator-authored, scores 0-1, 5 communities) and 59 bridged. + +| Maximum bonus | Hosted posts in first 20 / 50 / 100 | Longest hosted run in first 100 | Posts older than 40h ranked above a hosted post younger than 18h | +|---:|---:|---:|---:| +| 0 (version 1) | 1 / 2 / 19 | 4 | 21 | +| 1.5 | 1 / 7 / 30 | 2 | 5 | +| 2.0 | 1 / 7 / 33 | 3 | 4 | +| **3.0** | **2 / 11 / 36** | **4** | **1** | +| 4.0 | 3 / 13 / 38 | 3 | 0 | + +`3.0` was chosen: hosted posts reach 22% of the first 50 against 43% of fresh supply, without lengthening the longest hosted run. This is one snapshot, and aggregator posts arrive in batches, so the mix varies by time of day. These are dated observations, not test expectations. + ## 4. Candidate selection and hydration Build ordered candidate streams per community from the complete snapshot-eligible metadata. Compare their leading candidates and refill the winning stream after selection. A global top-50 or fixed per-community cap must not hide an alternative or terminate pagination early. diff --git a/internal/core/discover/hot_hosted_bonus_test.go b/internal/core/discover/hot_hosted_bonus_test.go new file mode 100644 index 0000000..6387be3 --- /dev/null +++ b/internal/core/discover/hot_hosted_bonus_test.go @@ -0,0 +1,80 @@ +//go:build integration + +package discover_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + discoverhandler "Coves/internal/api/handlers/discover" + "Coves/internal/core/discover" + "Coves/internal/db/postgres" + "Coves/tests/fixtures" + "Coves/tests/testkit" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetDiscover_HotSort_HostedCommunityPostOutranksTiedNonHostedPost(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + + ctx := context.Background() + testID := testkit.UniqueID(t) + + // The two community names differ only at the character after "hosted", and + // fixtures.Post still writes the LEGACY at:///social.coves.community.post/ + // shape — a live postv2 record is at:///social.coves.community.postv2/ + // — so here the community name, not the generated rkey, decides the URI + // ordering. "hostedb" > "hosteda" makes the NON-hosted post the winner of + // today's tie-breakers, which is the precondition this test asserts below. + hostedDID, err := fixtures.Community(ctx, db, "hosteda"+testID, "hosteda"+testID+".test") + require.NoError(t, err) + nonHostedDID, err := fixtures.Community(ctx, db, "hostedb"+testID, "hostedb"+testID+".test") + require.NoError(t, err) + + // Presence of a stored credential is the hosted fact; nothing decrypts it, + // and no fixture helper writes it. hosted_by_did is not the fact — every + // firehose-indexed community carries that field, including this test's + // non-hosted one. + _, err = db.ExecContext(ctx, + `UPDATE communities SET pds_refresh_token_encrypted = $2 WHERE did = $1`, + hostedDID, []byte("a stored refresh token")) + require.NoError(t, err, "granting %s credentials", hostedDID) + + createdAt := time.Now().Add(-4 * time.Hour) + hostedURI := fixtures.Post(t, db, hostedDID, "did:plc:hosted-bonus-author", + "Hosted community post", 0, createdAt) + nonHostedURI := fixtures.Post(t, db, nonHostedDID, "did:plc:hosted-bonus-author", + "Non-hosted community post", 0, createdAt) + + require.Greater(t, nonHostedURI, hostedURI, + "precondition: the non-hosted post must win the uri DESC tie-breaker, or this test could pass for the wrong reason") + + repository := postgres.NewDiscoverRepository(db, "hosted-bonus-cursor-secret") + service := discover.NewDiscoverService(repository) + handler := discoverhandler.NewGetDiscoverHandler(service, nil, nil) + + req := httptest.NewRequest(http.MethodGet, "/xrpc/social.coves.feed.getDiscover?sort=hot&limit=10", nil) + rec := httptest.NewRecorder() + handler.HandleGetDiscover(rec, req) + require.Equal(t, http.StatusOK, rec.Code, "Discover Hot response: %s", rec.Body.String()) + + var response discover.DiscoverResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &response)) + + uris := make([]string, len(response.Feed)) + for i, item := range response.Feed { + uris[i] = item.Post.URI + } + require.ElementsMatch(t, []string{hostedURI, nonHostedURI}, uris, + "both posts must be candidates: the bonus reorders the feed, it does not filter it") + + assert.Equal(t, []string{hostedURI, nonHostedURI}, uris, + "a Coves-hosted community's post must outrank an otherwise identically ranked post that wins the created_at/uri tie-breakers") +} diff --git a/internal/core/discover/hot_hosted_bonus_unit_test.go b/internal/core/discover/hot_hosted_bonus_unit_test.go new file mode 100644 index 0000000..285bbd7 --- /dev/null +++ b/internal/core/discover/hot_hosted_bonus_unit_test.go @@ -0,0 +1,306 @@ +package discover + +import ( + "crypto/sha256" + "encoding/binary" + "fmt" + "math" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The secrets these vectors were computed under. They stand for the +// repository's cursor secret, which is the key the bonus is derived with. +const ( + firstHostedBonusSecret = "discover-cursor-secret-one" + secondHostedBonusSecret = "discover-cursor-secret-two" +) + +// A post URI is at:///social.coves.community.postv2/, and the +// rkey is chosen by the author. That is the whole reason the bonus is keyed: an +// unkeyed digest of a string the author controls end to end can be ground +// offline until it hashes near the 3.0 ceiling, and every post that author +// writes then enters Discover at the top. +// +// The expected values were computed outside Go, from the formula rather than +// from this package's output: +// +// python3 -c "import hmac,hashlib,struct; mac=hmac.new(secret.encode(), b'discover-hot-hosted-bonus\x00'+uri.encode(), hashlib.sha256).digest(); n=struct.unpack('>Q',mac[:8])[0]; print(3.0*(n>>11)/float(2**53))" +// +// Pinning them as literals is what makes the bonus stable for a post's +// lifetime. A value derived from a seed, a clock or a process-local map would +// reproduce inside one run and silently reshuffle every ranked feed on the next +// deploy, so a test that only compared the function to itself would not notice. +var hostedCommunityBonusVectors = []struct { + secret string + uri string + bonus float64 +}{ + {firstHostedBonusSecret, "at://did:plc:ewvi7nxzyoun6zhxrhs64oiz/social.coves.community.postv2/3lbf2qk7xy22a", 1.8743999374955136}, + {secondHostedBonusSecret, "at://did:plc:ewvi7nxzyoun6zhxrhs64oiz/social.coves.community.postv2/3lbf2qk7xy22b", 1.908845968980569}, + {firstHostedBonusSecret, "at://did:plc:z72i7hdynmk6r22z27h6tvur/social.coves.community.postv2/3juvq6mrxyz2k", 0.8659818296073254}, + {secondHostedBonusSecret, "at://did:plc:vpkhqolt662uhesyj6nxm7ys/social.coves.community.postv2/3m5tj4pumyc77", 0.965717612953847}, + {firstHostedBonusSecret, "at://did:plc:44ybard66vv44zksje25o7dz/social.coves.community.postv2/3kzzzzzzzzzz2", 1.7494194956022269}, + {secondHostedBonusSecret, "at://did:plc:hdhoaan3xa3jiuq4fg4mefid/social.coves.community.postv2/3mvtj4pumycdd", 2.7601419001651606}, +} + +func TestHostedCommunityBonusReferenceVectors(t *testing.T) { + t.Parallel() + + for _, vector := range hostedCommunityBonusVectors { + assert.InDeltaf(t, vector.bonus, HostedCommunityBonus(vector.secret, vector.uri), 1e-15, + "bonus for %s under its server secret must be the value the formula fixes for that post forever", vector.uri) + } +} + +// unkeyedHostedCommunityBonus is the retired formula: a bare SHA-256 of the +// URI, with no server secret anywhere in it. +// +// It is restated here to be excluded. The keyed and the unkeyed value are both +// deterministic and both spread over [0, 3.0), so an implementation that +// accepted the secret and then ignored it would satisfy every other property in +// this file. Naming the old value and requiring the new one to differ from it +// is what rules that out. +func unkeyedHostedCommunityBonus(uri string) float64 { + digest := sha256.Sum256([]byte(uri)) + return 3.0 * float64(binary.BigEndian.Uint64(digest[:8])>>11) / (1 << 53) +} + +func TestHostedCommunityBonusIsNotTheUnkeyedDigest(t *testing.T) { + t.Parallel() + + for _, vector := range hostedCommunityBonusVectors { + assert.Greaterf(t, + math.Abs(HostedCommunityBonus(vector.secret, vector.uri)-unkeyedHostedCommunityBonus(vector.uri)), + 0.1, "bonus for %s must not be the unkeyed SHA-256 of the URI, which an author can grind offline", vector.uri) + } +} + +func TestHostedCommunityBonusVariesWithTheServerSecret(t *testing.T) { + t.Parallel() + + const uri = "at://did:plc:ewvi7nxzyoun6zhxrhs64oiz/social.coves.community.postv2/3lbf2qk7xy22a" + assert.Greater(t, + math.Abs(HostedCommunityBonus(firstHostedBonusSecret, uri)-HostedCommunityBonus(secondHostedBonusSecret, uri)), + 0.1, "one URI must hash to different bonuses under different server secrets: the secret is what the author cannot grind against") +} + +func TestHostedCommunityBonusSpread(t *testing.T) { + t.Parallel() + + const uriCount = 1000 + values := make([]float64, uriCount) + minimum, maximum, sum := 3.0, 0.0, 0.0 + for i := range values { + // Only the rkey varies: two posts from one author must not share a bonus. + uri := fmt.Sprintf("at://did:plc:ewvi7nxzyoun6zhxrhs64oiz/social.coves.community.postv2/3lbf2qk7xy%03d", i) + value := HostedCommunityBonus(firstHostedBonusSecret, uri) + require.GreaterOrEqualf(t, value, 0.0, "bonus for %s is negative", uri) + require.Lessf(t, value, 3.0, "bonus for %s is at or over the 3.0 ceiling", uri) + + values[i] = value + minimum = min(minimum, value) + maximum = max(maximum, value) + sum += value + } + + // A bonus that clustered would be indistinguishable from a constant: every + // hosted post would move by the same amount and the tie-breakers, not the + // bonus, would still decide the order. + assert.Lessf(t, minimum, 0.3, "the smallest of %d bonuses should reach the bottom of the range", uriCount) + assert.Greaterf(t, maximum, 2.7, "the largest of %d bonuses should reach the top of the range", uriCount) + assert.InDelta(t, 1.5, sum/float64(uriCount), 0.2, "the bonus should be spread evenly across [0, 3.0)") +} + +func TestHostedCommunityBonusIsStableAcrossCalls(t *testing.T) { + t.Parallel() + + const uri = "at://did:plc:ewvi7nxzyoun6zhxrhs64oiz/social.coves.community.postv2/3lbf2qk7xy22a" + assert.Equal(t, + HostedCommunityBonus(firstHostedBonusSecret, uri), + HostedCommunityBonus(firstHostedBonusSecret, uri), + "one secret and one URI must produce the identical value within a process, not merely a close one") +} + +// frozenRankingTime and the frozen table below are the pre-bonus ranking +// function's own output, captured from this package before the parameter +// existed and printed with %.17g. They are compared with == rather than a +// delta: a bonus of zero must leave every existing rank bit-for-bit identical, +// because a rank that drifted even in the last digit would reshuffle the ties +// that decide feed order. +var frozenRankingTime = time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC) + +var frozenDiscoverHotRanks = []struct { + name string + score int + age time.Duration + adjustment float64 + rank float64 +}{ + {"fresh zero score", 0, 0, 1, 0.35355339059327379}, + {"half-day-old zero score", 0, 15 * time.Hour, 1, 0.014266801472725471}, + {"small positive score", 5, 3 * time.Hour, 1, 0.24970255800090652}, + {"popular two-day-old post", 400, 48 * time.Hour, 0.8, 0.016391213593287227}, + {"very popular three-day-old post", 1000, 72 * time.Hour, 0.5, 0.0069974439678588476}, + {"single upvote, minutes old", 1, 30 * time.Minute, 0.8, 0.39326533884824877}, + {"lightly downvoted", -3, 6 * time.Hour, 1, -0.017071960142624978}, + {"heavily downvoted", -250, 36 * time.Hour, 0.5, -0.01931908965911432}, + {"future-dated positive score", 50, -5 * time.Hour, 1, 1.7436636742645033}, + {"future-dated zero score", 0, -90 * time.Minute, 0.5, 0.35355339059327379}, + {"fractional age", 7, 9*time.Hour + 45*time.Minute, 0.5, 0.050642357939854221}, +} + +func TestDiscoverHotRankWithoutBonusIsUnchanged(t *testing.T) { + t.Parallel() + + for _, frozen := range frozenDiscoverHotRanks { + createdAt := frozenRankingTime.Add(-frozen.age) + assert.Equalf(t, frozen.rank, + discoverHotRank(frozen.score, createdAt, frozenRankingTime, frozen.adjustment, 0), + "%s: a zero bonus must reproduce the pre-bonus rank exactly", frozen.name) + } +} + +// expectedRankWithBonus restates the contract rather than calling the code +// under test: numerator, then bonus for non-negative scores only, over the +// (age + 2)^1.5 decay. +func expectedRankWithBonus(score int, ageHours, adjustment, bonus float64) float64 { + numerator := 1.0 + switch { + case score > 0: + numerator += adjustment * math.Log1p(float64(score)) + case score < 0: + numerator -= math.Log1p(math.Abs(float64(score))) + } + if score >= 0 { + numerator += bonus + } + return numerator / math.Pow(ageHours+2, 1.5) +} + +func TestDiscoverHotRankAddsBonusToNumerator(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + score int + age time.Duration + ageHours float64 + adjustment float64 + bonus float64 + }{ + {"zero score, small bonus", 0, 15 * time.Hour, 15, 1, 0.125}, + {"zero score, near-maximum bonus", 0, 3 * time.Hour, 3, 0.5, 2.9999}, + {"positive score, mid bonus", 400, 48 * time.Hour, 48, 0.8, 1.5}, + {"positive score, fractional age", 7, 9*time.Hour + 45*time.Minute, 9.75, 0.5, 2.25}, + // Age is clamped at zero, so a future post ranks as if brand new and + // the bonus still applies. + {"future-dated positive score", 50, -5 * time.Hour, 0, 1, 1.0}, + } + + for _, testCase := range cases { + createdAt := frozenRankingTime.Add(-testCase.age) + assert.InDeltaf(t, + expectedRankWithBonus(testCase.score, testCase.ageHours, testCase.adjustment, testCase.bonus), + discoverHotRank(testCase.score, createdAt, frozenRankingTime, testCase.adjustment, testCase.bonus), + 1e-12, "%s: the bonus belongs in the numerator, under the same decay", testCase.name) + } +} + +// A downvoted post must not be able to buy its way back up the feed by being +// hosted here: the bonus is a tie-breaker among posts the community has not +// rejected, not a floor under the ones it has. +func TestDiscoverHotRankIgnoresBonusForNegativeScores(t *testing.T) { + t.Parallel() + + for _, frozen := range frozenDiscoverHotRanks { + if frozen.score >= 0 { + continue + } + createdAt := frozenRankingTime.Add(-frozen.age) + for _, bonus := range []float64{0.125, 1.5, 3.0} { + assert.Equalf(t, frozen.rank, + discoverHotRank(frozen.score, createdAt, frozenRankingTime, frozen.adjustment, bonus), + "%s: bonus %g must not lift a negative-score post", frozen.name, bonus) + } + } +} + +// The cliff at the first downvote, pinned as a ratio rather than described in +// prose: one downvote removes the WHOLE bonus, dropping a ceiling-bonus post's +// numerator from 4.0 to 1 - ln 2 — a thirteenfold fall — and that is an +// approved product decision, not an accident of the formula. +func TestDiscoverHotRankFirstDownvoteRemovesTheEntireBonus(t *testing.T) { + t.Parallel() + + const ageHours = 15.0 + createdAt := frozenRankingTime.Add(-ageHours * time.Hour) + decay := math.Pow(ageHours+2, 1.5) + + atZeroScore := discoverHotRank(0, createdAt, frozenRankingTime, 1, 3.0) + assert.InDelta(t, 4.0/decay, atZeroScore, 1e-15, + "an unvoted post at the bonus ceiling carries numerator 1 + 3.0") + + atOneDownvote := discoverHotRank(-1, createdAt, frozenRankingTime, 1, 3.0) + assert.Equal(t, discoverHotRank(-1, createdAt, frozenRankingTime, 1, 0), atOneDownvote, + "a single downvote must leave the rank exactly where the bonus-free formula puts it") + assert.InDelta(t, (1-math.Ln2)/decay, atOneDownvote, 1e-15, + "the score -1 numerator is 1 - ln 2, with no part of the bonus surviving") + + assert.InDelta(t, 4.0/(1-math.Ln2), atZeroScore/atOneDownvote, 1e-9, + "the drop across the first downvote is the full bonus, not a fraction of it") +} + +func TestDiscoverHotRankRejectsUnusableBonus(t *testing.T) { + t.Parallel() + + createdAt := frozenRankingTime.Add(-15 * time.Hour) + rankWith := func(bonus float64) float64 { + return discoverHotRank(0, createdAt, frozenRankingTime, 1, bonus) + } + + // A NaN or infinite bonus would poison the rank, and the snapshot builder + // rejects a nonfinite rank outright — one bad value would fail the whole + // build rather than one post. A negative bonus is not a bonus. + for _, bonus := range []float64{math.NaN(), math.Inf(1), math.Inf(-1), -0.5, -3.0} { + assert.Equalf(t, rankWith(0), rankWith(bonus), + "bonus %g must be treated as no bonus at all", bonus) + } + + for _, bonus := range []float64{3.0000001, 10, 1e9} { + assert.Equalf(t, rankWith(3.0), rankWith(bonus), + "bonus %g must clamp to the 3.0 ceiling", bonus) + } +} + +func TestDiscoverHotRankStaysStrictlyIncreasingInScore(t *testing.T) { + t.Parallel() + + createdAt := frozenRankingTime.Add(-4 * time.Hour) + previous := discoverHotRank(0, createdAt, frozenRankingTime, 0.5, 1.7) + for score := 1; score <= 50; score++ { + current := discoverHotRank(score, createdAt, frozenRankingTime, 0.5, 1.7) + assert.Greaterf(t, current, previous, + "score %d must rank above score %d at the same bonus: a bonus that flattened engagement would make votes stop mattering", score, score-1) + previous = current + } +} + +// The scenario the bonus exists for, and the one it must not exceed: a fresh +// post with no votes in a community hosted here beats a day-old popular post +// elsewhere, and does not beat it without the bonus. +func TestDiscoverHotRankHostedFreshPostOutranksOlderPopularPost(t *testing.T) { + t.Parallel() + + freshCreatedAt := frozenRankingTime.Add(-15 * time.Hour) + popularCreatedAt := frozenRankingTime.Add(-48 * time.Hour) + popular := discoverHotRank(400, popularCreatedAt, frozenRankingTime, 0.8, 0) + + assert.Greater(t, discoverHotRank(0, freshCreatedAt, frozenRankingTime, 1, 1.0), popular, + "a 15h-old hosted post with a 1.0 bonus should outrank a 48h-old post with 400 votes") + assert.Less(t, discoverHotRank(0, freshCreatedAt, frozenRankingTime, 1, 0), popular, + "without the bonus the same post must lose: the bonus is doing the work, not the decay") +} diff --git a/internal/core/discover/hot_ranking.go b/internal/core/discover/hot_ranking.go index c614281..8fde24d 100644 --- a/internal/core/discover/hot_ranking.go +++ b/internal/core/discover/hot_ranking.go @@ -1,17 +1,23 @@ package discover import ( + "crypto/hmac" + "crypto/sha256" + "encoding/binary" "math" "sort" "time" ) const ( - minimumReferencePosts = 5 - baselinePriorPosts = 20 - minimumAdjustment = 0.5 - recentCommunityWindow = 20 - repetitionPenaltyStep = 0.5 + minimumReferencePosts = 5 + baselinePriorPosts = 20 + minimumAdjustment = 0.5 + recentCommunityWindow = 20 + repetitionPenaltyStep = 0.5 + maximumHostedCommunityBonus = 3.0 + + hostedCommunityBonusDomainLabel = "discover-hot-hosted-bonus\x00" ) func communityReference(scoresByCommunity map[string][]int) (float64, bool) { @@ -60,7 +66,7 @@ func CommunityAdjustment(scores []int, reference float64, referenceAvailable boo return communityAdjustment(scores, reference, referenceAvailable) } -func discoverHotRank(score int, createdAt, rankingTime time.Time, communityAdjustment float64) float64 { +func discoverHotRank(score int, createdAt, rankingTime time.Time, communityAdjustment, hostedBonus float64) float64 { ageHours := rankingTime.Sub(createdAt).Hours() if ageHours < 0 { ageHours = 0 @@ -78,12 +84,35 @@ func discoverHotRank(score int, createdAt, rankingTime time.Time, communityAdjus numerator -= math.Log1p(math.Abs(float64(score))) } + if score >= 0 { + if math.IsNaN(hostedBonus) || math.IsInf(hostedBonus, 0) || hostedBonus < 0 { + hostedBonus = 0 + } + numerator += min(hostedBonus, maximumHostedCommunityBonus) + } + return numerator / math.Pow(ageHours+2, 1.5) } -// DiscoverHotRank returns the normalized, freshness-decayed base rank. -func DiscoverHotRank(score int, createdAt, rankingTime time.Time, communityAdjustment float64) float64 { - return discoverHotRank(score, createdAt, rankingTime, communityAdjustment) +// DiscoverHotRank returns the normalized, freshness-decayed base rank, including +// the hosted-community bonus for posts that are not negatively scored. +func DiscoverHotRank(score int, createdAt, rankingTime time.Time, communityAdjustment, hostedBonus float64) float64 { + return discoverHotRank(score, createdAt, rankingTime, communityAdjustment, hostedBonus) +} + +// HostedCommunityBonus returns the deterministic numerator bonus for a post in +// a Coves-hosted community, spread over [0, maximumHostedCommunityBonus). +// +// It is keyed with the server's cursor secret because the post author chooses +// the URI's rkey: an unkeyed digest could be ground offline until it landed at +// the ceiling. The label separates this domain from cursor signatures, which +// use the same key. +func HostedCommunityBonus(secret, uri string) float64 { + mac := hmac.New(sha256.New, []byte(secret)) + mac.Write([]byte(hostedCommunityBonusDomainLabel)) + mac.Write([]byte(uri)) + digest := mac.Sum(nil) + return maximumHostedCommunityBonus * float64(binary.BigEndian.Uint64(digest[:8])>>11) / (1 << 53) } func engagementSum(scores []int) float64 { diff --git a/internal/core/discover/hot_ranking_test.go b/internal/core/discover/hot_ranking_test.go index c523865..6dc2d0f 100644 --- a/internal/core/discover/hot_ranking_test.go +++ b/internal/core/discover/hot_ranking_test.go @@ -79,24 +79,24 @@ func TestDiscoverHotNormalization(t *testing.T) { twoHoursOld := rankingTime.Add(-2 * time.Hour) denominator := math.Pow(4, 1.5) - positive := discoverHotRank(15, twoHoursOld, rankingTime, 0.5) - zero := discoverHotRank(0, twoHoursOld, rankingTime, 0.5) - negative := discoverHotRank(-3, twoHoursOld, rankingTime, 0.5) + positive := discoverHotRank(15, twoHoursOld, rankingTime, 0.5, 0) + zero := discoverHotRank(0, twoHoursOld, rankingTime, 0.5, 0) + negative := discoverHotRank(-3, twoHoursOld, rankingTime, 0.5, 0) assert.InDelta(t, (1+0.5*math.Log(16))/denominator, positive, 1e-12) assert.InDelta(t, 1/denominator, zero, 1e-12) assert.InDelta(t, (1-math.Log(4))/denominator, negative, 1e-12) - assert.Greater(t, discoverHotRank(255, twoHoursOld, rankingTime, 0.5), positive) + assert.Greater(t, discoverHotRank(255, twoHoursOld, rankingTime, 0.5, 0), positive) assert.Greater(t, positive, zero) assert.Negative(t, negative) - assert.Less(t, discoverHotRank(-15, twoHoursOld, rankingTime, 0.5), negative) - assert.Equal(t, zero, discoverHotRank(0, twoHoursOld, rankingTime, 1), + assert.Less(t, discoverHotRank(-15, twoHoursOld, rankingTime, 0.5, 0), negative) + assert.Equal(t, zero, discoverHotRank(0, twoHoursOld, rankingTime, 1, 0), "normalization must not change zero-score rank") - assert.Equal(t, negative, discoverHotRank(-3, twoHoursOld, rankingTime, 1), + assert.Equal(t, negative, discoverHotRank(-3, twoHoursOld, rankingTime, 1, 0), "normalization must not change negative-score rank") - fresh := discoverHotRank(0, rankingTime, rankingTime, 1) - future := discoverHotRank(0, rankingTime.Add(48*time.Hour), rankingTime, 1) + fresh := discoverHotRank(0, rankingTime, rankingTime, 1, 0) + future := discoverHotRank(0, rankingTime.Add(48*time.Hour), rankingTime, 1, 0) assert.InDelta(t, 1/math.Pow(2, 1.5), fresh, 1e-12) assert.Equal(t, fresh, future, "future dates should clamp to zero age without gaining a boost") }) diff --git a/internal/db/postgres/discover_hot_algorithm_version_test.go b/internal/db/postgres/discover_hot_algorithm_version_test.go new file mode 100644 index 0000000..8ad9858 --- /dev/null +++ b/internal/db/postgres/discover_hot_algorithm_version_test.go @@ -0,0 +1,230 @@ +//go:build integration + +package postgres + +import ( + "context" + "database/sql" + "slices" + "testing" + + "Coves/internal/core/discover" + "Coves/tests/testkit" + + "github.com/lib/pq" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The algorithm version as a compatibility boundary. +// +// A snapshot is a frozen page order, and the cursor a reader holds is a pointer +// into one. The hosted-community bonus changes what base_rank MEANS, so a +// snapshot built before it and one built after it are two different orderings +// wearing the same name: a reader paging through the old one would see the old +// ranking silently, and a reader whose next page lands on a rebuilt snapshot +// would see posts repeat or vanish. algorithm_version is the field that keeps +// those two apart, and bumping it is the whole mechanism — it makes stale +// snapshots unreusable and stale cursors invalid in one move. +// +// Every assertion below is written against the LITERAL 1 and 2 rather than the +// discoverHotAlgorithmVersion constant. A test that asserts a row carries the +// constant the writer used is a tautology: it passes at version 1, at version 2 +// and at any version anyone ever sets, so it cannot notice the bump failing to +// happen. The literal is the point. +const ( + supersededDiscoverHotAlgorithmVersion = 1 + currentDiscoverHotAlgorithmVersion = 2 +) + +func TestDiscoverRepo_HotNewSnapshotsRecordAlgorithmVersionTwo(t *testing.T) { + t.Parallel() + + db := testkit.DB(t) + expected := seedDiscoverHotSnapshotCandidates(t, db, "algorithm-version-build", []int{400, 300, 200}) + + feed, _ := readDiscoverHotRoot(t, NewDiscoverRepository(db, "algorithm-version-build-secret"), "", 10) + require.Equal(t, expected, feed) + + versions := discoverHotSnapshotVersions(t, db, "") + require.Len(t, versions, 1, "the read must have built exactly one anonymous snapshot") + assert.Equal(t, currentDiscoverHotAlgorithmVersion, versions[0], + "a snapshot built under the hosted-bonus ranking must be stamped version 2") +} + +func TestDiscoverRepo_HotRefusesSupersededAlgorithmVersionSnapshot(t *testing.T) { + t.Parallel() + + db := testkit.DB(t) + expected := seedDiscoverHotSnapshotCandidates(t, db, "algorithm-version-reuse", []int{400, 300, 200, 100}) + // The seeded snapshots rank the same real posts in the OPPOSITE order to the + // one a fresh build produces. Reuse is then visible in the feed itself: a + // reused snapshot serves its own stored order, and a rebuilt one serves the + // ranking. Asserting on snapshot ids alone would not catch a read that + // created a new row and then paged from the stale one. + stale := slices.Clone(expected) + slices.Reverse(stale) + + repository := NewDiscoverRepository(db, "algorithm-version-reuse-secret").(*postgresDiscoverRepo) + const controlViewerDID = "did:plc:algorithmversionviewer" + createTestUser(t, db, "algorithmversionviewer.test", controlViewerDID) + + supersededID, supersededCursor := seedDiscoverHotVersionedSnapshot(t, db, repository, + supersededDiscoverHotAlgorithmVersion, "", expected) + currentID, currentCursor := seedDiscoverHotVersionedSnapshot(t, db, repository, + currentDiscoverHotAlgorithmVersion, controlViewerDID, expected) + + t.Run("a version one snapshot is not reused", func(t *testing.T) { + feed, _ := readDiscoverHotRoot(t, repository, "", 10) + assert.Equal(t, expected, feed, + "a superseded snapshot must be rebuilt, and the rebuild ranks the real candidates") + + rebuilt := discoverHotSnapshotVersionsExcluding(t, db, "", supersededID) + require.Len(t, rebuilt, 1, "the read must have built exactly one replacement snapshot") + assert.Equal(t, currentDiscoverHotAlgorithmVersion, rebuilt[0], "the replacement must be stamped version 2") + }) + + t.Run("a version one cursor is invalid", func(t *testing.T) { + feed, next, err := repository.GetDiscover(context.Background(), discover.GetDiscoverRequest{ + Sort: "hot", + Limit: 2, + Cursor: &supersededCursor, + }) + assert.ErrorIs(t, err, discover.ErrInvalidCursor, + "a cursor into a superseded snapshot must be refused, not silently served") + assert.Empty(t, feed) + assert.Nil(t, next) + }) + + // The control. Everything about these two fixtures is identical but the + // version, so if the version-2 snapshot is not reused and its cursor is not + // accepted, the fixtures are malformed and the assertions above prove + // nothing about the version. + t.Run("a version two snapshot is reused", func(t *testing.T) { + feed, _ := readDiscoverHotRoot(t, repository, controlViewerDID, 10) + assert.Equal(t, stale, feed, "a current, fresh, in-scope snapshot must be served from its stored order") + assert.Equal(t, []int{currentDiscoverHotAlgorithmVersion}, + discoverHotSnapshotVersions(t, db, controlViewerDID), + "reuse must not have built a second snapshot in this scope") + }) + + t.Run("a version two cursor is accepted", func(t *testing.T) { + feed, _, err := repository.GetDiscover(context.Background(), discover.GetDiscoverRequest{ + ViewerDID: controlViewerDID, + Sort: "hot", + Limit: 2, + Cursor: ¤tCursor, + }) + require.NoError(t, err, "a cursor into a current snapshot must still page") + assert.Equal(t, stale[:2], discoverURIs(feed)) + + // Paging a cursor must read the snapshot the cursor names, not build a + // fresh one and serve that: a rebuild here would return a plausible page + // of the right length in the right order, and the reader would silently + // cross from one frozen ordering into another mid-feed. + assert.Equal(t, []int64{currentID}, discoverHotSnapshotIDs(t, db, controlViewerDID), + "serving an accepted cursor must not have created a second snapshot in this scope") + }) +} + +// seedDiscoverHotVersionedSnapshot writes a snapshot that is valid in every +// respect except, possibly, its algorithm version: fresh, unexpired, in the +// given viewer's scope, with candidate rows for real posts and a checkpoint at +// the root of its ordering. It returns the snapshot id and a cursor built the +// way production builds one. +// +// The rows are written directly rather than through a read, because a read +// cannot be asked to produce a snapshot at a version the code does not currently +// write — which is exactly the row this suite needs. +func seedDiscoverHotVersionedSnapshot( + t *testing.T, + db *sql.DB, + repository *postgresDiscoverRepo, + algorithmVersion int, + viewerDID string, + uris []string, +) (int64, string) { + t.Helper() + + ctx := context.Background() + var snapshotID int64 + require.NoError(t, db.QueryRowContext(ctx, ` + INSERT INTO discover_hot_snapshots ( + algorithm_version, viewer_scope, ranking_time, expires_at + ) VALUES ($1, $2, NOW(), NOW() + INTERVAL '30 minutes') + RETURNING id + `, algorithmVersion, discoverHotViewerScope(viewerDID)).Scan(&snapshotID)) + + // Descending base ranks assigned back to front, so the stored order is the + // reverse of the ranking a rebuild would compute. + for position, uri := range uris { + baseRank := float64(position + 1) + _, err := db.ExecContext(ctx, ` + INSERT INTO discover_hot_candidates (snapshot_id, uri, community_did, base_rank, created_at) + SELECT $1, p.uri, p.community_did, $3, p.created_at + FROM posts p + WHERE p.uri = $2 + `, snapshotID, uri, baseRank) + require.NoErrorf(t, err, "seeding candidate %s", uri) + } + var candidates int + require.NoError(t, db.QueryRowContext(ctx, + `SELECT count(*) FROM discover_hot_candidates WHERE snapshot_id = $1`, snapshotID).Scan(&candidates)) + require.Equal(t, len(uris), candidates, "every seeded candidate must reference a real post") + + tx, err := db.BeginTx(ctx, nil) + require.NoError(t, err) + defer tx.Rollback() + checkpointID, err := repository.storeDiscoverHotCheckpoint(ctx, tx, snapshotID, discover.DiscoverHotSelectionState{ + CommunityPositions: map[string]int{}, + RecentCommunities: []string{}, + }) + require.NoError(t, err, "seeding the checkpoint the cursor points at") + require.NoError(t, tx.Commit()) + + cursor, err := repository.buildDiscoverHotCursor(snapshotID, checkpointID, viewerDID) + require.NoError(t, err) + return snapshotID, cursor +} + +// discoverHotSnapshotIDs returns every snapshot id in a viewer scope, oldest +// first, so a test can assert which snapshots exist rather than how many. +func discoverHotSnapshotIDs(t *testing.T, db *sql.DB, viewerDID string) []int64 { + t.Helper() + + var ids pq.Int64Array + require.NoError(t, db.QueryRowContext(context.Background(), ` + SELECT coalesce(array_agg(id ORDER BY id), '{}') + FROM discover_hot_snapshots + WHERE viewer_scope = $1 + `, discoverHotViewerScope(viewerDID)).Scan(&ids)) + return ids +} + +func discoverHotSnapshotVersions(t *testing.T, db *sql.DB, viewerDID string) []int { + t.Helper() + return queryDiscoverHotSnapshotVersions(t, db, viewerDID, 0) +} + +func discoverHotSnapshotVersionsExcluding(t *testing.T, db *sql.DB, viewerDID string, excludedID int64) []int { + t.Helper() + return queryDiscoverHotSnapshotVersions(t, db, viewerDID, excludedID) +} + +func queryDiscoverHotSnapshotVersions(t *testing.T, db *sql.DB, viewerDID string, excludedID int64) []int { + t.Helper() + + var versions pq.Int64Array + require.NoError(t, db.QueryRowContext(context.Background(), ` + SELECT coalesce(array_agg(algorithm_version ORDER BY id), '{}') + FROM discover_hot_snapshots + WHERE viewer_scope = $1 + AND id <> $2 + `, discoverHotViewerScope(viewerDID), excludedID).Scan(&versions)) + + result := make([]int, len(versions)) + for i, version := range versions { + result[i] = int(version) + } + return result +} diff --git a/internal/db/postgres/discover_hot_hosted_bonus_test.go b/internal/db/postgres/discover_hot_hosted_bonus_test.go new file mode 100644 index 0000000..18abe49 --- /dev/null +++ b/internal/db/postgres/discover_hot_hosted_bonus_test.go @@ -0,0 +1,380 @@ +//go:build integration + +package postgres + +import ( + "context" + "database/sql" + "fmt" + "math" + "testing" + "time" + + "Coves/internal/core/discover" + "Coves/tests/testkit" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The hosted-community bonus as the snapshot build actually stores it. +// +// discover.DiscoverHotRank already takes the bonus and internal/core/discover +// proves what it does with it, but a rank nothing persists ranks nothing: the +// value a page is ordered by is discover_hot_candidates.base_rank, written once +// per snapshot build. So the question here is the repository's, not the +// formula's — does the build ask "do we hold this community's PDS refresh +// token?" for every candidate, and does it feed the bonus, keyed under its own +// cursor secret, into the rank it writes down. +// +// The four communities are the four answers that predicate can give, and three +// of them are traps: +// +// - stored ciphertext: hosted, bonus. +// - non-NULL but EMPTY ciphertext: still hosted. Presence is the fact +// (internal/db/postgres/community_hosted.go); an implementation that +// decrypted, or that tested for a non-empty value, would silently drop the +// bonus for a community whose credentials are merely malformed. +// - NULL ciphertext with hosted_by_did naming this instance: NOT hosted. +// hosted_by_did is copied out of the community's own profile record, so it +// is a claim by whoever controls that repo — an implementation keyed off it +// lets anyone buy a Discover boost by writing one field. +// - NULL ciphertext, hosted elsewhere: not hosted, the ordinary federated case. +// +// Two builds, each checked against ITS OWN persisted ranking time, because a +// rank is only meaningful next to the clock that decayed it: comparing the +// second snapshot's ranks against the first snapshot's ranking time would fail +// for every post whether or not the bonus were applied, and passing the build's +// own time back in is the only way the bonus is the sole difference being +// measured. +const ( + // hostedBonusCursorSecret is the secret the repositories below are built + // with, and therefore the key the bonus must be derived from: the bonus is + // HMACed under the repository's own cursor secret, so an expectation that + // hashed the URI alone would pass against an implementation an author can + // grind rkeys against offline. + hostedBonusCursorSecret = "hosted-bonus-secret" + + // neutralCommunityAdjustment is what CommunityAdjustment returns when no + // community has enough history to form a reference. This suite seeds far + // fewer than minimumReferencePosts per community inside the history window, + // so the build computes exactly this — and the non-hosted score-25 post + // below is what proves the assumption rather than assuming it, since the + // adjustment only enters the numerator for a positive score. + neutralCommunityAdjustment = 1.0 + + // minimumSecretSeparation is how far apart two secrets' bonuses for one URI + // have to be before a stored rank can be attributed to one of them. Two + // secrets can always hash a given URI to nearby values by chance, and a + // post whose two bonuses nearly agree proves nothing about which key was + // used. + minimumSecretSeparation = 0.25 + + // hostedBonusRankTolerance is relative: the build and the assertion run the + // same float64 expression, so they agree to the last few bits. + hostedBonusRankTolerance = 1e-12 + + // minimumDistinguishingBonus keeps this test honest. HostedCommunityBonus + // spreads over [0, 3.0), so a URI can hash to a bonus small enough that a + // build ignoring it lands within any sane tolerance anyway. Every post here + // is seeded under an rkey whose bonus clears this, so a bonus-free + // implementation cannot pass by luck — and the posts that must NOT be given + // the bonus carry a large one too, so misapplying it is equally visible. + minimumDistinguishingBonus = 0.01 +) + +func TestDiscoverRepo_HotPersistedBaseRankCarriesHostedCommunityBonus(t *testing.T) { + t.Parallel() + + db := testkit.DB(t) + const authorDID = "did:plc:hotbonusauthor" + createTestUser(t, db, "hotbonusauthor.test", authorDID) + + credentialed := hostedCommunity(t, db, "hotbonuscreds") + emptyCiphertext := emptyCiphertextCommunity(t, db, "hotbonusempty") + claimingHost := uncredentialedCommunity(t, db, "hotbonusclaim") + federated := federatedCommunity(t, db, "hotbonusfederated") + + now := time.Now() + seeded := []struct { + community string + score int + age time.Duration + hosted bool + reason string + }{ + {credentialed, 0, 3 * time.Hour, true, "a stored refresh token is the hosted fact"}, + {credentialed, 25, 9 * time.Hour, true, "a positive score takes the bonus on top of its adjusted numerator"}, + {credentialed, -4, 20 * time.Hour, false, "a negatively scored post is not boosted for being hosted"}, + {emptyCiphertext, 0, 31 * time.Hour, true, "an empty ciphertext is still a stored credential"}, + {claimingHost, 0, 14 * time.Hour, false, "hosted_by_did naming this instance is a claim, not a credential"}, + {federated, 0, 44 * time.Hour, false, "a community hosted elsewhere earns nothing"}, + {federated, 25, 5 * time.Hour, false, "the neutral-adjustment witness: a positive score with no bonus"}, + } + type expectation struct { + uri string + score int + createdAt time.Time + bonus float64 + reason string + } + expected := make([]expectation, 0, len(seeded)) + for _, post := range seeded { + createdAt := now.Add(-post.age).Truncate(time.Second) + uri := seedDistinguishableBonusPost(t, db, post.community, authorDID, post.score, createdAt, + func(uri string) bool { + return discover.HostedCommunityBonus(hostedBonusCursorSecret, uri) > minimumDistinguishingBonus + }) + bonus := discover.HostedCommunityBonus(hostedBonusCursorSecret, uri) + require.Greaterf(t, bonus, minimumDistinguishingBonus, + "%s hashes to a bonus too small to tell an implementation apart", uri) + if !post.hosted { + bonus = 0 + } + expected = append(expected, expectation{ + uri: uri, + score: post.score, + createdAt: createdAt, + bonus: bonus, + reason: post.reason, + }) + } + + // Two distinct builds without waiting out discoverHotSnapshotReuse: the + // thirty-second reuse window is per viewer scope, so an authenticated read + // cannot reuse the anonymous snapshot and has to build its own. + const viewerDID = "did:plc:hotbonusviewer" + createTestUser(t, db, "hotbonusviewer.test", viewerDID) + readDiscoverHotRoot(t, NewDiscoverRepository(db, hostedBonusCursorSecret), "", 10) + anonymous := readPersistedDiscoverHotSnapshot(t, db, "") + readDiscoverHotRoot(t, NewDiscoverRepository(db, hostedBonusCursorSecret), viewerDID, 10) + authenticated := readPersistedDiscoverHotSnapshot(t, db, viewerDID) + require.NotEqual(t, anonymous.id, authenticated.id, "the two reads must have built separate snapshots") + + for _, snapshot := range []persistedDiscoverHotSnapshot{anonymous, authenticated} { + for _, post := range expected { + stored, ok := snapshot.baseRanks[post.uri] + require.Truef(t, ok, "snapshot %d is missing candidate %s", snapshot.id, post.uri) + want := discover.DiscoverHotRank(post.score, post.createdAt, snapshot.rankingTime, + neutralCommunityAdjustment, post.bonus) + assert.InEpsilonf(t, want, stored, hostedBonusRankTolerance, + "snapshot %d stored the wrong base rank for %s: %s", snapshot.id, post.uri, post.reason) + } + } +} + +// The key the bonus is derived from, rather than the fact that a bonus exists. +// +// An unkeyed hash of the AT-URI is public arithmetic: the rkey is the author's +// to choose, so an author can grind rkeys offline until one lands near the 3.0 +// ceiling and enter Discover at the top of every hosted community they can post +// in. Keying the hash with the repository's cursor secret — a value that never +// leaves the server — is what makes that search impossible to run. +// +// Two repositories over one database, each holding a different secret, are how +// that is measured: they index the identical post row, so the secret is the +// only thing that can separate the ranks they write down. Each snapshot is then +// checked against its own secret AND against what the other secret would have +// produced at that same ranking time, because the two builds were stamped at +// different instants and would store different ranks under one shared secret +// too — holding the clock fixed is what isolates the key. +func TestDiscoverRepo_HotHostedBonusIsKeyedByTheCursorSecret(t *testing.T) { + t.Parallel() + + db := testkit.DB(t) + const ( + authorDID = "did:plc:hotbonuskeyedauthor" + firstSecret = "hosted-bonus-first-cursor-secret" + secondSecret = "hosted-bonus-second-cursor-secret" + ) + createTestUser(t, db, "hotbonuskeyedauthor.test", authorDID) + + community := hostedCommunity(t, db, "hotbonuskeyed") + createdAt := time.Now().Add(-7 * time.Hour).Truncate(time.Second) + uri := seedDistinguishableBonusPost(t, db, community, authorDID, 0, createdAt, func(uri string) bool { + first := discover.HostedCommunityBonus(firstSecret, uri) + second := discover.HostedCommunityBonus(secondSecret, uri) + return first > minimumDistinguishingBonus && second > minimumDistinguishingBonus && + math.Abs(first-second) > minimumSecretSeparation + }) + + firstBonus := discover.HostedCommunityBonus(firstSecret, uri) + secondBonus := discover.HostedCommunityBonus(secondSecret, uri) + require.Greaterf(t, firstBonus, minimumDistinguishingBonus, + "%s hashes to a bonus too small to tell an implementation apart", uri) + require.Greaterf(t, secondBonus, minimumDistinguishingBonus, + "%s hashes to a bonus too small to tell an implementation apart", uri) + require.Greater(t, math.Abs(firstBonus-secondBonus), minimumSecretSeparation, + "no rkey in the search produced a bonus that moved when the secret changed: the bonus is not keyed by the cursor secret") + + // Separate viewer scopes, for the same reason as above: the thirty-second + // reuse window is per scope, so two reads in one scope would share a single + // snapshot and the second secret would never build anything. + const viewerDID = "did:plc:hotbonuskeyedviewer" + createTestUser(t, db, "hotbonuskeyedviewer.test", viewerDID) + + readDiscoverHotRoot(t, NewDiscoverRepository(db, firstSecret), "", 10) + firstSnapshot := readPersistedDiscoverHotSnapshot(t, db, "") + readDiscoverHotRoot(t, NewDiscoverRepository(db, secondSecret), viewerDID, 10) + secondSnapshot := readPersistedDiscoverHotSnapshot(t, db, viewerDID) + require.NotEqual(t, firstSnapshot.id, secondSnapshot.id, "the two reads must have built separate snapshots") + + for _, build := range []struct { + name string + secret string + otherSecret string + snapshot persistedDiscoverHotSnapshot + }{ + {"anonymous", firstSecret, secondSecret, firstSnapshot}, + {"authenticated", secondSecret, firstSecret, secondSnapshot}, + } { + stored, ok := build.snapshot.baseRanks[uri] + require.Truef(t, ok, "snapshot %d is missing candidate %s", build.snapshot.id, uri) + + own := discover.DiscoverHotRank(0, createdAt, build.snapshot.rankingTime, + neutralCommunityAdjustment, discover.HostedCommunityBonus(build.secret, uri)) + other := discover.DiscoverHotRank(0, createdAt, build.snapshot.rankingTime, + neutralCommunityAdjustment, discover.HostedCommunityBonus(build.otherSecret, uri)) + + assert.InEpsilonf(t, own, stored, hostedBonusRankTolerance, + "the %s snapshot must rank the post under the cursor secret ITS repository was built with", build.name) + assert.Greaterf(t, math.Abs(stored-other), math.Abs(own-other)/2, + "the %s snapshot stored a rank no better explained by its own secret than by the other repository's", build.name) + } + + assert.NotEqual(t, firstSnapshot.baseRanks[uri], secondSnapshot.baseRanks[uri], + "two repositories holding different cursor secrets must not agree on one post's base rank") +} + +// emptyCiphertextCommunity seeds a community whose credential column holds zero +// bytes — non-NULL, and therefore hosted. +// +// The value is deliberately not a valid ciphertext: nothing on this path +// decrypts it, and a community with a corrupt stored credential is still one +// this AppView provisioned and signs for. +func emptyCiphertextCommunity(t *testing.T, db *sql.DB, name string) string { + t.Helper() + + did := uncredentialedCommunity(t, db, name) + _, err := db.ExecContext(context.Background(), + `UPDATE communities SET pds_refresh_token_encrypted = $2 WHERE did = $1`, + did, []byte{}) + require.NoErrorf(t, err, "storing an empty credential for %s", did) + return did +} + +// federatedCommunity seeds a community this AppView merely indexes: no stored +// credential, and a hosted_by_did that names someone else. +// +// It is the ordinary federated shape, and the counterweight to +// uncredentialedCommunity — which leaves hosted_by_did naming THIS instance, so +// between the two an implementation cannot satisfy both by reading that column. +func federatedCommunity(t *testing.T, db *sql.DB, name string) string { + t.Helper() + + did := uncredentialedCommunity(t, db, name) + _, err := db.ExecContext(context.Background(), + `UPDATE communities SET hosted_by_did = $2 WHERE did = $1`, + did, "did:web:elsewhere.invalid") + require.NoErrorf(t, err, "rehosting %s elsewhere", did) + return did +} + +// seedDistinguishableBonusPost inserts one post under an rkey the caller is +// willing to measure with: usable rejects an rkey whose bonus would be too +// small, or too close to another secret's, to tell two implementations apart. +// +// The search is over rkeys rather than over anything the ranking reads, so it +// biases nothing: every candidate URI is as valid as the first, and the loop +// only refuses the small minority the caller cannot measure. +// +// An exhausted search seeds the last candidate anyway rather than failing here. +// A search that never succeeds is itself a finding about the implementation — +// that every rkey hashes the same way under two different secrets, say — and +// the caller states that as an assertion about the URI it gets back, where the +// message can name the property that was missing. +func seedDistinguishableBonusPost(t *testing.T, db *sql.DB, communityDID, authorDID string, score int, createdAt time.Time, usable func(uri string) bool) string { + t.Helper() + + base := testkit.UniqueIDWithPrefix(t, "hotbonus") + rkey := base + for attempt := 0; attempt < 100 && !usable(discoverHotPostURI(communityDID, rkey)); attempt++ { + rkey = fmt.Sprintf("%s%d", base, attempt) + } + + // score = upvotes - downvotes is the invariant the serving queries rely on. + upvotes, downvotes := score, 0 + if score < 0 { + upvotes, downvotes = 0, -score + } + uri := discoverHotPostURI(communityDID, rkey) + _, err := db.ExecContext(context.Background(), ` + INSERT INTO posts (uri, cid, rkey, author_did, community_did, title, created_at, score, upvote_count, downvote_count) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) + `, uri, "bafypost"+rkey, rkey, authorDID, communityDID, "hosted bonus post "+rkey, createdAt, score, upvotes, downvotes) + require.NoErrorf(t, err, "seeding post %s in %s", rkey, communityDID) + return uri +} + +// discoverHotPostURI builds this fixture's LEGACY post URI: a community-repo +// at:///social.coves.community.post/. +// +// A live record is a postv2 in the AUTHOR's repo, under an rkey the author +// picked — which is the reason the bonus is keyed by a server secret at all. +// The legacy shape is used here because visiblePostsPredicate fails CLOSED for +// a postv2 with no admission row: a postv2 fixture would need an accepted +// admission row carrying the post's own CID before it could even reach the +// candidate query, and seeding the admission engine to measure a ranking number +// would put this suite at the mercy of a gate it is not testing. Nothing on +// this path parses the URI, so the shape changes no answer here; what the bonus +// must be keyed by is asserted directly, in the test below. +func discoverHotPostURI(communityDID, rkey string) string { + return "at://" + communityDID + "/social.coves.community.post/" + rkey +} + +// persistedDiscoverHotSnapshot is what a build wrote down: the ranking time it +// stamped on the snapshot and the base rank it stored per candidate. +type persistedDiscoverHotSnapshot struct { + id int64 + rankingTime time.Time + baseRanks map[string]float64 +} + +// readPersistedDiscoverHotSnapshot returns the newest snapshot for a viewer +// scope. +// +// The algorithm version is deliberately not part of the lookup: what is under +// test is the number in base_rank, and pinning the version here would make this +// suite fail for the unrelated reason that the version was bumped. +func readPersistedDiscoverHotSnapshot(t *testing.T, db *sql.DB, viewerDID string) persistedDiscoverHotSnapshot { + t.Helper() + + ctx := context.Background() + snapshot := persistedDiscoverHotSnapshot{baseRanks: make(map[string]float64)} + require.NoError(t, db.QueryRowContext(ctx, ` + SELECT id, ranking_time + FROM discover_hot_snapshots + WHERE viewer_scope = $1 + ORDER BY created_at DESC + LIMIT 1 + `, discoverHotViewerScope(viewerDID)).Scan(&snapshot.id, &snapshot.rankingTime)) + + rows, err := db.QueryContext(ctx, ` + SELECT uri, base_rank + FROM discover_hot_candidates + WHERE snapshot_id = $1 + `, snapshot.id) + require.NoError(t, err) + defer rows.Close() + + for rows.Next() { + var ( + uri string + baseRank float64 + ) + require.NoError(t, rows.Scan(&uri, &baseRank)) + snapshot.baseRanks[uri] = baseRank + } + require.NoError(t, rows.Err()) + return snapshot +} diff --git a/internal/db/postgres/discover_hot_rank_sql_equivalence_test.go b/internal/db/postgres/discover_hot_rank_sql_equivalence_test.go index 1547f88..7f50fa0 100644 --- a/internal/db/postgres/discover_hot_rank_sql_equivalence_test.go +++ b/internal/db/postgres/discover_hot_rank_sql_equivalence_test.go @@ -42,7 +42,7 @@ func TestDiscoverHotRank_NeutralMatchesLegacyPostgresExpression(t *testing.T) { testCase.score, testCase.createdAt, rankingTime).Scan(&postgresRank) require.NoError(t, err) - goRank := discover.DiscoverHotRank(testCase.score, testCase.createdAt, rankingTime, 1) + goRank := discover.DiscoverHotRank(testCase.score, testCase.createdAt, rankingTime, 1, 0) absoluteError := math.Abs(goRank - postgresRank) scale := math.Max(math.Abs(goRank), math.Abs(postgresRank)) relativeError := absoluteError diff --git a/internal/db/postgres/discover_hot_repo.go b/internal/db/postgres/discover_hot_repo.go index c2750ac..6c0c720 100644 --- a/internal/db/postgres/discover_hot_repo.go +++ b/internal/db/postgres/discover_hot_repo.go @@ -21,7 +21,7 @@ import ( ) const ( - discoverHotAlgorithmVersion = 1 + discoverHotAlgorithmVersion = 2 discoverHotCursorVersion = "discover-hot-v1" discoverHotCursorMaxLength = 500 discoverHotSnapshotReuse = 30 * time.Second @@ -274,7 +274,11 @@ func (r *postgresDiscoverRepo) createDiscoverHotSnapshot(ctx context.Context, vi adjustment = discover.CommunityAdjustment(history[candidate.CommunityDID], reference, referenceAvailable) adjustments[candidate.CommunityDID] = adjustment } - candidate.BaseRank = discover.DiscoverHotRank(candidate.Score, candidate.CreatedAt, rankingTime, adjustment) + var hostedBonus float64 + if candidate.Hosted { + hostedBonus = discover.HostedCommunityBonus(r.cursorSecret, candidate.URI) + } + candidate.BaseRank = discover.DiscoverHotRank(candidate.Score, candidate.CreatedAt, rankingTime, adjustment, hostedBonus) if math.IsNaN(candidate.BaseRank) || math.IsInf(candidate.BaseRank, 0) { return 0, nil, fmt.Errorf("nonfinite Discover Hot rank for %s", candidate.URI) } @@ -361,6 +365,9 @@ func (r *postgresDiscoverRepo) recordDiscoverHotBuildAttempt(ctx context.Context type discoverHotRawCandidate struct { discover.DiscoverHotCandidate Score int + // Hosted is the presence of a stored PDS refresh token for the post's + // community, never its contents: the credential is not decrypted here. + Hosted bool } func queryDiscoverHotHistory(ctx context.Context, tx *sql.Tx, rankingTime time.Time) (map[string][]int, error) { @@ -404,8 +411,13 @@ func queryDiscoverHotCandidates(ctx context.Context, tx *sql.Tx, viewerDID strin queryLimit++ } rows, err := tx.QueryContext(ctx, ` - SELECT p.uri, p.community_did, p.score, p.created_at + SELECT p.uri, p.community_did, p.score, p.created_at, + c.pds_refresh_token_encrypted IS NOT NULL AS hosted FROM posts p`+visJoin+` + -- communities.did is UNIQUE, so this join cannot multiply candidate + -- rows: the candidate limit and the atomic overflow refusal both + -- depend on one row per post. A missing community reads as not hosted. + LEFT JOIN communities c ON c.did = p.community_did WHERE p.deleted_at IS NULL AND `+visWhere+` `+viewerFilter+` @@ -419,7 +431,7 @@ func queryDiscoverHotCandidates(ctx context.Context, tx *sql.Tx, viewerDID strin var candidates []discoverHotRawCandidate for rows.Next() { var candidate discoverHotRawCandidate - if err := rows.Scan(&candidate.URI, &candidate.CommunityDID, &candidate.Score, &candidate.CreatedAt); err != nil { + if err := rows.Scan(&candidate.URI, &candidate.CommunityDID, &candidate.Score, &candidate.CreatedAt, &candidate.Hosted); err != nil { return nil, fmt.Errorf("scan Discover Hot candidate: %w", err) } if len(candidates) == candidateLimit { -- 2.51.2