Skip to content

Commit c32db63

Browse files
committed
fix(agent): #102 review — stacked marks count, and a lone mark is not emphasis
Two findings, both on saysDone(), both right. 1. One mark was taken off a line start, where a model may stack several. "> ## **Done:** …" and "- > **Done:** …" were still read as unfinished, and with a "next " in the answer still nudged — the bug this branch is for, in another shape. Reproduced on the reviewed code through the loop: a second turn is requested for both. The line-start strip now takes every mark stacked there. 2. Every `*`, `_`, backtick and `~` was deleted before the line was looked for, and a lone mark is not emphasis: it is part of a name. A turn that said "Let me write config.yaml:" and then pasted ```yaml _done: false ``` was read as finished. develop nudges that turn; the reviewed code ended the run without the write it promised. So this was a regression the first commit made, and it reached further than YAML: a Python field, `_done: bool = False`, and a JS key, `_done: false,`, were read the same way. Emphasis is now unwrapped only as a pair, and only where it opens the line: an opening run of marks before a non-space, a closing run after one, and no word character after the closing run — so the `_` inside `default_factory` closes nothing. **Done:**, **Done**:, a whole sentence in bold or italics, `Done:` as code and **_Done:_** are all still found: 520 combinations of stacked marks, emphasis and where it closes were checked, and none is missed. Nine lone-mark lines that the reviewed code took for a Done line no longer are. Found while doing it: the line test could take seconds to say no. /(^|\n)\s*done/ lets the whitespace run across lines, so from every line start it read on through all the blank lines below. The work grows with the square of their number — 64,000 blank lines took 1.8 seconds on a quiet machine, 128,000 took 7 — and that is on develop, for a turn that degenerates into line breaks. This branch had widened it: once marks are stripped, a line of nothing but marks is a blank line too. The whitespace now stays on its line, /(^|\n)[^\S\n]*done/. It is the same test: over 5.6 million strings, exhaustive to six tokens and random beyond, the two never disagree. The same inputs now answer in milliseconds. The last-word test still reads the text as written, as decided. Its comment gave a reason that no longer holds — with pairs only, a fence's backticks are not taken away, so the pasted shell loop is nudged either way — and now gives the one that does: a turn whose last line is the quoted keyword, `done`. test/agentDone.test.js goes from 7 to 10 cases: stacked marks and every way a pair can close; lone marks, the pasted file among them; that pasted file through the real loop, still nudged; and the line test's pattern read out of the source, because no verdict can tell the two patterns apart and a clock in a test is a flake. It fails 4 cases on the reviewed commit. Thirteen mutations of the new code each fail a case. 50 suites, 972 cases, 0 failing (scripts/test-extensions.sh) on this commit's tree, on macOS with Node 24 and in a Linux container with Node 20. With the checkout's tsc, agent.js still reports develop's 94 diagnostics. Unchanged: a "- Done: parser" bullet in a progress list ends a run that also promises more work, and so does a YAML list item, "- done: false", in a pasted file. develop nudges both. They come with the list case, which reads a mark, not emphasis. A Done line behind an emoji is still not recognised. Not run in the editor against a live model.
1 parent 63249cb commit c32db63

2 files changed

Lines changed: 80 additions & 15 deletions

File tree

‎extensions/levelcode-ai/agent.js‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,7 @@ async function setupMcp(ctx, wsFolders, dbg) {
719719
* Did the model say it is finished — a line starting with "Done:" (what SYSTEM_BASE asks for), or
720720
* text that simply ends in "done"?
721721
*
722-
* The LINE is looked for with markdown marks stripped. A model that writes it as `**Done:** …`,
722+
* The LINE is looked for with its markdown marks taken off. A model that writes it as `**Done:** …`,
723723
* `## Done: …` or `- Done: …` has still said it; read literally, the marks sit between the line start
724724
* and the word, so a finished run was taken for an unfinished one and — when the answer also read as
725725
* a promise of more work, for which one "next " is enough — nudged until the model said Done a second
@@ -728,13 +728,20 @@ async function setupMcp(ctx, wsFolders, dbg) {
728728
* @returns {boolean}
729729
*/
730730
function saysDone(text) {
731-
// Strip quote, heading and list marks at line starts, then emphasis marks, before the line test.
732731
const plain = text
733-
.replace(/^[ \t]*(?:[>#]+|[-*+]|\d+\.)[ \t]+/gm, '')
734-
.replace(/[*_`~]+/g, '');
735-
// The last-word test reads the text AS WRITTEN. With the backticks gone, a turn that ends in a fenced
736-
// shell loop ends in "done" as well — and a command pasted where a tool call belonged is a stall.
737-
return /(^|\n)\s*done\s*:/i.test(plain) || /\bdone\.?\s*$/i.test(text.trim());
732+
// Quote, heading and list marks at a line start — as many as are stacked there (`> ## Done:`).
733+
.replace(/^[ \t]*(?:(?:[>#]+|[-*+]|\d+\.)[ \t]+)+/gm, '')
734+
// Then the emphasis that opens the line, and only as a PAIR: `**Done:** …`, `_Done: shipped._`.
735+
// A lone mark is part of a name. Deleting every `_` made a Done line of `_done: false` in a pasted
736+
// file, and the turn that pasted it instead of writing it was no longer nudged.
737+
.replace(/^[ \t]*[*_~`]+(?=\S)(.*?\S)[*_~`]+(?!\w)/gm, '$1');
738+
// `[^\S\n]` is whitespace that stays on its line — the same test as `\s*` there, at a different cost.
739+
// With `\s*` each line start read on through every blank line below it, and a line of nothing but
740+
// marks is blank by now: the work grew with the square of their number, and 64,000 took seconds.
741+
if (/(^|\n)[^\S\n]*done\s*:/i.test(plain)) { return true; }
742+
// The last-word test reads the text AS WRITTEN, as it always has. The marks come off to find a line;
743+
// taken off the last word too, a turn that ends by quoting a loop's `done` would pass for a finish.
744+
return /\bdone\.?\s*$/i.test(text.trim());
738745
}
739746

740747
async function runAgent(ctx) {

‎extensions/levelcode-ai/test/agentDone.test.js‎

Lines changed: 66 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,12 @@ const PASTED_LOOP = 'Let me run this to rename them:\n'
106106
+ 'done\n'
107107
+ '```';
108108

109+
// And a file pasted where a write_file call belonged. `_done` is a key: one mark, and nothing closes it.
110+
const PASTED_FILE = 'Let me write config.yaml:\n'
111+
+ '```yaml\n'
112+
+ '_done: false\n'
113+
+ '```';
114+
109115
(async () => {
110116
try {
111117
// ---- 1. the verdict --------------------------------------------------------------------------
@@ -126,6 +132,39 @@ const PASTED_LOOP = 'Let me run this to rename them:\n'
126132
}
127133
});
128134

135+
test('VERDICT: the marks may be stacked, and the emphasis may hold the word or the whole sentence', () => {
136+
const done = [
137+
// #102 review: one mark was taken off the line start, where a model may stack several.
138+
'> ## **Done:** quote, heading and bold',
139+
'- > **Done:** list, quote and bold',
140+
'> - Done: a quoted list item',
141+
// Emphasis is unwrapped as a pair, so every way of closing it has to be found.
142+
'**Done**: the colon outside the bold',
143+
'**Done: the whole sentence in bold.**',
144+
'_Done: the whole sentence in italics._',
145+
'**_Done:_** two kinds at once',
146+
// And the line may still sit below blank ones, indented — with marks or without.
147+
'The suite passes.\n\n\n\t Done: after blank lines, behind a tab and a space.',
148+
'The suite passes.\n\n **Done:** indented, and in no list.'
149+
];
150+
for (const text of done) {
151+
assert.strictEqual(saysDone(text), true, 'not read as finished: ' + JSON.stringify(text));
152+
}
153+
});
154+
155+
test('VERDICT: a lone mark belongs to a name — it is not emphasis', () => {
156+
// #102 review: every `_` used to be deleted, and that made a Done line of a key in a pasted file.
157+
// The second text is here for its later `_`: inside a word, it closes nothing.
158+
const names = [
159+
PASTED_FILE,
160+
'_done: bool = field(default_factory=bool)',
161+
'*done: x'
162+
];
163+
for (const text of names) {
164+
assert.strictEqual(saysDone(text), false, 'read as finished: ' + JSON.stringify(text));
165+
}
166+
});
167+
129168
test('VERDICT: the word alone, or no word at all, is not a Done line', () => {
130169
// Stripping marks must not turn "done" somewhere in a sentence into the line the prompt asks for.
131170
const notDone = [
@@ -143,10 +182,12 @@ const PASTED_LOOP = 'Let me run this to rename them:\n'
143182
assert.strictEqual(saysDone('The diagram is in docs/flow.html and it renders. All done.'), true);
144183
});
145184

146-
test('VERDICT: the "done" that closes a shell loop is not the model saying it', () => {
147-
// Why that second clause reads the text as written, and only the Done LINE is looked for with the
148-
// marks stripped: take the fence's backticks away and this turn ends in the word.
185+
test('VERDICT: a "done" that is code is not the model saying it', () => {
186+
// The shell's own keyword, closing a pasted loop — and the same keyword quoted on a turn's last
187+
// line. The second is why that clause reads the text as written: the marks come off to find a
188+
// LINE, and taken off here as well they would leave this turn ending in the word.
149189
assert.strictEqual(saysDone(PASTED_LOOP), false);
190+
assert.strictEqual(saysDone('Let me check how the loop is closed. Its last line is:\n`done`'), false);
150191
});
151192

152193
// ---- 2. the loop asks it ---------------------------------------------------------------------
@@ -171,11 +212,28 @@ const PASTED_LOOP = 'Let me run this to rename them:\n'
171212
assert.deepStrictEqual(r.end, ['done']);
172213
});
173214

174-
await testAsync('LOOP: a command pasted in a fence is still nudged, though its last word is "done"', async () => {
175-
const r = await run([PASTED_LOOP, 'Done: renamed the files.']);
176-
assert.strictEqual(r.nudges, 1, 'a pasted command passed for a finished run');
177-
assert.strictEqual(r.asked, 2);
178-
assert.deepStrictEqual(r.end, ['done']);
215+
await testAsync('LOOP: code pasted in a fence is still nudged, whatever word or key it holds', async () => {
216+
for (const pasted of [PASTED_LOOP, PASTED_FILE]) {
217+
const r = await run([pasted, 'Done: it is on disk now.']);
218+
assert.strictEqual(r.nudges, 1, 'pasted code passed for a finished run: ' + JSON.stringify(pasted));
219+
assert.strictEqual(r.asked, 2);
220+
assert.deepStrictEqual(r.end, ['done']);
221+
}
222+
});
223+
224+
// ---- 3. the cost of asking -------------------------------------------------------------------
225+
226+
test('SOURCE: the line test does not read on across blank lines', () => {
227+
// `(^|\n)\s*done` and `(^|\n)[^\S\n]*done` accept exactly the same texts. The first one reads from
228+
// every line start through all the blank lines below it, so its work grows with the square of
229+
// their number: a turn that is mostly line breaks — or mostly lines of bare marks, once those are
230+
// stripped — took seconds at 64,000 lines. No verdict can tell the two apart, and a clock in a
231+
// test is a flake; so the pattern itself is read out of the source, as agentNoWorkspace.test.js
232+
// reads NEEDS_ROOT.
233+
const src = fs.readFileSync(path.join(__dirname, '..', 'agent.js'), 'utf8');
234+
const m = /\/\(\^\|\\n\)(.*?)done\\s\*:\/i\.test\(plain\)/.exec(src);
235+
assert.ok(m, 'agent.js no longer tests for the Done line where this suite looks');
236+
assert.ok(!m[1].includes('\\s'), 'the line test may match whitespace across lines again: ' + m[1]);
179237
});
180238
} finally {
181239
fs.rmSync(root, { recursive: true, force: true });

0 commit comments

Comments
 (0)