Skip to content

fix(signals): index reviewer identities even when the artefact fails schema validation #105187

Description

@andrewm4894

Problem

The suggested reviewer index (SignalReportSuggestedReviewer, added in #104619) drops a report's reviewers entirely when its current suggested_reviewers artefact fails schema validation.

_rows_for_artefact in products/signals/backend/suggested_reviewer_index.py calls SuggestedReviewers.model_validate_json(artefact.content) and returns no rows on ValidationError. The most common failure is string_too_long on relevant_commits[].reason, whose 500 character limit landed in #102550 on 2026-09-17. Artefacts written before that, and any agent that still writes a longer reason, fail validation whole. The report then has no rows in the index, so:

  • it is missing from the "For you" and teammate scopes for everyone named on it,
  • is_suggested_reviewer is false on it,
  • and every section count that applies the reviewer scope is short by one.

On one large project about 5% of reports with a reviewers artefact are in this state. The jsonb filter that #104619 replaced never validated the content, so it matched these reports. This is a silent regression in who sees what.

The live path has the same behavior as the backfill: a fresh report whose artefact fails validation gets no index rows at write time, and only a later reviewer write that happens to validate repairs it.

Proposal

The index only needs identities. Make _rows_for_artefact lenient about everything else:

  1. Try the full schema first, as now.
  2. On ValidationError, fall back to parsing the content as a JSON list and extracting user_uuid and github_login from each dict entry, skipping entries with neither. Keep the same dedupe (one row per (user_uuid, lowercased login)) and the same lowercasing.
  3. Keep the existing warning log, but make it say the artefact was indexed by identity only, so the two cases can be told apart in logs.
  4. Only when the content is not a JSON list at all should the artefact contribute no rows.

Consider the same lenience where else the schema gates reviewer identities on the read path, for example the helpers in views.py that derive logins, uuids and source skills from reviewer artefacts, so the index and those readers agree.

After the fix deploys, the index has to be rebuilt once for every project so the affected reports pick up their rows. #105174 covers making that rebuild automatic; until it lands, backfill_suggested_reviewer_index --team-id <id> does it per project.

Tests

Extend the existing filter tests in test_signal_report_artefact_api.py:

  • A reviewers artefact with a relevant_commits[].reason over 500 characters still makes the report findable through the suggested_reviewers list filter, and is_suggested_reviewer is true for the named user.
  • An artefact whose content is not a list still yields no rows and does not raise.

Context

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions