Copy link

Copy Markdown

Contributor

bryphe-coder approved these changes Feb 1, 2022

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@@            Coverage Diff             @@
##             main     #123      +/-   ##
==========================================
- Coverage   72.09%   72.03%   -0.06%     
==========================================
  Files          91       91              
  Lines        3773     3773              
  Branches       59       59              
==========================================
- Hits         2720     2718       -2     
- Misses        832      835       +3     
+ Partials      221      220       -1     

dependabot Bot deleted the dependabot/npm_and_yarn/types/node-14.18.10 branch February 1, 2022 23:04

Previously the mobile PR badge in the agent detail top bar showed the
full PR title, which got truncated and was unreadable on small screens.

Show only the PR number (e.g. #123) colored to match the PR state icon
(green for open, gray for draft, purple for merged, red for closed).
The desktop badge retains its existing icon + title behavior.

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:"#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._

Before the list resolved the board rendered as an empty board with
"0 chats · 0 cards", and an active search with no result yet, or a
failed one, admitted every card, so a broken search looked like an
unfiltered board. Loading and failure now render as such; prior search
results stay while the next search loads, and a failed background
refetch keeps the board (and any open note editor) mounted with the
failure shown inline.

Board storage is keyed by user id so accounts sharing a browser do not
inherit each other's columns and pinned windows. The old anonymous key
is not migrated; that state is browser-local by design.

Also from the second review round: the PR chip's accessible name keeps
its visible "#123", a comment claiming the list API caps pages at 200
is gone (only the messages endpoint does), narrating comments are cut
to the constraint they explain, a partial DOMRect fixture is a real
rectangle, a presence-only Inbox test is dropped in favour of the
command-level coverage, and one single-use span wrapper is inlined.

Before the list resolved the board rendered as an empty board with
"0 chats · 0 cards", and an active search with no result yet, or a
failed one, admitted every card, so a broken search looked like an
unfiltered board. Loading and failure now render as such; prior search
results stay while the next search loads, and a failed background
refetch keeps the board (and any open note editor) mounted with the
failure shown inline.

Board storage is keyed by user id so accounts sharing a browser do not
inherit each other's columns and pinned windows. The old anonymous key
is not migrated; that state is browser-local by design.

Also from the second review round: the PR chip's accessible name keeps
its visible "#123", a comment claiming the list API caps pages at 200
is gone (only the messages endpoint does), narrating comments are cut
to the constraint they explain, a partial DOMRect fixture is a real
rectangle, a presence-only Inbox test is dropped in favour of the
command-level coverage, and one single-use span wrapper is inlined.