Implement further PR review feedback from Claude from a comprehensive review across performance, security, maintenance, etc

Author: jhodapp

__tests__/components/ui/coaching-sessions/coaching-notes/coaching-notes-markdown-extensions.test.tsx

@@ -112,5 +112,89 @@ describe('Coaching Notes Markdown Extensions', () => {
         expect(bodyCells.length).toBe(8)
       })
     })
+
+    it('should handle malformed markdown tables gracefully', async () => {
+      const TestEditor = () => {
+        const editor = useEditor({
+          extensions: Extensions(yDoc, null),
+          content: '',
+        })
+
+        if (!editor) return null
+
+        // Missing separator row - should not crash
+        const malformedTable = `| Name | Age |
+| Alice | 30 |`
+
+        editor.commands.insertContent(malformedTable, { contentType: 'markdown' })
+
+        return <EditorContent editor={editor} />
+      }
+
+      const { container } = render(<TestEditor />)
+
+      // Should not crash - might render as paragraph or incomplete table
+      await waitFor(() => {
+        expect(container).toBeTruthy()
+      })
+    })
+
+    it('should handle tables with leading spaces', async () => {
+      const TestEditor = () => {
+        const editor = useEditor({
+          extensions: Extensions(yDoc, null),
+          content: '',
+        })
+
+        if (!editor) return null
+
+        const tableWithSpaces = `  | Name | Age |
+  | --- | --- |
+  | Alice | 30 |`
+
+        editor.commands.insertContent(tableWithSpaces, { contentType: 'markdown' })
+
+        return <EditorContent editor={editor} />
+      }
+
+      const { container } = render(<TestEditor />)
+
+      await waitFor(() => {
+        const table = container.querySelector('table')
+        expect(table).toBeTruthy()
+
+        const headerCells = container.querySelectorAll('th')
+        expect(headerCells.length).toBe(2)
+      })
+    })
+
+    it('should handle empty table cells', async () => {
+      const TestEditor = () => {
+        const editor = useEditor({
+          extensions: Extensions(yDoc, null),
+          content: '',
+        })
+
+        if (!editor) return null
+
+        const emptyTable = `| Name | Age |
+| --- | --- |
+|  |  |`
+
+        editor.commands.insertContent(emptyTable, { contentType: 'markdown' })
+
+        return <EditorContent editor={editor} />
+      }
+
+      const { container } = render(<TestEditor />)
+
+      await waitFor(() => {
+        const table = container.querySelector('table')
+        expect(table).toBeTruthy()
+
+        const bodyCells = container.querySelectorAll('td')
+        expect(bodyCells.length).toBe(2)
+      })
+    })
   })
 })

package-lock.json

@@ -65,6 +65,7 @@
     "clsx": "^2.1.1",
     "cmdk": "^1.0.4",
     "date-fns": "^2.28.0",
+    "just-debounce-it": "^3.2.0",
     "lucide-react": "^0.469.0",
     "next": "15.4.7",
     "next-themes": "^0.4.6",

package.json

@@ -1,6 +1,10 @@
 import Link from "@tiptap/extension-link";
 import { Extension, markInputRule, markPasteRule } from "@tiptap/core";
 import type { Editor } from "@tiptap/react";
+import debounce from "just-debounce-it";
+
+/** Number of characters to search before and after cursor position when looking for zombie links */
+const ZOMBIE_LINK_SEARCH_RANGE = 100;
 
 /**
  * Triggers the link creation flow by setting an empty href,
@@ -91,7 +95,9 @@ export const LinkKeyboardShortcut = Extension.create({
 
 // Extension to clean up empty/zombie links automatically
 // This ensures consistent behavior whether link was created via button or keyboard
-export const LinkZombieCleanup = Extension.create({
+export const LinkZombieCleanup = Extension.create<{
+  debouncedCleanup?: (...args: any[]) => void;
+}>({
   name: "linkZombieCleanup",
 
   onCreate() {
@@ -114,8 +120,8 @@ export const LinkZombieCleanup = Extension.create({
           let foundEmptyLink = false;
 
           // Search around cursor position for empty links
-          const searchFrom = Math.max(0, from - 100);
-          const searchTo = Math.min(state.doc.content.size, to + 100);
+          const searchFrom = Math.max(0, from - ZOMBIE_LINK_SEARCH_RANGE);
+          const searchTo = Math.min(state.doc.content.size, to + ZOMBIE_LINK_SEARCH_RANGE);
 
           state.doc.nodesBetween(searchFrom, searchTo, (node, pos) => {
             if (foundEmptyLink) return false;
@@ -146,8 +152,20 @@ export const LinkZombieCleanup = Extension.create({
       }
     };
 
-    this.editor.on("selectionUpdate", cleanupEmptyLinks);
-    this.editor.on("transaction", cleanupEmptyLinks);
+    // Debounce cleanup to reduce performance impact during rapid editor changes
+    // 50ms delay is imperceptible while preventing excessive DOM searches
+    this.options.debouncedCleanup = debounce(cleanupEmptyLinks, 50);
+
+    this.editor.on("selectionUpdate", this.options.debouncedCleanup);
+    this.editor.on("transaction", this.options.debouncedCleanup);
+  },
+
+  onDestroy() {
+    // Clean up event listeners to prevent memory leaks
+    if (this.options.debouncedCleanup) {
+      this.editor.off("selectionUpdate", this.options.debouncedCleanup);
+      this.editor.off("transaction", this.options.debouncedCleanup);
+    }
   },
 });
 

src/components/ui/coaching-sessions/coaching-notes/extended-link-extension.tsx

@@ -1,4 +1,5 @@
 import type { Token } from "marked";
+import { marked } from "marked";
 import type {
   MarkdownParseHelpers,
   MarkdownRendererHelpers,
@@ -82,16 +83,6 @@ function isTableToken(token: unknown): token is TableToken {
   );
 }
 
-/** Type guard to check if a cell is a table header cell */
-function isTableHeaderCell(cell: TableCellNode): cell is TableHeaderCell {
-  return cell.type === 'tableHeader';
-}
-
-/** Type guard to check if a cell is a table body cell */
-function isTableBodyCell(cell: TableCellNode): cell is TableBodyCell {
-  return cell.type === 'tableCell';
-}
-
 const createHeaderCell = (
   cell: TableCellToken,
   helpers: MarkdownParseHelpers
@@ -232,7 +223,7 @@ const renderTableMarkdown = (
   const headerRow = rows[0];
 
   // Render header row and separator
-  if (headerRow?.content) {
+  if (headerRow?.content && headerRow.content.length > 0) {
     markdown += renderHeaderRow(headerRow, helpers);
     markdown += renderSeparatorRow(createColumnCount(headerRow.content.length));
   }
@@ -273,20 +264,29 @@ export const TableMarkdownPasteHandler = Extension.create({
             const text = event.clipboardData?.getData("text/plain");
             if (!text) return false;
 
-            // Check if the pasted text looks like a markdown table
-            // Construct regex from parts to avoid Tailwind CSS picking it up
-            const pipeChar = "|";
-            const tablePattern = new RegExp(
-              `\\${pipeChar}.*\\${pipeChar}.*\\n\\${pipeChar}[\\-:\\s${pipeChar}]+\\${pipeChar}`
-            );
-            const hasMarkdownTable = tablePattern.test(text);
+            // Use marked's tokenizer to detect tables (handles leading whitespace correctly)
+            const hasMarkdownTable = (() => {
+              try {
+                const tokens = marked.lexer(text);
+                return tokens.some(token => token.type === 'table');
+              } catch {
+                // If tokenization fails, fall back to default paste
+                return false;
+              }
+            })();
 
             if (hasMarkdownTable && this.editor.markdown) {
-              // Parse as markdown and insert
-              this.editor.commands.insertContent(text, {
-                contentType: "markdown",
-              });
-              return true; // Prevent default paste
+              try {
+                // Parse as markdown and insert
+                this.editor.commands.insertContent(text, {
+                  contentType: "markdown",
+                });
+                return true; // Prevent default paste
+              } catch (error) {
+                console.error("Failed to paste markdown table:", error);
+                // Fall through to default paste behavior
+                return false;
+              }
             }
 
             return false; // Allow default paste for non-table content