hackathon - search/notebook: basic end-to-end notebook result rendering

[bobheadxi]

This is a hackathon project, and is not being merged into main! See https://sourcegraph.com/notebooks/Tm90ZWJvb2s6NTE5

Implements end-to-end "rendering" of some stub Notebook results. For real results, see https://github.com/sourcegraph/sourcegraph/pull/33170

This was quite tedious - to define a new search thing, we have to:

1. create a job
2. create a match type
3. create an event type + adapters
4. copy the above to web app
5. create custom UI (all the search results UI are very tightly coupled to the result type) to handle this result type, adding to the mess

I had to extend Match.Key with an identifier that's not a repository, and bypass the repository permissions filter here: notebooks-search...notebooks-search-e2e-basic?expand=1#diff-99275f59b8

Thought: A generic match + event type + UI for generic match types would be really nice IMO. Then the only custom stuff we need to create is a job to extend search with new backends, and everything else would ideally take care of itself. I suspect extending this to include notebook blocks, then batch changes, then etc. will not be very pretty

My first thought is that there seem to be roughly 2 categories:

• "Entity" things (repos, commits, files, etc + notebooks)
• "Content" things that fall under an entity (content, symbols, etc + notebook blocks)

[sourcegraph-bot]

Codenotify: Notifying subscribers in CODENOTIFY files for diff 86ce3e1...5dec431.

Notify	File(s)
@beyang	internal/search/job/job.go internal/search/notebook/notebook.go internal/search/result/match.go internal/search/result/notebook.go internal/search/streaming/http/events.go
@camdencheek	cmd/frontend/internal/search/search.go internal/search/job/job.go internal/search/notebook/notebook.go internal/search/result/match.go internal/search/result/notebook.go internal/search/streaming/http/events.go
@keegancsmith	cmd/frontend/internal/search/search.go internal/search/job/job.go internal/search/notebook/notebook.go internal/search/result/match.go internal/search/result/notebook.go internal/search/streaming/http/events.go
@limitedmage	client/search-ui/src/components/SearchResult.tsx client/search-ui/src/results/StreamingSearchResultsList.tsx

[keegancsmith]

cc @rvantonder and @camdencheek who have been doing lots of stuff around how search results move around the frontend and likely have opinions :)

[rvantonder]

Yes, please share more context: what would you like to achieve, what motivated this change (is it Hackathon related?). It seems like you generally want to search notebooks--do you want to search particular parts of notebooks? Or are they just going to be treated like a serialized string/content? If you're not ready to share yet and just hacking, feel free to ignore, but since I was pinged I'll give some input.

Sourcegraph's toplevel result "type" is repositories: its content and associated metadata. A "notebook" result type diverges quite a bit from this view. Doesn't mean we can't support it, but the question becomes: what is the source of data for a notebook? Is it the Sourcegraph database? Will notebook search be confined to notebooks on a Sourcegraph instance?

The state of this PR looks draft-y, so maybe you're just getting feedback from Tomas for now, but please add @sourcegraph/search-product as a reviewer once you're interested in actually merging code. If you do want a review in this state, it's probably a good idea to share a bit more on the context/goals since this is basically a new product feature addition and very much part of the domain of @sourcegraph/search-product to help you with implementation/feedback.

---

Background/comments on implementation

This was quite tedious - to define a new search thing, we have to:

1. create a job
2. create a match type
3. create an event type + adapters

I hear you, but I have to chuckle because at least this is a reasonable and possible thing to add a result type and keep it clean from other search code. You should have seen the state of things a year+ ago.

Thought: A generic match + event type + UI for generic match types would be really nice IMO.

Not going to go into the UI side of things, but this has long been related to stuff that Camden and I have worked towards for months (for me, years). Honestly, this area of search code is still being revamped so it's unlikely to shape up the way you envision soon. It sounds straightforward but it just takes time to unentangle everything. We just recently moved search out of graphql resolver code so people don't have to deal with that!

[bobheadxi]

Thanks for the detailed response Rijnard! I'll take a closer look tomorrow, but just wanted to share a quick response in case this PR comes across as concerning 😆

Yes, please share more context: what would you like to achieve, what motivated this change (is it Hackathon related?).

This is @tsenart and I's hackathon project yes! This PR is going to notebooks-search (not main), where it will stay for the duration of this hackathon!

It seems like you generally want to search notebooks--do you want to search particular parts of notebooks? Or are they just going to be treated like a serialized string/content?

Yes, we want to expose search synonymous to repo and file contents, i.e. notebooks (by name) and notebook blocks (by content match or similar). More details in our notes here: https://sourcegraph.com/notebooks/Tm90ZWJvb2s6NTE5

Follow-up PR that fleshes the implementation out: https://github.com/sourcegraph/sourcegraph/pull/33170

[rvantonder]

all good, happy hacking!

[bobheadxi]

Sourcegraph's toplevel result "type" is repositories: its content and associated metadata. A "notebook" result type diverges quite a bit from this view.

Yeah I had a feeling here as well, hence the addition of Key.ID here with a mind towards arbitrary non-repo-key'd data. Might need some more thinking here

Doesn't mean we can't support it, but the question becomes: what is the source of data for a notebook? Is it the Sourcegraph database? Will notebook search be confined to notebooks on a Sourcegraph instance?

I think the answer at the moment is yes for all of the above, based on the design of notebooks (there's .snb.md but that's covered by normal search as part of repo content already, so nothing to do here perhaps?)

I hear you, but I have to chuckle because at least this is a reasonable and possible thing to add a result type and keep it clean from other search code. You should have seen the state of things a year+ ago.

Yeah, I complain about copying types and adapters but the state of things on the backend here is honestly pretty nicely set up for extending in this manner 😁 Great work here, I've been peeking at a lot of the work in this area and it seems like quite the undertaking!

(I'll add that tedious but not painful is a pretty good state of things in my mind 😛 )

It sounds straightforward but it just takes time to unentangle everything.

Definitely my first impression here as well! My comment regarding the generic result type is more along the lines of something that could exist alongside the existing types and be somewhat synonymous with what's there already, i.e. instead of building out a bunch of notebook-specific stuff we could generalize it more, which was an idea we threw around