diff --git a/app/src/domain/gloss.ts b/app/src/domain/gloss.ts index 6e8d9a8..1c59d7b 100644 --- a/app/src/domain/gloss.ts +++ b/app/src/domain/gloss.ts @@ -1,4 +1,29 @@ -/* Multi-sentence ::gloss blocks. +/* Two workarounds for lib/blocks.js, which ships unchanged. + + ── 1. Directive markup leaking into the message body ────────────────── + + parse() builds `body` by removing each block it recognised, but it does + the removals in an order that defeats one of them: + + if (w) body = body.slice(0, body.indexOf("::words")); // truncate + if (t) body = body.replace(t[0], ""); // substring + + RE.task's terminator `(?:\n::|$)` is INSIDE the match, so t[0] ends with + the "\n::" of whatever block follows. When that block is ::words, the + truncation on the line above has already cut those two colons off, the + substring no longer occurs, replace() finds nothing, and the entire task + block stays in the body — rendered as prose above the exercise it was + supposed to become. + + It only bites when ::task precedes ::words. The stub tutor emits them the + other way round, which is why this survived every test until a real model + chose the other order. Nothing in the prompt requires one. + + The fix here: drop anything from the first surviving directive line on. + Safe precisely because parse() has already removed the blocks it handled + correctly, so a `::` left in the body is by definition one that leaked. + + ── 2. Multi-sentence ::gloss blocks ─────────────────────────────────── The system prompt tells the tutor it may put several sentences in one gloss block, each closed by its own "=" line. lib/blocks.js parse() sets @@ -43,15 +68,28 @@ function splitSentences(body: string): string[] { * parse(), with multi-sentence gloss blocks split correctly. * Use this everywhere instead of calling parse() directly. */ +/** + * Cut the body at the first directive line that survived parse(). + * + * Exported for the test that pins the lib bug this exists for. + */ +export function stripLeakedBlocks(body: string): string { + const lines = body.split("\n"); + const at = lines.findIndex((l) => l.trimStart().startsWith("::")); + return at === -1 ? body : lines.slice(0, at).join("\n").trimEnd(); +} + export function parseMessage(text: string): ParsedMessage { const parsed = parse(text); - if (!parsed.gloss) return parsed; + const body = stripLeakedBlocks(parsed.body); + + if (!parsed.gloss) return { ...parsed, body }; const match = text.match(GLOSS_BLOCK); - if (!match?.[1]) return parsed; + if (!match?.[1]) return { ...parsed, body }; const sentences = splitSentences(match[1]); - if (sentences.length < 2) return parsed; // the common case; nothing to fix + if (sentences.length < 2) return { ...parsed, body }; // common case const blocks: GlossBlock[] = []; for (const s of sentences) { @@ -59,7 +97,7 @@ export function parseMessage(text: string): ParsedMessage { if (one.gloss) blocks.push(...one.gloss); } - return blocks.length ? { ...parsed, gloss: blocks } : parsed; + return blocks.length ? { ...parsed, body, gloss: blocks } : { ...parsed, body }; } export type { GlossBlock, ParsedMessage }; diff --git a/app/src/ui/tutor/TutorTab.tsx b/app/src/ui/tutor/TutorTab.tsx index fd32d04..ee29643 100644 --- a/app/src/ui/tutor/TutorTab.tsx +++ b/app/src/ui/tutor/TutorTab.tsx @@ -368,15 +368,12 @@ export function TutorTab() { on every unrelated re-render of this tab. */ const streamingBody = useMemo(() => { if (streaming === null) return ""; - /* A directive line arrives before the block it opens is complete, and - until then parse() has no reason to treat it as anything but prose — - so "::task match" and "::words" rendered as visible text for exactly - one frame and then vanished. That blink was the flicker. Blocks are - not rendered during streaming anyway, so drop the markers outright - and the preview only ever grows. */ - const lines = parseMessage(streaming) - .body.split("\n") - .filter((l) => !l.trimStart().startsWith("::")); + /* parseMessage() drops directive markup from the body, which during a + stream also covers the half-arrived kind: a directive line shows up + before the block it opens is complete, and until then parse() has no + reason to treat it as anything but prose. That is what made + "::task match" appear for one frame and vanish. */ + const lines = parseMessage(streaming).body.split("\n"); /* MessageBody renders a blank line as a 9px gap. Dropping a directive leaves the blank line that preceded it at the end of the preview, so a gap opened and closed on every block boundary. */ @@ -478,10 +475,19 @@ export function TutorTab() { return (
{you ? "나" : "선생님"} -
- - {parsed?.gloss && } -
+ {/* A turn can be nothing but blocks — some models write no + prose around an exercise at all. Rendering the bubble + anyway left an empty box above it. */} + {(() => { + const body = parsed ? parsed.body : t.body; + if (!body.trim() && !parsed?.gloss) return null; + return ( +
+ + {parsed?.gloss && } +
+ ); + })()} {/* An answered exercise stays rendered, read-only. It used to collapse to a single line, which threw away what the @@ -511,7 +517,10 @@ export function TutorTab() { {busy && (
선생님 - {streaming === null ? ( + {/* Keep the dots up while the reply so far is only block + markup: there is genuinely nothing to read yet, and an + empty bubble reads as a failure rather than as waiting. */} + {streamingBody.trim() === "" ? (
diff --git a/test/domain/block-leak.test.ts b/test/domain/block-leak.test.ts new file mode 100644 index 0000000..af2fc88 --- /dev/null +++ b/test/domain/block-leak.test.ts @@ -0,0 +1,92 @@ +/* The ::task block leaking into the message body. + + Reported from the app: the raw text "::task translate" and its five + sentences appeared as prose above the exercise those same lines had + already been rendered into. + + It is lib/blocks.js, not the model. parse() removes ::words by + truncating the body at its index, then removes ::task by substring — + but RE.task's terminator (?:\n::|$) is part of the match, so t[0] ends + with the "\n::" of the following ::words that the truncation just cut + off. The substring no longer occurs, replace() is a no-op, and the whole + task block stays in the body. + + Order is what decides it. The stub tutor emits ::words before ::task and + is therefore fine; a real model emitted ::task first. Nothing in the + prompt requires either order, so this was always reachable. + + The fixture is a verbatim capture from gpt-oss-20b through the server. */ + +import { describe, it, expect } from "vitest"; +import { parse } from "@lib/blocks.js"; +import { parseMessage, stripLeakedBlocks } from "@app/domain/gloss.js"; + +import type { ParsedMessage } from "@lib/blocks.js"; + +/** Narrow the Task union; only translate tasks appear in these fixtures. */ +function translateItems(task: ParsedMessage["task"]): { q: string }[] { + if (!task || task.type !== "translate") throw new Error(`expected a translate task, got ${task?.type}`); + return task.items; +} + +const CAPTURED = "::task translate\n\ub098 \uac00\ub2e4 \n\ub108 \uba39\ub2e4 \n\uc6b0\ub9ac \ub9c8\uc2dc\ub2e4 \n\uc774 \uc790\ub2e4 \n\uadf8 \ub098\ubb34 \uc11c\ub2e4 \n\n::words\n\ub098 | I, me (casual) | pron \n\uac00\ub2e4 | to go | verb (plain) \n\ub108 | you (casual) | pron \n\uba39\ub2e4 | to eat | verb (plain) \n\uc6b0\ub9ac | we, our | pron \n\ub9c8\uc2dc\ub2e4 | to drink | verb (plain) \n\uc774 | this | det \n\uc790\ub2e4 | to sleep | verb (plain) \n\uadf8 | that (near you) | det \n\ub098\ubb34 | tree | noun \n\uc11c\ub2e4 | to stand | verb (plain) \n\n::progress 20"; + +describe("lib/blocks.js — the leak, pinned as it is", () => { + it("leaves the whole ::task block in body when ::task precedes ::words", () => { + const r = parse(CAPTURED); + // The blocks themselves parse correctly... + expect(r.task?.type).toBe("translate"); + expect(translateItems(r.task)).toHaveLength(5); + expect(r.words).toHaveLength(11); + // ...and the body still carries the markup that produced them. + expect(r.body).toContain("::task translate"); + expect(r.body).toContain("나 가다"); + }); + + it("is fine in the other order, which is why the stub never showed it", () => { + const swapped = "Here you go.\n\n::words\n나 | I | pron\n\n::task translate\n나 가다\n"; + expect(parse(swapped).body).not.toContain("::task"); + }); +}); + +describe("parseMessage — the workaround", () => { + it("keeps the blocks and drops the leaked markup", () => { + const r = parseMessage(CAPTURED); + expect(r.body).not.toContain("::"); + expect(r.body).not.toContain("나 가다"); + // Nothing the blocks needed was lost. + expect(r.task?.type).toBe("translate"); + expect(translateItems(r.task)).toHaveLength(5); + expect(r.words).toHaveLength(11); + expect(r.progress?.score).toBe(20); + }); + + it("keeps prose that comes before the blocks", () => { + const r = parseMessage("좋아요. Try these.\n\n::task translate\n나 가다\n\n::words\n나 | I | pron\n"); + expect(r.body).toBe("좋아요. Try these."); + expect(translateItems(r.task)).toHaveLength(1); + }); + + it("still handles the order the stub uses", () => { + const r = parseMessage("Here you go.\n\n::words\n나 | I | pron\n\n::task translate\n나 가다\n"); + expect(r.body).toBe("Here you go."); + expect(translateItems(r.task)).toHaveLength(1); + }); + + it("leaves an ordinary message alone", () => { + const r = parseMessage("좋아요!\n\nThat is the shape of it."); + expect(r.body).toBe("좋아요!\n\nThat is the shape of it."); + }); +}); + +describe("stripLeakedBlocks", () => { + it("does nothing without a directive", () => { + expect(stripLeakedBlocks("a\n\nb")).toBe("a\n\nb"); + }); + it("cuts from the first directive line", () => { + expect(stripLeakedBlocks("keep\n::task translate\ndrop\n::words\ndrop")).toBe("keep"); + }); + it("does not trip on a colon that is not at the start of a line", () => { + expect(stripLeakedBlocks("see:: this")).toBe("see:: this"); + }); +});