Can we make a note to change this to use StrEnum once we're 3.11+?

I wonder if it's worth defining a TypeAlias for this (perhaps just alias for List[FileAttachment]). It feels pretty repetitive.

This is great work.
You're going to scroll down and see a giant comment. Don't worry, it's mostly a deep dive on invariance/covariance in typing. But it explains how List[FileAttachment] is, non-obviously, the wrong type to use for a list of file attachments in many cases.
The rest are, for the most part, small doc-related things and some missing types.

Can you add a module docstring while here?

This should probably be a Sequence[FileAttachment], rather than a List[FileAttachment], and I'll go over why. It's a subtle thing, something I've been finding out the hard way about, and will be pointing out more in code reviews.
List, Dict, Set, Tuple, etc. are very specific types that represent these mutable objects and behaviors. They're great for returning when we really want the caller to have all the same behaviors that the function itself has, such as adding/deleting items. But they're less ideal when we want to return results intended just for reading.
Instead, we want to consider more general types, like Sequence, Mapping, and Iterator. These describe a subset of the behaviors, the read-only parts.
There are a couple benefits to using more general types here:

It conveys to the caller that the result is immutable/read-only.
These general types are covariant, whereas the specific types are invariant, and these affect how the types can be used.

Let's dig into what #2 means.

Variance
An invariant type is a specific type, and can only be assigned or passed in where that specific type is expected. A covariant type can be assigned that type or any subtype.
This is easier with examples:

class A:
    pass

class B(A):
    pass

class C(B):
    pass

# Lists are invariant:
l1: List[B] = [B(), C()]        # OK
l2: List[B] = [A()]             # Not OK: Supertype assigned
l3: List[A] = l1                # Not OK: List[A] and List[B] are invariant
l4: List[Optional[B]] = l1      # Not OK: Same reason

# Sequences are covariant:
s1: Sequence[B] = [B(), C()]    # OK
s2: Sequence[B] = [A()]         # Not OK: Supertype assigned
s3: Sequence[A] = s1            # OK: Sequence[A] is covariant, allowing subtype of Sequence[B]
s4: Sequence[Optional[B]] = s1  # OK: Same reason

# Let's try mixing-and-matching:
s5: Sequence[B] = l1            # OK: List[B] is a subtype of covariant Sequence[B]
l5: List[B] = s1                # Not OK: Sequence[B] is not a subtype of List[B].


(Note that the variance here is about the whole type, such as List[B], though that can be controlled if you were making your own Generics. Bigger topic.)
There's also contra-variance, which is the opposite of covariance (they can use a type or any parent type).
The reason for List being invariant is that l1 is a type that expects B or subclasses as items, any B or a subclass can be appended to it, and all read-related functions can therefore ensure a B or subclass. However, l3 expects A or subclasses, meaning a SubA could be appended in, which List[B] would not allow. That's why the types are not compatible in the l3 = l1 assignment above.
However, s1 doesn't have this problem. There are no mutable operations exposed on a Sequence (or the other general types I listed), so the guarantee is always that items of the type are going to be returend. So we can do s3 = s1 without a problem. s3 says that all items are a A or subclass, and since we can't mutate it, and Sequence[B] is a B or subclass, that fulfills the contract.
Same with Optional[...] in those types, because Optional[B] means B | None, which is a supertype of B, like A is a supertype of B.
(Does that make sense? It might not at first. There's a lot to wrap one's head around, and that only gets worse once you dive in further.)

Guidelines
We can follow two simple rules to keep this relatively sane:

Use the general types (Sequence, etc.) when returning or accepting values that are not/should not be modified.
Use the specific types when building the values or returning/accepting values meant to be modified.


In this case...
Now, it probably doesn't make sense to make a TypeAlias for every possible value, but it very much depends. djblets.util.typing definitely has mutable and immutable versions of these types. But depending on context here, we may want a List[FileAttachment], a Sequence[FileAttachment], or an Iterator[FileAttachment], and maybe even more. And then there's Optional and friends.
I know David mentioned that the Optional[List[FileAttachment]] was getting wordy, though, which is true. It's even more wordy with a Sequence. The problem in either case is that a TypeAlias here only saves two characters (especially if keeping the Optional[...]).
In some cases, if the type is really module-specific, a private TypeAlias defined in TYPE_CHECKING would be the way to go, but too many modules need this.
So it probably makes sense to define a FileAttachmentSequence and use that. If it ends up exceeding the line length limits, you can try:

def foo(
   ...,
   inactive_file_attachments: Optional[
       FileAttachmentSequence
   ] = None,
   ...
) -> ...:


Like that.
Thanks for reading all this. Good copy/paste fodder that I should probably put in Notion.

Can you add a module docstring?

Are these meant to be used as labels, or IDs? "State" reads to me like an internal value, but the contents read like human-readable.
If it's internal state IDs, let's use slug format (lowercase, - instead of spaces).
If it needs to be shown to users, we'd have to localize, so no enums. In this case, I'd still recommend we use slugs here, and then map that to localized strings elsewhere.

We should designate all these keyword-only arguments.

If the method is on the same class, you can leave off the class name.

Each of these should be typed as Set[Any].
Note the Any is actually correct, as django-stubs types the primary key as an Any. This is because technically, it can be a string, integer, UUID, or something else model/backend-specific.

The problem with the or here is that if the caller explicitly passes in an empty list, it'll cause a query. So some more verbose code's going to ultimately be needed here.
Same below.

I couldn't find the change.. Where does sort= come from?

These should be typed.

Should designate these keyword-only arguments.

I'd say we probably should avoid the **kwargs for this case, since this is going to always want to exactly match the other method.
(Alternatively we could just accept **kwargs and pass that in, but we'd lose all typing... Currently there's no way to say "the arguments of the function <blah>").

For here and the reference below, the full module path to ReviewRequest must be specified.
For this sort of case, where you want the displayed text to show the ClassName.method format for a class in another module, you must do:

:py:meth:`ReviewRequest.get_file_attachment_state()
<reviewboard.reviews.models.review_request.ReviewRequest.get_file_attachment_state>`


Note that:

The () should be on there when explicitly naming this, to match the normal output of :py:meth:
You can wrap at any . within the reference (custom extension of beanbag-docutils).

Missing , optional.

Missing a type.

Missing a type.

Ship It!