From 999e673dd05ae5c31fbef491d0fcacc50277f5ba Mon Sep 17 00:00:00 2001 From: Guilhem Date: Tue, 11 Nov 2025 18:28:55 +0100 Subject: [PATCH] fix layout check --- .../lib/components/graph/FlowGraphV2.svelte | 30 +- .../lib/components/graph/groupNoteUtils.ts | 57 ++++ .../components/graph/noteManager.svelte.ts | 291 +++--------------- 3 files changed, 120 insertions(+), 258 deletions(-) diff --git a/frontend/src/lib/components/graph/FlowGraphV2.svelte b/frontend/src/lib/components/graph/FlowGraphV2.svelte index 34178f4680..c205fceadd 100644 --- a/frontend/src/lib/components/graph/FlowGraphV2.svelte +++ b/frontend/src/lib/components/graph/FlowGraphV2.svelte @@ -70,7 +70,7 @@ import type { AssetWithAltAccessType } from '../assets/lib' import type { AIModuleAction } from '../copilot/chat/flow/core' import { setGraphContext } from './graphContext' - import { buildNodeSpacingMap } from './groupNoteUtils' + import { buildNodeSpacingMap, getNoteStateSignature } from './groupNoteUtils' let useDataflow: Writable = writable(false) let showAssets: Writable = writable(true) @@ -272,16 +272,28 @@ type NodeDep = { id: string; parentIds?: string[]; offset?: number } type NodePos = { position: { x: number; y: number } } - let lastNodes: [NodeDep[], (NodeDep & NodePos)[]] | undefined = undefined + type LayoutCacheKey = { + nodes: NodeDep[] + noteSignature: ReturnType + } + let lastLayoutCache: { key: LayoutCacheKey; result: (NodeDep & NodePos)[] } | undefined = + undefined + function layoutNodes( nodes: NodeDep[], groupNotes: FlowNote[] = [], noteTextHeights: Record = {} ): (NodeDep & NodePos)[] { - let lastResult = lastNodes?.[1] - if (lastResult && deepEqual(nodes, lastNodes?.[0])) { - console.debug('layoutNodes', 'same nodes') - return lastResult + // Create comprehensive cache key that includes all layout dependencies + const currentCacheKey: LayoutCacheKey = { + nodes, + noteSignature: getNoteStateSignature(groupNotes, noteTextHeights) + } + + // Check if we can use cached result + if (lastLayoutCache && deepEqual(currentCacheKey, lastLayoutCache.key)) { + console.debug('layoutNodes', 'cache hit - same nodes and notes') + return lastLayoutCache.result } console.debug('layoutNodes', nodes.length) let seenId: string[] = [] @@ -360,7 +372,11 @@ } })) - lastNodes = [nodes, newNodes] + // Update cache with new results + lastLayoutCache = { + key: currentCacheKey, + result: newNodes + } return newNodes } diff --git a/frontend/src/lib/components/graph/groupNoteUtils.ts b/frontend/src/lib/components/graph/groupNoteUtils.ts index 99e487d204..6be396b463 100644 --- a/frontend/src/lib/components/graph/groupNoteUtils.ts +++ b/frontend/src/lib/components/graph/groupNoteUtils.ts @@ -151,3 +151,60 @@ export function buildNodeSpacingMap( return spacingMap } + +/** + * Creates a stable hash of the noteTextHeights object for cache comparison + */ +export function hashNoteTextHeights(noteTextHeights: Record): string { + const entries = Object.entries(noteTextHeights) + .sort(([a], [b]) => a.localeCompare(b)) // Sort for stable hash + + return JSON.stringify(entries) +} + +/** + * Extracts note state signature for layout cache comparison + */ +export function getNoteStateSignature(groupNotes: FlowNote[], noteTextHeights: Record) { + return { + notesCount: groupNotes.length, + noteIds: groupNotes.map(n => n.id).sort(), + textHeightHash: hashNoteTextHeights(noteTextHeights) + } +} + +/** + * Extracts layout-affecting signature for change detection + * Only includes properties that affect graph layout (structure, grouping) + */ +export function getLayoutSignature(notes: FlowNote[]) { + return { + notesCount: notes.length, + noteIds: notes.map(n => n.id).sort(), + // Group memberships affect layout spacing + groupMemberships: notes + .filter(note => note.type === 'group') + .map(note => ({ + id: note.id, + containedIds: note.contained_node_ids?.slice().sort() || [] + })) + .sort((a, b) => a.id.localeCompare(b.id)) + } +} + +/** + * Extracts property-only signature for change detection + * Only includes visual/content properties that don't affect layout + */ +export function getPropertySignature(notes: FlowNote[]) { + return notes + .map(note => ({ + id: note.id, + text: note.text, + color: note.color, + locked: note.locked || false, + position: { ...note.position }, + size: { ...note.size } + })) + .sort((a, b) => a.id.localeCompare(b.id)) +} diff --git a/frontend/src/lib/components/graph/noteManager.svelte.ts b/frontend/src/lib/components/graph/noteManager.svelte.ts index 77c7b0efff..34d088ebbd 100644 --- a/frontend/src/lib/components/graph/noteManager.svelte.ts +++ b/frontend/src/lib/components/graph/noteManager.svelte.ts @@ -1,6 +1,8 @@ import type { FlowNote } from '$lib/gen' import type { Node } from '@xyflow/svelte' import { calculateNodesBounds } from './util' +import { getLayoutSignature, getPropertySignature } from './groupNoteUtils' +import { deepEqual } from 'fast-equals' export type NodePosition = { id: string @@ -22,20 +24,12 @@ export class NoteManager { // Track notes for layout change detection #notes: () => FlowNote[] - #previousStructuralState: { count: number; groupMemberships: Record } = $state({ - count: 0, - groupMemberships: {} + #previousLayoutSignature: ReturnType = $state({ + notesCount: 0, + noteIds: [], + groupMemberships: [] }) - #previousPropertyState: Record< - string, - { - text: string - locked: boolean - color: string - position: { x: number; y: number } - size: { width: number; height: number } - } - > = $state({}) + #previousPropertySignature: ReturnType = $state([]) // Function to update nodes array with reactivity #setNodes: (nodes: Node[]) => void @@ -53,30 +47,27 @@ export class NoteManager { this.#getNodes = getNodes this.#editMode = editMode - // Effect to monitor both structural and property changes in notes + // Effect to monitor note changes with dual signature tracking $effect(() => { const currentNotes = this.#notes() - const currentStructuralState = this.#extractStructuralState(currentNotes) - const currentPropertyState = this.#extractPropertyState(currentNotes) + const currentLayoutSignature = getLayoutSignature(currentNotes) + const currentPropertySignature = getPropertySignature(currentNotes) - const hasStructuralChanges = this.#hasStructuralChanges( - currentStructuralState, - this.#previousStructuralState - ) - const propertyChanges = this.#getPropertyChanges( - currentPropertyState, - this.#previousPropertyState + const hasLayoutChanges = !deepEqual(currentLayoutSignature, this.#previousLayoutSignature) + const hasPropertyChanges = !deepEqual( + currentPropertySignature, + this.#previousPropertySignature ) - if (hasStructuralChanges) { + if (hasLayoutChanges) { // Structural changes require full re-render - this.#previousStructuralState = currentStructuralState - this.#previousPropertyState = currentPropertyState + this.#previousLayoutSignature = currentLayoutSignature + this.#previousPropertySignature = currentPropertySignature this.render() - } else if (propertyChanges.length > 0) { + } else if (hasPropertyChanges) { // Property changes can be handled with fast updates - this.#updateNodesProperties(propertyChanges) - this.#previousPropertyState = currentPropertyState + this.#updateNodesProperties(currentNotes) + this.#previousPropertySignature = currentPropertySignature } }) } @@ -88,238 +79,40 @@ export class NoteManager { this.renderCount++ } - /** - * Extract structural state from notes for change detection - */ - #extractStructuralState(notes: FlowNote[]): { - count: number - groupMemberships: Record - } { - const groupMemberships: Record = {} - - // Extract group memberships for group notes - notes - .filter((note) => note.type === 'group') - .forEach((note) => { - if (note.contained_node_ids) { - groupMemberships[note.id] = [...note.contained_node_ids].sort() // Sort for consistent comparison - } - }) - - return { - count: notes.length, - groupMemberships - } - } - - /** - * Check if there are structural changes that affect layout - */ - #hasStructuralChanges( - current: { count: number; groupMemberships: Record }, - previous: { count: number; groupMemberships: Record } - ): boolean { - // Check if note count changed - if (current.count !== previous.count) { - return true - } - - // Check if group memberships changed - const currentGroups = Object.keys(current.groupMemberships) - const previousGroups = Object.keys(previous.groupMemberships) - - // Different number of group notes - if (currentGroups.length !== previousGroups.length) { - return true - } - - // Check each group's membership - for (const groupId of currentGroups) { - const currentMembers = current.groupMemberships[groupId] - const previousMembers = previous.groupMemberships[groupId] - - // Group didn't exist before or membership changed - if (!previousMembers || !this.#arraysEqual(currentMembers, previousMembers)) { - return true - } - } - - return false - } - - /** - * Helper to compare two sorted arrays for equality - */ - #arraysEqual(a: string[], b: string[]): boolean { - if (a.length !== b.length) { - return false - } - return a.every((value, index) => value === b[index]) - } - - /** - * Extract property state from notes for change detection - */ - #extractPropertyState( - notes: FlowNote[] - ): Record< - string, - { - text: string - locked: boolean - color: string - position: { x: number; y: number } - size: { width: number; height: number } - } - > { - const propertyState: Record< - string, - { - text: string - locked: boolean - color: string - position: { x: number; y: number } - size: { width: number; height: number } - } - > = {} - - for (const note of notes) { - propertyState[note.id] = { - text: note.text, - locked: note.locked || false, - color: note.color, - position: { ...note.position }, - size: { ...note.size } - } - } - - return propertyState - } - - /** - * Get property changes between current and previous state - */ - #getPropertyChanges( - current: Record< - string, - { - text: string - locked: boolean - color: string - position: { x: number; y: number } - size: { width: number; height: number } - } - >, - previous: Record< - string, - { - text: string - locked: boolean - color: string - position: { x: number; y: number } - size: { width: number; height: number } - } - > - ): Array<{ noteId: string; property: string; oldValue: any; newValue: any }> { - const changes: Array<{ noteId: string; property: string; oldValue: any; newValue: any }> = [] - - for (const noteId of Object.keys(current)) { - const currentNote = current[noteId] - const previousNote = previous[noteId] - - if (!previousNote) continue // New note, will be handled by structural changes - - // Check each property for changes - if (currentNote.text !== previousNote.text) { - changes.push({ - noteId, - property: 'text', - oldValue: previousNote.text, - newValue: currentNote.text - }) - } - if (currentNote.locked !== previousNote.locked) { - changes.push({ - noteId, - property: 'locked', - oldValue: previousNote.locked, - newValue: currentNote.locked - }) - } - if (currentNote.color !== previousNote.color) { - changes.push({ - noteId, - property: 'color', - oldValue: previousNote.color, - newValue: currentNote.color - }) - } - if ( - currentNote.position.x !== previousNote.position.x || - currentNote.position.y !== previousNote.position.y - ) { - changes.push({ - noteId, - property: 'position', - oldValue: previousNote.position, - newValue: currentNote.position - }) - } - if ( - currentNote.size.width !== previousNote.size.width || - currentNote.size.height !== previousNote.size.height - ) { - changes.push({ - noteId, - property: 'size', - oldValue: previousNote.size, - newValue: currentNote.size - }) - } - } - - return changes + getCache(): Record { + return this.#cache } /** * Update node properties using setter function for proper reactivity + * Only updates visual properties that don't affect layout */ - #updateNodesProperties( - changes: Array<{ noteId: string; property: string; oldValue: any; newValue: any }> - ): void { + #updateNodesProperties(currentNotes: FlowNote[]): void { const currentNodes = this.#getNodes() if (currentNodes.length === 0) return // Create a new array with updated nodes to trigger reactivity const updatedNodes = currentNodes.map((node) => { - const change = changes.find((c) => c.noteId === node.id) - if (!change) return node + const note = currentNotes.find((n) => n.id === node.id) + if (!note) return node // Clone the node to avoid mutation const updatedNode = { ...node, data: { ...node.data } } - switch (change.property) { - case 'text': - if (updatedNode.data) updatedNode.data.text = change.newValue - break - case 'locked': - if (updatedNode.data) updatedNode.data.locked = change.newValue - // Update draggable property based on lock state and edit mode - updatedNode.draggable = updatedNode.data.isGroupNote - ? false - : this.#editMode && !change.newValue - break - case 'color': - if (updatedNode.data) updatedNode.data.color = change.newValue - break - case 'position': - updatedNode.position = { ...change.newValue } - break - case 'size': - updatedNode.width = change.newValue.width - updatedNode.height = change.newValue.height - updatedNode.style = `width: ${change.newValue.width}px; height: ${change.newValue.height}px;` - break + // Update properties that don't affect layout + if (updatedNode.data) { + updatedNode.data.text = note.text + updatedNode.data.color = note.color + updatedNode.data.locked = note.locked || false + } + + // Update draggable property based on lock state and edit mode + const isGroupNote = note.type === 'group' + updatedNode.draggable = isGroupNote ? false : this.#editMode && !note.locked + if (!isGroupNote) { + updatedNode.width = note.size.width + updatedNode.height = note.size.height + updatedNode.position = note.position } return updatedNode @@ -329,10 +122,6 @@ export class NoteManager { this.#setNodes(updatedNodes) } - getCache(): Record { - return this.#cache - } - /** * Calculate position and size for group notes based on contained nodes */