Skip to content

refactor(console): dep-inject model provider payloads with @model_validate - #41539

Merged
asukaminato0721 merged 4 commits into
langgenius:mainfrom
ShousenZHANG:refactor/dep-inject-model-providers
Sep 1, 2026
Merged

refactor(console): dep-inject model provider payloads with @model_validate#41539
asukaminato0721 merged 4 commits into
langgenius:mainfrom
ShousenZHANG:refactor/dep-inject-model-providers

Conversation

@ShousenZHANG

@ShousenZHANG ShousenZHANG commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

part of #36659

Moves the eight inline parses in console/workspace/model_providers.py onto the @model_validate decorator, the same change as #41374, #41490, #41491, #41500 and #41501. Claimed in this comment.

Six read the JSON body. The other two — ModelProviderListApi.get and ModelProviderCredentialApi.get — already read request.args.to_dict(flat=True), which is exactly what the decorator's GET branch does, so they convert the same way AccountAvatarApi.get and EducationAutoCompleteApi.get did. request is then unused in the module and drops out of the flask import.

One thing worth flagging

ModelProviderCredentialApi.delete is a DELETE, and the decorator reads request.args first, falling back to the JSON body only when the query string is empty. The console sends that request with a body and no query string (web/service/use-models.ts), so behaviour is unchanged — but it is a real difference from the previous unconditional body read, so I would rather state it than have it found.

Deliberately left alone

The rest of console/workspace: skills.py wraps every parse in except ValidationError -> {"code": ..., "message": ...}, 400, and members.py / workspace.py / rbac.py have open PRs with hunks on the parse lines themselves.

How did you test it?

  • pytest tests/unit_tests/controllers/console/2118 passed, against 2110 on main; the eight extra are the parametrized decorator test added here. The single pre-existing failure (test_datasets_document_download.py, a Windows temp-file lock) reproduces identically on main and is in a file this PR does not touch.
  • ruff check / ruff format --check — clean; pyrefly check — 0 diagnostics
  • Enumerated the test call sites by handler identity: 16 belong to the converted handlers and now pass the validated model; the 5 that belong to ModelProviderSummaryListApi, ModelProviderCreditsApi and ModelProviderPaymentCheckoutUrlApi are untouched
  • Added a bound-method test for the decorators themselves, parametrized over all eight handlers. The existing tests all unwrap the view and build the model by hand, so without it the decorators would ship unexercised. Verified by mutation: dropping any one of the eight decorators fails exactly one case, and both sources the decorator reads are covered — query args for the two get handlers, the JSON body for the rest
  • Runtime signature check on all eight wrapped handlers and on the three deliberately untouched ones

This PR was fully generated with an AI assistant. I have reviewed the changes and run the relevant tests.

…idate

Moves the eight inline parses in console/workspace/model_providers.py onto the
@model_validate decorator. Six read the JSON body; the two get handlers already
read request.args.to_dict(flat=True), which is what the decorator's GET branch
does, so they convert the same way AccountAvatarApi.get and
EducationAutoCompleteApi.get did.

request is no longer used in the module, so the flask import drops it.

The existing tests unwrap the view and pass the model themselves, which leaves
the decorators unexercised, so this also adds one bound-method test asserting the
422 the decorator raises for an invalid query. Removing any decorator now fails it.
Copilot AI lite review requested due to automatic review settings August 31, 2026 13:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-08-31 16:25:40.736934292 +0000
+++ /tmp/pyrefly_pr.txt	2026-08-31 16:25:31.472893528 +0000
@@ -3533,7 +3533,7 @@
 ERROR Missing argument `endpoint_id` in function `controllers.console.workspace.endpoint.EndpointIdPayload.__init__` [missing-argument]
    --> tests/unit_tests/controllers/console/workspace/test_endpoint.py:513:46
 ERROR Argument `Literal['team']` is not assignable to parameter `plan` with type `CloudPlan | None` in function `services.workspace_service.EffectiveCreditPool.__init__` [bad-argument-type]
-   --> tests/unit_tests/controllers/console/workspace/test_model_providers.py:236:18
+   --> tests/unit_tests/controllers/console/workspace/test_model_providers.py:260:18
 ERROR Argument `list[FromClause]` is not assignable to parameter `tables` with type `Sequence[Table] | None` in function `sqlalchemy.sql.schema.MetaData.create_all` [bad-argument-type]
   --> tests/unit_tests/controllers/console/workspace/test_workspace.py:54:54
 ERROR `SimpleNamespace` is not assignable to attribute `db` with type `SQLAlchemy` [bad-assignment]

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 61.77% 61.78% +0.01%
Strict coverage 61.37% 61.38% +0.01%
Typed symbols 42,986 43,000 +14
Untyped symbols 26,777 26,777 0
Modules 3290 3290 0

The decorator test only exercised ModelProviderListApi; dropping any of the other
seven decorators left the suite green. It is now parametrized over all eight
handlers, covering both sources the decorator reads — query args for the two get
handlers, the JSON body for the rest — and each of the eight is individually
detected when its decorator is removed.

Also drops the blank lines the parse statements left behind, and reattaches the
credential comment to the call it describes.
CI's Python Style job runs pyrefly over tests/ with its own config, which the
new parametrized test tripped: unannotated parameters, a bare dict return on the
payload helper, and typing.Any (banned by ruff TID251). Annotated with object,
leaving the file at its baseline diagnostic count.
scripts/check_no_new_getattr.py rejects net-new getattr() in changed code, and
the parametrized decorator test dispatched through getattr(api, verb). Each row
now carries the bound call directly.
@asukaminato0721
asukaminato0721 added this pull request to the merge queue Sep 1, 2026
Merged via the queue into langgenius:main with commit 1977355 Sep 1, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants