From ba3055912ad575d19baf39e7e5cdad1ba55a7660 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Wed, 16 Sep 2026 19:50:11 +0200 Subject: [PATCH] =?UTF-8?q?fix(tutor):=20find=20gloss=20blocks=20in=20the?= =?UTF-8?q?=20raw=20text=20=E2=80=94=20the=20new=20parse()=20still=20leaks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 16 Sep lib fixed both defects gloss.ts used to work around: a gloss block closes at each "=" line, and every block is collected. So the sentence splitting goes. Three defects remain in how a block ENDS, measured on the new lib: · deleting one block still deletes the "::" of the next, so a whole ::words block reaches the prose as "words" plus its rows; · an unclosed gloss still swallows the following paragraph as parts; · two gloss blocks written back to back share one terminator, and lib never sees the second. The old rebuild also only re-parsed the FIRST gloss block, which against a lib that accumulates would have dropped every block after it. One pass over the raw text now yields both the prose and each gloss block's content, a gloss block ending where its rows end; lib parses each block on its own, so the row format is still entirely lib's. Co-Authored-By: Claude Opus 5 (1M context) --- app/src/domain/gloss.ts | 200 ++++++++++++++------------------- test/domain/block-leak.test.ts | 20 +++- test/domain/gloss.test.ts | 3 +- 3 files changed, 102 insertions(+), 121 deletions(-) diff --git a/app/src/domain/gloss.ts b/app/src/domain/gloss.ts index 9e40b79..cc6afac 100644 --- a/app/src/domain/gloss.ts +++ b/app/src/domain/gloss.ts @@ -1,107 +1,50 @@ -/* Two workarounds for lib/blocks.js, which ships unchanged. +/* Where a tutor message's blocks END — a workaround for lib/blocks.js, + which ships unchanged. - ── 1. Directive markup leaking into the message body ────────────────── + The 16 Sep lib fixed the two defects this module used to work around: a + gloss block now closes at each "=" line, and every block is collected + rather than only the first. What remains is how a block ends, and both + remaining defects reach the student. - parse() builds `body` by removing each block it recognised, but it does - the removals in an order that defeats one of them: + ── 1. Removing one block takes the next block's colons ──────────────── - if (w) body = body.slice(0, body.indexOf("::words")); // truncate - if (t) body = body.replace(t[0], ""); // substring + parse() builds `body` by deleting each block with a regex whose + terminator `(?:\n::|$)` is INSIDE the match. Deleting one block therefore + deletes the "::" that opens the block after it, which is then no longer + recognisable as a block at all and stays behind as prose: - 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. + ::task translate -> words + 나 가다 나 | I, me (casual) | pron + ::words + 나 | I, me (casual) | pron - 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. + Which block leaks depends on the order the model chose, and nothing in + the prompt fixes one. So the prose is read from the RAW text, where every + marker is still intact. That is not a repair of parse()'s body — it is + just reading. - 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. A block runs to the next "::", or to the end ──────────────────── - ── 2. Multi-sentence ::gloss blocks ─────────────────────────────────── + Every local model tested forgets the closing "::" on a gloss block. lib + then reads the paragraph that follows as gloss PARTS, so two sentences + of English render as Korean example text inside the card. And two gloss + blocks written back to back share one terminator, so lib never sees the + second. - 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 - `en` when it meets "=" but never closes the block, so every sentence's - parts pile into one run-on line and only the last translation survives. - - lib/ ships unchanged, so the fix lives here, at the call site: split the - block on its "=" lines and parse each sentence as its own single-sentence - block. The output is exactly what parse() would have produced if it closed - the block, so nothing downstream has to know. - - (If lib/blocks.js is ever revised, the one-line fix there is `cur = null` - after setting `en`, and this module can go. test/lib/blocks.test.ts pins - the current behaviour so the change is visible when it happens.) */ + So gloss blocks are found in the raw text too. A gloss block ends where + its rows end — a row is `한글 | role | gloss`, a translation starts with + "=" — and lib parses each block on its own, so the row format is still + entirely lib's. */ import { parse } from "@lib/blocks.js"; import type { GlossBlock, ParsedMessage } from "@lib/blocks.js"; -const GLOSS_BLOCK = /::gloss\s*\n([\s\S]*?)(?:\n::|$)/; - -/** Split a gloss block's body into one chunk per "=" line. */ -function splitSentences(body: string): string[] { - const out: string[] = []; - let current: string[] = []; - - for (const line of body.split("\n")) { - const l = line.trim(); - if (!l || l.startsWith("::")) continue; - current.push(l); - if (l.startsWith("=")) { - out.push(current.join("\n")); - current = []; - } - } - // A trailing sentence with no "=" is still worth rendering. - if (current.length) out.push(current.join("\n")); - - return out; -} - -/** - * parse(), with multi-sentence gloss blocks split correctly. - * Use this everywhere instead of calling parse() directly. - */ -/** - * The prose of a message: every line that is not inside a block. - * - * Computed from the RAW text, not from parse()'s body. Repairing that body - * cannot be made to work, because parse() removes each block by string - * surgery and the removals damage each other — a block's terminator - * `(?:\n::|$)` is inside its own match, so removing one block takes the - * two colons belonging to the NEXT one with it. What is left behind is not - * even recognisable as a directive any more: - * - * **Example sentence** - * *나 바다* - * task translate ← the "::" is gone; the items follow as prose - * 나 바다 - * - * An earlier version of this dropped everything from the first line - * starting with "::", which cannot see that. The raw text always has its - * markers, so scanning it is not a repair at all — it is just reading. - * - * The grammar is small: a line of exactly "::" closes a block, any other - * line beginning "::" opens one (::gloss, ::task translate, ::words, - * ::progress 40 | note), and a block runs until it is closed, until the - * next block opens, or to the end. Trailing whitespace is common — this - * model ends every line with markdown's two spaces — so nothing here - * compares exact strings. - */ /** * Is this a line of ::gloss content? * - * A gloss row is `한글 | English | note` and a translation line starts with - * `=`. Nothing else belongs. The check matters because a model that forgets - * the closing `::` leaves its next paragraph inside the block, and lib then - * reads that prose as gloss parts — which is how two sentences of English - * ended up rendered as Korean example text inside a gloss card. + * A gloss row is `한글 | role | English` and a translation line starts with + * `=`. Nothing else belongs, so the first line that is neither ends the + * block whether or not the model closed it. */ function isGlossContent(line: string): boolean { const t = line.trim(); @@ -115,54 +58,79 @@ export function glossContent(body: string): string { return (end === -1 ? lines : lines.slice(0, end)).join("\n"); } -export function proseOf(raw: string): string { - const out: string[] = []; +interface Scanned { + prose: string[]; + /** The content of each gloss block, in order, trimmed to its rows. */ + glosses: string[]; +} + +/** + * One pass over the raw text. + * + * The grammar is small: a line of exactly "::" closes a block, any other + * line beginning "::" opens one (::gloss, ::task translate, ::words, + * ::progress 40 | note), and a block runs until it is closed, until the + * next block opens, or to the end — except a gloss block, which also ends + * at its first line that is not a gloss row. Trailing whitespace is common + * (one model ends every line with markdown's two spaces), so nothing here + * compares exact strings. + */ +function scan(raw: string): Scanned { + const prose: string[] = []; + const glosses: string[] = []; let block: string | null = null; + let gloss: string[] = []; + + const enter = (next: string | null) => { + if (block === "gloss") glosses.push(gloss.join("\n")); + block = next; + gloss = []; + }; for (const line of raw.split("\n")) { const t = line.trim(); if (t === "::") { - block = null; + enter(null); continue; } if (t.startsWith("::")) { - block = t.slice(2).split(/\s/)[0] ?? null; + enter(t.slice(2).split(/\s/)[0] ?? null); continue; } - // An unclosed gloss ends where its rows do; what follows is prose again, - // and belongs in the message rather than swallowed into the card. - if (block === "gloss" && !isGlossContent(line)) block = null; - if (!block) out.push(line); - } + // An unclosed gloss ends where its rows do; what follows is prose again. + if (block === "gloss" && !isGlossContent(line)) enter(null); - return out.join("\n").trim(); + if (block === "gloss") gloss.push(line); + else if (!block) prose.push(line); + } + enter(null); + + return { prose, glosses }; } +/** The prose of a message: every line that is not inside a block. */ +export function proseOf(raw: string): string { + return scan(raw).prose.join("\n").trim(); +} + +/** + * parse(), with the body and the gloss blocks read from the raw text. + * Use this everywhere instead of calling parse() directly. + */ export function parseMessage(text: string): ParsedMessage { const parsed = parse(text); - // lib/ owns extracting the blocks; the prose around them is ours. - const body = proseOf(text); + const { prose, glosses } = scan(text); + const body = prose.join("\n").trim(); - if (!parsed.gloss) return { ...parsed, body }; - - const match = text.match(GLOSS_BLOCK); - if (!match?.[1]) return { ...parsed, body }; - - /* Rebuilt unconditionally, not only when there are several sentences. - The single-sentence case is where an unclosed block hurts: lib has - already absorbed the following paragraph into it, and returning its - gloss untouched would keep that prose inside the card. Re-parsing from - glossContent() drops it, and proseOf() has put it back in the message. - For a well-formed single-sentence block this reproduces lib exactly. */ - const sentences = splitSentences(glossContent(match[1])); + if (!glosses.length) return { ...parsed, body, gloss: null }; const blocks: GlossBlock[] = []; - for (const s of sentences) { - const one = parse(`::gloss\n${s}\n::`); + for (const content of glosses) { + const one = parse(`::gloss\n${content}\n::`); if (one.gloss) blocks.push(...one.gloss); } - return blocks.length ? { ...parsed, body, gloss: blocks } : { ...parsed, body }; + return { ...parsed, body, gloss: blocks.length ? blocks : null }; } export type { GlossBlock, ParsedMessage }; diff --git a/test/domain/block-leak.test.ts b/test/domain/block-leak.test.ts index 40ff934..58ab658 100644 --- a/test/domain/block-leak.test.ts +++ b/test/domain/block-leak.test.ts @@ -1,8 +1,7 @@ -/* The ::task block leaking into the message body. +/* Blocks 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. + Reported from the app, twice: raw exercise markup appeared as prose above + the exercise it had already been rendered into. It is lib/blocks.js, not the model. parse() deletes each block with a regex whose terminator (?:\n::|$) is part of the match, so deleting one @@ -180,6 +179,11 @@ describe("an unclosed ::gloss block", () => { "나 바다", ].join("\n"); + it("lib alone reads the following prose as gloss parts — pinned as it is", () => { + const parts = (parse(RAW).gloss ?? []).flatMap((g) => g.parts.map((p) => p.ko)); + expect(parts).toContain("Now practice these."); + }); + it("ends the block where its rows end", () => { expect(glossContent("나 | S | I\n= I sea\n\nprose here\nmore prose")).toBe("나 | S | I\n= I sea\n"); }); @@ -201,6 +205,14 @@ describe("an unclosed ::gloss block", () => { expect(body).not.toMatch(/::/); }); + it("finds a second gloss block written straight after the first", () => { + // Both blocks share one terminator, so lib alone never sees the second. + const src = "::gloss\n나 | S | I\n= I.\n::gloss\n밥 | O | rice\n= Rice.\n::"; + expect(parse(src).gloss).toHaveLength(1); + expect(parseMessage(src).gloss!.map((g) => g.en)).toEqual(["I.", "Rice."]); + expect(parseMessage(src).body).toBe(""); + }); + it("leaves a properly closed block exactly as lib parses it", () => { const closed = "::gloss\n물 | N | water\n= Water.\n::\nAfter."; const g = parseMessage(closed).gloss ?? []; diff --git a/test/domain/gloss.test.ts b/test/domain/gloss.test.ts index 5ebfabb..f86e3aa 100644 --- a/test/domain/gloss.test.ts +++ b/test/domain/gloss.test.ts @@ -1,4 +1,5 @@ -/* The multi-sentence gloss workaround. */ +/* Gloss blocks as the app renders them: lib parses the rows, parseMessage + decides where each block ends. */ import { describe, it, expect } from "vitest"; import { parseMessage } from "@app/domain/gloss.js";