langgenius/dify
1
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-07-21 20:15:31 -04:00
Code Issues 775 Packages Projects Releases 10 Wiki Activity

[PR #31514] refactor: pass BaseModel instances instead of dict #33274

New Issue
Closed
opened 2026-02-21 20:52:59 -05:00 by yindo · 0 comments
yindo commented 2026-02-21 20:52:59 -05:00
Owner
Copy Link
Copy Source

Original Pull Request: https://github.com/langgenius/dify/pull/31514

State: closed
Merged: Yes

Summary

This PR refactors several service methods to accept Pydantic BaseModel instances directly instead of dict parameters, improving type safety and eliminating redundant model validation.

Motivation

As discussed in #31497, passing dict parameters between functions and then immediately calling model_validate() is redundant and loses type safety benefits. This PR demonstrates the refactoring pattern for a subset of the codebase.

Changes

1. RagPipelineTransformService._deal_knowledge_index()

Before:

def _deal_knowledge_index(self, ..., retrieval_model: dict, ...):
    retrieval_setting = RetrievalSetting.model_validate(retrieval_model)
    # Use retrieval_setting...

After:

def _deal_knowledge_index(self, ..., retrieval_model: RetrievalSetting | None, ...):
    # Use retrieval_model directly with type safety

2. RagPipelineDslService.get_leaked_dependencies() & AppDslService.get_leaked_dependencies()

Before:

def get_leaked_dependencies(cls, ..., dsl_dependencies: list[dict]):
    dependencies = [PluginDependency.model_validate(dep) for dep in dsl_dependencies]
    return DependenciesAnalysisService.get_leaked_dependencies(...)

After:

def get_leaked_dependencies(cls, ..., dsl_dependencies: list[PluginDependency]):
    return DependenciesAnalysisService.get_leaked_dependencies(...)

Benefits

  • ✅ Type Safety: Function parameters now have proper Pydantic types, enabling IDE autocomplete and type checking
  • ✅ Performance: Eliminated redundant model_validate() calls
  • ✅ Maintainability: Clearer function contracts and better code documentation
  • ✅ No Breaking Changes: All changes are to internal/private methods only

Testing

  • make lint - passed
  • make type-check - passed (0 errors, 0 warnings)
  • Unit tests for dataset services - 229 passed
  • Integration tests for app_dsl_service - 8 passed

Files Changed

  • api/services/rag_pipeline/rag_pipeline_transform_service.py
  • api/services/rag_pipeline/rag_pipeline_dsl_service.py
  • api/services/app_dsl_service.py

Related Issue

Part of #31497

Notes

  • This is an incremental refactoring. More opportunities exist in the codebase following the same pattern.
  • Database JSON fields (e.g., dataset.retrieval_model) correctly retain .model_dump() for persistence.
  • Validation now happens at the boundary (when reading from database/external sources) rather than being repeated in internal function calls.
**Original Pull Request:** https://github.com/langgenius/dify/pull/31514 **State:** closed **Merged:** Yes --- ## Summary This PR refactors several service methods to accept Pydantic BaseModel instances directly instead of dict parameters, improving type safety and eliminating redundant model validation. ## Motivation As discussed in #31497, passing dict parameters between functions and then immediately calling `model_validate()` is redundant and loses type safety benefits. This PR demonstrates the refactoring pattern for a subset of the codebase. ## Changes ### 1. RagPipelineTransformService._deal_knowledge_index() **Before:** ```python def _deal_knowledge_index(self, ..., retrieval_model: dict, ...): retrieval_setting = RetrievalSetting.model_validate(retrieval_model) # Use retrieval_setting... ``` **After:** ```python def _deal_knowledge_index(self, ..., retrieval_model: RetrievalSetting | None, ...): # Use retrieval_model directly with type safety ``` ### 2. RagPipelineDslService.get_leaked_dependencies() & AppDslService.get_leaked_dependencies() **Before:** ```python def get_leaked_dependencies(cls, ..., dsl_dependencies: list[dict]): dependencies = [PluginDependency.model_validate(dep) for dep in dsl_dependencies] return DependenciesAnalysisService.get_leaked_dependencies(...) ``` **After:** ```python def get_leaked_dependencies(cls, ..., dsl_dependencies: list[PluginDependency]): return DependenciesAnalysisService.get_leaked_dependencies(...) ``` ## Benefits - ✅ **Type Safety**: Function parameters now have proper Pydantic types, enabling IDE autocomplete and type checking - ✅ **Performance**: Eliminated redundant `model_validate()` calls - ✅ **Maintainability**: Clearer function contracts and better code documentation - ✅ **No Breaking Changes**: All changes are to internal/private methods only ## Testing - [x] `make lint` - passed - [x] `make type-check` - passed (0 errors, 0 warnings) - [x] Unit tests for dataset services - 229 passed - [x] Integration tests for app_dsl_service - 8 passed ## Files Changed - `api/services/rag_pipeline/rag_pipeline_transform_service.py` - `api/services/rag_pipeline/rag_pipeline_dsl_service.py` - `api/services/app_dsl_service.py` ## Related Issue Part of #31497 ## Notes - This is an incremental refactoring. More opportunities exist in the codebase following the same pattern. - Database JSON fields (e.g., `dataset.retrieval_model`) correctly retain `.model_dump()` for persistence. - Validation now happens at the boundary (when reading from database/external sources) rather than being repeated in internal function calls.
yindo added the pull-request label 2026-02-21 20:52:59 -05:00
yindo closed this issue 2026-02-21 20:52:59 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Knowledge Pipeline
Proposal
WIP
cloud
dependencies
feat:webapp
good first issue
outdated
performance
pull-request
Mirrored from GitHub Pull Request
 python:uv
refactor
size:L
stale
status: accepting prs
status: blocked
status: in discussion
stt & tts
vulnerablity
web
wont-fix
🌊 feat:workflow
🌚 invalid
🐞 bug
👻 feat:rag
💪 enhancement
📚 documentation
🔨 feat:tools
🙋‍♂️ question
🙏 help wanted
🤔 cant-reproduce
🤖 feat:agent
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: langgenius/dify#33274
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?