Skip to content

feat(pi-agents-tmux): the Transcript tab renders an event timeline; open-in-editor key (VST-327) - #1442

Merged
Brad (bmethod) merged 11 commits into
mainfrom
vst-327
Aug 17, 2026
Merged

feat(pi-agents-tmux): the Transcript tab renders an event timeline; open-in-editor key (VST-327)#1442
Brad (bmethod) merged 11 commits into
mainfrom
vst-327

Conversation

@bmethod

Copy link
Copy Markdown
Collaborator

Rebuilds the Agents-popup Transcript tab as an event timeline (VST-327), pi-agents-tmux 2.8.4.

Timeline instead of raw JSONL: on the reported 862 KB reviewer transcript, 64% of events fell through describeTranscriptEvent to raw JSONL dumps (561 KB of rendered text). The formatter now emits one row per event — elapsed stamp, kind, capped one-line detail (160 chars) — covering input, assistant text/thinking previews, turn boundaries, writer lifecycle records, and stderr. No event type falls through: unrecognized types render as their type and size. A tool call collapses into a single row pairing start with result (name, primary argument, status, duration, result size); tool_execution_update payloads fold in instead of dumping; a tool-call-only assistant message renders as its calls. Errors, aborts, timeouts, and non-zero exits are -marked rows the renderer paints in the error tone; benign lifecycle records stay neutral.

Reads on the record boundary: the 24 KB tail (2.8% of the sample, always opening mid-JSON) becomes a 256 KB streaming byte-window — only the final window is materialized, the dropped prefix is newline-counted through a fixed 64 KB buffer, the cut lands on a line boundary, and the timeline states how many earlier events are not shown. A single record larger than the window renders as a labeled row, never a blank tab.

Hardening from the pre-PR cross-model review: decoded text is scrubbed of C0/C1 terminal controls before rendering (JSON.parse revives escaped OSC/CSI the raw view kept inert); native pane-session message records render as conversation content rather than failure rows.

Editor access: e in the trace viewer opens the item's file in $VISUAL/$EDITOR in its own tmux window (notify fallback outside tmux / with no editor), listed in the footer hint.

The timeline renderer lives in the new transcript-timeline.ts (the ratchet seam); transcripts.ts keeps the wire-format/reconstruction half. Tests: new tests/transcript-timeline.test.ts (14 pins incl. control-scrub, pairing, elision, dropped-count, tail boundary); one legacy raw-format pin updated. 369 green.

Closes VST-327.

https://claude.ai/code/session_012epxJEzGqT7q3qcFhdZUt5

Copilot AI balanced review requested due to automatic review settings August 17, 2026 13:22
@linear-code

linear-code Bot commented Aug 17, 2026

Copy link
Copy Markdown

VST-327

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@macroscopeapp

macroscopeapp Bot commented Aug 17, 2026

Copy link
Copy Markdown

Approvability

Verdict: Needs human review

This PR introduces substantial new user-facing functionality: a timeline renderer replacing raw JSONL transcript display, a streaming tail-reader, and an 'open in editor' keybinding. New feature additions of this scope warrant human review despite being well-scoped and tested.

No code changes detected at 0688760. Prior analysis still applies.

You can customize Macroscope's approvability policy. Learn more.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR upgrades the pi-agents-tmux “Transcript” UI from rendering raw JSONL into a compact, safer event timeline, and adds ergonomics for opening trace items directly in an editor.

Changes:

  • Added a new transcript timeline formatter (one row per event; tool-call pairing; capped previews; failure marking; control-sequence scrubbing).
  • Implemented a byte-budgeted, line-boundary tail reader that reports how many events were dropped.
  • Added a trace viewer keybinding (e) to open the currently selected item in $VISUAL/$EDITOR (via a tmux window).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
tools/size-ratchet-baseline.tsv Updates size baseline for modified tests.
pi-extensions/pi-agents-tmux/tests/transcript-timeline.test.ts New tests for timeline formatting + transcript tail-reading behavior.
pi-extensions/pi-agents-tmux/tests/dashboard-ux.test.ts Updates assertions to match new timeline transcript rendering.
pi-extensions/pi-agents-tmux/package.json Bumps extension version to 2.8.4.
pi-extensions/pi-agents-tmux/extensions/subagent/transcripts.ts Exposes shared helpers used by the new timeline formatter.
pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts New timeline formatter implementation.
pi-extensions/pi-agents-tmux/extensions/subagent/renderers.ts Adds readTranscriptTail() for budgeted transcript reads.
pi-extensions/pi-agents-tmux/extensions/subagent/browser/trace-viewer.ts Adds footer hint + e key to open the selected item in an editor.
pi-extensions/pi-agents-tmux/extensions/subagent/browser/monitor-task-detail.ts Switches transcript rendering to timeline + uses tail reader + timeline-aware line styling.
pi-extensions/pi-agents-tmux/CHANGELOG.md Documents 2.8.4 changes.
CHANGELOG.md Notes the transcript timeline + editor open behavior in Unreleased.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts Outdated
Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts Outdated
Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts Outdated
Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts Outdated
Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts Outdated
Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/renderers.ts
Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcripts.ts
Copilot AI review requested due to automatic review settings August 17, 2026 13:26

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0b38fef9b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts Outdated
Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (4)

pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts:157

  • code !== 0 will mark an exit as a failure if record.code is a string like "0" (since "0" !== 0 is true), even though it renders as code 0. Consider normalizing record.code to a number first (e.g., via numberValue(record.code) with an explicit fallback) and only treating it as a failure when it’s a finite non-zero number.
		if (recordType === "exit") {
			const code = record.code ?? "unknown";
			push(stamp, "exit", `code ${code}`, code !== 0);
			continue;
		}

pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts:220

  • stringValue currently treats "" as a valid string, so an empty toolName / name can produce labels like tool , and an empty toolCallId can yield an empty map key (potentially mixing unrelated tool calls). A concrete fix is to trim and treat empty/whitespace-only strings as absent at these call sites (or introduce a nonEmptyStringValue helper) before computing name/id.
			case "tool_execution_start": {
				const name = stringValue(event.toolName ?? event.tool_name) ?? stringValue(event.name) ?? "tool";
				const target = primaryToolArgument(event.args ?? event.arguments ?? event.input ?? event.params);
				const label = target ? `tool ${name} (${oneLine(target, 60)})` : `tool ${name}`;
				const id = stringValue(event.toolCallId ?? event.tool_call_id) ?? `name:${name}`;
				const row = push(stamp, label, "no result recorded");

pi-extensions/pi-agents-tmux/extensions/subagent/renderers.ts:268

  • The docstring says the tail read happens “without ever cutting mid-record” and that “the cut lands on a line boundary”, but the implementation explicitly allows “a single record larger than the whole window” to be cut mid-line (so the formatter can label it). Please update this comment to document that exception so the behavior matches the documented contract.
/**
 * Tail-read a JSONL transcript on a byte budget without ever cutting
 * mid-record: only the final window is materialized (the dropped prefix is
 * newline-counted through a small reusable buffer, never held in memory),
 * the cut lands on a line boundary, and the dropped-event count lets the
 * caller say what is not shown instead of rendering a leading fragment.
 */
export async function readTranscriptTail(filePath: string | undefined, maxBytes = 256_000): Promise<{ text: string; droppedLines: number } | undefined> {

pi-extensions/pi-agents-tmux/extensions/subagent/browser/trace-viewer.ts:69

  • The manual-open guidance message interpolates the raw path, which can be confusing when the path contains spaces/shell-special characters. Since this file already imports shellQuote, consider quoting the displayed path in these notifications (and potentially quoting the editor+path command in the “not inside tmux” message) to give users a copy/paste-safe command.
	const editor = process.env.VISUAL?.trim() || process.env.EDITOR?.trim();
	if (!editor) {
		ctx.ui.notify(`Set $VISUAL or $EDITOR to open ${path}`, "warning");
		return;
	}
	if (!process.env.TMUX) {
		ctx.ui.notify(`Not inside tmux — open manually: ${editor} ${path}`, "warning");
		return;
	}

Copilot AI review requested due to automatic review settings August 17, 2026 13:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pi-extensions/pi-agents-tmux/extensions/subagent/renderers.ts:285

  • The tail reader still performs an O(file_size) scan of the entire dropped prefix to count newlines. For very large transcripts this can introduce noticeable latency and disk I/O in the UI path (even though the payload is tail-bounded). Consider bounding the counting work (e.g., cap the prefix scan to a maximum number of bytes/time and report an ‘at least N earlier events not shown’ message), or make dropped-line counting optional (only count if size is under a threshold; otherwise omit/approximate).
export async function readTranscriptTail(filePath: string | undefined, maxBytes = 256_000): Promise<{ text: string; droppedLines: number } | undefined> {
	if (!filePath) return undefined;
	let handle: fs.promises.FileHandle | undefined;
	try {
		handle = await fs.promises.open(filePath, "r");
		const { size } = await handle.stat();
		if (size <= maxBytes) return { droppedLines: 0, text: (await handle.readFile()).toString("utf-8") };
		const start = size - maxBytes;
		let droppedLines = 0;
		const chunk = Buffer.alloc(64 * 1024);
		for (let position = 0; position < start; ) {
			const { bytesRead } = await handle.read(chunk, 0, Math.min(chunk.length, start - position), position);
			if (bytesRead <= 0) break;
			for (let index = 0; index < bytesRead; index += 1) if (chunk[index] === 10) droppedLines += 1;
			position += bytesRead;
		}

pi-extensions/pi-agents-tmux/extensions/subagent/browser/trace-viewer.ts:68

  • These notifications interpolate path directly into UI output. If a transcript path ever contains control characters (possible on POSIX filesystems), they could be interpreted by the terminal/UI output path. Consider sanitizing path before displaying (e.g., stripping C0/C1 controls similar to stripTerminalControls, or rendering via a safe/escaped representation) to prevent terminal control injection via filenames.
export function openTraceItemInEditor(ctx: ExtensionContext, path: string | undefined): void {
	if (!path) return;
	const editor = process.env.VISUAL?.trim() || process.env.EDITOR?.trim();
	if (!editor) {
		ctx.ui.notify(`Set $VISUAL or $EDITOR to open ${path}`, "warning");
		return;
	}
	if (!process.env.TMUX) {
		ctx.ui.notify(`Not inside tmux — open manually: ${editor} ${path}`, "warning");
		return;
	}

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 32c1844eff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts Outdated
Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts Outdated
Copilot AI review requested due to automatic review settings August 17, 2026 14:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts:164

  • Exit failure detection uses code !== 0, which will mark non-numeric but benign representations (e.g., string "0") as failures. To avoid false ✖ markings, normalize the exit code to a number when possible (e.g., treat numeric strings as numbers) and only mark failure when the normalized code is a non-zero finite number; otherwise render as unknown without forcing the failure tone.
		if (recordType === "exit") {
			const code = record.code ?? "unknown";
			push(stamp, "exit", `code ${code}`, code !== 0);
			continue;
		}

pi-extensions/pi-agents-tmux/extensions/subagent/renderers.ts:301

  • The read(prev, ...) result is not checked. If the read returns bytesRead === 0 (or fails partially), prev[0] will remain 0 and the code will incorrectly treat the tail as mid-record, potentially dropping the first kept line and miscounting droppedLines. Capture bytesRead from this read and only apply the fragment-drop logic when bytesRead === 1 and prev[0] !== 10.
		const prev = Buffer.alloc(1);
		await handle.read(prev, 0, 1, start - 1);
		if (prev[0] !== 10) {
			// The window opens mid-record: drop the fragment (counted — its
			// newline sits inside the window). A single record larger than the
			// whole window has no boundary; it stays as one line for the
			// formatter to label rather than vanishing into a blank tab.
			const boundary = text.indexOf("\n");
			if (boundary !== -1) {
				droppedLines += 1;
				text = text.slice(boundary + 1);
			}
		}

pi-extensions/pi-agents-tmux/extensions/subagent/renderers.ts:285

  • Counting dropped lines scans the entire dropped prefix (0..start) on every read. For very large transcripts this can negate the benefits of tailing (high I/O latency) even though only the last maxBytes are needed for rendering. Consider making dropped-line counting optional or capped (e.g., stop after N MB / N lines and report >= N), or persist an index/offset elsewhere if exact counts are needed.
		const start = size - maxBytes;
		let droppedLines = 0;
		const chunk = Buffer.alloc(64 * 1024);
		for (let position = 0; position < start; ) {
			const { bytesRead } = await handle.read(chunk, 0, Math.min(chunk.length, start - position), position);
			if (bytesRead <= 0) break;
			for (let index = 0; index < bytesRead; index += 1) if (chunk[index] === 10) droppedLines += 1;
			position += bytesRead;
		}

pi-extensions/pi-agents-tmux/extensions/subagent/browser/trace-viewer.ts:69

  • New behavior is introduced for the e key path, but the success/fallback paths in openTraceItemInEditor() (no editor configured, not in tmux, spawn error) are not covered here. Since this package already has node:test coverage for transcript rendering, consider adding focused unit tests by injecting/wrapping spawn (or factoring command construction into a pure function) so you can assert the notifications/command generation deterministically.
export function openTraceItemInEditor(ctx: ExtensionContext, path: string | undefined): void {
	if (!path) return;
	const editor = process.env.VISUAL?.trim() || process.env.EDITOR?.trim();
	if (!editor) {
		ctx.ui.notify(`Set $VISUAL or $EDITOR to open ${path}`, "warning");
		return;
	}
	if (!process.env.TMUX) {
		ctx.ui.notify(`Not inside tmux — open manually: ${editor} ${path}`, "warning");
		return;
	}
	const child = spawn("tmux", ["new-window", "-n", "transcript", `${editor} ${shellQuote(path)}`], { detached: true, stdio: "ignore" });
	child.on("error", (error) => ctx.ui.notify(`Could not open editor window: ${error instanceof Error ? error.message : String(error)}`, "error"));
	child.unref();
}

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e0117ae67f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts Outdated
Copilot AI review requested due to automatic review settings August 17, 2026 15:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pi-extensions/pi-agents-tmux/extensions/subagent/renderers.ts:285

  • The dropped-line counting loop always scans the entire prefix (0..start) to count newlines, which makes opening the Transcript tab O(file size) I/O even though only the tail is displayed. For very large transcripts this can noticeably stall the UI. Consider making the dropped count optional / capped (e.g., stop counting after N MB and return an “unknown/at least N” marker), or adjust the UI copy to not require an exact count (e.g., “earlier events not shown”) when the file exceeds a threshold.
export async function readTranscriptTail(filePath: string | undefined, maxBytes = 256_000): Promise<{ text: string; droppedLines: number } | undefined> {
	if (!filePath) return undefined;
	let handle: fs.promises.FileHandle | undefined;
	try {
		handle = await fs.promises.open(filePath, "r");
		const { size } = await handle.stat();
		if (size <= maxBytes) return { droppedLines: 0, text: (await handle.readFile()).toString("utf-8") };
		const start = size - maxBytes;
		let droppedLines = 0;
		const chunk = Buffer.alloc(64 * 1024);
		for (let position = 0; position < start; ) {
			const { bytesRead } = await handle.read(chunk, 0, Math.min(chunk.length, start - position), position);
			if (bytesRead <= 0) break;
			for (let index = 0; index < bytesRead; index += 1) if (chunk[index] === 10) droppedLines += 1;
			position += bytesRead;
		}

pi-extensions/pi-agents-tmux/extensions/subagent/browser/trace-viewer.ts:72

  • This builds a single shell command string for tmux from $VISUAL/$EDITOR plus a quoted path. While path is quoted, editor is injected verbatim into a shell command context (tmux runs the shell-command), which can behave unexpectedly with metacharacters or complex editor commands. A safer approach is to parse $VISUAL/$EDITOR into argv tokens and re-quote each token for the shell-command string (or otherwise constrain/validate editor), so the command is robust across common editor values like code --wait and avoids accidental shell interpretation.
export function openTraceItemInEditor(ctx: ExtensionContext, path: string | undefined): void {
	if (!path) return;
	const editor = process.env.VISUAL?.trim() || process.env.EDITOR?.trim();
	if (!editor) {
		ctx.ui.notify(`Set $VISUAL or $EDITOR to open ${path}`, "warning");
		return;
	}
	if (!process.env.TMUX) {
		ctx.ui.notify(`Not inside tmux — open manually: ${editor} ${path}`, "warning");
		return;
	}
	const child = spawn("tmux", ["new-window", "-n", "transcript", `${editor} ${shellQuote(path)}`], { detached: true, stdio: "ignore" });
	child.on("error", (error) => ctx.ui.notify(`Could not open editor window: ${error instanceof Error ? error.message : String(error)}`, "error"));
	child.unref();
}

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 442528f0ad

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Copilot AI review requested due to automatic review settings August 17, 2026 16:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (4)

pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts:24

  • Using toFixed(1) for durations under 60s can round up to "60.0s" (e.g., 59.95s), which contradicts the intended "never render 60s" behavior. Consider flooring to one decimal place (or clamping to < 60.0) so the sub-minute format cannot produce 60.0s; alternatively, treat rounded values >= 60.0 as 1m 0s.
function formatToolDuration(ms: number | undefined): string | undefined {
	if (ms === undefined || !Number.isFinite(ms) || ms < 0) return undefined;
	if (ms < 1000) return `${ms}ms`;
	if (ms < 60_000) return `${(ms / 1000).toFixed(1)}s`;
	const totalSeconds = Math.round(ms / 1000);
	return `${Math.floor(totalSeconds / 60)}m ${totalSeconds % 60}s`;
}

pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts:78

  • imagesCount is currently gated by truthiness, so an explicit 0 (a meaningful value) is treated the same as "missing" and omitted from the label. If the intent is to reflect whether the field is present, consider checking imagesCount !== undefined instead of relying on truthiness.
function describeInputEvent(event: any): TimelineRow {
	const delivery = inputDeliveryLabel(event.streamingBehavior ?? event.streaming_behavior) ?? "idle";
	const source = stringValue(event.source);
	const preview = stringValue(event.textPreview ?? event.text_preview ?? event.text) ?? "";
	const truncated = event.textTruncated === true || event.text_truncated === true;
	const imagesCount = numberValue(event.imagesCount ?? event.images_count);
	const meta = [delivery, source, imagesCount ? `${imagesCount} image${imagesCount === 1 ? "" : "s"}` : undefined].filter(Boolean).join(", ");
	return { stamp: "", kind: `input (${meta})`, detail: [preview, truncated ? "(truncated)" : ""].filter(Boolean).join(" ") };
}

pi-extensions/pi-agents-tmux/extensions/subagent/renderers.ts:300

  • readTranscriptTail uses a null sentinel via "firstLine = null as never" while the variable is typed as Buffer | undefined, plus a "firstLine !== null" guard. This works at runtime but is brittle and confusing for future changes. Prefer typing firstLine as Buffer | null (or introduce a separate boolean like doneReadingOrigin) to remove the need for type assertions and sentinel gymnastics.
		let originTs: unknown;
		let firstLine: Buffer | undefined;
		const chunk = Buffer.alloc(64 * 1024);
		for (let position = 0; position < start; ) {
			const { bytesRead } = await handle.read(chunk, 0, Math.min(chunk.length, start - position), position);
			if (bytesRead <= 0) break;
			if (originTs === undefined && firstLine !== null) {
				const boundary = chunk.subarray(0, bytesRead).indexOf(10);
				const piece = chunk.subarray(0, boundary === -1 ? bytesRead : boundary);
				firstLine = firstLine ? Buffer.concat([firstLine, piece]) : Buffer.from(piece);
				if (boundary !== -1 || firstLine.length > 1024 * 1024) {
					try {
						const parsed = JSON.parse(firstLine.toString("utf-8"));
						originTs = parsed?.ts ?? parsed?.timestamp;
					} catch {
						originTs = undefined;
					}
					firstLine = null as never;
				}
			}

pi-extensions/pi-agents-tmux/extensions/subagent/browser/trace-viewer.ts:72

  • This passes a full command string to tmux (which may involve shell parsing on tmux's side), and only shell-quotes the path. If $EDITOR/$VISUAL includes spaces/args or shell metacharacters, behavior can be surprising and may be unsafe if any part of the inputs is not fully controlled. Consider using tmux's "command [arguments]" form by passing editor + args + path as separate arguments (after parsing $EDITOR into argv), which avoids shell interpretation and reduces quoting/injection risks.
export function openTraceItemInEditor(ctx: ExtensionContext, path: string | undefined): void {
	if (!path) return;
	const editor = process.env.VISUAL?.trim() || process.env.EDITOR?.trim();
	if (!editor) {
		ctx.ui.notify(`Set $VISUAL or $EDITOR to open ${path}`, "warning");
		return;
	}
	if (!process.env.TMUX) {
		ctx.ui.notify(`Not inside tmux — open manually: ${editor} ${path}`, "warning");
		return;
	}
	const child = spawn("tmux", ["new-window", "-n", "transcript", `${editor} ${shellQuote(path)}`], { detached: true, stdio: "ignore" });
	child.on("error", (error) => ctx.ui.notify(`Could not open editor window: ${error instanceof Error ? error.message : String(error)}`, "error"));
	child.unref();
}

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dcb428f089

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts Outdated
Comment thread pi-extensions/pi-agents-tmux/extensions/subagent/renderers.ts
Copilot AI review requested due to automatic review settings August 17, 2026 16:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts:166

  • exit rows mark failure via code !== 0, but record.code may not always be a number (e.g. it could be the string "0" depending on the writer). In that case this will incorrectly mark successful exits as failures. Consider normalizing record.code to a finite number (or parsing numeric strings) before comparing, and only treating non-zero numeric codes as errors.
			const code = record.code ?? "unknown";
			push(stamp, "exit", `code ${code}`, code !== 0);

pi-extensions/pi-agents-tmux/extensions/subagent/renderers.ts:295

  • firstLine = Buffer.concat(...) on each loop iteration can become quadratic for large first records (up to the 1MB bound) due to repeated reallocation/copying. A more efficient approach is to push the sub-buffers into an array and Buffer.concat(chunks) once at the end (or stop-reading exactly at the newline and slice), which keeps this linear-time.
			let firstLine = Buffer.alloc(0);
			for (let position = 0; position < size && firstLine.length <= 1024 * 1024; ) {
				const { bytesRead } = await handle.read(chunk, 0, chunk.length, position);
				if (bytesRead <= 0) break;
				const boundary = chunk.subarray(0, bytesRead).indexOf(10);
				firstLine = Buffer.concat([firstLine, chunk.subarray(0, boundary === -1 ? bytesRead : boundary)]);

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e76dac8cab

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Copilot AI review requested due to automatic review settings August 17, 2026 16:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (3)

pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts:24

  • The < 60_000 branch can still render "60.0s" because (ms / 1000).toFixed(1) rounds (e.g., 59_960ms -> 60.0s), which contradicts the “never render 60s” expectation. Consider formatting seconds via integer math (or switching to 1m 0s when rounded seconds reach 60) so the seconds-only format never emits 60.0s.
function formatToolDuration(ms: number | undefined): string | undefined {
	if (ms === undefined || !Number.isFinite(ms) || ms < 0) return undefined;
	if (ms < 1000) return `${ms}ms`;
	if (ms < 60_000) return `${(ms / 1000).toFixed(1)}s`;
	const totalSeconds = Math.round(ms / 1000);
	return `${Math.floor(totalSeconds / 60)}m ${totalSeconds % 60}s`;
}

pi-extensions/pi-agents-tmux/extensions/subagent/transcript-timeline.ts:78

  • imagesCount is treated as “absent” when it is 0 due to the truthiness check (imagesCount ? ... : undefined). If the record explicitly includes 0 images, it will be silently dropped from the meta string. If you want “0 images” to be rendered when provided, switch this check to imagesCount !== undefined.
	const truncated = event.textTruncated === true || event.text_truncated === true;
	const imagesCount = numberValue(event.imagesCount ?? event.images_count);
	const meta = [delivery, source, imagesCount ? `${imagesCount} image${imagesCount === 1 ? "" : "s"}` : undefined].filter(Boolean).join(", ");
	return { stamp: "", kind: `input (${meta})`, detail: [preview, truncated ? "(truncated)" : ""].filter(Boolean).join(" ") };

pi-extensions/pi-agents-tmux/extensions/subagent/renderers.ts:307

  • Counting droppedLines by scanning every byte from the beginning up to start makes readTranscriptTail() O(file_size) even though only the last maxBytes are returned. For very large transcripts, opening the Transcript tab could become noticeably slow due to reading the entire dropped prefix. If exact counts aren’t strictly required, consider a faster alternative (e.g., approximate counts, or storing a running line/event count in the transcript writer so the tail reader can avoid a full-prefix scan).
		for (let position = 0; position < start; ) {
			const { bytesRead } = await handle.read(chunk, 0, Math.min(chunk.length, start - position), position);
			if (bytesRead <= 0) break;
			for (let index = 0; index < bytesRead; index += 1) if (chunk[index] === 10) droppedLines += 1;
			position += bytesRead;
		}

@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

const boundary = text.indexOf("\n");
if (boundary !== -1) {
droppedLines += 1;
text = text.slice(boundary + 1);
}

P2 Badge Handle an oversized final JSONL record

When the newest JSONL record alone exceeds maxBytes—for example, a live pane's large assistant or tool-result message—text contains no newline, so this branch retains only the trailing fragment and does not increment droppedLines. formatTranscriptForDisplay then reports a valid record as ✖ unparseable line, omits the dropped-events warning, and loses the event type and detail. Treat the no-boundary case as an oversized truncated record, or read enough of it to preserve its metadata, rather than passing the fragment to the JSON parser.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

@bmethod
Brad (bmethod) added this pull request to the merge queue Aug 17, 2026
Merged via the queue into main with commit dd818a5 Aug 17, 2026
23 checks passed
@bmethod
Brad (bmethod) deleted the vst-327 branch August 17, 2026 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants