Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 51 additions & 22 deletions apps/staged/src/lib/features/actions/ActionOutputModal.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
listenToActionOutput,
listenToActionStatus,
} from './actions';
import { processChunksToLines, type TerminalLine } from './processOutput';
import { createIncrementalProcessor, type TerminalLine } from './processOutput';

interface Props {
executionId: string;
Expand Down Expand Up @@ -73,7 +73,8 @@

let status = $state<ActionStatus>('running');
let exitCode = $state<number | null>(null);
let outputChunks = $state<OutputChunk[]>([]);
let displayLines = $state<TerminalLine[]>([]);
let lineProcessor = createIncrementalProcessor();
let loading = $state(true);
let error = $state<string | null>(null);
let saveError = $state<string | null>(null);
Expand All @@ -84,6 +85,22 @@
let shouldAutoScroll = $state(true);
const backdropDismiss = createBackdropDismissHandlers({ onDismiss: () => onClose() });

// rAF batching for incoming output chunks
let pendingChunks: OutputChunk[] = [];
let flushRaf: number | null = null;

function flushPendingChunks() {
flushRaf = null;
if (pendingChunks.length > 0) {
const chunks = pendingChunks;
pendingChunks = [];
displayLines = lineProcessor.process(chunks);
if (shouldAutoScroll) {
tick().then(() => scrollToBottom());
}
}
}

// ANSI to HTML converter
const ansiConverter = new Convert({
fg: '#e0e0e0',
Expand Down Expand Up @@ -149,8 +166,8 @@
}
}

/** Derived display lines — recomputed whenever outputChunks changes. */
let displayLines = $derived(processChunksToLines(outputChunks));
// Render cache — avoids re-running ANSI conversion + sanitization for unchanged lines
let renderLineCache = new Map<string, string>();

// =========================================================================
// Lifecycle
Expand All @@ -172,7 +189,14 @@
// Reset state for the new execution
status = 'running';
exitCode = null;
outputChunks = [];
displayLines = [];
lineProcessor = createIncrementalProcessor();
renderLineCache = new Map();
pendingChunks = [];
if (flushRaf !== null) {
cancelAnimationFrame(flushRaf);
flushRaf = null;
}
loading = true;
error = null;
shouldAutoScroll = true;
Expand All @@ -195,7 +219,7 @@
error = null;
const buffer = await getActionOutputBuffer(executionId);
if (buffer) {
outputChunks = buffer;
displayLines = lineProcessor.process(buffer);
}
} catch (e: any) {
error = e?.message || 'Failed to load action output';
Expand All @@ -207,20 +231,16 @@

async function setupListeners() {
try {
// Listen for output events
// Listen for output events — batched via requestAnimationFrame
unlistenOutput = await listenToActionOutput((event: ActionOutputEvent) => {
if (event.executionId === executionId) {
outputChunks = [
...outputChunks,
{
chunk: event.chunk,
stream: event.stream,
timestamp: Date.now(),
},
];
// Auto-scroll to bottom if user is already at bottom
if (shouldAutoScroll) {
tick().then(() => scrollToBottom());
pendingChunks.push({
chunk: event.chunk,
stream: event.stream,
timestamp: Date.now(),
});
if (flushRaf === null) {
flushRaf = requestAnimationFrame(flushPendingChunks);
}
}
});
Expand All @@ -246,6 +266,11 @@
}

function cleanup() {
if (flushRaf !== null) {
cancelAnimationFrame(flushRaf);
flushRaf = null;
}
pendingChunks = [];
if (unlistenOutput) {
unlistenOutput();
unlistenOutput = null;
Expand Down Expand Up @@ -315,10 +340,14 @@
// =========================================================================

function renderLine(line: TerminalLine): string {
// Convert ANSI codes to HTML
const html = ansiConverter.toHtml(line.text);
// Sanitize the HTML to prevent XSS
return sanitize(html);
const text = line.text;
let cached = renderLineCache.get(text);
if (cached !== undefined) return cached;
// Convert ANSI codes to HTML, then sanitize to prevent XSS
const html = ansiConverter.toHtml(text);
cached = sanitize(html);
renderLineCache.set(text, cached);
return cached;
}

function getStatusIcon(s: ActionStatus) {
Expand Down
111 changes: 110 additions & 1 deletion apps/staged/src/lib/features/actions/processOutput.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect } from 'vitest';
import { processChunksToLines } from './processOutput';
import { processChunksToLines, createIncrementalProcessor } from './processOutput';
import type { OutputChunk } from './actions';

/** Helper to build an OutputChunk. */
Expand Down Expand Up @@ -115,3 +115,112 @@ describe('processChunksToLines', () => {
expect(texts([chunk('hel'), chunk('lo\nwor'), chunk('ld\n')])).toEqual(['hello', 'world']);
});
});

// ---------------------------------------------------------------------------
// createIncrementalProcessor
// ---------------------------------------------------------------------------

describe('createIncrementalProcessor', () => {
/** Helper: feed chunks one-at-a-time and return the final snapshot. */
function feedOneByOne(chunks: OutputChunk[]): string[] {
const proc = createIncrementalProcessor();
let result: ReturnType<typeof proc.process> = [];
for (const c of chunks) {
result = proc.process([c]);
}
return result.map((l) => l.text);
}

/** Helper: feed all chunks at once. */
function feedAll(chunks: OutputChunk[]): string[] {
const proc = createIncrementalProcessor();
return proc.process(chunks).map((l) => l.text);
}

// ---------------------------------------------------------------------------
// Parity with processChunksToLines
// ---------------------------------------------------------------------------

it('matches batch output for simple newlines', () => {
const chunks = [chunk('hello\nworld\n')];
expect(feedAll(chunks)).toEqual(texts(chunks));
});

it('matches batch output for \\r\\n', () => {
const chunks = [chunk('hello\r\nworld\r\n')];
expect(feedAll(chunks)).toEqual(texts(chunks));
});

it('matches batch output for bare \\r overwrites', () => {
const chunks = [chunk('10%\r20%\r30%\r40%\n')];
expect(feedAll(chunks)).toEqual(texts(chunks));
});

it('matches batch output for interleaved stdout/stderr', () => {
const chunks = [chunk('out\n', 'stdout'), chunk('err\n', 'stderr')];
const proc = createIncrementalProcessor();
let result = proc.process([chunks[0]]);
result = proc.process([chunks[1]]);
expect(result.map((l) => l.stream)).toEqual(['stdout', 'stderr']);
});

// ---------------------------------------------------------------------------
// Incremental accumulation
// ---------------------------------------------------------------------------

it('accumulates lines across multiple process() calls', () => {
const proc = createIncrementalProcessor();
proc.process([chunk('line1\n')]);
const result = proc.process([chunk('line2\n')]);
expect(result.map((l) => l.text)).toEqual(['line1', 'line2']);
});

it('shows in-progress line until finalized', () => {
const proc = createIncrementalProcessor();
let result = proc.process([chunk('partial')]);
expect(result.map((l) => l.text)).toEqual(['partial']);
result = proc.process([chunk(' more\n')]);
expect(result.map((l) => l.text)).toEqual(['partial more']);
});

// ---------------------------------------------------------------------------
// \\r\\n split across process() calls — the key correctness case
// ---------------------------------------------------------------------------

it('preserves line text when \\r\\n is split across calls', () => {
const proc = createIncrementalProcessor();
// First call ends with \r — we don't yet know if it's bare \r or \r\n
let result = proc.process([chunk('hello\r')]);
// The text should still be visible (not discarded)
expect(result.map((l) => l.text)).toEqual(['hello']);
// Second call starts with \n — confirms it was \r\n, line should be finalized
result = proc.process([chunk('\nworld\n')]);
expect(result.map((l) => l.text)).toEqual(['hello', 'world']);
});

it('bare \\r across calls correctly overwrites when followed by non-\\n', () => {
const proc = createIncrementalProcessor();
proc.process([chunk('old text\r')]);
const result = proc.process([chunk('new text\n')]);
expect(result.map((l) => l.text)).toEqual(['new text']);
});

it('multiple trailing \\r across calls resolve correctly', () => {
const proc = createIncrementalProcessor();
proc.process([chunk('10%\r')]);
proc.process([chunk('20%\r')]);
proc.process([chunk('30%\r')]);
const result = proc.process([chunk('done\n')]);
expect(result.map((l) => l.text)).toEqual(['done']);
});

// ---------------------------------------------------------------------------
// Batched chunks (multiple chunks in one process() call)
// ---------------------------------------------------------------------------

it('handles multiple chunks in a single process() call', () => {
const proc = createIncrementalProcessor();
const result = proc.process([chunk('hello\n'), chunk('world\n')]);
expect(result.map((l) => l.text)).toEqual(['hello', 'world']);
});
});
75 changes: 75 additions & 0 deletions apps/staged/src/lib/features/actions/processOutput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,78 @@ export function processChunksToLines(chunks: OutputChunk[]): TerminalLine[] {

return lines;
}

/**
* Incremental line processor that maintains state across calls.
*
* Unlike `processChunksToLines` (which reprocesses every chunk from scratch),
* this only processes *new* chunks each time `process()` is called, appending
* to an internal list of finalized lines. This turns the per-update cost from
* O(total-characters) to O(new-characters).
*/
export function createIncrementalProcessor() {
const finalizedLines: TerminalLine[] = [];
let currentText = '';
let currentStream: 'stdout' | 'stderr' = 'stdout';
let pendingCR = false;

return {
/**
* Feed new chunks and return the full (finalized + in-progress) line list.
*/
process(chunks: OutputChunk[]): TerminalLine[] {
for (const chunk of chunks) {
const raw = chunk.chunk;
const stream = chunk.stream;

for (let i = 0; i < raw.length; i++) {
const ch = raw[i];

if (pendingCR) {
pendingCR = false;
if (ch === '\n') {
finalizedLines.push({ text: currentText, stream: currentStream });
currentText = '';
currentStream = stream;
continue;
} else {
currentText = '';
currentStream = stream;
}
}

if (ch === '\n') {
finalizedLines.push({ text: currentText, stream: currentStream });
currentText = '';
currentStream = stream;
} else if (ch === '\r') {
if (i + 1 < raw.length && raw[i + 1] === '\n') {
finalizedLines.push({ text: currentText, stream: currentStream });
currentText = '';
currentStream = stream;
i++;
} else if (i + 1 < raw.length) {
currentText = '';
currentStream = stream;
} else {
pendingCR = true;
}
} else {
currentText += ch;
currentStream = stream;
}
}
}

if (pendingCR) {
// Don't discard — just return what we have; the bare CR will resolve
// on the next call when more data arrives (or on final snapshot).
}

if (currentText.length > 0) {
return [...finalizedLines, { text: currentText, stream: currentStream }];
Comment on lines +167 to +168

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clear pending carriage-return text before returning lines

createIncrementalProcessor.process still emits currentText when a chunk ends with a bare \r, because pendingCR is left unresolved at return-time. For progress-style output like "in progress\r", that text should be considered overwritten (the legacy parser explicitly cleared it), but this path keeps it visible and it can be treated as real output in the modal/save flow until another chunk arrives. This is a behavioral regression from the existing processChunksToLines semantics for trailing bare carriage returns.

Useful? React with 👍 / 👎.

}
return [...finalizedLines];
},
};
}