Skip to content

Move manual dictionary loading off the EDT - #67

Merged
barad1tos merged 5 commits into
mainfrom
refactor/edt-safe-dictionary-loading
Aug 14, 2026
Merged

barad1tos merged 5 commits into
mainfrom
refactor/edt-safe-dictionary-loading

Conversation

@barad1tos

@barad1tos barad1tos commented Aug 14, 2026 •

Copy link
Copy Markdown
Owner

── Summary ─────────────────────────────────

Manual dictionary loading currently performs SDEF generation, parser initialization, and PSI creation on the event dispatch thread, so a slow application dictionary can freeze the IDE. Move the expensive loading phase into cancellable project background tasks while preserving file selection, application naming, cache precedence, partial-success publication, and request order.

── Changes ─────────────────────────────────

  • Refactor: LoadDictionaryAction.kt — Queue manual loads per project, report progress, preserve FIFO ordering, publish completed dictionaries on the EDT, drop publication after project disposal, and retain application/file context when a background load fails.
  • Refactor: AppleScriptProjectDictionaryService.kt — Separate background dictionary initialization from synchronized EDT PSI publication without changing the public nullable loading API.
  • Test: EdtBridgeGuardTest.kt — Cover background execution, real cache publication, cancellation, FIFO ordering, multi-file partial success, disposal-safe cache and daemon behavior, and exact failing-request context.

── Validation ──────────────────────────────

  • ./gradlew test koverXmlReport koverVerify verifyPlugin verifyPluginStructure detekt ktlintCheck --rerun-tasks --stacktrace — 529 tests passed; coverage gates passed; plugin compatible with IC 251, IC 252, and IU 261.
  • Mutation checks — Dropping the second multi-file request, restarting the daemon after project disposal, and reporting the first request instead of the failing request each fail the corresponding regression test.
  • javap -classpath build/classes/kotlin/main -p com.intellij.plugin.applescript.lang.ide.actions.DictionaryLoadQueue — Platform-instantiable public no-arg JVM constructor present.
  • IDEA inspections — No warnings or errors in all changed files.
  • git diff --check — Passed.

── Notes ───────────────────────────────────

Review focus: DictionaryLoadQueue lifecycle transitions and the split between background dictionary initialization and synchronized EDT PSI publication. Child-process tree termination is intentionally unchanged and remains a separate follow-up.

- Prevent manual SDEF generation and parsing from blocking the UI while preserving file selection and application-name prompts
- Serialize cancellable project tasks, publish successful PSI on the EDT under service synchronization, and discard pending work after project disposal
- Cover background execution, cache publication, cancellation, FIFO ordering, and disposal

Impact: Manual dictionary loading stays responsive without changing cache precedence or partial-success behavior

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @barad1tos, you have reached your weekly rate limit of 1500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Reviewer's Guide

Moves manual AppleScript dictionary loading off the EDT by introducing a per-project background load queue, separating background dictionary initialization from EDT-bound PSI publication, and adding concurrency tests for threading, cancellation, FIFO ordering, and project disposal behavior.

Sequence diagram for background dictionary loading off the EDT

sequenceDiagram
    actor User
    participant LoadDictionaryAction
    participant DictionaryLoadQueue
    participant TaskBackgroundable
    participant AppleScriptProjectDictionaryService
    participant DaemonCodeAnalyzer

    User->>LoadDictionaryAction: actionPerformed
    LoadDictionaryAction->>LoadDictionaryAction: loadSelectedDictionaries
    LoadDictionaryAction->>AppleScriptProjectDictionaryService: getService
    LoadDictionaryAction->>DictionaryLoadQueue: submit(createTask)

    DictionaryLoadQueue->>TaskBackgroundable: startTask(queue)
    TaskBackgroundable->>TaskBackgroundable: run(indicator)
    loop for each DictionaryLoadRequest
        TaskBackgroundable->>TaskBackgroundable: indicator.checkCanceled
        TaskBackgroundable->>AppleScriptProjectDictionaryService: loadDictionaryInfo
        AppleScriptProjectDictionaryService-->>TaskBackgroundable: DictionaryInfo?
    end

    TaskBackgroundable->>DictionaryLoadQueue: onFinished -> taskFinished
    TaskBackgroundable->>AppleScriptProjectDictionaryService: publishDictionaries
    AppleScriptProjectDictionaryService->>AppleScriptProjectDictionaryService: createFromLoadedInfo
    AppleScriptProjectDictionaryService-->>DaemonCodeAnalyzer: settingsChanged

    DictionaryLoadQueue->>DictionaryLoadQueue: startNext
Loading

File-Level Changes

Change Details Files
Introduce a per-project DictionaryLoadQueue and refactor manual dictionary loading into cancellable background tasks while preserving EDT publication and request ordering.
  • Added DictionaryLoadQueue project service to queue backgroundable tasks, ensure single active load, and skip tasks for disposed projects.
  • Refactored directory chooser setup and dictionary selection into DictionaryLoadRequest objects for single or multiple application loads.
  • Implemented queueDictionaryLoads to wrap load operations in Task.Backgroundable with progress reporting, cancellation mapping, and EDT publish callbacks.
  • Changed manual load entry point to use AppleScriptProjectDictionaryService background info loading and deferred cache publication on EDT.
src/main/kotlin/com/intellij/plugin/applescript/lang/ide/actions/LoadDictionaryAction.kt
Split AppleScriptProjectDictionaryService dictionary creation into background info initialization and synchronized EDT PSI publication without altering the public API.
  • Extracted loadDictionaryInfo to perform file-based DictionaryInfo initialization outside the EDT.
  • Updated createDictionaryFromFile to delegate to loadDictionaryInfo then createFromInfo, preserving the public nullable contract.
  • Added synchronized createFromLoadedInfo that asserts EDT, wraps PSI creation in runReadAction, and is used by background load completion to publish dictionaries.
src/main/kotlin/com/intellij/plugin/applescript/lang/dictionary/project/AppleScriptProjectDictionaryService.kt
Extend EdtBridgeGuardTest to cover new manual load threading behavior, cancellation semantics, FIFO ordering, and DictionaryLoadQueue lifecycle.
  • Added testManualLoadThreading to assert loadSelectedDictionaries runs dictionary initialization off the EDT, publishes on EDT, and populates the project cache.
  • Added testManualLoadCancellation to ensure CancellationException from loadInfo is converted to completion via ProcessCanceledException and afterPublish still runs.
  • Added testManualRequestOrder to verify FIFO publication order when multiple manual loads complete out of order.
  • Added testManualLoadQueueOrder and testQueueDropsDisposedLoads to validate DictionaryLoadQueue start sequencing and disposal-aware task dropping, plus helper waitForCompletion using PlatformTestUtil.waitWithEventsDispatching.
src/test/kotlin/com/intellij/plugin/applescript/test/concurrency/EdtBridgeGuardTest.kt

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@codecov

codecov Bot commented Aug 14, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.10448% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...plescript/lang/ide/actions/LoadDictionaryAction.kt 77.41% 3 Missing and 11 partials ⚠️

📢 Thoughts on this report? Let us know!

- Prove every selected file runs while successful dictionaries still publish

- Verify project disposal skips cache publication and daemon restart

- Group injectable load effects behind one narrow internal seam

Impact: manual loads keep partial successes without leaking post-disposal updates
- Preserve the application name, file path, and original cause when a background manual load fails
- Lock the failing request with a two-file mutation-sensitive regression test

Impact: Manual dictionary load failures now identify the exact file that interrupted the batch
@barad1tos
barad1tos merged commit 5c7662a into main Aug 14, 2026
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant