Skip to content

Commit 76cb8e6

Browse files
leonardocardosoleonardocardoso
committed
Fix crash, use full-screen navigation, and add screenshot test
- Replace O(m*n) LCS diff with CollectionDifference (Myers) to fix SIGTERM on large files - Move file I/O off main thread to prevent beach ball - Change from sheet to full-screen pushed view for better usability - Add File History screenshot test and showcase image
1 parent 6b3a7cd commit 76cb8e6

10 files changed

Lines changed: 437 additions & 69 deletions

File tree

‎Poirot.xcodeproj/project.pbxproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@
162162
BC707E1DB76EFC4B9A387DF5 /* MCPServerStatusCheckerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 72818A517BE04CA1CA1A39A0 /* MCPServerStatusCheckerTests.swift */; };
163163
BF54E120AAC32B833CE2C26D /* OutputStylesListView.swift in Sources */ = {isa = PBXBuildFile; fileRef = B8C238E86ADA2AFDEE195CA8 /* OutputStylesListView.swift */; };
164164
C029653D52153190DD47D3A6 /* ScreenshotTests_ConfigScreens.swift in Sources */ = {isa = PBXBuildFile; fileRef = C703191A35D40C2CFD905A48 /* ScreenshotTests_ConfigScreens.swift */; };
165+
C0A93DD9CC51DF2737C96EC9 /* ScreenshotTests_FileHistory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 32AFC13983EBE2608C7EAF8B /* ScreenshotTests_FileHistory.swift */; };
165166
C1719FB80745ABC561A4A6E8 /* MarqueeText.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0582B51CC0F05277E8134681 /* MarqueeText.swift */; };
166167
C3EB0DF6E512C0B1762C70EE /* SidebarView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4DA932278008D948F5DF449C /* SidebarView.swift */; };
167168
C4A26E38C665512B50DDD2B5 /* Project.swift in Sources */ = {isa = PBXBuildFile; fileRef = 86D2EE5CD7194B8CC20DD119 /* Project.swift */; };
@@ -269,6 +270,7 @@
269270
2DD7203015405A5BB4546AD7 /* ProviderDescribing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProviderDescribing.swift; sourceTree = "<group>"; };
270271
2E27598FA5317EBA2F3566DD /* StatCard.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatCard.swift; sourceTree = "<group>"; };
271272
2ED8108B09E1C0D619F8D98C /* PreferredTerminal.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PreferredTerminal.swift; sourceTree = "<group>"; };
273+
32AFC13983EBE2608C7EAF8B /* ScreenshotTests_FileHistory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ScreenshotTests_FileHistory.swift; sourceTree = "<group>"; };
272274
3498814047BCC0D3779C7559 /* SessionLoaderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionLoaderTests.swift; sourceTree = "<group>"; };
273275
35B31FDA865C64004095C39C /* UpdateCheckerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpdateCheckerTests.swift; sourceTree = "<group>"; };
274276
377822548107CA8150432777 /* FrontmatterParser.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FrontmatterParser.swift; sourceTree = "<group>"; };
@@ -731,6 +733,7 @@
731733
924D1F914ACBAAC3056FE513 /* ScreenshotTests_DebugLog.swift */,
732734
26B9CD4BBB58483858519DD3 /* ScreenshotTests_EditDiff.swift */,
733735
DD4F57D1DA2A3D3742A92544 /* ScreenshotTests_Facets.swift */,
736+
32AFC13983EBE2608C7EAF8B /* ScreenshotTests_FileHistory.swift */,
734737
03EE4E7DD33073A345CDBD22 /* ScreenshotTests_FullViews.swift */,
735738
3F8DCF6F5F9B1A533DC502B6 /* ScreenshotTests_History.swift */,
736739
FBA2F6AD1DF80D4A3736BCCE /* ScreenshotTests_Memory.swift */,
@@ -1312,6 +1315,7 @@
13121315
E8B4B7B22F20F214C172BB65 /* ScreenshotTests_DebugLog.swift in Sources */,
13131316
14B9725EFD095A18BEED46A8 /* ScreenshotTests_EditDiff.swift in Sources */,
13141317
2F0FA328AEAE94264FE62AEA /* ScreenshotTests_Facets.swift in Sources */,
1318+
C0A93DD9CC51DF2737C96EC9 /* ScreenshotTests_FileHistory.swift in Sources */,
13151319
5F6D0972E1F839E780FCFCF5 /* ScreenshotTests_FullViews.swift in Sources */,
13161320
CB3AC04050A0C0BB836B2F81 /* ScreenshotTests_History.swift in Sources */,
13171321
60772D853F1D0426A70A664A /* ScreenshotTests_Memory.swift in Sources */,

‎Poirot/Resources/Localizable.xcstrings‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,18 @@
9393
}
9494
}
9595
},
96+
"%lld %@ changed" : {
97+
"comment" : "A description of how many files have been changed in the current session. The number is dynamically updated as files are selected.",
98+
"isCommentAutoGenerated" : true,
99+
"localizations" : {
100+
"en" : {
101+
"stringUnit" : {
102+
"state" : "new",
103+
"value" : "%1$lld %2$@ changed"
104+
}
105+
}
106+
}
107+
},
96108
"%lld entries" : {
97109
"comment" : "A text label showing the total number of entries in the debug log. The placeholder is replaced by the actual count of entries.",
98110
"isCommentAutoGenerated" : true
@@ -670,6 +682,14 @@
670682
},
671683
"File" : {
672684

685+
},
686+
"File History" : {
687+
"comment" : "The title of the view.",
688+
"isCommentAutoGenerated" : true
689+
},
690+
"File History (%lld files)" : {
691+
"comment" : "A button that presents a sheet showing a list of previous sessions.",
692+
"isCommentAutoGenerated" : true
673693
},
674694
"Filter by Tool (⌘T)" : {
675695
"comment" : "A help string for a button that filters sessions by tool.",
@@ -810,6 +830,10 @@
810830
"comment" : "The name of the person behind the app.",
811831
"isCommentAutoGenerated" : true
812832
},
833+
"Loading file history…" : {
834+
"comment" : "A message displayed while loading file history.",
835+
"isCommentAutoGenerated" : true
836+
},
813837
"Loading more…" : {
814838
"comment" : "A text indicating that more content is being loaded.",
815839
"isCommentAutoGenerated" : true
@@ -869,6 +893,10 @@
869893
"comment" : "A message displayed when there are no debug log entries available.",
870894
"isCommentAutoGenerated" : true
871895
},
896+
"No File History" : {
897+
"comment" : "A label displayed when a user has no file history.",
898+
"isCommentAutoGenerated" : true
899+
},
872900
"No matching entries" : {
873901
"comment" : "A message displayed when there are no debug log entries that match the current search query or filters.",
874902
"isCommentAutoGenerated" : true
@@ -889,6 +917,10 @@
889917
"comment" : "A message displayed when a search query returns no results. The argument is the string “”.",
890918
"isCommentAutoGenerated" : true
891919
},
920+
"No versioned file snapshots were found for this session." : {
921+
"comment" : "A description displayed when a user has no file history entries.",
922+
"isCommentAutoGenerated" : true
923+
},
892924
"Not Helpful" : {
893925
"comment" : "Human-readable label for a \"not helpful\" response from the AI.",
894926
"isCommentAutoGenerated" : true
@@ -1067,6 +1099,10 @@
10671099
"comment" : "A placeholder text for a search bar in the search overlay.",
10681100
"isCommentAutoGenerated" : true
10691101
},
1102+
"Select a file to view changes" : {
1103+
"comment" : "A description displayed when no file is selected in the file history view.",
1104+
"isCommentAutoGenerated" : true
1105+
},
10701106
"Selected" : {
10711107
"comment" : "Label for the selected date in the daily activity chart.",
10721108
"isCommentAutoGenerated" : true
@@ -1314,6 +1350,10 @@
13141350
},
13151351
"URL" : {
13161352

1353+
},
1354+
"v%lld" : {
1355+
"comment" : "A version number displayed in a button within the file history view. The number is prefixed with an \"v\" and followed by a space.",
1356+
"isCommentAutoGenerated" : true
13171357
},
13181358
"Version %@ (%@)" : {
13191359
"comment" : "The version and build number of the app, displayed in a smaller font.",

‎Poirot/Sources/App/AppState.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ final class AppState {
9191

9292
var selectedSession: Session? {
9393
didSet {
94+
isShowingFileHistory = false
9495
guard !isNavigatingHistory else { return }
9596
// Push to history when user navigates to a new session
9697
if let session = selectedSession {
@@ -189,6 +190,7 @@ final class AppState {
189190
var sidebarKeyboardIndex: Int = -1
190191
var sessionSearchQuery: String = ""
191192
var isSessionSearchActive: Bool = false
193+
var isShowingFileHistory: Bool = false
192194
var isToolFilterActive: Bool = false
193195
var activeToolFilters: Set<String> = []
194196
var projects: [Project] = []

‎Poirot/Sources/App/ContentView.swift‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,10 @@ struct ContentView: View {
333333
switch appState.selectedNav.id {
334334
case NavigationItem.sessions.id:
335335
if let session = appState.selectedSession {
336-
if appState.isLoadingSession || (session.messages.isEmpty && session.fileURL != nil) {
336+
if appState.isShowingFileHistory {
337+
FileHistoryView(session: session)
338+
.transition(.opacity)
339+
} else if appState.isLoadingSession || (session.messages.isEmpty && session.fileURL != nil) {
337340
SessionSkeletonView()
338341
.transition(.opacity)
339342
} else {

‎Poirot/Sources/Utilities/LineDiff.swift‎

Lines changed: 41 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -22,55 +22,57 @@ nonisolated enum LineDiff {
2222
}
2323

2424
static func diff(oldLines: [String], newLines: [String]) -> [DiffLine] {
25-
let m = oldLines.count
26-
let n = newLines.count
25+
let changes = newLines.difference(from: oldLines)
2726

28-
// LCS table
29-
var dp = [[Int]](repeating: [Int](repeating: 0, count: n + 1), count: m + 1)
30-
for i in 1 ... max(m, 1) where i <= m {
31-
for j in 1 ... max(n, 1) where j <= n {
32-
if oldLines[i - 1] == newLines[j - 1] {
33-
dp[i][j] = dp[i - 1][j - 1] + 1
34-
} else {
35-
dp[i][j] = max(dp[i - 1][j], dp[i][j - 1])
36-
}
27+
// Build lookup of removals and insertions by offset
28+
var removals: [Int: String] = [:]
29+
var insertions: [Int: String] = [:]
30+
for change in changes {
31+
switch change {
32+
case let .remove(offset, element, _):
33+
removals[offset] = element
34+
case let .insert(offset, element, _):
35+
insertions[offset] = element
3736
}
3837
}
3938

40-
// Backtrack to produce diff
4139
var result: [DiffLine] = []
42-
var lineId = 0
43-
var i = m
44-
var j = n
40+
var oldIdx = 0
41+
var newIdx = 0
4542

46-
func append(kind: DiffLine.Kind, text: String, oldNum: Int?, newNum: Int?) {
47-
result.append(DiffLine(id: lineId, kind: kind, text: text, oldLineNumber: oldNum, newLineNumber: newNum))
48-
lineId += 1
49-
}
50-
51-
while i > 0 || j > 0 {
52-
if i > 0, j > 0, oldLines[i - 1] == newLines[j - 1] {
53-
append(kind: .context, text: oldLines[i - 1], oldNum: i, newNum: j)
54-
i -= 1
55-
j -= 1
56-
} else if j > 0, i == 0 || dp[i][j - 1] >= dp[i - 1][j] {
57-
append(kind: .added, text: newLines[j - 1], oldNum: nil, newNum: j)
58-
j -= 1
43+
while oldIdx < oldLines.count || newIdx < newLines.count {
44+
if let text = removals[oldIdx] {
45+
result.append(DiffLine(
46+
id: result.count,
47+
kind: .removed,
48+
text: text,
49+
oldLineNumber: oldIdx + 1,
50+
newLineNumber: nil
51+
))
52+
oldIdx += 1
53+
} else if let text = insertions[newIdx] {
54+
result.append(DiffLine(
55+
id: result.count,
56+
kind: .added,
57+
text: text,
58+
oldLineNumber: nil,
59+
newLineNumber: newIdx + 1
60+
))
61+
newIdx += 1
5962
} else {
60-
append(kind: .removed, text: oldLines[i - 1], oldNum: i, newNum: nil)
61-
i -= 1
63+
result.append(DiffLine(
64+
id: result.count,
65+
kind: .context,
66+
text: newLines[newIdx],
67+
oldLineNumber: oldIdx + 1,
68+
newLineNumber: newIdx + 1
69+
))
70+
oldIdx += 1
71+
newIdx += 1
6272
}
6373
}
6474

65-
return result.reversed().enumerated().map { index, line in
66-
DiffLine(
67-
id: index,
68-
kind: line.kind,
69-
text: line.text,
70-
oldLineNumber: line.oldLineNumber,
71-
newLineNumber: line.newLineNumber
72-
)
73-
}
75+
return result
7476
}
7577

7678
static func unifiedText(from lines: [DiffLine]) -> String {

‎Poirot/Sources/Views/Session/FileHistoryView.swift‎

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ struct FileHistoryView: View {
2424
@Environment(\.fileHistoryLoader)
2525
private var fileHistoryLoader
2626

27-
@Environment(\.dismiss)
28-
private var dismiss
27+
@Environment(AppState.self)
28+
private var appState
2929

3030
var body: some View {
3131
VStack(spacing: 0) {
@@ -44,10 +44,9 @@ struct FileHistoryView: View {
4444
}
4545
}
4646
}
47-
.frame(minWidth: 700, minHeight: 500)
4847
.background(PoirotTheme.Colors.bgApp)
4948
.task {
50-
loadHistory()
49+
await loadHistory()
5150
}
5251
}
5352

@@ -79,7 +78,7 @@ struct FileHistoryView: View {
7978
Spacer()
8079

8180
Button {
82-
dismiss()
81+
appState.isShowingFileHistory = false
8382
} label: {
8483
Image(systemName: "xmark.circle.fill")
8584
.font(PoirotTheme.Typography.large)
@@ -184,7 +183,7 @@ struct FileHistoryView: View {
184183
|| (selectedVersionIndex == nil && index == entry.versions.count - 1)
185184
Button {
186185
selectedVersionIndex = index
187-
loadVersionContent(entry: entry, versionIndex: index)
186+
Task { await loadVersionContent(entry: entry, versionIndex: index) }
188187
} label: {
189188
HStack(spacing: PoirotTheme.Spacing.xs) {
190189
Image(systemName: isSelected ? "circle.fill" : "circle")
@@ -243,7 +242,7 @@ struct FileHistoryView: View {
243242
ProgressView()
244243
.frame(maxWidth: .infinity, maxHeight: .infinity)
245244
.task(id: currentKey) {
246-
loadVersionContent(entry: entry, versionIndex: effectiveIndex)
245+
await loadVersionContent(entry: entry, versionIndex: effectiveIndex)
247246
}
248247
} else {
249248
ScrollView {
@@ -261,7 +260,7 @@ struct FileHistoryView: View {
261260
ProgressView()
262261
.frame(maxWidth: .infinity, maxHeight: .infinity)
263262
.task(id: currentKey) {
264-
loadVersionContent(entry: entry, versionIndex: effectiveIndex)
263+
await loadVersionContent(entry: entry, versionIndex: effectiveIndex)
265264
}
266265
} else {
267266
ScrollView {
@@ -325,42 +324,47 @@ struct FileHistoryView: View {
325324

326325
// MARK: - Data Loading
327326

328-
private func loadHistory() {
327+
private func loadHistory() async {
329328
let loader = fileHistoryLoader
330329
let sessionId = session.id
331330
let projectPath = session.projectPath
332-
entries = loader.loadFileHistory(for: sessionId, projectPath: projectPath)
331+
let loaded = await Task.detached {
332+
loader.loadFileHistory(for: sessionId, projectPath: projectPath)
333+
}.value
334+
entries = loaded
333335
isLoading = false
334336
iconBounce += 1
335337

336338
// Auto-select first file
337339
if let first = entries.first {
338340
selectedFile = first
339-
loadVersionContent(entry: first, versionIndex: first.versions.count - 1)
341+
await loadVersionContent(entry: first, versionIndex: first.versions.count - 1)
340342
}
341343
}
342344

343-
private func loadVersionContent(entry: FileHistoryEntry, versionIndex: Int) {
345+
private func loadVersionContent(entry: FileHistoryEntry, versionIndex: Int) async {
344346
let loader = fileHistoryLoader
345347
let sessionId = session.id
346348

347349
// Load the selected version
348350
let version = entry.versions[versionIndex]
349351
let key = "\(sessionId)/\(version.backupFileName)"
350352
if fileContents[key] == nil {
351-
fileContents[key] = loader.loadFileContent(
352-
for: sessionId, backupFileName: version.backupFileName
353-
) ?? ""
353+
let content = await Task.detached {
354+
loader.loadFileContent(for: sessionId, backupFileName: version.backupFileName)
355+
}.value
356+
fileContents[key] = content ?? ""
354357
}
355358

356359
// Also load previous version for diff
357360
if versionIndex > 0 {
358361
let prev = entry.versions[versionIndex - 1]
359362
let prevKey = "\(sessionId)/\(prev.backupFileName)"
360363
if fileContents[prevKey] == nil {
361-
fileContents[prevKey] = loader.loadFileContent(
362-
for: sessionId, backupFileName: prev.backupFileName
363-
) ?? ""
364+
let content = await Task.detached {
365+
loader.loadFileContent(for: sessionId, backupFileName: prev.backupFileName)
366+
}.value
367+
fileContents[prevKey] = content ?? ""
364368
}
365369
}
366370
}
@@ -388,7 +392,7 @@ struct FileHistoryView: View {
388392
return components.dropLast().joined(separator: "/")
389393
}
390394

391-
nonisolated(unsafe) private static let timeFormatter: DateFormatter = {
395+
private static let timeFormatter: DateFormatter = {
392396
let fmt = DateFormatter()
393397
fmt.dateFormat = "HH:mm:ss"
394398
return fmt

0 commit comments

Comments
 (0)