From 0b87823ac56efada34d7accf6bf3973bb66965ab Mon Sep 17 00:00:00 2001
From: Ngo Quoc Dat <datlechin@gmail.com>
Date: Tue, 14 Jul 2026 16:22:56 +0700
Subject: [PATCH] fix(datagrid): resolve header comments from the header view
 to stop a sort crash (#1869)

Claude-Session: https://claude.ai/code/session_01P7qP2VjoTKfq4Nh19roxtu
---
 CHANGELOG.md                                  |  4 ++
 .../Views/Results/DataGridColumnPool.swift    | 64 ++++++++++---------
 .../Views/Results/SortableHeaderCell.swift    | 13 ++--
 .../Views/Results/SortableHeaderView.swift    | 15 +++++
 .../Results/DataGridColumnPoolTests.swift     | 16 +++--
 .../Results/SortableHeaderCellTests.swift     | 59 +++++++++++++++--
 6 files changed, 120 insertions(+), 51 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1a4081f5e..6f1c46614 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fixed a crash when clicking a column header on a table whose columns have comments. Sorting such a table quit the app immediately. (#1869)
+
 ## [0.57.0] - 2026-07-14
 
 ### Added
diff --git a/TablePro/Views/Results/DataGridColumnPool.swift b/TablePro/Views/Results/DataGridColumnPool.swift
index 9da21d528..4226de7d9 100644
--- a/TablePro/Views/Results/DataGridColumnPool.swift
+++ b/TablePro/Views/Results/DataGridColumnPool.swift
@@ -41,7 +41,8 @@ final class DataGridColumnPool {
 
         let willRestoreWidths = !(savedLayout?.columnWidths.isEmpty ?? true)
         let hiddenFromLayout = savedLayout?.hiddenColumns ?? []
-        var commentsChanged = false
+        var comments: [NSUserInterfaceItemIdentifier: String] = [:]
+        var showsComments = false
 
         for slot in 0..<pooledColumns.count {
             let column = pooledColumns[slot]
@@ -50,28 +51,30 @@ final class DataGridColumnPool {
                 let resolvedWidth = willRestoreWidths
                     ? (savedLayout?.columnWidths[columnName] ?? widthCalculator(columnName, slot))
                     : widthCalculator(columnName, slot)
-                if configureColumn(
+                let comment = displayableComment(columnComments[columnName])
+                configureColumn(
                     column,
                     name: columnName,
                     columnType: slot < columnTypes.count ? columnTypes[slot] : nil,
-                    comment: columnComments[columnName],
+                    comment: comment,
                     width: resolvedWidth,
                     isEditable: isEditable
-                ) {
-                    commentsChanged = true
-                }
+                )
                 let hidden = hiddenFromLayout.contains(columnName) || hiddenColumnNames.contains(columnName)
                 if column.isHidden != hidden {
                     column.isHidden = hidden
                 }
+                if let comment {
+                    comments[column.identifier] = comment
+                    if !hidden {
+                        showsComments = true
+                    }
+                }
             } else if !column.isHidden {
                 column.isHidden = true
             }
         }
-        updateHeaderHeight(in: tableView, showsComments: hasVisibleComments(visibleCount: visibleCount))
-        if commentsChanged {
-            tableView.headerView?.needsDisplay = true
-        }
+        applyComments(comments, showsComments: showsComments, in: tableView)
 
         let targetOrder = computeTargetOrder(
             visibleCount: visibleCount,
@@ -190,18 +193,13 @@ final class DataGridColumnPool {
         comment: String?,
         width: CGFloat,
         isEditable: Bool
-    ) -> Bool {
+    ) {
         if !(column.headerCell is SortableHeaderCell) || column.headerCell.stringValue != name {
             let cell = SortableHeaderCell(textCell: name)
             cell.font = column.headerCell.font
             cell.alignment = column.headerCell.alignment
             column.headerCell = cell
         }
-        var commentChanged = false
-        if let headerCell = column.headerCell as? SortableHeaderCell, headerCell.headerComment != comment {
-            headerCell.headerComment = comment
-            commentChanged = true
-        }
 
         var tooltip: String
         if let typeName = columnType?.rawType ?? columnType?.displayName {
@@ -209,14 +207,14 @@ final class DataGridColumnPool {
         } else {
             tooltip = name
         }
-        if let comment, !comment.isEmpty {
+        if let comment {
             tooltip += "\n\(comment)"
         }
         if column.headerToolTip != tooltip {
             column.headerToolTip = tooltip
         }
 
-        let label = String(format: String(localized: "Column: %@"), name)
+        let label = accessibilityLabel(name: name, comment: comment)
         if column.headerCell.accessibilityLabel() != label {
             column.headerCell.setAccessibilityLabel(label)
         }
@@ -230,23 +228,27 @@ final class DataGridColumnPool {
         if column.sortDescriptorPrototype?.key != name {
             column.sortDescriptorPrototype = NSSortDescriptor(key: name, ascending: true)
         }
+    }
 
-        return commentChanged
+    private func accessibilityLabel(name: String, comment: String?) -> String {
+        let label = String(format: String(localized: "Column: %@"), name)
+        guard let comment else { return label }
+        return "\(label), \(comment)"
     }
 
-    private func hasVisibleComments(visibleCount: Int) -> Bool {
-        pooledColumns.enumerated().contains { slot, column in
-            guard slot < visibleCount,
-                  !column.isHidden,
-                  let cell = column.headerCell as? SortableHeaderCell,
-                  let comment = cell.headerComment else {
-                return false
-            }
-            return !comment.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty
-        }
+    private func displayableComment(_ comment: String?) -> String? {
+        guard let comment else { return nil }
+        let trimmed = comment.trimmingCharacters(in: .whitespacesAndNewlines)
+        return trimmed.isEmpty ? nil : trimmed
     }
 
-    private func updateHeaderHeight(in tableView: NSTableView, showsComments: Bool) {
-        (tableView.headerView as? SortableHeaderView)?.showsComments = showsComments
+    private func applyComments(
+        _ comments: [NSUserInterfaceItemIdentifier: String],
+        showsComments: Bool,
+        in tableView: NSTableView
+    ) {
+        guard let headerView = tableView.headerView as? SortableHeaderView else { return }
+        headerView.showsComments = showsComments
+        headerView.updateComments(comments)
     }
 }
diff --git a/TablePro/Views/Results/SortableHeaderCell.swift b/TablePro/Views/Results/SortableHeaderCell.swift
index 3856236a2..2a2de237b 100644
--- a/TablePro/Views/Results/SortableHeaderCell.swift
+++ b/TablePro/Views/Results/SortableHeaderCell.swift
@@ -13,7 +13,6 @@ final class SortableHeaderCell: NSTableHeaderCell {
     var isValueFiltered: Bool = false
     var isFunnelVisible: Bool = false
     var supportsValueFilter: Bool = true
-    var headerComment: String?
 
     private static let indicatorPadding: CGFloat = 4
     private static let indicatorSpacing: CGFloat = 2
@@ -59,7 +58,7 @@ final class SortableHeaderCell: NSTableHeaderCell {
             in: titleRect(forBounds: cellFrame),
             font: titleFont(isSorted: sortDirection != nil),
             color: foreground,
-            comment: visibleComment,
+            comment: visibleComment(in: controlView),
             commentColor: commentColor(emphasized: isColumnSelected)
         )
 
@@ -167,10 +166,9 @@ final class SortableHeaderCell: NSTableHeaderCell {
         return NSFontManager.shared.convert(baseFont, toHaveTrait: .boldFontMask)
     }
 
-    private var visibleComment: String? {
-        guard let headerComment else { return nil }
-        let trimmed = headerComment.trimmingCharacters(in: .whitespacesAndNewlines)
-        return trimmed.isEmpty ? nil : trimmed
+    private func visibleComment(in controlView: NSView?) -> String? {
+        guard let headerView = controlView as? SortableHeaderView else { return nil }
+        return headerView.comment(for: self)
     }
 
     private func foregroundColor(emphasized: Bool) -> NSColor {
@@ -261,9 +259,6 @@ final class SortableHeaderCell: NSTableHeaderCell {
         if isValueFiltered {
             components.append(String(localized: "Filtered"))
         }
-        if let visibleComment {
-            components.append(visibleComment)
-        }
         return components.joined(separator: ", ")
     }
 
diff --git a/TablePro/Views/Results/SortableHeaderView.swift b/TablePro/Views/Results/SortableHeaderView.swift
index a713ad274..eb5e11a97 100644
--- a/TablePro/Views/Results/SortableHeaderView.swift
+++ b/TablePro/Views/Results/SortableHeaderView.swift
@@ -73,6 +73,7 @@ final class SortableHeaderView: NSTableHeaderView {
     private var hoveredColumnIndex: Int?
 
     private let naturalHeight: CGFloat
+    private var commentsByColumn: [NSUserInterfaceItemIdentifier: String] = [:]
 
     var commentHeaderHeight: CGFloat {
         naturalHeight + SortableHeaderCell.commentLineHeight
@@ -85,6 +86,20 @@ final class SortableHeaderView: NSTableHeaderView {
         }
     }
 
+    func updateComments(_ comments: [NSUserInterfaceItemIdentifier: String]) {
+        guard commentsByColumn != comments else { return }
+        commentsByColumn = comments
+        needsDisplay = true
+    }
+
+    func comment(for cell: NSCell) -> String? {
+        guard let tableView,
+              let column = tableView.tableColumns.first(where: { $0.headerCell === cell }) else {
+            return nil
+        }
+        return commentsByColumn[column.identifier]
+    }
+
     override init(frame frameRect: NSRect) {
         naturalHeight = frameRect.height > 0 ? frameRect.height : Self.fallbackHeight
         super.init(frame: frameRect)
diff --git a/TableProTests/Views/Results/DataGridColumnPoolTests.swift b/TableProTests/Views/Results/DataGridColumnPoolTests.swift
index 8d0aef66c..4252b4732 100644
--- a/TableProTests/Views/Results/DataGridColumnPoolTests.swift
+++ b/TableProTests/Views/Results/DataGridColumnPoolTests.swift
@@ -371,10 +371,12 @@ struct DataGridColumnPoolTests {
         }
     }
 
-    @Test("reconcile assigns column comments to sortable header cells")
-    func reconcile_assignsColumnCommentsToHeaderCells() throws {
+    @Test("reconcile publishes column comments to the header view")
+    func reconcile_publishesColumnCommentsToHeaderView() throws {
         let pool = DataGridColumnPool()
         let tableView = makeTableView()
+        let headerView = SortableHeaderView(frame: NSRect(x: 0, y: 0, width: 200, height: 28))
+        tableView.headerView = headerView
         let schema = ColumnIdentitySchema(columns: ["id", "email"])
 
         pool.reconcile(
@@ -390,8 +392,9 @@ struct DataGridColumnPoolTests {
 
         let emailColumn = try #require(dataColumns(in: tableView).first { $0.headerCell.stringValue == "email" })
         let headerCell = try #require(emailColumn.headerCell as? SortableHeaderCell)
-        #expect(headerCell.headerComment == "Primary contact address")
+        #expect(headerView.comment(for: headerCell) == "Primary contact address")
         #expect(emailColumn.headerToolTip == "email (Text)\nPrimary contact address")
+        #expect(headerCell.accessibilityLabel() == "Column: email, Primary contact address")
 
         pool.reconcile(
             tableView: tableView,
@@ -404,8 +407,9 @@ struct DataGridColumnPoolTests {
             widthCalculator: defaultWidthCalculator
         )
 
-        #expect(headerCell.headerComment == nil)
+        #expect(headerView.comment(for: headerCell) == nil)
         #expect(emailColumn.headerToolTip == "email (Text)")
+        #expect(headerCell.accessibilityLabel() == "Column: email")
     }
 
     @Test("reconcile expands and restores header height based on visible comments")
@@ -485,7 +489,7 @@ struct DataGridColumnPoolTests {
 
         let emailColumn = try #require(dataColumns(in: tableView).first { $0.headerCell.stringValue == "email" })
         let headerCell = try #require(emailColumn.headerCell as? SortableHeaderCell)
-        #expect(headerCell.headerComment == "Primary contact address")
+        #expect(headerView.comment(for: headerCell) == "Primary contact address")
         #expect(headerView.frame.height == headerView.commentHeaderHeight)
     }
 
@@ -523,7 +527,7 @@ struct DataGridColumnPoolTests {
 
         let emailColumn = try #require(dataColumns(in: tableView).first { $0.headerCell.stringValue == "email" })
         let headerCell = try #require(emailColumn.headerCell as? SortableHeaderCell)
-        #expect(headerCell.headerComment == "Work address")
+        #expect(headerView.comment(for: headerCell) == "Work address")
         #expect(emailColumn.headerToolTip == "email (Text)\nWork address")
         #expect(headerView.frame.height == heightAfterFirstReconcile)
     }
diff --git a/TableProTests/Views/Results/SortableHeaderCellTests.swift b/TableProTests/Views/Results/SortableHeaderCellTests.swift
index 7b4c28273..b2a9334ce 100644
--- a/TableProTests/Views/Results/SortableHeaderCellTests.swift
+++ b/TableProTests/Views/Results/SortableHeaderCellTests.swift
@@ -59,13 +59,62 @@ struct SortableHeaderCellTests {
         #expect(prioritizedWidth < sortedWidth)
     }
 
-    @Test("Accessibility label includes the visible header comment")
-    func accessibilityLabelIncludesHeaderComment() {
+    @Test("Accessibility label appends the sort and filter state to the column label")
+    func accessibilityLabelAppendsSortAndFilterState() {
         let cell = SortableHeaderCell(textCell: "email")
-        cell.headerComment = "Primary contact address"
-        cell.setAccessibilityLabel("Column: email")
+        cell.setAccessibilityLabel("Column: email, Primary contact address")
+        cell.sortDirection = .ascending
+        cell.isValueFiltered = true
 
-        #expect(cell.accessibilityLabel() == "Column: email, Primary contact address")
+        #expect(cell.accessibilityLabel() == "Column: email, Primary contact address, Sorted ascending, Filtered")
+    }
+
+    @Test("Header comment is resolved from the header view, not stored on the cell")
+    func headerCommentIsResolvedFromHeaderView() {
+        let comment = heapAllocatedComment()
+        let header = makeHeader(comment: comment)
+
+        #expect(header.view.comment(for: header.cell) == comment)
+    }
+
+    @Test("A copied header cell resolves no comment, so the overflow filler draws none")
+    func copiedHeaderCellResolvesNoComment() throws {
+        let header = makeHeader(comment: heapAllocatedComment())
+        let copy = try #require(header.cell.copy() as? SortableHeaderCell)
+
+        #expect(header.view.comment(for: copy) == nil)
+    }
+
+    @Test("Header cell survives the shallow copies AppKit makes while drawing the header")
+    func headerCellSurvivesShallowCopies() {
+        let comment = heapAllocatedComment()
+        let header = makeHeader(comment: comment)
+
+        for _ in 0..<3 {
+            autoreleasepool {
+                _ = header.cell.copy()
+            }
+        }
+
+        #expect(header.view.comment(for: header.cell) == comment)
+    }
+
+    private func heapAllocatedComment() -> String {
+        String(repeating: "Primary contact address", count: 1)
+    }
+
+    private func makeHeader(comment: String) -> (view: SortableHeaderView, cell: SortableHeaderCell) {
+        let tableView = NSTableView()
+        let cell = SortableHeaderCell(textCell: "email")
+        let column = NSTableColumn(identifier: NSUserInterfaceItemIdentifier("email"))
+        column.headerCell = cell
+        tableView.addTableColumn(column)
+
+        let headerView = SortableHeaderView(frame: NSRect(x: 0, y: 0, width: 200, height: 28))
+        tableView.headerView = headerView
+        headerView.updateComments([column.identifier: comment])
+
+        return (headerView, cell)
     }
 }