fix(clients): remove timeout limit for completion requests (TabbyML#1263

) * fix(agent): remove completion timeout limit. update slowResponseTime check rules. * fix(agent): fix response time health check sample window. * fix(vscode): show only status bar warning icon instead of notification for response time issues. * fix(intellij): show only status bar warning icon instead of notification for response time issues.
v2hack · Jan 22, 2024 · 84f27ce · 84f27ce
1 parent 474992e
commit 84f27ce
Show file tree

Hide file tree

Showing 9 changed files with 293 additions and 163 deletions.
diff --git a/clients/intellij/node_scripts/tabby-agent.js b/clients/intellij/node_scripts/tabby-agent.js
diff --git a/clients/intellij/node_scripts/wasm/LICENSES b/clients/intellij/node_scripts/wasm/LICENSES
@@ -0,0 +1,153 @@
+tree-sitter.wasm (https://github.com/tree-sitter/tree-sitter)
+
+The MIT License (MIT)
+
+Copyright (c) 2018-2023 Max Brunsfeld
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+
+---
+
+tree-sitter-go.wasm (https://github.com/tree-sitter/tree-sitter-go)
+
+The MIT License (MIT)
+
+Copyright (c) 2014 Max Brunsfeld
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+
+---
+
+tree-sitter-python.wasm (https://github.com/tree-sitter/tree-sitter-python)
+
+The MIT License (MIT)
+
+Copyright (c) 2016 Max Brunsfeld
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+
+---
+
+tree-sitter-ruby.wasm (https://github.com/tree-sitter/tree-sitter-ruby)
+
+The MIT License (MIT)
+
+Copyright (c) 2016 Rob Rix
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+
+---
+
+tree-sitter-rust.wasm (https://github.com/tree-sitter/tree-sitter-rust)
+
+The MIT License (MIT)
+
+Copyright (c) 2017 Maxim Sokolov
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+
+---
+
+tree-sitter-tsx.wasm (https://github.com/tree-sitter/tree-sitter-typescript)
+
+The MIT License (MIT)
+
+Copyright (c) 2017 GitHub
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
diff --git a/clients/intellij/src/main/kotlin/com/tabbyml/intellijtabby/actions/CheckIssueDetail.kt b/clients/intellij/src/main/kotlin/com/tabbyml/intellijtabby/actions/CheckIssueDetail.kt
@@ -1,5 +1,6 @@
 package com.tabbyml.intellijtabby.actions
 
+import com.intellij.icons.AllIcons
 import com.intellij.openapi.actionSystem.*
 import com.intellij.openapi.application.invokeLater
 import com.intellij.openapi.components.service
@@ -170,6 +171,11 @@ class CheckIssueDetail : AnAction() {
       muted += listOf("slowCompletionResponseTime", "highCompletionTimeoutRate")
     }
     e.presentation.isVisible = agentService.currentIssue.value != null && agentService.currentIssue.value !in muted
+    e.presentation.icon = if (agentService.currentIssue.value == "connectionFailed") {
+      AllIcons.General.Error
+    } else {
+      AllIcons.General.Warning
+    }
   }
 
   override fun getActionUpdateThread(): ActionUpdateThread {

diff --git a/clients/intellij/src/main/kotlin/com/tabbyml/intellijtabby/agent/AgentService.kt b/clients/intellij/src/main/kotlin/com/tabbyml/intellijtabby/agent/AgentService.kt
@@ -47,8 +47,6 @@ class AgentService : Disposable {
   var issueNotification: Notification? = null
     private set
 
-  private var completionResponseWarningShown = false
-
   enum class Status {
     INITIALIZING,
     INITIALIZATION_FAILED,
@@ -177,7 +175,6 @@ class AgentService : Disposable {
     scope.launch {
       agent.status.collect { status ->
         if (status == Agent.Status.READY) {
-          completionResponseWarningShown = false
           invokeLater {
             issueNotification?.expire()
           }
@@ -187,22 +184,13 @@ class AgentService : Disposable {
 
     scope.launch {
       agent.currentIssue.collect { issueName ->
-        val showCompletionResponseWarnings = !completionResponseWarningShown &&
-            !settings.notificationsMuted.contains("completionResponseTimeIssues")
-        val message = when (issueName) {
-          "connectionFailed" -> "Cannot connect to Tabby server"
-          "slowCompletionResponseTime" -> if (showCompletionResponseWarnings) {
-            completionResponseWarningShown = true
-            "Completion requests appear to take too much time"
-          } else {
-            return@collect
-          }
-
-          "highCompletionTimeoutRate" -> if (showCompletionResponseWarnings) {
-            completionResponseWarningShown = true
-            "Most completion requests timed out"
-          } else {
-            return@collect
+        val notification = when (issueName) {
+          "connectionFailed" -> Notification(
+            "com.tabbyml.intellijtabby.notification.warning",
+            "Cannot connect to Tabby server",
+            NotificationType.ERROR,
+          ).apply {
+            addAction(ActionManager.getInstance().getAction("Tabby.CheckIssueDetail"))
           }
 
           else -> {
@@ -212,22 +200,6 @@ class AgentService : Disposable {
             return@collect
           }
         }
-        val notification = Notification(
-          "com.tabbyml.intellijtabby.notification.warning",
-          message,
-          NotificationType.WARNING,
-        )
-        notification.addAction(ActionManager.getInstance().getAction("Tabby.CheckIssueDetail"))
-        if (issueName in listOf("slowCompletionResponseTime", "highCompletionTimeoutRate")) {
-          notification.addAction(
-            object : AnAction("Don't Show Again") {
-              override fun actionPerformed(e: AnActionEvent) {
-                issueNotification?.expire()
-                settings.notificationsMuted += listOf("completionResponseTimeIssues")
-              }
-            }
-          )
-        }
         invokeLater {
           issueNotification?.expire()
           issueNotification = notification

diff --git a/clients/tabby-agent/src/AgentConfig.ts b/clients/tabby-agent/src/AgentConfig.ts
@@ -19,7 +19,6 @@ export type AgentConfig = {
       mode: "adaptive" | "fixed";
       interval: number;
     };
-    timeout: number;
   };
   postprocess: {
     limitScope: {
@@ -60,7 +59,7 @@ export const defaultAgentConfig: AgentConfig = {
     endpoint: "http://localhost:8080",
     token: "",
     requestHeaders: {},
-    requestTimeout: 30000, // 30s
+    requestTimeout: 2 * 60 * 1000, // 2 minutes
   },
   completion: {
     prompt: {
@@ -76,7 +75,6 @@ export const defaultAgentConfig: AgentConfig = {
       mode: "adaptive",
       interval: 250, // ms
     },
-    timeout: 4000, // ms
   },
   postprocess: {
     limitScope: {

diff --git a/clients/tabby-agent/src/CompletionProviderStats.ts b/clients/tabby-agent/src/CompletionProviderStats.ts
@@ -62,11 +62,11 @@ export class CompletionProviderStats {
     checks: {
       disable: false,
       // Mark status as healthy if the latency is less than the threshold for each latest windowSize requests.
-      healthy: { windowSize: 3, latency: 2400 },
+      healthy: { windowSize: 1, latency: 3000 },
       // If there is at least {count} requests, and the average response time is higher than the {latency}, show warning
-      slowResponseTime: { latency: 3200, count: 3 },
+      slowResponseTime: { latency: 5000, count: 1 },
       // If there is at least {count} timeouts, and the timeout rate is higher than the {rate}, show warning
-      highTimeoutRate: { rate: 0.5, count: 3 },
+      highTimeoutRate: { rate: 0.5, count: 1 },
     },
   };
 
@@ -83,12 +83,6 @@ export class CompletionProviderStats {
 
   private recentCompletionRequestLatencies: Windowed = new Windowed(this.config.windowSize);
 
-  updateConfigByRequestTimeout(timeout: number) {
-    this.config.checks.healthy.latency = timeout * 0.6;
-    this.config.checks.slowResponseTime.latency = timeout * 0.8;
-    this.resetWindowed();
-  }
-
   add(value: CompletionProviderStatsEntry): void {
     const { triggerMode, cacheHit, aborted, requestSent, requestLatency, requestCanceled, requestTimeout } = value;
     if (!aborted) {
@@ -199,7 +193,7 @@ export class CompletionProviderStats {
 
     if (
       latencies
-        .slice(-Math.max(this.config.windowSize, config.healthy.windowSize))
+        .slice(-Math.min(this.config.windowSize, config.healthy.windowSize))
         .every((latency) => latency < config.healthy.latency)
     ) {
       return "healthy";

diff --git a/clients/tabby-agent/src/TabbyAgent.ts b/clients/tabby-agent/src/TabbyAgent.ts
@@ -110,12 +110,6 @@ export class TabbyAgent extends EventEmitter implements Agent {
       }
     }
 
-    if (oldConfig.completion.timeout !== this.config.completion.timeout) {
-      this.completionProviderStats.updateConfigByRequestTimeout(this.config.completion.timeout);
-      this.popIssue("slowCompletionResponseTime");
-      this.popIssue("highCompletionTimeoutRate");
-    }
-
     const event: AgentEvent = { event: "configUpdated", config: this.config };
     this.logger.debug({ event }, "Config updated");
     super.emit("configUpdated", event);
@@ -496,10 +490,7 @@ export class TabbyAgent extends EventEmitter implements Agent {
                 segments,
                 user: this.auth?.user,
               },
-              signal: this.createAbortSignal({
-                signal,
-                timeout: this.config.completion.timeout,
-              }),
+              signal: this.createAbortSignal({ signal }),
             };
             this.logger.debug(
               { requestId, requestOptions, url: this.config.server.endpoint + requestPath },

diff --git a/clients/tabby-agent/src/configFile.ts b/clients/tabby-agent/src/configFile.ts
@@ -27,12 +27,6 @@ const configTomlTemplate = `## Tabby agent configuration file
 # Header1 = "Value1" # list your custom headers here
 # Header2 = "Value2" # values can be strings, numbers or booleans
 
-## Completion
-## (Since 1.1.0) You can set the completion request timeout here.
-## Note that there is also a timeout config at the server side.
-# [completion]
-# timeout = 4000 # 4s
-
 ## Logs
 ## You can set the log level here. The log file is located at ~/.tabby-client/agent/logs/.
 # [logs]
@@ -65,7 +59,6 @@ const typeCheckSchema: Record<string, string> = {
   "completion.debounce": "object",
   "completion.debounce.mode": "string",
   "completion.debounce.interval": "number",
-  "completion.timeout": "number",
   postprocess: "object",
   "postprocess.limitScopeByIndentation": "object",
   "postprocess.limitScopeByIndentation.experimentalKeepBlockScopeWhenCompletingLine": "boolean",

diff --git a/clients/vscode/src/TabbyStatusBarItem.ts b/clients/vscode/src/TabbyStatusBarItem.ts
@@ -22,7 +22,6 @@ export class TabbyStatusBarItem {
   private item = window.createStatusBarItem(StatusBarAlignment.Right);
   private extensionContext: ExtensionContext;
   private completionProvider: TabbyCompletionProvider;
-  private completionResponseWarningShown = false;
 
   private subStatusForReady = [
     {
@@ -158,23 +157,11 @@ export class TabbyStatusBarItem {
       console.debug("Tabby agent issuesUpdated", { event });
       const status = agent().getStatus();
       this.fsmService.send(status);
-      const showCompletionResponseWarnings =
-        !this.completionResponseWarningShown &&
-        !this.extensionContext.globalState
-          .get<string[]>("notifications.muted", [])
-          .includes("completionResponseTimeIssues");
       if (event.issues.includes("connectionFailed")) {
-        // Only show this notification when user modifies the settings, do not show it when initializing
-        // FIXME: refactor this use a flag marks the event is trigger by modifying settings or initializing
+        // Do not show it when initializing
         if (status !== "notInitialized") {
           notifications.showInformationWhenDisconnected();
         }
-      } else if (showCompletionResponseWarnings && event.issues.includes("highCompletionTimeoutRate")) {
-        this.completionResponseWarningShown = true;
-        notifications.showInformationWhenHighCompletionTimeoutRate();
-      } else if (showCompletionResponseWarnings && event.issues.includes("slowCompletionResponseTime")) {
-        this.completionResponseWarningShown = true;
-        notifications.showInformationWhenSlowCompletionResponseTime();
       }
     });
   }