fix(ai): batch_design parser accepts pretty-printed multi-line JSON

executeLine's three parse regexes (assign / bindless / call) missed
the `s` flag, so `.+` stopped at the first newline and rejected any
pretty-printed JSON body — even though splitOperations already
groups balanced `()`/`[]`/`{}` spans into one logical line. Kimi K2.5
was primed into this style by elements.md examples, tripping the
latent bug on 3/24 A/B prompts; baseline models never pretty-printed
so the bug stayed masked. Regression test locks the fix: bound +
bindless insert + bound update with newline-embedded bodies now
parse, and a genuinely malformed single-line input still surfaces
as an error instead of being silently swallowed.

See openpencil-docs/superpowers/notes/2026-04-21-kimi-k25-regression-rca.md.
This commit is contained in:
Fini 2026-04-21 22:03:11 +08:00
parent 0eb5ca1ada
commit 05f3044648
2 changed files with 117 additions and 3 deletions

View file

@ -0,0 +1,104 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { writeFile, unlink, readFile, mkdir } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { handleBatchDesign } from '../tools/batch-design';
import { invalidateCache } from '../document-manager';
/**
* Regression for the 2026-04-21 Kimi K2.5 A/B regression: pretty-printed
* multi-line JSON inside `I(...)` / `U(...)` / etc. must parse. `splitOperations`
* already preserves multi-line balanced bodies; `executeLine`'s parse regex
* had `.+` without the `s` flag, so `.` stopped at the first `\n` and the
* whole line was mis-classified as unparseable. See
* openpencil-docs/superpowers/notes/2026-04-21-kimi-k25-regression-rca.md.
*/
describe('batch_design — multi-line pretty-printed body (Kimi K2.5 regression)', () => {
const TMP = join(tmpdir(), 'openpencil-batch-design-multiline');
const EMPTY = JSON.stringify({ version: '1.0.0', children: [] });
beforeEach(async () => {
await mkdir(TMP, { recursive: true });
});
afterEach(async () => {
for (const f of ['a.op']) {
try {
const fp = join(TMP, f);
invalidateCache(fp);
await unlink(fp);
} catch {}
}
});
async function fresh(name: string): Promise<string> {
const fp = join(TMP, name);
await writeFile(fp, EMPTY, 'utf-8');
return fp;
}
it('accepts an insert with pretty-printed multi-line JSON body', async () => {
const fp = await fresh('a.op');
const operations = [
'root=I(null, {',
' "type": "frame",',
' "name": "Root",',
' "width": 375,',
' "height": 200,',
' "layout": "vertical"',
'})',
].join('\n');
const res = await handleBatchDesign({ filePath: fp, operations, postProcess: false });
expect(res.errors).toBeUndefined();
expect(res.results.length).toBe(1);
const saved = JSON.parse(await readFile(fp, 'utf-8'));
const children = (saved.children ?? saved.pages?.[0]?.children) as Record<string, unknown>[];
expect(children.length).toBe(1);
expect(children[0].name).toBe('Root');
expect(children[0].layout).toBe('vertical');
});
it('accepts bindless insert with multi-line body (Kimi style)', async () => {
const fp = await fresh('a.op');
const operations =
'I(null, {\n "type": "frame",\n "name": "NoBind",\n "width": 100,\n "height": 100\n})';
const res = await handleBatchDesign({ filePath: fp, operations, postProcess: false });
expect(res.errors).toBeUndefined();
expect(res.results.length).toBe(1);
const saved = JSON.parse(await readFile(fp, 'utf-8'));
const children = (saved.children ?? saved.pages?.[0]?.children) as Record<string, unknown>[];
expect(children[0].name).toBe('NoBind');
});
it('accepts update call with multi-line patch body (insert + update in one batch)', async () => {
const fp = await fresh('a.op');
// Bindings live in the per-handleBatchDesign Map — insert + update must
// be in the same call for the `root` binding to resolve on U(). In a
// real agent flow the two lines arrive concatenated in one payload.
const operations = [
'root=I(null, {"type":"frame","name":"Pre","width":100,"height":100})',
'U(root, {',
' "name": "After",',
' "width": 200',
'})',
].join('\n');
const res = await handleBatchDesign({ filePath: fp, operations, postProcess: false });
expect(res.errors).toBeUndefined();
const saved = JSON.parse(await readFile(fp, 'utf-8'));
const children = (saved.children ?? saved.pages?.[0]?.children) as Record<string, unknown>[];
expect(children[0].name).toBe('After');
expect(children[0].width).toBe(200);
});
it('still rejects genuinely malformed single-line input', async () => {
const fp = await fresh('a.op');
// Missing the closing `)` — should surface as an error, not silently pass
const res = await handleBatchDesign({
filePath: fp,
operations: 'x=I(null, {"type":"frame","name":"Bad"',
postProcess: false,
});
// splitOperations keeps unclosed lines; executeLine can't match → error collected
expect(res.errors).toBeDefined();
expect(res.errors!.length).toBeGreaterThan(0);
});
});

View file

@ -165,9 +165,19 @@ async function executeLine(
// Parse: binding=OP(args) or OP(args)
// Binding is optional for I/C/R/G — auto-generated when omitted (agents
// sometimes write `I(parent, data)` without the binding prefix).
const assignMatch = line.match(/^(\w+)\s*=\s*([ICRMG])\((.+)\)$/);
const bindlessAssignMatch = !assignMatch && line.match(/^([ICRG])\((.+)\)$/);
const callMatch = line.match(/^([UDM])\((.+)\)$/);
//
// `s` flag (dotAll) is REQUIRED: splitOperations above yields one logical
// line per top-level op and does NOT split on newlines inside balanced
// `()`/`[]`/`{}`. Pretty-printed JSON bodies (e.g. Kimi K2.5 in the
// 2026-04-21 A/B regression) carry literal `\n` inside the arg list; a
// `.+` without `s` stops at the first newline and mis-fails the whole
// line. See
// openpencil-docs/superpowers/notes/2026-04-21-kimi-k25-regression-rca.md
// for the exact failure mode. `^`/`$` still anchor to string boundaries
// (no `m` flag), which is what we want.
const assignMatch = line.match(/^(\w+)\s*=\s*([ICRMG])\((.+)\)$/s);
const bindlessAssignMatch = !assignMatch && line.match(/^([ICRG])\((.+)\)$/s);
const callMatch = line.match(/^([UDM])\((.+)\)$/s);
// Normalize bindless form to an auto-generated binding so the rest of the
// logic below can treat it uniformly.