fix: reduce cold server detail reads (#202)

Author: gusmar2017

apps/web/lib/queries-reliability.test.ts

@@ -1,4 +1,4 @@
-import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 type Result = { data: unknown; count?: number; error?: { message: string; code?: string } | null };
 const state = vi.hoisted(() => ({
@@ -36,11 +36,16 @@ vi.mock('./supabase', () => ({ supabase: {
 import { getServerBySlug, listServers, getServersSitemapPage, __resetReadmeLengthProbe } from './queries';
 const outage: Result = { data: null, error: { message: 'timeout', code: '57014' } };
 const server = { id: 'one', slug: 'one', canonical_slug: 'one', registry_status: 'active' };
+let errorLog: ReturnType<typeof vi.spyOn>;
+let warnLog: ReturnType<typeof vi.spyOn>;
 
 beforeEach(() => {
   state.results = []; state.signals = []; state.reads = 0; state.cache.clear();
   __resetReadmeLengthProbe();
+  errorLog = vi.spyOn(console, 'error').mockImplementation(() => undefined);
+  warnLog = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
 });
+afterEach(() => vi.restoreAllMocks());
 
 describe('directory failures do not become persistent content', () => {
   it('fails a canonical lookup promptly without hiding its error behind legacy lookup, then recovers', async () => {
@@ -58,6 +63,24 @@ describe('directory failures do not become persistent content', () => {
     expect(state.signals).toHaveLength(3);
     expect(new Set(state.signals).size).toBe(1);
   });
+  it('skips the tools read only when both authoritative fields prove there are none', async () => {
+    const noTools = { ...server, has_tools: false, tool_count: 0 };
+    state.results.push({ data: noTools });
+    expect(await getServerBySlug('no-tools')).toMatchObject({ ...noTools, tools: [] });
+    expect(state.reads).toBe(1);
+    expect(await getServerBySlug('no-tools')).toMatchObject({ tools: [] });
+    expect(state.reads).toBe(1);
+  });
+  it.each([
+    { has_tools: true, tool_count: 0 },
+    { has_tools: false, tool_count: 1 },
+    { has_tools: undefined, tool_count: undefined },
+  ])('queries tools when tool state is positive or unknown: %o', async toolState => {
+    const slug = `state-${String(toolState.has_tools)}-${String(toolState.tool_count)}`;
+    state.results.push({ data: { ...server, ...toolState } }, { data: [{ id: 1 }] });
+    expect((await getServerBySlug(slug))?.tools).toHaveLength(1);
+    expect(state.reads).toBe(2);
+  });
   it('only treats two successful absent lookups as missing', async () => {
     state.results.push({ data: null }, outage);
     await expect(getServerBySlug('missing')).rejects.toThrow();
@@ -67,9 +90,33 @@ describe('directory failures do not become persistent content', () => {
   it('does not cache lost tool documentation as an empty tools list', async () => {
     state.results.push({ data: server }, outage);
     await expect(getServerBySlug('one')).rejects.toThrow();
+    expect(errorLog).toHaveBeenCalledWith(
+      '[queries] server detail query failed',
+      expect.objectContaining({ event: 'server_detail_query_error', stage: 'tools' })
+    );
     state.results.push({ data: server }, { data: [{ name: 'query' }] });
     expect((await getServerBySlug('one'))?.tools).toHaveLength(1);
   });
+  it('logs a structured failing stage without upstream messages or identifiers', async () => {
+    state.results.push({ data: null, error: { message: 'secret upstream URL', code: '57014' } });
+    await expect(getServerBySlug('private-request-slug')).rejects.toThrow('temporarily unavailable');
+    expect(errorLog).toHaveBeenCalledWith(
+      '[queries] server detail query failed',
+      expect.objectContaining({
+        event: 'server_detail_query_error', stage: 'canonical', error_code: '57014',
+      })
+    );
+    expect(JSON.stringify(errorLog.mock.calls)).not.toContain('secret upstream URL');
+    expect(JSON.stringify(errorLog.mock.calls)).not.toContain('private-request-slug');
+  });
+  it('logs a structured slow stage with duration', async () => {
+    vi.spyOn(performance, 'now').mockReturnValueOnce(0).mockReturnValueOnce(1001);
+    state.results.push({ data: { ...server, has_tools: false, tool_count: 0 } });
+    await getServerBySlug('slow');
+    expect(warnLog).toHaveBeenCalledWith('[queries] server detail query slow', {
+      event: 'server_detail_query_slow', stage: 'canonical', duration_ms: 1001,
+    });
+  });
   it('does not cache listing with a failed count as a successful zero result', async () => {
     state.results.push(outage, { data: [server] });
     await expect(listServers({})).rejects.toThrow();

apps/web/lib/queries.ts

@@ -12,6 +12,54 @@ import { normalizeListParams } from './filter-utils';
 
 // All sequential detail reads share one deadline; leave room for rendering under 15s.
 const QUERY_TIMEOUT_MS = 6000;
+const DETAIL_QUERY_SLOW_MS = 1000;
+
+type DetailQueryStage = 'canonical' | 'slug_fallback' | 'tools';
+type ObservedQueryResult = { error?: { code?: string; message?: string } | null };
+
+/**
+ * Records only operational metadata. Deliberately excludes the requested slug,
+ * query URL, response data and upstream error message so logs cannot capture
+ * credentials, README content, or identifiers from request paths.
+ */
+async function observeDetailQuery<T extends ObservedQueryResult>(
+  stage: DetailQueryStage,
+  signal: AbortSignal,
+  query: PromiseLike<T>
+): Promise<T> {
+  const startedAt = performance.now();
+  try {
+    const result = await query;
+    const durationMs = Math.round(performance.now() - startedAt);
+    if (result.error) {
+      console.error('[queries] server detail query failed', {
+        event: 'server_detail_query_error',
+        stage,
+        duration_ms: durationMs,
+        error_code: result.error.code ?? 'unknown',
+        aborted: signal.aborted,
+      });
+    } else if (durationMs >= DETAIL_QUERY_SLOW_MS) {
+      console.warn('[queries] server detail query slow', {
+        event: 'server_detail_query_slow',
+        stage,
+        duration_ms: durationMs,
+      });
+    }
+    return result;
+  } catch (error) {
+    const durationMs = Math.round(performance.now() - startedAt);
+    console.error('[queries] server detail query failed', {
+      event: 'server_detail_query_error',
+      stage,
+      duration_ms: durationMs,
+      error_code: error instanceof Error ? error.name : 'unknown',
+      aborted: signal.aborted,
+    });
+    throw error;
+  }
+}
+
 function assertAvailable(error: { message?: string } | null | undefined): void {
   if (error) throw new Error('Directory temporarily unavailable');
 }
@@ -192,42 +240,58 @@ async function _getServerBySlug(slug: string): Promise<ServerWithTools | null> {
   // Try canonical_slug first (populated after migration 005 runs).
   // If no match, fall back to the mutable slug column (pre-migration or community servers).
   // The same signal covers all lookups, including the legacy fallback.
-  let { data: server, error } = await supabase
-    .from('servers')
-    .select(SERVER_DETAIL_COLUMNS)
-    .eq('canonical_slug', slug)
-    .abortSignal(signal)
-    .maybeSingle();
+  let { data: server, error } = await observeDetailQuery(
+    'canonical',
+    signal,
+    supabase
+      .from('servers')
+      .select(SERVER_DETAIL_COLUMNS)
+      .eq('canonical_slug', slug)
+      .abortSignal(signal)
+      .maybeSingle()
+  );
 
   assertAvailable(error);
   if (!server) {
     // Defensive fallback: resolve by the mutable slug column.
     // This path is hit before migration 005 is applied, or for rows where
     // canonical_slug has not yet been backfilled.
-    const result = await supabase
-      .from('servers')
-      .select(SERVER_DETAIL_COLUMNS)
-      .eq('slug', slug)
-      .abortSignal(signal)
-      .maybeSingle();
+    const result = await observeDetailQuery(
+      'slug_fallback',
+      signal,
+      supabase
+        .from('servers')
+        .select(SERVER_DETAIL_COLUMNS)
+        .eq('slug', slug)
+        .abortSignal(signal)
+        .maybeSingle()
+    );
     server = result.data;
     error = result.error;
   }
 
   assertAvailable(error);
   if (!server) return null;
 
-  // Skip the tools fetch for deprecated rows — the page will call notFound() immediately,
-  // so the tools data is never used. Return early with an empty tools array.
-  if (server.registry_status === 'deprecated') {
+  // Skip the tools fetch when authoritative row fields prove it cannot return
+  // anything. The exact comparisons are deliberate: legacy/unknown states
+  // still query server_tools and therefore cannot hide tool documentation.
+  if (
+    server.registry_status === 'deprecated' ||
+    (server.has_tools === false && server.tool_count === 0)
+  ) {
     return { ...server, tools: [] } as ServerWithTools;
   }
 
-  const { data: tools, error: toolsError } = await supabase
-    .from('server_tools')
-    .select('*')
-    .eq('server_id', server.id)
-    .abortSignal(signal);
+  const { data: tools, error: toolsError } = await observeDetailQuery(
+    'tools',
+    signal,
+    supabase
+      .from('server_tools')
+      .select('*')
+      .eq('server_id', server.id)
+      .abortSignal(signal)
+  );
 
   assertAvailable(toolsError);
   return { ...server, tools: tools || [] } as ServerWithTools;

supabase/migrations/010_readme_length_generated_column.sql

@@ -55,9 +55,10 @@
 --
 -- APPLYING
 -- --------
--- NOT APPLIED as of this commit. Adding a STORED generated column rewrites
--- the table, which will detoast and re-read every README once — a one-time
--- cost, but a real one on the current instance. Apply it in a quiet window.
+-- Applied to the production mcp-find project on 2026-09-11 through the linked
+-- Supabase CLI. The pre-apply JS signal count and post-apply generated-column
+-- count both returned 1,429 active rows; the boundary parity check inspected
+-- 3 rows and found 0 eligibility disagreements. IF NOT EXISTS keeps reruns safe.
 --
 -- Deploy order is NOT load-bearing: apps/web/lib/queries.ts probes for this
 -- column and falls back to selecting readme_content (the old behaviour, and