Skip to content

Extract Model CRUD out of the crud/__init__.py monolith - #2457

Merged
akwasigroch merged 1 commit into
mainfrom
refactor/extract-model-crud
Aug 12, 2026
Merged

Extract Model CRUD out of the crud/__init__.py monolith#2457
akwasigroch merged 1 commit into
mainfrom
refactor/extract-model-crud

Conversation

@akwasigroch

@akwasigroch akwasigroch commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Purpose

crud/__init__.py is a 1946-line monolith that the codebase is splitting one entity at a time, following #2411, #2412, #2438, #2439, #2441, #2442, #2449, #2450 and #2451. This takes the Model block out. Per apps/backend/AGENTS.md the package only shrinks from here — nothing new goes back into __init__.py.

What Changed

  • Moved get_model, get_models, create_model, update_model and delete_model into a new crud/model.py (149 lines), byte-for-byte. The internal coupling stays inside the new module: update_model and delete_model both call get_model.
  • crud/__init__.py drops 142 lines. The Tool block directly below is untouched.
  • Nothing is re-exported, matching the earlier splits. All 25 call sites across 13 files now use from rhesis.backend.app.crud import model as model_crud. The alias matters here — many of these modules already import models, and model/models is easy to confuse.
  • Dropped test_model_connection. Zero callers repo-wide. The /models/{id}/test endpoint has its own async test_model_connection in routers/model.py backed by ModelConnectionService; the crud version was a stub whose entire body was return True inside a try that could never raise. Dead code that read as live because a real function shares its name.
  • The module docstring records the is_protected system-model rules, since they are enforced across two functions and are easy to miss: protected models reject edits to name, model_name, provider_type_id, key, endpoint, is_protected and icon, and reject deletion outright, while tags, status, owner and assignee stay editable.

Additional Context

Testing

Pure move plus one dead-function deletion, so the existing suite is the check.

cd apps/backend
uv run pytest ../../tests/backend/routes/test_model.py ../../tests/backend/utils/test_llm_delegation.py ../../tests/backend/security/test_organization_filtering.py
# 70 passed

uv run pytest ../../tests/backend/crud ../../tests/backend/security ../../tests/backend/utils ../../tests/backend/services
# 3237 passed, 4 skipped, 1 xfailed

uv run pytest ../../tests/backend/routes ../../tests/backend/app ../../tests/backend/metrics
# 1823 passed, 40 skipped

Ruff clean on all 13 touched files. A repo-wide grep for the six old crud.* names returns nothing.

Two things worth a reviewer's eye:

routers/services.py was already using the name model_crud, but bound to the whole crud package rather than a submodule — exactly the reach-through pattern apps/backend/AGENTS.md warns about, which only worked because the alias happened to resolve get_model off the monolith. It now points at the real submodule.

get_model collides three ways in this area: this crud function, the SDK factory rhesis.sdk.models.factory.get_model, and the router endpoint. utils/user_model_utils.py imports both the SDK factory and model_crud, and tests/backend/utils/test_llm_delegation.py deliberately patches both ...user_model_utils.get_model (SDK) and ...user_model_utils.model_crud.get_model (crud).

@peqy

peqy Bot commented Aug 12, 2026

Copy link
Copy Markdown

Looks good. Clean extraction of the model CRUD into crud/model.py with call sites updated consistently, and dropping the unused test_model_connection stub reduces misleading surface area. Ship it.

Move the model functions out of crud/__init__.py into crud/model.py,
following the split already done for source, task, tag and others.
Consumers import the module directly; nothing is re-exported.

Drops test_model_connection, a stub that only ever returned True and had
no callers -- the /models/{id}/test endpoint uses ModelConnectionService
instead.

routers/services.py already aliased the whole crud package as
model_crud; it now aliases the submodule under the same name.
@akwasigroch
akwasigroch force-pushed the refactor/extract-model-crud branch from 2023b7a to a6187cc Compare August 12, 2026 15:23

@peqy peqy Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good. The Model CRUD was cleanly extracted into crud/model.py, call sites were updated consistently, and removing the unused test_model_connection stub reduces misleading surface area. Ship it.

@akwasigroch
akwasigroch merged commit a632d89 into main Aug 12, 2026
15 of 16 checks passed
@akwasigroch
akwasigroch deleted the refactor/extract-model-crud branch August 12, 2026 15:40
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.

1 participant