aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authoramankumarsingh77 <amankumarsingh7702@gmail.com>2026-07-28 22:29:16 +0530
committerAman Kumar <145339995+amankumarsingh77@users.noreply.github.com>2026-08-02 19:12:46 +0530
commitb484c017807acb902eaaaa4e242f8b79f8e8eba6 (patch)
tree4bd58916c7642e8211b4b86ab6162aa5c7a202d2
parent1b5c4240a5e679a322f636098a4b370cac084875 (diff)
refactor: act on the fallow audit
Unused exports it found that my own sweep missed, because I had grepped by name and these names occur as local variables elsewhere: deadline and the Deadline type in util/time.ts were genuinely unreferenced and are gone; isWireMessage, resolveUserDataDir, CLEAR_SELECTION_LABEL and the ExpandableText type are used only inside their own file and no longer export. isWireMessage became internal when server.ts moved to the shared serialize/deserialize codec. Duplication: console-buffer and network-buffer were the same capacity-bounded map with an overflow flag, sharing eviction, the drain tail and clear() across three clone groups. That is now cdp/record-store.ts, with tests — neither buffer had any. The three Network follow-up events also repeated the same decode-and-find preamble; one `update` helper covers them, and drops the redundant optional chaining that survived a `!id` guard. formatBytes existed verbatim in js.ts and network.ts; it is now util/bytes.ts. Duplication overall: 2.7% to 2.0%, and none of the remainder is in src. .fallowrc.json records the two entry points the tool cannot infer — the pi extension (declared under pi.extensions) and the daemon (spawned by path) — without which five reachable files read as dead. sharp is ignored because it is loaded through a variable specifier in a dynamic import so it can stay optional. agentDir is exported solely for a test's dynamic import, so it is suppressed by name rather than globally enabling ignoreExportsUsedInFile, which would have hidden four of the five findings above. Not acted on: the "split high-impact file" suggestions for session.ts, client.ts and ref-resolve.ts (119 LOC) are size and fan-in heuristics with no defect behind them, and splitting them would add the indirection this refactor set out to remove. The claim that state.ts is 100% dead is an artifact of the missing entry point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
-rw-r--r--.fallowrc.json13
-rw-r--r--src/cdp/console-buffer.ts24
-rw-r--r--src/cdp/network-buffer.ts85
-rw-r--r--src/cdp/record-store.ts61
-rw-r--r--src/daemon/protocol.ts2
-rw-r--r--src/domains/js.ts7
-rw-r--r--src/domains/network.ts6
-rw-r--r--src/domains/render.ts2
-rw-r--r--src/profile/command.ts2
-rw-r--r--src/profile/picker.ts2
-rw-r--r--src/util/bytes.ts5
-rw-r--r--src/util/time.ts13
-rw-r--r--test/cdp/record-store-test.ts95
13 files changed, 219 insertions, 98 deletions
diff --git a/.fallowrc.json b/.fallowrc.json
new file mode 100644
index 0000000..7c29349
--- /dev/null
+++ b/.fallowrc.json
@@ -0,0 +1,13 @@
+{
+ "entry": [
+ "src/index.ts",
+ "src/daemon/index.ts",
+ "scripts/check-boundaries.ts",
+ "test/**/*-test.ts",
+ "test/manual/*.ts"
+ ],
+ "ignoreDependencies": ["sharp"],
+ "ignoreExports": [
+ { "file": "src/profile/paths.ts", "exports": ["agentDir"] }
+ ]
+}
diff --git a/src/cdp/console-buffer.ts b/src/cdp/console-buffer.ts
index 53dd6c2..9718603 100644
--- a/src/cdp/console-buffer.ts
+++ b/src/cdp/console-buffer.ts
@@ -1,4 +1,5 @@
import { decodeEvent } from "./events";
+import { createRecordStore } from "./record-store";
export type ConsoleLevel = "log" | "info" | "warn" | "error" | "debug";
@@ -111,21 +112,10 @@ const formatStackTrace = (
};
export const createConsoleBuffer = (capacity = 500): ConsoleBuffer => {
- const records = new Map<number, ConsoleRecord>();
+ const records = createRecordStore<number, ConsoleRecord>(capacity);
let nextSeq = 1;
- let overflowed = false;
-
- const evictOldestIfFull = (): void => {
- while (records.size >= capacity) {
- const oldest = records.keys().next();
- if (oldest.done) return;
- records.delete(oldest.value);
- overflowed = true;
- }
- };
const insert = (rec: ConsoleRecord): void => {
- evictOldestIfFull();
records.set(rec.seq, rec);
};
@@ -187,20 +177,12 @@ export const createConsoleBuffer = (capacity = 500): ConsoleBuffer => {
matched.push({ ...r });
}
- const total = matched.length;
- const limit = Math.min(filter.limit ?? 50, 500);
- const limited = matched.slice(-limit);
-
- const bufferOverflowed = overflowed;
- overflowed = false;
-
- return { records: limited, total, bufferOverflowed };
+ return records.page(matched, filter.limit);
},
clear() {
records.clear();
nextSeq = 1;
- overflowed = false;
},
};
};
diff --git a/src/cdp/network-buffer.ts b/src/cdp/network-buffer.ts
index 0366ed4..20ca0fe 100644
--- a/src/cdp/network-buffer.ts
+++ b/src/cdp/network-buffer.ts
@@ -1,4 +1,5 @@
-import { decodeEvent } from "./events";
+import { decodeEvent, type EventParamsOf } from "./events";
+import { createRecordStore } from "./record-store";
export type NetworkRecord = {
requestId: string;
@@ -51,17 +52,24 @@ const compileUrlMatcher = (pattern: string): ((url: string) => boolean) => {
return (url) => url.includes(pattern);
};
+type UpdateEvent = "Network.responseReceived" | "Network.loadingFinished" | "Network.loadingFailed";
+
export const createNetworkBuffer = (capacity = 500): NetworkBuffer => {
- const records = new Map<string, NetworkRecord>();
- let overflowed = false;
-
- const evictOldestIfFull = (): void => {
- while (records.size >= capacity) {
- const oldest = records.keys().next();
- if (oldest.done) return;
- records.delete(oldest.value);
- overflowed = true;
- }
+ const records = createRecordStore<string, NetworkRecord>(capacity);
+
+ // All three follow-up events say the same thing first: decode, find the request they belong to, give up quietly if it is gone.
+ const update = <E extends UpdateEvent>(
+ method: E,
+ p: unknown,
+ apply: (r: NetworkRecord, params: EventParamsOf<E>) => void,
+ ): void => {
+ const decoded = decodeEvent(method, p);
+ if (!decoded.success) return;
+ const id = decoded.data.requestId;
+ if (!id) return;
+ const r = records.get(id);
+ if (!r) return;
+ apply(r, decoded.data);
};
return {
@@ -73,7 +81,6 @@ export const createNetworkBuffer = (capacity = 500): NetworkBuffer => {
const method = params?.request.method;
if (!id || !url || !method) return;
records.delete(id);
- evictOldestIfFull();
const postData = params.request.postData;
records.set(id, {
requestId: id,
@@ -86,40 +93,28 @@ export const createNetworkBuffer = (capacity = 500): NetworkBuffer => {
},
ingestResponseReceived(p) {
- const decoded = decodeEvent("Network.responseReceived", p);
- const params = decoded.success ? decoded.data : undefined;
- const id = params?.requestId;
- if (!id) return;
- const r = records.get(id);
- if (!r) return;
- if (params?.response?.status !== undefined) r.status = params.response.status;
- if (params?.response?.statusText !== undefined && params.response.statusText !== "") r.statusText = params.response.statusText;
- if (params?.response?.mimeType !== undefined) r.mimeType = params.response.mimeType;
- // type is more accurate on responseReceived (CDP sometimes refines it)
- if (params?.type !== undefined) r.type = params.type;
+ update("Network.responseReceived", p, (r, params) => {
+ if (params.response?.status !== undefined) r.status = params.response.status;
+ if (params.response?.statusText !== undefined && params.response.statusText !== "") r.statusText = params.response.statusText;
+ if (params.response?.mimeType !== undefined) r.mimeType = params.response.mimeType;
+ // type is more accurate on responseReceived (CDP sometimes refines it)
+ if (params.type !== undefined) r.type = params.type;
+ });
},
ingestLoadingFinished(p) {
- const decoded = decodeEvent("Network.loadingFinished", p);
- const params = decoded.success ? decoded.data : undefined;
- const id = params?.requestId;
- if (!id) return;
- const r = records.get(id);
- if (!r) return;
- if (params?.encodedDataLength !== undefined) r.responseBodySize = params.encodedDataLength;
- r.durationMs = Date.now() - r.requestStartedMs;
+ update("Network.loadingFinished", p, (r, params) => {
+ if (params.encodedDataLength !== undefined) r.responseBodySize = params.encodedDataLength;
+ r.durationMs = Date.now() - r.requestStartedMs;
+ });
},
ingestLoadingFailed(p) {
- const decoded = decodeEvent("Network.loadingFailed", p);
- const params = decoded.success ? decoded.data : undefined;
- const id = params?.requestId;
- if (!id) return;
- const r = records.get(id);
- if (!r) return;
- r.failed = true;
- if (params?.errorText !== undefined) r.errorText = params.errorText;
- r.durationMs = Date.now() - r.requestStartedMs;
+ update("Network.loadingFailed", p, (r, params) => {
+ r.failed = true;
+ if (params.errorText !== undefined) r.errorText = params.errorText;
+ r.durationMs = Date.now() - r.requestStartedMs;
+ });
},
drain(filter) {
@@ -141,19 +136,11 @@ export const createNetworkBuffer = (capacity = 500): NetworkBuffer => {
matched.push({ ...r });
}
- const total = matched.length;
- const limit = Math.min(filter.limit ?? 50, 500);
- const limited = matched.slice(-limit);
-
- const bufferOverflowed = overflowed;
- overflowed = false;
-
- return { records: limited, total, bufferOverflowed };
+ return records.page(matched, filter.limit);
},
clear() {
records.clear();
- overflowed = false;
},
};
};
diff --git a/src/cdp/record-store.ts b/src/cdp/record-store.ts
new file mode 100644
index 0000000..04b71ff
--- /dev/null
+++ b/src/cdp/record-store.ts
@@ -0,0 +1,61 @@
+export type DrainPage<T> = {
+ readonly records: T[];
+ readonly total: number;
+ readonly bufferOverflowed: boolean;
+};
+
+export type RecordStore<K, T> = {
+ set(key: K, rec: T): void;
+ get(key: K): T | undefined;
+ delete(key: K): void;
+ values(): IterableIterator<T>;
+ // Takes the overflow flag as it pages, so each drain reports only the loss since the previous one.
+ page(matched: T[], limit: number | undefined): DrainPage<T>;
+ clear(): void;
+};
+
+const MAX_LIMIT = 500;
+const DEFAULT_LIMIT = 50;
+
+export const createRecordStore = <K, T>(capacity: number): RecordStore<K, T> => {
+ const records = new Map<K, T>();
+ let overflowed = false;
+
+ const evictOldestIfFull = (): void => {
+ while (records.size >= capacity) {
+ const oldest = records.keys().next();
+ if (oldest.done) return;
+ records.delete(oldest.value);
+ overflowed = true;
+ }
+ };
+
+ return {
+ set(key, rec) {
+ evictOldestIfFull();
+ records.set(key, rec);
+ },
+ get(key) {
+ return records.get(key);
+ },
+ delete(key) {
+ records.delete(key);
+ },
+ values() {
+ return records.values();
+ },
+ page(matched, limit) {
+ const bufferOverflowed = overflowed;
+ overflowed = false;
+ return {
+ records: matched.slice(-Math.min(limit ?? DEFAULT_LIMIT, MAX_LIMIT)),
+ total: matched.length,
+ bufferOverflowed,
+ };
+ },
+ clear() {
+ records.clear();
+ overflowed = false;
+ },
+ };
+};
diff --git a/src/daemon/protocol.ts b/src/daemon/protocol.ts
index 2501980..78c2977 100644
--- a/src/daemon/protocol.ts
+++ b/src/daemon/protocol.ts
@@ -35,7 +35,7 @@ export type WireControl = {
export type WireMessage = WireRequest | WireResponse | WireEvent | WireControl;
-export const isWireMessage = (v: unknown): v is WireMessage => {
+const isWireMessage = (v: unknown): v is WireMessage => {
if (!isRecord(v)) return false;
const t = v["type"];
if (typeof t !== "string") return false;
diff --git a/src/domains/js.ts b/src/domains/js.ts
index b43387c..d2588d0 100644
--- a/src/domains/js.ts
+++ b/src/domains/js.ts
@@ -6,6 +6,7 @@ import { Type } from "typebox";
import { Markdown, Text } from "@mariozechner/pi-tui";
import { getMarkdownTheme, keyHint } from "@mariozechner/pi-coding-agent";
import { type Result, err, ok } from "../util/result";
+import { formatBytes } from "../util/bytes";
import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool";
import { evalJs } from "./cdp-call";
import { applyTruncation } from "../util/truncate";
@@ -13,12 +14,6 @@ import { asNumber, asRecord, asString, isRecord } from "../util/guards";
const COMPACT_PREVIEW_BYTES = 120;
-const formatBytes = (n: number): string => {
- if (n >= 1_048_576) return `${(n / 1_048_576).toFixed(1)} MB`;
- if (n >= 1024) return `${(n / 1024).toFixed(1)} KB`;
- return `${n} B`;
-};
-
// The AsyncFunction constructor is the documented way to compile user source; a plain `unknown` cast would lose the constructor signature, so this typed cast stays.
const AsyncFunction = Object.getPrototypeOf(async () => {}).constructor as
new (...args: ReadonlyArray<string>) => (...args: ReadonlyArray<unknown>) => Promise<unknown>;
diff --git a/src/domains/network.ts b/src/domains/network.ts
index 5520f40..aeb1aa6 100644
--- a/src/domains/network.ts
+++ b/src/domains/network.ts
@@ -2,6 +2,7 @@ import { Type } from "typebox";
import { Markdown, Text } from "@mariozechner/pi-tui";
import { getMarkdownTheme, keyHint } from "@mariozechner/pi-coding-agent";
import { type Result, err, ok } from "../util/result";
+import { formatBytes } from "../util/bytes";
import { defineBrowserTool, type ToolErr, type ToolOk } from "../util/tool";
import { cdpCall } from "./cdp-call";
import { applyTruncation } from "../util/truncate";
@@ -99,11 +100,6 @@ const renderNetworkMarkdown = (
const failed = records.filter((r) => r.failed || (r.status !== undefined && r.status >= 500)).length;
const totalBytes = records.reduce((s, r) => s + (r.responseBodySize ?? 0), 0);
- const formatBytes = (n: number): string => {
- if (n >= 1_048_576) return `${(n / 1_048_576).toFixed(1)} MB`;
- if (n >= 1024) return `${(n / 1024).toFixed(1)} KB`;
- return `${n} B`;
- };
const showWindow = (() => {
const finished = records.filter((r) => r.durationMs !== undefined);
if (finished.length === 0) return "";
diff --git a/src/domains/render.ts b/src/domains/render.ts
index 3f7f0dc..2f8d07f 100644
--- a/src/domains/render.ts
+++ b/src/domains/render.ts
@@ -6,7 +6,7 @@ import { isRecord } from "../util/guards";
const COMPACT_PREVIEW_LINES = 6;
-export type ExpandableText = {
+type ExpandableText = {
readonly summary: string;
readonly body: string;
readonly fullOutputPath?: string;
diff --git a/src/profile/command.ts b/src/profile/command.ts
index 7475924..0703ae8 100644
--- a/src/profile/command.ts
+++ b/src/profile/command.ts
@@ -7,7 +7,7 @@ import { browserNameForUserDataDir, userDataDirCandidates } from "./paths";
import { pinFor, promptForProfile } from "./picker";
import { clearPin, readPin, writePin } from "./store";
-export const resolveUserDataDir = async (client: BrowserClient): Promise<Result<string, string>> => {
+const resolveUserDataDir = async (client: BrowserClient): Promise<Result<string, string>> => {
const connected = client.userDataDir();
if (connected) return ok(connected);
diff --git a/src/profile/picker.ts b/src/profile/picker.ts
index d2a057b..ede43a6 100644
--- a/src/profile/picker.ts
+++ b/src/profile/picker.ts
@@ -4,7 +4,7 @@ import type { ExtensionContext } from "@mariozechner/pi-coding-agent";
import type { BrowserProfile } from "./list";
import type { ProfilePin } from "./store";
-export const CLEAR_SELECTION_LABEL = "— Clear selection (use whichever window is focused) —";
+const CLEAR_SELECTION_LABEL = "— Clear selection (use whichever window is focused) —";
export type ProfileChoice =
| { readonly kind: "profile"; readonly profile: BrowserProfile }
diff --git a/src/util/bytes.ts b/src/util/bytes.ts
new file mode 100644
index 0000000..43ad49c
--- /dev/null
+++ b/src/util/bytes.ts
@@ -0,0 +1,5 @@
+export const formatBytes = (n: number): string => {
+ if (n >= 1_048_576) return `${(n / 1_048_576).toFixed(1)} MB`;
+ if (n >= 1024) return `${(n / 1024).toFixed(1)} KB`;
+ return `${n} B`;
+};
diff --git a/src/util/time.ts b/src/util/time.ts
index d16435f..ae751f8 100644
--- a/src/util/time.ts
+++ b/src/util/time.ts
@@ -14,16 +14,3 @@ export const sleep = (ms: number, signal?: AbortSignal): Promise<void> =>
}, ms);
signal?.addEventListener("abort", onAbort, { once: true });
});
-
-export type Deadline = {
- readonly remainingMs: () => number;
- readonly expired: () => boolean;
-};
-
-export const deadline = (totalMs: number): Deadline => {
- const end = Date.now() + totalMs;
- return {
- remainingMs: () => Math.max(0, end - Date.now()),
- expired: () => Date.now() >= end,
- };
-};
diff --git a/test/cdp/record-store-test.ts b/test/cdp/record-store-test.ts
new file mode 100644
index 0000000..c259f87
--- /dev/null
+++ b/test/cdp/record-store-test.ts
@@ -0,0 +1,95 @@
+import { test, describe } from "node:test";
+import assert from "node:assert/strict";
+import { createRecordStore } from "../../src/cdp/record-store";
+
+const fill = (store: ReturnType<typeof createRecordStore<number, string>>, n: number): void => {
+ for (let i = 1; i <= n; i++) store.set(i, `r${i}`);
+};
+
+describe("record store capacity", () => {
+ test("holds up to capacity without reporting a loss", () => {
+ const store = createRecordStore<number, string>(3);
+ fill(store, 3);
+ assert.equal([...store.values()].length, 3);
+ assert.equal(store.page([...store.values()], undefined).bufferOverflowed, false);
+ });
+
+ test("evicts the oldest past capacity and reports the loss", () => {
+ const store = createRecordStore<number, string>(3);
+ fill(store, 5);
+ assert.deepEqual([...store.values()], ["r3", "r4", "r5"]);
+ assert.equal(store.page([...store.values()], undefined).bufferOverflowed, true);
+ });
+
+ test("the overflow flag is taken, so a second drain reports no new loss", () => {
+ const store = createRecordStore<number, string>(2);
+ fill(store, 4);
+ assert.equal(store.page([], undefined).bufferOverflowed, true);
+ assert.equal(store.page([], undefined).bufferOverflowed, false);
+ });
+
+ test("clear resets both the records and the overflow flag", () => {
+ const store = createRecordStore<number, string>(2);
+ fill(store, 4);
+ store.clear();
+ assert.equal([...store.values()].length, 0);
+ assert.equal(store.page([], undefined).bufferOverflowed, false);
+ });
+
+ test("get returns the stored object by reference so callers can mutate in place", () => {
+ const store = createRecordStore<string, { status?: number }>(4);
+ store.set("a", {});
+ const held = store.get("a");
+ assert.ok(held);
+ held.status = 200;
+ assert.equal(store.get("a")?.status, 200);
+ });
+
+ test("delete removes a record without flagging an overflow", () => {
+ const store = createRecordStore<number, string>(4);
+ fill(store, 2);
+ store.delete(1);
+ assert.deepEqual([...store.values()], ["r2"]);
+ assert.equal(store.page([], undefined).bufferOverflowed, false);
+ });
+
+ test("re-setting an existing key does not grow the store", () => {
+ const store = createRecordStore<number, string>(4);
+ store.set(1, "first");
+ store.set(1, "second");
+ assert.deepEqual([...store.values()], ["second"]);
+ });
+});
+
+describe("record store paging", () => {
+ test("total counts every match while records carry only the page", () => {
+ const store = createRecordStore<number, string>(100);
+ const matched = Array.from({ length: 120 }, (_, i) => `r${i}`);
+ const page = store.page(matched, 10);
+ assert.equal(page.total, 120);
+ assert.equal(page.records.length, 10);
+ });
+
+ test("the page is the newest records, not the oldest", () => {
+ const store = createRecordStore<number, string>(10);
+ const page = store.page(["a", "b", "c"], 2);
+ assert.deepEqual(page.records, ["b", "c"]);
+ });
+
+ test("an absent limit defaults to 50", () => {
+ const store = createRecordStore<number, string>(600);
+ const matched = Array.from({ length: 80 }, (_, i) => `r${i}`);
+ assert.equal(store.page(matched, undefined).records.length, 50);
+ });
+
+ test("a limit above the 500 ceiling is capped", () => {
+ const store = createRecordStore<number, string>(1000);
+ const matched = Array.from({ length: 700 }, (_, i) => `r${i}`);
+ assert.equal(store.page(matched, 9_000).records.length, 500);
+ });
+
+ test("a limit larger than the match count returns every match", () => {
+ const store = createRecordStore<number, string>(10);
+ assert.equal(store.page(["a", "b"], 50).records.length, 2);
+ });
+});