diff --git a/src/background.ts b/src/background.ts index 95edc51..e751939 100644 --- a/src/background.ts +++ b/src/background.ts @@ -8,7 +8,7 @@ import { listRecords, parseAtUri, resolveDid } from './lib/atproto' import { cached } from './lib/cache' import { detectPage } from './lib/detection' import { type IconState, badgeFor, iconStateFor, titleFor } from './lib/icon' -import { followingDids } from './lib/subscribers' +import { FOLLOWS_CAP, type FollowList, followingDids } from './lib/subscribers' import { startSignIn } from './signin' import type { FollowSet, Msg, PageState, SessionInfo } from './lib/types' @@ -141,17 +141,17 @@ async function getBlocks(refresh: boolean): Promise { // // The one read in the extension whose size is set by how sociable the user is // rather than by what is on the page: `listRecords` pages a hundred at a time, -// and an account following fifty thousand people is five hundred requests. It -// runs here rather than in the popup for that reason. The popup asks and draws -// whatever comes back whenever it comes back; a walk this long must never be -// something a window is waiting on, and the worker outliving the popup means -// closing it does not throw the walk away. +// so FOLLOWS_CAP follows is a hundred requests. It runs here rather than in the +// popup for that reason. The popup asks and draws whatever comes back whenever +// it comes back; a walk this long must never be something a window is waiting +// on, and the worker outliving the popup means closing it does not throw the +// walk away. // // Cached under `graph`, which is a day and on disk (src/lib/cache.ts), so the // full walk is a once-a-day event and not a once-a-popup one. /** Walks in flight, keyed by account: two askers share one rather than starting two. */ -const followWalks = new Map>() +const followWalks = new Map>() async function getFollows(refresh: boolean): Promise { const session = await getSession() @@ -161,23 +161,25 @@ async function getFollows(refresh: boolean): Promise { // A walk that bypassed the cache also satisfies an asker that would have // accepted a cached answer; the reverse is not true. const inFlight = followWalks.get(did) - if (inFlight && !refresh) return { did, dids: await inFlight } + if (inFlight && !refresh) return { did, ...(await inFlight) } - const work = cached( + const work = cached( { scope: 'graph', subject: did, name: 'follows' }, async () => { - const set = await followingDids(did) - // The walk stops at listRecords' 2000-record cap, which is invisible - // from its result: a count here is what makes a heavy follower's - // missing answer explicable afterwards (see TODO.md). - console.debug(`[substandard] read ${set.size} follow records for ${did}`) - return [...set] + const list = await followingDids(did) + // What the cap did, rather than only what the walk found: past it the + // set is a prefix, and the cards read absence from it. + console.debug( + `[substandard] read ${list.dids.length} follow records for ${did}`, + list.truncated ? `(stopped at the ${FOLLOWS_CAP} cap)` : '', + ) + return list }, { refresh }, ) followWalks.set(did, work) try { - return { did, dids: await work } + return { did, ...(await work) } } finally { if (followWalks.get(did) === work) followWalks.delete(did) } diff --git a/src/lib/atproto.ts b/src/lib/atproto.ts index a7e3783..1bb5c2b 100644 --- a/src/lib/atproto.ts +++ b/src/lib/atproto.ts @@ -201,11 +201,27 @@ export interface ListedRecord { value: T } -/** List every record in a collection (public, paginated). */ +/** + * Where a walk stops when the caller does not say. A backstop against a repo + * large enough to page forever, not a considered limit for any one collection: + * a caller that knows how big its collection gets, and what it costs to stop + * short, passes its own. + */ +export const LIST_LIMIT = 2000 + +/** + * List every record in a collection (public, paginated), up to `limit`. + * + * A result of exactly `limit` may or may not be the whole collection — the + * walk stops without asking whether there was more. Callers that care read + * `records.length >= limit` as "possibly incomplete", which is wrong only for + * a collection whose size is exactly the cap. + */ export async function listRecords( pds: string, did: string, collection: string, + limit: number = LIST_LIMIT, ): Promise[]> { const out: ListedRecord[] = [] let cursor: string | undefined @@ -220,7 +236,7 @@ export async function listRecords( ) out.push(...page.records) cursor = page.cursor - } while (cursor && out.length < 2000) + } while (cursor && out.length < limit) return out } diff --git a/src/lib/profile.ts b/src/lib/profile.ts index 1eaa753..411b039 100644 --- a/src/lib/profile.ts +++ b/src/lib/profile.ts @@ -120,7 +120,7 @@ export function ownerSubtitle(owner: Owner): string | undefined { /** * Whether the card says the reader follows this account. * - * Three ways the answer is no, and only one of them is "you do not follow + * Four ways the answer is no, and only one of them is "you do not follow * them": * * - A blocked account never gets the line. Blocking somebody does not delete @@ -133,6 +133,10 @@ export function ownerSubtitle(owner: Owner): string | undefined { * evidence of not following, and the line is only ever drawn as a fact. * - The reader's own publication. Nobody follows themselves, and the line * would read as a broken one rather than as an answer. + * - A follow set that stopped at its cap (`viewer.truncated`). Absence from + * a prefix is not absence. Nothing extra is needed to handle it — a hit is + * still a hit, and a miss already draws nothing — but it is the reason the + * missing line must never be read as "not following". */ export function showsFollowing(owner: Owner, blocked: boolean, viewer?: Viewer): boolean { if (blocked || !viewer || viewer.did === owner.did) return false diff --git a/src/lib/subscribers.test.ts b/src/lib/subscribers.test.ts index 2bdfc8e..115b0cf 100644 --- a/src/lib/subscribers.test.ts +++ b/src/lib/subscribers.test.ts @@ -6,6 +6,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { CONSTELLATION, + FOLLOWS_CAP, distinctDidsUrl, followedSubscribers, followingDids, @@ -116,9 +117,35 @@ describe('distinctDidsUrl', () => { describe('followingDids', () => { it('reads the viewer’s own follow records, not an appview', async () => { const calls = stubNetwork({ byTarget: {}, follows: [FRIEND] }) - expect(await followingDids(ME)).toEqual(new Set([FRIEND])) + expect(await followingDids(ME)).toEqual({ dids: [FRIEND], truncated: false }) expect(calls.some((u) => u.includes('bsky.app'))).toBe(false) }) + + it('says so when the walk stopped at the cap, so absence is not read as a no', async () => { + // A repo that always has another page: the walk stops on the cap, not on + // running out, which is exactly the case that used to be invisible. + vi.stubGlobal( + 'fetch', + vi.fn(async (input: string) => { + const url = String(input) + const identity = identityRoutes(url) + if (identity) return identity + return new Response( + JSON.stringify({ + records: Array.from({ length: 100 }, (_, i) => ({ + uri: `at://${ME}/app.bsky.graph.follow/${i}`, + cid: 'bafcid', + value: { subject: `did:plc:followed${i}` }, + })), + cursor: 'more', + }), + ) + }), + ) + const list = await followingDids(ME) + expect(list.truncated).toBe(true) + expect(list.dids.length).toBeLessThanOrEqual(FOLLOWS_CAP) + }) }) describe('scanSubscribers', () => { diff --git a/src/lib/subscribers.ts b/src/lib/subscribers.ts index 0d9c829..8b9e924 100644 --- a/src/lib/subscribers.ts +++ b/src/lib/subscribers.ts @@ -52,6 +52,13 @@ export interface SubscriberSummary { followedTotal: number /** True when there were more subscribers than SCAN_CAP left to look through. */ truncated: boolean + /** + * True when the viewer's own follow set stopped at FOLLOWS_CAP. The other + * end of the same intersection: this caps who could be recognised, rather + * than who was looked at. Optional because entries cached before it existed + * do not carry it. + */ + followsTruncated?: boolean } /** @@ -122,18 +129,44 @@ async function subscriberDids( return { dids: dids.slice(0, cap), total, truncated: !!cursor && dids.length >= cap } } +/** + * How many follow records one walk will read. A hundred per request, so this + * is a hundred requests against the account's own PDS, once a day, in the + * background worker (src/background.ts). + * + * The number is set by how long that worker can be relied on to live, not by + * politeness to the PDS: an MV3 service worker stays up while its event is + * being handled, and a walk that runs past that is killed with nothing cached + * and nothing to show for it. Ten thousand covers all but a small tail of + * accounts inside a wait that comfortably fits. Going further wants a walk + * that can resume from a cursor rather than a bigger number here (see + * TODO.md). + */ +export const FOLLOWS_CAP = 10_000 + +export interface FollowList { + dids: string[] + /** The walk stopped at the cap: this is a prefix of the follows, not all of them. */ + truncated: boolean +} + /** * The DIDs the account follows, read from its own repo rather than from an - * appview: the follows are its records, and listRecords already caps the walk. - * An account following more than that cap keeps the faces it has; the row - * does not claim to be exhaustive. + * appview. `truncated` is the part that used to be invisible: past the cap the + * set is a prefix, and a caller that reads absence from it — the owner card's + * "Following" line — would otherwise turn "we stopped looking" into "no". */ -export async function followingDids(did: string): Promise> { +export async function followingDids(did: string): Promise { const { pds } = await resolveDid(did) - const records = await listRecords<{ subject?: string }>(pds, did, 'app.bsky.graph.follow') - const out = new Set() - for (const r of records) if (r.value.subject) out.add(r.value.subject) - return out + const records = await listRecords<{ subject?: string }>( + pds, + did, + 'app.bsky.graph.follow', + FOLLOWS_CAP, + ) + const dids = new Set() + for (const r of records) if (r.value.subject) dids.add(r.value.subject) + return { dids: [...dids], truncated: records.length >= FOLLOWS_CAP } } /** Handle and avatar for a face, from that account's own PDS. Never throws. */ @@ -155,6 +188,11 @@ export interface Viewer { * popup can cache it for the session (see loadSubscribers in popup.ts). */ following: Set + /** + * The set stopped at FOLLOWS_CAP, so it is a prefix. Absence from it means + * "not in the part we read", which is not the same as "not followed". + */ + truncated?: boolean } /** diff --git a/src/lib/types.ts b/src/lib/types.ts index 73382ee..881a827 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -98,6 +98,8 @@ export interface FollowSet { /** Whose follows these are. The account can change between ask and answer. */ did: string dids: string[] + /** The walk stopped at its cap, so absence from `dids` is not a "no". */ + truncated: boolean } /** Messages between content script / popup and the background worker. */ diff --git a/src/popup/cards/follows.test.ts b/src/popup/cards/follows.test.ts index ce4cf73..ccb74f4 100644 --- a/src/popup/cards/follows.test.ts +++ b/src/popup/cards/follows.test.ts @@ -22,7 +22,7 @@ function hostFor(did?: string): CardHost { } /** What the worker answers with, for whoever it finds in the session mirror. */ -const answer = (did: string, ...dids: string[]): FollowSet => ({ did, dids }) +const answer = (did: string, ...dids: string[]): FollowSet => ({ did, dids, truncated: false }) /** A reply that does not settle until the test says so. */ function deferred() { diff --git a/src/popup/cards/follows.ts b/src/popup/cards/follows.ts index d455399..ca52c29 100644 --- a/src/popup/cards/follows.ts +++ b/src/popup/cards/follows.ts @@ -54,7 +54,7 @@ async function read(did: string, refresh: boolean): Promise // Labelled with the account the worker answered for, not the one this // asked about: an account switch between the two makes them different, and // every caller checks the label before drawing anything from the set. - return { did: set.did, following: new Set(set.dids) } + return { did: set.did, following: new Set(set.dids), truncated: set.truncated } } catch (err) { // The subscriber count survives without the faces, and the owner card // survives without the line; see summarizeSubscribers. diff --git a/src/popup/cards/subscribers.test.ts b/src/popup/cards/subscribers.test.ts index 44f98ff..d64f51f 100644 --- a/src/popup/cards/subscribers.test.ts +++ b/src/popup/cards/subscribers.test.ts @@ -17,6 +17,7 @@ const put = vi.fn() vi.mock('../../lib/subscribers', () => ({ SCAN_CAP: 500, + FOLLOWS_CAP: 10_000, scanSubscribers: (...a: unknown[]) => scanSubscribers(...a), followedSubscribers: (...a: unknown[]) => followedSubscribers(...a), })) @@ -31,7 +32,7 @@ vi.mock('../../lib/cache', () => ({ vi.mock('../dom', () => ({ $: () => ({}) })) vi.mock('../../lib/atproto', () => ({ bskyProfileUrl: (h: string) => `https://bsky.app/${h}` })) -const { subscribersCard } = await import('./subscribers') +const { facesCaveat, subscribersCard } = await import('./subscribers') const PUB = 'at://did:plc:pub00000000000000000000/site.standard.publication/self' const ME = 'did:plc:viewer000000000000000000' @@ -102,6 +103,12 @@ describe('the subscribers card', () => { expect(followedSubscribers).not.toHaveBeenCalled() }) + it('carries the follow set’s own cap into the summary', async () => { + viewerFollows.mockResolvedValue({ did: ME, following: new Set([FRIEND]), truncated: true }) + await subscribersCard.load?.(hostFor(ME), false) + expect(put.mock.calls[0]?.[1]).toMatchObject({ followsTruncated: true }) + }) + it('serves a stored summary without touching the index', async () => { peek.mockResolvedValue({ total: 9, followed: [], followedTotal: 0, truncated: false }) await subscribersCard.load?.(hostFor(ME), false) @@ -109,3 +116,21 @@ describe('the subscribers card', () => { expect(draws).toHaveLength(1) }) }) + +describe('facesCaveat', () => { + const summary = { total: 9, followed: [], followedTotal: 1, truncated: false } + + it('says nothing when neither cap bit', () => { + expect(facesCaveat(summary)).toBe('') + }) + + it('names both windows a face can be missing from', () => { + const both = facesCaveat({ ...summary, truncated: true, followsTruncated: true }) + expect(both).toContain('500 subscribers') + expect(both).toContain(`${(10_000).toLocaleString()} accounts you follow`) + }) + + it('reads an entry cached before the follow cap existed as untruncated', () => { + expect(facesCaveat({ ...summary, truncated: true })).not.toContain('you follow') + }) +}) diff --git a/src/popup/cards/subscribers.ts b/src/popup/cards/subscribers.ts index 96d7de2..43a34fe 100644 --- a/src/popup/cards/subscribers.ts +++ b/src/popup/cards/subscribers.ts @@ -9,6 +9,7 @@ import { bskyProfileUrl } from '../../lib/atproto' import { peek, put } from '../../lib/cache' import { + FOLLOWS_CAP, type FollowedSubscriber, SCAN_CAP, type SubscriberSummary, @@ -68,6 +69,7 @@ export const subscribersCard: Card = { total: scan.total, ...(await followedSubscribers(scan, viewer)), truncated: scan.truncated, + followsTruncated: viewer.truncated, } // Not `cached`: a summary that could not be built is a hidden row, and a // hidden row is not an answer worth remembering for a day. @@ -90,8 +92,9 @@ export const subscribersCard: Card = { parts.push(summary.total === 1 ? '1 subscriber' : `${summary.total} subscribers`) const count = $('subscriber-count') count.textContent = parts.join(' · ') - // The count is exact whatever happened; only the faces come from a window. - count.title = summary.truncated ? `Faces are from the first ${SCAN_CAP} subscribers.` : '' + // The count is exact whatever happened; only the faces come from a window, + // and there are two windows they can be missing from. + count.title = facesCaveat(summary) row.hidden = false }, @@ -103,6 +106,19 @@ export const subscribersCard: Card = { }, } +/** + * Why a face the reader expected might not be here. Either cap can do it: the + * subscribers we looked at, and the follows we could recognise them from. + */ +export function facesCaveat(summary: SubscriberSummary): string { + const parts: string[] = [] + if (summary.truncated) parts.push(`Faces are from the first ${SCAN_CAP} subscribers.`) + if (summary.followsTruncated) { + parts.push(`Matched against the first ${FOLLOWS_CAP.toLocaleString()} accounts you follow.`) + } + return parts.join(' ') +} + /** Hold an answer and redraw. Called twice per load: the count, then the faces. */ function show(host: CardHost, uri: string, summary: SubscriberSummary): void { subscribers = { uri, summary }