diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fb6a96..cb97835 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ ### Changed +- **Rapid annotations now discard stale results** — when the IDE triggers multiple checks for the + same file in quick succession (e.g. series of saves), each file has a generation counter. If a + newer check starts while an older one is still running, the older result is discarded on + completion. Only the last check's annotations are applied to the editor (see + `DocscribeAnnotator.fileGeneration`). - **All commands now use project Ruby SDK** — `bundle exec docscribe` (via `DocscribeRunner` and `DocscribeDaemon.fallback()`) and `bundle exec docscribe --version` (via `DocscribeDaemon.performGemCheck()`) now prepend the Ruby SDK's `bin/` directory to `PATH` diff --git a/src/main/kotlin/com/florexlabs/docscribe/annotator/DocscribeAnnotator.kt b/src/main/kotlin/com/florexlabs/docscribe/annotator/DocscribeAnnotator.kt index 66ee0f8..33fffc8 100644 --- a/src/main/kotlin/com/florexlabs/docscribe/annotator/DocscribeAnnotator.kt +++ b/src/main/kotlin/com/florexlabs/docscribe/annotator/DocscribeAnnotator.kt @@ -15,7 +15,9 @@ import com.intellij.openapi.project.Project import com.intellij.openapi.util.TextRange import com.intellij.psi.PsiDocumentManager import com.intellij.psi.PsiFile +import org.jetbrains.annotations.VisibleForTesting import java.util.Objects +import java.util.concurrent.ConcurrentHashMap /** * Information collected by the annotator before running the background check. @@ -38,10 +40,16 @@ data class AnnotatorFileInfo( * Triggers automatically when a Ruby file is opened or saved. Uses JSON output for structured parsing. * Skips unsaved documents (docscribe reads from disk) and caches results by file modification stamp. * - * Cache invalidation: the [AnnotatorFileInfo.configHash] is derived from [DocscribeSettings], - * so changing settings (e.g. `hideCommentsByDefault`) automatically invalidates cached annotations - * by producing a different configHash. The DocscribeSettingsChangeListener also clears the cache - * explicitly on settings change. + * ## Concurrency + * When the IDE triggers multiple rapid annotations for the same file (e.g. during a series of quick saves), + * only the last check result is applied. Each file has a generation counter — if a newer check starts + * while an older one is still running, the older result is discarded on completion. + * + * ## Cache invalidation + * The [AnnotatorFileInfo.configHash] is derived from [DocscribeSettings], so changing settings + * (e.g. `hideCommentsByDefault`) automatically invalidates cached annotations by producing a + * different configHash. The DocscribeSettingsChangeListener also clears the cache explicitly + * on settings change. */ class DocscribeAnnotator : ExternalAnnotator() { /** @@ -104,12 +112,20 @@ class DocscribeAnnotator : ExternalAnnotator /** * Run docscribe check on the collected file, using the cache if the file is unchanged. * + * Each file has a generation counter. When a new check starts for a file, the counter is + * incremented. If an older check completes and finds its generation is stale (a newer check + * already started), the result is discarded. This ensures that during rapid saves only the + * last check result is applied. + * * @param info The file information collected by [collectInformation]. * @return Parsed [DocscribeOutput], or `null` if the file has no issues or the check failed. */ override fun doAnnotate(info: AnnotatorFileInfo): DocscribeOutput? { + val filePath = info.filePath + val generation = fileGeneration.merge(filePath, 1L) { _, old -> old + 1 } + val cache = DocscribeAnnotatorCache.getInstance() - val cached = cache.get(info.projectDir, info.filePath, info.fileStamp, info.configHash) + val cached = cache.get(info.projectDir, filePath, info.fileStamp, info.configHash) if (cached != null) { return if (cached.files.isEmpty()) null else cached } @@ -117,11 +133,15 @@ class DocscribeAnnotator : ExternalAnnotator val options = RunOptions( projectDir = info.projectDir, - file = info.filePath, + file = filePath, strategy = DocscribeStrategy.CHECK, formatJson = true, ) val result = DocscribeDaemon.executeWithFallback(info.project, options) + + // Another check for the same file started while this one was running — discard + if (fileGeneration[filePath] != generation) return null + val output = when { !result.success -> null @@ -130,7 +150,7 @@ class DocscribeAnnotator : ExternalAnnotator } if (output != null) { - cache.put(info.projectDir, info.filePath, info.fileStamp, info.configHash, output) + cache.put(info.projectDir, filePath, info.fileStamp, info.configHash, output) } return if (output == null || output.files.isEmpty()) null else output } @@ -172,4 +192,16 @@ class DocscribeAnnotator : ExternalAnnotator } } } + + companion object { + /** + * Generation counter per file path. + * + * Incremented each time [doAnnotate] starts for a file. When an annotation completes, + * its generation is compared to the current value — if they differ, a newer check ran + * and the stale result is discarded (see [doAnnotate]). + */ + @VisibleForTesting + internal val fileGeneration = ConcurrentHashMap() + } } diff --git a/src/main/kotlin/com/florexlabs/docscribe/runner/DocscribeRunner.kt b/src/main/kotlin/com/florexlabs/docscribe/runner/DocscribeRunner.kt index 3a65291..995e312 100644 --- a/src/main/kotlin/com/florexlabs/docscribe/runner/DocscribeRunner.kt +++ b/src/main/kotlin/com/florexlabs/docscribe/runner/DocscribeRunner.kt @@ -82,6 +82,10 @@ interface CommandExecutor { * Default [CommandExecutor] that uses IntelliJ's [GeneralCommandLine] and [CapturingProcessHandler]. * * Applies a 120-second timeout. Exit code 2 is treated as a failure (indistinguishable from timeout). + * + * @property environment Environment variables to pass to the child process. + * Merged with the parent process environment; keys in this map take precedence. + * Use to inject SDK paths (e.g. prepend Ruby SDK bin dir to `PATH`). */ class DefaultCommandExecutor( private val environment: Map = emptyMap(), diff --git a/src/test/kotlin/com/florexlabs/docscribe/annotator/DocscribeAnnotatorTest.kt b/src/test/kotlin/com/florexlabs/docscribe/annotator/DocscribeAnnotatorTest.kt index d774da5..8e9f21c 100644 --- a/src/test/kotlin/com/florexlabs/docscribe/annotator/DocscribeAnnotatorTest.kt +++ b/src/test/kotlin/com/florexlabs/docscribe/annotator/DocscribeAnnotatorTest.kt @@ -35,4 +35,43 @@ class DocscribeAnnotatorTest : BasePlatformTestCase() { assertNotNull(info) assertEquals("test.rb", info!!.filePath.substringAfterLast("/")) } + + fun testDoAnnotateReturnsNullWhenDocscribeNotAvailable() { + val annotator = DocscribeAnnotator() + val file = myFixture.configureByText("test.rb", "class Foo\nend") + val info = annotator.collectInformation(file)!! + // No docscribe gem in test env — should return null without throwing + val result = annotator.doAnnotate(info) + assertNull(result) + } + + fun testFileGenerationIncrementsOnNewAnnotation() { + DocscribeAnnotator.fileGeneration.clear() + val annotator = DocscribeAnnotator() + val file = myFixture.configureByText("test.rb", "class Foo\nend") + val info = annotator.collectInformation(file)!! + val filePath = info.filePath + + assertEquals(0L, DocscribeAnnotator.fileGeneration.getOrDefault(filePath, 0L)) + annotator.doAnnotate(info) + assertEquals(1L, DocscribeAnnotator.fileGeneration.getOrDefault(filePath, 0L)) + annotator.doAnnotate(info) + assertEquals(2L, DocscribeAnnotator.fileGeneration.getOrDefault(filePath, 0L)) + } + + fun testFileGenerationSeparatePerFile() { + DocscribeAnnotator.fileGeneration.clear() + val annotator = DocscribeAnnotator() + val file1 = myFixture.configureByText("foo.rb", "class Foo\nend") + val file2 = myFixture.configureByText("bar.rb", "class Bar\nend") + val info1 = annotator.collectInformation(file1)!! + val info2 = annotator.collectInformation(file2)!! + + annotator.doAnnotate(info1) + annotator.doAnnotate(info2) + annotator.doAnnotate(info1) + + assertEquals(2L, DocscribeAnnotator.fileGeneration.getOrDefault(info1.filePath, 0L)) + assertEquals(1L, DocscribeAnnotator.fileGeneration.getOrDefault(info2.filePath, 0L)) + } } diff --git a/src/test/kotlin/com/florexlabs/docscribe/runner/DocscribeRunnerTest.kt b/src/test/kotlin/com/florexlabs/docscribe/runner/DocscribeRunnerTest.kt index cc7cea6..9b48fb0 100644 --- a/src/test/kotlin/com/florexlabs/docscribe/runner/DocscribeRunnerTest.kt +++ b/src/test/kotlin/com/florexlabs/docscribe/runner/DocscribeRunnerTest.kt @@ -86,6 +86,26 @@ class DocscribeRunnerTest { } } + @Test + fun `DefaultCommandExecutor with environment does not throw`() { + val executor = DefaultCommandExecutor(mapOf("PATH" to "/test/bin:/usr/bin")) + val tempDir = createTempDirectory().toFile() + try { + File(tempDir, "Gemfile").writeText( + """ + source "https://rubygems.org" + gem "docscribe" + """.trimIndent(), + ) + DocscribeRunner.runDocscribe( + RunOptions(projectDir = tempDir.absolutePath, strategy = DocscribeStrategy.CHECK), + executor, + ) + } finally { + tempDir.deleteRecursively() + } + } + @Test fun `runDocscribe with no file uses no file arg`() { val executor = RecordingExecutor()