feat: filter chats by diff URL via the q search parameter (#24970)

Adds a `diff_url:` term to the `q` search parameter on `GET
/api/experimental/chats` so callers can look up the chat associated with
a particular pull request, merge request, or any other URL persisted on
the chat's diff status.

```
q=diff_url:"https://github.com/coder/coder/pull/123"
```

Match is case-insensitive. When the URL lives on a delegated sub-agent's
diff status, the parent chat is returned so the relationship surfaces
from a single lookup.

<details>
<summary>Design notes</summary>

- **Forge-agnostic.** Reuses the existing `chat_diff_statuses.url`
column rather than introducing a `pr:` vocabulary, since the SDK already
documents the URL as "may point to a pull request or a branch page
depending on whether a PR has been opened." Works for GitHub PRs, GitLab
MRs, branch pages, etc.
- **Composes with `archived:`.** The two terms can be combined:
`q=archived:true diff_url:"..."`.
- **Case handling.** The parser used to lowercase the entire `q` string
up front, which would mangle URL path segments. Switched to lowercasing
only the field key inside `searchTerms` (already happens there) and
keeping the value as the caller typed it. The SQL comparison lowercases
on both sides.
- **Validation.** `diff_url` must be a syntactically valid HTTP(S) URL
with a non-empty host. No forge-specific validation.
- **Index.** Adds `idx_chat_diff_statuses_url_lower` on `LOWER(url)` so
the lookup is cheap even on large datasets.
- **Sub-agent fan-in.** `EXISTS` clause matches when the URL lives on
the chat itself or any chat with `root_chat_id` equal to the chat's id,
so a delegated sub-agent's PR pulls in its parent.
- **Deferred.** Sentinels like `pr:any` / `pr:none` and a forge-agnostic
state filter (`diff_state:open|merged|closed`) were intentionally left
out of this change. They couple cleanly to a second forge or a clearer
product call, and shipping them now would lock in vocabulary we may want
to revisit.

</details>

## Tests

- `coderd/searchquery`: parser tests for valid URLs, case handling (key
insensitive, value preserved), composition with `archived:`, and
validation errors (non-HTTP scheme, missing host, malformed URL).
- `coderd/exp_chats_test.go`: end-to-end coverage hitting `ListChats`.
Verifies a root chat matches its own URL, a parent chat surfaces when
only a sub-agent has the URL, lookups are case-insensitive, non-matching
URLs return empty, and invalid URLs return `400`.

---

_This PR was authored by a Coder Agent on behalf of @kylecarbs._
This commit is contained in:
Kyle Carberry
2026-05-13 11:06:42 -04:00
committed by GitHub
parent 8eb7051987
commit 5040ab6fca
14 changed files with 288 additions and 11 deletions
+1 -1
View File
@@ -78,7 +78,7 @@ const docTemplate = `{
"parameters": [
{
"type": "string",
"description": "Search query",
"description": "Search query. Supports archived:bool and diff_url:\u003curl\u003e terms (quote URLs).",
"name": "q",
"in": "query"
},
+1 -1
View File
@@ -59,7 +59,7 @@
"parameters": [
{
"type": "string",
"description": "Search query",
"description": "Search query. Supports archived:bool and diff_url:\u003curl\u003e terms (quote URLs).",
"name": "q",
"in": "query"
},
+2
View File
@@ -3851,6 +3851,8 @@ CREATE INDEX idx_chat_debug_steps_stale ON chat_debug_steps USING btree (updated
CREATE INDEX idx_chat_diff_statuses_stale_at ON chat_diff_statuses USING btree (stale_at);
CREATE INDEX idx_chat_diff_statuses_url_lower ON chat_diff_statuses USING btree (lower(url)) WHERE ((url IS NOT NULL) AND (url <> ''::text));
CREATE INDEX idx_chat_file_links_chat_id ON chat_file_links USING btree (chat_id);
CREATE INDEX idx_chat_files_org ON chat_files USING btree (organization_id);
@@ -0,0 +1 @@
DROP INDEX IF EXISTS idx_chat_diff_statuses_url_lower;
@@ -0,0 +1,5 @@
-- Index on LOWER(url) supports case-insensitive lookups when filtering
-- chats by their associated diff URL (e.g. a pull request URL).
CREATE INDEX idx_chat_diff_statuses_url_lower
ON chat_diff_statuses (LOWER(url))
WHERE url IS NOT NULL AND url <> '';
+1
View File
@@ -764,6 +764,7 @@ func (q *sqlQuerier) GetAuthorizedChats(ctx context.Context, arg GetChatsParams,
arg.Archived,
arg.AfterID,
arg.LabelFilter,
arg.DiffURL,
arg.OffsetOpt,
arg.LimitOpt,
)
+20 -2
View File
@@ -6992,6 +6992,22 @@ WHERE
WHEN $4::jsonb IS NOT NULL THEN chats.labels @> $4::jsonb
ELSE true
END
-- Match chats whose linked diff URL (e.g. a pull request URL)
-- equals the given value, case-insensitively. The URL may live on
-- a delegated sub-agent's diff status, so we surface the root chat
-- when any descendant matches.
AND CASE
WHEN $5::text IS NOT NULL THEN EXISTS (
SELECT 1
FROM chat_diff_statuses cds
JOIN chats c2 ON c2.id = cds.chat_id
WHERE cds.url IS NOT NULL
AND cds.url <> ''
AND LOWER(cds.url) = LOWER($5::text)
AND (c2.id = chats.id OR c2.root_chat_id = chats.id)
)
ELSE true
END
-- Paginate over root chats only. Children are fetched
-- separately via GetChildChatsByParentIDs and embedded under
-- each parent. Other callers that need the full set should
@@ -7008,11 +7024,11 @@ ORDER BY
-pin_order DESC,
updated_at DESC,
id DESC
OFFSET $5
OFFSET $6
LIMIT
-- The chat list is unbounded and expected to grow large.
-- Default to 50 to prevent accidental excessively large queries.
COALESCE(NULLIF($6 :: int, 0), 50)
COALESCE(NULLIF($7 :: int, 0), 50)
`
type GetChatsParams struct {
@@ -7020,6 +7036,7 @@ type GetChatsParams struct {
Archived sql.NullBool `db:"archived" json:"archived"`
AfterID uuid.UUID `db:"after_id" json:"after_id"`
LabelFilter pqtype.NullRawMessage `db:"label_filter" json:"label_filter"`
DiffURL sql.NullString `db:"diff_url" json:"diff_url"`
OffsetOpt int32 `db:"offset_opt" json:"offset_opt"`
LimitOpt int32 `db:"limit_opt" json:"limit_opt"`
}
@@ -7035,6 +7052,7 @@ func (q *sqlQuerier) GetChats(ctx context.Context, arg GetChatsParams) ([]GetCha
arg.Archived,
arg.AfterID,
arg.LabelFilter,
arg.DiffURL,
arg.OffsetOpt,
arg.LimitOpt,
)
+16
View File
@@ -377,6 +377,22 @@ WHERE
WHEN sqlc.narg('label_filter')::jsonb IS NOT NULL THEN chats.labels @> sqlc.narg('label_filter')::jsonb
ELSE true
END
-- Match chats whose linked diff URL (e.g. a pull request URL)
-- equals the given value, case-insensitively. The URL may live on
-- a delegated sub-agent's diff status, so we surface the root chat
-- when any descendant matches.
AND CASE
WHEN sqlc.narg('diff_url')::text IS NOT NULL THEN EXISTS (
SELECT 1
FROM chat_diff_statuses cds
JOIN chats c2 ON c2.id = cds.chat_id
WHERE cds.url IS NOT NULL
AND cds.url <> ''
AND LOWER(cds.url) = LOWER(sqlc.narg('diff_url')::text)
AND (c2.id = chats.id OR c2.root_chat_id = chats.id)
)
ELSE true
END
-- Paginate over root chats only. Children are fetched
-- separately via GetChildChatsByParentIDs and embedded under
-- each parent. Other callers that need the full set should
+1
View File
@@ -182,6 +182,7 @@ sql:
api_version: APIVersion
avatar_url: AvatarURL
created_by_avatar_url: CreatedByAvatarURL
diff_url: DiffURL
dbcrypt_key: DBCryptKey
session_count_vscode: SessionCountVSCode
session_count_jetbrains: SessionCountJetBrains
+2 -1
View File
@@ -312,7 +312,7 @@ func (api *API) chatsByWorkspace(rw http.ResponseWriter, r *http.Request) {
// @Security CoderSessionToken
// @Tags Chats
// @Produce json
// @Param q query string false "Search query"
// @Param q query string false "Search query. Supports archived:bool and diff_url:<url> terms (quote URLs)."
// @Param label query string false "Filter by label as key:value. Repeat for multiple (AND logic)."
// @Success 200 {array} codersdk.Chat
// @Router /api/experimental/chats [get]
@@ -368,6 +368,7 @@ func (api *API) listChats(rw http.ResponseWriter, r *http.Request) {
Archived: searchParams.Archived,
AfterID: paginationParams.AfterID,
LabelFilter: labelFilter,
DiffURL: searchParams.DiffURL,
// #nosec G115 - Pagination offsets are small and fit in int32
OffsetOpt: int32(paginationParams.Offset),
// #nosec G115 - Pagination limits are small and fit in int32
+130
View File
@@ -1409,6 +1409,136 @@ func TestListChats(t *testing.T) {
require.Len(t, c.Children, 2, "each root should embed its 2 children")
}
})
t.Run("DiffURLFilter", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client, db := newChatClientWithDatabase(t)
user := coderdtest.CreateFirstUser(t, client.Client)
modelConfig := createChatModelConfig(t, client)
// Helper that creates a chat (root or child) with a diff status URL.
create := func(title, url string, parentID uuid.NullUUID) database.Chat {
rootID := parentID
chat := dbgen.Chat(t, db, database.Chat{
OrganizationID: user.OrganizationID,
OwnerID: user.UserID,
LastModelConfigID: modelConfig.ID,
Title: title,
ParentChatID: parentID,
RootChatID: rootID,
})
if url != "" {
staleAt := time.Now().UTC().Add(time.Hour).Truncate(time.Second)
_, err := db.UpsertChatDiffStatusReference(
dbauthz.AsSystemRestricted(ctx),
database.UpsertChatDiffStatusReferenceParams{
ChatID: chat.ID,
Url: sql.NullString{String: url, Valid: true},
GitBranch: "feature/test",
GitRemoteOrigin: "git@github.com:coder/coder.git",
StaleAt: staleAt,
},
)
require.NoError(t, err)
}
return chat
}
// Root chat directly linked to the target PR.
rootWithPR := create("root with pr", "https://github.com/coder/coder/pull/1", uuid.NullUUID{})
// Root chat whose sub-agent owns the PR. The filter should still
// surface the parent because the URL lives on a descendant.
rootWithChildPR := create("root with child pr", "", uuid.NullUUID{})
_ = create(
"sub-agent with pr",
"https://github.com/coder/coder/pull/2",
uuid.NullUUID{UUID: rootWithChildPR.ID, Valid: true},
)
// Root chat with an unrelated PR; should not match either filter.
_ = create("unrelated pr", "https://github.com/coder/coder/pull/999", uuid.NullUUID{})
// Root chat with no diff status at all.
_ = create("no diff", "", uuid.NullUUID{})
// Archived root chat that points at the same URL as `rootWithPR`.
// Used to verify the archived filter and the diff_url filter
// compose at the SQL layer rather than ignoring each other.
archivedWithPR := create(
"archived with pr",
"https://github.com/coder/coder/pull/3",
uuid.NullUUID{},
)
require.NoError(t, client.UpdateChat(ctx, archivedWithPR.ID, codersdk.UpdateChatRequest{
Archived: ptr.Ref(true),
}))
t.Run("MatchesRoot", func(t *testing.T) {
chats, err := client.ListChats(ctx, &codersdk.ListChatsOptions{
Query: `diff_url:"https://github.com/coder/coder/pull/1"`,
})
require.NoError(t, err)
require.Len(t, chats, 1)
require.Equal(t, rootWithPR.ID, chats[0].ID)
})
t.Run("MatchesViaSubAgent", func(t *testing.T) {
chats, err := client.ListChats(ctx, &codersdk.ListChatsOptions{
Query: `diff_url:"https://github.com/coder/coder/pull/2"`,
})
require.NoError(t, err)
require.Len(t, chats, 1, "root chat should surface even when only a child has the PR")
require.Equal(t, rootWithChildPR.ID, chats[0].ID)
})
t.Run("CaseInsensitive", func(t *testing.T) {
chats, err := client.ListChats(ctx, &codersdk.ListChatsOptions{
Query: `diff_url:"HTTPS://GITHUB.COM/CODER/CODER/PULL/1"`,
})
require.NoError(t, err)
require.Len(t, chats, 1)
require.Equal(t, rootWithPR.ID, chats[0].ID)
})
t.Run("NoMatch", func(t *testing.T) {
chats, err := client.ListChats(ctx, &codersdk.ListChatsOptions{
Query: `diff_url:"https://github.com/coder/coder/pull/424242"`,
})
require.NoError(t, err)
require.Empty(t, chats)
})
t.Run("InvalidURL", func(t *testing.T) {
_, err := client.ListChats(ctx, &codersdk.ListChatsOptions{
Query: `diff_url:"ftp://example.com/x"`,
})
sdkErr := requireSDKError(t, err, http.StatusBadRequest)
require.NotEmpty(t, sdkErr.Validations, "expected validation error")
require.Equal(t, "diff_url", sdkErr.Validations[0].Field)
})
t.Run("ArchivedFilteredOut", func(t *testing.T) {
// Default archived filter is false, so an archived chat with
// a matching diff URL must not surface.
chats, err := client.ListChats(ctx, &codersdk.ListChatsOptions{
Query: `diff_url:"https://github.com/coder/coder/pull/3"`,
})
require.NoError(t, err)
require.Empty(t, chats, "archived chat must not match the default filter")
})
t.Run("ArchivedTrueComposes", func(t *testing.T) {
chats, err := client.ListChats(ctx, &codersdk.ListChatsOptions{
Query: `archived:true diff_url:"https://github.com/coder/coder/pull/3"`,
})
require.NoError(t, err)
require.Len(t, chats, 1)
require.Equal(t, archivedWithPR.ID, chats[0].ID)
})
})
}
func TestListChatModels(t *testing.T) {
+34 -2
View File
@@ -544,6 +544,9 @@ func Tasks(ctx context.Context, db database.Store, query string, actorID uuid.UU
//
// Supported query parameters:
// - archived: boolean (default: false, excludes archived chats unless explicitly set)
// - diff_url: string (matches chats whose linked diff URL equals the
// given value, case-insensitively; URLs typically contain ':' so
// they must be quoted, e.g. q=diff_url:"https://github.com/o/r/pull/1")
func Chats(query string) (database.GetChatsParams, []codersdk.ValidationError) {
filter := database.GetChatsParams{
// Default to hiding archived chats.
@@ -554,8 +557,10 @@ func Chats(query string) (database.GetChatsParams, []codersdk.ValidationError) {
return filter, nil
}
// Always lowercase for all searches.
query = strings.ToLower(query)
// Lowercase the keys so they match regardless of how the caller
// types them, but preserve value casing because some filters
// (e.g. diff_url) may include URL path segments where case is
// meaningful.
values, errors := searchTerms(query, func(term string, _ url.Values) error {
return xerrors.Errorf("unsupported search term: %q", term)
})
@@ -565,11 +570,38 @@ func Chats(query string) (database.GetChatsParams, []codersdk.ValidationError) {
parser := httpapi.NewQueryParamParser()
filter.Archived = parser.NullableBoolean(values, filter.Archived, "archived")
if diffURL := parser.String(values, "", "diff_url"); diffURL != "" {
if err := validateDiffURL(diffURL); err != nil {
parser.Errors = append(parser.Errors, codersdk.ValidationError{
Field: "diff_url",
Detail: err.Error(),
})
} else {
filter.DiffURL = sql.NullString{String: diffURL, Valid: true}
}
}
parser.ErrorExcessParams(values)
return filter, parser.Errors
}
// validateDiffURL checks that the value is a syntactically valid HTTP(S)
// URL. The check is intentionally forge-agnostic because the diff URL on
// a chat may point to a pull request, merge request, branch page, etc.
func validateDiffURL(raw string) error {
u, err := url.Parse(raw)
if err != nil {
return xerrors.Errorf("diff_url is not a valid URL: %w", err)
}
if u.Scheme != "http" && u.Scheme != "https" {
return xerrors.Errorf("diff_url must use http or https scheme, got %q", u.Scheme)
}
if u.Host == "" {
return xerrors.New("diff_url must include a host")
}
return nil
}
func searchTerms(query string, defaultKey func(term string, values url.Values) error) (url.Values, []codersdk.ValidationError) {
searchValues := make(url.Values)
+70
View File
@@ -1239,6 +1239,17 @@ func TestSearchChats(t *testing.T) {
Archived: sql.NullBool{Bool: true, Valid: true},
},
},
{
// Documents that uppercase boolean values still parse. The Chats
// parser intentionally does not pre-lowercase the query because
// diff_url path segments are case-meaningful, so this guards
// against regressions if the blanket lowercase is ever re-added.
Name: "ArchivedTrueUpperCase",
Query: "archived:TRUE",
Expected: database.GetChatsParams{
Archived: sql.NullBool{Bool: true, Valid: true},
},
},
{
Name: "ArchivedFalse",
Query: "archived:false",
@@ -1266,6 +1277,65 @@ func TestSearchChats(t *testing.T) {
Query: "archived:",
ExpectedErrorContains: "cannot start or end with ':'",
},
{
Name: "DiffURL",
Query: `diff_url:"https://github.com/coder/coder/pull/123"`,
Expected: database.GetChatsParams{
Archived: sql.NullBool{Bool: false, Valid: true},
DiffURL: sql.NullString{
String: "https://github.com/coder/coder/pull/123",
Valid: true,
},
},
},
{
Name: "DiffURLPreservesValueCase",
Query: `diff_url:"https://github.com/Coder/Coder/pull/123"`,
Expected: database.GetChatsParams{
Archived: sql.NullBool{Bool: false, Valid: true},
DiffURL: sql.NullString{
String: "https://github.com/Coder/Coder/pull/123",
Valid: true,
},
},
},
{
Name: "DiffURLKeyCaseInsensitive",
Query: `Diff_URL:"https://github.com/coder/coder/pull/1"`,
Expected: database.GetChatsParams{
Archived: sql.NullBool{Bool: false, Valid: true},
DiffURL: sql.NullString{
String: "https://github.com/coder/coder/pull/1",
Valid: true,
},
},
},
{
Name: "DiffURLWithArchived",
Query: `archived:true diff_url:"https://gitlab.com/foo/bar/-/merge_requests/9"`,
Expected: database.GetChatsParams{
Archived: sql.NullBool{Bool: true, Valid: true},
DiffURL: sql.NullString{
String: "https://gitlab.com/foo/bar/-/merge_requests/9",
Valid: true,
},
},
},
{
Name: "DiffURLInvalidScheme",
Query: `diff_url:"ftp://example.com/x"`,
ExpectedErrorContains: "http or https scheme",
},
{
Name: "DiffURLMissingHost",
Query: `diff_url:"https:///pull/1"`,
ExpectedErrorContains: "must include a host",
},
{
Name: "DiffURLMalformed",
Query: `diff_url:"http://%41:8080/"`,
ExpectedErrorContains: "not a valid URL",
},
}
for _, c := range testCases {
+4 -4
View File
@@ -17,10 +17,10 @@ Experimental: this endpoint is subject to change.
### Parameters
| Name | In | Type | Required | Description |
|---------|-------|--------|----------|----------------------------------------------------------------|
| `q` | query | string | false | Search query |
| `label` | query | string | false | Filter by label as key:value. Repeat for multiple (AND logic). |
| Name | In | Type | Required | Description |
|---------|-------|--------|----------|-----------------------------------------------------------------------------|
| `q` | query | string | false | Search query. Supports archived:bool and diff_url:<url> terms (quote URLs). |
| `label` | query | string | false | Filter by label as key:value. Repeat for multiple (AND logic). |
### Example responses