[P1 · Robustness] Broad except Exception mislabels permanent dispatch errors as broker_failure and retries them forever.

The except Exception wrapping this call (below, ~L591) assumes a transient broker outage: it logs result=broker_failure and reverts the rows SENDING → PENDING to retry next tick. But it catches every exception type from dispatch_webhook_notification identically, including permanent ones:

• ValueError from enqueue_task (priority range / reply_key+callback mutual-exclusion validation)
• TypeError/serialization errors from _json_safe(...) on a non-encodable payload
• programming errors (AttributeError/KeyError) in the seam, or a DB error from the _org_identifier(org_id) lookup (evaluated inside the try as a call arg)

All of these are permanent — they will fail identically every flush tick, re-render + re-dispatch the group, and emit a full logger.exception traceback to Sentry mislabeled as a transport failure, until the dispatch-attempt cap eventually dead-letters it.

Fix: branch on error class — mark permanent errors (ValueError, serialization TypeError) DEAD_LETTER immediately with a distinct metric (e.g. result=dispatch_error), and keep the revert-to-PENDING path only for genuine transport/broker exceptions.

Minor related: a PG enqueue failure is logged with logger.exception both inside enqueue_task and again here → two Sentry issues per PG failure vs one on the Celery path. Pick one owner for the traceback.

[P1 · Tests] The PR's actual behavioral change here is untested.

The six new tests exercise dispatch_webhook_notification in isolation, but nothing covers this call site — where the two highest-risk regressions live:

1. The two-org-values contract. Routing uses the string id (organization_id=_org_identifier(org_id)) while the worker's buffer-mark contract uses the pk (kwargs["organization_id"]=org_id). Swapping them would pass every existing test yet silently (a) fail routing closed to Celery for every org (a pk never matches a Flipt string-id rule) and (b) strand rows in SENDING (the mark endpoint filters on the pk). Add a mock-based test asserting call.kwargs["organization_id"] == "<string id>" and call.kwargs["kwargs"]["organization_id"] == <pk>.
2. The revert-on-failure recovery. test_pg_enqueue_failure_propagates... only asserts the dispatch raises; the safety-critical half — _send_clubbed reverting only SENDING rows to PENDING, dispatched_at=None, dispatch_attempts F(-1), leaving already-DISPATCHED rows untouched — is untested. Best as a real DB test (seed one SENDING + one DISPATCHED row, patch dispatch to raise, assert the conditional update).
3. _org_identifier itself has no test: org found → string id (not "None"), org missing → None.

[P2 · Observability + comment accuracy] Silent None return, and the "org deleted" framing is misleading given the CASCADE FK.

NotificationBuffer.organization is on_delete=models.CASCADE, so deleting an Organization cascade-deletes its buffer rows — the "live buffer row + missing org" state this docstring describes is essentially unreachable in normal operation. If _org_identifier does return None, it signals a real data-integrity anomaly (dangling FK), yet it's swallowed with no log here; the only trace is resolve_transport's warning keyed on the random dispatch UUID, untraceable back to the org/rows.

Fix: log a warning (including org_pk) when the lookup returns None, and reword the comment to "data-anomaly guard" rather than an expected-deletion path.

Nits: "negligible before the webhook HTTP call it precedes" — the HTTP call happens later in the worker, not here; prefer "negligible relative to the downstream webhook dispatch." Also org_pk: Any could be narrowed to int (it's a pk throughout).

[P2 · Type design] The organization_id name is overloaded across the boundary — the core latent trap in this seam.

This param is the org string identifier (for Flipt routing), while kwargs["organization_id"] (passed by the caller) is the org pk (int, the worker buffer-mark contract). Same name, two incompatible meanings, both effectively untyped (str | None and a value inside dict[str, Any]). A future maintainer who "unifies" or swaps them gets silent mis-routing, not a type error: resolve_transport would str() the pk to "5", match no org segment, and quietly force Celery.

Cheapest high-value fix: rename the routing param to org_string_id / routing_org_identifier (one call-site edit) so the two identities can't be confused. Optionally encode them as distinct NewTypes (OrgStringId, OrgPk) to make a swap a type error at zero runtime cost.

[P2 · Comment accuracy] The return value's "usable by callers that surface it in an API response" is aspirational — no caller uses it.

The sole caller, _send_clubbed, is a fire-and-forget transaction.on_commit path and discards the return entirely; there is no API response in sight. The claim invites a future maintainer to believe a real consumer depends on it.

The rest of the Returns: block is accurate and worth keeping: I confirmed the function returns the minted dispatch_id (uuid str) on the PG path — deliberately not enqueue_task's int msg_id — giving Celery-parity str semantics, and three tests pin the dispatch_id == task_id == Flipt bucket invariant.

Fix: drop the parenthetical or mark it explicitly hypothetical ("returned for symmetry / any future caller").

[P3 · Simplify] organization_id or None is redundant.

resolve_transport already normalizes falsy input on its first line (if not organization_id: return celery), so "" and None resolve identically — the or None (converting "" → None) can't change the outcome. It's also unreachable in practice: the sole caller passes _org_identifier(...), whose .values_list(...).first() yields str | None, never "". Passing organization_id directly is behavior-preserving (the test_none_org_fails_closed_to_celery identity assertion still holds).

Note: the sibling org_id=organization_id or "" on the PG path is not redundant — it satisfies enqueue_task's org_id: str contract, so leave that one.