I'll note, this looks slightly redundant with the above TryGetAs. If the original bind_id isn't an AddrPattern, it'll fetch the same inst twice. I think that's the common case, too.

You might consider something like:

auto bind_inst = import_ir_.insts().Get(bind_id);
if (auto addr = bind_inst.TryAs<SemIR::AddrPattern>()) {
  bind_id = addr->inner_id;
  bind_inst = import_ir_.insts().Get(bind_id);
}
auto bind_const_id = bind_inst.Is<SemIR::BindSymbolicName>()

Although maybe this will be in the noise...

Done.

I was curious whether this would make any difference, so I took a look at the assembly. Looks like insts().Get is generally not getting inlined (probably because the CHECK makes it quite large), so my conclusion is... yes, this probably helps, but I'm sad that it's needed. Maybe we should use a DCHECK in ValueStore::Get?

Can bind_const_id ever point at an error? i.e., somewhat concerned that this might fail on an import of an interface with an invalid parameter. How tested are invalid generic imports?

If we get here then in the imported IR we had a BindSymbolicName, and the bind_const_id is the result of importing that BindSymbolicName, which can't produce an error. We only support a few very specific instruction patterns here, and hit a fatal error a few lines below if we see anything not matching those patterns (unchanged in this PR).

To your broader point, I don't think we have good checking for imports of invalid generics. Is that something we're generally testing and expecting to work currently? I can see tooling use cases where it'd be important, but it's not something I've really been thinking about.

The important thing to me is really just that check shouldn't crash. Imports of invalid generics shouldn't cause a crash. I've added tests for some various invalid import cases. I don't think you've added tests for that as part of adding generics.