Refactor the codebase to remove the dup functions and simplify the namings. (#37)

Author: brakit-ai

src/analysis/engine.ts

@@ -14,7 +14,7 @@ import { createDefaultScanner, type SecurityScanner } from "./rules/index.js";
 import { computeInsights, type Insight } from "./insights.js";
 import { insightToIssue, securityFindingToIssue } from "./issue-mappers.js";
 import { computeIssueId } from "../utils/issue-id.js";
-import { extractActiveEndpoints, windowByEndpoint } from "./insights/prepare.js";
+import { extractActiveEndpoints, keepRecentPerEndpoint } from "./insights/prepare.js";
 
 export type { AnalysisUpdate };
 
@@ -71,7 +71,7 @@ export class AnalysisEngine {
     const logs = this.services.logStore.getAll() as readonly TracedLog[];
     const fetches = this.services.fetchStore.getAll() as readonly TracedFetch[];
 
-    const requests = windowByEndpoint(allRequests);
+    const requests = keepRecentPerEndpoint(allRequests);
     const flows = groupRequestsIntoFlows(requests);
 
     this.cachedFindings = this.scanner.scan({ requests, logs });
@@ -86,24 +86,18 @@ export class AnalysisEngine {
     });
 
     const issueStore = this.services.issueStore;
+    const currentIssueIds = new Set<string>();
 
-    // Upsert security findings into IssueStore
     for (const finding of this.cachedFindings) {
-      issueStore.upsert(securityFindingToIssue(finding), "passive");
-    }
-
-    // Upsert performance/reliability insights into IssueStore
-    for (const insight of this.cachedInsights) {
-      issueStore.upsert(insightToIssue(insight), "passive");
+      const issue = securityFindingToIssue(finding);
+      issueStore.upsert(issue, "passive");
+      currentIssueIds.add(computeIssueId(issue));
     }
 
-    // Build the set of currently-detected issue IDs for reconciliation
-    const currentIssueIds = new Set<string>();
-    for (const finding of this.cachedFindings) {
-      currentIssueIds.add(computeIssueId(securityFindingToIssue(finding)));
-    }
     for (const insight of this.cachedInsights) {
-      currentIssueIds.add(computeIssueId(insightToIssue(insight)));
+      const issue = insightToIssue(insight);
+      issueStore.upsert(issue, "passive");
+      currentIssueIds.add(computeIssueId(issue));
     }
 
     const activeEndpoints = extractActiveEndpoints(allRequests);

src/analysis/group.ts

@@ -6,9 +6,36 @@ import { randomUUID } from "node:crypto";
 import type { TracedRequest, LabeledRequest, RequestFlow } from "../types/index.js";
 import { FLOW_GAP_MS, DASHBOARD_PREFIX } from "../constants/index.js";
 import { isErrorStatus } from "../utils/http-status.js";
+import { stripQueryString } from "../utils/endpoint.js";
 import { labelRequest, prettifyEndpoint, prettifyPageName, capitalize, deriveActionVerb } from "./label.js";
 import { getEffectivePath } from "./categorize.js";
-import { markDuplicates, collapsePolling, detectWarnings } from "./transforms.js";
+import { flagDuplicateRequests, mergePollingSequences, collectRequestWarnings } from "./transforms.js";
+
+function shouldStartNewFlow(
+  labeled: LabeledRequest,
+  currentRequests: LabeledRequest[],
+  lastEndTime: number,
+  currentSourcePage: string | undefined,
+  startedAt: number,
+): boolean {
+  if (currentRequests.length === 0) return false;
+
+  const sourcePage = labeled.sourcePage;
+
+  // 1. Page change — referer switched, indicating user navigated
+  const isNewPage =
+    sourcePage !== undefined &&
+    currentSourcePage !== undefined &&
+    sourcePage !== currentSourcePage;
+
+  // 2. Time gap — idle period exceeds FLOW_GAP_MS, suggesting a new action
+  const isTimeGap = startedAt - lastEndTime > FLOW_GAP_MS;
+
+  // 3. Page load — HTML/navigation request marks a fresh page lifecycle
+  const isPageLoad = labeled.category === "page-load" || labeled.category === "navigation";
+
+  return isNewPage || isTimeGap || isPageLoad;
+}
 
 export function groupRequestsIntoFlows(
   requests: readonly TracedRequest[],
@@ -26,28 +53,13 @@ export function groupRequestsIntoFlows(
     const labeled = labelRequest(req);
     if (labeled.category === "static") continue;
 
-    const sourcePage = labeled.sourcePage;
-    const gap = currentRequests.length > 0 ? req.startedAt - lastEndTime : 0;
-
-    // A new flow starts when any of three signals fire:
-    // 1. Page change — referer switched, indicating user navigated
-    // 2. Time gap — idle period exceeds FLOW_GAP_MS, suggesting a new action
-    // 3. Page load — HTML/navigation request marks a fresh page lifecycle
-    const isNewPage =
-      currentRequests.length > 0 &&
-      sourcePage !== undefined &&
-      currentSourcePage !== undefined &&
-      sourcePage !== currentSourcePage;
-    const isPageLoad = labeled.category === "page-load" || labeled.category === "navigation";
-    const isTimeGap = currentRequests.length > 0 && gap > FLOW_GAP_MS;
-
-    if (currentRequests.length > 0 && (isNewPage || isTimeGap || isPageLoad)) {
+    if (shouldStartNewFlow(labeled, currentRequests, lastEndTime, currentSourcePage, req.startedAt)) {
       flows.push(buildFlow(currentRequests));
       currentRequests = [];
     }
 
     currentRequests.push(labeled);
-    currentSourcePage = sourcePage ?? currentSourcePage;
+    currentSourcePage = labeled.sourcePage ?? currentSourcePage;
     lastEndTime = Math.max(lastEndTime, req.startedAt + req.durationMs);
   }
 
@@ -59,8 +71,8 @@ export function groupRequestsIntoFlows(
 }
 
 function buildFlow(rawRequests: LabeledRequest[]): RequestFlow {
-  markDuplicates(rawRequests);
-  const requests = collapsePolling(rawRequests);
+  flagDuplicateRequests(rawRequests);
+  const requests = mergePollingSequences(rawRequests);
 
   const first = requests[0];
   const startTime = first.startedAt;
@@ -86,7 +98,7 @@ function buildFlow(rawRequests: LabeledRequest[]): RequestFlow {
     startTime,
     totalDurationMs: Math.round(endTime - startTime),
     hasErrors: requests.some((r) => isErrorStatus(r.statusCode)),
-    warnings: detectWarnings(rawRequests),
+    warnings: collectRequestWarnings(rawRequests),
     sourcePage,
     redundancyPct,
   };
@@ -100,16 +112,16 @@ function getDominantSourcePage(requests: LabeledRequest[]): string {
     }
   }
 
-  let best = "";
-  let bestCount = 0;
+  let mostCommonPage = "";
+  let highestCount = 0;
   for (const [page, count] of counts) {
-    if (count > bestCount) {
-      best = page;
-      bestCount = count;
+    if (count > highestCount) {
+      mostCommonPage = page;
+      highestCount = count;
     }
   }
 
-  return best || requests[0]?.path?.split("?")[0] || "/";
+  return mostCommonPage || (requests[0]?.path ? stripQueryString(requests[0].path) : "") || "/";
 }
 
 function deriveFlowLabel(requests: LabeledRequest[], sourcePage: string): string {
@@ -122,7 +134,7 @@ function deriveFlowLabel(requests: LabeledRequest[], sourcePage: string): string
     requests[0];
 
   if (trigger.category === "page-load" || trigger.category === "navigation") {
-    const pageName = prettifyPageName(trigger.path.split("?")[0]);
+    const pageName = prettifyPageName(stripQueryString(trigger.path));
     return `${pageName} Page`;
   }
 

src/analysis/insights/prepare.ts

@@ -4,14 +4,14 @@ import type {
   PreparedInsightContext,
   EndpointGroup,
 } from "./types.js";
-import { groupBy } from "../../utils/collections.js";
+import { groupBy, getOrCreate } from "../../utils/collections.js";
 import { getEndpointKey } from "../../utils/endpoint.js";
 import { DASHBOARD_PREFIX } from "../../constants/index.js";
 import { isErrorStatus } from "../../utils/http-status.js";
 import { INSIGHT_WINDOW_PER_ENDPOINT } from "../../constants/config.js";
 import { getQueryShape, getQueryInfo } from "./query-helpers.js";
 
-function createEndpointGroup(): EndpointGroup {
+function emptyEndpointGroup(): EndpointGroup {
   return {
     total: 0,
     errors: 0,
@@ -24,23 +24,26 @@ function createEndpointGroup(): EndpointGroup {
   };
 }
 
-export function windowByEndpoint(
+/**
+ * Limits analysis to the N most recent requests per endpoint so that old,
+ * stale data does not skew insight calculations. The window size is
+ * controlled by INSIGHT_WINDOW_PER_ENDPOINT.
+ */
+export function keepRecentPerEndpoint(
   requests: readonly TracedRequest[],
 ): TracedRequest[] {
   const byEndpoint = new Map<string, TracedRequest[]>();
-  for (const r of requests) {
-    const ep = getEndpointKey(r.method, r.path);
-    let list = byEndpoint.get(ep);
-    if (!list) {
-      list = [];
-      byEndpoint.set(ep, list);
-    }
-    list.push(r);
+  for (const request of requests) {
+    const endpointKey = getEndpointKey(request.method, request.path);
+    const list = getOrCreate(byEndpoint, endpointKey, () => []);
+    list.push(request);
   }
+
   const windowed: TracedRequest[] = [];
   for (const [, reqs] of byEndpoint) {
     windowed.push(...reqs.slice(-INSIGHT_WINDOW_PER_ENDPOINT));
   }
+
   return windowed;
 }
 
@@ -49,73 +52,77 @@ export function windowByEndpoint(
  * Used by AnalysisEngine to determine which endpoints were active
  * during a recompute cycle for evidence-based issue resolution.
  */
+function filterUserRequests(requests: readonly TracedRequest[]): TracedRequest[] {
+  return requests.filter(
+    (request) =>
+      !request.isStatic &&
+      !request.isHealthCheck &&
+      (!request.path || !request.path.startsWith(DASHBOARD_PREFIX)),
+  );
+}
+
 export function extractActiveEndpoints(
   requests: readonly TracedRequest[],
 ): Set<string> {
   const endpoints = new Set<string>();
-  for (const r of requests) {
-    if (
-      !r.isStatic &&
-      !r.isHealthCheck &&
-      (!r.path || !r.path.startsWith(DASHBOARD_PREFIX))
-    ) {
-      endpoints.add(getEndpointKey(r.method, r.path));
-    }
+  for (const request of filterUserRequests(requests)) {
+    endpoints.add(getEndpointKey(request.method, request.path));
   }
   return endpoints;
 }
 
-export function prepareContext(ctx: InsightContext): PreparedInsightContext {
-  const nonStatic = ctx.requests.filter(
-    (r) =>
-      !r.isStatic &&
-      !r.isHealthCheck &&
-      (!r.path || !r.path.startsWith(DASHBOARD_PREFIX)),
-  );
-
-  const queriesByReq = groupBy(ctx.queries, (q) => q.parentRequestId);
-  const fetchesByReq = groupBy(ctx.fetches, (f) => f.parentRequestId);
-
-  const reqById = new Map(nonStatic.map((r) => [r.id, r]));
-
-  const recent = windowByEndpoint(nonStatic);
+function aggregateEndpointMetrics(
+  recent: readonly TracedRequest[],
+  queriesByReq: Map<string, TracedQuery[]>,
+  fetchesByReq: Map<string, { durationMs: number }[]>,
+): Map<string, EndpointGroup> {
   const endpointGroups = new Map<string, EndpointGroup>();
-  for (const r of recent) {
-    const ep = getEndpointKey(r.method, r.path);
-    let g = endpointGroups.get(ep);
-    if (!g) {
-      g = createEndpointGroup();
-      endpointGroups.set(ep, g);
-    }
-    g.total++;
-    if (isErrorStatus(r.statusCode)) g.errors++;
-    g.totalDuration += r.durationMs;
-    g.totalSize += r.responseSize ?? 0;
+  for (const request of recent) {
+    const endpointKey = getEndpointKey(request.method, request.path);
+    const group = getOrCreate(endpointGroups, endpointKey, emptyEndpointGroup);
+    group.total++;
+    if (isErrorStatus(request.statusCode)) group.errors++;
+    group.totalDuration += request.durationMs;
+    group.totalSize += request.responseSize ?? 0;
 
-    const reqQueries: TracedQuery[] = queriesByReq.get(r.id) ?? [];
-    g.queryCount += reqQueries.length;
-    for (const q of reqQueries) {
-      g.totalQueryTimeMs += q.durationMs;
-      const shape = getQueryShape(q);
-      const info = getQueryInfo(q);
-      let sd = g.queryShapeDurations.get(shape);
-      if (!sd) {
-        sd = {
-          totalMs: 0,
-          count: 0,
-          label: info.op + (info.table ? ` ${info.table}` : ""),
-        };
-        g.queryShapeDurations.set(shape, sd);
-      }
-      sd.totalMs += q.durationMs;
-      sd.count++;
+    const reqQueries: TracedQuery[] = queriesByReq.get(request.id) ?? [];
+    group.queryCount += reqQueries.length;
+    for (const query of reqQueries) {
+      group.totalQueryTimeMs += query.durationMs;
+      const shape = getQueryShape(query);
+      const info = getQueryInfo(query);
+      const shapeDuration = getOrCreate(group.queryShapeDurations, shape, () => ({
+        totalMs: 0,
+        count: 0,
+        label: info.op + (info.table ? ` ${info.table}` : ""),
+      }));
+      shapeDuration.totalMs += query.durationMs;
+      shapeDuration.count++;
     }
 
-    const reqFetches = fetchesByReq.get(r.id) ?? [];
-    for (const f of reqFetches) {
-      g.totalFetchTimeMs += f.durationMs;
+    const reqFetches = fetchesByReq.get(request.id) ?? [];
+    for (const fetch of reqFetches) {
+      group.totalFetchTimeMs += fetch.durationMs;
     }
   }
+  return endpointGroups;
+}
+
+/**
+ * Pre-computes lookup tables (queries-by-request, fetches-by-request,
+ * request-by-id) and aggregated per-endpoint metrics used by all insight
+ * rules, so each rule can focus on detection logic rather than data wrangling.
+ */
+export function buildInsightContext(ctx: InsightContext): PreparedInsightContext {
+  const nonStatic = filterUserRequests(ctx.requests);
+
+  const queriesByReq = groupBy(ctx.queries, (query) => query.parentRequestId);
+  const fetchesByReq = groupBy(ctx.fetches, (fetch) => fetch.parentRequestId);
+
+  const reqById = new Map(nonStatic.map((request) => [request.id, request]));
+
+  const recent = keepRecentPerEndpoint(nonStatic);
+  const endpointGroups = aggregateEndpointMetrics(recent, queriesByReq, fetchesByReq);
 
   return {
     ...ctx,