feat(platform): Add LLM registry admin write API - #12467
Conversation
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
platform/frontend
github-actions
Bot
added
the
conflicts
|
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
Rewrote actions.ts to directly call new admin API endpoints: - POST/PATCH/DELETE /api/llm/providers - POST/PATCH/DELETE /api/llm/models Changes: - Removed dependency on generated API client (was failing) - Direct fetch() calls with proper error handling - Updated getLlmRegistryPage.ts to use new action functions - Proper FormData parsing for all CRUD operations - Revalidates /admin/llms path after mutations This connects the UI (copied from PR #11699) to our new admin API (PR #12467). Creators and Migrations endpoints not yet implemented in backend (placeholders return empty arrays).
894395a to
164b8ec
Compare
Rewrote actions.ts to directly call new admin API endpoints: - POST/PATCH/DELETE /api/llm/providers - POST/PATCH/DELETE /api/llm/models Changes: - Removed dependency on generated API client (was failing) - Direct fetch() calls with proper error handling - Updated getLlmRegistryPage.ts to use new action functions - Proper FormData parsing for all CRUD operations - Revalidates /admin/llms path after mutations This connects the UI (copied from PR #11699) to our new admin API (PR #12467). Creators and Migrations endpoints not yet implemented in backend (placeholders return empty arrays).
164b8ec to
89b3f83
Compare
Rewrote actions.ts to directly call new admin API endpoints: - POST/PATCH/DELETE /api/llm/providers - POST/PATCH/DELETE /api/llm/models Changes: - Removed dependency on generated API client (was failing) - Direct fetch() calls with proper error handling - Updated getLlmRegistryPage.ts to use new action functions - Proper FormData parsing for all CRUD operations - Revalidates /admin/llms path after mutations This connects the UI (copied from PR #11699) to our new admin API (PR #12467). Creators and Migrations endpoints not yet implemented in backend (placeholders return empty arrays).
89b3f83 to
344a51f
Compare
Rewrote actions.ts to directly call new admin API endpoints: - POST/PATCH/DELETE /api/llm/providers - POST/PATCH/DELETE /api/llm/models Changes: - Removed dependency on generated API client (was failing) - Direct fetch() calls with proper error handling - Updated getLlmRegistryPage.ts to use new action functions - Proper FormData parsing for all CRUD operations - Revalidates /admin/llms path after mutations This connects the UI (copied from PR #11699) to our new admin API (PR #12467). Creators and Migrations endpoints not yet implemented in backend (placeholders return empty arrays).
![autogpt-pr-reviewer[bot]](https://avatars.githubusercontent.com/u/130738209?s=60&v=4){kind=small}
There was a problem hiding this comment.
All 8 specialists have reported. Let me compile the final verdict.
PR #12467 — feat(platform): Implement LLM registry admin API functionality
Author: Bentlybro | Files: admin_model.py (+35/−18), admin_routes.py (+231/−35), db_write.py (+294/−0 new)
🎯 Verdict: REQUEST_CHANGES
What This PR Does
Replaces six 501 "Not Implemented" stubs in the LLM registry admin API with full CRUD implementations. Adds a new db_write.py database layer for creating, updating, and deleting LLM providers and models via Prisma, with cache refresh after every mutation. Also fixes a missing auth decorator on the create_provider endpoint.
Specialist Findings
🛡️ Security create_provider was missing Security(requires_admin_user) — now added ✅). However, all 4 update/delete handlers have a bug where HTTPException(404) is swallowed by except Exception and returned as a generic 500. creator_id is handled inconsistently between create (relational connect) and update (flat creatorId key) — may bypass FK validation on updates. Log injection risk via f-strings with user-controlled slugs/names (low severity, admin-only). No max_length on any string fields.
admin_routes.py:107,159,194,271,307 — HTTPException swallowed by broad except Exception
db_write.py:260 — creator_id inconsistency between create/update paths
🏗️ Architecture prisma.models inline and query the DB directly (slug→ID lookups), breaking the abstraction that db_write.py provides. db_write.py then re-fetches the same entity by ID — doubling lookups. Cache invalidation sits in db_write.py importing llm_registry (inverted dependency). Response mapping uses raw dict[str, Any] instead of Pydantic response models, losing OpenAPI documentation and type safety.
admin_routes.py:126,178,246,291 — inline import prisma.models breaks layer separation
db_write.py:289 — cache invalidation in DB layer inverts the dependency
⚡ Performance ✅ — Admin-only endpoints, not on a hot path. However, every mutation does 2-3 redundant DB round-trips (route lookup + db_write re-lookup + actual operation) plus a full registry reload (find_many with 3 JOINs on all models). Unnecessary include= clauses on create/update operations load relations (Models, Costs, Creator, Provider) that the response mapper never uses. Acceptable for infrequent admin use, but a bulk-import scenario would be painful.
db_write.py:96,131,215,247 — unnecessary include= JOINs on every operation
🧪 Testing ❌ — Zero tests added. 294 lines of new business logic with no test coverage. The codebase has established test patterns (credit_admin_routes_test.py) that this PR should follow. The HTTPException-swallowing bug would have been caught by even basic route tests. Missing: CRUD happy paths, 404 handling, auth enforcement regression test, duplicate slug/name handling, delete_provider with-models guard, Pydantic validator boundaries.
📖 Quality import prisma.models in 4 function bodies, duplicated try/except boilerplate across all 6 handlers (could be a decorator), _build_provider_data only used by create (not update), creator_id handled differently in create vs update paths. The f-string logging is consistent with the project's existing style.
db_write.py:260-261 — creator_id uses flat key in update vs relational connect in create
📦 Product create_provider is fixed. But: 404s return as 500s (bad admin UX), no active-usage guard on model deletion (admin can break running workflows), executor cache won't refresh (TODOs acknowledged), and no admin-specific GET endpoints for inspecting individual providers/models. Generic 500 messages hide actionable error context from admins.
db_write.py:292-294 has TODOs
db_write.py:273 — no workflow-usage check before model deletion
📬 Discussion conflicts label is applied. Not merge-ready from a process standpoint.
🔎 QA ✅ — Backend health confirmed. All 6 admin endpoints registered in OpenAPI spec. Auth enforcement verified (401 without credentials on all endpoints). Frontend unaffected — landing page, signup, dashboard, build page, marketplace, and library all load correctly. No new console errors. No regressions.
Blockers (Must Fix)
-
admin_routes.py:107,159,194,227,271,307— HTTPException swallowed byexcept Exception: All 4 update/delete handlers raiseHTTPException(404)inside atryblock that catchesException— converting all 404s into generic 500s. Fix: addexcept HTTPException: raisebeforeexcept Exception, or move lookups outside the try. Flagged by: Security, Architect, Performance, Testing, Quality, Product (6/8 specialists) -
Zero test coverage: 294 lines of new DB logic + 6 rewritten route handlers with no tests. The codebase has established patterns to follow (
credit_admin_routes_test.py). At minimum: CRUD happy paths, 404 propagation, auth enforcement, and the delete-provider-with-models guard. Flagged by: Testing -
Merge conflicts: PR has
mergeable: CONFLICTINGstatus. Must be resolved before review can be finalized. Flagged by: Discussion
Should Fix (Follow-up OK)
-
admin_routes.py:126,178,246,291— Move slug→ID lookups intodb_write.py: Route handlers directly queryprisma.models, breaking the DB abstraction layer and causing double lookups.db_writefunctions should accept slug/name and handle resolution internally. -
db_write.py:260-261— Inconsistentcreator_idhandling: Create uses{"Creator": {"connect": {"id": ...}}}(relational), update uses flatdata["creatorId"] = creator_id. Should be consistent — use relational connect pattern in both. -
admin_routes.py:32,50— Replacedict[str, Any]responses with Pydantic models: AddLlmProviderResponse/LlmModelResponsetoadmin_model.pyand setresponse_model=on route decorators for OpenAPI docs and type safety. -
db_write.py:96,131,215,247— Remove unnecessaryinclude=clauses:include={"Models": True}on provider create/update andinclude={"Costs": True, "Creator": True, "Provider": True}on model create/update load relations the response mapper doesn't use. -
db_write.py:292-294— Document executor cache propagation gap: The TODO about publishing refresh notifications to executors means model changes won't propagate to running agents. Should be documented in API docs or release notes.
Risk Assessment
Merge risk: MEDIUM | Rollback: EASY (revert to 501 stubs — no data migration involved)
The HTTPException bug is a clear functional defect that affects every update/delete endpoint. Combined with zero tests and merge conflicts, this PR needs another iteration before it's merge-ready. The architecture and implementation are fundamentally sound — the issues are fixable without a redesign.
REVIEW_COMPLETE
PR: #12467
Verdict: REQUEST_CHANGES
Blockers: 3
344a51f to
bf427af
Compare
Rewrote actions.ts to directly call new admin API endpoints: - POST/PATCH/DELETE /api/llm/providers - POST/PATCH/DELETE /api/llm/models Changes: - Removed dependency on generated API client (was failing) - Direct fetch() calls with proper error handling - Updated getLlmRegistryPage.ts to use new action functions - Proper FormData parsing for all CRUD operations - Revalidates /admin/llms path after mutations This connects the UI (copied from PR #11699) to our new admin API (PR #12467). Creators and Migrations endpoints not yet implemented in backend (placeholders return empty arrays).
bf427af to
5a3f5d7
Compare
github-actions
Bot
removed
the
conflicts
|
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
be328c1 to
afe119c
Compare
github-actions
Bot
removed
the
conflicts
|
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
c74652d to
5b2d459
Compare
|
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
github-actions
Bot
added
the
conflicts
Add admin write API endpoints for LLM registry management:
- POST /api/llm/models - Create model
- PATCH /api/llm/models/{slug} - Update model
- DELETE /api/llm/models/{slug} - Delete model
- POST /api/llm/providers - Create provider
- PATCH /api/llm/providers/{name} - Update provider
- DELETE /api/llm/providers/{name} - Delete provider
All endpoints require admin authentication via requires_admin_user.
Request/response models defined in admin_model.py:
- CreateLlmModelRequest, UpdateLlmModelRequest
- CreateLlmProviderRequest, UpdateLlmProviderRequest
Implementation coming in follow-up commits (currently returns 501 Not Implemented).
This builds on:
- PR #12357: Schema foundation
- PR #12359: Registry core
- PR #12371: Public read API
Implement full CRUD operations for admin API: Database layer (db_write.py): - create_provider, update_provider, delete_provider - create_model, update_model, delete_model - refresh_runtime_caches - invalidates in-memory registry after mutations - Proper validation and error handling Admin routes (admin_routes.py): - All endpoints now functional (no more 501) - Proper error responses (400 for validation, 404 for not found, 500 for server errors) - Lookup by slug/name before operations - Cache refresh after all mutations Features: - Provider deletion blocked if models exist (FK constraint) - All mutations refresh registry cache automatically - Proper logging for audit trail - Admin auth enforced on all endpoints Based on original implementation from PR #11699 (upstream-llm branch). Builds on: - PR #12357: Schema foundation - PR #12359: Registry core - PR #12371: Public read API
Introduce a read endpoint for LLM model creators: add _map_creator_response serializer and an admin-only GET /llm/creators route that queries prisma.models.LlmModelCreator (ordered by name), logs results, and returns serialized creators with error handling. Also update frontend OpenAPI spec with the /api/llm/creators GET operation.
…slashes
Model slugs like 'openai/gpt-oss-120b' contain forward slashes which
FastAPI's default {slug} parameter doesn't capture. Using {slug:path}
allows the full slug to be captured as a single parameter.
…ation
- GET /llm/admin/providers - list all providers from DB (includes empty ones)
- GET /llm/admin/models - list all models with costs and creator info
- POST /llm/creators - create new creator
- PATCH /llm/creators/{name} - update creator
- DELETE /llm/creators/{name} - delete creator (with model check)
- Create LlmModelCost records when creating a model
- Resolve provider name to ID in create_model
- Add costs field to CreateLlmModelRequest
…te, disable with migration, revert
- GET /llm/models/{slug}/usage - count AgentNodes using a model
- DELETE /llm/models/{slug} with optional replacement_model_slug for safe migration
- POST /llm/models/{slug}/toggle with migration support when disabling
- GET /llm/migrations - list model migrations (with include_reverted filter)
- POST /llm/migrations/{id}/revert - revert a migration (restores nodes, re-enables source model)
- Transactional migration: counts nodes, migrates atomically, creates LlmModelMigration audit record
- Ported from original PR #11699's db.py
…_model When setting is_recommended=True on a model, first clears the flag on all other models within the same transaction so only one model can be recommended at a time.
…lidation and pub/sub After any admin DB mutation, clear the shared Redis cache, refresh this process's in-memory state, then publish a notification so all other workers reload from Redis without hitting the database.
…xception passthrough admin_routes.py: fix 6 routes where HTTPException(404) was caught by 'except Exception' and re-raised as 500. Added 'except HTTPException: raise' before each generic except block. conftest.py: local mock_jwt_user/admin fixtures for llm test dir admin_routes_test.py (new, 28 tests): - Full provider CRUD incl. has-models 400 and server 500 paths - Full model CRUD incl. toggle, usage, migration, revert - Full creator CRUD and admin list endpoints db_write_test.py (new, 28 tests): - Provider/model create, update, delete all branches - Single-recommended enforcement in update_model - delete_model with/without replacement, all error cases - toggle_model: enable, disable (with and without migration) - revert_migration: all branches incl. already reverted, not found - refresh_runtime_caches: all 3 operations verified
…mended model guard db_write.py: - Add _node_model_value() helper: strips provider prefix from slug so queries match the enum value stored in AgentNode.constantInput (e.g. 'openai/gpt-4o' -> 'gpt-4o') - get_model_usage: use _node_model_value(slug) — was always returning 0 - toggle_model_with_migration: use _node_model_value for both the SELECT (finding nodes) and the UPDATE (writing the replacement value) admin_routes.py: - toggle_model: guard against disabling the recommended model with 400 'Cannot disable the recommended model. Change it first.'
c4dacec to
da53308
Compare
github-actions
Bot
removed
the
conflicts
|
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
…azy imports to top-level - Add ToggleLlmModelRequest, CreateLlmCreatorRequest, UpdateLlmCreatorRequest Pydantic models - Add LlmProviderAdminResponse, LlmModelAdminResponse, LlmCreatorAdminResponse response models - Replace dict[str, Any] route return types with typed Pydantic models - Move all lazy import prisma.models calls to top-level - Move publish_registry_refresh_notification import to top-level in db_write - Remove hasattr check on CreateLlmModelRequest.costs (always present) - Rename provider_id -> provider_name in CreateLlmModelRequest to match actual semantics - Type toggle_model, create_creator, update_creator request bodies
|
|
|
Closing in favor of the LLM registry restack: the admin write API (now with AuditLog + source tracking) is now #13611 (fresh re-cut onto current dev — the original migrations and seed data had drifted ~4 months). The design and much of the code here carried over directly; @Bentlybro is credited as co-author on the carried commits. Full stack starts at #13605. Thanks for the groundwork — the reviewed schema and cache design survived contact with the restack almost unchanged. |
github-project-automation
Bot
moved this from 🚧 Needs work
to ✅ Done
in AutoGPT development kanban
3 participants
Summary
Add admin write API for LLM registry management - Part 4 of 5 in incremental rollout.
Builds on PR #12371 (public read API) to provide full CRUD operations for admins.
Changes
Admin Write Endpoints (`backend/server/v2/llm/admin_routes.py`)
All endpoints require admin authentication (`requires_admin_user`):
Models:
Providers:
Database Layer (`backend/server/v2/llm/db_write.py`)
Request Models (`admin_model.py`)
Features
✅ Admin auth required on all endpoints
✅ Registry cache refresh after every mutation
✅ Proper validation (400 for bad data, 404 for not found, 500 for errors)
✅ FK constraint enforcement
✅ Audit logging
✅ Based on original PR #11699 implementation
Testing
Manual testing scripts provided:
Requires admin JWT token. Full CRUD flow: create provider → create model → update → delete.
Stacked PRs