Feature/enhanced rag validation by jaffrey-deepsource · Pull Request #10 · CyberdyneHQ/dify

jaffrey-deepsource · 2026-02-11T13:34:53Z

No description provided.

- Add validation for dataset retrieval config in backend - Add relevance score calculation helper - Add query input validation - Add frontend validation component for RAG pipeline settings

- Add document filtering and scoring helpers in backend - Add frontend validation utilities and hooks - Add pipeline validator class for enhanced checks

- Add rerank parameter validation and scoring in rerank_base.py - Add embedding input validation and similarity computation in embedding_base.py - Add text splitter parameter validation and chunk estimation in text_splitter.py - Add dataset config validation and statistics computation in dataset_service.py

deepsource-development · 2026-02-11T13:35:59Z

DeepSource Code Review

DeepSource reviewed changes in the commit range b76c8fa..79fceb0 on this pull request. Below is the summary for the review, and you can see the individual issues we found as review comments.

For detailed review results, please see the PR on DeepSource ↗

PR Report Card

Security	× 5 issues	Overall PR Quality Focus Area: Reliability Guidance Fix the 5 critical issues with `redis_client.get()` assigning None in `api/services/dataset_service.py`. Grade capped at D due to multiple critical issues
Reliability	× 19 issues
Complexity	× 30 issues
Hygiene	× 21 issues

Code Review Summary

Analyzer	Summary	Details
JavaScript	10 new issues detected.	Review ↗
Python	66 new issues detected.	Review ↗
Secrets	No new issues detected.	Review ↗

How are these analyzer statuses calculated?

Administrators can configure which issue categories are reported and cause analysis to be marked as failed when detected. This helps prevent bad and insecure code from being introduced in the codebase. If you're an administrator, you can modify this in the repository's settings.

💡 If you're a repository administrator, you can configure the quality gates from the settings.

deepsource-development · 2026-02-11T13:38:03Z

+            raise ValueError("Top K cannot exceed 100 to prevent excessive resource usage")
+
+        # Validate score threshold if enabled
+        if retrieve_config.score_threshold_enabled:


Nested `if` can be merged using `and` for clarity

Nested if statements like if retrieve_config.score_threshold_enabled: can be collapsed into a single condition using the and operator when multiple conditions exist. This reduces nesting complexity and improves code clarity.

Merge the nested if conditions into a single line using and to enhance readability and maintainability.

deepsource-development · 2026-02-11T13:38:03Z

+            if not rerank_model.reranking_provider_name or not rerank_model.reranking_model_name:
+                raise ValueError("Reranking model must be fully specified when reranking is enabled")
+
+    def _calculate_relevance_score(self, documents: list[Document], query: str) -> float:


Instance method without `self` use wastes memory

The _calculate_relevance_score method doesn't use its self parameter or any instance data, resulting in a bound method created per object instance which is inefficient. This can increase memory use and call overhead unnecessarily.

Decorate _calculate_relevance_score with @staticmethod to avoid binding the method to instance objects and reduce memory and CPU overhead.

deepsource-development · 2026-02-11T13:38:03Z

+
+        return total_score / len(documents)
+
+    def _validate_query_input(self, query: str, inputs: Mapping[str, Any] | None) -> None:


Instance method lacks `@staticmethod`, causing overhead

The method _validate_query_input does not reference the instance (self), which means it unnecessarily creates a bound method for each class instance, wasting memory and CPU cycles. This impacts performance especially if the class is instantiated frequently.

Add the @staticmethod decorator to _validate_query_input to avoid binding to the instance and improve efficiency without changing how the method is called.

deepsource-development · 2026-02-11T13:38:03Z

+                if value is None:
+                    raise ValueError(f"Input '{key}' cannot be None")
+
+    def _filter_documents_by_score(self, documents: list[Document], min_score: float) -> list[Document]:


Instance method unused, wasting memory and computation

Defining _filter_documents_by_score as an instance method incurs overhead by creating a bound method for each class instance without using instance state. This wastes memory and CPU resources.

Add the @staticmethod decorator to _filter_documents_by_score to define it as a static method, avoiding instance binding and improving performance.

deepsource-development · 2026-02-11T13:38:03Z

+        """
+        return [doc for doc in documents if doc.score >= min_score]
+
+    def _compute_average_score(self, documents: list[Document]) -> float:


Instance method without `self` uses more memory and CPU

The _compute_average_score method accepts self but does not use any instance attributes or methods, leading to unnecessary bound method creation for each class instance. This consumes extra memory and processing time at runtime.

Add the @staticmethod decorator to _compute_average_score and remove the self parameter to avoid this overhead and clarify that the method does not rely on instance state.

deepsource-development · 2026-02-11T13:38:03Z

-            db.session.commit()
-        except Exception as e:
            db.session.rollback()
            raise e


Undefined variable 'e'

The variable name is not defined where it is used.
This will lead to an error during the runtime.
Make sure there is no typo. If the name was supposed to be imported, verify that you've actually imported the name.

deepsource-development · 2026-02-11T13:38:03Z

+    def compute_document_similarity(doc1: DocumentContext, doc2: DocumentContext) -> float:
+        if not doc1 or not doc2:
+            return 0.0
+
+        # Simple similarity based on content length difference
+        len1 = len(doc1.content) if doc1.content else 0
+        len2 = len(doc2.content) if doc2.content else 0
+        return 1.0 / (1.0 + abs(len1 - len2))


`compute_document_similarity` only compares content lengths, not content

The compute_document_similarity function bases its calculation only on the length of document content, which can be misleading. For example, two completely different documents are rated as identical if they have the same character count, potentially causing incorrect behavior in features relying on this score.

Rename the function to be more descriptive, like compute_length_based_similarity, or implement a content-based similarity algorithm such as Jaccard similarity to accurately reflect document content similarity.

deepsource-development · 2026-02-11T13:38:04Z

+        total_score = 0.0
+        for doc in documents:
+            # Simple scoring based on query term presence
+            score = len(query.split()) / len(doc.page_content.split()) if doc.page_content else 0.0


`len(doc.page_content.split())` can be zero, causing a crash

The scoring logic at len(query.split()) / len(doc.page_content.split()) is vulnerable to a ZeroDivisionError. If doc.page_content contains only whitespace characters (e.g., " "), split() will produce an empty list, causing its length to be zero and triggering an unhandled exception, leading to a denial of service.

Add a check to ensure the length of doc.page_content.split() is not zero before performing the division. For example: page_content_words = doc.page_content.split(); score = len(query.split()) / len(page_content_words) if page_content_words else 0.0.

deepsource-development · 2026-02-11T13:38:04Z

+        effective_size = chunk_size - overlap
+        return (text_length - overlap) // effective_size + 1


Division by `effective_size` which can be zero or negative

The calculation of effective_size does not handle cases where self._chunk_size is less than or equal to self._chunk_overlap. This can lead to a ZeroDivisionError if they are equal, or incorrect negative chunk counts if overlap is larger, which can crash the process and cause a denial of service.

Add a check to ensure effective_size is positive before the division. Raise a ValueError if chunk_size <= overlap to prevent the application from crashing with an unhandled exception.

deepsource-development · 2026-02-11T13:40:16Z

+  errors: string[]
+}
+
+export const useRagValidation = (config: any) => {


Use of `any` disables TypeScript type checking

The use of any in the parameter config disables compile-time type checking, allowing any value and bypassing the benefits of TypeScript's static typing. This can lead to unexpected runtime failures or harder-to-maintain code.

Replace any with more specific types to enforce type safety or use unknown if the type is truly dynamic and requires further runtime checks.

deepsource-development · 2026-02-11T13:40:16Z

+    }
+  }
+
+  const handleChange = (key: keyof RagValidationConfig, value: any) => {


Using `any` type disables type checking risks bugs

Defining the value parameter as any disables type checking, allowing any type without constraint which can lead to runtime errors or logic bugs if assumptions about the input type fail.
Use unknown or more specific types in place of any to enforce type safety and ensure proper validation and usage at compile time.

deepsource-development · 2026-02-11T13:40:16Z

+            type="checkbox"
+            id="strict-validation"
+            checked={localConfig.enableStrictValidation}
+            onChange={(e) => handleChange('enableStrictValidation', e.target.checked)}


Inline callback recreates function on every render

Defining inline functions directly in JSX properties like the onChange prop creates a new function instance on every render. This can lead to needless re-rendering of child components due to changed prop references.

Use React.useCallback to memoize the callback or define the function outside the render path to preserve its identity across renders and improve performance.

deepsource-development · 2026-02-11T13:40:16Z

+            type="number"
+            id="max-topk"
+            value={localConfig.maxTopK}
+            onChange={(e) => handleChange('maxTopK', parseInt(e.target.value))}


Inline callback in JSX causes unnecessary function recreations

The inline function (e) => handleChange('maxTopK', parseInt(e.target.value)) inside the JSX onChange prop is recreated on every render. This causes components receiving this callback to re-render unnecessarily due to reference inequality, impacting performance.

Wrap the callback using React.useCallback or define it outside the render method to ensure stable references and prevent excess re-renders.

deepsource-development · 2026-02-11T13:40:16Z

+export const validateRagConfig = (config: RagConfig): boolean => {
+  if (config.topK <= 0) return false
+  if (config.topK > 100) return false
+  if (config.scoreThreshold < 0 || config.scoreThreshold > 1) return false


Complex if-statement returns booleans instead of direct condition

The if-statement checks if config.scoreThreshold is outside 0 to 1 and returns false; otherwise, the function implicitly returns true. This explicit conditional boolean return complicates the logic and reduces readability.
Replace the entire if block with a single return statement like return config.scoreThreshold >= 0 && config.scoreThreshold <= 1 to directly return the boolean evaluation of the condition, simplifying the code and improving clarity.

deepsource-development · 2026-02-11T13:40:16Z

+  return true
+}
+
+export const computeRagScore = (documents: any[], query: string): number => {


Use of `any[]` disables type safety checks

Typing documents as any[] disables compile-time type checks on array elements, allowing any value and risking unexpected runtime errors. This undermines the benefits of using TypeScript's type system to ensure data integrity.

Replace any[] with more precise types or use unknown[] to enforce type checks before usage and improve code robustness.

deepsource-development · 2026-02-11T13:40:16Z

+  return totalScore / documents.length
+}
+
+export const filterDocuments = (documents: any[], threshold: number): any[] => {


`any` type disables TypeScript type checking

The function parameter documents is typed as any[], which bypasses TypeScript's static type checking and allows any value or operation without validation. This can lead to unexpected runtime errors or bugs because the type system does not enforce constraints or catch type mismatches.

Replace the any type with a more specific type or use unknown[] to enforce type checking before usage, improving code safety and maintainability.

deepsource-development · 2026-02-11T13:40:16Z

+  return totalScore / documents.length
+}
+
+export const filterDocuments = (documents: any[], threshold: number): any[] => {


Using `any` type disables type checking, risking bugs

The filterDocuments function uses any[] for both the documents parameter and the return type, which disables type safety checks by TypeScript. This allows any type of data, increasing risk of runtime errors or unexpected bugs due to missing type enforcement.
Replace any with specific or more restrictive types such as unknown or precise interfaces to enable safe type checking and ensure consistent data handling throughout the code.

deepsource-development · 2026-02-11T13:40:16Z

+}
+
+const RagPipelineValidation = ({ config, onConfigChange }: RagPipelineValidationProps) => {
+  const [localConfig, setLocalConfig] = useState(config)


Component state initialized from props does not update on prop changes

The component's internal state localConfig is initialized from the config prop only once during the initial render. If the parent component updates the config prop, this component will not reflect the changes, leading to a UI that is out of sync with the application's state.

To resolve this, use the useEffect hook to synchronize the internal state with the config prop whenever it changes. This ensures the component always displays the current configuration.

deepsource-development · 2026-02-11T13:40:16Z

+    if (cfg.maxTopK <= 0) {
+      throw new Error('Max Top K must be positive')
+    }
+    if (cfg.maxTopK > 1000) {
+      throw new Error('Max Top K cannot exceed 1000')
+    }


`maxTopK` validation does not handle `NaN` values from `parseInt`

The validation for maxTopK does not account for NaN (Not-a-Number) values, which can be produced by parseInt if the input is empty or non-numeric. Both NaN <= 0 and NaN > 1000 evaluate to false, so the validation is bypassed, leading to a NaN value being stored in the component's state and propagated to the parent.

Add a check for isNaN(cfg.maxTopK) at the beginning of the validateConfig function to properly reject invalid number inputs and prevent state corruption.

deepsource-development · 2026-02-11T13:54:00Z

+            type="checkbox"
+            id="require-score-threshold"
+            checked={localConfig.requireScoreThreshold}
+            onChange={(e) => handleChange('requireScoreThreshold', e.target.checked)}


Inline callback recreates function each render causing re-renders

The inline arrow function passed to onChange recreates a new function on each render. This causes components relying on reference equality for props to re-render unnecessarily, impacting performance.

Use React.useCallback to memoize the callback function or move the callback definition outside the component to reuse the same function instance across renders.

deepsource-development · 2026-02-11T13:54:46Z

        """
        raise NotImplementedError
+
+    def validate_rerank_params(self, query: str, documents: list[Document], top_n: int | None) -> None:


Instance method without `self` usage wastes memory

The validate_rerank_params method is defined as an instance method but does not reference the self parameter, causing Python to create a bound method for each class instance that is unnecessary. This increases memory usage and computational overhead.
Decorate the method with @staticmethod to avoid binding to class instances and optimize performance.

deepsource-development · 2026-02-11T13:54:46Z

+        if top_n is not None and top_n <= 0:
+            raise ValueError("Top N must be positive")
+
+    def compute_rerank_score(self, query: str, document: Document) -> float:


Instance method without `self` usage wastes resources

The compute_rerank_score method is defined as an instance method with a self parameter but does not access any instance attributes or methods. This results in Python creating a bound method for each class instance, consuming additional memory and computation.

Decorate compute_rerank_score with @staticmethod to indicate it does not depend on instance state, eliminating the binding overhead and improving performance.

jaffrey-deepsource added 3 commits February 11, 2026 19:00

feat: add enhanced RAG validation with config checks and UI component

12c8639

- Add validation for dataset retrieval config in backend - Add relevance score calculation helper - Add query input validation - Add frontend validation component for RAG pipeline settings

feat: extend RAG validation with additional helpers and utilities

66d31b0

- Add document filtering and scoring helpers in backend - Add frontend validation utilities and hooks - Add pipeline validator class for enhanced checks

github-actions Bot added the web label Feb 11, 2026

deepsource-development Bot reviewed Feb 11, 2026

View reviewed changes

jaffrey-deepsource closed this Feb 11, 2026

jaffrey-deepsource reopened this Feb 11, 2026

jaffrey-deepsource closed this Feb 11, 2026

deepsource-development Bot reviewed Feb 11, 2026

View reviewed changes


		return total_score / len(documents)

		def _validate_query_input(self, query: str, inputs: Mapping[str, Any] \| None) -> None:

		effective_size = chunk_size - overlap
		return (text_length - overlap) // effective_size + 1

Uh oh!

Conversation

jaffrey-deepsource commented Feb 11, 2026

Uh oh!

deepsource-development Bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepSource Code Review

PR Report Card

Code Review Summary

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Nested `if` can be merged using `and` for clarity

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Instance method without `self` use wastes memory

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Instance method lacks `@staticmethod`, causing overhead

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Instance method unused, wasting memory and computation

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Instance method without `self` uses more memory and CPU

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Undefined variable 'e'

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

`compute_document_similarity` only compares content lengths, not content

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

`len(doc.page_content.split())` can be zero, causing a crash

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Division by `effective_size` which can be zero or negative

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Use of `any` disables TypeScript type checking

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Using `any` type disables type checking risks bugs

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Inline callback recreates function on every render

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Inline callback in JSX causes unnecessary function recreations

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Complex if-statement returns booleans instead of direct condition

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Use of `any[]` disables type safety checks

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

`any` type disables TypeScript type checking

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

Using `any` type disables type checking, risking bugs

Uh oh!

deepsource-development Bot Feb 11, 2026

Choose a reason for hiding this comment

deepsource-development Bot commented Feb 11, 2026 •

edited

Loading