feat: support aibridge filtering by multiple clients#22231
feat: support aibridge filtering by multiple clients#22231
clients#22231Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
jakehwll
changed the title
clientclients
jakehwll
force-pushed
the
jakehwll/KIWI-20-aibridge-filter-multiple-clients
branch
from
c76f8f5 to
94f68d1
Compare
Documentation CheckUpdates Needed
Already Addressed
Automated review via Coder Tasks |
![chatgpt-codex-connector[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2adee55fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Emit each comma-separated part as its own key:value pair so | ||
| // the backend's searchTerms parser can handle them via | ||
| // url.Values.Add (repeated keys). Quote values with spaces. | ||
| for (const part of value.split(",")) { |
There was a problem hiding this comment.
Serializing by value.split(",") changes the meaning of any value that legitimately contains a comma. In AI Bridge this is especially risky for client, which is stored as free-form text from interception input; a single client like "Foo, Bar" is emitted as two separate client: terms on the next update, so exact matching no longer works and the selected filter can silently drift.
Useful? React with 👍 / 👎.
| // Matches key:value pairs where the value is either a quoted string | ||
| // or unquoted (no spaces). Repeated keys (e.g. client:A client:"B C") | ||
| // are merged into a comma-separated value. | ||
| const keyValuePair = /([\w-]+):("([^"]*)"|[^\s]+)/g; |
There was a problem hiding this comment.
This pattern has different character sets for different parts of the filter value ([\w-+], [^"], [^\s]), not sure if that's intentional? If our character set for filter keys/values is known, and consistent, I'd probably suggest making these all use the same ranges as much as possible.
| // the backend's searchTerms parser can handle them via | ||
| // url.Values.Add (repeated keys). Quote values with spaces. | ||
| for (const part of value.split(",")) { | ||
| const serialized = part.includes(" ") ? `"${part}"` : part; |
There was a problem hiding this comment.
There's potential for excess whitespace here. Should we consider "${part.trim()}" or similar?
There was a problem hiding this comment.
goalng changes LGTM
From manual testing the same issue with eager requests to backend on every input field value change.
Also when filter doesn't match anything spinning wheel is visible which is a bit confusing. Looks like request is in progress. I'd assume some Nothing matches string:
jakehwll
force-pushed
the
jakehwll/KIWI-39-filter-interceptions-by-model
branch
from
3683ce5 to
08fa8ae
Compare
jakehwll
force-pushed
the
jakehwll/KIWI-20-aibridge-filter-multiple-clients
branch
from
12975ca to
650ac3c
Compare
|
Closing in favour of #22694, the frontend doesn't have a pretty way to filter against multiple values and I want to look at this harder (I'm pretty unsatisfied with the current state of filtering here). |
3 participants
Adds a new
/aibridge/clientsGET endpoint to list AI Bridge clients with filtering and pagination support.GET /aibridge/clientsendpoint with authentication and paginationListAIBridgeClientsquery with authorization support and prefix-based client filtering