Code Insights: use point-in-time search for drilldown#61953
Conversation
e54cfeb to
2fb33d3
Compare
| if len(defaults) == 0 { | ||
| return inputQuery, nil | ||
| } |
There was a problem hiding this comment.
No need to do a parse/stringify round trip if there are no defaults to apply. This is actually meaningful because parsing a rev: query without adding the repo: parts is invalid, which causes errors.
| } | ||
| } | ||
|
|
||
| func TestPointTimeQuery(t *testing.T) { |
There was a problem hiding this comment.
Copied directly from TestPointDiffQuery
peterguy
left a comment
There was a problem hiding this comment.
tested locally; looks great
vovakulikov
left a comment
There was a problem hiding this comment.
@camdencheek my apologies that I couldn't check this PR earlier, left some comments after manual testing, maybe I checked it wrongly but worth double-checking with you I think.
| return BasicQuery(query), nil | ||
| } | ||
|
|
||
| func PointInTimeQuery(diffInfo PointDiffQueryOpts) (BasicQuery, error) { |
There was a problem hiding this comment.
I don't think it's a regression since this code works in the same way as the diffQuery resolver (maybe it happened before, but demo staging works fine, not sure), but it looks like now it's possible to break the query that already has a repo filter.
I checked this on demo staging and this works in the following way
- If the insight's original query has a
repo:filter with specific repositories we just add post-process filters to the original repo filters - if the insight's original query has a
repo:.*filter when we add some post-process filters we just use these filters as repo: filter with norepo:.*in the result query
After this PR, I think the second case breaks; if you add repository post-filters to the insight with repo:* in the repo query, the result query will be repo:* repo:<post-process-repo-filter-value> which suddenly breaks the result for some reason.
To clarify when I'm talking about post-process filters I mean these filters form UI under the filter button in the insight card on the dashboard
cc @camdencheek
This implements a change in Code Insights drilldown behavior to show the set of results represented by that point rather than a diff search between the timestamps of the two points. This is built on top of the new
rev:at.time()filter.Closes https://github.com/sourcegraph/sourcegraph/issues/60739
Test plan
Copied all tests we already had for diffQuery. Manual end-to-end test which is shown in the attached video.
screenshot-2024-04-18_11-52-06.mp4