Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions __tests__/unit/services/current-user-id-cache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/**
* Learning who is reading costs a network call, so do it once.
*
* `supabase.auth.getUser()` validates the token against `/auth/v1/user` — it is
* not a local read. Enrichment calls it once per pass, and enrichment runs once
* per node while a reply tree is built, so opening a thread fired one
* round-trip per reply just to re-learn the same id. Production 2026-08-28:
* eight `/auth/v1/user` calls to open one post.
*
* The concurrency case matters as much as the repeat case: enrichment is
* deliberately parallel, so several callers arrive together and must collapse
* onto one request rather than racing to make several.
*/

import { getCurrentUserId, __resetCurrentUserIdCache } from '@/services/timeline/processors/social-shared';

const getUser = jest.fn();
jest.mock('@/lib/supabase/browser', () => ({
__esModule: true,
default: { auth: { getUser: (...a: unknown[]) => getUser(...a) } },
}));

describe('getCurrentUserId', () => {
beforeEach(() => {
jest.clearAllMocks();
__resetCurrentUserIdCache();
getUser.mockResolvedValue({ data: { user: { id: 'u1' } } });
});

it('asks the network once, however many times it is called', async () => {
expect(await getCurrentUserId()).toBe('u1');
expect(await getCurrentUserId()).toBe('u1');
expect(await getCurrentUserId()).toBe('u1');

expect(getUser).toHaveBeenCalledTimes(1);
});

it('collapses concurrent callers onto one request', async () => {
const [a, b, c] = await Promise.all([
getCurrentUserId(),
getCurrentUserId(),
getCurrentUserId(),
]);

expect([a, b, c]).toEqual(['u1', 'u1', 'u1']);
expect(getUser).toHaveBeenCalledTimes(1);
});

it('does not cache a failure as "signed out"', async () => {
__resetCurrentUserIdCache();
getUser.mockRejectedValueOnce(new Error('network blip'));

expect(await getCurrentUserId()).toBeNull();

// A blip must not outlive itself and render the whole timeline as
// signed-out for the rest of the page.
getUser.mockResolvedValue({ data: { user: { id: 'u1' } } });
expect(await getCurrentUserId()).toBe('u1');
});

it('caches a genuine signed-out answer', async () => {
__resetCurrentUserIdCache();
getUser.mockResolvedValue({ data: { user: null } });

expect(await getCurrentUserId()).toBeNull();
expect(await getCurrentUserId()).toBeNull();

expect(getUser).toHaveBeenCalledTimes(1);
});
});
142 changes: 142 additions & 0 deletions __tests__/unit/services/reply-tree-query-count.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/**
* Opening a thread must not cost a round-trip per reply.
*
* `getReplies` recursed: one query AND one `enrichEventsForDisplay` per node.
* Enrichment is itself several requests — profiles, projects, the reader's id,
* and the three reaction tables — so a thread cost roughly six round-trips per
* reply. Measured in production 2026-08-28 by opening one post: eight
* `/auth/v1/user` calls alone, for a thread with three replies.
*
* The cost is what these tests hold. Asserting only "the tree is correct" is
* what let the shape regress in the first place — the recursive version built a
* perfectly correct tree.
*/

import { getReplies } from '@/services/timeline/queries/eventQueries';

const from = jest.fn();
const enrich = jest.fn();

jest.mock('@/lib/supabase/browser', () => ({
__esModule: true,
default: { from: (...a: unknown[]) => from(...a) },
}));
jest.mock('@/services/timeline/processors/enrichment', () => ({
enrichEventsForDisplay: (...a: unknown[]) => enrich(...a),
}));
jest.mock('@/services/timeline/processors/reaction-state', () => ({
attachReactionState: async <T,>(x: T) => x,
EMPTY_REACTION_STATE: {},
}));
jest.mock('@/lib/supabase/untyped', () => ({ callRpc: jest.fn() }));

/**
* A fake table that actually honours the parent filter.
*
* An argument-blind mock returns the same rows whatever is asked for, which
* makes "one query per level" and "one query per node" indistinguishable — the
* first version of this file had one, and a mutation replacing `.in(parents)`
* with `.eq(parents[0])` stayed green. The filter has to be real for the count
* assertions below to mean anything.
*/
function tableOf(rows: Array<Record<string, unknown>>) {
from.mockImplementation(() => {
let parents: string[] = [];
const chain: Record<string, unknown> = {};
for (const m of ['select', 'eq', 'order']) {
chain[m] = (col?: string, val?: unknown) => {
if (col === 'parent_event_id') {
parents = [String(val)];
}
return chain;
};
}
chain.in = (col: string, vals: string[]) => {
if (col === 'parent_event_id') {
parents = vals;
}
return chain;
};
chain.limit = () =>
Promise.resolve({
data: rows.filter(r => parents.includes(String(r.parent_event_id))),
error: null,
});
return chain;
});
}

describe('getReplies query cost', () => {
beforeEach(() => {
jest.clearAllMocks();
// Enrichment echoes its input, mapped to the display shape the tree needs.
enrich.mockImplementation(async (rows: Array<Record<string, unknown>>) =>
rows.map(r => ({ id: r.id, parentEventId: r.parent_event_id }))
);
});

it('asks once per DEPTH, not once per reply', async () => {
// Three replies to the root, none nested.
tableOf([
{ id: 'r1', parent_event_id: 'root' },
{ id: 'r2', parent_event_id: 'root' },
{ id: 'r3', parent_event_id: 'root' },
]);

const result = await getReplies('root');

expect(result.success).toBe(true);
expect(result.replies).toHaveLength(3);
// Two queries: the level with replies, and the one that came back empty.
// The recursive version made four for this shape, and one more for every
// additional reply.
expect(from).toHaveBeenCalledTimes(2);
});

it('asks for every sibling in ONE query, not one per sibling', async () => {
// Two branches, each with a child. A per-node fetch would either take four
// queries or — asking only about the first parent — silently lose r2's
// child, so this pins both the count and the completeness.
tableOf([
{ id: 'r1', parent_event_id: 'root' },
{ id: 'r2', parent_event_id: 'root' },
{ id: 'r1a', parent_event_id: 'r1' },
{ id: 'r2a', parent_event_id: 'r2' },
]);

const result = await getReplies('root');

// depth 0 (root), depth 1 (r1+r2 together), depth 2 (empty) = 3.
expect(from).toHaveBeenCalledTimes(3);
expect(result.replies?.map(r => r.id)).toEqual(['r1', 'r2']);
expect(result.replies?.[0].replies?.[0].id).toBe('r1a');
expect(result.replies?.[1].replies?.[0].id).toBe('r2a');
});

it('enriches the whole tree exactly once', async () => {
tableOf([
{ id: 'r1', parent_event_id: 'root' },
{ id: 'r2', parent_event_id: 'root' },
{ id: 'r3', parent_event_id: 'r1' },
]);

await getReplies('root');

expect(enrich).toHaveBeenCalledTimes(1);
// And it got every node, so nothing is left un-enriched.
expect(enrich.mock.calls[0][0]).toHaveLength(3);
});

it('still nests replies under the right parent', async () => {
tableOf([
{ id: 'r1', parent_event_id: 'root' },
{ id: 'r2', parent_event_id: 'r1' },
]);

const result = await getReplies('root');

expect(result.replies?.[0].id).toBe('r1');
expect(result.replies?.[0].replies?.[0].id).toBe('r2');
expect(result.replies?.[0].replyCount).toBe(1);
});
});
54 changes: 46 additions & 8 deletions src/services/timeline/processors/social-shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,52 @@ import { logger } from '@/utils/logger';
// and custom RPCs (like/unlike/comment) are also absent — cast required.
export const db = supabase as any;

/**
* Who is reading, cached for the page.
*
* `supabase.auth.getUser()` is a NETWORK call — it validates the token against
* `/auth/v1/user`. This is called once per enrichment pass, and enrichment runs
* once per node while a reply tree is built, so opening a thread fired one
* round-trip per reply just to re-learn the same id. Measured on a three-reply
* thread: eight `/auth/v1/user` calls for one page.
*
* Cached as the in-flight PROMISE, not the value, so concurrent callers — and
* enrichment is deliberately concurrent — collapse onto one request instead of
* racing to make several. Only a resolved id is kept; a failure is not cached,
* because caching "nobody is signed in" would outlive the blip that caused it
* and quietly render the whole timeline as signed-out.
*
* The id cannot change without a sign-in or sign-out, and both replace the
* page, so a page-lifetime cache is the correct scope.
*/
let cachedUserId: string | null | undefined;
let userIdInFlight: Promise<string | null> | null = null;

export async function getCurrentUserId(): Promise<string | null> {
try {
const {
data: { user },
} = await supabase.auth.getUser();
return user?.id || null;
} catch (error) {
logger.error('Error getting current user ID', error, 'Timeline');
return null;
if (cachedUserId !== undefined) {
return cachedUserId;
}
if (!userIdInFlight) {
userIdInFlight = (async () => {
try {
const {
data: { user },
} = await supabase.auth.getUser();
cachedUserId = user?.id || null;
return cachedUserId;
} catch (error) {
logger.error('Error getting current user ID', error, 'Timeline');
return null;
} finally {
userIdInFlight = null;
}
})();
}
return userIdInFlight;
}

/** Test seam, and the hook a sign-out would use if this ever needs clearing. */
export function __resetCurrentUserIdCache(): void {
cachedUserId = undefined;
userIdInFlight = null;
}
64 changes: 42 additions & 22 deletions src/services/timeline/queries/eventQueries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,42 +74,62 @@ export async function getReplies(
limit: number = 50
): Promise<{ success: boolean; replies?: TimelineDisplayEvent[]; error?: string }> {
try {
const buildTree = async (parentId: string, depth: number): Promise<TimelineDisplayEvent[]> => {
// Limit depth to avoid accidental cycles
if (depth > 3) {
return [];
}
// Fetch the tree a LEVEL at a time, then enrich the whole thing once.
//
// This used to recurse per node: one query and one enrichEventsForDisplay
// per reply. Enrichment is itself several round-trips — profiles, projects,
// the reader's id, and the three reaction tables — so a thread cost roughly
// six requests per reply, and opening a three-reply thread fired eight
// `/auth/v1/user` calls alone. Level-order asks once per depth regardless of
// width, and enriches once regardless of both.
const MAX_DEPTH = 3;
const levels: Array<Record<string, unknown>>[] = [];
let frontier = [eventId];

for (let depth = 0; depth <= MAX_DEPTH && frontier.length > 0; depth++) {
const { data: childEvents, error } = await supabase
.from(TIMELINE_TABLES.EVENTS)
.select('*')
.eq('parent_event_id', parentId)
.in('parent_event_id', frontier)
.eq('is_deleted', false)
.order('created_at', { ascending: true })
.limit(depth === 0 ? limit : 50);
.limit(depth === 0 ? limit : 200);

if (error) {
logger.error('Error fetching replies', error, 'Timeline');
return [];
break;
}

const rows = (childEvents || []) as Array<Record<string, unknown>>;
if (rows.length === 0) {
break;
}
levels.push(rows);
frontier = rows.map(row => String(row.id));
}

const enriched = await enrichEventsForDisplay(levels.flat());

const enrichedChildren = await enrichEventsForDisplay(childEvents || []);
// Assemble parent → children from the flat list. Every node is visited
// once, and a reply whose parent did not come back (deleted mid-read)
// simply does not attach, rather than orphaning the whole branch.
const byParent = new Map<string, TimelineDisplayEvent[]>();
for (const reply of enriched) {
const parentId = reply.parentEventId ?? '';
byParent.set(parentId, [...(byParent.get(parentId) ?? []), reply]);
}

// Recursively fetch children for each reply, in parallel — a sequential
// loop here made thread latency grow linearly with reply count
return Promise.all(
enrichedChildren.map(async reply => {
const nestedReplies = await buildTree(reply.id, depth + 1);
return {
...reply,
replies: nestedReplies,
replyCount: nestedReplies.length,
};
})
);
const attach = (parentId: string, depth: number): TimelineDisplayEvent[] => {
if (depth > MAX_DEPTH) {
return [];
}
return (byParent.get(parentId) ?? []).map(reply => {
const nested = attach(reply.id, depth + 1);
return { ...reply, replies: nested, replyCount: nested.length };
});
};

const replies = await buildTree(eventId, 0);
const replies = attach(eventId, 0);
return { success: true, replies };
} catch (error) {
logger.error('Error fetching replies', error, 'Timeline');
Expand Down
Loading