fix: chat filter being cleared by lucaseduoli · Pull Request #618 · langflow-ai/openrag

lucaseduoli · 2025-12-05T19:35:53Z

This PR fixes the problem where the chat filter is cleared right after onboarding.

Closes #612

Copilot

Pull request overview

This PR fixes an issue where the chat filter was being cleared immediately after onboarding by adjusting the order of operations and improving the onboarding completion tracking across the application.

Key Changes:

Reordered filter setup to occur before starting a new conversation in the onboarding flow
Added onboarding completion tracking in the chat context to better control query execution
Improved error message handling in provider validation to show more helpful error details
Enhanced task tracking in file upload to prevent premature completion

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
frontend/contexts/chat-context.tsx	Added onboarding completion state tracking and improved filter preservation logic in `startNewConversation`
frontend/components/chat-renderer.tsx	Reordered operations to set default filter before calling `startNewConversation`
frontend/app/onboarding/_components/onboarding-upload.tsx	Enhanced task tracking by matching specific upload task IDs instead of counting all tasks
frontend/app/onboarding/_components/onboarding-card.tsx	Added provider-specific checks to `alreadyConfigured` prop for OpenAI, WatsonX, and Ollama
frontend/app/chat/page.tsx	Added conversation loading check and onboarding completion gating for nudges query
frontend/lib/upload-utils.ts	Added `taskId` field to upload result for better task tracking
frontend/app/api/queries/useProviderHealthQuery.ts	Added onboarding completion and active ingestion checks to prevent health checks during setup
frontend/app/api/queries/useGetNudgesQuery.ts	Added onboarding completion and LLM health checks to prevent nudges during errors
frontend/app/api/queries/useGetConversationsQuery.ts	Added onboarding completion check to prevent conversation fetching during setup
frontend/components/provider-health-banner.tsx	Improved error message display to avoid showing duplicates when LLM and embedding have same error
frontend/app/onboarding/_components/ibm-onboarding.tsx	Code formatting changes only (tabs instead of spaces)
src/api/provider_validation.py	Added helper functions for parsing error messages and improved error handling to preserve original error details

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-05T22:14:52Z

+  }, []);
+
+  const setOnboardingComplete = useCallback((complete: boolean) => {
+    setIsOnboardingComplete(complete);


The setOnboardingComplete callback (lines 145-147) is defined with useCallback but has an empty dependency array. This is correct since it only calls setIsOnboardingComplete which is a state setter. However, this callback is exposed in the context (line 424) but is not actually used to update localStorage. When setOnboardingComplete(true) is called (e.g., line 173 in chat-renderer.tsx), it only updates the local state but doesn't clear the ONBOARDING_STEP_KEY from localStorage. This could cause inconsistency between the context state and localStorage. Consider having this callback also update localStorage when appropriate.

Suggested change

setIsOnboardingComplete(complete);

setIsOnboardingComplete(complete);

if (typeof window !== "undefined") {

if (complete) {

localStorage.removeItem(ONBOARDING_STEP_KEY);

} else {

localStorage.setItem(ONBOARDING_STEP_KEY, "incomplete");

}

}

Copilot · 2025-12-05T22:14:52Z

+	const lastLoadedConversationRef = useRef<string | null>(null);
+	const { addTask } = useTask();
+
+  console.log(endpoint, refreshTrigger);


This console.log statement appears to be leftover debug code and should be removed. The parameters endpoint and refreshTrigger are not being used in any meaningful way here.

Suggested change

console.log(endpoint, refreshTrigger);

Copilot · 2025-12-05T22:14:52Z

+  }, [
+    tasks,
+    currentStep,
+    onComplete,
+    refetchNudges,
+    shouldCreateFilter,
+    uploadedFilename,
+    uploadedTaskId,
+    createFilterMutation,
+    isCreatingFilter,
+  ]);


The dependency array is missing isCreatingFilter as a dependency. While createFilterMutation is included, the isCreatingFilter state is being checked within the effect (line 122), so it should be included in the dependencies to ensure the effect has access to the current value.

Copilot · 2025-12-05T22:14:53Z

+  // Include hasChatError in queryKey so React Query refetches when it changes
+  // This ensures the health check runs with test_completion=true when chat errors occur
+  const testCompletion = params?.test_completion ?? hasChatError;
+  const queryKey = ["provider", "health", testCompletion, hasChatError];


The query key includes hasChatError (line 118) but this value is also used in the enabled condition (line 160-164). When hasChatError changes, React Query will refetch due to the changed key, but the component will also re-render due to the context update. This could cause redundant fetches. Consider removing hasChatError from the query key since testCompletion already captures its value.

Suggested change

const queryKey = ["provider", "health", testCompletion, hasChatError];

const queryKey = ["provider", "health", testCompletion];

Copilot · 2025-12-05T22:14:53Z

+				(!!debouncedEndpoint && !!debouncedApiKey && !!debouncedProjectId) ||
+				getFromEnv ||


The enabled property in the query options (line 102-105) requires that all 3 fields (endpoint, apiKey, projectId) are populated OR getFromEnv is true OR alreadyConfigured is true. However, when getFromEnv is true, the apiKey will be empty (line 98), so the condition !!debouncedApiKey will be false. This means the query will only be enabled when getFromEnv || alreadyConfigured is true, not when all 3 fields are filled. Consider revising the condition to: (!!debouncedEndpoint && (getFromEnv || !!debouncedApiKey) && !!debouncedProjectId) || alreadyConfigured

Suggested change

(!!debouncedEndpoint && !!debouncedApiKey && !!debouncedProjectId) ||

getFromEnv ||

(!!debouncedEndpoint && (getFromEnv || !!debouncedApiKey) && !!debouncedProjectId) ||

Copilot · 2025-12-05T22:14:54Z

+        # If the error message contains JSON, parse it to extract just the message
+        error_str = str(e)
+        if "IBM Watson API error: " in error_str:
+            json_part = error_str.split("IBM Watson API error: ", 1)[1]
+            parsed_message = _parse_json_error_message(json_part)
+            if parsed_message != json_part:
+                raise Exception(f"IBM Watson API error: {parsed_message}")


Lines 573-574 and 588-594 contain duplicate logic for parsing JSON error messages, similar to the pattern in the completion test. The exception handler re-parses error messages that were already parsed in line 574. Consider removing this duplicate parsing logic.

Suggested change

# If the error message contains JSON, parse it to extract just the message

error_str = str(e)

if "IBM Watson API error: " in error_str:

json_part = error_str.split("IBM Watson API error: ", 1)[1]

parsed_message = _parse_json_error_message(json_part)

if parsed_message != json_part:

raise Exception(f"IBM Watson API error: {parsed_message}")

Copilot · 2025-12-05T22:14:55Z

    // Force immediate refresh to ensure sidebar shows correct state
    refreshConversations(true);
-  }, [endpoint, refreshConversations]);
+  }, [endpoint, refreshConversations, conversationData, placeholderConversation]);


[nitpick] The dependency array for startNewConversation includes conversationData and placeholderConversation (line 335), but these are checked inside the function (lines 265-267). This means the callback will be recreated whenever these values change, which could cause unnecessary re-renders of child components that use this callback. Since these are only used for conditional logic inside the function, they don't need to be in the dependency array unless the callback's behavior should change when they change. Consider if this is intentional or if the dependency array should be simplified.

Suggested change

}, [endpoint, refreshConversations, conversationData, placeholderConversation]);

}, [endpoint, refreshConversations]);

Copilot · 2025-12-05T22:14:55Z

+          console.log("[CONVERSATION] No default filter set, preserving existing filter if any");
+          // Don't clear the filter - it may have been set by storeDefaultFilterForNewConversations
+        }
      }


The logic on lines 302-313 has an issue: when there's no default filter and no existing conversation (first time after onboarding), the filter is preserved. However, this is inside the if (typeof window !== "undefined") block which checks localStorage. If window is undefined (server-side rendering), the entire block is skipped and the filter behavior is undefined. Consider adding an else clause after line 314 to handle the server-side case explicitly, or document that this only runs client-side.

Suggested change

}

}

} else {

// Server-side rendering: filter logic is skipped.

// Explicitly preserve existing filter state or set to null as needed.

// For SSR, we choose to preserve the existing filter state.

console.log("[CONVERSATION] SSR: preserving existing filter state");

// Optionally, you could setConversationFilterState(null) here if desired.

Copilot · 2025-12-05T22:14:55Z

+														(messages[index - 2]?.content.match(
+															FILES_REGEX,
+														)?.[0] ??
+															undefined) &&
+														message.content === FILE_CONFIRMATION
+															? undefined
+															: message.content
+													}
+													files={
+														index >= 2
+															? (messages[index - 2]?.content.match(
+																	FILES_REGEX,
+																)?.[0] ?? undefined)


This use of variable 'undefined' always evaluates to false.

Suggested change

(messages[index - 2]?.content.match(

FILES_REGEX,

)?.[0] ??

undefined) &&

message.content === FILE_CONFIRMATION

? undefined

: message.content

}

files={

index >= 2

? (messages[index - 2]?.content.match(

FILES_REGEX,

)?.[0] ?? undefined)

messages[index - 2]?.content.match(FILES_REGEX)?.[0] != null &&

message.content === FILE_CONFIRMATION

? undefined

: message.content

}

files={

index >= 2

? messages[index - 2]?.content.match(FILES_REGEX)?.[0]

Copilot · 2025-12-05T22:14:56Z

+            # Generic format: {"detail": "..."}
+            if "detail" in error_data:
+                return error_data["detail"]
+    except (json.JSONDecodeError, ValueError, TypeError):


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except (json.JSONDecodeError, ValueError, TypeError):

except (json.JSONDecodeError, ValueError, TypeError):

# JSON parsing errors are expected if the input is not valid JSON.

# In this case, we fall back to returning the original error text.

edwinjosechittilappilly · 2025-12-05T22:59:35Z

@phact can you take a stab at the conflict?
While I review other PRs?

fixed chat filter being cleared after onboarding

1eda6c8

lucaseduoli requested a review from phact December 5, 2025 19:35

lucaseduoli self-assigned this Dec 5, 2025

lucaseduoli linked an issue Dec 5, 2025 that may be closed by this pull request

Can someone repro: inconsistencies in initial filter creation and application immediately after onboarding #589

Closed

phact requested a review from Copilot December 5, 2025 22:09

Copilot started reviewing on behalf of phact December 5, 2025 22:09 View session

Copilot AI reviewed Dec 5, 2025

View reviewed changes

phact approved these changes Dec 5, 2025

View reviewed changes

Merge branch 'main' into fix/chat_filter_clearing

ce10191

phact merged commit d908174 into main Dec 5, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: chat filter being cleared#618

fix: chat filter being cleared#618
phact merged 2 commits intomainfrom
fix/chat_filter_clearing

lucaseduoli commented Dec 5, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

Copilot AI Dec 5, 2025

Uh oh!

edwinjosechittilappilly commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

-    setIsOnboardingComplete(complete);
+    setIsOnboardingComplete(complete);
+    if (typeof window !== "undefined") {
+      if (complete) {
+        localStorage.removeItem(ONBOARDING_STEP_KEY);
+      } else {
+        localStorage.setItem(ONBOARDING_STEP_KEY, "incomplete");
+      }
+    }

	const queryKey = ["provider", "health", testCompletion, hasChatError];
	const queryKey = ["provider", "health", testCompletion];

		(!!debouncedEndpoint && !!debouncedApiKey && !!debouncedProjectId) \|\|
		getFromEnv \|\|

	(!!debouncedEndpoint && !!debouncedApiKey && !!debouncedProjectId) \|\|
	getFromEnv \|\|
	(!!debouncedEndpoint && (getFromEnv \|\| !!debouncedApiKey) && !!debouncedProjectId) \|\|

	}, [endpoint, refreshConversations, conversationData, placeholderConversation]);
	}, [endpoint, refreshConversations]);

-      }
+      }
+    } else {
+      // Server-side rendering: filter logic is skipped.
+      // Explicitly preserve existing filter state or set to null as needed.
+      // For SSR, we choose to preserve the existing filter state.
+      console.log("[CONVERSATION] SSR: preserving existing filter state");
+      // Optionally, you could setConversationFilterState(null) here if desired.

Conversation

lucaseduoli commented Dec 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

edwinjosechittilappilly commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants