fix(api): hoists fork types, removes dead code, adds missing tests

Author: wgordon17

src/app/services/api.ts

@@ -272,6 +272,22 @@ interface GraphQLPRSearchResponse {
   rateLimit?: { remaining: number; resetAt: string };
 }
 
+interface ForkCandidate {
+  pr: PullRequest;
+  headOwner: string;
+  headRepo: string;
+  sha: string;
+}
+
+interface ForkRepoResult {
+  object: { statusCheckRollup: { state: string } | null } | null;
+}
+
+interface ForkQueryResponse {
+  rateLimit?: { remaining: number; resetAt: string };
+  [key: string]: ForkRepoResult | { remaining: number; resetAt: string } | undefined | null;
+}
+
 // ── GraphQL search query constants ───────────────────────────────────────────
 
 const ISSUES_SEARCH_QUERY = `
@@ -626,12 +642,6 @@ async function graphqlSearchPRs(
   // differs from base repo owner, query the head repo's commit statusCheckRollup.
   // GitHub copies fork PR commits into base repo (refs/pull/N/head), so most PRs
   // resolve via the base repo. The fallback handles cases where CI runs only on the fork.
-  interface ForkCandidate {
-    pr: PullRequest;
-    headOwner: string;
-    headRepo: string;
-    sha: string;
-  }
   const forkCandidates: ForkCandidate[] = [];
 
   for (const [databaseId, pr] of prMap) {
@@ -669,14 +679,6 @@ async function graphqlSearchPRs(
       const forkQuery = `query(${varDefs.join(", ")}) {\n${fragments.join("\n")}\nrateLimit { remaining resetAt }\n}`;
 
       try {
-        interface ForkRepoResult {
-          object: { statusCheckRollup: { state: string } | null } | null;
-        }
-        interface ForkQueryResponse {
-          rateLimit?: { remaining: number; resetAt: string };
-          [key: string]: ForkRepoResult | { remaining: number; resetAt: string } | undefined | null;
-        }
-
         const forkResponse = await octokit.graphql<ForkQueryResponse>(forkQuery, variables);
         if (forkResponse.rateLimit) updateGraphqlRateLimit(forkResponse.rateLimit as { remaining: number; resetAt: string });
 

src/app/services/poll.ts

@@ -203,7 +203,7 @@ const REJITTER_WINDOW_MS = 30_000; // ±30 seconds jitter
 const REVISIT_THRESHOLD_MS = 2 * 60 * 1000; // 2 minutes
 
 // Sources managed by the poll coordinator — used for reconciliation
-const POLL_MANAGED_SOURCES = new Set(["poll", "search", "graphql", "rate-limit", "notifications"]);
+const POLL_MANAGED_SOURCES = new Set(["poll", "graphql", "rate-limit", "notifications"]);
 
 function withJitter(intervalMs: number): number {
   const jitter = (Math.random() * 2 - 1) * REJITTER_WINDOW_MS;

src/app/stores/cache.ts

@@ -100,30 +100,6 @@ export async function clearCache(): Promise<void> {
   await db.clear("cache");
 }
 
-/**
- * Evicts cache entries whose key starts with `prefix` but is NOT in `keepKeys`.
- * Used to clean up per-PR cache entries for PRs no longer in the active set.
- */
-export async function evictByPrefix(
-  prefix: string,
-  keepKeys: Set<string>
-): Promise<number> {
-  const db = await getDb();
-  const tx = db.transaction("cache", "readwrite");
-  const range = IDBKeyRange.bound(prefix, prefix + "\uffff");
-  let cursor = await tx.store.openCursor(range);
-  let count = 0;
-  while (cursor) {
-    if (!keepKeys.has(cursor.key as string)) {
-      await cursor.delete();
-      count++;
-    }
-    cursor = await cursor.continue();
-  }
-  await tx.done;
-  return count;
-}
-
 export async function evictStaleEntries(maxAgeMs: number): Promise<number> {
   const db = await getDb();
   const tx = db.transaction("cache", "readwrite");

tests/components/layout/Header.test.tsx

@@ -103,6 +103,17 @@ describe("Header", () => {
     vi.mocked(githubService.getCoreRateLimit).mockRestore();
   });
 
+  it("shows GraphQL rate limit with label when available", () => {
+    vi.spyOn(githubService, "getGraphqlRateLimit").mockReturnValue({
+      remaining: 4800,
+      resetAt: new Date("2024-01-10T09:00:00Z"),
+    });
+    render(() => <Header />);
+    screen.getByText(/GraphQL/);
+    screen.getByText(/4800\/5k\/hr/);
+    vi.mocked(githubService.getGraphqlRateLimit).mockRestore();
+  });
+
   it("does not show rate limit when not available", () => {
     render(() => <Header />);
     expect(screen.queryByText(/\/5k\/hr/)).toBeNull();

tests/integration/data-pipeline.test.ts

@@ -32,9 +32,14 @@ const rawRun = {
   head_sha: "abc1234",
   head_branch: "main",
   run_number: 1,
+  run_attempt: 1,
   html_url: "https://github.com/octocat/Hello-World/actions/runs/9001",
   created_at: "2024-01-15T09:00:00Z",
   updated_at: "2024-01-15T09:10:00Z",
+  run_started_at: "2024-01-15T09:00:00Z",
+  completed_at: "2024-01-15T09:10:00Z",
+  display_title: "CI",
+  actor: { login: "octocat", avatar_url: "https://avatars.githubusercontent.com/u/583231?v=4" },
 };
 
 const graphqlIssueNode = {

tests/services/api.test.ts

@@ -453,10 +453,10 @@ const graphqlPRNode = {
   commits: { nodes: [{ commit: { statusCheckRollup: { state: "SUCCESS" } } }] },
 };
 
-function makeGraphqlPRResponse(nodes = [graphqlPRNode], hasNextPage = false) {
+function makeGraphqlPRResponse(nodes = [graphqlPRNode], hasNextPage = false, issueCount?: number) {
   return {
     search: {
-      issueCount: nodes.length,
+      issueCount: issueCount ?? nodes.length,
       pageInfo: { hasNextPage, endCursor: hasNextPage ? "cursor1" : null },
       nodes,
     },
@@ -691,6 +691,32 @@ describe("fetchPullRequests", () => {
     expect(pushNotification).not.toHaveBeenCalled();
   });
 
+  it("caps at 1000 PRs and warns via pushNotification", async () => {
+    vi.mocked(pushNotification).mockClear();
+
+    const manyNodes = Array.from({ length: 1000 }, (_, i) => ({ ...graphqlPRNode, databaseId: i + 1 }));
+    const octokit = makePROctokit(async (_query, variables) => {
+      const q = (variables as Record<string, unknown>).q as string;
+      if (q.includes("involves:")) {
+        return makeGraphqlPRResponse(manyNodes, true, 1500);
+      }
+      return makeGraphqlPRResponse([]);
+    });
+
+    const result = await fetchPullRequests(
+      octokit as unknown as ReturnType<typeof import("../../src/app/services/github").getClient>,
+      [testRepo],
+      "octocat"
+    );
+
+    expect(result.pullRequests.length).toBe(1000);
+    expect(pushNotification).toHaveBeenCalledWith(
+      "search/prs",
+      expect.stringContaining("capped at 1,000"),
+      "warning"
+    );
+  });
+
   it("returns empty result when repos is empty", async () => {
     const octokit = makePROctokit();
     const result = await fetchPullRequests(

tests/services/github.test.ts

@@ -1,7 +1,7 @@
 import "fake-indexeddb/auto";
 import { describe, it, expect, vi, beforeEach } from "vitest";
 import { createRoot } from "solid-js";
-import { createGitHubClient, cachedRequest, getClient, initClientWatcher } from "../../src/app/services/github";
+import { createGitHubClient, cachedRequest, getClient, initClientWatcher, getGraphqlRateLimit, updateGraphqlRateLimit } from "../../src/app/services/github";
 import { clearCache } from "../../src/app/stores/cache";
 
 // ── createGitHubClient ───────────────────────────────────────────────────────
@@ -292,3 +292,31 @@ describe("getClient / initClientWatcher", () => {
     void authModule;
   });
 });
+
+// ── getGraphqlRateLimit / updateGraphqlRateLimit ─────────────────────────────
+
+describe("getGraphqlRateLimit", () => {
+  it("returns null before any update", () => {
+    // May be non-null if a prior test called updateGraphqlRateLimit;
+    // verify the function is callable and returns the expected shape
+    const rl = getGraphqlRateLimit();
+    expect(rl === null || (typeof rl === "object" && "remaining" in rl)).toBe(true);
+  });
+
+  it("converts ISO 8601 resetAt string to Date", () => {
+    const iso = "2024-06-01T12:00:00Z";
+    updateGraphqlRateLimit({ remaining: 4500, resetAt: iso });
+    const rl = getGraphqlRateLimit();
+    expect(rl).not.toBeNull();
+    expect(rl!.remaining).toBe(4500);
+    expect(rl!.resetAt).toBeInstanceOf(Date);
+    expect(rl!.resetAt.getTime()).toBe(new Date(iso).getTime());
+  });
+
+  it("overwrites previous value on subsequent updates", () => {
+    updateGraphqlRateLimit({ remaining: 5000, resetAt: "2024-06-01T12:00:00Z" });
+    updateGraphqlRateLimit({ remaining: 3000, resetAt: "2024-06-01T13:00:00Z" });
+    const rl = getGraphqlRateLimit();
+    expect(rl!.remaining).toBe(3000);
+  });
+});

tests/stores/cache.test.ts

@@ -7,7 +7,6 @@ import {
   deleteCacheEntry,
   clearCache,
   evictStaleEntries,
-  evictByPrefix,
   cachedFetch,
 } from "../../src/app/stores/cache";
 
@@ -203,64 +202,6 @@ describe("evictStaleEntries", () => {
   });
 });
 
-// ── qa-8: evictByPrefix ──────────────────────────────────────────────────────
-
-describe("evictByPrefix", () => {
-  it("deletes prefix entries not in keepKeys, retains kept and non-prefix entries", async () => {
-    // Seed two "pr-detail:" entries and one non-prefixed entry
-    await setCacheEntry("pr-detail:octocat/Hello-World:42", { pr: 42 }, "etag-42");
-    await setCacheEntry("pr-detail:octocat/Hello-World:99", { pr: 99 }, "etag-99");
-    await setCacheEntry("other:key", { other: true }, "etag-other");
-
-    // Keep pr-detail:octocat/Hello-World:42 but evict :99
-    const keepKeys = new Set(["pr-detail:octocat/Hello-World:42"]);
-    const count = await evictByPrefix("pr-detail:", keepKeys);
-
-    // Only :99 should have been evicted
-    expect(count).toBe(1);
-    // Kept prefix entry must still exist
-    expect(await getCacheEntry("pr-detail:octocat/Hello-World:42")).toBeDefined();
-    // Evicted prefix entry must be gone
-    expect(await getCacheEntry("pr-detail:octocat/Hello-World:99")).toBeUndefined();
-    // Non-prefix entry must be untouched
-    expect(await getCacheEntry("other:key")).toBeDefined();
-  });
-
-  it("evicts all prefix entries when keepKeys is empty", async () => {
-    await setCacheEntry("pr-detail:a/b:1", { pr: 1 }, null);
-    await setCacheEntry("pr-detail:a/b:2", { pr: 2 }, null);
-    await setCacheEntry("keep:this", { keep: true }, null);
-
-    const count = await evictByPrefix("pr-detail:", new Set());
-
-    expect(count).toBe(2);
-    expect(await getCacheEntry("pr-detail:a/b:1")).toBeUndefined();
-    expect(await getCacheEntry("pr-detail:a/b:2")).toBeUndefined();
-    expect(await getCacheEntry("keep:this")).toBeDefined();
-  });
-
-  it("returns 0 when no entries match the prefix", async () => {
-    await setCacheEntry("other:entry", { data: true }, null);
-
-    const count = await evictByPrefix("pr-detail:", new Set());
-
-    expect(count).toBe(0);
-    expect(await getCacheEntry("other:entry")).toBeDefined();
-  });
-
-  it("returns 0 when all prefix entries are in keepKeys", async () => {
-    await setCacheEntry("pr-detail:x/y:1", { pr: 1 }, null);
-    await setCacheEntry("pr-detail:x/y:2", { pr: 2 }, null);
-
-    const keepKeys = new Set(["pr-detail:x/y:1", "pr-detail:x/y:2"]);
-    const count = await evictByPrefix("pr-detail:", keepKeys);
-
-    expect(count).toBe(0);
-    expect(await getCacheEntry("pr-detail:x/y:1")).toBeDefined();
-    expect(await getCacheEntry("pr-detail:x/y:2")).toBeDefined();
-  });
-});
-
 describe("cachedFetch", () => {
   it("calls fetchFn with null headers when no cache entry exists", async () => {
     const fetchFn = vi.fn().mockResolvedValue({