From e63072c216383700a23504be89782f4c69657174 Mon Sep 17 00:00:00 2001 From: Guilhem Date: Wed, 9 Sep 2026 18:51:55 +0200 Subject: [PATCH] fix(frontend): restore heading sizes in note markdown and keep group notes on id change (#11047) * fix(frontend): restore heading sizes in note markdown and keep group notes on id change Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * fix(frontend): scope the note cleanup deferral per note Address local review nits: the header comment overstated the heading ramp (h3 sits at body size in the xs and sm scales), and the mid-update deferral bailed out of cleanup for every group note rather than the one holding the unrendered module. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * perf(frontend): traverse modules without collecting a discarded array Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * test(frontend): show the three markdown prose presets in the kitchen sink The Markdown tab rendered only the default preset, so a change to the shared heading scale could not be compared across the surfaces that use it. The chat sample gains headings for the same reason: it is the only place the assistant bubble renders at panel width without an AI provider. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * refactor(frontend): validate group notes against the flow, not the render A group note's members are module ids, so the flow's own module list decides whether one still exists. Validating against the rendered nodes instead needed a special case for collapsed groups, and still dropped a live module in the pass after its id changed. Both cases are the same mistake, and checking the source of truth removes them together with the collapsedModuleIds parameter. Path completion keeps working off the rendered graph, since it needs the edges, and now skips a note whose members it cannot all see rather than dropping them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * fix(frontend): name a new note "Note", not after its internal type The prefill lands inside the user's own note, and "Free note" / "Group note" are the serialized type, not words anyone says about a note they just drew. The menus that create them already say "Add note". The heading goes to h2 as well: h3 computes to the body size in the note scale, so the prefill's own title did not read as one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE * docs(frontend): describe what collapsedModuleIds is still for Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0182GroR5mvi9ksRTMmBT7nE --------- Co-authored-by: Claude Opus 5 (1M context) --- .../flows/map/FlowModuleSchemaMap.svelte | 13 ++++ .../lib/components/graph/FlowGraphV2.svelte | 3 +- .../src/lib/components/graph/NoteTool.svelte | 4 +- .../components/graph/PaneContextMenu.svelte | 2 +- .../lib/components/graph/noteEditor.svelte.ts | 37 ++++++------ .../lib/components/graph/noteEditor.test.ts | 59 +++++++++++++++++++ .../lib/components/graph/noteUtils.svelte.ts | 2 +- frontend/src/lib/components/markdownProse.ts | 10 +++- frontend/src/routes/kitchen_sink/+page.svelte | 24 +++++++- 9 files changed, 125 insertions(+), 29 deletions(-) create mode 100644 frontend/src/lib/components/graph/noteEditor.test.ts diff --git a/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte b/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte index 7e70b0c215..22b74230b4 100644 --- a/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte +++ b/frontend/src/lib/components/flows/map/FlowModuleSchemaMap.svelte @@ -806,6 +806,19 @@ } } } + // Group notes reference their members by module id. A stale id here is not + // merely cosmetic: cleanupGroupNotes drops ids it cannot resolve and deletes + // the note once none are left. + const notes = flowStore.val.value.notes + if (notes) { + for (const note of notes) { + if (note.contained_node_ids) { + note.contained_node_ids = note.contained_node_ids.map((nid) => + nid === id ? newId : nid + ) + } + } + } flowStateStore.val[newId] = flowStateStore.val[id] delete flowStateStore.val[id] refreshStateStore(flowStore) diff --git a/frontend/src/lib/components/graph/FlowGraphV2.svelte b/frontend/src/lib/components/graph/FlowGraphV2.svelte index f83b67267a..74430cffde 100644 --- a/frontend/src/lib/components/graph/FlowGraphV2.svelte +++ b/frontend/src/lib/components/graph/FlowGraphV2.svelte @@ -855,7 +855,8 @@ ...aiToolNodesResult.toolNodes ] - // Collect module IDs hidden inside collapsed groups so note cleanup preserves them + // Module IDs hidden inside collapsed groups: a note whose members are all in here has + // nothing on screen to wrap, so it is skipped. const collapsedModuleIds = new Set() for (const n of finalNodes) { if (n.type === 'collapsedGroup') { diff --git a/frontend/src/lib/components/graph/NoteTool.svelte b/frontend/src/lib/components/graph/NoteTool.svelte index ad41bd974a..d6f05244d1 100644 --- a/frontend/src/lib/components/graph/NoteTool.svelte +++ b/frontend/src/lib/components/graph/NoteTool.svelte @@ -86,7 +86,7 @@ // Create the actual note using NoteEditor context if (noteEditorContext?.noteEditor) { noteEditorContext.noteEditor.addNote({ - text: '### Free note\nDouble click to edit me', + text: '## Note\nDouble click to edit me', position, size, color: DEFAULT_NOTE_COLOR, @@ -107,7 +107,7 @@ if (!noteEditorContext?.noteEditor || !contextMenuPosition) return noteEditorContext.noteEditor.addNote({ - text: '### Free note\nDouble click to edit me', + text: '## Note\nDouble click to edit me', position: contextMenuPosition, size: { width: 300, height: 200 }, color: DEFAULT_NOTE_COLOR, diff --git a/frontend/src/lib/components/graph/PaneContextMenu.svelte b/frontend/src/lib/components/graph/PaneContextMenu.svelte index 37be64bffc..7f3409812b 100644 --- a/frontend/src/lib/components/graph/PaneContextMenu.svelte +++ b/frontend/src/lib/components/graph/PaneContextMenu.svelte @@ -68,7 +68,7 @@ function handleAddStickyNote() { if (noteEditorContext?.noteEditor && pendingFlowPosition) { noteEditorContext.noteEditor.addNote({ - text: '### Free note\nDouble click to edit me', + text: '## Note\nDouble click to edit me', position: { x: pendingFlowPosition.x, y: pendingFlowPosition.y - (graphContext?.yOffset || 0) diff --git a/frontend/src/lib/components/graph/noteEditor.svelte.ts b/frontend/src/lib/components/graph/noteEditor.svelte.ts index 50c2df7bfa..e3f17f576c 100644 --- a/frontend/src/lib/components/graph/noteEditor.svelte.ts +++ b/frontend/src/lib/components/graph/noteEditor.svelte.ts @@ -6,6 +6,7 @@ import { DEFAULT_GROUP_NOTE_COLOR, getNextAvailableColor } from './noteColors' import { generateId } from './util' import { getContext, setContext } from 'svelte' import { completeAndSplitGroup } from './groupDetectionUtils' +import { forEachFlowModule } from '../flows/dfs' /** * Utility class for editing flow notes via direct flowStore mutations @@ -158,10 +159,7 @@ export class NoteEditor { /** * Create a group note containing the specified node IDs */ - createGroupNote( - nodeIds: string[], - text: string = '### Group note\nDouble click to edit me' - ): string { + createGroupNote(nodeIds: string[], text: string = '## Note\nDouble click to edit me'): string { // Filter ids in case they contain subflow nodes let filteredNodeIds: string[] = nodeIds let subflowIds: string[] = [] @@ -219,10 +217,7 @@ export class NoteEditor { /** * Clean up group notes using DAG path completion */ - cleanupGroupNotes( - flowNodes: { id: string; parentIds?: string[] }[], - collapsedModuleIds?: Set - ): void { + cleanupGroupNotes(flowNodes: { id: string; parentIds?: string[] }[]): void { if (!this.isAvailable()) { return } @@ -232,19 +227,23 @@ export class NoteEditor { if (groupNotes.length === 0) return let hasChanges = false - const nodeSet = new Set(flowNodes.map((n) => n.id)) + const renderedIds = new Set(flowNodes.map((n) => n.id)) - // Include collapsed module IDs as valid — they are hidden but still exist - if (collapsedModuleIds) { - for (const id of collapsedModuleIds) { - nodeSet.add(id) - } - } + // A note's members are module ids, and the flow's own modules are what say whether one + // still exists. The rendered nodes are a view of them: a live module is absent from it + // while its group is collapsed, and again in the pass after its id changed, so pruning + // against the render alone deletes steps out of notes that are perfectly valid. + const moduleIds = new Set() + forEachFlowModule(this.flowStore.val.value?.modules ?? [], (mod) => { + moduleIds.add(mod.id) + }) // Step 1: Clean invalid nodes from existing group notes for (const note of groupNotes) { const originalIds = note.contained_node_ids || [] - const validIds = originalIds.filter((id) => nodeSet.has(id)) + // Path completion below can add ids that exist only in the graph (group + // boundaries), so a rendered node counts as valid alongside a live module. + const validIds = originalIds.filter((id) => moduleIds.has(id) || renderedIds.has(id)) if (validIds.length !== originalIds.length) { note.contained_node_ids = validIds @@ -259,9 +258,9 @@ export class NoteEditor { const originalNodes = note.contained_node_ids || [] if (originalNodes.length === 0) continue - // Skip path completion for notes that reference collapsed modules, - // since the DAG is incomplete when groups are collapsed - if (collapsedModuleIds && originalNodes.some((id) => collapsedModuleIds.has(id))) { + // Path completion walks the rendered edges, so it can only place members it can + // see; one it cannot would come back as unreachable and be dropped from the note. + if (originalNodes.some((id) => !renderedIds.has(id))) { continue } diff --git a/frontend/src/lib/components/graph/noteEditor.test.ts b/frontend/src/lib/components/graph/noteEditor.test.ts new file mode 100644 index 0000000000..5ec38601ef --- /dev/null +++ b/frontend/src/lib/components/graph/noteEditor.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect, vi } from 'vitest' + +// Mock modules that transitively import CSS/Monaco +vi.mock('monaco-editor', () => ({})) +vi.mock('@xyflow/svelte', () => ({})) + +import type { FlowModule, OpenFlow } from '$lib/gen' +import type { StateStore } from '$lib/utils' +import type { ExtendedOpenFlow } from '../flows/types' +import { NoteEditor } from './noteEditor.svelte' + +function makeFlowStore( + moduleIds: string[], + containedNodeIds: string[] +): StateStore { + const modules: FlowModule[] = moduleIds.map((id) => ({ + id, + value: { type: 'rawscript', content: '', language: 'bun' } as any + })) + const flow: OpenFlow = { + summary: '', + value: { + modules, + notes: [ + { + id: 'note', + text: 'note', + color: 'yellow', + type: 'group', + contained_node_ids: containedNodeIds + } + ] + }, + schema: {} + } + return { val: flow as ExtendedOpenFlow } as StateStore +} + +function containedIds(flowStore: StateStore): string[] | undefined { + return flowStore.val.value.notes?.[0]?.contained_node_ids +} + +describe('cleanupGroupNotes', () => { + it('keeps a module the graph has not rendered', () => { + // Both the pass after a module id changed and a collapsed group leave a live module + // out of the rendered nodes. + const flowStore = makeFlowStore(['renamed', 'b'], ['renamed', 'b']) + new NoteEditor(flowStore).cleanupGroupNotes([{ id: 'b' }]) + + expect(containedIds(flowStore)).toEqual(['renamed', 'b']) + }) + + it('drops a module that no longer exists in the flow', () => { + const flowStore = makeFlowStore(['b'], ['deleted', 'b']) + new NoteEditor(flowStore).cleanupGroupNotes([{ id: 'b' }]) + + expect(containedIds(flowStore)).toEqual(['b']) + }) +}) diff --git a/frontend/src/lib/components/graph/noteUtils.svelte.ts b/frontend/src/lib/components/graph/noteUtils.svelte.ts index 577b421a9e..7cb6e17ece 100644 --- a/frontend/src/lib/components/graph/noteUtils.svelte.ts +++ b/frontend/src/lib/components/graph/noteUtils.svelte.ts @@ -264,7 +264,7 @@ export function computeNoteNodes( if (editMode) { if (noteEditorContext?.noteEditor?.isAvailable()) { - noteEditorContext.noteEditor.cleanupGroupNotes(nodes, collapsedModuleIds) + noteEditorContext.noteEditor.cleanupGroupNotes(nodes) } } diff --git a/frontend/src/lib/components/markdownProse.ts b/frontend/src/lib/components/markdownProse.ts index aedd2f45b1..3f79ab79a5 100644 --- a/frontend/src/lib/components/markdownProse.ts +++ b/frontend/src/lib/components/markdownProse.ts @@ -4,10 +4,14 @@ * call site with layout-only classes (padding, width, bg); anything typographic * belongs here. * - * - 'xs': micro scale for dense secondary panes (chat reasoning blocks) + * - 'xs': micro scale for dense secondary panes (chat reasoning blocks, group notes) * - 'sm': compact chat-bubble scale (assistant messages, flow/app chat, settings) * - 'doc': same rhythm and body size as 'sm', with a taller heading ramp * (lg/base/sm) and semibold headings for document-like surfaces (artifacts) + * + * h1 and h2 step above the body size in every preset, so `#` and `##` read as + * headings rather than bold body text. Below that the tight 'xs' and 'sm' scales + * run out of room, and h3 down differentiates by weight and colour alone. */ // Kept as literal template parts: Tailwind's scanner reads class names verbatim @@ -28,8 +32,8 @@ const bodyXs = 'text-primary prose-p:text-primary prose-li:text-primary prose-p:text-xs prose-li:text-xs prose-code:text-xs prose-pre:text-xs prose-table:text-xs' export const markdownProse = { - xs: `${base} prose-sm leading-snug prose-p:text-2xs prose-li:text-2xs prose-code:text-2xs prose-pre:text-2xs prose-headings:font-medium prose-headings:text-secondary prose-headings:mt-2 prose-headings:mb-1 prose-h1:text-2xs prose-h2:text-2xs prose-h3:text-2xs prose-h4:text-2xs prose-h5:text-2xs prose-h6:text-2xs prose-strong:text-secondary`, - sm: `${base} ${rhythm} ${bodyXs} prose-headings:mt-3 prose-headings:mb-1 prose-headings:font-medium prose-headings:text-emphasis prose-h1:text-sm prose-h2:text-xs prose-h3:text-xs prose-h4:text-xs prose-h5:text-xs prose-h6:text-xs`, + xs: `${base} prose-sm leading-snug prose-p:text-2xs prose-li:text-2xs prose-code:text-2xs prose-pre:text-2xs prose-headings:font-medium prose-headings:text-secondary prose-headings:mt-2 prose-headings:mb-1 prose-h1:text-sm prose-h2:text-xs prose-h3:text-2xs prose-h4:text-2xs prose-h5:text-2xs prose-h6:text-2xs prose-strong:text-secondary`, + sm: `${base} ${rhythm} ${bodyXs} prose-headings:mt-3 prose-headings:mb-1 prose-headings:font-medium prose-headings:text-emphasis prose-h1:text-base prose-h2:text-sm prose-h3:text-xs prose-h4:text-xs prose-h5:text-xs prose-h6:text-xs`, doc: `${base} ${rhythm} ${bodyXs} prose-headings:mt-8 prose-headings:mb-2 prose-headings:font-semibold prose-headings:text-emphasis prose-h1:text-lg prose-h2:text-base prose-h3:text-sm prose-h4:text-xs prose-h5:text-xs prose-h6:text-xs prose-pre:bg-transparent prose-pre:p-0` } as const diff --git a/frontend/src/routes/kitchen_sink/+page.svelte b/frontend/src/routes/kitchen_sink/+page.svelte index a001da222f..b3ae4ef196 100644 --- a/frontend/src/routes/kitchen_sink/+page.svelte +++ b/frontend/src/routes/kitchen_sink/+page.svelte @@ -24,6 +24,8 @@ ## Heading 2 +### Heading 3 + Body text with **bold**, *italic*, a [link](https://windmill.dev), and \`inline code\` that must stay readable in both themes. > A block quote should be legible too. @@ -68,7 +70,13 @@ Raw sanitized HTML (via rehypeRaw) must keep its content, not render empty: // AssistantMessage, and fenced code blocks go through CodeDisplay → // HighlightCode. Exercise several languages + prose so the code-block styling // can be tuned against the real render path, not an approximation. - const chatSampleContent = `Here's how you'd wire up the trigger. First, some prose with \`inline code\`, a [link](https://windmill.dev), and **bold** text so we can see how code sits next to surrounding content. + const chatSampleContent = `# Wiring up the trigger + +Here's how you'd wire up the trigger. First, some prose with \`inline code\`, a [link](https://windmill.dev), and **bold** text so we can see how code sits next to surrounding content. + +## The script + +### A subsection \`\`\`python def main(name: str = "world"): @@ -188,7 +196,19 @@ That's the full round-trip.` - +
+ The three markdownProse presets, same source. Each is sized for its own + surface: xs for group notes and chat reasoning, sm for chat + bubbles, sticky notes and markdown job results, doc for artifacts. +
+
+ {#each ['xs', 'sm', 'doc'] as const as prose} +
+
{prose}
+ +
+ {/each} +