fix: address PR review on maintainer identity comparison (DE-980)

Four follow-ups from PR #4277 review, all aimed at preserving prior
behaviour rather than changing the feature:

- compare_and_update_maintainers: guard the end-date pass with a "still
  mentioned in source" check. The old github_username-keyed dict always
  included extracted maintainers regardless of identity-lookup outcome,
  so a transient resolution failure never wrongly end-dated a maintainer
  still present in the file. Restore that invariant under the new
  identity-based keying.
- get_maintainers_for_repo: add mi."endDate" IS NULL. Reinstates the
  reactivation path the old platform='github' filter incidentally
  provided for email-linked maintainers (end-dated rows fell out of the
  comparison set, hit the "new maintainer" branch, and got reactivated
  via upsert's ON CONFLICT ... SET "endDate" = NULL).
- insert_new_maintainers: restore concurrent upserts via
  asyncio.gather + Semaphore(MAX_CONCURRENT_CHUNKS). The earlier refactor
  accidentally serialised them, regressing first-run performance on
  large MAINTAINERS files.
- _resolve_maintainers: use self.MAX_CONCURRENT_CHUNKS instead of the
  hardcoded literal so concurrency tuning stays in one place.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>

Author: joanagmaia

services/apps/git_integration/src/crowdgit/database/crud.py

@@ -410,11 +410,17 @@ async def get_maintainers_for_repo(repo_id: str):
     # actually persists. A stricter read filter hides rows from the incremental diff,
     # which leads to spurious re-inserts and missed end-dates for email-linked
     # maintainers (e.g. platform='git' rows on the linux kernel repo).
+    #
+    # endDate IS NULL keeps the comparison set limited to active maintainers, matching
+    # the previous behaviour for email-linked maintainers (where the old platform filter
+    # incidentally excluded them and let the "new maintainer" branch reactivate them
+    # via the upsert's `endDate = NULL` on conflict). Without this, end-dated rows would
+    # sit in the comparison set and block reactivation when the maintainer reappears.
     maintainers_sql_query = """
         SELECT mi.role, mi."originalRole", mi."repoUrl", mi."repoId", mi."identityId", mem.value as github_username
             FROM "maintainersInternal" mi
             JOIN "memberIdentities" mem ON mi."identityId" = mem.id
-        WHERE mi."repoId" = $1 AND mem."deletedAt" is null
+        WHERE mi."repoId" = $1 AND mi."endDate" IS NULL AND mem."deletedAt" is null
         """
     return await query(
         maintainers_sql_query,

services/apps/git_integration/src/crowdgit/services/maintainer/maintainer_service.py

@@ -173,8 +173,9 @@ async def _resolve_maintainers(
     ) -> list[tuple[MaintainerInfoItem, str]]:
         # Centralised resolver used by both the first-run and incremental paths so
         # they share identical lookup + fallback semantics. The semaphore caps DB
-        # concurrency at 3 (large MAINTAINERS files can carry thousands of entries).
-        semaphore = asyncio.Semaphore(3)
+        # concurrency at MAX_CONCURRENT_CHUNKS (large MAINTAINERS files can carry
+        # thousands of entries).
+        semaphore = asyncio.Semaphore(self.MAX_CONCURRENT_CHUNKS)
 
         async def resolve(m: MaintainerInfoItem) -> tuple[MaintainerInfoItem, str | None]:
             async with semaphore:
@@ -195,14 +196,21 @@ async def insert_new_maintainers(
         self, repo_url: str, repo_id: str, maintainers: list[MaintainerInfoItem]
     ):
         resolved = await self._resolve_maintainers(maintainers)
-        for maintainer, identity_id in resolved:
-            role = maintainer.normalized_title
-            original_role = self.make_role(maintainer.title)
-            await upsert_maintainer(repo_id, identity_id, repo_url, role, original_role)
-            self.logger.info(
-                f"Successfully upserted maintainer {maintainer.github_username} "
-                f"with identity_id {identity_id}"
-            )
+        # Preserve the previous behaviour of running upserts concurrently so first-run
+        # processing of large MAINTAINERS files (thousands of entries) doesn't serialise.
+        semaphore = asyncio.Semaphore(self.MAX_CONCURRENT_CHUNKS)
+
+        async def upsert(maintainer: MaintainerInfoItem, identity_id: str) -> None:
+            async with semaphore:
+                role = maintainer.normalized_title
+                original_role = self.make_role(maintainer.title)
+                await upsert_maintainer(repo_id, identity_id, repo_url, role, original_role)
+                self.logger.info(
+                    f"Successfully upserted maintainer {maintainer.github_username} "
+                    f"with identity_id {identity_id}"
+                )
+
+        await asyncio.gather(*[upsert(m, identity_id) for m, identity_id in resolved])
 
     async def compare_and_update_maintainers(
         self,
@@ -243,13 +251,34 @@ async def compare_and_update_maintainers(
                 f"with identity_id {identity_id} role {role}"
             )
 
-        for identity_id, role in current_by_key:
-            if (identity_id, role) not in new_by_key:
-                self.logger.info(
-                    f"Maintainer with identity {identity_id} role {role} no longer exists, "
-                    f"updating its endDate..."
+        # Defensive guard for the end-date pass: the previous implementation keyed
+        # the "new" set by github_username, so any maintainer present in the source
+        # was unconditionally in that set and never wrongly end-dated. The identity-
+        # based key now only contains successfully resolved entries, so resolution
+        # failures (transient DB issues, stale extraction values) could end-date a
+        # maintainer who is still in the file. Skip end-dating when the current
+        # row's identifying value still appears in the extracted source.
+        mentioned_values = set()
+        for m in maintainers:
+            for v in (m.github_username, m.email):
+                if v and v != "unknown":
+                    mentioned_values.add(v.lower())
+
+        for (identity_id, role), current in current_by_key.items():
+            if (identity_id, role) in new_by_key:
+                continue
+            current_value = (current.get("github_username") or "").lower()
+            if current_value and current_value in mentioned_values:
+                self.logger.warning(
+                    f"Maintainer with identity {identity_id} role {role} could not be "
+                    f"re-resolved but is still mentioned in the source; skipping end-date"
                 )
-                await set_maintainer_end_date(repo_id, identity_id, role, change_date)
+                continue
+            self.logger.info(
+                f"Maintainer with identity {identity_id} role {role} no longer exists, "
+                f"updating its endDate..."
+            )
+            await set_maintainer_end_date(repo_id, identity_id, role, change_date)
 
     async def save_maintainers(
         self,