feat(macos): Add tree-sitter syntax highlighting via Neon#149
Merged
2witstudios merged 12 commits intomainfrom Mar 16, 2026
Merged
feat(macos): Add tree-sitter syntax highlighting via Neon#1492witstudios merged 12 commits intomainfrom
2witstudios merged 12 commits intomainfrom
Conversation
Add syntax highlighting for Swift, Rust, JavaScript, TypeScript, Python, JSON, and Bash using tree-sitter grammars and the Neon highlighting framework. - Pin Neon to main branch for Swift 6.2 compatibility - Pin tree-sitter-swift to with-generated-files tag - Pin tree-sitter-javascript v0.23.1 and tree-sitter-python v0.23.6 (last versions with hardcoded scanner.c sources for SPM) - Add TreeSitterTypeScript and TreeSitterPython package dependencies - Update SyntaxHighlightManager for Neon main API (Token struct, locationTransformer, actor isolation) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds many Tree‑sitter and Neon Swift packages, implements a TreeSitter/Neon-backed SyntaxHighlightManager with language/query merging and token→attribute mapping, wires language-aware highlighting into the macOS editor, simplifies tab state to a single currentFile, and restructures editor/worktree/project UI around currentFile and breadcrumbs. Changes
Sequence Diagram(s)sequenceDiagram
participant Worktree as WorktreeDetailView
participant Editor as EditorNSView
participant Highlighter as SyntaxHighlightManager
participant Parser as TreeSitterParser
participant AttrProv as TokenAttributeProvider
Worktree->>Editor: openFile(path) / pass language
Editor->>Highlighter: setLanguage(language)
Highlighter->>Highlighter: resolve LanguageConfiguration
Highlighter->>Parser: load parser + queries
Editor->>Highlighter: content changed -> request highlight
Highlighter->>Parser: parse(content)
Parser-->>Highlighter: nodes/tokens
Highlighter->>AttrProv: map tokens -> attributes
AttrProv-->>Highlighter: attributes
Highlighter->>Editor: apply attributes to NSTextView
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Cache monospaced fonts as static constants to avoid per-token allocation, and remove locationTransformer that returned nil (disabling Neon's position mapping). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/purepoint-macos/purepoint-macos/Views/Editor/EditorNSView.swift (1)
119-127:⚠️ Potential issue | 🟠 MajorRefresh highlighting after programmatic tab swaps.
updateNSViewrebuilds language state before replacing the buffer, butSyntaxHighlightManager.rebuildHighlighter()makes its >1MB decision from the currenttextViewcontents. If this reused view is still holding a previous large document, the rebuild bails out against the old buffer and never retries aftersetContent, so the next smaller file opens with no highlighting.🐛 Proposed fix
func updateNSView(_ nsView: EditorNSView, context: Context) { nsView.onContentChanged = onContentChanged nsView.onSave = onSave nsView.setEditable(isEditable && !isBinary) - nsView.setLanguage(language) // Only update content if it differs (avoid resetting cursor) if !isBinary && nsView.getContent() != content { nsView.setContent(content) + nsView.invalidateHighlighting() } + + nsView.setLanguage(language) }Add a small forwarder on
EditorNSView:func invalidateHighlighting() { highlightManager?.invalidate() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Views/Editor/EditorNSView.swift` around lines 119 - 127, The current updateNSView flow can leave SyntaxHighlightManager.rebuildHighlighter() using the old large buffer and skipping rebuilds when you later call setContent; add a small forwarder on EditorNSView (e.g., func invalidateHighlighting() that calls highlightManager?.invalidate()) and invoke this from updateNSView immediately after setContent (or right after replacing the buffer) so the highlight manager is forced to re-evaluate the new text and rebuild highlighting for the new document.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift`:
- Around line 42-45: The current check uses NSTextStorage.length (UTF-16 code
units) which doesn't reflect file byte size; change the large-file guard in the
block that references textView.textStorage and Self.maxHighlightSize to measure
the byte count of the text (e.g., compute storage.string.lengthOfBytes(using:
.utf8) or Data(storage.string.utf8).count) and compare that to
Self.maxHighlightSize, and keep the call to clearTemporaryAttributes() and early
return when the byte-size check exceeds the limit; also handle the unlikely
failure by treating a failed conversion as exceeding the limit to preserve the
guard.
---
Outside diff comments:
In `@apps/purepoint-macos/purepoint-macos/Views/Editor/EditorNSView.swift`:
- Around line 119-127: The current updateNSView flow can leave
SyntaxHighlightManager.rebuildHighlighter() using the old large buffer and
skipping rebuilds when you later call setContent; add a small forwarder on
EditorNSView (e.g., func invalidateHighlighting() that calls
highlightManager?.invalidate()) and invoke this from updateNSView immediately
after setContent (or right after replacing the buffer) so the highlight manager
is forced to re-evaluate the new text and rebuild highlighting for the new
document.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69ad2066-efb8-4d96-9f31-671c21eaa87d
📒 Files selected for processing (5)
apps/purepoint-macos/purepoint-macos.xcodeproj/project.pbxprojapps/purepoint-macos/purepoint-macos.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedapps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swiftapps/purepoint-macos/purepoint-macos/Views/Detail/WorktreeDetailView.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/EditorNSView.swift
apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift
Outdated
Show resolved
Hide resolved
apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift (1)
33-35: Reuse the highlighter's own invalidation path instead of rebuilding on appearance changes.
TextViewHighlighterexposes aninvalidate(_:)method for incremental updates. When the appearance changes, destroying and rebuilding the entire highlighter is unnecessary; instead, forward the invalidation to the existing instance and only rebuild if none exists yet.func invalidate() { - rebuildHighlighter() + if let highlighter { + highlighter.invalidate(.all) + } else { + rebuildHighlighter() + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift` around lines 33 - 35, The current invalidate() implementation calls rebuildHighlighter() unconditionally; change it to forward appearance changes to the existing highlighter by calling TextViewHighlighter's invalidate(_:) on the stored instance (e.g., the property holding the highlighter) and only call rebuildHighlighter() if that instance is nil. Specifically, update invalidate() to check the highlighter property, call its invalidate(appearance) (or appropriate parameter) for incremental updates, and fall back to rebuildHighlighter() when no highlighter exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift`:
- Around line 55-58: The Configuration initializer for TextViewHighlighter is
missing the required locationTransformer parameter; update the initialization
where TextViewHighlighter.Configuration(languageConfiguration: langConfig,
attributeProvider: Self.attributeProvider) is created to include a
locationTransformer argument. Either pass the existing transformer (e.g.
Self.locationTransformer) if one exists, or implement and pass a
closure/function that matches the required locationTransformer signature to
map/highlight locations, ensuring you reference
TextViewHighlighter.Configuration, langConfig, and Self.attributeProvider when
making the change.
---
Nitpick comments:
In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift`:
- Around line 33-35: The current invalidate() implementation calls
rebuildHighlighter() unconditionally; change it to forward appearance changes to
the existing highlighter by calling TextViewHighlighter's invalidate(_:) on the
stored instance (e.g., the property holding the highlighter) and only call
rebuildHighlighter() if that instance is nil. Specifically, update invalidate()
to check the highlighter property, call its invalidate(appearance) (or
appropriate parameter) for incremental updates, and fall back to
rebuildHighlighter() when no highlighter exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c519fda6-579a-488d-806d-2f22464c6373
📒 Files selected for processing (1)
apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift
apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift
Show resolved
Hide resolved
Add SPM packages and language configurations for TSX, C++, Go, Ruby, PHP, C#, Scala, Haskell, Lua, XML, CSS, YAML, and TOML. Add TSX as a distinct language case with file detection. Refactor GitService gh path resolution. Exclude LocalPackages/ from version control. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The LineNumberRulerView was counting lines from position 0 on every draw call — O(n) for line n at ~60Hz. For large files scrolled deep, this caused line numbers to vanish entirely. Fix: cache a lineStarts array (rebuilt only on text changes), then binary search for the first visible line — O(log n) per frame instead of O(n). Also call rebuildLineStarts() from EditorNSView.setContent() when file content is replaced programmatically. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace multi-tab EditorState (openTabs, activeTabId, closeTab, setActiveTab) with a single currentFile property. The editor now shows whichever file is selected in the tree, replacing the previous file — no tab bar, no tab switching logic, no multiple files in memory. - EditorState: currentFile replaces openTabs/activeTabId, simplified file watching to track a single directory - ProjectDetailView & WorktreeDetailView: replace EditorTabBar with a file breadcrumb bar (icon + name + dirty indicator), add Changes toggle button to the header - Delete EditorTabBar.swift Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two highlighting fixes: 1. Flash on file switch: clearTemporaryAttributes() was called unconditionally at the top of rebuildHighlighter(), removing all colors before the new highlighter was ready (1-frame flash of unstyled text). Now only clears when transitioning to an unhighlighted state (plaintext, too-large, error). When switching between highlighted languages, the new highlighter overwrites the old — no gap. 2. Markdown inline: tree-sitter-markdown uses a split parser where block-level handles headings/code blocks but delegates all inline content (bold, italic, links, code spans) via injection to markdown_inline. Add the injection handler and italic font support so **bold**, *italic*, `code`, and [links](url) all highlight. Also adds yaml/toml injection mappings for markdown frontmatter. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/purepoint-macos/purepoint-macos/State/EditorState.swift (1)
55-67:⚠️ Potential issue | 🟡 MinorAlert dismissed before reload completes; errors silently swallowed.
externalChangeAlert = nilis set synchronously at line 66, but the actual file read happens asynchronously in the Task. If the read fails, the alert is already dismissed and the user sees no indication of failure.🔧 Suggested fix
func reloadFile() { guard let file = currentFile else { return } let path = file.id Task { do { let result = try await FileIOService.readFile(at: path) currentFile?.content = result.content currentFile?.isDirty = false currentFile?.lastModified = FileIOService.fileModificationDate(at: path) - } catch {} + externalChangeAlert = nil + } catch { + // Keep alert visible on failure so user can retry + } } - externalChangeAlert = nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/State/EditorState.swift` around lines 55 - 67, In reloadFile(), the asynchronous Task swallows errors and externalChangeAlert is cleared immediately; change the flow so the alert is only cleared after a successful read and handle/report errors: move the externalChangeAlert = nil assignment to run after the await completes (inside the Task), catch and surface the error (e.g. set a state like externalChangeAlert to an error variant or call a UI error handler) instead of an empty catch, and/or log the error via FileIOService read path so that functions reloadFile(), currentFile?.content, currentFile?.isDirty, and currentFile?.lastModified are only updated on success and the alert remains visible on failure.
♻️ Duplicate comments (1)
apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift (1)
66-69:⚠️ Potential issue | 🟡 MinorUTF-16 length check still doesn't reflect file byte size.
This was flagged in a previous review.
storage.lengthreturns UTF-16 code units, not file bytes. A file with many non-ASCII characters could exceed 1MB on disk while passing this check, potentially causing performance issues.🔧 Suggested fix
- if let storage = textView.textStorage, storage.length > Self.maxHighlightSize { + if let storage = textView.textStorage, storage.string.utf8.count > Self.maxHighlightSize { clearTemporaryAttributes() return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift` around lines 66 - 69, The UTF-16 code unit check using storage.length is incorrect for file byte-size limits; replace it with a byte-size check using the text content encoded as UTF-8. In the SyntaxHighlightManager method where you currently do `if let storage = textView.textStorage, storage.length > Self.maxHighlightSize { ... }`, compute the bytes like `let byteCount = textView.string.data(using: .utf8)?.count ?? 0` (or use NSString.byteLength(using: .utf8)) and compare `byteCount > Self.maxHighlightSize`, then call `clearTemporaryAttributes()` and return; ensure Self.maxHighlightSize is interpreted as bytes and handle the optional data safely.
🧹 Nitpick comments (5)
apps/purepoint-macos/purepoint-macos/Services/GitService.swift (1)
289-290: Prefer failable UTF-8 decode for command output parsing.At
Line 289, usingString(bytes: data, encoding: .utf8)avoids silent lossy decoding and matches the SwiftLint rule already reported.Proposed refactor
- let path = String(decoding: data, as: UTF8.self) - .trimmingCharacters(in: .whitespacesAndNewlines) + guard let decoded = String(bytes: data, encoding: .utf8) else { + return nil + } + let path = decoded.trimmingCharacters(in: .whitespacesAndNewlines)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Services/GitService.swift` around lines 289 - 290, Replace the silent lossy decoding call String(decoding: data, as: UTF8.self) with the failable initializer String(bytes: data, encoding: .utf8) and handle the optional result where `path` is used (e.g., in the surrounding GitService method that produces `path`); if decoding fails return/throw/log an appropriate error or skip processing so malformed output isn't treated as a valid path. Ensure you update references to the previous `path` variable to unwrap the optional safely and provide a clear error path.apps/purepoint-macos/purepoint-macos.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved (1)
4-21: Branch-based dependency pins may affect build reproducibility.
neon,swifttreesitter, and most tree-sitter language packages are pinned to branches (main/master) rather than specific versions. While this is common for tree-sitter grammars that lack stable releases, it means builds may break if upstream introduces breaking changes.Consider pinning to specific revisions or versions where available for critical dependencies like
neonandswifttreesitter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved` around lines 4 - 21, Package pins for dependencies like "neon" (currently branch "main"), "swifttreesitter" and tree-sitter language packages are using branch-based pins which harms reproducibility; update Package.resolved (and your Package.swift/Package.resolved workflow) to pin these to specific revisions or tagged versions instead of branches—replace branch entries for identities such as "neon", "swifttreesitter" and any tree-sitter language packages with their exact "revision" or "version" values (commit SHAs or semver tags) so CI/builds use an immutable, reproducible dependency set..gitignore (1)
52-53: Consider documenting LocalPackages setup.The
LocalPackages/directory is ignored but referenced byXCLocalSwiftPackageReferenceentries in the Xcode project. New developers cloning the repo won't know how to populate these local tree-sitter packages (yaml, lua, css, python, javascript).Consider adding a README or setup script explaining how to clone/symlink these packages.
Would you like me to draft a setup script or documentation for the LocalPackages directory?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 52 - 53, Add documentation and/or a setup script explaining how to populate apps/purepoint-macos/LocalPackages referenced by XCLocalSwiftPackageReference entries in the Xcode project; specifically create a README (e.g., LocalPackages/README) or a bootstrap script that lists required tree-sitter packages (yaml, lua, css, python, javascript), shows the exact git clone or symlink commands to populate LocalPackages/, and documents how to re-sync or update those local package references so new developers can reproduce the project build environment.apps/purepoint-macos/purepoint-macos/State/EditorState.swift (1)
103-122: Silent error handling incheckForExternalChanges.The empty
catch {}block at line 119 silently discards file read errors. While this may be intentional (file might be temporarily locked), consider logging or handling specific error cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/State/EditorState.swift` around lines 103 - 122, The empty catch in checkForExternalChanges silently discards errors from FileIOService.readFile(at:), so update the Task's error handling to capture the thrown error and surface it (e.g., log it via your app logger or os_log, and/or set a recoverable UI state such as externalChangeAlert or a new fileReadError property) instead of swallowing it; specifically, modify the Task inside checkForExternalChanges to use catch { let err in Logger.shared.error("Failed to read file \(filePath): \(err)") /* or update externalChangeAlert/currentFile state to indicate read failure */ } so failures from FileIOService.readFile, and the filePath/file.id, are recorded and lastModified is not left inconsistent.apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift (1)
83-92: locationTransformer has O(n) complexity per invocation.The transformer splits the entire substring up to
offseton every call (components(separatedBy: "\n")). For large files with many tokens, this could be expensive. Consider caching line start offsets (similar to the line ruler optimization mentioned in the commit messages).However, this is acceptable for initial implementation given Neon likely batches these calls.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift` around lines 83 - 92, The locationTransformer closure is O(n) per call because it builds and splits the substring up to offset each time; change it to use cached line start offsets instead of components(separatedBy:), e.g., maintain a reusable array of lineStartOffsets updated when textStorage changes and compute row/column by binary-searching that array (referencing locationTransformer, textView/textStorage, and Point) so you avoid scanning the whole prefix on every invocation; ensure the cache is invalidated/updated on edits to the text storage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/purepoint-macos/purepoint-macos.xcodeproj/project.pbxproj`:
- Around line 726-746: The repo includes XCLocalSwiftPackageReference entries
for local TreeSitter packages (tree-sitter-yaml, tree-sitter-lua,
tree-sitter-css, tree-sitter-python, tree-sitter-javascript) that must live in
LocalPackages/ (which is gitignored); update the contributing guide to add a
"Local TreeSitter packages" setup section that lists those package names,
instructs developers to either clone the corresponding tree-sitter language
repos into LocalPackages/ or build them from source (brief commands: clone the
upstream repo, run the language's build/generate steps), show the exact
destination directory name (LocalPackages/<package-name>), note LocalPackages/
is gitignored, and add a verification step (run the Xcode build or swift package
resolve) to confirm the project builds.
In `@apps/purepoint-macos/purepoint-macos/Models/EditorTab.swift`:
- Around line 103-105: Update the file-extension-to-language mapping so
`.gitconfig` and `.gitmodules` are mapped to INI-style parsing instead of TOML:
locate the dictionary entries containing the keys ".editorconfig", ".gitconfig",
".gitmodules" (in EditorTab.swift) and change the values for ".gitconfig" and
".gitmodules" from `.toml` to the appropriate `.ini`/INI language enum used by
your app so they get INI syntax highlighting.
In `@apps/purepoint-macos/purepoint-macos/Services/GitService.swift`:
- Around line 285-287: The code calls try? process.run() and then
process.waitUntilExit(), which can crash if the process failed to start; update
the caller (the function in GitService that launches the Process) to check the
result of starting the process and return early on failure instead of calling
waitUntilExit(): replace the try? usage with a do/catch or a guard that attempts
try process.run() (or checks a non-nil result) and on catch/failure log/return,
then only call process.waitUntilExit() and inspect process.terminationStatus
when the run succeeded; reference process.run(), process.waitUntilExit(), and
process.terminationStatus when making the change.
In `@apps/purepoint-macos/purepoint-macos/Views/Detail/WorktreeDetailView.swift`:
- Around line 150-158: The save/reload race occurs because saveFile() and
reloadFile() capture the file identity at start but after awaits update mutable
currentFile via optional chaining; fix by consistently using the originally
captured reference (e.g., the local file or path captured at method entry) for
all post-await state updates (isDirty, lastModified, content) or explicitly
verify currentFile still equals the captured file before applying changes;
update saveFile(), reloadFile(), and any callers that use currentFile after
awaits to reference the captured file variable (or bail/skip updates if
currentFile has changed).
In `@apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift`:
- Around line 86-91: The early return when glyphRange.length == 0 prevents
showing line 1 for empty files; change the logic in the update/paint routine
that calls layoutManager.glyphRange(forBoundingRect:in:) and
lineIndex(forCharacter:) so that when glyphRange.length == 0 you still treat the
visible character index as 0 (e.g., compute a visibleCharIndex =
(glyphRange.length > 0) ? layoutManager.characterRange(forGlyphRange:
glyphRange, actualGlyphRange: nil).location : 0) and call
lineIndex(forCharacter: visibleCharIndex) to render line "1" in the gutter; keep
the normal path for non-empty files so existing layoutManager and textContainer
usage remains unchanged.
---
Outside diff comments:
In `@apps/purepoint-macos/purepoint-macos/State/EditorState.swift`:
- Around line 55-67: In reloadFile(), the asynchronous Task swallows errors and
externalChangeAlert is cleared immediately; change the flow so the alert is only
cleared after a successful read and handle/report errors: move the
externalChangeAlert = nil assignment to run after the await completes (inside
the Task), catch and surface the error (e.g. set a state like
externalChangeAlert to an error variant or call a UI error handler) instead of
an empty catch, and/or log the error via FileIOService read path so that
functions reloadFile(), currentFile?.content, currentFile?.isDirty, and
currentFile?.lastModified are only updated on success and the alert remains
visible on failure.
---
Duplicate comments:
In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift`:
- Around line 66-69: The UTF-16 code unit check using storage.length is
incorrect for file byte-size limits; replace it with a byte-size check using the
text content encoded as UTF-8. In the SyntaxHighlightManager method where you
currently do `if let storage = textView.textStorage, storage.length >
Self.maxHighlightSize { ... }`, compute the bytes like `let byteCount =
textView.string.data(using: .utf8)?.count ?? 0` (or use
NSString.byteLength(using: .utf8)) and compare `byteCount >
Self.maxHighlightSize`, then call `clearTemporaryAttributes()` and return;
ensure Self.maxHighlightSize is interpreted as bytes and handle the optional
data safely.
---
Nitpick comments:
In @.gitignore:
- Around line 52-53: Add documentation and/or a setup script explaining how to
populate apps/purepoint-macos/LocalPackages referenced by
XCLocalSwiftPackageReference entries in the Xcode project; specifically create a
README (e.g., LocalPackages/README) or a bootstrap script that lists required
tree-sitter packages (yaml, lua, css, python, javascript), shows the exact git
clone or symlink commands to populate LocalPackages/, and documents how to
re-sync or update those local package references so new developers can reproduce
the project build environment.
In
`@apps/purepoint-macos/purepoint-macos.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved`:
- Around line 4-21: Package pins for dependencies like "neon" (currently branch
"main"), "swifttreesitter" and tree-sitter language packages are using
branch-based pins which harms reproducibility; update Package.resolved (and your
Package.swift/Package.resolved workflow) to pin these to specific revisions or
tagged versions instead of branches—replace branch entries for identities such
as "neon", "swifttreesitter" and any tree-sitter language packages with their
exact "revision" or "version" values (commit SHAs or semver tags) so CI/builds
use an immutable, reproducible dependency set.
In `@apps/purepoint-macos/purepoint-macos/Services/GitService.swift`:
- Around line 289-290: Replace the silent lossy decoding call String(decoding:
data, as: UTF8.self) with the failable initializer String(bytes: data, encoding:
.utf8) and handle the optional result where `path` is used (e.g., in the
surrounding GitService method that produces `path`); if decoding fails
return/throw/log an appropriate error or skip processing so malformed output
isn't treated as a valid path. Ensure you update references to the previous
`path` variable to unwrap the optional safely and provide a clear error path.
In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift`:
- Around line 83-92: The locationTransformer closure is O(n) per call because it
builds and splits the substring up to offset each time; change it to use cached
line start offsets instead of components(separatedBy:), e.g., maintain a
reusable array of lineStartOffsets updated when textStorage changes and compute
row/column by binary-searching that array (referencing locationTransformer,
textView/textStorage, and Point) so you avoid scanning the whole prefix on every
invocation; ensure the cache is invalidated/updated on edits to the text
storage.
In `@apps/purepoint-macos/purepoint-macos/State/EditorState.swift`:
- Around line 103-122: The empty catch in checkForExternalChanges silently
discards errors from FileIOService.readFile(at:), so update the Task's error
handling to capture the thrown error and surface it (e.g., log it via your app
logger or os_log, and/or set a recoverable UI state such as externalChangeAlert
or a new fileReadError property) instead of swallowing it; specifically, modify
the Task inside checkForExternalChanges to use catch { let err in
Logger.shared.error("Failed to read file \(filePath): \(err)") /* or update
externalChangeAlert/currentFile state to indicate read failure */ } so failures
from FileIOService.readFile, and the filePath/file.id, are recorded and
lastModified is not left inconsistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 22682600-5ddb-48f2-ba46-7ce900acf962
📒 Files selected for processing (12)
.gitignoreapps/purepoint-macos/purepoint-macos.xcodeproj/project.pbxprojapps/purepoint-macos/purepoint-macos.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolvedapps/purepoint-macos/purepoint-macos/Models/EditorTab.swiftapps/purepoint-macos/purepoint-macos/Services/GitService.swiftapps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swiftapps/purepoint-macos/purepoint-macos/State/EditorState.swiftapps/purepoint-macos/purepoint-macos/Views/Detail/ProjectDetailView.swiftapps/purepoint-macos/purepoint-macos/Views/Detail/WorktreeDetailView.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/EditorNSView.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift
💤 Files with no reviewable changes (1)
- apps/purepoint-macos/purepoint-macos/Views/Editor/EditorTabBar.swift
Comment on lines
+150
to
+158
| editorState.updateContent(content: newContent) | ||
| }, | ||
| onSave: { | ||
| Task { | ||
| do { | ||
| try await editorState.saveFile(id: tab.id) | ||
| try await editorState.saveFile() | ||
| saveError = nil | ||
| } catch { | ||
| saveError = "Failed to save \(tab.name): \(error.localizedDescription)" | ||
| saveError = "Failed to save \(file.name): \(error.localizedDescription)" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="$(fd -t f 'EditorState.swift' | head -n 1)"
echo "Inspecting: $FILE"
# Show save/reload implementations and nearby state usage.
rg -n -C6 'func saveFile\(|func reloadFile\(|currentFile|externalChangeAlert' "$FILE"Repository: 2witstudios/PurePoint
Length of output: 3335
Save/reload operations apply state updates to the wrong file if currentFile changes mid-operation.
saveFile() (line 155) and reloadFile() (lines 293–300) capture file identity at entry but use optional chaining on mutable currentFile after await statements. A fast file switch between the async call and completion causes state updates (isDirty, lastModified, content) to apply to the newly active file instead of the original.
Both methods should use the captured file or path reference consistently throughout, or validate that currentFile hasn't changed before applying post-operation updates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/purepoint-macos/purepoint-macos/Views/Detail/WorktreeDetailView.swift`
around lines 150 - 158, The save/reload race occurs because saveFile() and
reloadFile() capture the file identity at start but after awaits update mutable
currentFile via optional chaining; fix by consistently using the originally
captured reference (e.g., the local file or path captured at method entry) for
all post-await state updates (isDirty, lastModified, content) or explicitly
verify currentFile still equals the captured file before applying changes;
update saveFile(), reloadFile(), and any callers that use currentFile after
awaits to reference the captured file variable (or bail/skip updates if
currentFile has changed).
Owner
Author
There was a problem hiding this comment.
Valid observation. The save/reload race and task cancellation in EditorState are pre-existing and out of scope for this PR (which focuses on syntax highlighting). Noted for a follow-up.
There was a problem hiding this comment.
@2witstudios, understood — thanks for the confirmation. Would you like me to open a follow-up GitHub issue to track the save/reload race condition in EditorState so it doesn't get lost?
🧠 Learnings used
Learnt from: 2witstudios
Repo: 2witstudios/PurePoint PR: 141
File: apps/purepoint-macos/purepoint-macos/State/StreamAccumulator.swift:93-124
Timestamp: 2026-03-10T19:10:35.275Z
Learning: In `apps/purepoint-macos/purepoint-macos/State/StreamAccumulator.swift`, the Claude CLI streaming protocol sends `contentBlockDelta` events as the primary, most-current text source and periodic `assistant` snapshots as checkpoints that can lag behind. The `finalize()` method intentionally uses a "merge rather than replace" strategy when `contentBlocks` is already non-empty from deltas: only `.toolUse` `input` JSON is patched from the snapshot (deltas only carry `{}` as a placeholder), and text is never rebuilt from the snapshot to avoid regression. The snapshot is only used as a full fallback when no deltas were received at all.
Learnt from: 2witstudios
Repo: 2witstudios/PurePoint PR: 116
File: crates/pu-engine/src/output_buffer.rs:66-68
Timestamp: 2026-03-09T00:36:02.768Z
Learning: In `crates/pu-engine/src/output_buffer.rs`, `has_substantive_content()` is intentionally stateless and evaluated per raw PTY read chunk. The maintainers consciously chose not to add cross-write ANSI/UTF-8 accumulator state because: (1) Claude Code spinner frames arrive as atomic PTY writes so split sequences don't occur in practice; (2) split substantive content is fine because the next chunk resets `last_content_write` within the 30 s idle timeout; (3) split ANSI tails miscounted as printable are a false positive that only delays Waiting detection — it fails safe. Do not suggest stateful ANSI parsing for this file unless the agent target changes.
apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift
Show resolved
Hide resolved
Split DiffState.loadTask into separate unstagedTask and prTask so watcher-triggered refresh cycles don't cancel the PR fetch before it completes. Replace non-existent "pull.request" SF Symbol with "arrow.triangle.pull". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/purepoint-macos/purepoint-macos/State/DiffState.swift (1)
79-85: Refresh button won't update PR list, which may confuse users on the PR tab.Per the relevant code snippets,
ProjectDetailView(line 95) andWorktreeDetailView(line 99) both calldiffState.refresh()from a refresh button. When users are viewing the "Pull Requests" tab and click refresh, they won't see updated PRs sincerefresh()now only fetches unstaged changes.Consider either:
- Documenting this behavior in the UI (e.g., tooltip clarification), or
- Adding a conditional PR refresh when the active tab is
.prDiffs♻️ Optional: conditionally refresh PRs when on PR tab
func refresh() { let path = currentWorktreePath ?? currentProjectRoot guard let path else { return } unstagedTask?.cancel() unstagedTask = Task { await fetchUnstaged(path: path) } - // Don't re-fetch PRs on refresh — they don't change on file save + // Optionally refresh PRs when user explicitly requests refresh on PR tab + if activeTab == .prDiffs { + let branch = currentWorktreePath != nil ? nil : nil // Adjust if branch is needed + prTask?.cancel() + prTask = Task { await fetchPRs(cwd: path, branch: branch) } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/State/DiffState.swift` around lines 79 - 85, The refresh() method currently only re-fetches unstaged changes via fetchUnstaged and cancels/starts unstagedTask, so clicking Refresh on the Pull Requests tab won't update PRs; update refresh() in DiffState to detect the active tab (e.g., an enum case .prDiffs) and, when active tab is .prDiffs, call the existing PR-fetching method (or add one if missing) to refresh PR data in addition to or instead of fetchUnstaged; reference refresh(), unstagedTask, fetchUnstaged, and the active-tab property (or add one to DiffState) to implement the conditional PR refresh.apps/purepoint-macos/purepoint-macos/Views/Detail/ProjectDetailView.swift (1)
108-128: Significant code duplication with WorktreeDetailView.The following components are nearly identical between
ProjectDetailViewandWorktreeDetailView:
fileBreadcrumb(lines 108-128)editorContent(lines 132-163)binaryPlaceholder/editorPlaceholder(lines 295-322)externalChangeBannerandbannerView(lines 326-378)Consider extracting these into shared components (e.g., an
EditorPaneview or a shared utility extension) to improve maintainability and reduce the risk of divergence.Also applies to: 295-378
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Views/Detail/ProjectDetailView.swift` around lines 108 - 128, Duplicate UI logic exists between ProjectDetailView and WorktreeDetailView for fileBreadcrumb, editorContent, binaryPlaceholder/editorPlaceholder, and externalChangeBanner/bannerView; extract these into one or more shared SwiftUI views (e.g., EditorPane, FileBreadcrumbView, PlaceholderView, ExternalChangeBanner) and replace the duplicated methods in both classes with calls to the new reusable views. Ensure the new views accept the necessary inputs used in the originals (e.g., EditorTab, isDirty, language icon, file name, binding/state for editor content and external-change state) and preserve styling (Theme.cardHeaderBackground, font sizes, Circle indicator, padding) and any callbacks so behavior remains identical. Update ProjectDetailView and WorktreeDetailView to construct the shared views and remove the duplicated methods (fileBreadcrumb, editorContent, binaryPlaceholder, editorPlaceholder, externalChangeBanner, bannerView).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/purepoint-macos/purepoint-macos/Views/Detail/ProjectDetailView.swift`:
- Around line 42-47: The in-flight Tasks started by EditorState's async ops
(e.g., openFile(), reloadFile()) are orphaned when replacing editorState in the
.task(id: project.projectRoot) block: call editorState.stopWatching() then
immediately cancel any pending Tasks created by that EditorState before
assigning editorState = EditorState(); modify EditorState to retain references
to its Tasks (or expose a cancelAll() that uses Task.cancel() or a TaskGroup)
and invoke that cancellation from this block so openFile()/reloadFile() tasks
are cancelled and cannot update the discarded EditorState instance.
---
Nitpick comments:
In `@apps/purepoint-macos/purepoint-macos/State/DiffState.swift`:
- Around line 79-85: The refresh() method currently only re-fetches unstaged
changes via fetchUnstaged and cancels/starts unstagedTask, so clicking Refresh
on the Pull Requests tab won't update PRs; update refresh() in DiffState to
detect the active tab (e.g., an enum case .prDiffs) and, when active tab is
.prDiffs, call the existing PR-fetching method (or add one if missing) to
refresh PR data in addition to or instead of fetchUnstaged; reference refresh(),
unstagedTask, fetchUnstaged, and the active-tab property (or add one to
DiffState) to implement the conditional PR refresh.
In `@apps/purepoint-macos/purepoint-macos/Views/Detail/ProjectDetailView.swift`:
- Around line 108-128: Duplicate UI logic exists between ProjectDetailView and
WorktreeDetailView for fileBreadcrumb, editorContent,
binaryPlaceholder/editorPlaceholder, and externalChangeBanner/bannerView;
extract these into one or more shared SwiftUI views (e.g., EditorPane,
FileBreadcrumbView, PlaceholderView, ExternalChangeBanner) and replace the
duplicated methods in both classes with calls to the new reusable views. Ensure
the new views accept the necessary inputs used in the originals (e.g.,
EditorTab, isDirty, language icon, file name, binding/state for editor content
and external-change state) and preserve styling (Theme.cardHeaderBackground,
font sizes, Circle indicator, padding) and any callbacks so behavior remains
identical. Update ProjectDetailView and WorktreeDetailView to construct the
shared views and remove the duplicated methods (fileBreadcrumb, editorContent,
binaryPlaceholder, editorPlaceholder, externalChangeBanner, bannerView).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec006308-bfd4-456e-809a-7b3c44c218a6
📒 Files selected for processing (3)
apps/purepoint-macos/purepoint-macos/State/DiffState.swiftapps/purepoint-macos/purepoint-macos/Views/Detail/ProjectDetailView.swiftapps/purepoint-macos/purepoint-macos/Views/Detail/WorktreeDetailView.swift
Comment on lines
42
to
47
| .task(id: project.projectRoot) { | ||
| editorState.stopWatching() | ||
| editorState = EditorState() | ||
| diffState.loadForProject(projectRoot: project.projectRoot) | ||
| fileTreeState.load(worktreePath: project.projectRoot) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if EditorState has any mechanism to cancel in-flight tasks
fd -t f 'EditorState.swift' --exec cat {} \; | rg -n -C3 'Task|cancel|saveFile|reloadFile|openFile'Repository: 2witstudios/PurePoint
Length of output: 1738
🏁 Script executed:
fd -t f 'EditorState.swift' --exec wc -l {} \;Repository: 2witstudios/PurePoint
Length of output: 131
🏁 Script executed:
fd -t f 'EditorState.swift' --exec cat {} \;Repository: 2witstudios/PurePoint
Length of output: 3703
In-flight async tasks may become orphaned when project changes.
When the project changes, stopWatching() only stops the file watcher—not any in-flight openFile() or reloadFile() tasks. These Task instances are created without references stored for cancellation and continue running against the old EditorState instance after line 44 creates a new one.
While actual file writes succeed safely, the orphaned tasks waste resources and their completion callbacks update discarded state. Consider storing Task references or using a TaskGroup to cancel pending operations when stopping.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/purepoint-macos/purepoint-macos/Views/Detail/ProjectDetailView.swift`
around lines 42 - 47, The in-flight Tasks started by EditorState's async ops
(e.g., openFile(), reloadFile()) are orphaned when replacing editorState in the
.task(id: project.projectRoot) block: call editorState.stopWatching() then
immediately cancel any pending Tasks created by that EditorState before
assigning editorState = EditorState(); modify EditorState to retain references
to its Tasks (or expose a cancelAll() that uses Task.cancel() or a TaskGroup)
and invoke that cancellation from this block so openFile()/reloadFile() tasks
are cancelled and cannot update the discarded EditorState instance.
Owner
Author
There was a problem hiding this comment.
Valid point about orphaned async tasks. This is pre-existing behavior and out of scope for this syntax-highlighting PR. Noted for a follow-up to add task cancellation in EditorState/ProjectDetailView.
There was a problem hiding this comment.
@2witstudios, understood — happy to defer this to a follow-up. Would you like me to open a GitHub issue to track the task cancellation work in EditorState/ProjectDetailView so it doesn't get lost?
- Fix CI: clone LocalPackages tree-sitter repos in workflow - Fix 1MB size check to use utf8.count instead of UTF-16 length - Fix .cfg/.ini/.conf incorrectly mapped to TOML (now plaintext) - Fix .gitconfig/.gitmodules/.editorconfig mapped to plaintext - Fix process.run() crash risk in findGhPath with do/catch - Fix empty file showing no line number (draw "1" for empty glyphRange) - Fix setLanguage() before setContent() causing stale 1MB check Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/purepoint-macos/purepoint-macos/Services/GitService.swift (1)
292-294: Use failable UTF-8 conversion to satisfy lint and avoid lossy decode behavior.At Line 293, prefer
String(bytes:encoding:)overString(decoding:as:)forData→Stringconversion.Suggested diff
- let path = String(decoding: data, as: UTF8.self) - .trimmingCharacters(in: .whitespacesAndNewlines) + let path = (String(bytes: data, encoding: .utf8) ?? "") + .trimmingCharacters(in: .whitespacesAndNewlines)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Services/GitService.swift` around lines 292 - 294, Replace the lossily-decoding call that uses String(decoding:as:) with a failable UTF-8 conversion using String(bytes:encoding:); specifically, where the code reads the git output into data and sets path (the lines using variables data and path in GitService.swift), change to use String(bytes: data, encoding: .utf8) and handle the optional (e.g., guard/if let) before trimming with trimmingCharacters(in: .whitespacesAndNewlines) so you don't silently lose data when decoding fails..github/workflows/macos.yml (1)
50-54: Pin grammar clones to immutable refs for reproducible CI.Lines 50–54 clone default-branch HEADs without pinning. This makes builds non-deterministic and can break unexpectedly when upstream repos change. Pin each grammar to a tag or commit SHA and checkout that exact ref.
Proposed workflow change
- name: Clone local tree-sitter packages run: | mkdir -p apps/purepoint-macos/LocalPackages - git clone --depth 1 https://github.com/tree-sitter/tree-sitter-css.git apps/purepoint-macos/LocalPackages/tree-sitter-css - git clone --depth 1 https://github.com/tree-sitter/tree-sitter-javascript.git apps/purepoint-macos/LocalPackages/tree-sitter-javascript - git clone --depth 1 https://github.com/tree-sitter-grammars/tree-sitter-lua.git apps/purepoint-macos/LocalPackages/tree-sitter-lua - git clone --depth 1 https://github.com/tree-sitter/tree-sitter-python.git apps/purepoint-macos/LocalPackages/tree-sitter-python - git clone --depth 1 https://github.com/tree-sitter-grammars/tree-sitter-yaml.git apps/purepoint-macos/LocalPackages/tree-sitter-yaml + clone_and_pin () { + local repo_url="$1" + local dest="$2" + local ref="$3" # tag or commit SHA + git clone --depth 1 "$repo_url" "$dest" + git -C "$dest" fetch --depth 1 origin "$ref" + git -C "$dest" checkout --detach "$ref" + } + clone_and_pin https://github.com/tree-sitter/tree-sitter-css.git apps/purepoint-macos/LocalPackages/tree-sitter-css <css-ref> + clone_and_pin https://github.com/tree-sitter/tree-sitter-javascript.git apps/purepoint-macos/LocalPackages/tree-sitter-javascript <javascript-ref> + clone_and_pin https://github.com/tree-sitter-grammars/tree-sitter-lua.git apps/purepoint-macos/LocalPackages/tree-sitter-lua <lua-ref> + clone_and_pin https://github.com/tree-sitter/tree-sitter-python.git apps/purepoint-macos/LocalPackages/tree-sitter-python <python-ref> + clone_and_pin https://github.com/tree-sitter-grammars/tree-sitter-yaml.git apps/purepoint-macos/LocalPackages/tree-sitter-yaml <yaml-ref>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/macos.yml around lines 50 - 54, The git clone commands currently pulling HEAD of various tree-sitter repos (e.g., https://github.com/tree-sitter/tree-sitter-css.git, tree-sitter-javascript.git, tree-sitter-lua.git, tree-sitter-python.git, tree-sitter-yaml.git) must be pinned to immutable refs; update each clone in the CI workflow to either clone a specific tag/commit with --branch <TAG_OR_SHA> --single-branch or perform the existing shallow clone then run git -C apps/purepoint-macos/LocalPackages/<repo-dir> fetch --depth=1 origin <TAG_OR_SHA> && git -C ... checkout --force <TAG_OR_SHA> (or git reset --hard <SHA>) so the checked-out revision is an exact tag or commit SHA rather than HEAD.apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift (1)
83-91: Location transform path is allocation-heavy per lookup.The current
substring+components(separatedBy:)approach does repeated full-prefix allocations. Consider switching to a line-index/line-start strategy to avoid this hot-path overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift` around lines 83 - 91, The locationTransformer closure is allocation-heavy because it builds a substring and splits it into lines for every lookup; instead scan the text up to offset without creating new strings — e.g., use the backing NSString/NSString.UTF16View to iterate bytes/UTF-16 code units up to offset and count newline characters to compute row, and compute column by measuring distance from the last newline to offset (or use NSString.getLineStart(_:end:contentsEnd:for:) APIs to find the line start and compute column), updating the closure that references textView/textStorage and returning Point(row: row, column: col) with no intermediate allocations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/purepoint-macos/purepoint-macos/Models/EditorTab.swift`:
- Line 71: The current language mapping in EditorTab.swift routes "graphql" and
"gql" to the JavaScript grammar (case "graphql", "gql": return .javascript),
causing incorrect highlighting; update that mapping to return .plaintext instead
so GraphQL files fall back to plain text until a proper GraphQL parser/grammar
is added—locate the switch/lookup that contains the "graphql","gql" case and
change the returned symbol from .javascript to .plaintext.
In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift`:
- Around line 143-145: The .c and .java enum cases currently return nil (in
SyntaxHighlightManager) which causes detected C/Java files to lose highlighting;
update the handler that switches on language (the cases for .c and .java in
SyntaxHighlightManager) to return an appropriate Highlighter/HighlighterConfig
instead of nil — either add the tree-sitter-c and tree-sitter-java SPM packages
and construct their configs, or temporarily map .c to the existing .cpp
highlighter and .java to the existing Java highlighter implementation you
already use elsewhere; ensure you update both occurrences noted (around the
.c/.cpp block and the .java case) so detection and highlighting are consistent.
In `@apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift`:
- Around line 55-66: rebuildLineStarts currently skips a final empty line when a
file ends with a newline; change the cache builder (rebuildLineStarts) to use
NSString/CF getLineStart(_:end:contentsEnd:) to obtain the line start and
contentsEnd and append the final line start even when it equals s.length so
trailing-newline lines are represented; then update the drawing logic that
iterates lineStarts (the draw loop that uses glyph ranges and checks charIdx ==
stringLength) to handle the case where the final line has no glyphs by using
layoutManager.extraLineFragmentRect(for:) (or asking the layout manager for the
line fragment rect at that character index) to compute the y-position and render
the line number for charIdx == stringLength.
---
Nitpick comments:
In @.github/workflows/macos.yml:
- Around line 50-54: The git clone commands currently pulling HEAD of various
tree-sitter repos (e.g., https://github.com/tree-sitter/tree-sitter-css.git,
tree-sitter-javascript.git, tree-sitter-lua.git, tree-sitter-python.git,
tree-sitter-yaml.git) must be pinned to immutable refs; update each clone in the
CI workflow to either clone a specific tag/commit with --branch <TAG_OR_SHA>
--single-branch or perform the existing shallow clone then run git -C
apps/purepoint-macos/LocalPackages/<repo-dir> fetch --depth=1 origin
<TAG_OR_SHA> && git -C ... checkout --force <TAG_OR_SHA> (or git reset --hard
<SHA>) so the checked-out revision is an exact tag or commit SHA rather than
HEAD.
In `@apps/purepoint-macos/purepoint-macos/Services/GitService.swift`:
- Around line 292-294: Replace the lossily-decoding call that uses
String(decoding:as:) with a failable UTF-8 conversion using
String(bytes:encoding:); specifically, where the code reads the git output into
data and sets path (the lines using variables data and path in
GitService.swift), change to use String(bytes: data, encoding: .utf8) and handle
the optional (e.g., guard/if let) before trimming with trimmingCharacters(in:
.whitespacesAndNewlines) so you don't silently lose data when decoding fails.
In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift`:
- Around line 83-91: The locationTransformer closure is allocation-heavy because
it builds a substring and splits it into lines for every lookup; instead scan
the text up to offset without creating new strings — e.g., use the backing
NSString/NSString.UTF16View to iterate bytes/UTF-16 code units up to offset and
count newline characters to compute row, and compute column by measuring
distance from the last newline to offset (or use
NSString.getLineStart(_:end:contentsEnd:for:) APIs to find the line start and
compute column), updating the closure that references textView/textStorage and
returning Point(row: row, column: col) with no intermediate allocations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f95197f-fa5f-470b-b3eb-88512135edf0
📒 Files selected for processing (6)
.github/workflows/macos.ymlapps/purepoint-macos/purepoint-macos/Models/EditorTab.swiftapps/purepoint-macos/purepoint-macos/Services/GitService.swiftapps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/EditorNSView.swiftapps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift
Comment on lines
+143
to
+145
| case .c: | ||
| return nil // TODO: Add tree-sitter-c SPM package (SPM resolution conflict) | ||
| case .cpp: |
There was a problem hiding this comment.
C and Java are detected upstream but never get a highlighter config.
Returning nil for .c and .java means those files silently lose syntax highlighting despite being classified as supported languages by detection.
Also applies to: 156-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift`
around lines 143 - 145, The .c and .java enum cases currently return nil (in
SyntaxHighlightManager) which causes detected C/Java files to lose highlighting;
update the handler that switches on language (the cases for .c and .java in
SyntaxHighlightManager) to return an appropriate Highlighter/HighlighterConfig
instead of nil — either add the tree-sitter-c and tree-sitter-java SPM packages
and construct their configs, or temporarily map .c to the existing .cpp
highlighter and .java to the existing Java highlighter implementation you
already use elsewhere; ensure you update both occurrences noted (around the
.c/.cpp block and the .java case) so detection and highlighting are consistent.
Owner
Author
There was a problem hiding this comment.
Intentional — the // TODO comments explain the reason: tree-sitter-c has an SPM resolution conflict, and tree-sitter-java collides with tree-sitter-javascript in Xcode's package resolver. Both return nil which falls back to plain monospace text (no broken highlighting). Adding these parsers requires resolving the SPM conflicts first, which is tracked for follow-up.
There was a problem hiding this comment.
@2witstudios, understood — the SPM conflicts make these nil returns the right call for now, and the // TODO comments already document the blockers clearly. Thanks for the context!
✏️ Learnings added
Learnt from: 2witstudios
Repo: 2witstudios/PurePoint PR: 149
File: apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift:143-145
Timestamp: 2026-03-16T19:20:14.929Z
Learning: In `apps/purepoint-macos/purepoint-macos/Services/SyntaxHighlightManager.swift`, the `.c` and `.java` cases in `SyntaxHighlightManager.languageConfiguration(for:)` intentionally return `nil` (no syntax highlighting) due to unresolved SPM conflicts: `tree-sitter-c` has an SPM resolution conflict, and `tree-sitter-java` collides with `tree-sitter-javascript` in Xcode's package resolver. These fall back to plain monospace text and are tracked for follow-up once the SPM conflicts are resolved. Do not flag these as missing highlighting implementations.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: 2witstudios
Repo: 2witstudios/PurePoint PR: 149
File: apps/purepoint-macos/purepoint-macos.xcodeproj/project.pbxproj:726-746
Timestamp: 2026-03-16T18:46:20.927Z
Learning: In the PurePoint macOS project (apps/purepoint-macos), Xcode 26 causes linker errors when local SPM packages (XCLocalSwiftPackageReference in project.pbxproj) have non-essential files present (e.g., Python/Go/Rust bindings, test directories, metadata files). The fix is to strip those files after cloning so SPM auto-discovers only the C sources in `src/`. This applies to tree-sitter language grammar packages cloned into LocalPackages/ (tree-sitter-css, tree-sitter-javascript, tree-sitter-lua, tree-sitter-python, tree-sitter-yaml).
Comment on lines
+55
to
+66
| func rebuildLineStarts() { | ||
| guard let textView = clientView as? NSTextView else { return } | ||
| let s = textView.string as NSString | ||
| lineStarts = [0] | ||
| var idx = 0 | ||
| while idx < s.length { | ||
| let range = s.lineRange(for: NSRange(location: idx, length: 0)) | ||
| let next = NSMaxRange(range) | ||
| if next < s.length { lineStarts.append(next) } | ||
| idx = next | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file structure and locate the relevant file
find . -name "LineNumberRulerView.swift" -type fRepository: 2witstudios/PurePoint
Length of output: 142
🏁 Script executed:
# Let's read the LineNumberRulerView.swift file to see the current implementation
cat -n apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swiftRepository: 2witstudios/PurePoint
Length of output: 5605
🏁 Script executed:
# Let's also check the EditorNSView.swift file for context
cat -n apps/purepoint-macos/purepoint-macos/Views/Editor/EditorNSView.swift | head -100Repository: 2witstudios/PurePoint
Length of output: 4167
🌐 Web query:
AppKit NSLayoutManager extraLineFragmentRect terminal newline behavior
💡 Result:
In AppKit/TextKit, a trailing paragraph separator (typically a terminal \n) is treated specially: there’s no visible “newline glyph”, but the insertion point still needs a place to appear after that final line break. To support that, NSLayoutManager creates an extra line fragment at the end of the document.
layoutManager.extraLineFragmentRectis the line fragment rectangle reserved for the caret at end-of-text when the text is empty or ends with a final paragraph separator; otherwise it’sNSZeroRect. [1]layoutManager.extraLineFragmentUsedRectis the (usually narrower) rect that encloses the insertion point within that extra fragment. [1]- Practically: if your string ends with
\n, the caret is considered to be on a new, empty last line (like Terminal/editors that show the cursor on the line after the final newline), and that’s represented byextraLineFragmentRect/UsedRect, not bylineFragmentRect(forGlyphAt:)of the last “real” line. [1][2]
This is why code that computes “last line rect” / scrolling / caret coordinates often needs to explicitly check extraLineFragmentRect != .zero and use it when the selection is at the very end and the text ends in a newline. [2]
Sources:
[1] Apple NSLayoutManager Class Reference (extra line fragment description) (leopard-adc.pepas.com)
[2] “Text view adventures, Part 4” (newline-at-end triggers extraLineFragmentRect; scrolling/caret implications) (lapcatsoftware.com)
Citations:
- 1: https://leopard-adc.pepas.com/documentation/Cocoa/Reference/ApplicationKit/Classes/NSLayoutManager_Class/NSLayoutManager_Class.pdf?utm_source=openai
- 2: https://lapcatsoftware.com/articles/textview4.html?utm_source=openai
🏁 Script executed:
# Let's verify the NSString.lineRange behavior with a simple test
# and understand the getLineStart behavior difference
cat > /tmp/test_linerange.swift << 'EOF'
import Foundation
// Test case 1: String with terminal newline
let str1 = "foo\n" as NSString
let range1 = str1.lineRange(for: NSRange(location: 0, length: 0))
print("String 'foo\\n' (length \(str1.length)):")
print(" lineRange result: \(range1)")
print(" NSMaxRange: \(NSMaxRange(range1))")
// Test getLineStart for same string
var ls1 = 0, le1 = 0, lce1 = 0
str1.getLineStart(&ls1, end: &le1, contentsEnd: &lce1, for: NSRange(location: 0, length: 0))
print(" getLineStart: start=\(ls1), end=\(le1), contentsEnd=\(lce1)")
// Try to get the next line
if le1 < str1.length {
var ls2 = 0, le2 = 0, lce2 = 0
str1.getLineStart(&ls2, end: &le2, contentsEnd: &lce2, for: NSRange(location: le1, length: 0))
print(" Line 2: start=\(ls2), end=\(le2), contentsEnd=\(lce2)")
}
print()
// Test case 2: String without terminal newline
let str2 = "foo" as NSString
let range2 = str2.lineRange(for: NSRange(location: 0, length: 0))
print("String 'foo' (length \(str2.length)):")
print(" lineRange result: \(range2)")
print(" NSMaxRange: \(NSMaxRange(range2))")
var ls3 = 0, le3 = 0, lce3 = 0
str2.getLineStart(&ls3, end: &le3, contentsEnd: &lce3, for: NSRange(location: 0, length: 0))
print(" getLineStart: start=\(ls3), end=\(le3), contentsEnd=\(lce3)")
print()
// Test case 3: Multiple lines with terminal newline
let str3 = "foo\nbar\n" as NSString
print("String 'foo\\nbar\\n' (length \(str3.length)):")
var lineIdx = 0
var lineNum = 1
while lineIdx < str3.length {
let range = str3.lineRange(for: NSRange(location: lineIdx, length: 0))
print(" Line \(lineNum): range=\(range), NSMaxRange=\(NSMaxRange(range))")
lineIdx = NSMaxRange(range)
lineNum += 1
}
EOF
# This will fail because we can't run Swift directly, but let's document the expected behavior
echo "The key difference:"
echo "- lineRange: returns the range including the newline character"
echo "- getLineStart/end/contentsEnd: separates the actual line end (with newline) from content end"
echo "For 'foo\\n': getLineStart would show contentsEnd < lineEnd, indicating a newline"Repository: 2witstudios/PurePoint
Length of output: 322
🏁 Script executed:
# Let's look for how lineRange behaves with the final newline
# by examining if there are any tests or comments in the codebase
rg -i "linerange|extralinefragment" --type swift -A 3 -B 3Repository: 2witstudios/PurePoint
Length of output: 871
Handle newline-terminated files via the extra line fragment.
Files ending with a newline don't record the final blank line in lineStarts. On line 63, the condition next < s.length skips EOF—so for "foo\n" (length 4), when next becomes 4, the append is rejected. AppKit represents the blank line created by a terminal newline via layoutManager.extraLineFragmentRect, which has no glyph-backed range. The draw loop (lines 111–116) assumes every line has a glyph range and will silently skip the missing final line. This breaks line numbering for the common case of newline-at-EOF.
Use getLineStart(_:end:contentsEnd:) in the cache builder to detect trailing newlines and append the final line start even when it equals the string length. Update the draw logic to handle extraLineFragmentRect when charIdx == stringLength.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/purepoint-macos/purepoint-macos/Views/Editor/LineNumberRulerView.swift`
around lines 55 - 66, rebuildLineStarts currently skips a final empty line when
a file ends with a newline; change the cache builder (rebuildLineStarts) to use
NSString/CF getLineStart(_:end:contentsEnd:) to obtain the line start and
contentsEnd and append the final line start even when it equals s.length so
trailing-newline lines are represented; then update the drawing logic that
iterates lineStarts (the draw loop that uses glyph ranges and checks charIdx ==
stringLength) to handle the case where the final line has no glyphs by using
layoutManager.extraLineFragmentRect(for:) (or asking the layout manager for the
line fragment rect at that character index) to compute the y-position and render
the line number for charIdx == stringLength.
Owner
Author
There was a problem hiding this comment.
Valid observation about the trailing-newline edge case. The current implementation handles the common case (empty file → draw "1"), and files ending with a newline still show correct line numbers for all content lines. The missing final empty line number is a minor visual polish item — noted for follow-up.
There was a problem hiding this comment.
@2witstudios, understood — the content lines are numbered correctly and the missing final blank-line number is purely cosmetic. Would you like me to open a GitHub issue to track this as a follow-up polish item so it doesn't get lost?
🧠 Learnings used
Learnt from: 2witstudios
Repo: 2witstudios/PurePoint PR: 116
File: crates/pu-engine/src/output_buffer.rs:66-68
Timestamp: 2026-03-09T00:36:02.768Z
Learning: In `crates/pu-engine/src/output_buffer.rs`, `has_substantive_content()` is intentionally stateless and evaluated per raw PTY read chunk. The maintainers consciously chose not to add cross-write ANSI/UTF-8 accumulator state because: (1) Claude Code spinner frames arrive as atomic PTY writes so split sequences don't occur in practice; (2) split substantive content is fine because the next chunk resets `last_content_write` within the 30 s idle timeout; (3) split ANSI tails miscounted as printable are a false positive that only delays Waiting detection — it fails safe. Do not suggest stateful ANSI parsing for this file unless the agent target changes.
Learnt from: 2witstudios
Repo: 2witstudios/PurePoint PR: 141
File: apps/purepoint-macos/purepoint-macosTests/StreamAccumulatorTests.swift:129-149
Timestamp: 2026-03-10T19:10:57.484Z
Learning: In `StreamAccumulator.finalize` (apps/purepoint-macos/purepoint-macos/State/StreamAccumulator.swift), the intentional behavior is: if `.contentBlockDelta` events have already populated `contentBlocks`, `finalize` preserves that delta-driven text as the authoritative source and does NOT replace it with the `.assistant(content:)` snapshot. The snapshot is only used to patch `toolUse` input fields (which carry full JSON in the snapshot vs. partial `{}` during streaming). If no deltas have been accumulated, `finalize` falls back to applying the snapshot fully.
Learnt from: 2witstudios
Repo: 2witstudios/PurePoint PR: 149
File: apps/purepoint-macos/purepoint-macos.xcodeproj/project.pbxproj:726-746
Timestamp: 2026-03-16T18:46:20.927Z
Learning: In the PurePoint macOS project (apps/purepoint-macos), Xcode 26 causes linker errors when local SPM packages (XCLocalSwiftPackageReference in project.pbxproj) have non-essential files present (e.g., Python/Go/Rust bindings, test directories, metadata files). The fix is to strip those files after cloning so SPM auto-discovers only the C sources in `src/`. This applies to tree-sitter language grammar packages cloned into LocalPackages/ (tree-sitter-css, tree-sitter-javascript, tree-sitter-lua, tree-sitter-python, tree-sitter-yaml).
Xcode 26 doesn't respect the explicit `sources` parameter in local SPM packages, causing scanner.c to not be compiled and resulting in linker errors for CSS and JavaScript external scanner symbols. Fix: strip non-essential files (bindings, examples, JSON metadata) from cloned repos so SPM auto-discovers all C sources in src/ correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The find command was matching the package directory itself (not just its children), causing rm -rf to delete the entire directory tree including src/. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Xcode 26 ignores the `sources` parameter in local SPM packages. Instead of explicit sources, restructure each package: set path to "src", move public headers into src/include/, copy queries into src/queries/, and remove non-C/H metadata files. SPM then auto-discovers all .c files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
GraphQL has its own syntax that doesn't match JavaScript highlighting. Route .graphql/.gql to .plaintext until a tree-sitter-graphql parser is added. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SyntaxHighlightManagerEditorLanguagethrough toEditorNSViewso highlighting activates based on file typeEditorThemecolor palette for token categories (keywords, strings, comments, types, functions, etc.)DiffState.loadTaskinto independentunstagedTaskandprTaskso watcher-triggered refreshes don't cancel the PR fetch"pull.request"with"arrow.triangle.pull"in ProjectDetailView and WorktreeDetailViewTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes / UX
Chores