From c16881f7a298e422b3c6400e2f92ac9b1a2c9794 Mon Sep 17 00:00:00 2001 From: cracthatrock <336640790+cracthatrock@users.noreply.github.com> Date: Mon, 5 Oct 2026 21:13:19 -0700 Subject: [PATCH 1/3] fix(ui): show source line numbers in rendering errors Signed-off-by: cracthatrock <336640790+cracthatrock@users.noreply.github.com> --- app/ui/src/render.test.ts | 21 ++++++++++++++++++++- app/ui/src/render.ts | 3 ++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/app/ui/src/render.test.ts b/app/ui/src/render.test.ts index ecb476b..86abcc4 100644 --- a/app/ui/src/render.test.ts +++ b/app/ui/src/render.test.ts @@ -1,7 +1,26 @@ // @vitest-environment jsdom -import { describe, expect, it } from "vitest" +import { describe, expect, it, vi } from "vitest" +import * as engine from "@engine" import { renderSource, sanitize } from "./render" +describe("renderSource errors", () => { + it("includes the source line when a diagram fails to compile", () => { + const result = renderSource('node a "A"\nnode b "B" rigth of a', false) + expect(result.error).toBe('line 2: node "b": "rigth" is not a direction') + expect(result.size).toEqual({ w: 560, h: 120 }) + }) + it.each([undefined, 0])("keeps errors without a usable line unchanged (%s)", line => { + const compile = vi.spyOn(engine, "compile").mockImplementationOnce(() => { + throw Object.assign(new Error("cannot compile diagram"), { line }) + }) + try { + expect(renderSource("", false).error).toBe("cannot compile diagram") + } finally { + compile.mockRestore() + } + }) +}) + describe("sanitize", () => { it("drops the three bypasses the review reproduced", () => { const img = sanitize('') diff --git a/app/ui/src/render.ts b/app/ui/src/render.ts index 67d27d6..af77e02 100644 --- a/app/ui/src/render.ts +++ b/app/ui/src/render.ts @@ -10,7 +10,8 @@ export function renderSource(source: string, dark: boolean): Rendered { const svg = sanitize(compile(source, { theme: dark ? THEMES.dark : THEMES.light })) return { svg, size: svgSize(svg) } } catch (e) { - return { error: e instanceof Error ? e.message : String(e), size: { w: 560, h: 120 } } + const line = e && typeof (e as { line?: number }).line === "number" && (e as { line: number }).line > 0 ? `line ${(e as { line: number }).line}: ` : "" + return { error: line + (e instanceof Error ? e.message : String(e)), size: { w: 560, h: 120 } } } } From 37d36553ea29578b45b7a20b6f4ba6987e4a75a7 Mon Sep 17 00:00:00 2001 From: Banna Date: Tue, 6 Oct 2026 09:51:25 -0400 Subject: [PATCH 2/3] A refused source is described in the engine's own words, in the app and on the server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engine's SourceError already has format() ('line N: …', what its command-line tool prints). renderSource and the server's check() each re-derived it from a duck-typed .line; both now use the engine's error type, so the rule lives in one place. renderSource also returns the line on its own (Rendered.line) for the editor. Tests use the engine's real error type instead of a mocked compile: a statement written over several lines names the line at fault, a line of 0 or a plain error keeps its text, and check() (refused saves, scheduled drafts, Arch Agent's check_diagram) says the same as the editor. Signed-off-by: Banna Agent: Banna --- app/core/src/engine.ts | 6 +++--- app/core/test/engine.test.ts | 16 ++++++++++++++++ app/ui/src/render.test.ts | 36 ++++++++++++++++++++++++------------ app/ui/src/render.ts | 14 ++++++++++---- 4 files changed, 53 insertions(+), 19 deletions(-) create mode 100644 app/core/test/engine.test.ts diff --git a/app/core/src/engine.ts b/app/core/src/engine.ts index 61b147a..a0d68a2 100644 --- a/app/core/src/engine.ts +++ b/app/core/src/engine.ts @@ -3,7 +3,7 @@ // a file. Diagrams are capped at 200 KB and the engine refuses ambiguity instead of searching, so a check is fast // (0.1–0.4 s for large diagrams). -import { compile, parse, resolve, THEMES } from "../../../engine/src/index.js" +import { compile, parse, resolve, SourceError, THEMES } from "../../../engine/src/index.js" export { parse, resolve } @@ -13,8 +13,8 @@ export function check(source: string): string | null { compile(source) return null } catch (e) { - const line = e && typeof (e as { line?: number }).line === "number" && (e as { line: number }).line > 0 ? `line ${(e as { line: number }).line}: ` : "" - return (line + String(e instanceof Error ? e.message : e)).slice(0, 2000) + // the engine's own "line N: …" (SourceError.format), as the app's editor shows it (ui/src/render.ts `refusal`) + return (e instanceof SourceError && e.line > 0 ? e.format() : String(e instanceof Error ? e.message : e)).slice(0, 2000) } } diff --git a/app/core/test/engine.test.ts b/app/core/test/engine.test.ts new file mode 100644 index 0000000..49e4246 --- /dev/null +++ b/app/core/test/engine.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from "vitest" +import { check } from "../src/engine.js" + +// check() is what a refused save, the scheduled drafts and Arch Agent's check_diagram report; the app's editor shows the +// same words (ui/src/render.ts `refusal`), so a line number reads the same everywhere (#32) +describe("check", () => { + it("is null when the source renders", () => { + expect(check('node a "A"\nnode b "B" right of a')).toBeNull() + }) + it("names the line of the mistake", () => { + expect(check('node a "A"\nnode b "B" rigth of a')).toBe('line 2: node "b": "rigth" is not a direction') + }) + it("names the line at fault in a statement written over several lines", () => { + expect(check('node a "A"\nnode b "B"\n style: rounded\n rigth of a')).toBe('line 4: node "b": "rigth" is not a direction') + }) +}) diff --git a/app/ui/src/render.test.ts b/app/ui/src/render.test.ts index 86abcc4..c2d08a6 100644 --- a/app/ui/src/render.test.ts +++ b/app/ui/src/render.test.ts @@ -1,23 +1,35 @@ // @vitest-environment jsdom -import { describe, expect, it, vi } from "vitest" -import * as engine from "@engine" -import { renderSource, sanitize } from "./render" +import { describe, expect, it } from "vitest" +import { SourceError } from "@engine" +import { refusal, renderSource, sanitize } from "./render" describe("renderSource errors", () => { it("includes the source line when a diagram fails to compile", () => { const result = renderSource('node a "A"\nnode b "B" rigth of a', false) expect(result.error).toBe('line 2: node "b": "rigth" is not a direction') + expect(result.line).toBe(2) expect(result.size).toEqual({ w: 560, h: 120 }) }) - it.each([undefined, 0])("keeps errors without a usable line unchanged (%s)", line => { - const compile = vi.spyOn(engine, "compile").mockImplementationOnce(() => { - throw Object.assign(new Error("cannot compile diagram"), { line }) - }) - try { - expect(renderSource("", false).error).toBe("cannot compile diagram") - } finally { - compile.mockRestore() - } + it("names the line at fault in a statement written over several lines, not the line it starts on", () => { + const result = renderSource('node a "A"\nnode b "B"\n style: rounded\n rigth of a', false) + expect(result.error).toBe('line 4: node "b": "rigth" is not a direction') + expect(result.line).toBe(4) + }) + it("has no error and no line when the source renders", () => { + const result = renderSource('node a "A"', false) + expect(result.error).toBeUndefined() + expect(result.line).toBeUndefined() + }) +}) + +describe("refusal", () => { + it("prints a source error the way the engine's command-line tool does", () => { + expect(refusal(new SourceError('two placements for "server"', 12))).toEqual({ error: 'line 12: two placements for "server"', line: 12 }) + }) + it("keeps errors without a usable line unchanged", () => { + expect(refusal(new SourceError("cannot compile diagram", 0))).toEqual({ error: "cannot compile diagram" }) + expect(refusal(new Error("cannot compile diagram"))).toEqual({ error: "cannot compile diagram" }) + expect(refusal("cannot compile diagram")).toEqual({ error: "cannot compile diagram" }) }) }) diff --git a/app/ui/src/render.ts b/app/ui/src/render.ts index af77e02..564d1c3 100644 --- a/app/ui/src/render.ts +++ b/app/ui/src/render.ts @@ -1,8 +1,8 @@ import DOMPurify from "dompurify" -import { THEMES, compile } from "@engine" +import { SourceError, THEMES, compile } from "@engine" import { svgSize, type Size } from "./model" -export type Rendered = { svg: string; size: Size; error?: undefined } | { svg?: undefined; size: Size; error: string } +export type Rendered = { svg: string; size: Size; error?: undefined; line?: undefined } | { svg?: undefined; size: Size; error: string; line?: number } /** Render a source with the engine in a theme. A refusal is returned, not thrown: the card shows it in place. */ export function renderSource(source: string, dark: boolean): Rendered { @@ -10,11 +10,17 @@ export function renderSource(source: string, dark: boolean): Rendered { const svg = sanitize(compile(source, { theme: dark ? THEMES.dark : THEMES.light })) return { svg, size: svgSize(svg) } } catch (e) { - const line = e && typeof (e as { line?: number }).line === "number" && (e as { line: number }).line > 0 ? `line ${(e as { line: number }).line}: ` : "" - return { error: line + (e instanceof Error ? e.message : String(e)), size: { w: 560, h: 120 } } + return { ...refusal(e), size: { w: 560, h: 120 } } } } +/** The engine's refusal as the user reads it: "line 2: …" in the engine's own words (`SourceError.format`, as its + * command-line tool prints it), and the line on its own so the editor can mark it. No usable line, no prefix. */ +export function refusal(e: unknown): { error: string; line?: number } { + if (e instanceof SourceError && e.line > 0) return { error: e.format(), line: e.line } + return { error: e instanceof Error ? e.message : String(e) } +} + const INTERNAL = /^#\/[a-z0-9][a-z0-9-]{0,62}(\/[a-z0-9][a-z0-9-]{0,62})?$/ const ANCHOR = /^#[A-Za-z][\w.-]*$/ const WEB = /^https?:\/\/[^\s"'<>]+$/i From ec14d67f0a720f8e6cd521f0b70fb5d0cd90f649 Mon Sep 17 00:00:00 2001 From: Banna Date: Tue, 6 Oct 2026 09:51:25 -0400 Subject: [PATCH 3/3] The editor marks the line the engine refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The status line says 'line 14: …'; the gutter now marks line 14 in the danger color with a rule beside it, in both themes, and the mark goes when the mistake does. The browser test types a mistake while editing: the status line and the card name its line, the gutter marks that same line, Save stays disabled, and deleting the line clears both. Signed-off-by: Banna Agent: Banna --- app/e2e/app.e2e.ts | 13 +++++++++++++ app/ui/src/App.tsx | 9 ++++++--- app/ui/src/index.css | 11 ++++++++++- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/app/e2e/app.e2e.ts b/app/e2e/app.e2e.ts index 3b0c90e..a3b9210 100644 --- a/app/e2e/app.e2e.ts +++ b/app/e2e/app.e2e.ts @@ -135,6 +135,19 @@ async function main() { await page.keyboard.press("Control+End") await page.keyboard.type('\nnode cache "Menu cache" below db style: store\n') await page.waitForFunction(() => document.querySelector("[data-card=system]")?.textContent?.includes("Menu cache")) + // a mistake: the status line and the card name its line, the gutter marks that line, Save waits (#32) + await page.keyboard.type('node oops "Oops" rigth of db') + await page.waitForFunction(() => /^line \d+: node "oops": "rigth" is not a direction$/.test(document.querySelector("[data-testid=status]")?.textContent?.trim() ?? "")) + const said = (await page.locator("[data-testid=status]").textContent())!.trim().match(/^line (\d+):/)![1] + const typedOn = String((await ta.inputValue()).split("\n").length) + const marked = (await page.locator("[data-error-line]").count()) ? (await page.locator("[data-error-line]").textContent())?.trim() : "no line" + if (said !== typedOn || marked !== typedOn) throw new Error(`the mistake is on line ${typedOn}; the status says ${said}, the gutter marks ${marked}`) + if (!(await page.locator("[data-card=system]").textContent())?.includes(`line ${said}: node "oops"`)) throw new Error("the card does not name the line") + if (await page.isEnabled("[data-testid=save]")) throw new Error("Save is enabled on a source that does not render") + await page.keyboard.press("Shift+Home") + await page.keyboard.press("Backspace") + await page.waitForFunction(() => document.querySelector("[data-testid=status]")?.textContent?.startsWith("Renders")) + if (await page.locator("[data-error-line]").count()) throw new Error("the gutter still marks a line after the mistake is gone") await page.keyboard.press("Control+s") // no git identity: the app asks who commits, then the save goes through await page.waitForSelector("[data-testid=identity-dialog]") diff --git a/app/ui/src/App.tsx b/app/ui/src/App.tsx index 039848f..61a84f2 100644 --- a/app/ui/src/App.tsx +++ b/app/ui/src/App.tsx @@ -785,7 +785,7 @@ export default function App() { ✕ - (setDrafts(d => ({ ...d, [sel]: v })), current?.sample && tourEvent("source.changed"))} /> + (setDrafts(d => ({ ...d, [sel]: v })), current?.sample && tourEvent("source.changed"))} />
{selCard?.r.error ?? (current?.source.kind === "github" ? "Renders. ⌘/Ctrl-S saves it as a change waiting for review; the card updates as you type." : "Renders. ⌘/Ctrl-S saves it; the card updates as you type.")}
@@ -1010,7 +1010,8 @@ function ProjectTitleDialog({ project, onClose, onSaved }: { project: Project; o } -function Editor({ value, onChange }: { value: string; onChange: (v: string) => void }) { +/** The source editor: a textarea with line numbers; the line the engine refused (`errorLine`) is marked in the gutter. */ +function Editor({ value, errorLine, onChange }: { value: string; errorLine?: number; onChange: (v: string) => void }) { const ta = useRef(null) const gutter = useRef(null) const lines = value.split("\n").length @@ -1018,7 +1019,9 @@ function Editor({ value, onChange }: { value: string; onChange: (v: string) => v
{Array.from({ length: lines }, (_, i) => ( -
{i + 1}
+
+ {i + 1} +
))}