Address additional code review feedback

- Add validation for theme kind parameter in notification handler
- Document HighContrast theme handling (treated as dark)
- Prevent fallback detection from overriding explicit theme notifications
- Add flag to track whether theme was explicitly set

Co-authored-by: kv9898 <[email protected]>

Author: Copilot

apps/lsp/src/config.ts

@@ -137,6 +137,7 @@ export class ConfigurationManager extends Disposable {
   private _settings: Settings;
   private _logger: ILogger;
   private _activeColorThemeKind: "light" | "dark" = "dark";
+  private _themeExplicitlySet = false;
 
   constructor(
     private readonly connection_: Connection,
@@ -181,11 +182,13 @@ export class ConfigurationManager extends Disposable {
 
     // Fallback: try to detect theme from name if we haven't received an explicit notification yet
     // This is a best-effort approach for compatibility, but won't work with autoDetectColorScheme
-    // Default to dark theme if neither Light nor Dark is in the theme name
-    if (settings.workbench.colorTheme.includes("Light")) {
-      this._activeColorThemeKind = "light";
-    } else if (settings.workbench.colorTheme.includes("Dark")) {
-      this._activeColorThemeKind = "dark";
+    // Only apply fallback if theme hasn't been explicitly set via notification
+    if (!this._themeExplicitlySet) {
+      if (settings.workbench.colorTheme.includes("Light")) {
+        this._activeColorThemeKind = "light";
+      } else if (settings.workbench.colorTheme.includes("Dark")) {
+        this._activeColorThemeKind = "dark";
+      }
     }
 
     this._onDidChangeConfiguration.fire(this._settings);
@@ -217,6 +220,7 @@ export class ConfigurationManager extends Disposable {
   public setActiveColorThemeKind(kind: "light" | "dark") {
     if (this._activeColorThemeKind !== kind) {
       this._activeColorThemeKind = kind;
+      this._themeExplicitlySet = true;
       this._onDidChangeConfiguration.fire(this._settings);
     }
   }

apps/lsp/src/index.ts

@@ -225,9 +225,14 @@ connection.onInitialized(async () => {
   }
 
   // listen for color theme changes from the client
-  connection.onNotification("quarto/didChangeActiveColorTheme", (params: { kind: "light" | "dark" }) => {
+  connection.onNotification("quarto/didChangeActiveColorTheme", (params: { kind: string }) => {
     logger.logNotification('didChangeActiveColorTheme');
-    configManager.setActiveColorThemeKind(params.kind);
+    // Validate the theme kind before using it
+    if (params.kind === "light" || params.kind === "dark") {
+      configManager.setActiveColorThemeKind(params.kind);
+    } else {
+      logger.logError(`Invalid theme kind received: ${params.kind}`);
+    }
   });
 
   // initialize connection to quarto

apps/vscode/src/lsp/client.ts

@@ -156,6 +156,8 @@ export async function activateLsp(
   // Helper to send current theme to LSP server
   const sendThemeNotification = () => {
     if (client) {
+      // Map VS Code theme kinds to light/dark. HighContrast themes are treated as dark
+      // since they typically have light text on dark backgrounds
       const kind = window.activeColorTheme.kind === ColorThemeKind.Light ? "light" : "dark";
       client.sendNotification("quarto/didChangeActiveColorTheme", { kind });
     }