1. Let's create a dedicated type for holding both results. That way the value can be passed around, and the relationship between the values is clear when the return value is passed around ("these symbols were obtained in this uploadID").
2. nit: Go convention would be to use "ID", not "Id"

I don't agree with 1.

Once we also allow getting the symbol from the request arg rather than by reading a range in a syntactic index, we'll be getting an "unrelated" uploadId instead.

The uploadId is then also used to look up a bunch of other documents that have nothing to do with the symbols.

1. I know the logging code internally does some de-duplication, but I think since this loop (over stream.Results) is all in one function, it would be better to record these errors separately and emit a single log message after the loop is complete. E.g. you can collect the files seen multiple times in a collections.Set.
2. I think all of these should be Warn, and not Error. Is the code behaving in an unexpected way? Yes. Can we continue operating without worrying about some functionality not working properly? I'd say so, we can skip over these unexpected outputs.

Honestly at that point I'd rather just drop these asserts. I don't want to clutter our logic here with checking invariants in the search client/service. Sounds good?

We have tests for enry's code so that if something changes there in a way that would break us, we get to know sooner rather than later. I see these warnings in the same vein, especially since these are not documented post-conditions of the functions that we're calling.

Wrt https://github.com/sourcegraph/sourcegraph/pull/63268/commits/5ccd3a1deb97c949859896098cf2b6b5b2106014, I would prefer that we catch bugs sooner rather than later, so weakly against removing those checks, but leaving the final decision up to you.

8638879 looking good? I just assumed .String() on a set does something reasonable