feat: add AI Bridge request logs model filter #21340
Conversation
Addressing feedback found in #21252 > [!IMPORTANT] > This pull-request removes endpoints from `ExperimentalHandler` from `coderd.go` and promotes the endpoints within the frontend. This means that we will no longer be serving AI Bridge under the `/api/experimental/` prefix now that things reached release in [`v2.29.0`](https://github.com/coder/coder/releases/tag/v2.29.0). > > ### Migration > > The `/api/experimental/aibridge` prefix has been removed. Any clients, scripts, or integrations that previously called AI Bridge endpoints under `/api/experimental/aibridge` must be updated to use the `/api/v2/aibridge` stable API routes introduced in v2.29.0. | Position | Pull-request | | -------- | ------------ | | | [fix: improve AI Bridge request logs UI/UX](#21252) | | | [feat: add AI Bridge request logs model filter](#21259) | | ✅ | [fix: promote AIBridge from `ExperimentalHandler`](#21278) | --------- Co-authored-by: Susana Ferreira <susana@coder.com>
b2a7d9b to
379b418
Compare
| -- Authorize Filter clause will be injected below in ListAIBridgeModelsAuthorized | ||
| -- @authorize_filter | ||
| GROUP BY | ||
| model |
There was a problem hiding this comment.
nit: SELECT DISTINCT model could be used instead of GROUP BY model
for me it is a bit easier to read.
| return nil, xerrors.Errorf("insert authorized filter: %w", err) | ||
| } | ||
|
|
||
| query := fmt.Sprintf("-- name: ListAIBridgeModels :many\n%s", filtered) |
There was a problem hiding this comment.
nit: -- name: ListAuthorizedAIBridgeModels
| ended_at IS NOT NULL | ||
| -- Filter model | ||
| AND CASE | ||
| WHEN @model::text != '' THEN aibridge_interceptions.model ILIKE '%' || @model::text || '%' |
There was a problem hiding this comment.
I'm a bit confused why we would want to filter model by model name? There shouldn't be that many unique models? Could we return all models and filter in the frontend (in doogfood there are distinct 28 models now)?
Problem with filtering by text in backend is efficiency, aibridge_interceptions table can be quite large.
If I understand correctly, current code will do full table scan.
To use existing B-tree index on model column filtering should be case sensitive prefix matching: aibridge_interceptions.model LIKE @model::text || '%'
or different index needs to be created: https://www.postgresql.org/docs/current/pgtrgm.html#PGTRGM-INDEX
Also if I understand correctly authorization will add some WHERE initiator_id = "..." clause. There is no composite index but I don't think it matters that much in this case as there are single column indexes on both initiator_id and model columns.
| // TODO: remove with Beta release. | ||
| r.Route("/aibridge", aibridgeHandler(api, apiKeyMiddleware)) | ||
| api.AGPL.ExperimentalHandler.Group(func(_ chi.Router) { | ||
| // Add enterprise-only experimental routes here |
There was a problem hiding this comment.
I don't like removing experimental paths here. It should be separate issue / PR.
Recreation of #21259
This implements a simple filter for querying against AI Bridge with a given model to the frontend UI. With a simple set of backend changes to return back the models which are filterable against.
Furthermore, I made a few changes to ensure that the imports of
/filterare more understandable/scoped, they now live underRequestLogsFilter.Preview
aibridge-model-filter.mp4
ExperimentalHandler<RequestLogsPrompt />)<SyntaxHighlighter />for AI BridgeToken Usages Metadata@codex