Stop flooring common_likes to zero
_likes_overlap.common is CAST(SUM(decay * decay) AS INTEGER). Each decay
factor is at most 1, so their product is always just under it: a pair
sharing one shared like sums to about 0.998 and truncates to 0.
That value is used twice. It is stored as common_likes and shown to readers
as the reason for a recommendation ("N shared likes"), and it is the
numerator of the likes term added to the pair's similarity. Flooring it
therefore both hid real overlap and silently removed the LikesWeight signal
for any pair whose decayed overlap had not yet reached 1.
ROUND fixes it without changing the decay.
The existing TestComputeUserSimilarity asserts only that some pairs exist,
which is how this survived -- the column was never read.
The new test uses a like an hour old rather than a fresh one, and that
detail matters: a like written in the same instant the computation runs has
an age of about zero, so its decay rounds to exactly 1.0 and the truncation
does not bite. The first version of this test passed against the unfixed
code for precisely that reason. Likes arriving from a PDS are always at
least minutes old, so the hour-old case is the realistic one.
Confirmed to fail against the previous code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>351bd41 parent: 7e1c610 modified
internal/cluster/jaccard.go +7 -2 | @@ -288,10 +288,15 @@ func (e *Engine) ComputeUserSimilarity(ctx context.Context) error { | ||
| 288 | 288 | if _, err := tx.ExecContext(ctx, ` |
| 289 | 289 | INSERT INTO _likes_overlap (user_a, user_b, common) |
| 290 | 290 | SELECT l1.author_did, l2.author_did, |
| 291 | - CAST(SUM( | |
| 291 | + -- ROUND, not a bare CAST: each decay factor is at most 1, so | |
| 292 | + -- their product is always just under it. Truncating meant a | |
| 293 | + -- pair sharing one recent like summed to 0.9999 and stored 0, | |
| 294 | + -- which both hid the overlap from the reader ("N shared | |
| 295 | + -- likes") and zeroed the likes term of their similarity. | |
| 296 | + CAST(ROUND(SUM( | |
| 292 | 297 | EXP(-0.023 * CAST(julianday('now') - julianday(l1.created_at) AS REAL)) |
| 293 | 298 | * EXP(-0.023 * CAST(julianday('now') - julianday(l2.created_at) AS REAL)) |
| 294 | - ) AS INTEGER) | |
| 299 | + )) AS INTEGER) | |
| 295 | 300 | FROM articles.likes l1 |
| 296 | 301 | JOIN articles.likes l2 ON l1.feed_url = l2.feed_url AND l1.article_url = l2.article_url |
| 297 | 302 | AND l1.author_did < l2.author_did |
| @@ -288,10 +288,15 @@ func (e *Engine) ComputeUserSimilarity(ctx context.Context) error { | |||
| 288 | if _, err := tx.ExecContext(ctx, ` | 288 | if _, err := tx.ExecContext(ctx, ` |
| 289 | INSERT INTO _likes_overlap (user_a, user_b, common) | 289 | INSERT INTO _likes_overlap (user_a, user_b, common) |
| 290 | SELECT l1.author_did, l2.author_did, | 290 | SELECT l1.author_did, l2.author_did, |
| 291 | - CAST(SUM( | 291 | + -- ROUND, not a bare CAST: each decay factor is at most 1, so |
| 292 | + -- their product is always just under it. Truncating meant a | ||
| 293 | + -- pair sharing one recent like summed to 0.9999 and stored 0, | ||
| 294 | + -- which both hid the overlap from the reader ("N shared | ||
| 295 | + -- likes") and zeroed the likes term of their similarity. | ||
| 296 | + CAST(ROUND(SUM( | ||
| 292 | EXP(-0.023 * CAST(julianday('now') - julianday(l1.created_at) AS REAL)) | 297 | EXP(-0.023 * CAST(julianday('now') - julianday(l1.created_at) AS REAL)) |
| 293 | * EXP(-0.023 * CAST(julianday('now') - julianday(l2.created_at) AS REAL)) | 298 | * EXP(-0.023 * CAST(julianday('now') - julianday(l2.created_at) AS REAL)) |
| 294 | - ) AS INTEGER) | 299 | + )) AS INTEGER) |
| 295 | FROM articles.likes l1 | 300 | FROM articles.likes l1 |
| 296 | JOIN articles.likes l2 ON l1.feed_url = l2.feed_url AND l1.article_url = l2.article_url | 301 | JOIN articles.likes l2 ON l1.feed_url = l2.feed_url AND l1.article_url = l2.article_url |
| 297 | AND l1.author_did < l2.author_did | 302 | AND l1.author_did < l2.author_did |
modified
internal/cluster/jaccard_test.go +79 -0 | @@ -2,12 +2,14 @@ package cluster | ||
| 2 | 2 | |
| 3 | 3 | import ( |
| 4 | 4 | "context" |
| 5 | + "database/sql" | |
| 5 | 6 | "fmt" |
| 6 | 7 | "hash/fnv" |
| 7 | 8 | "log/slog" |
| 8 | 9 | "os" |
| 9 | 10 | "strings" |
| 10 | 11 | "testing" |
| 12 | + "time" | |
| 11 | 13 | |
| 12 | 14 | vec "github.com/asg017/sqlite-vec-go-bindings/cgo" |
| 13 | 15 | "pkg.rbrt.fr/glean/internal/db" |
| @@ -892,3 +894,80 @@ func TestNormalizeFeedScores(t *testing.T) { | ||
| 892 | 894 | assert.Equal(t, recs[1].Score, 4.0/9.0) |
| 893 | 895 | assert.Equal(t, recs[2].Score, 0.0) |
| 894 | 896 | } |
| 897 | + | |
| 898 | +// TestComputeUserSimilarity above asserts only that some pairs exist, which | |
| 899 | +// is how the common_likes truncation survived: the column was never read. | |
| 900 | +// | |
| 901 | +// Each decay factor is at most 1, so their product is always just under it. | |
| 902 | +// A bare CAST(... AS INTEGER) therefore floors a single shared like to zero, | |
| 903 | +// which both hides the overlap from the reader ("N shared likes") and zeroes | |
| 904 | +// the likes term of that pair's similarity. | |
| 905 | +// | |
| 906 | +// The like is deliberately an hour old rather than brand new. A like written | |
| 907 | +// in the same instant the computation runs has an age of ~0, so its decay | |
| 908 | +// rounds to exactly 1.0 and the truncation does not bite -- which is not a | |
| 909 | +// case that occurs outside a test, since likes arrive from a PDS already | |
| 910 | +// minutes to days old. | |
| 911 | +func TestUserSimilarity_SingleSharedLikeCounts(t *testing.T) { | |
| 912 | + ctx := context.Background() | |
| 913 | + dbs := setupClusterTestDB(t) | |
| 914 | + seedClusterData(t, ctx, dbs) | |
| 915 | + | |
| 916 | + const feedURL = "https://a.com/feed" | |
| 917 | + const articleURL = "https://a.com/shared-article" | |
| 918 | + anHourAgo := time.Now().Add(-time.Hour) | |
| 919 | + for _, did := range []string{"did:test:alice", "did:test:bob"} { | |
| 920 | + assert.NilError(t, dbs.Articles.CreateLike(ctx, &db.Like{ | |
| 921 | + URI: "at://" + did + "/app.bsky.feed.like/shared", | |
| 922 | + AuthorDID: did, | |
| 923 | + FeedURL: feedURL, | |
| 924 | + ArticleURL: articleURL, | |
| 925 | + CreatedAt: sql.NullTime{Time: anHourAgo, Valid: true}, | |
| 926 | + })) | |
| 927 | + } | |
| 928 | + | |
| 929 | + engine := newTestEngine(dbs) | |
| 930 | + assert.NilError(t, engine.ComputeUserSimilarity(ctx)) | |
| 931 | + | |
| 932 | + var commonLikes int | |
| 933 | + err := dbs.SQLDB().QueryRowContext(ctx, ` | |
| 934 | + SELECT common_likes FROM recs.user_similarity | |
| 935 | + WHERE user_a = ? AND user_b = ? | |
| 936 | + `, "did:test:alice", "did:test:bob").Scan(&commonLikes) | |
| 937 | + assert.NilError(t, err) | |
| 938 | + assert.Equal(t, commonLikes, 1, | |
| 939 | + "one shared like should count as one, not be floored to zero") | |
| 940 | +} | |
| 941 | + | |
| 942 | +// The rounding must not undo the decay: a like from long ago should still | |
| 943 | +// count for nothing. | |
| 944 | +func TestUserSimilarity_StaleSharedLikeDecays(t *testing.T) { | |
| 945 | + ctx := context.Background() | |
| 946 | + dbs := setupClusterTestDB(t) | |
| 947 | + seedClusterData(t, ctx, dbs) | |
| 948 | + | |
| 949 | + const feedURL = "https://a.com/feed" | |
| 950 | + const articleURL = "https://a.com/ancient-article" | |
| 951 | + longAgo := time.Now().AddDate(0, 0, -400) | |
| 952 | + for _, did := range []string{"did:test:alice", "did:test:bob"} { | |
| 953 | + assert.NilError(t, dbs.Articles.CreateLike(ctx, &db.Like{ | |
| 954 | + URI: "at://" + did + "/app.bsky.feed.like/ancient", | |
| 955 | + AuthorDID: did, | |
| 956 | + FeedURL: feedURL, | |
| 957 | + ArticleURL: articleURL, | |
| 958 | + CreatedAt: sql.NullTime{Time: longAgo, Valid: true}, | |
| 959 | + })) | |
| 960 | + } | |
| 961 | + | |
| 962 | + engine := newTestEngine(dbs) | |
| 963 | + assert.NilError(t, engine.ComputeUserSimilarity(ctx)) | |
| 964 | + | |
| 965 | + var commonLikes int | |
| 966 | + err := dbs.SQLDB().QueryRowContext(ctx, ` | |
| 967 | + SELECT common_likes FROM recs.user_similarity | |
| 968 | + WHERE user_a = ? AND user_b = ? | |
| 969 | + `, "did:test:alice", "did:test:bob").Scan(&commonLikes) | |
| 970 | + assert.NilError(t, err) | |
| 971 | + assert.Equal(t, commonLikes, 0, | |
| 972 | + "a 400-day-old shared like should decay to nothing") | |
| 973 | +} | |
| @@ -2,12 +2,14 @@ package cluster | |||
| 2 | 2 | ||
| 3 | import ( | 3 | import ( |
| 4 | "context" | 4 | "context" |
| 5 | + "database/sql" | ||
| 5 | "fmt" | 6 | "fmt" |
| 6 | "hash/fnv" | 7 | "hash/fnv" |
| 7 | "log/slog" | 8 | "log/slog" |
| 8 | "os" | 9 | "os" |
| 9 | "strings" | 10 | "strings" |
| 10 | "testing" | 11 | "testing" |
| 12 | + "time" | ||
| 11 | 13 | ||
| 12 | vec "github.com/asg017/sqlite-vec-go-bindings/cgo" | 14 | vec "github.com/asg017/sqlite-vec-go-bindings/cgo" |
| 13 | "pkg.rbrt.fr/glean/internal/db" | 15 | "pkg.rbrt.fr/glean/internal/db" |
| @@ -892,3 +894,80 @@ func TestNormalizeFeedScores(t *testing.T) { | |||
| 892 | assert.Equal(t, recs[1].Score, 4.0/9.0) | 894 | assert.Equal(t, recs[1].Score, 4.0/9.0) |
| 893 | assert.Equal(t, recs[2].Score, 0.0) | 895 | assert.Equal(t, recs[2].Score, 0.0) |
| 894 | } | 896 | } |
| 897 | + | ||
| 898 | +// TestComputeUserSimilarity above asserts only that some pairs exist, which | ||
| 899 | +// is how the common_likes truncation survived: the column was never read. | ||
| 900 | +// | ||
| 901 | +// Each decay factor is at most 1, so their product is always just under it. | ||
| 902 | +// A bare CAST(... AS INTEGER) therefore floors a single shared like to zero, | ||
| 903 | +// which both hides the overlap from the reader ("N shared likes") and zeroes | ||
| 904 | +// the likes term of that pair's similarity. | ||
| 905 | +// | ||
| 906 | +// The like is deliberately an hour old rather than brand new. A like written | ||
| 907 | +// in the same instant the computation runs has an age of ~0, so its decay | ||
| 908 | +// rounds to exactly 1.0 and the truncation does not bite -- which is not a | ||
| 909 | +// case that occurs outside a test, since likes arrive from a PDS already | ||
| 910 | +// minutes to days old. | ||
| 911 | +func TestUserSimilarity_SingleSharedLikeCounts(t *testing.T) { | ||
| 912 | + ctx := context.Background() | ||
| 913 | + dbs := setupClusterTestDB(t) | ||
| 914 | + seedClusterData(t, ctx, dbs) | ||
| 915 | + | ||
| 916 | + const feedURL = "https://a.com/feed" | ||
| 917 | + const articleURL = "https://a.com/shared-article" | ||
| 918 | + anHourAgo := time.Now().Add(-time.Hour) | ||
| 919 | + for _, did := range []string{"did:test:alice", "did:test:bob"} { | ||
| 920 | + assert.NilError(t, dbs.Articles.CreateLike(ctx, &db.Like{ | ||
| 921 | + URI: "at://" + did + "/app.bsky.feed.like/shared", | ||
| 922 | + AuthorDID: did, | ||
| 923 | + FeedURL: feedURL, | ||
| 924 | + ArticleURL: articleURL, | ||
| 925 | + CreatedAt: sql.NullTime{Time: anHourAgo, Valid: true}, | ||
| 926 | + })) | ||
| 927 | + } | ||
| 928 | + | ||
| 929 | + engine := newTestEngine(dbs) | ||
| 930 | + assert.NilError(t, engine.ComputeUserSimilarity(ctx)) | ||
| 931 | + | ||
| 932 | + var commonLikes int | ||
| 933 | + err := dbs.SQLDB().QueryRowContext(ctx, ` | ||
| 934 | + SELECT common_likes FROM recs.user_similarity | ||
| 935 | + WHERE user_a = ? AND user_b = ? | ||
| 936 | + `, "did:test:alice", "did:test:bob").Scan(&commonLikes) | ||
| 937 | + assert.NilError(t, err) | ||
| 938 | + assert.Equal(t, commonLikes, 1, | ||
| 939 | + "one shared like should count as one, not be floored to zero") | ||
| 940 | +} | ||
| 941 | + | ||
| 942 | +// The rounding must not undo the decay: a like from long ago should still | ||
| 943 | +// count for nothing. | ||
| 944 | +func TestUserSimilarity_StaleSharedLikeDecays(t *testing.T) { | ||
| 945 | + ctx := context.Background() | ||
| 946 | + dbs := setupClusterTestDB(t) | ||
| 947 | + seedClusterData(t, ctx, dbs) | ||
| 948 | + | ||
| 949 | + const feedURL = "https://a.com/feed" | ||
| 950 | + const articleURL = "https://a.com/ancient-article" | ||
| 951 | + longAgo := time.Now().AddDate(0, 0, -400) | ||
| 952 | + for _, did := range []string{"did:test:alice", "did:test:bob"} { | ||
| 953 | + assert.NilError(t, dbs.Articles.CreateLike(ctx, &db.Like{ | ||
| 954 | + URI: "at://" + did + "/app.bsky.feed.like/ancient", | ||
| 955 | + AuthorDID: did, | ||
| 956 | + FeedURL: feedURL, | ||
| 957 | + ArticleURL: articleURL, | ||
| 958 | + CreatedAt: sql.NullTime{Time: longAgo, Valid: true}, | ||
| 959 | + })) | ||
| 960 | + } | ||
| 961 | + | ||
| 962 | + engine := newTestEngine(dbs) | ||
| 963 | + assert.NilError(t, engine.ComputeUserSimilarity(ctx)) | ||
| 964 | + | ||
| 965 | + var commonLikes int | ||
| 966 | + err := dbs.SQLDB().QueryRowContext(ctx, ` | ||
| 967 | + SELECT common_likes FROM recs.user_similarity | ||
| 968 | + WHERE user_a = ? AND user_b = ? | ||
| 969 | + `, "did:test:alice", "did:test:bob").Scan(&commonLikes) | ||
| 970 | + assert.NilError(t, err) | ||
| 971 | + assert.Equal(t, commonLikes, 0, | ||
| 972 | + "a 400-day-old shared like should decay to nothing") | ||
| 973 | +} | ||