From 908221e896fdc084d9d368f9021bb564f01daeb9 Mon Sep 17 00:00:00 2001 From: siltomato Date: Fri, 18 Jul 2025 19:50:13 -0400 Subject: [PATCH 1/2] SF-3370 add web worker and improve blot formats delta creation --- .vscode/settings.json | 1 + .../ClientApp/angular.json | 6 +- .../ClientApp/package-lock.json | 7 + .../ClientApp/package.json | 1 + .../insight-formatting-utils.spec.ts | 481 ++++++++++++++++++ .../insight-formatting-utils.ts | 264 ++++++++++ .../insight-formatting.worker.ts | 36 ++ .../quill-insight-render.service.ts | 112 +++- .../ClientApp/src/tsconfig.spec.json | 1 - .../ClientApp/tsconfig.worker.json | 9 + src/SIL.XForge.Scripture/Startup.cs | 11 +- 11 files changed, 897 insertions(+), 32 deletions(-) create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting-utils.spec.ts create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting-utils.ts create mode 100644 src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting.worker.ts create mode 100644 src/SIL.XForge.Scripture/ClientApp/tsconfig.worker.json diff --git a/.vscode/settings.json b/.vscode/settings.json index cd544c8df8e..d0963208bc1 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -16,6 +16,7 @@ "Charis", "checkmark", "combobox", + "Comlink", "commenters", "compodoc", "deconsolidate", diff --git a/src/SIL.XForge.Scripture/ClientApp/angular.json b/src/SIL.XForge.Scripture/ClientApp/angular.json index a35c7f6508e..d16503730fb 100644 --- a/src/SIL.XForge.Scripture/ClientApp/angular.json +++ b/src/SIL.XForge.Scripture/ClientApp/angular.json @@ -63,7 +63,8 @@ "buildOptimizer": false, "sourceMap": true, "optimization": false, - "namedChunks": true + "namedChunks": true, + "webWorkerTsConfig": "tsconfig.worker.json" }, "configurations": { "production": { @@ -157,7 +158,8 @@ "includePaths": ["node_modules"] }, "assets": ["src/favicon.ico", "src/assets", "src/manifest.json"], - "sourceMap": false + "sourceMap": false, + "webWorkerTsConfig": "tsconfig.worker.json" } }, "lint": { diff --git a/src/SIL.XForge.Scripture/ClientApp/package-lock.json b/src/SIL.XForge.Scripture/ClientApp/package-lock.json index f3cbd413b85..543957ef288 100644 --- a/src/SIL.XForge.Scripture/ClientApp/package-lock.json +++ b/src/SIL.XForge.Scripture/ClientApp/package-lock.json @@ -37,6 +37,7 @@ "bowser": "^2.11.0", "bson-objectid": "^2.0.3", "buffer": "^5.7.1", + "comlink": "^4.4.2", "crc-32": "^1.2.2", "file-saver": "^2.0.5", "jszip": "^3.10.1", @@ -10948,6 +10949,12 @@ "node": ">= 0.8" } }, + "node_modules/comlink": { + "version": "4.4.2", + "resolved": "https://registry.npmjs.org/comlink/-/comlink-4.4.2.tgz", + "integrity": "sha512-OxGdvBmJuNKSCMO4NTl1L47VRp6xn2wG4F/2hYzB6tiCb709otOxtEYCSvK80PtjODfXXZu8ds+Nw5kVCjqd2g==", + "license": "Apache-2.0" + }, "node_modules/commander": { "version": "12.1.0", "resolved": "https://registry.npmjs.org/commander/-/commander-12.1.0.tgz", diff --git a/src/SIL.XForge.Scripture/ClientApp/package.json b/src/SIL.XForge.Scripture/ClientApp/package.json index e4b43f7bbdf..3539cfb3115 100644 --- a/src/SIL.XForge.Scripture/ClientApp/package.json +++ b/src/SIL.XForge.Scripture/ClientApp/package.json @@ -61,6 +61,7 @@ "bowser": "^2.11.0", "bson-objectid": "^2.0.3", "buffer": "^5.7.1", + "comlink": "^4.4.2", "crc-32": "^1.2.2", "file-saver": "^2.0.5", "jszip": "^3.10.1", diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting-utils.spec.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting-utils.spec.ts new file mode 100644 index 00000000000..8fb1a037006 --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting-utils.spec.ts @@ -0,0 +1,481 @@ +import Delta from 'quill-delta'; +import { LynxInsightType } from 'realtime-server/lib/esm/scriptureforge/models/lynx-insight'; +import { + analyzeOverlaps, + applyNonOverlappingOperations, + applyOverlappingGroup, + FormatOperation, + OverlapAnalysis, + processFormatOperations, + WorkerLynxInsight +} from './insight-formatting-utils'; + +describe('Insight formatting utils', () => { + function createTestInsight( + id: string, + type: LynxInsightType = 'warning', + index: number, + length: number + ): WorkerLynxInsight { + return { + id, + type, + range: { index, length } + }; + } + + function createFormatOperation( + typeKey: string, + index: number, + length: number, + insights: WorkerLynxInsight[] + ): FormatOperation { + return { + typeKey, + index, + length, + formatValue: insights + }; + } + + describe('analyzeOverlaps', () => { + it('should return empty arrays for empty input', () => { + const result: OverlapAnalysis = analyzeOverlaps([]); + + expect(result.nonOverlapping).toEqual([]); + expect(result.overlappingGroups).toEqual([]); + }); + + it('should identify non-overlapping operations', () => { + const insight1 = createTestInsight('1', 'warning', 0, 5); + const insight2 = createTestInsight('2', 'error', 10, 5); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 0, 5, [insight1]), + createFormatOperation('lynx-insight-error', 10, 5, [insight2]) + ]; + + const result: OverlapAnalysis = analyzeOverlaps(ops); + + expect(result.nonOverlapping).toEqual(ops); + expect(result.overlappingGroups).toEqual([]); + }); + + it('should identify overlapping operations', () => { + const insight1 = createTestInsight('1', 'warning', 0, 10); + const insight2 = createTestInsight('2', 'warning', 5, 10); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 0, 10, [insight1]), + createFormatOperation('lynx-insight-warning', 5, 10, [insight2]) + ]; + + const result: OverlapAnalysis = analyzeOverlaps(ops); + + expect(result.nonOverlapping).toEqual([]); + expect(result.overlappingGroups).toEqual([ops]); + }); + + it('should handle mixed overlapping and non-overlapping operations', () => { + const insight1 = createTestInsight('1', 'warning', 0, 5); + const insight2 = createTestInsight('2', 'warning', 3, 5); + const insight3 = createTestInsight('3', 'error', 20, 5); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 0, 5, [insight1]), + createFormatOperation('lynx-insight-warning', 3, 5, [insight2]), + createFormatOperation('lynx-insight-error', 20, 5, [insight3]) + ]; + + const result: OverlapAnalysis = analyzeOverlaps(ops); + + expect(result.nonOverlapping).toEqual([ops[2]]); + expect(result.overlappingGroups).toEqual([[ops[0], ops[1]]]); + }); + + it('should handle multiple overlapping groups', () => { + const insight1 = createTestInsight('1', 'warning', 0, 5); + const insight2 = createTestInsight('2', 'warning', 3, 5); + const insight3 = createTestInsight('3', 'error', 20, 5); + const insight4 = createTestInsight('4', 'error', 22, 5); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 0, 5, [insight1]), + createFormatOperation('lynx-insight-warning', 3, 5, [insight2]), + createFormatOperation('lynx-insight-error', 20, 5, [insight3]), + createFormatOperation('lynx-insight-error', 22, 5, [insight4]) + ]; + + const result: OverlapAnalysis = analyzeOverlaps(ops); + + expect(result.nonOverlapping).toEqual([]); + expect(result.overlappingGroups).toEqual([ + [ops[0], ops[1]], + [ops[2], ops[3]] + ]); + }); + + it('should handle adjacent operations as non-overlapping', () => { + const insight1 = createTestInsight('1', 'info', 0, 5); + const insight2 = createTestInsight('2', 'warning', 5, 5); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-info', 0, 5, [insight1]), + createFormatOperation('lynx-insight-warning', 5, 5, [insight2]) + ]; + + const result: OverlapAnalysis = analyzeOverlaps(ops); + + expect(result.nonOverlapping).toEqual(ops); + expect(result.overlappingGroups).toEqual([]); + }); + }); + + describe('applyNonOverlappingOperations', () => { + it('should return base delta for empty operations', () => { + const baseDelta = new Delta().retain(10, { bold: null }); + const result = applyNonOverlappingOperations(baseDelta, []); + + expect(result).toBe(baseDelta); + }); + + it('should apply single operation correctly', () => { + const baseDelta = new Delta().retain(10, { bold: null }); + const insight = createTestInsight('1', 'warning', 0, 5); + const ops: FormatOperation[] = [createFormatOperation('lynx-insight-warning', 0, 5, [insight])]; + + const result = applyNonOverlappingOperations(baseDelta, ops); + + const expected = new Delta([ + { retain: 5, attributes: { bold: null, 'lynx-insight-warning': [insight] } }, + { retain: 5, attributes: { bold: null } } + ]); + expect(result).toEqual(expected); + }); + + it('should apply multiple non-overlapping operations', () => { + const baseDelta = new Delta().retain(30, { bold: null }); + const insight1 = createTestInsight('1', 'info', 0, 5); + const insight2 = createTestInsight('2', 'warning', 10, 5); + const insight3 = createTestInsight('3', 'error', 20, 5); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-info', 0, 5, [insight1]), + createFormatOperation('lynx-insight-warning', 10, 5, [insight2]), + createFormatOperation('lynx-insight-error', 20, 5, [insight3]) + ]; + + const result = applyNonOverlappingOperations(baseDelta, ops); + + const expected = new Delta([ + { retain: 5, attributes: { bold: null, 'lynx-insight-info': [insight1] } }, + { retain: 5, attributes: { bold: null } }, + { retain: 5, attributes: { bold: null, 'lynx-insight-warning': [insight2] } }, + { retain: 5, attributes: { bold: null } }, + { retain: 5, attributes: { bold: null, 'lynx-insight-error': [insight3] } }, + { retain: 5, attributes: { bold: null } } + ]); + + expect(result).toEqual(expected); + }); + + it('should handle operations with gaps correctly', () => { + const baseDelta = new Delta().retain(25, { bold: null }); + const insight1 = createTestInsight('1', 'warning', 2, 3); + const insight2 = createTestInsight('2', 'error', 10, 5); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 2, 3, [insight1]), + createFormatOperation('lynx-insight-error', 10, 5, [insight2]) + ]; + + const result = applyNonOverlappingOperations(baseDelta, ops); + + const expected = new Delta([ + { retain: 2, attributes: { bold: null } }, + { retain: 3, attributes: { bold: null, 'lynx-insight-warning': [insight1] } }, + { retain: 5, attributes: { bold: null } }, + { retain: 5, attributes: { bold: null, 'lynx-insight-error': [insight2] } }, + { retain: 10, attributes: { bold: null } } + ]); + expect(result).toEqual(expected); + }); + }); + + describe('applyOverlappingGroup', () => { + it('should return base delta for empty group', () => { + const baseDelta = new Delta().retain(10, { bold: null }); + const result = applyOverlappingGroup(baseDelta, []); + + expect(result).toBe(baseDelta); + }); + + it('should apply multiple overlapping operations using compose', () => { + const baseDelta = new Delta().retain(10, { bold: null }); + const insight1 = createTestInsight('1', 'warning', 0, 8); + const insight2 = createTestInsight('2', 'error', 2, 6); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 0, 8, [insight1]), + createFormatOperation('lynx-insight-error', 2, 6, [insight2]) + ]; + + const result = applyOverlappingGroup(baseDelta, ops); + + const expectedDelta = new Delta([ + { retain: 2, attributes: { bold: null, 'lynx-insight-warning': [insight1] } }, + { + retain: 6, + attributes: { + bold: null, + 'lynx-insight-warning': [insight1], + 'lynx-insight-error': [insight2] + } + }, + { retain: 2, attributes: { bold: null } } + ]); + + expect(result).toEqual(expectedDelta); + }); + + it('should apply overlapping operations of the same type', () => { + const baseDelta = new Delta().retain(15, { bold: null }); + const insight1 = createTestInsight('1', 'warning', 0, 10); + const insight2 = createTestInsight('2', 'warning', 5, 8); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 0, 10, [insight1]), + createFormatOperation('lynx-insight-warning', 5, 8, [insight2]) + ]; + + const result = applyOverlappingGroup(baseDelta, ops); + + const expectedDelta = new Delta([ + { retain: 5, attributes: { bold: null, 'lynx-insight-warning': [insight1] } }, + { retain: 5, attributes: { bold: null, 'lynx-insight-warning': [insight1, insight2] } }, + { retain: 3, attributes: { bold: null, 'lynx-insight-warning': [insight2] } }, + { retain: 2, attributes: { bold: null } } + ]); + + expect(result).toEqual(expectedDelta); + }); + }); + + describe('processFormatOperations', () => { + it('should handle empty operations array', () => { + const baseDelta = new Delta().retain(10, { bold: null }); + const result = processFormatOperations(baseDelta, []); + + expect(result).toBe(baseDelta); + }); + + it('should handle mixed overlapping and non-overlapping operations', () => { + const baseDelta = new Delta().retain(30, { bold: null }); + const insight1 = createTestInsight('1', 'warning', 0, 8); + const insight2 = createTestInsight('2', 'error', 5, 8); + const insight3 = createTestInsight('3', 'info', 20, 5); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 0, 8, [insight1]), + createFormatOperation('lynx-insight-error', 5, 8, [insight2]), + createFormatOperation('lynx-insight-info', 20, 5, [insight3]) + ]; + + const result = processFormatOperations(baseDelta, ops); + + const expectedDelta = new Delta([ + { retain: 5, attributes: { bold: null, 'lynx-insight-warning': [insight1] } }, + { + retain: 3, + attributes: { + bold: null, + 'lynx-insight-warning': [insight1], + 'lynx-insight-error': [insight2] + } + }, + { retain: 5, attributes: { bold: null, 'lynx-insight-error': [insight2] } }, + { retain: 7, attributes: { bold: null } }, + { retain: 5, attributes: { bold: null, 'lynx-insight-info': [insight3] } }, + { retain: 5, attributes: { bold: null } } + ]); + + expect(result).toEqual(expectedDelta); + }); + + it('should handle multiple overlapping and non-overlapping groups', () => { + const baseDelta = new Delta().retain(40, { bold: null }); + const insight1 = createTestInsight('1', 'warning', 0, 10); + const insight2 = createTestInsight('2', 'error', 5, 10); // Overlaps with insight1 + const insight3 = createTestInsight('3', 'info', 25, 5); // Non-overlapping + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 0, 10, [insight1]), + createFormatOperation('lynx-insight-error', 5, 10, [insight2]), + createFormatOperation('lynx-insight-info', 25, 5, [insight3]) + ]; + + const result = processFormatOperations(baseDelta, ops); + + const expected = new Delta([ + { retain: 5, attributes: { bold: null, 'lynx-insight-warning': [insight1] } }, + { + retain: 5, + attributes: { + bold: null, + 'lynx-insight-warning': [insight1], + 'lynx-insight-error': [insight2] + } + }, + { retain: 5, attributes: { bold: null, 'lynx-insight-error': [insight2] } }, + { retain: 10, attributes: { bold: null } }, + { retain: 5, attributes: { bold: null, 'lynx-insight-info': [insight3] } }, + { retain: 10, attributes: { bold: null } } + ]); + expect(result).toEqual(expected); + }); + + it('should handle cross-group overlapping of same insight type', () => { + const baseDelta = new Delta().retain(30, { bold: null }); + + // Create insights that will form two overlapping groups, but with cross-group overlap + const insight1 = createTestInsight('1', 'warning', 0, 15); // Group 1 + const insight2 = createTestInsight('2', 'warning', 5, 15); // Group 1 (overlaps with insight1) + const insight3 = createTestInsight('3', 'warning', 10, 15); // Group 2 (overlaps with insight2 from Group 1) + const insight4 = createTestInsight('4', 'warning', 18, 10); // Group 2 (overlaps with insight3) + + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 0, 15, [insight1]), + createFormatOperation('lynx-insight-warning', 5, 15, [insight2]), + createFormatOperation('lynx-insight-warning', 10, 15, [insight3]), + createFormatOperation('lynx-insight-warning', 18, 10, [insight4]) + ]; + + const result = processFormatOperations(baseDelta, ops); + + expect(result.ops).toBeDefined(); + + // Find operations that have warning insights + const warningOps = result.ops!.filter( + op => op.attributes != null && op.attributes['lynx-insight-warning'] != null + ); + + expect(warningOps.length).toBeGreaterThan(0); + + // In the cross-group overlapping region (10-15), we should have insights from both groups + // This tests that applyOverlappingGroup results are properly merged across groups + const crossGroupOverlapOp = warningOps.find(op => { + const insights = op.attributes!['lynx-insight-warning'] as WorkerLynxInsight[]; + return Array.isArray(insights) && insights.length >= 3; // Should have insights from both groups + }); + + expect(crossGroupOverlapOp).toBeDefined(); + + if (crossGroupOverlapOp) { + const insights = crossGroupOverlapOp.attributes!['lynx-insight-warning'] as WorkerLynxInsight[]; + // The overlapping region should contain insights from multiple groups + const insightIds = insights.map(insight => insight.id); + expect(insightIds).toContain('1'); // From Group 1 + expect(insightIds).toContain('2'); // From Group 1 + expect(insightIds).toContain('3'); // From Group 2 + } + + // Verify all insights are preserved somewhere + const allInsights = warningOps.flatMap(op => op.attributes!['lynx-insight-warning'] as WorkerLynxInsight[]); + const allInsightIds = allInsights.map(insight => insight.id); + expect(allInsightIds).toContain('1'); + expect(allInsightIds).toContain('2'); + expect(allInsightIds).toContain('3'); + expect(allInsightIds).toContain('4'); + }); + + it('should produce precise delta operations with exact boundaries', () => { + const baseDelta = new Delta().retain(50, { bold: null }); + const insight1 = createTestInsight('1', 'warning', 10, 8); + const insight2 = createTestInsight('2', 'error', 14, 8); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 10, 8, [insight1]), + createFormatOperation('lynx-insight-error', 14, 8, [insight2]) + ]; + + const result = processFormatOperations(baseDelta, ops); + + const expectedDelta = new Delta([ + { retain: 10, attributes: { bold: null } }, + { retain: 4, attributes: { bold: null, 'lynx-insight-warning': [insight1] } }, + { + retain: 4, + attributes: { + bold: null, + 'lynx-insight-warning': [insight1], + 'lynx-insight-error': [insight2] + } + }, + { retain: 4, attributes: { bold: null, 'lynx-insight-error': [insight2] } }, + { retain: 28, attributes: { bold: null } } + ]); + + expect(result).toEqual(expectedDelta); + }); + + it('should process non-overlapping operations efficiently', () => { + const baseDelta = new Delta().retain(100, { italic: true }); + const insight1 = createTestInsight('1', 'info', 5, 10); + const insight2 = createTestInsight('2', 'warning', 30, 8); + const insight3 = createTestInsight('3', 'error', 60, 12); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-info', 5, 10, [insight1]), + createFormatOperation('lynx-insight-warning', 30, 8, [insight2]), + createFormatOperation('lynx-insight-error', 60, 12, [insight3]) + ]; + + const result = processFormatOperations(baseDelta, ops); + + const expectedDelta = new Delta([ + { retain: 5, attributes: { italic: true } }, + { retain: 10, attributes: { italic: true, 'lynx-insight-info': [insight1] } }, + { retain: 15, attributes: { italic: true } }, + { retain: 8, attributes: { italic: true, 'lynx-insight-warning': [insight2] } }, + { retain: 22, attributes: { italic: true } }, + { retain: 12, attributes: { italic: true, 'lynx-insight-error': [insight3] } }, + { retain: 28, attributes: { italic: true } } + ]); + + expect(result).toEqual(expectedDelta); + }); + + it('should deduplicate insights with same id in overlapping regions', () => { + const baseDelta = new Delta().retain(30, { bold: null }); + const insight1 = createTestInsight('duplicate', 'warning', 5, 10); + const insight2 = createTestInsight('duplicate', 'warning', 8, 10); // Same id - should be deduplicated + const insight3 = createTestInsight('unique', 'warning', 10, 8); + const ops: FormatOperation[] = [ + createFormatOperation('lynx-insight-warning', 5, 10, [insight1]), + createFormatOperation('lynx-insight-warning', 8, 10, [insight2]), + createFormatOperation('lynx-insight-warning', 10, 8, [insight3]) + ]; + + const result = processFormatOperations(baseDelta, ops); + + const warningOps = result.ops!.filter( + op => op.attributes != null && op.attributes['lynx-insight-warning'] != null + ); + + // Each operation should have at most one instance of the duplicate insight + warningOps.forEach(op => { + const insights = op.attributes!['lynx-insight-warning'] as WorkerLynxInsight[]; + const duplicateInsights = insights.filter(insight => insight.id === 'duplicate'); + expect(duplicateInsights.length).toBeLessThanOrEqual(1); + }); + + // Verify both unique insights are present + const allInsights = warningOps.flatMap(op => op.attributes!['lynx-insight-warning'] as WorkerLynxInsight[]); + const uniqueIds = new Set(allInsights.map(insight => insight.id)); + expect(uniqueIds.has('duplicate')).toBe(true); + expect(uniqueIds.has('unique')).toBe(true); + expect(uniqueIds.size).toBe(2); + }); + + it('should maintain original delta structure when no insights are applied', () => { + const originalOps = [ + { retain: 5, attributes: { bold: true } }, + { retain: 10, attributes: { italic: true } }, + { retain: 15 } + ]; + const baseDelta = new Delta(originalOps); + + const result = processFormatOperations(baseDelta, []); + + expect(result).toBe(baseDelta); // Should return the exact same instance + expect(result.ops).toEqual(originalOps); + }); + }); +}); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting-utils.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting-utils.ts new file mode 100644 index 00000000000..a1f2bfc9f64 --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting-utils.ts @@ -0,0 +1,264 @@ +import Delta, { AttributeMap } from 'quill-delta'; + +/** + * Minimal insight interface for formatting operations. + * Used instead of 'LynxInsight' in web worker to avoid importing Angular dependencies to worker. + */ +export interface WorkerLynxInsight { + id: string; + type: string; + range: { + index: number; + length: number; + }; + // Other properties as needed + [key: string]: any; +} + +/** + * Represents a formatting operation to be applied to text. + */ +export interface FormatOperation { + typeKey: string; + index: number; + length: number; + formatValue: WorkerLynxInsight[]; +} + +/** + * Result of overlap analysis for format operations. + */ +export interface OverlapAnalysis { + nonOverlapping: FormatOperation[]; + overlappingGroups: FormatOperation[][]; +} + +/** + * Analyzes format operations to detect insight range overlaps and group them accordingly. + */ +export function analyzeOverlaps(sortedOps: FormatOperation[]): OverlapAnalysis { + const nonOverlapping: FormatOperation[] = []; + const overlappingGroups: FormatOperation[][] = []; + + let currentGroup: FormatOperation[] | null = null; + let lastEnd: number = -1; + + for (const op of sortedOps) { + const opStart: number = op.index; + const opEnd: number = op.index + op.length; + + // Check if this operation overlaps with the previous one + if (opStart < lastEnd) { + // Overlapping - ensure we have a group + if (currentGroup == null) { + // Start new overlapping group, include the previous non-overlapping op + const prevOp: FormatOperation | undefined = nonOverlapping.pop(); + currentGroup = prevOp != null ? [prevOp, op] : [op]; + } else { + currentGroup.push(op); + } + } else { + // Non-overlapping + if (currentGroup != null) { + // Close current overlapping group + overlappingGroups.push(currentGroup); + currentGroup = null; + } + nonOverlapping.push(op); + } + + lastEnd = Math.max(lastEnd, opEnd); + } + + // Don't forget the last group if it exists + if (currentGroup != null) { + overlappingGroups.push(currentGroup); + } + + return { nonOverlapping, overlappingGroups }; +} + +/** + * Applies non-overlapping operations efficiently by building delta operations directly + * (not calling `delta.compose()` after each). + */ +export function applyNonOverlappingOperations(baseDelta: Delta, operations: FormatOperation[]): Delta { + if (operations.length === 0) return baseDelta; + + // Build operations array directly without compose + const newOps: Array<{ retain: number; attributes?: { [key: string]: any } }> = []; + let currentIndex: number = 0; + + for (const op of operations) { + // Add retain operation to reach the operation index + if (op.index > currentIndex) { + newOps.push({ retain: op.index - currentIndex }); + } + + // Add the format operation + newOps.push({ + retain: op.length, + attributes: { [op.typeKey]: op.formatValue } + }); + + currentIndex = op.index + op.length; + } + + const directDelta: Delta = new Delta(newOps); + return baseDelta.compose(directDelta); +} + +/** + * Applies overlapping operations using `delta.compose()` after each. + * Handles merging of same-type insights in overlapping regions. + */ +export function applyOverlappingGroup(baseDelta: Delta, group: FormatOperation[]): Delta { + if (group.length === 0) { + return baseDelta; + } + + if (group.length === 1) { + // Single operation, use simple compose + const operation: FormatOperation = group[0]; + const deltaToApply: Delta = new Delta() + .retain(operation.index) + .retain(operation.length, { [operation.typeKey]: operation.formatValue }); + return baseDelta.compose(deltaToApply); + } + + // For multiple overlapping operations, we need custom merging logic + // to handle same-type insights properly + let result: Delta = baseDelta; + + // Apply operations one by one, but with custom attribute merging + for (const operation of group) { + const deltaToApply: Delta = new Delta() + .retain(operation.index) + .retain(operation.length, { [operation.typeKey]: operation.formatValue }); + + // Custom compose that merges insight arrays of the same type + result = composeWithInsightMerging(result, deltaToApply); + } + + return result; +} + +/** + * Custom compose function that merges insight arrays when the same insight type + * appears in overlapping regions, rather than overwriting them. + */ +function composeWithInsightMerging(baseDelta: Delta, otherDelta: Delta): Delta { + // Check if we need custom merging at all + const needsMerging: boolean = otherDelta.ops.some( + op => op.attributes && Object.keys(op.attributes).some(key => key.startsWith('lynx-insight-')) + ); + + if (!needsMerging) { + return baseDelta.compose(otherDelta); + } + + // Temporarily override AttributeMap.compose to use our custom merging + const originalCompose = Delta.AttributeMap.compose; + + try { + Delta.AttributeMap.compose = (a?: AttributeMap, b?: AttributeMap, keepNull?: boolean) => { + // Use our custom merging for insight attributes + const merged: AttributeMap | undefined = mergeInsightAttributes(a, b); + + // For null handling, follow the original logic + if (!keepNull && merged) { + // Filter out null/undefined values + const filtered: any = {}; + for (const key in merged) { + if (merged[key] != null) { + filtered[key] = merged[key]; + } + } + return filtered; + } + + return Object.keys(merged || {}).length > 0 ? merged : undefined; + }; + + // Use Delta's standard compose with our custom attribute merging + return baseDelta.compose(otherDelta); + } finally { + // Always restore the original function + Delta.AttributeMap.compose = originalCompose; + } +} + +/** + * Merges attributes, with special handling for insight arrays. + */ +function mergeInsightAttributes( + baseAttrs: AttributeMap | null | undefined, + otherAttrs: AttributeMap | null | undefined +): AttributeMap | undefined { + if (baseAttrs == null && otherAttrs == null) { + return undefined; + } + + if (baseAttrs == null) { + return { ...otherAttrs }; + } + + if (otherAttrs == null) { + return { ...baseAttrs }; + } + + const merged: AttributeMap = { ...baseAttrs }; + + for (const key in otherAttrs) { + if (key.startsWith('lynx-insight-') && merged[key] != null) { + // Merge insight arrays - ensure both are arrays + const baseInsights: WorkerLynxInsight[] = Array.isArray(merged[key]) + ? (merged[key] as WorkerLynxInsight[]) + : [merged[key] as WorkerLynxInsight]; + const otherInsights: WorkerLynxInsight[] = Array.isArray(otherAttrs[key]) + ? (otherAttrs[key] as WorkerLynxInsight[]) + : [otherAttrs[key] as WorkerLynxInsight]; + + // Deduplicate by insight id and merge + const seenIds = new Set(baseInsights.map((insight: WorkerLynxInsight) => insight.id)); + const newInsights = otherInsights.filter((insight: WorkerLynxInsight) => !seenIds.has(insight.id)); + + merged[key] = [...baseInsights, ...newInsights]; + } else { + // For non-insight attributes or when base doesn't have this key, use other's value + merged[key] = otherAttrs[key]; + } + } + + return merged; +} + +/** + * Main processing function that applies format operations efficiently. + * Handles both overlapping and non-overlapping operations optimally. + */ +export function processFormatOperations(baseDelta: Delta, formatOperations: FormatOperation[]): Delta { + // Sort operations by index for overlap detection + const sortedOps: FormatOperation[] = [...formatOperations].sort((a, b) => a.index - b.index); + + // Detect and group overlapping operations - overlapping insights need 'delta.compose()' after each + // in order for quill to process them correctly + const { nonOverlapping, overlappingGroups } = analyzeOverlaps(sortedOps); + + let delta: Delta = baseDelta; + + // Process non-overlapping operations efficiently (no 'delta.compose()' needed) + if (nonOverlapping.length > 0) { + delta = applyNonOverlappingOperations(delta, nonOverlapping); + } + + // Process overlapping groups - each group gets applied with insight merging + for (const group of overlappingGroups) { + // Apply the group to a clean delta, then merge with accumulated result + const cleanBaseDelta = new Delta().retain(delta.length()); + const groupDelta: Delta = applyOverlappingGroup(cleanBaseDelta, group); + delta = composeWithInsightMerging(delta, groupDelta); + } + + return delta; +} diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting.worker.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting.worker.ts new file mode 100644 index 00000000000..41a7b3feeaa --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/insight-formatting.worker.ts @@ -0,0 +1,36 @@ +import * as Comlink from 'comlink'; +import Delta from 'quill-delta'; +import { FormatOperation, processFormatOperations } from './insight-formatting-utils'; + +/** + * Interface for the insight formatting worker api. + * Shared between worker implementation and service usage. + */ +export interface InsightFormattingWorkerApi { + formatInsights( + editorLength: number, + formatsToRemove: { [key: string]: null }, + formatOperations: FormatOperation[] + ): Promise; +} + +class InsightFormattingWorker implements InsightFormattingWorkerApi { + async formatInsights( + editorLength: number, + formatsToRemove: { [key: string]: null }, + formatOperations: FormatOperation[] + ): Promise { + try { + // Apply removal of formats + const delta: Delta = new Delta().retain(editorLength, formatsToRemove); + + return processFormatOperations(delta, formatOperations); + } catch (error) { + console.error('Worker: Error in formatInsights:', error); + throw error; + } + } +} + +const worker = new InsightFormattingWorker(); +Comlink.expose(worker); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/quill-insight-render.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/quill-insight-render.service.ts index 07ec5e23e96..951591b6289 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/quill-insight-render.service.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/quill-insight-render.service.ts @@ -1,4 +1,5 @@ -import { Injectable } from '@angular/core'; +import { Injectable, OnDestroy } from '@angular/core'; +import * as Comlink from 'comlink'; import Quill, { Delta } from 'quill'; import { LynxInsightTypes } from 'realtime-server/lib/esm/scriptureforge/models/lynx-insight'; import { StringMap } from 'rich-text'; @@ -10,22 +11,37 @@ import { LynxInsightOverlayRef, LynxInsightOverlayService } from '../lynx-insigh import { LynxInsightStateService } from '../lynx-insight-state.service'; import { getLeadingInsight, getMostNestedInsight } from '../lynx-insight-util'; import { LynxInsightBlot } from './blots/lynx-insight-blot'; +import { FormatOperation, processFormatOperations, WorkerLynxInsight } from './insight-formatting-utils'; +import { InsightFormattingWorkerApi } from './insight-formatting.worker'; import { QuillLynxEditorAdapter } from './quill-lynx-editor-adapter'; @Injectable({ providedIn: 'root' }) -export class QuillInsightRenderService extends InsightRenderService { +export class QuillInsightRenderService extends InsightRenderService implements OnDestroy { readonly prefix = 'lynx-insight'; readonly editorAttentionClass = `${this.prefix}-attention`; readonly activeInsightClass = `action-overlay-active`; readonly cursorActiveClass = `cursor-active`; + private workerApi?: Comlink.Remote; + constructor( private readonly overlayService: LynxInsightOverlayService, private insightState: LynxInsightStateService ) { super(); + + // Create worker if browser supports it + if (Worker != null) { + try { + const worker = new Worker(new URL('./insight-formatting.worker', import.meta.url)); + this.workerApi = Comlink.wrap(worker); + } catch (error) { + // Log the error for debugging but continue gracefully + console.error('Failed to initialize insight-formatting worker:', error); + } + } } /** @@ -57,19 +73,56 @@ export class QuillInsightRenderService extends InsightRenderService { * Creates a delta with all the insights' formatting applied, and sets the editor contents to that delta. * This avoids multiple calls to quill `formatText`, which will re-render the DOM after each call. */ - private refreshInsightFormatting(insights: LynxInsight[], editor: Quill): void { + private async refreshInsightFormatting(insights: LynxInsight[], editor: Quill): Promise { + const formatsToRemove: StringMap = this.getFormatsToRemove(); + const formatOperations: FormatOperation[] = this.prepareFormatOperations(insights); + + let resultDelta: Delta | null = null; + + // Try worker first if available + if (this.workerApi != null) { + try { + resultDelta = await this.workerApi.formatInsights(editor.getLength(), formatsToRemove, formatOperations); + } catch (error) { + // Worker failed, will fall back to main thread below + console.error('Insight formatting worker failed, falling back to main thread processing.', error); + } + } + + // Use main thread if worker unavailable or failed + resultDelta ??= this.formatInsightsOnMainThread(editor.getLength(), formatsToRemove, formatOperations); + + // Update editor + editor.updateContents(resultDelta, 'api'); + } + + ngOnDestroy(): void { + if (this.workerApi != null) { + this.workerApi[Comlink.releaseProxy](); + } + } + + private getFormatsToRemove(): StringMap { + const formatsToRemove: StringMap = {}; + for (const type of LynxInsightTypes) { + formatsToRemove[`${this.prefix}-${type}`] = null; + } + return formatsToRemove; + } + + private prepareFormatOperations(insights: LynxInsight[]): FormatOperation[] { // Group insights by type and range const insightsByTypeAndRange = new Map>(); for (const insight of insights) { - const typeKey = `${this.prefix}-${insight.type}`; - const rangeKey = `${insight.range.index}:${insight.range.length}`; + const typeKey: string = `${this.prefix}-${insight.type}`; + const rangeKey: string = `${insight.range.index}:${insight.range.length}`; if (!insightsByTypeAndRange.has(typeKey)) { insightsByTypeAndRange.set(typeKey, new Map()); } - const rangeMap = insightsByTypeAndRange.get(typeKey)!; + const rangeMap: Map = insightsByTypeAndRange.get(typeKey)!; if (!rangeMap.has(rangeKey)) { rangeMap.set(rangeKey, []); @@ -78,33 +131,36 @@ export class QuillInsightRenderService extends InsightRenderService { rangeMap.get(rangeKey)!.push(insight); } - // Prepare formats to remove - const formatsToRemove: StringMap = {}; - for (const type of LynxInsightTypes) { - formatsToRemove[`${this.prefix}-${type}`] = null; - } - - // Apply removal of formats - let delta = new Delta().retain(editor.getLength(), formatsToRemove); + const formatOperations: FormatOperation[] = []; - // Apply formats by group, merging each format op with the result of the prev (let quill handle overlapping formats) for (const [typeKey, rangeMap] of insightsByTypeAndRange.entries()) { for (const [rangeKey, rangeInsights] of rangeMap.entries()) { - const [indexStr, lengthStr] = rangeKey.split(':'); - const index = parseInt(indexStr, 10); - const length = parseInt(lengthStr, 10); - - // Pass single insight or array based on count - const formatValue = rangeInsights.length === 1 ? rangeInsights[0] : rangeInsights; - - const deltaToApply = new Delta().retain(index).retain(length, { [typeKey]: formatValue }); - - delta = delta.compose(deltaToApply); + const [indexStr, lengthStr]: string[] = rangeKey.split(':'); + const index: number = Number.parseInt(indexStr, 10); + const length: number = Number.parseInt(lengthStr, 10); + const formatValue: WorkerLynxInsight[] = rangeInsights as WorkerLynxInsight[]; + + formatOperations.push({ + typeKey, + index, + length, + formatValue + }); } } - // Update editor - editor.updateContents(delta, 'api'); + return formatOperations; + } + + private formatInsightsOnMainThread( + editorLength: number, + formatsToRemove: StringMap, + formatOperations: FormatOperation[] + ): Delta { + // Apply removal of formats + const baseDelta: Delta = new Delta().retain(editorLength, formatsToRemove); + + return processFormatOperations(baseDelta, formatOperations); } renderActionOverlay( @@ -114,7 +170,7 @@ export class QuillInsightRenderService extends InsightRenderService { actionOverlayActive: boolean ): void { this.overlayService.close(); - let editorAttention = false; + let editorAttention: boolean = false; if (actionOverlayActive) { const leadingInsight: LynxInsight | undefined = getLeadingInsight(insights); diff --git a/src/SIL.XForge.Scripture/ClientApp/src/tsconfig.spec.json b/src/SIL.XForge.Scripture/ClientApp/src/tsconfig.spec.json index 909c42ca258..70add2d529d 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/tsconfig.spec.json +++ b/src/SIL.XForge.Scripture/ClientApp/src/tsconfig.spec.json @@ -2,7 +2,6 @@ "extends": "../tsconfig.json", "compilerOptions": { "outDir": "../out-tsc/spec", - "module": "es2015", "types": ["jasmine", "node"] }, "files": ["test.ts", "polyfills.ts"], diff --git a/src/SIL.XForge.Scripture/ClientApp/tsconfig.worker.json b/src/SIL.XForge.Scripture/ClientApp/tsconfig.worker.json new file mode 100644 index 00000000000..6cc331e24fa --- /dev/null +++ b/src/SIL.XForge.Scripture/ClientApp/tsconfig.worker.json @@ -0,0 +1,9 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "./out-tsc/worker", + "lib": ["es2018", "webworker"], + "types": [] + }, + "include": ["src/**/*.worker.ts"] +} diff --git a/src/SIL.XForge.Scripture/Startup.cs b/src/SIL.XForge.Scripture/Startup.cs index 94ed2738878..d8cb271c78b 100644 --- a/src/SIL.XForge.Scripture/Startup.cs +++ b/src/SIL.XForge.Scripture/Startup.cs @@ -81,6 +81,7 @@ public class Startup private static readonly HashSet ProductionSpaPostRoutes = []; private static readonly HashSet SpaPostRoutes = []; private const string SpaGetRoutesLynxPrefix = "node_modules_sillsdev_lynx"; + private const string SpaGetRoutesWorkerSuffix = "worker_ts.js"; public Startup(IConfiguration configuration, IWebHostEnvironment env, ILoggerFactory loggerFactory) { @@ -345,8 +346,16 @@ internal bool IsSpaRoute(HttpContext context) int periodIndex = path.IndexOf("."); prefix = prefix[..(periodIndex - 1)]; } - if (context.Request.Method == HttpMethods.Get && prefix.StartsWith(SpaGetRoutesLynxPrefix)) + + bool isLazyChunkRoute = + context.Request.Method == HttpMethods.Get + && (prefix.StartsWith(SpaGetRoutesLynxPrefix) || prefix.EndsWith(SpaGetRoutesWorkerSuffix)); + + if (isLazyChunkRoute) + { return true; + } + return (context.Request.Method == HttpMethods.Get && SpaGetRoutes.Contains(prefix)) || (context.Request.Method == HttpMethods.Post && SpaPostRoutes.Contains(prefix)); } From 706cf8681015e657d871885072cbf88449dd63bc Mon Sep 17 00:00:00 2001 From: siltomato Date: Thu, 31 Jul 2025 16:54:52 -0400 Subject: [PATCH 2/2] move function placment in file --- .../quill-services/quill-insight-render.service.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/quill-insight-render.service.ts b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/quill-insight-render.service.ts index 951591b6289..1d548d6d3cb 100644 --- a/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/quill-insight-render.service.ts +++ b/src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/lynx/insights/quill-services/quill-insight-render.service.ts @@ -44,6 +44,12 @@ export class QuillInsightRenderService extends InsightRenderService implements O } } + ngOnDestroy(): void { + if (this.workerApi != null) { + this.workerApi[Comlink.releaseProxy](); + } + } + /** * Renders the insights in the editor, applying formatting, action menus, and attention (opacity overlay). */ @@ -96,12 +102,6 @@ export class QuillInsightRenderService extends InsightRenderService implements O editor.updateContents(resultDelta, 'api'); } - ngOnDestroy(): void { - if (this.workerApi != null) { - this.workerApi[Comlink.releaseProxy](); - } - } - private getFormatsToRemove(): StringMap { const formatsToRemove: StringMap = {}; for (const type of LynxInsightTypes) {