Merge PR #4056: fix(tui): smooth write streaming previews (@roboomp)
This commit is contained in:
can1357
@@ -18,6 +18,9 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed the advisor docs and system prompt still describing a hard read-only observer after `WATCHDOG.yml` `tools:` grants were opened to every built-in tool (including `edit`, `write`, `bash`, `eval`, `browser`). `docs/advisor-watchdog.md` now documents the `WATCHDOG.yml` roster format, the default `read`/`grep`/`glob` grant, and the safety implications of broadening the grant; the advisor system prompt no longer asserts read-only access. ([#4044](https://github.com/can1357/oh-my-pi/issues/4044))
|
||||
### Fixed
|
||||
|
||||
- Fixed long streaming previews for `write` (`content`), `edit` (`input`), and `eval` (`code`) updating in throttle-sized chunks by decoding the streamed string arguments incrementally between full JSON re-parses. ([#4043](https://github.com/can1357/oh-my-pi/issues/4043))
|
||||
|
||||
## [16.2.12] - 2026-07-01
|
||||
|
||||
|
||||
@@ -51,6 +51,21 @@ const IDLE_RECAP_MIN_SECONDS = 1;
|
||||
const IDLE_RECAP_MAX_SECONDS = 3600;
|
||||
|
||||
const RAW_PARTIAL_JSON_RENDERERS: Record<string, true> = { bash: true, edit: true, apply_patch: true };
|
||||
// Top-level string args a renderer reads mid-stream. The reveal controller
|
||||
// decodes these fields incrementally between throttled full-JSON parses so a
|
||||
// long payload updates preview args at reveal cadence instead of stalling for
|
||||
// STREAMING_JSON_PARSE_MIN_GROWTH bytes at a time. Nested-array modes (edit
|
||||
// patch/replace `edits[].diff`) still fall through to the throttled parse.
|
||||
const STREAMING_STRING_KEYS_BY_TOOL: Record<string, readonly string[]> = {
|
||||
write: ["content"],
|
||||
edit: ["input"],
|
||||
eval: ["code"],
|
||||
};
|
||||
|
||||
function streamingStringKeysForTool(toolName: string, rawInput: boolean): readonly string[] | undefined {
|
||||
if (rawInput) return undefined;
|
||||
return STREAMING_STRING_KEYS_BY_TOOL[toolName];
|
||||
}
|
||||
|
||||
function exposesRawPartialJson(toolName: string, rawInput: boolean, tool: unknown): boolean {
|
||||
if (rawInput) return true;
|
||||
@@ -647,6 +662,7 @@ export class EventController {
|
||||
rawInput,
|
||||
exposeRawPartialJson: exposesRawPartialJson(content.name, rawInput, tool),
|
||||
fullArgs: content.arguments,
|
||||
streamingStringKeys: streamingStringKeysForTool(content.name, rawInput),
|
||||
});
|
||||
} else {
|
||||
this.#toolArgsReveal.finish(content.id);
|
||||
|
||||
@@ -11,6 +11,258 @@ type ToolArgsRevealControllerOptions = {
|
||||
requestRender(): void;
|
||||
};
|
||||
|
||||
type StreamingJsonStringExtractorResult = {
|
||||
values: Record<string, string>;
|
||||
changed: boolean;
|
||||
};
|
||||
|
||||
function decodeJsonStringEscape(ch: string): string {
|
||||
switch (ch) {
|
||||
case '"':
|
||||
case "\\":
|
||||
case "/":
|
||||
return ch;
|
||||
case "b":
|
||||
return "\b";
|
||||
case "f":
|
||||
return "\f";
|
||||
case "n":
|
||||
return "\n";
|
||||
case "r":
|
||||
return "\r";
|
||||
case "t":
|
||||
return "\t";
|
||||
default:
|
||||
return ch;
|
||||
}
|
||||
}
|
||||
|
||||
function isHexDigit(ch: string): boolean {
|
||||
return (ch >= "0" && ch <= "9") || (ch >= "a" && ch <= "f") || (ch >= "A" && ch <= "F");
|
||||
}
|
||||
|
||||
type StreamingJsonStringExtractorState = "scan" | "candidate" | "afterCandidate" | "beforeValue" | "target";
|
||||
|
||||
class StreamingJsonStringExtractor {
|
||||
readonly #keys: Set<string>;
|
||||
#source = "";
|
||||
#offset = 0;
|
||||
#state: StreamingJsonStringExtractorState = "scan";
|
||||
#candidate = "";
|
||||
#candidateEscaped = false;
|
||||
#candidateUnicode = "";
|
||||
#matchedKey: string | undefined;
|
||||
#targetKey: string | undefined;
|
||||
#targetEscaped = false;
|
||||
#targetUnicode = "";
|
||||
#values: Record<string, string> = {};
|
||||
#changed = false;
|
||||
|
||||
constructor(keys: readonly string[]) {
|
||||
this.#keys = new Set(keys);
|
||||
}
|
||||
|
||||
reset(): void {
|
||||
this.#source = "";
|
||||
this.#offset = 0;
|
||||
this.#state = "scan";
|
||||
this.#candidate = "";
|
||||
this.#candidateEscaped = false;
|
||||
this.#candidateUnicode = "";
|
||||
this.#matchedKey = undefined;
|
||||
this.#targetKey = undefined;
|
||||
this.#targetEscaped = false;
|
||||
this.#targetUnicode = "";
|
||||
this.#values = {};
|
||||
this.#changed = false;
|
||||
}
|
||||
|
||||
update(prefix: string): StreamingJsonStringExtractorResult {
|
||||
if (!prefix.startsWith(this.#source)) {
|
||||
this.reset();
|
||||
}
|
||||
this.#source = prefix;
|
||||
this.#changed = false;
|
||||
while (this.#offset < prefix.length) {
|
||||
const ch = prefix[this.#offset]!;
|
||||
switch (this.#state) {
|
||||
case "scan":
|
||||
this.#scan(ch);
|
||||
break;
|
||||
case "candidate":
|
||||
this.#readCandidate(ch);
|
||||
break;
|
||||
case "afterCandidate":
|
||||
this.#afterCandidate(ch);
|
||||
break;
|
||||
case "beforeValue":
|
||||
this.#beforeValue(ch);
|
||||
break;
|
||||
case "target":
|
||||
this.#readTarget(ch);
|
||||
break;
|
||||
}
|
||||
}
|
||||
return { values: { ...this.#values }, changed: this.#changed };
|
||||
}
|
||||
|
||||
#scan(ch: string): void {
|
||||
if (ch === '"') {
|
||||
this.#candidate = "";
|
||||
this.#candidateEscaped = false;
|
||||
this.#candidateUnicode = "";
|
||||
this.#state = "candidate";
|
||||
}
|
||||
this.#offset++;
|
||||
}
|
||||
|
||||
#readCandidate(ch: string): void {
|
||||
if (this.#candidateUnicode) {
|
||||
this.#readCandidateUnicode(ch);
|
||||
return;
|
||||
}
|
||||
if (this.#candidateEscaped) {
|
||||
if (ch === "u") {
|
||||
this.#candidateUnicode = "u";
|
||||
} else {
|
||||
this.#candidate += decodeJsonStringEscape(ch);
|
||||
this.#candidateEscaped = false;
|
||||
}
|
||||
this.#offset++;
|
||||
return;
|
||||
}
|
||||
if (ch === "\\") {
|
||||
this.#candidateEscaped = true;
|
||||
this.#offset++;
|
||||
return;
|
||||
}
|
||||
if (ch === '"') {
|
||||
this.#matchedKey = this.#keys.has(this.#candidate) ? this.#candidate : undefined;
|
||||
this.#state = "afterCandidate";
|
||||
this.#offset++;
|
||||
return;
|
||||
}
|
||||
this.#candidate += ch;
|
||||
this.#offset++;
|
||||
}
|
||||
|
||||
#readCandidateUnicode(ch: string): void {
|
||||
if (isHexDigit(ch)) {
|
||||
this.#candidateUnicode += ch;
|
||||
if (this.#candidateUnicode.length === 5) {
|
||||
this.#candidate += String.fromCharCode(Number.parseInt(this.#candidateUnicode.slice(1), 16));
|
||||
this.#candidateUnicode = "";
|
||||
this.#candidateEscaped = false;
|
||||
}
|
||||
} else {
|
||||
this.#candidate += this.#candidateUnicode + ch;
|
||||
this.#candidateUnicode = "";
|
||||
this.#candidateEscaped = false;
|
||||
}
|
||||
this.#offset++;
|
||||
}
|
||||
|
||||
#afterCandidate(ch: string): void {
|
||||
if (/\s/.test(ch)) {
|
||||
this.#offset++;
|
||||
return;
|
||||
}
|
||||
const matchedKey = this.#matchedKey;
|
||||
this.#matchedKey = undefined;
|
||||
if (ch === ":" && matchedKey) {
|
||||
this.#targetKey = matchedKey;
|
||||
this.#state = "beforeValue";
|
||||
this.#offset++;
|
||||
return;
|
||||
}
|
||||
this.#state = "scan";
|
||||
}
|
||||
|
||||
#beforeValue(ch: string): void {
|
||||
if (/\s/.test(ch)) {
|
||||
this.#offset++;
|
||||
return;
|
||||
}
|
||||
if (ch === '"' && this.#targetKey) {
|
||||
if (this.#values[this.#targetKey]) {
|
||||
this.#values[this.#targetKey] = "";
|
||||
this.#changed = true;
|
||||
}
|
||||
this.#targetEscaped = false;
|
||||
this.#targetUnicode = "";
|
||||
this.#state = "target";
|
||||
this.#offset++;
|
||||
return;
|
||||
}
|
||||
this.#targetKey = undefined;
|
||||
this.#state = "scan";
|
||||
}
|
||||
|
||||
#readTarget(ch: string): void {
|
||||
if (this.#targetUnicode) {
|
||||
this.#readTargetUnicode(ch);
|
||||
return;
|
||||
}
|
||||
if (this.#targetEscaped) {
|
||||
if (ch === "u") {
|
||||
this.#targetUnicode = "u";
|
||||
} else {
|
||||
this.#appendTarget(decodeJsonStringEscape(ch));
|
||||
this.#targetEscaped = false;
|
||||
}
|
||||
this.#offset++;
|
||||
return;
|
||||
}
|
||||
if (ch === "\\") {
|
||||
this.#targetEscaped = true;
|
||||
this.#offset++;
|
||||
return;
|
||||
}
|
||||
if (ch === '"') {
|
||||
this.#targetKey = undefined;
|
||||
this.#state = "scan";
|
||||
this.#offset++;
|
||||
return;
|
||||
}
|
||||
this.#appendTarget(ch);
|
||||
this.#offset++;
|
||||
}
|
||||
|
||||
#readTargetUnicode(ch: string): void {
|
||||
if (isHexDigit(ch)) {
|
||||
this.#targetUnicode += ch;
|
||||
if (this.#targetUnicode.length === 5) {
|
||||
this.#appendTarget(String.fromCharCode(Number.parseInt(this.#targetUnicode.slice(1), 16)));
|
||||
this.#targetUnicode = "";
|
||||
this.#targetEscaped = false;
|
||||
}
|
||||
} else {
|
||||
this.#appendTarget(this.#targetUnicode + ch);
|
||||
this.#targetUnicode = "";
|
||||
this.#targetEscaped = false;
|
||||
}
|
||||
this.#offset++;
|
||||
}
|
||||
|
||||
#appendTarget(text: string): void {
|
||||
if (!this.#targetKey || text.length === 0) return;
|
||||
this.#values[this.#targetKey] = `${this.#values[this.#targetKey] ?? ""}${text}`;
|
||||
this.#changed = true;
|
||||
}
|
||||
}
|
||||
|
||||
function createStringExtractor(keys: readonly string[] | undefined): StreamingJsonStringExtractor | undefined {
|
||||
return keys && keys.length > 0 ? new StreamingJsonStringExtractor(keys) : undefined;
|
||||
}
|
||||
|
||||
function sameStringKeys(a: readonly string[], b: readonly string[] | undefined): boolean {
|
||||
if (a.length !== (b?.length ?? 0)) return false;
|
||||
for (let i = 0; i < a.length; i++) {
|
||||
if (a[i] !== b?.[i]) return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
type RevealEntry = {
|
||||
component: ToolArgsRevealComponent | undefined;
|
||||
/** Latest raw streamed argument text (JSON for function tools, raw text for custom tools). */
|
||||
@@ -29,6 +281,9 @@ type RevealEntry = {
|
||||
displayArgs: Record<string, unknown>;
|
||||
/** Raw prefix carried by `displayArgs.__partialJson`. */
|
||||
displayPrefix: string;
|
||||
/** JSON string fields decoded incrementally between full JSON parses. */
|
||||
streamingStringKeys: readonly string[];
|
||||
stringExtractor: StreamingJsonStringExtractor | undefined;
|
||||
};
|
||||
|
||||
/** Clamp a slice end into `text`, never splitting a surrogate pair: a prefix
|
||||
@@ -46,6 +301,7 @@ type ToolArgsRevealTarget = {
|
||||
rawInput: boolean;
|
||||
exposeRawPartialJson: boolean;
|
||||
fullArgs: Record<string, unknown>;
|
||||
streamingStringKeys?: readonly string[];
|
||||
};
|
||||
|
||||
type DisplayArgsStep = {
|
||||
@@ -62,6 +318,7 @@ function resetDisplayState(entry: RevealEntry): void {
|
||||
entry.parsedLen = 0;
|
||||
entry.displayArgs = initialDisplayArgs();
|
||||
entry.displayPrefix = "";
|
||||
entry.stringExtractor?.reset();
|
||||
}
|
||||
|
||||
/** Display args for a revealed prefix. Function-tool JSON is parsed at the same
|
||||
@@ -91,6 +348,11 @@ function displayArgsForPrefix(entry: RevealEntry, prefix: string, forceParse = f
|
||||
parsedChanged = true;
|
||||
}
|
||||
}
|
||||
const extracted = entry.stringExtractor?.update(prefix);
|
||||
if (extracted?.changed) {
|
||||
entry.parsedArgs = { ...entry.parsedArgs, ...extracted.values };
|
||||
parsedChanged = true;
|
||||
}
|
||||
|
||||
const rawPrefixChanged = entry.exposeRawPartialJson && prefix !== entry.displayPrefix;
|
||||
if (!parsedChanged && !rawPrefixChanged) return { args: entry.displayArgs, changed: false };
|
||||
@@ -133,7 +395,7 @@ export class ToolArgsRevealController {
|
||||
* through in the caller's legacy shape (`{ ...args, __partialJson }`).
|
||||
*/
|
||||
setTarget(id: string, partialJson: string, target: ToolArgsRevealTarget): Record<string, unknown> {
|
||||
const { rawInput, exposeRawPartialJson, fullArgs } = target;
|
||||
const { rawInput, exposeRawPartialJson, fullArgs, streamingStringKeys } = target;
|
||||
if (!this.#getSmoothStreaming()) {
|
||||
// Toggle may flip mid-call: drop any live entry so ticks stop.
|
||||
this.#entries.delete(id);
|
||||
@@ -151,13 +413,21 @@ export class ToolArgsRevealController {
|
||||
parsedLen: 0,
|
||||
displayArgs: initialDisplayArgs(),
|
||||
displayPrefix: "",
|
||||
streamingStringKeys: streamingStringKeys ?? [],
|
||||
stringExtractor: createStringExtractor(streamingStringKeys),
|
||||
};
|
||||
this.#entries.set(id, entry);
|
||||
} else {
|
||||
if (entry.rawInput !== rawInput || entry.exposeRawPartialJson !== exposeRawPartialJson) {
|
||||
if (
|
||||
entry.rawInput !== rawInput ||
|
||||
entry.exposeRawPartialJson !== exposeRawPartialJson ||
|
||||
!sameStringKeys(entry.streamingStringKeys, streamingStringKeys)
|
||||
) {
|
||||
entry.rawInput = rawInput;
|
||||
entry.exposeRawPartialJson = exposeRawPartialJson;
|
||||
resetDisplayState(entry);
|
||||
entry.streamingStringKeys = streamingStringKeys ?? [];
|
||||
entry.stringExtractor = createStringExtractor(streamingStringKeys);
|
||||
}
|
||||
// Streams only append; a non-prefix target means a rewind — snap into range.
|
||||
if (!partialJson.startsWith(entry.target)) {
|
||||
|
||||
@@ -34,11 +34,18 @@ function drain(frames: number): void {
|
||||
}
|
||||
}
|
||||
|
||||
function jsonTarget(options: { fullArgs?: Record<string, unknown>; exposeRawPartialJson?: boolean } = {}) {
|
||||
function jsonTarget(
|
||||
options: {
|
||||
fullArgs?: Record<string, unknown>;
|
||||
exposeRawPartialJson?: boolean;
|
||||
streamingStringKeys?: readonly string[];
|
||||
} = {},
|
||||
) {
|
||||
return {
|
||||
rawInput: false,
|
||||
exposeRawPartialJson: options.exposeRawPartialJson ?? false,
|
||||
fullArgs: options.fullArgs ?? {},
|
||||
streamingStringKeys: options.streamingStringKeys,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -136,6 +143,67 @@ describe("tool args reveal", () => {
|
||||
expect(secondPartial.length - firstPartial.length).toBeGreaterThanOrEqual(STREAMING_JSON_PARSE_MIN_GROWTH);
|
||||
});
|
||||
|
||||
it("refreshes parsed write content on raw-prefix frames below the JSON parse throttle", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const initialContent = "x".repeat(STREAMING_JSON_PARSE_MIN_GROWTH + 24);
|
||||
const appendedJsonContent = "line 1\\nline 2\\n";
|
||||
const appendedContent = "line 1\nline 2\n";
|
||||
const initial = `{"path":"a.ts","content":"${initialContent}`;
|
||||
const next = `${initial}${appendedJsonContent}`;
|
||||
|
||||
const renderArgs = controller.setTarget(
|
||||
"call-1",
|
||||
initial,
|
||||
jsonTarget({ exposeRawPartialJson: true, streamingStringKeys: ["content"] }),
|
||||
);
|
||||
expect(renderArgs.content).toBe(initialContent);
|
||||
controller.bind("call-1", component);
|
||||
|
||||
controller.setTarget(
|
||||
"call-1",
|
||||
next,
|
||||
jsonTarget({ exposeRawPartialJson: true, streamingStringKeys: ["content"] }),
|
||||
);
|
||||
drain(20);
|
||||
|
||||
const latest = component.frames.at(-1);
|
||||
expect(latest).toBeDefined();
|
||||
expect(partialOf(latest!)).toBe(next);
|
||||
expect(latest!.content).toBe(`${initialContent}${appendedContent}`);
|
||||
});
|
||||
|
||||
it("extracts multiple string keys concurrently across throttled JSON parses", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
// Two long fields (edit-style `input`, write-style `content`) inside the
|
||||
// same JSON. A sub-throttle append must refresh BOTH decoded values on the
|
||||
// next reveal frame — proving the extractor generalizes past `content`.
|
||||
const initialInput = "y".repeat(STREAMING_JSON_PARSE_MIN_GROWTH + 8);
|
||||
const initialContent = "x".repeat(STREAMING_JSON_PARSE_MIN_GROWTH + 16);
|
||||
const initial = `{"input":"${initialInput}","content":"${initialContent}`;
|
||||
const appendedContent = "tail";
|
||||
const next = `${initial}${appendedContent}`;
|
||||
|
||||
const streamingStringKeys = ["input", "content"];
|
||||
const renderArgs = controller.setTarget(
|
||||
"call-1",
|
||||
initial,
|
||||
jsonTarget({ exposeRawPartialJson: true, streamingStringKeys }),
|
||||
);
|
||||
expect(renderArgs.input).toBe(initialInput);
|
||||
expect(renderArgs.content).toBe(initialContent);
|
||||
controller.bind("call-1", component);
|
||||
|
||||
controller.setTarget("call-1", next, jsonTarget({ exposeRawPartialJson: true, streamingStringKeys }));
|
||||
drain(20);
|
||||
|
||||
const latest = component.frames.at(-1);
|
||||
expect(latest).toBeDefined();
|
||||
expect(latest!.input).toBe(initialInput);
|
||||
expect(latest!.content).toBe(`${initialContent}${appendedContent}`);
|
||||
});
|
||||
|
||||
it("passes the full target through untouched when smoothing is disabled", () => {
|
||||
vi.useFakeTimers();
|
||||
const requestRender = vi.fn();
|
||||
|
||||
Reference in New Issue
Block a user