ADFA-5403 | Fix LlmInferenceService resolution for Voice-to-Code - #93
ADFA-5403 | Fix LlmInferenceService resolution for Voice-to-Code#93jatezzz wants to merge 3 commits into
Conversation
…(ADFA-5403) context.services is a per-plugin registry that never holds it, so Voice-to-Code always inserted the raw transcript. Resolve per use, bound generation with a timeout, strip markdown fences, and target the open file's language, not Kotlin.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
hal-eisen-adfa
left a comment
hal-eisen-adfa
left a comment
There was a problem hiding this comment.
Automated review (Claude Code), high and medium findings only. Low-severity notes (duplicate fence stripper, LinkageError on SharedServices, no unit tests) are left out.
F09 (Medium) — line 52, outside the diff, so no inline comment is possible.
private var llmService: LlmInferenceService? = null is a plain non-volatile var, but this PR turns it into a lazily-populated cache. activate() writes it on the main thread; handleTranscript reads it from scope.launch on Dispatchers.IO. There is no happens-before edge, so the IO thread can keep seeing null and re-resolve on every transcript. recordingState in this same class is already @Volatile.
Resolve the LLM service through getPluginService as well, and drop the cached reference when AI Core unloads. Tune the generation request (system prompt, temperature, maxTokens) instead of scraping fences off the reply, size the timeout to that token budget, and await the future cancellably so plugin teardown unwinds it. Rewrite the fence stripper to handle a lead-in line, an unclosed fence and a one-line fenced reply. Guard logging against an uninitialized context during teardown.
|
@hal-eisen-adfa F09: fixed. |
hal-eisen-adfa
left a comment
hal-eisen-adfa
left a comment
There was a problem hiding this comment.
Second automated review pass (Claude Code, xhigh). All thirteen findings from the first pass are addressed in code and the plugin builds clean; thanks. Five new issues below, four of them in the code this round introduced.
F16 (the Speech-to-Text max_ide_version 26.30 vs ai-core min_ide_version 26.35 gap) is dropped — we do not enforce those ranges.
| logger?.warn("Code generation failed: ${response.error}") | ||
| null | ||
| } | ||
| } catch (e: CancellationException) { |
There was a problem hiding this comment.
F14 (High) — the F04 rethrow strands the toolbar on PROCESSING and drops the transcript.
withTimeoutOrNull only swallows its own TimeoutCancellationException. An external cancel arrives as a plain CancellationException, which this catch now rethrows, so the scope.launch body at line 332 dies before withContext(Dispatchers.Main) at line 339 — and its finally { setState(RecordingState.IDLE) } at line 351 never runs.
AI Core keeps one shared future: @Volatile currentGeneration (LlmInferenceServiceImpl.kt:24), and cancelGeneration() at :227-229 does currentGeneration?.cancel(true). The chat Stop button and AiCorePlugin.dispose() both call it, so a user stopping an unrelated AI Core chat cancels our future. The spinner then stays on ic_processing until the plugin reloads, no toast fires, and the dictated text is never inserted.
Before this PR the blocking .get() threw a CancellationException that the broad catch swallowed, so the raw transcript still reached the editor — this is a regression from the F04 fix. The rethrow itself is right; wrap the scope.launch body in try/finally so the state always returns to IDLE.
| val lines = raw.trim().lines() | ||
| // The fence can open on any line: a model often writes "Here is the code:" first. | ||
| val opening = lines.indexOfFirst { it.trimStart().startsWith(FENCE) } | ||
| if (opening < 0) return raw.trim() |
There was a problem hiding this comment.
F17 (High) — an unfenced reply is inserted into the file as if it were code.
opening < 0 returns raw.trim() unchanged. A refusal or a prose answer — I cannot write that code because ... — is non-blank, so generationFailed stays false, insertCodeAtCursor writes the English sentence into the open .kt file at the cursor, and the user is toasted "Voice input inserted at the cursor."
SuggestionProvider.sanitizeCompletion guards this with an isPreamble() filter plus a first-usable-line scan; this copy has neither. The exposure is higher now that "return only the code" moved from the user prompt into systemPrompt (line 375), because only backends that honour LlmConfig.systemPrompt apply it.
|
|
||
| val afterFence = lines[opening].trim().removePrefix(FENCE) | ||
| // A one-line reply closes on its opening line; otherwise that line is only the info string. | ||
| val firstCodeLine = if (afterFence.endsWith(FENCE)) { |
There was a problem hiding this comment.
F18 (High) — code on the opening fence line is dropped unless the whole block is one line.
firstCodeLine is computed only inside the afterFence.endsWith(FENCE) branch. For a reply that opens ```kotlin fun main() { and closes on a later line, the else branch sets firstCodeLine = "", so fun main() { is discarded and the editor receives println("hi") + } — an unbalanced fragment inserted at the cursor.
sanitizeCompletion (SuggestionProvider.kt:155-161) computes the opener unconditionally and prepends it when non-empty. Do the same: lift stripLanguageInfo(afterFence.removeSuffix(FENCE).trim()) out of the branch.
| logger?.info("AI Core went away - dropped the cached LlmInferenceService") | ||
| } | ||
|
|
||
| override fun deactivate(): Boolean { |
There was a problem hiding this comment.
F19 (High) — deactivate() does half of the F03 fix.
It destroys the recognizer only. It does not remove aiCoreLifecycleListener, does not clear llmService, and does not cancel scope.
- Disable the plugin while generation runs and, up to
GENERATION_TIMEOUT_SECONDS(132 s) later,insertCodeAtCursorwrites into the user's file from a plugin they just turned off. - The host keeps the listener (registered at line 97, removed only at line 175), so the deactivated instance still receives callbacks.
llmServicestill holds AI Core's router — the ClassLoader pin the KDoc at lines 150-152 says this design prevents.
GeminiPlugin removes its listener in both deactivate() (:184) and dispose() (:218, wrapped in runCatching).
| * @param fenceLine the opening fence line with its fence markers already removed | ||
| * @return the line without a leading language tag, empty when that was all it held | ||
| */ | ||
| private fun stripLanguageInfo(fenceLine: String): String = |
There was a problem hiding this comment.
F20 (Medium) — stripLanguageInfo deletes the first token of real code.
The test is only "is the first whitespace-delimited token in LANGUAGE_TAGS", and that set holds c, js, ts, py, text and code — all ordinary identifiers.
```c = a + b```→= a + b. The assignment target is gone and an invalid line is inserted.```bash -c "echo hi"```→-c "echo hi".
Inherited verbatim from SuggestionProvider.kt:187, where the result was ghost text the user could ignore. Here it is written into the file. Strip the tag only when the line really is an info string — e.g. require the tag to match the requested language, or require the remainder to be non-empty and the tag to be the whole first token of a line that held nothing else.
2 participants
Description
Updated
SpeechToTextPluginto resolve theLlmInferenceServicefrom the correct global registry. Previously, the plugin failed to find the service in the local context, causing Voice-to-Code to insert raw transcribed text instead of generated code. The service is now resolved fromSharedServicesfirst, with the localcontext.servicesused as a fallback. Resolution is dynamically retried on every use to account for parallel loading delays when the AI Core activates.Details
resolveLlmService()to cache successful lookups and retry upon failure.GENERATION_TIMEOUT_SECONDS) to prevent UI freezes.stripCodeFences()to remove markdown from the generated output so the editor receives raw code.Logto the plugin'scontext.logger.Screen_Recording_20260903_155612_Code.on.the.Go.mp4
Ticket
ADFA-5403
Parent: ADFA-5402