fix(congress-trades): address all review findings

- Move response.ok check inside backOff callback so 429/5xx are retried
- fetchCapitolTradesPages() throws on any page failure — no partial caching
- Cache string[] directly; remove PAGE_BREAK sentinel join/split
- Export mergePageTrades() as a pure function for testability
- timeoutMs: 90_000 → 210_000 (5 pages × 3 attempts × 15s + backoff)
- Extract MIN_SIGNIFICANT_SCORE constant (was magic number 2)
- Fix tautological test → real predicate verifying filter invariants
- Add Cleo Fields watchlist surfacing test (score 2 = threshold)
- Add mergePageTrades unit tests: single page, multi-page, URL dedup,
  duplicate URLs across pages, empty pages, pages with no trades
- Fix error handling test: stub globalThis.fetch instead of relying on
  test-env network timeout (was 7.5s, now instant)

Author: gisk0

src/sources/congress-trades.ts

@@ -21,6 +21,7 @@ import tickerSectors from "../data/ticker-sectors.json";
 const CAPITOL_TRADES_URL = "https://www.capitoltrades.com/trades";
 const CACHE_TTL_MS = 6 * 60 * 60 * 1000; // 6 hours
 const PAGES_TO_FETCH = 5; // ~60 trades instead of ~12
+const MIN_SIGNIFICANT_SCORE = 2;
 
 // ============================================================================
 // Types
@@ -459,7 +460,7 @@ export const filterTrades = (trades: CongressTrade[]): CongressTrade[] => {
     .filter((t) => t.ticker && t.ticker !== "N/A")
     .filter((t) => !excludedTickerSet.has(t.ticker))
     .filter((t) => t.amountLower >= 100_000)
-    .filter((t) => t.score >= 2)
+    .filter((t) => t.score >= MIN_SIGNIFICANT_SCORE)
     .sort((a, b) => b.score - a.score);
 };
 
@@ -602,22 +603,36 @@ export const formatDeduplicatedItem = (
 // Data Source
 // ============================================================================
 
+/**
+ * Fetch a single page from Capitol Trades. Throws on any non-OK response,
+ * so the caller (backOff) can retry correctly on retriable HTTP statuses.
+ */
 const fetchCapitolTradesPage = async (page: number): Promise<string> => {
   const url =
     page === 1 ? CAPITOL_TRADES_URL : `${CAPITOL_TRADES_URL}?page=${page}`;
 
-  const response = await backOff(
-    () =>
-      fetch(url, {
+  return backOff(
+    async () => {
+      const response = await fetch(url, {
         headers: {
           "User-Agent":
             "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36",
           Accept:
             "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8",
           "Accept-Language": "en-US,en;q=0.9",
         },
-        signal: AbortSignal.timeout(30_000),
-      }),
+        signal: AbortSignal.timeout(15_000),
+      });
+
+      // Throw inside the backOff callback so retriable statuses (429, 5xx) are retried.
+      if (!response.ok) {
+        throw new Error(
+          `Capitol Trades page ${page} returned ${response.status}`,
+        );
+      }
+
+      return response.text();
+    },
     {
       numOfAttempts: 3,
       startingDelay: 1000,
@@ -632,71 +647,64 @@ const fetchCapitolTradesPage = async (page: number): Promise<string> => {
       },
     },
   );
-
-  if (!response.ok) {
-    throw new Error(`Capitol Trades page ${page} returned ${response.status}`);
-  }
-
-  return response.text();
 };
 
-const fetchCapitolTradesHTML = async (): Promise<string> => {
-  // Fetch multiple pages sequentially to avoid rate limiting
+/**
+ * Fetch PAGES_TO_FETCH pages from Capitol Trades and return them as an array
+ * of HTML strings. Throws if page 1 fails; throws if any later page fails (no
+ * partial caching).
+ */
+const fetchCapitolTradesPages = async (): Promise<string[]> => {
   const pages: string[] = [];
   for (let i = 1; i <= PAGES_TO_FETCH; i++) {
-    try {
-      const html = await fetchCapitolTradesPage(i);
-      pages.push(html);
-      // Small delay between pages to be polite
-      if (i < PAGES_TO_FETCH) {
-        await new Promise((resolve) => setTimeout(resolve, 500));
-      }
-    } catch (error) {
-      const message = error instanceof Error ? error.message : String(error);
-      console.warn(
-        `[congress-trades] Failed to fetch page ${i}, stopping pagination: ${message}`,
-      );
-      break;
+    const html = await fetchCapitolTradesPage(i);
+    pages.push(html);
+    // Small delay between pages to avoid hammering the server
+    if (i < PAGES_TO_FETCH) {
+      await new Promise((resolve) => setTimeout(resolve, 500));
     }
   }
-  if (pages.length === 0) {
-    throw new Error("Failed to fetch any pages from Capitol Trades");
-  }
   console.log(`[congress-trades] Fetched ${pages.length} pages`);
-  // Return pages joined — parseCapitolTradesHTML will parse each independently
-  return pages.join("\n<!-- PAGE_BREAK -->\n");
+  return pages;
+};
+
+/**
+ * Merge parsed trades from multiple HTML pages, deduplicating by trade URL.
+ * Trades without a URL are always included (Capitol Trades always has URLs,
+ * but defensive against layout changes).
+ */
+export const mergePageTrades = (htmlPages: string[]): CongressTrade[] => {
+  const seenUrls = new Set<string>();
+  const trades: CongressTrade[] = [];
+  for (const html of htmlPages) {
+    for (const trade of parseCapitolTradesHTML(html)) {
+      if (!trade.url || !seenUrls.has(trade.url)) {
+        if (trade.url) seenUrls.add(trade.url);
+        trades.push(trade);
+      }
+    }
+  }
+  return trades;
 };
 
 export const congressTradesSource: DataSource = {
   name: "Congress Trades",
   priority: 6,
-  timeoutMs: 90_000, // 5 pages × ~15s each worst case
+  // Budget: 5 pages × 3 attempts × 15s + backoff + 500ms delays ≈ 3.5 min worst-case
+  timeoutMs: 210_000,
 
   fetch: async (date: Date): Promise<BriefingSection> => {
     const dateKey = date.toISOString().split("T")[0];
     const cacheKey = `congress-trades-${dateKey}`;
 
     try {
-      const combinedHtml = await withCache(cacheKey, fetchCapitolTradesHTML, {
+      const htmlPages = await withCache(cacheKey, fetchCapitolTradesPages, {
         ttlMs: CACHE_TTL_MS,
       });
 
-      // Parse each page separately and merge, deduplicating by trade URL
-      const pages = combinedHtml.split("\n<!-- PAGE_BREAK -->\n");
-      const seenUrls = new Set<string>();
-      const allTrades: CongressTrade[] = [];
-      for (const html of pages) {
-        const pageTrades = parseCapitolTradesHTML(html);
-        for (const trade of pageTrades) {
-          if (!trade.url || !seenUrls.has(trade.url)) {
-            if (trade.url) seenUrls.add(trade.url);
-            allTrades.push(trade);
-          }
-        }
-      }
-
+      const allTrades = mergePageTrades(htmlPages);
       console.log(
-        `[congress-trades] Parsed ${allTrades.length} trades across ${pages.length} pages, filtering...`,
+        `[congress-trades] Parsed ${allTrades.length} trades across ${htmlPages.length} pages, filtering...`,
       );
 
       const filtered = filterTrades(allTrades);
@@ -709,7 +717,7 @@ export const congressTradesSource: DataSource = {
           items: [
             {
               text: "No significant trades recently",
-              detail: `${allTrades.length} trades checked across ${pages.length} pages, none passed filters`,
+              detail: `${allTrades.length} trades checked across ${htmlPages.length} pages, none passed filters`,
             },
           ],
         };

tests/congress-trades.test.ts

@@ -16,6 +16,7 @@ import {
   formatDeduplicatedItem,
   formatTradeItem,
   getCommitteeRelevance,
+  mergePageTrades,
   mockCongressTradesSource,
   parseAmountRange,
   parseAmountValue,
@@ -608,17 +609,108 @@ describe("formatTradeItem", () => {
 // ============================================================================
 
 describe("congressTradesSource.fetch error handling", () => {
-  it("returns empty items on fetch failure", async () => {
-    // The real source will fail in test env (no network / no real URL)
-    // We test that it gracefully returns empty items
-    const result = await congressTradesSource.fetch(new Date());
-    const section = result as {
-      title: string;
-      items: readonly { text: string }[];
-    };
-    expect(section.title).toBe("Congress Trades");
-    // Either empty or has "No significant trades" message — both are valid error handling
-    expect(Array.isArray(section.items)).toBe(true);
+  it("returns empty items when all pages fail to fetch", async () => {
+    // Stub global fetch to always reject
+    const origFetch = globalThis.fetch;
+    globalThis.fetch = Object.assign(
+      async () => {
+        throw new Error("Network error");
+      },
+      { preconnect: () => {} },
+    ) as unknown as typeof fetch;
+    try {
+      // Use a unique date so cache doesn't serve a prior result
+      const result = await congressTradesSource.fetch(new Date("2099-01-01"));
+      const section = result as {
+        title: string;
+        items: readonly { text: string }[];
+      };
+      expect(section.title).toBe("Congress Trades");
+      expect(Array.isArray(section.items)).toBe(true);
+      expect(section.items.length).toBe(0);
+    } finally {
+      globalThis.fetch = origFetch;
+    }
+  });
+});
+
+// ============================================================================
+// mergePageTrades — multi-page merge and deduplication
+// ============================================================================
+
+describe("mergePageTrades", () => {
+  const fixturePath = path.join(__dirname, "fixtures", "capitoltrades.html");
+  const singlePageHtml = fs.readFileSync(fixturePath, "utf8");
+
+  it("returns same trades as single-page parseCapitolTradesHTML", () => {
+    const expected = parseCapitolTradesHTML(singlePageHtml);
+    const result = mergePageTrades([singlePageHtml]);
+    expect(result.length).toBe(expected.length);
+  });
+
+  it("merges trades from two pages into one array", () => {
+    const singlePageTrades = parseCapitolTradesHTML(singlePageHtml);
+    const result = mergePageTrades([singlePageHtml, singlePageHtml]);
+    // All trades on page 2 have the same URL as page 1 → all deduped
+    // (if any trade lacks a URL it would be duplicated; fixture trades all have URLs)
+    const uniqueUrls = new Set(
+      singlePageTrades.map((t) => t.url).filter(Boolean),
+    );
+    expect(result.length).toBe(
+      uniqueUrls.size + (singlePageTrades.length - uniqueUrls.size),
+    );
+  });
+
+  it("deduplicates trades with the same URL across pages", () => {
+    // Build two HTML snippets that produce one trade each with identical URLs
+    const tradeHtml = (url: string, ticker: string) => `
+      <html><body><table>
+        <tr><th>H</th></tr>
+        <tr>
+          <td><h2 class="politician-name"><a href="/politicians/1">Nancy Pelosi</a></h2>
+            <div class="politician-info">
+              <span class="q-field party">Democrat</span>
+              <span class="q-field chamber">House</span>
+              <span class="q-field us-state-compact">CA</span>
+            </div>
+          </td>
+          <td>
+            <h3 class="issuer-name"><a>Corp</a></h3>
+            <span class="issuer-ticker">${ticker}:US</span>
+          </td>
+          <td>Yesterday</td>
+          <td>15 Mar2026</td>
+          <td>5 days</td>
+          <td>Self</td>
+          <td>buy</td>
+          <td>1M–5M</td>
+          <td>$100</td>
+          <td><a href="${url}">detail</a></td>
+        </tr>
+      </table></body></html>`;
+
+    const page1 = tradeHtml("/trades/999", "NVDA");
+    const page2 = tradeHtml("/trades/999", "NVDA"); // same URL → duplicate
+    const page3 = tradeHtml("/trades/888", "AAPL"); // different URL → not a duplicate
+
+    const result = mergePageTrades([page1, page2, page3]);
+    const tickers = result.map((t) => t.ticker);
+    // /trades/999 should appear once, /trades/888 once
+    expect(tickers.filter((t) => t === "NVDA").length).toBe(1);
+    expect(tickers.filter((t) => t === "AAPL").length).toBe(1);
+    expect(result.length).toBe(2);
+  });
+
+  it("handles empty pages array", () => {
+    expect(mergePageTrades([])).toEqual([]);
+  });
+
+  it("handles a page with no parseable trades", () => {
+    const noTrades =
+      "<html><body><table><tr><th>H</th></tr></table></body></html>";
+    const result = mergePageTrades([singlePageHtml, noTrades]);
+    const expected = parseCapitolTradesHTML(singlePageHtml);
+    expect(result.length).toBe(expected.length);
   });
 });
 
@@ -990,9 +1082,23 @@ describe("sanity check: no significant trades (2026-02-25 default page)", () =>
     expect(names).toContain("Jonathan Jackson");
   });
 
-  it("all trades are below $100K or score below 2", () => {
-    for (const t of trades) {
-      expect(t.amountLower < 100_000 || t.score < 2 || t.score >= 2).toBe(true);
+  it("Cleo Fields $100K watchlist trades surface at threshold 2", () => {
+    // Cleo Fields (2× politician multiplier) × $100K base score 1 = score 2.
+    // These trades should now surface since MIN_SIGNIFICANT_SCORE = 2.
+    const fieldsTrades = filtered.filter((t) => t.politician === "Cleo Fields");
+    expect(fieldsTrades.length).toBeGreaterThanOrEqual(1);
+    for (const t of fieldsTrades) {
+      expect(t.score).toBe(2);
+      expect(t.amountLower).toBe(100_000);
+    }
+  });
+
+  it("non-watchlist $100K trades are still excluded (score 1 < threshold)", () => {
+    const excluded = trades.filter(
+      (t) => t.amountLower >= 100_000 && t.score < 2,
+    );
+    for (const t of excluded) {
+      expect(filtered).not.toContain(t);
     }
   });
 });