Just small inline comments so far; changes seem sane but I haven't pulled this down to test at all yet.
for (const int face_index : src_faces.index_range())?
Do we need an UNUSED_VARS here to silence a possible warning? I'm not really sure why I haven't seen any of the other unused vars in this file give a warning while doing other changes.
nit: I personally think BLI_assert(!face.is_empty()) is clearer
Ah, true. I forgot to put a note here that I was considering if a nested any would make this more readable.
Invalid is probably not the right word here - it's more that it seemed like something we didn't want. I think it's probably fine to expose this as an option but not set it for any of our keymaps,…
I have this defaulted to True for backwards compatibility if there are scripts using this, but I think defaulting to False probably makes more sense from a design perspective, interested in thoughts here.
I added this as a hidden property because it feels like it would lead to a confusing experience if users messed around with it, it doesn't seem necessary to expose