Repository navigation
refactor(api): replace json.loads with Pydantic validation in repositories and tasks - #43704
Merged
asukaminato0721 merged 6 commits intoOct 9, 2026
Conversation
…te_json _source_mapping and _mapping already validated through a TypeAdapter, but they decoded the string with json.loads first and then validated the resulting object. validate_json performs the decode and the validation in one pass; malformed input raises ValidationError, which the existing except clause already covers, so behaviour is unchanged. Part of langgenius#33092
get_workspace_logo cast the json.loads result straight to TenantCustomConfigDict, which asserted the shape without checking it. TypeAdapter.validate_json decodes and validates in one pass. The TypedDict is total=False, so partial configurations keep working, and this function only reads replace_webapp_logo, so ignoring any other stored key is not observable here. Part of langgenius#33092
…e them Both tasks decoded the payload with json.loads and immediately passed the result to a Pydantic model. model_validate_json and validate_json do the decode and the validation in one pass: - enterprise_telemetry_task: TelemetryEnvelope.model_validate_json - process_tenant_plugin_autoupgrade_check_task: the adapter accepts MarketplacePluginSnapshot | None, because a cached miss is stored as JSON null and must keep returning None instead of falling through to False Part of langgenius#33092
_parse_recipient_payload read two keys out of a raw dict and handed the TYPE string back as a RecipientType, so a payload carrying an unknown channel or a non-string email reached the caller unchecked. A model validates both fields in one pass and returns a real enum. The caller already skipped anything that was not EMAIL_MEMBER or EMAIL_EXTERNAL, so unusable payloads still end up as a skipped recipient. The helper had no tests at all, so cover both the accepted and the rejected shapes. Part of langgenius#33092
Both queue tasks decoded the uploaded payload with json.loads and then iterated it, so a payload that was not a list of objects only failed later inside a worker thread. TypeAdapter(list[dict[str, Any]]) checks the shape at the task boundary; RagPipelineTaskProxy already writes exactly that shape. Entries stay plain mappings because run_single_rag_pipeline_task still validates each one into RagPipelineInvokeEntity in its own thread context. Part of langgenius#33092
haloneko
requested review from
JohnJyong,
QuantumGhost,
WH-2099 and
laipz8200
as code owners
October 8, 2026 12:32
ghfind-review
Bot
added
the
review: medium
github-actions
Bot
added
the
size:M
haloneko
changed the title
Contributor
Pyrefly Diffbase → PR--- /tmp/pyrefly_base.txt 2026-10-08 13:32:48.337779963 +0000
+++ /tmp/pyrefly_pr.txt 2026-10-08 13:32:38.569721335 +0000
@@ -3860,6 +3860,8 @@
--> tests/unit_tests/core/moderation/api/test_api.py:193:78
ERROR Argument `Session` is not assignable to parameter `session` with type `scoped_session[Unknown]` in function `core.moderation.api.api.ApiModeration._get_api_based_extension` [bad-argument-type]
--> tests/unit_tests/core/moderation/api/test_api.py:196:76
+ERROR Argument `Literal['invalid']` is not assignable to parameter `config` with type `dict[str, Any] | None` in function `core.moderation.base.Moderation.__init__` [bad-argument-type]
+ --> tests/unit_tests/core/moderation/test_sensitive_word_filter.py:1278:92
ERROR Method `trace` inherited from class `BaseTraceInstance` has no implementation and cannot be accessed via `super()` [missing-attribute]
--> tests/unit_tests/core/ops/test_base_trace_instance.py:22:9
ERROR Argument `() -> Session` is not assignable to parameter `session_factory` with type `sessionmaker[@_]` in function `sqlalchemy.orm.scoping.scoped_session.__init__` [bad-argument-type]
|
Contributor
Pyrefly Type Coverage
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #43704 +/- ##
==========================================
- Coverage 88.69% 88.69% -0.01%
==========================================
Files 5191 5191
Lines 326420 326421 +1
Branches 65424 65423 -1
==========================================
- Hits 289512 289505 -7
- Misses 31681 31689 +8
Partials 5227 5227
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@asukaminato0721 Ready for review. This is the repositories/ + tasks/ slice of #33092 — replaces json.loads with Pydantic validation, no behaviour changes intended. 65 tests pass, lint clean. PTAL when you get a chance. Thanks! |
asukaminato0721
approved these changes
Oct 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Fixes #<issue number>.Summary
Replace
json.loadsplus the manual dict handling around it with Pydantic validation, in the repositories and tasks layers.repositories/data_source/credential_repository.py,repositories/knowledge/document_repository.py: drop the redundantvalidate_python(json.loads(x))double parse in favour ofvalidate_json(x).repositories/upload_file_delivery_repository.py: replacecast(TenantCustomConfigDict, json.loads(x))with aTypeAdapterthat actually checks the shape.tasks/enterprise_telemetry_task.py:json.loads+model_validate→model_validate_json.tasks/process_tenant_plugin_autoupgrade_check_task.py: parse and validate the cached manifest in one pass; the adapter acceptsNoneso a cached miss still returnsNone.tasks/mail_human_input_delivery_task.py:_parse_recipient_payloadvalidates the payload and returns a realRecipientTypeinstead of a raw string. It used to forward a non-stringemailand an unknownTYPEunchecked; the caller already discarded those. New tests cover both shapes.tasks/rag_pipeline/*: validate the queued payload aslist[dict[str, Any]].No other behaviour change is intended — each conversion keeps the same validation strength as the code it replaces.
Screenshots
Checklist
make lint && make type-check(backend) andvp staged(frontend) to appease the lint godsmakeis unavailable on Windows, so the underlying commands were run directly —ruff check ./api,ruff format --check ./api,pyrefly check— all clean.vp stagedwas not run.Part of #33092
From CodeBuddy