Merge pull request #4170 from continuedev/dallin/docs-issues

sestinj · web-flow · commit 6c8f035c74e7 · 2025-02-15T10:10:27.000-08:00
Docs Fixes and Embeddings Provider Test Run
diff --git a/core/indexing/docs/DocsService.ts b/core/indexing/docs/DocsService.ts
@@ -157,7 +157,8 @@ export default class DocsService {
 
       const currentStatus = this.statuses.get(doc.startUrl);
       if (currentStatus) {
-        return currentStatus;
+        this.handleStatusUpdate(currentStatus);
+        return;
       }
 
       const sharedStatus = {
@@ -211,11 +212,7 @@ export default class DocsService {
     if (isAborted) {
       return true;
     }
-    // // Handle indexing disabled change mid-indexing
-    // if (this.config.disableIndexing) {
-    //   this.abort(startUrl);
-    //   return true;
-    // }
+
     // Handle embeddings provider change mid-indexing
     if (this.config.embeddingsProvider.embeddingId !== startedWithEmbedder) {
       this.abort(startUrl);
@@ -277,10 +274,6 @@ export default class DocsService {
       const oldConfig = this.config;
       this.config = newConfig; // IMPORTANT - need to set up top, other methods below use this without passing it in
 
-      // if (this.config.disableIndexing) {
-      //   return;
-      // }
-
       // No point in indexing if no docs context provider
       const hasDocsContextProvider = this.hasDocsContextProvider();
       if (!hasDocsContextProvider) {
@@ -368,25 +361,25 @@ export default class DocsService {
     siteIndexingConfig: SiteIndexingConfig,
     forceReindex: boolean = false,
   ): Promise<void> {
-    // if (this.config.disableIndexing) {
-    //   console.warn("Attempting to add/index docs when indexing is disabled");
-    //   return;
-    // }
     const { startUrl, useLocalCrawling, maxDepth } = siteIndexingConfig;
 
+    // First, if indexing is already in process, don't attempt
+    // This queue is necessary because indexAndAdd is invoked circularly by config edits
+    // TODO shouldn't really be a gap between adding and checking in queue but probably fine
+    if (this.docsIndexingQueue.has(startUrl)) {
+      return;
+    }
+
     const { isPreindexed, provider } =
       await this.getEmbeddingsProvider(startUrl);
     if (isPreindexed) {
       console.warn("Attempted to indexAndAdd pre-indexed doc");
       return;
     }
-
-    // Queue - indexAndAdd is invoked circularly by config edits. This prevents duplicate runs
-    if (this.docsIndexingQueue.has(startUrl)) {
-      return;
-    }
-
     const startedWithEmbedder = provider.embeddingId;
+
+    // Check if doc has been successfully indexed with the given embedder
+    // Note at this point we know it's not a pre-indexed doc
     const indexExists = await this.hasMetadata(startUrl);
 
     // Build status update - most of it is fixed values
@@ -404,22 +397,6 @@ export default class DocsService {
       url: siteIndexingConfig.startUrl,
     };
 
-    // Clear current indexes if reIndexing
-    if (indexExists) {
-      if (forceReindex) {
-        await this.deleteIndexes(startUrl);
-      } else {
-        this.handleStatusUpdate({
-          ...fixedStatus,
-          progress: 1,
-          description: "Complete",
-          status: "complete",
-          debugInfo: "Already indexed",
-        });
-        return;
-      }
-    }
-
     // If not force-reindexing and has failed with same config, don't reattempt
     if (!forceReindex) {
       const globalContext = new GlobalContext();
@@ -441,6 +418,31 @@ export default class DocsService {
       }
     }
 
+    if (indexExists && !forceReindex) {
+      this.handleStatusUpdate({
+        ...fixedStatus,
+        progress: 1,
+        description: "Complete",
+        status: "complete",
+        debugInfo: "Already indexed",
+      });
+      return;
+    }
+
+    // Do a test run on the embedder
+    // This particular failure will not mark as a failed config in global context
+    // Since SiteIndexingConfig is likely to be valid
+    try {
+      await provider.embed(["continue-test-run"]);
+    } catch (e) {
+      console.error("Failed to test embeddings connection", e);
+      void this.ide.showToast(
+        "error",
+        "Failed to test embeddings connection. check your embeddings model configuration",
+      );
+      return;
+    }
+
     const markFailedInGlobalContext = () => {
       const globalContext = new GlobalContext();
       const failedDocs = globalContext.get("failedDocs") ?? [];
@@ -463,6 +465,11 @@ export default class DocsService {
     try {
       this.docsIndexingQueue.add(startUrl);
 
+      // Clear current indexes if reIndexing
+      if (indexExists && forceReindex) {
+        await this.deleteIndexes(startUrl);
+      }
+
       this.addToConfig(siteIndexingConfig);
 
       this.handleStatusUpdate({
@@ -564,9 +571,10 @@ export default class DocsService {
         });
 
         try {
-          const subpathEmbeddings = await provider.embed(
-            article.chunks.map((c) => c.content),
-          );
+          const subpathEmbeddings =
+            article.chunks.length > 0
+              ? await provider.embed(article.chunks.map((c) => c.content))
+              : [];
           chunks.push(...article.chunks);
           embeddings.push(...subpathEmbeddings);
           const toWait = 100 * this.docsIndexingQueue.size + 50;
@@ -592,7 +600,6 @@ export default class DocsService {
         });
 
         void this.ide.showToast("info", `Failed to index ${startUrl}`);
-        this.docsIndexingQueue.delete(startUrl);
         markFailedInGlobalContext();
         return;
       }
@@ -635,11 +642,6 @@ export default class DocsService {
         favicon,
       });
 
-      this.docsIndexingQueue.delete(startUrl);
-
-      if (this.shouldCancel(startUrl, startedWithEmbedder)) {
-        return;
-      }
       this.handleStatusUpdate({
         ...fixedStatus,
         description: "Complete",
@@ -655,7 +657,11 @@ export default class DocsService {
 
       removeFromFailedGlobalContext();
     } catch (e) {
-      let description = `Error getting docs from: ${siteIndexingConfig.startUrl}`;
+      console.error(
+        `Error indexing docs at: ${siteIndexingConfig.startUrl}`,
+        e,
+      );
+      let description = `Error indexing docs at: ${siteIndexingConfig.startUrl}`;
       if (e instanceof Error) {
         if (
           e.message.includes("github.com") &&
@@ -664,7 +670,6 @@ export default class DocsService {
           description = "Github rate limit exceeded"; // This text is used verbatim elsewhere
         }
       }
-      console.error("Error indexing docs", e);
       this.handleStatusUpdate({
         ...fixedStatus,
         description,
@@ -916,7 +921,7 @@ export default class DocsService {
       // Anything found in old config, new config, AND sqlite that doesn't match should be reindexed
       // TODO if only favicon and title change, only update, don't embed
       // Otherwise anything found in new config that isn't in sqlite should be added/indexed
-      const newDocs: SiteIndexingConfig[] = [];
+      const addedDocs: SiteIndexingConfig[] = [];
       const changedDocs: SiteIndexingConfig[] = [];
       for (const doc of newConfigDocs) {
         const currentIndexedDoc = currentStartUrls.includes(doc.startUrl);
@@ -953,14 +958,14 @@ export default class DocsService {
             }
           }
         } else {
-          newDocs.push(doc);
+          addedDocs.push(doc);
           void Telemetry.capture("add_docs_config", { url: doc.startUrl });
         }
       }
 
       await Promise.allSettled([
         ...changedDocs.map((doc) => this.indexAndAdd(doc, true)),
-        ...newDocs.map((doc) => this.indexAndAdd(doc)),
+        ...addedDocs.map((doc) => this.indexAndAdd(doc)),
       ]);
 
       for (const doc of deletedDocs) {
diff --git a/gui/src/components/indexing/DocsIndexingStatus.tsx b/gui/src/components/indexing/DocsIndexingStatus.tsx
@@ -91,7 +91,6 @@ function DocsIndexingStatus({ docConfig }: IndexingStatusViewerProps) {
 
   return (
     <div className="mt-2 flex w-full flex-col">
-      {/* {`${status.type} - ${status.id} - ${status.status} - ${status.progress} - ${status.description} - ${status.icon}`} */}
       <div
         className={`flex flex-row items-center justify-between gap-2 text-sm`}
       >
diff --git a/gui/src/components/indexing/DocsIndexingStatuses.tsx b/gui/src/components/indexing/DocsIndexingStatuses.tsx
@@ -7,27 +7,47 @@ import { useAppSelector } from "../../redux/hooks";
 import { useContext, useMemo } from "react";
 import { ExclamationTriangleIcon } from "@heroicons/react/24/outline";
 import { IdeMessengerContext } from "../../context/IdeMessenger";
+import { IndexingStatus } from "core";
 
 function DocsIndexingStatuses() {
   const dispatch = useDispatch();
   const config = useAppSelector((store) => store.config.config);
   const ideMessenger = useContext(IdeMessengerContext);
+  const indexingStatuses = useAppSelector(
+    (store) => store.indexing.indexing.statuses,
+  );
 
   const hasDocsProvider = useMemo(() => {
     return !!config.contextProviders?.some(
       (provider) => provider.title === "docs",
     );
   }, [config]);
 
-  const configDocs = useMemo(() => {
-    return config.docs ?? [];
-  }, [config]);
+  // TODO - this might significantly impact performance during indexing
+  const sortedConfigDocs = useMemo(() => {
+    const sorter = (status: IndexingStatus["status"]) => {
+      // TODO - further sorting?
+      if (status === "indexing" || status === "paused") return 0;
+      if (status === "failed") return 1;
+      if (status === "aborted" || status === "pending") return 2;
+      return 3;
+    };
+
+    const docs = [...(config.docs ?? [])];
+    docs.sort((a, b) =>
+      sorter(indexingStatuses[b.startUrl]?.status ?? "pending") >
+      sorter(indexingStatuses[a.startUrl]?.status ?? "pending")
+        ? -1
+        : 1,
+    );
+    return docs;
+  }, [config, indexingStatuses]);
 
   return (
     <div className="flex flex-col gap-1">
       <div className="flex flex-row items-center justify-between">
         <h3 className="mb-0 mt-0 text-xl">@docs indexes</h3>
-        {configDocs.length ? (
+        {sortedConfigDocs.length ? (
           <SecondaryButton
             className="!my-0 flex h-7 flex-col items-center justify-center"
             onClick={() => {
@@ -41,7 +61,7 @@ function DocsIndexingStatuses() {
       </div>
       <span className="text-xs text-stone-500">
         {hasDocsProvider ? (
-          configDocs.length ? (
+          sortedConfigDocs.length ? (
             "Manage your documentation sources"
           ) : (
             "No docs yet"
@@ -72,7 +92,7 @@ function DocsIndexingStatuses() {
       </span>
       <div className="flex max-h-[170px] flex-col gap-1 overflow-y-auto overflow-x-hidden pr-2">
         <div>
-          {configDocs.length === 0 && (
+          {sortedConfigDocs.length === 0 && (
             <SecondaryButton
               className="flex h-7 flex-col items-center justify-center"
               onClick={() => {
@@ -84,7 +104,7 @@ function DocsIndexingStatuses() {
             </SecondaryButton>
           )}
         </div>
-        {configDocs.map((doc) => {
+        {sortedConfigDocs.map((doc) => {
           return <DocsIndexingStatus key={doc.startUrl} docConfig={doc} />;
         })}
       </div>

Original file line number	Diff line number	Diff line change
`@@ -91,7 +91,6 @@ function DocsIndexingStatus({ docConfig }: IndexingStatusViewerProps) {`
`91`	`91`
`92`	`92`	`return (`
`93`	`93`	`<div className="mt-2 flex w-full flex-col">`
`94`		- {/* {`${status.type} - ${status.id} - ${status.status} - ${status.progress} - ${status.description} - ${status.icon}`} */}
`95`	`94`	`<div`
`96`	`95`	className={`flex flex-row items-center justify-between gap-2 text-sm`}
`97`	`96`	`>`