Skip to content

refactor: replace dict with BedrockRetrievalSetting BaseModel in knowledge_service#34080

Merged
asukaminato0721 merged 2 commits intolanggenius:mainfrom
faizkhairi:refactor/basemodel-knowledge-service
Mar 25, 2026
Merged

refactor: replace dict with BedrockRetrievalSetting BaseModel in knowledge_service#34080
asukaminato0721 merged 2 commits intolanggenius:mainfrom
faizkhairi:refactor/basemodel-knowledge-service

Conversation

@faizkhairi
Copy link
Copy Markdown
Contributor

@faizkhairi faizkhairi commented Mar 25, 2026

Description

Replace the untyped dict parameter in ExternalDatasetTestService.knowledge_retrieval() with a Pydantic BaseModel for type safety, IDE autocompletion, and validation at the boundary.

Changes

File Change
api/services/knowledge_service.py Create BedrockRetrievalSetting model, update function signature
api/controllers/console/datasets/external.py Update BedrockRetrievalPayload.retrieval_setting type from dict to BedrockRetrievalSetting
api/tests/unit_tests/services/test_knowledge_service.py Update all test callsites to use BedrockRetrievalSetting instances

Before

def knowledge_retrieval(retrieval_setting: dict, query: str, knowledge_id: str):
    retrieval_setting.get("top_k")
    retrieval_setting.get("score_threshold", 0.0)

After

class BedrockRetrievalSetting(BaseModel):
    top_k: int | None = Field(default=None)
    score_threshold: float = Field(default=0.0)

def knowledge_retrieval(retrieval_setting: BedrockRetrievalSetting, query: str, knowledge_id: str):
    retrieval_setting.top_k
    retrieval_setting.score_threshold

Part of #31497

@faizkhairi
refactor: replace dict with BedrockRetrievalSetting BaseModel in know…
e54b846 
…ledge_service

Replace the untyped dict parameter in ExternalDatasetTestService.knowledge_retrieval()
with a Pydantic BaseModel for type safety, IDE autocompletion, and validation.

- Create BedrockRetrievalSetting model with top_k and score_threshold fields
- Update knowledge_retrieval() signature to accept BedrockRetrievalSetting
- Update BedrockRetrievalPayload to use BedrockRetrievalSetting type
- Update all test callsites to use BedrockRetrievalSetting instances

Part of langgenius#31497
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. refactor labels Mar 25, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 25, 2026

Pyrefly Diff

No changes detected.

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Mar 25, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 25, 2026

Pyrefly Diff

No changes detected.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 25, 2026
@asukaminato0721 asukaminato0721 merged commit ff9cf6c into langgenius:main Mar 25, 2026
14 checks passed
@faizkhairi
Copy link
Copy Markdown
Contributor Author

faizkhairi commented Mar 25, 2026 •
edited
Loading

thanks @asukaminato0721 ! happy to help and contribute to Dify AI's project 🎉 😄

GareArc pushed a commit that referenced this pull request Mar 25, 2026
@faizkhairi @autofix-ci @GareArc
refactor: replace dict with BedrockRetrievalSetting BaseModel in know…
da32a11 
…ledge_service (#34080)

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asukaminato0721 asukaminato0721 asukaminato0721 approved these changes

@JohnJyong JohnJyong Awaiting requested review from JohnJyong JohnJyong is a code owner

@laipz8200 laipz8200 Awaiting requested review from laipz8200 laipz8200 is a code owner

@QuantumGhost QuantumGhost Awaiting requested review from QuantumGhost QuantumGhost is a code owner

Assignees

No one assigned

Labels

lgtm This PR has been approved by a maintainer refactor size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@faizkhairi @asukaminato0721