From b484c017807acb902eaaaa4e242f8b79f8e8eba6 Mon Sep 17 00:00:00 2001 From: amankumarsingh77 Date: Tue, 28 Jul 2026 22:29:16 +0530 Subject: refactor: act on the fallow audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/cdp/console-buffer.ts | 24 ++----------- src/cdp/network-buffer.ts | 85 ++++++++++++++++++++--------------------------- src/cdp/record-store.ts | 61 ++++++++++++++++++++++++++++++++++ src/daemon/protocol.ts | 2 +- src/domains/js.ts | 7 +--- src/domains/network.ts | 6 +--- src/domains/render.ts | 2 +- src/profile/command.ts | 2 +- src/profile/picker.ts | 2 +- src/util/bytes.ts | 5 +++ src/util/time.ts | 13 -------- 11 files changed, 111 insertions(+), 98 deletions(-) create mode 100644 src/cdp/record-store.ts create mode 100644 src/util/bytes.ts (limited to 'src') 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(); + const records = createRecordStore(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(); - 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(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 = ( + method: E, + p: unknown, + apply: (r: NetworkRecord, params: EventParamsOf) => 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 = { + readonly records: T[]; + readonly total: number; + readonly bufferOverflowed: boolean; +}; + +export type RecordStore = { + set(key: K, rec: T): void; + get(key: K): T | undefined; + delete(key: K): void; + values(): IterableIterator; + // 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; + clear(): void; +}; + +const MAX_LIMIT = 500; +const DEFAULT_LIMIT = 50; + +export const createRecordStore = (capacity: number): RecordStore => { + const records = new Map(); + 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) => (...args: ReadonlyArray) => Promise; 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> => { +const resolveUserDataDir = async (client: BrowserClient): Promise> => { 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 => }, 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, - }; -}; -- cgit v1.2.3