refactor: simplify register providers

[9romise]

This PR aims to make logic more reactive

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• ✅ Review completed - (🔄 Check again to review again)

📝 Walkthrough

Walkthrough

Adds a useActiveExtractor composable (with extractorEntries) to infer the active extractor from the editor document. Replaces registerDiagnosticCollection with a useDiagnostics hook that performs diagnostic collection. Refactors the extension entry point to register hover, completion and code-action providers by iterating extractorEntries and removes direct extractor instantiations. Deletes two exported pattern constants, adds a TypeScript path alias #composables/*, and adds a .vscode/settings.json spell‑check entry including npmx.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The pull request description 'make logic more reactive' is vague and generic, using non-descriptive terms that don't convey meaningful information about the specific changes.	Provide a more detailed description explaining what logic is being made reactive, why this refactoring improves the codebase, and summarise the key changes such as the introduction of composables and reactive patterns.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/composable

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

src/providers/diagnostics/index.ts (1)

68-90: Nested forEach with async callbacks creates fire-and-forget execution.

The dependencies.forEach(async ...) and enabledRules.value.forEach(async ...) pattern means collectDiagnostics returns immediately whilst async work continues in the background. Whilst the version check in flush prevents stale diagnostics, rapid document switches can cause:

1. Wasted API calls to getPackageInfo for documents no longer active
2. No way to await completion or track in-flight operations

Consider using for...of with early-exit checks, or an AbortController pattern if you want to cancel in-flight requests when the document changes.

♻️ Optional refactor with early-exit checks

-    dependencies.forEach(async (dep) => {
+    for (const dep of dependencies) {
+      if (document.version !== activeDocument.value?.version)
+        return
+
       try {
         const pkg = await getPackageInfo(dep.name)
         if (!pkg)
-          return
+          continue
 
-        enabledRules.value.forEach(async (rule) => {
+        for (const rule of enabledRules.value) {
           const diagnostic = await rule(dep, pkg)
 
           if (diagnostic) {
             diagnostics.push({
               source: displayName,
               range: extractor.getNodeRange(document, diagnostic.node),
               ...diagnostic,
             })
 
             flush(document, diagnostics)
           }
-        })
+        }
       } catch (err) {
         logger.warn(`Failed to check ${dep.name}: ${err}`)
       }
-    })
+    }

Note: This serialises processing which may slow down collection. If parallel execution is desired for performance, consider Promise.allSettled with an early version check before processing each result.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/providers/diagnostics/index.ts (2)

61-83: Async callbacks in forEach don't await – diagnostics may arrive out of order or be incomplete on rapid edits.

Using forEach with async callbacks fires all promises concurrently without awaiting them. Combined with the nested async forEach for rules, this means:

1. collectDiagnostics returns before any diagnostics are actually collected
2. If the document changes rapidly, a new collectDiagnostics call could clear diagnostics while the previous batch is still being fetched

The debounced flush mitigates some UI flicker, but consider using Promise.all with map for more predictable behaviour.

♻️ Suggested refactor using Promise.all

-    dependencies.forEach(async (dep) => {
+    await Promise.all(dependencies.map(async (dep) => {
       try {
         const pkg = await getPackageInfo(dep.name)
         if (!pkg)
           return

-        enabledRules.value.forEach(async (rule) => {
+        await Promise.all(enabledRules.value.map(async (rule) => {
           const diagnostic = await rule(dep, pkg)

           if (diagnostic) {
             diagnostics.push({
               source: displayName,
               range: extractor.getNodeRange(document, diagnostic.node),
               ...diagnostic,
             })

             flush()
           }
-        })
+        }))
       } catch (err) {
         logger.warn(`Failed to check ${dep.name}: ${err}`)
       }
-    })
+    }))

---

48-48: Consider: clearing diagnostics immediately may cause visual flicker.

Deleting diagnostics at the start of collectDiagnostics means users see a brief flash where all diagnostics disappear before new ones appear. An alternative is to build the new diagnostics set first and then replace atomically, though this adds complexity.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/providers/diagnostics/index.ts (2)

64-86: ⚠️ Potential issue | 🟠 Major

forEach with async callbacks does not await—diagnostics collection is fire-and-forget.

Using forEach with async callbacks means the iterations are not awaited. Each dep and rule callback fires independently, and collectDiagnostics returns before any diagnostics are actually collected. This can cause race conditions when the document text changes rapidly.

Consider using Promise.all with map or a for...of loop:

🐛 Proposed fix using Promise.all

-    dependencies.forEach(async (dep) => {
-      try {
-        const pkg = await getPackageInfo(dep.name)
-        if (!pkg)
-          return
-
-        enabledRules.value.forEach(async (rule) => {
-          const diagnostic = await rule(dep, pkg)
-
-          if (diagnostic) {
-            diagnostics.push({
-              source: displayName,
-              range: extractor.getNodeRange(document, diagnostic.node),
-              ...diagnostic,
-            })
-
-            flush()
-          }
-        })
-      } catch (err) {
-        logger.warn(`Failed to check ${dep.name}: ${err}`)
-      }
-    })
+    await Promise.all(dependencies.map(async (dep) => {
+      try {
+        const pkg = await getPackageInfo(dep.name)
+        if (!pkg)
+          return
+
+        const results = await Promise.all(enabledRules.value.map(rule => rule(dep, pkg)))
+        for (const diagnostic of results) {
+          if (diagnostic) {
+            diagnostics.push({
+              source: displayName,
+              range: extractor.getNodeRange(document, diagnostic.node),
+              ...diagnostic,
+            })
+          }
+        }
+        flush()
+      } catch (err) {
+        logger.warn(`Failed to check ${dep.name}: ${err}`)
+      }
+    }))

---

60-62: ⚠️ Potential issue | 🟠 Major

Debounced function created inside collectDiagnostics defeats the debouncing purpose.

Each invocation of collectDiagnostics creates a new debounced flush function with its own timer. When the watcher triggers rapid successive calls, each call gets an independent debounce, so they don't consolidate as intended.

Move flush outside collectDiagnostics or use a module-level debounced function with the document URI as a key.

🛠️ Suggested approach

+  const flush = debounce((uri: Uri, items: Diagnostic[]) => {
+    diagnosticCollection.set(uri, items)
+  }, 100)
+
   async function collectDiagnostics() {
     // ...
     const diagnostics: Diagnostic[] = []
-
-    const flush = debounce(() => {
-      diagnosticCollection.set(document.uri, [...diagnostics])
-    }, 100)
     
     // Inside the loop, call:
-    flush()
+    flush(document.uri, [...diagnostics])
   }

🧹 Nitpick comments (1)

src/providers/diagnostics/index.ts (1)

89-89: Consider adding cancellation for in-flight diagnostic collections.

When activeDocumentText changes rapidly (e.g., during typing), multiple collectDiagnostics calls may overlap. Without cancellation, older collections continue running and may overwrite results from newer collections.

Consider using an AbortController or a version counter to discard stale results.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/providers/diagnostics/index.ts (1)

64-86: ⚠️ Potential issue | 🟠 Major

Async forEach does not await callbacks, causing race conditions.

The nested forEach(async ...) pattern is fire-and-forget. The outer collectDiagnostics function returns immediately whilst async operations continue in the background. If the function is called again before prior operations complete (e.g., rapid document edits), multiple invocations will race and potentially intermingle diagnostics from different document states.

Consider using for...of with await for sequential processing, or Promise.all with map for parallel processing with proper completion tracking.

🛠️ Proposed fix using Promise.all for parallel processing

-    dependencies.forEach(async (dep) => {
-      try {
-        const pkg = await getPackageInfo(dep.name)
-        if (!pkg)
-          return
-
-        enabledRules.value.forEach(async (rule) => {
-          const diagnostic = await rule(dep, pkg)
-
-          if (diagnostic) {
-            diagnostics.push({
-              source: displayName,
-              range: extractor.getNodeRange(document, diagnostic.node),
-              ...diagnostic,
-            })
-
-            flush()
-          }
-        })
-      } catch (err) {
-        logger.warn(`Failed to check ${dep.name}: ${err}`)
-      }
-    })
+    await Promise.all(dependencies.map(async (dep) => {
+      try {
+        const pkg = await getPackageInfo(dep.name)
+        if (!pkg)
+          return
+
+        const results = await Promise.all(enabledRules.value.map(rule => rule(dep, pkg)))
+
+        for (const diagnostic of results) {
+          if (diagnostic) {
+            diagnostics.push({
+              source: displayName,
+              range: extractor.getNodeRange(document, diagnostic.node),
+              ...diagnostic,
+            })
+          }
+        }
+        flush()
+      }
+      catch (err) {
+        logger.warn(`Failed to check ${dep.name}: ${err}`)
+      }
+    }))

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/providers/diagnostics/index.ts (1)

64-86: ⚠️ Potential issue | 🟠 Major

Race condition: forEach with async callbacks does not await, causing concurrent mutations.

Array.forEach() ignores the returned promises, so all dependency checks run concurrently without coordination. The shared diagnostics array is mutated by multiple concurrent async callbacks, leading to:

1. Unpredictable ordering of diagnostics
2. If the document changes mid-collection, stale async work continues pushing to the old array
3. collectDiagnostics returns before work completes, so rapid document edits can interleave results

🛠️ Proposed fix using Promise.all with proper async/await

-    dependencies.forEach(async (dep) => {
+    await Promise.all(dependencies.map(async (dep) => {
       try {
         const pkg = await getPackageInfo(dep.name)
         if (!pkg)
           return
 
-        enabledRules.value.forEach(async (rule) => {
-          const diagnostic = await rule(dep, pkg)
-
-          if (diagnostic) {
-            diagnostics.push({
-              source: displayName,
-              range: extractor.getNodeRange(document, diagnostic.node),
-              ...diagnostic,
-            })
-
-            flush(document.uri, diagnostics)
-          }
-        })
+        const results = await Promise.all(
+          enabledRules.value.map(rule => rule(dep, pkg))
+        )
+
+        for (const diagnostic of results) {
+          if (diagnostic) {
+            diagnostics.push({
+              source: displayName,
+              range: extractor.getNodeRange(document, diagnostic.node),
+              ...diagnostic,
+            })
+          }
+        }
       } catch (err) {
         logger.warn(`Failed to check ${dep.name}: ${err}`)
       }
-    })
+    }))
+
+    flush(document.uri, diagnostics)

This also moves flush outside the loop, calling it once after all diagnostics are collected, which is more efficient and avoids redundant debounced calls.