fix: expand Gerrit channel URLs in health score v2 maintainer and development pipes (IN-1229) - #4472
fix: expand Gerrit channel URLs in health score v2 maintainer and development pipes (IN-1229)#4472joanagmaia wants to merge 16 commits into
Conversation
…elopment pipes (IN-1229) Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
PR SummaryMedium Risk Overview Maintainer, development, lifecycle, and raw inputs snapshot pipes now join Repo population is aligned across v2 pipes: Reviewed by Cursor Bugbot for commit fa03845. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Pull request overview
Expands Gerrit repository URLs so Health Score v2 correctly associates Gerrit activity with repositories.
Changes:
- Adds plain and
/q/project:Gerrit channel mappings. - Routes maintainer and development metrics through expanded channels.
- Uses integration metadata for Gerrit detection and counts Gerrit merges toward bus factor.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
health_score_v2_maintainer.pipe |
Expands channels for maintainer metrics and Gerrit classification. |
health_score_v2_development.pipe |
Expands channels for development activity metrics. |
Suppressed comments (2)
services/libs/tinybird/pipes/health_score_v2_maintainer.pipe:75
- This second channel-producing branch also reads both ReplacingMergeTree sources without
FINAL, allowing stale versions to generate duplicate mappings and weight matched fact rows multiple times. Match the canonicalrepos_to_channels.pipe:41-42reads.
FROM repositories r
INNER JOIN integrations i ON r.sourceIntegrationId = i.id
services/libs/tinybird/pipes/health_score_v2_development.pipe:43
- Both tables in this Gerrit branch are ReplacingMergeTree sources. Without
FINAL, stale repository or integration versions create duplicate Gerrit mappings, and the downstream joins count matching facts more than once. This should followrepos_to_channels.pipe:41-42.
FROM repositories r
INNER JOIN integrations i ON r.sourceIntegrationId = i.id
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…-1229) Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
services/libs/tinybird/pipes/health_score_v2_development.pipe:17
- This is a change-log note describing what this PR just changed, which the project guideline explicitly disallows because it will become stale as the implementation evolves. Remove this historical note; the node itself should remain the source of truth.
- Gerrit channel fix (IN-1229): activity rows for Gerrit repos ingested via opendev.org (and
other Gerrit hosts) are stored under the /q/project: channel variant, not the plain repo URL.
Added a `channels` node that expands each repo URL to all its possible channel forms — matching
the logic in repos_to_channels.pipe — so joins against activityRelations, pull_requests_analyzed,
and issues_analyzed correctly match Gerrit activity.
services/libs/tinybird/pipes/health_score_v2_maintainer.pipe:42
- This is a change-log note describing what this PR just changed, which the project guideline explicitly disallows because it will become stale as the implementation evolves. Remove this historical note; the node itself should remain the source of truth.
- Gerrit channel fix (IN-1229): activity rows for Gerrit repos ingested via opendev.org (and
other Gerrit hosts) are stored under the /q/project: channel variant, not the plain repo URL.
Added a `channels` node that expands each repo URL to all its possible channel forms — matching
the logic in repos_to_channels.pipe — so joins against activityRelations, pull_requests_analyzed,
and issues_analyzed correctly match Gerrit activity. isGerrit now derived from
…229) Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (3)
services/libs/tinybird/pipes/health_score_v2_raw_inputs_snapshot.pipe:27
- This changelog-style note documents the change just made, which the repository guideline explicitly disallows in code comments. Remove it; IN-1229 and the PR description already preserve this history.
- Gerrit channel fix (IN-1229): mirrors the same fix applied to health_score_v2_maintainer.pipe
and health_score_v2_development.pipe — all activityRelations, pull_requests_analyzed, and
issues_analyzed joins now route through a `channels` node that expands each Gerrit repo URL to
its /q/project: variant so Gerrit activity is correctly matched.
services/libs/tinybird/pipes/health_score_v2_maintainer.pipe:42
- This changelog-style note documents the change just made, which the repository guideline explicitly disallows in code comments. Remove it; IN-1229 and the PR description already preserve this history.
- Gerrit channel fix (IN-1229): activity rows for Gerrit repos ingested via opendev.org (and
other Gerrit hosts) are stored under the /q/project: channel variant, not the plain repo URL.
Added a `channels` node that expands each repo URL to all its possible channel forms — matching
the logic in repos_to_channels.pipe — so joins against activityRelations, pull_requests_analyzed,
and issues_analyzed correctly match Gerrit activity. isGerrit now derived from
services/libs/tinybird/pipes/health_score_v2_development.pipe:17
- This changelog-style note documents the change just made, which the repository guideline explicitly disallows in code comments. Remove it; IN-1229 and the PR description already preserve this history.
- Gerrit channel fix (IN-1229): activity rows for Gerrit repos ingested via opendev.org (and
other Gerrit hosts) are stored under the /q/project: channel variant, not the plain repo URL.
Added a `channels` node that expands each repo URL to all its possible channel forms — matching
the logic in repos_to_channels.pipe — so joins against activityRelations, pull_requests_analyzed,
and issues_analyzed correctly match Gerrit activity.
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
services/libs/tinybird/pipes/health_score_v2_maintainer.pipe:51
- This duplicates the Gerrit URL expansion already owned by
repos_to_channels.pipe:35-88, and the same copy is added to the other two health-score pipes. Any future Gerrit URL rule must now be kept in four implementations or these scores will silently diverge. Please expose a shared repository-to-channel mapping node/pipe (includingrepoUrland the metadata needed here) and consume it from all three calculations.
SELECT
…N-1229) Introduce repos_channels_ds datasource and repos_channels_copy.pipe to pre-materialize the repo URL→channel expansion (plain + /q/project: variants for Gerrit). Health score v2 pipes now join repos_channels_ds instead of inlining the UNION ALL expansion in each pipe — eliminates duplication and avoids re-scanning repositories+integrations with FINAL once per join. Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
services/libs/tinybird/pipes/repos_channels_copy.pipe:48
- This refresh runs at 01:00, but
health_score_v2_raw_inputs_snapshot.piperuns at 00:30 on the first of each month (line 310). The snapshot therefore reads the previous day's mapping and can omit activity for repositories added or reclassified as Gerrit during that interval. Run this copy before 00:30 so the snapshot uses the current mapping.
COPY_SCHEDULE 0 1 * * *
…N-1229) - Drop 'Gerrit channel fix (IN-1229):' bullets from pipe DESCRIPTION blocks; history belongs in the PR, not inline documentation - Move repos_channels_copy.pipe schedule from 01:00 to 00:00 UTC so the channel mapping is fresh before the monthly snapshot at 00:30 on the 1st Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
…N-1229) repos_channels_ds is a secondary derived table — if empty on first deploy or after a failed copy job, building allRepos from it would wipe health_score_v2_maintainer_ds entirely. Source allRepos from repositories FINAL + integrations FINAL directly; use repos_channels_ds only for the activity joins where it's needed for Gerrit channel expansion. Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
services/libs/tinybird/pipes/repos_channels_copy.pipe:48
- The daily refresh makes this lookup stale for repository changes between 00:00 and the 02:00/02:10 score runs. A repo created after midnight is still present in the development/final live repository base, but absent from this mapping, so its activity joins miss and its maintainer row is omitted until the following day; platform, exclusion, and deletion updates have the same lag. Refresh this small lookup immediately before consumers (or inline the expansion) rather than claiming the midnight snapshot is always current.
COPY_SCHEDULE 0 0 * * *
services/libs/tinybird/pipes/health_score_v2_raw_inputs_snapshot.pipe:225
- These expanded lifecycle-related inputs now diverge from the label actually computed in
health_score_v2_lifecycle.pipe:115-175, which still groups activity relations, issues, and PRs by the raw channel. For Gerrit repositories this snapshot records commits and unanswered activity under the plain repo URL while the production lifecycle pipe sees zero, so validation can reconstruct a different lifecycle state. Route the lifecycle pipe through the same mapping as part of this change.
FROM activityRelations_deduplicated_cleaned_bucket_union ar
INNER JOIN repos_channels_ds ch ON ar.channel = ch.channel
WHERE ar.type = 'authored-commit'
…d signal detail pipes (IN-1229) Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (3)
services/libs/tinybird/pipes/health_score_v2_maintainer.pipe:191
- This removes every archived and excluded repository from the maintainer datasource, although the final combiner still includes all non-deleted repositories (
health_score_v2.pipe:64) and the security datasource still scores them (health_score_v2_security.pipe:120). Layer-2 rescaling can therefore inflate an archived repo's remaining security category to 100. It also makesisExcludedalways false and bypasses the documented blocked-responsiveness handling above. Keep these repositories in the category base.
WHERE r.deletedAt IS NULL AND r.archived = false AND r.excluded = false
services/libs/tinybird/pipes/health_score_v2_lifecycle.pipe:112
- Filtering excluded repositories here breaks the lifecycle datasource's one-row-per-repository contract:
health_score_v2.pipe:64still emits those repositories and its LEFT JOIN will now produce a NULL lifecycle label rather than their actual archived/activity-derived state. Keep all non-deleted repositories in this base, as before.
WHERE deletedAt IS NULL AND excluded = false
services/libs/tinybird/pipes/health_score_v2_development.pipe:157
- Dropping archived and excluded repositories from this category is inconsistent with the all-repository base in
health_score_v2.pipe:64. Their development category becomes unavailable rather than reflecting its real zero/stale signals, so the final score is rescaled over fewer category weights and can be overstated. Preserve the prior non-deleted repository population.
WHERE deletedAt IS NULL AND archived = false AND excluded = false
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (5)
services/libs/tinybird/pipes/health_score_v2_development.pipe:157
- Because
repositoriesretains replaced versions, filtering this non-FINALread can select a stale row where the repo was not yet archived or excluded. That defeats the eligibility change and continues materializing development scores for the repo.
SELECT DISTINCT url
FROM repositories
WHERE deletedAt IS NULL AND archived = false AND excluded = false
services/libs/tinybird/pipes/health_score_v2.pipe:68
repositoriesis aReplacingMergeTreekeyed by URL/version, so this predicate is currently applied to every historical version. A repo whose latest row is archived, excluded, or deleted can still enter the final output through an older eligible row;DISTINCTonly collapses the surviving URL afterward. Read the finalized row before enforcing the new eligibility policy.
SELECT DISTINCT url AS repoUrl
FROM repositories
WHERE deletedAt IS NULL AND archived = false AND excluded = false
services/libs/tinybird/pipes/health_score_v2_security.pipe:124
- This new eligibility filter runs before
ReplacingMergeTreeversions are collapsed. An olderarchived = false/excluded = falseversion therefore keeps a currently ineligible repo in the security-score datasource. UseFINAL, as the signal-detail and channel-copy queries already do.
SELECT DISTINCT url
FROM repositories
WHERE deletedAt IS NULL AND archived = false AND excluded = false
services/libs/tinybird/pipes/health_score_v2_impact.pipe:14
- The exclusion predicate is evaluated against all stored
ReplacingMergeTreeversions here. If a repo was recently changed toexcluded = true(or deleted), an older row still passes and its impact score remains materialized. Finalize the repository state before filtering.
FROM (SELECT DISTINCT url FROM repositories WHERE deletedAt IS NULL AND excluded = false) AS base
services/libs/tinybird/pipes/health_score_v2_lifecycle.pipe:112
- This non-
FINALread can return both old and current(url, archived)values from theReplacingMergeTree.DISTINCT url, archivedpreserves both tuples, so an archived repo can produce duplicate lifecycle rows (including conflicting archived/active labels), while a newly excluded repo can remain present through its old version.
SELECT DISTINCT url, archived
FROM repositories
WHERE deletedAt IS NULL AND excluded = false
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (5)
services/libs/tinybird/pipes/health_score_v2_development.pipe:157
- Because
repositoriesis a versionedReplacingMergeTree, this non-FINALfilter can see an olderarchived = false/excluded = falseversion alongside the current ineligible version. That leaves archived or excluded repositories in the development output until a merge happens. Readrepositories FINALfor deterministic eligibility.
FROM repositories
WHERE deletedAt IS NULL AND archived = false AND excluded = false
services/libs/tinybird/pipes/health_score_v2.pipe:68
repositoriesis aReplacingMergeTreeversioned byupdatedAt, so filtering mutablearchived/excludedvalues withoutFINALcan retain the URL through an older unmergedfalserow after the current row has been archived or excluded. The aggregate can therefore continue publishing a score for an ineligible repository; read the finalized repository state as the maintainer and signal-detail pipes do.
FROM repositories
WHERE deletedAt IS NULL AND archived = false AND excluded = false
services/libs/tinybird/pipes/health_score_v2_security.pipe:124
- This eligibility filter reads mutable columns from the versioned
repositoriesdatasource withoutFINAL. Until background merges remove the previous version, an archived or excluded repository still has an older eligible row and continues receiving a security score. Use the finalized repository state here.
FROM repositories
WHERE deletedAt IS NULL AND archived = false AND excluded = false
services/libs/tinybird/pipes/health_score_v2_lifecycle.pipe:112
- This reads mutable
archivedandexcludedfields from aReplacingMergeTreewithoutFINAL. If either value changes, old and current versions can coexist; an exclusion can leave the old eligible row, while an archive transition can produce both(url, false)and(url, true)and therefore duplicate/conflicting lifecycle rows. Read the finalized state beforeDISTINCT.
SELECT DISTINCT url, archived
FROM repositories
WHERE deletedAt IS NULL AND excluded = false
services/libs/tinybird/pipes/health_score_v2_impact.pipe:14
- Filtering
excludedon the non-finalizedReplacingMergeTreecan match a staleexcluded = falseversion after the repository has been excluded, so the impact pipe still emits the repository contrary to the new eligibility rule. UseFINALbefore applying the mutable-state filter.
FROM (SELECT DISTINCT url FROM repositories WHERE deletedAt IS NULL AND excluded = false) AS base
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 37eef68. Configure here.
… and security pipes (IN-1229) Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
…-1229) Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
…(IN-1229) Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
9e22e10 to
849feff
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (3)
services/libs/tinybird/pipes/health_score_v2.pipe:68
- This base still includes archived repositories, so the final copy emits archived rows (with category scores missing) even though this PR states archived repos are excluded from the aggregator. Add the archived predicate here to align the materialized output with the component eligibility rules.
WHERE deletedAt IS NULL AND excluded = false
services/libs/tinybird/pipes/health_score_v2_maintainer.pipe:187
- Newly recognized custom-domain Gerrit repos with no matching PR row now enter the Gerrit branch, where the existing
responsivenessAvailableexpression is false whenr.repoUrlis empty. Their explicit 0-point responsiveness score is therefore removed fromcoveredWeightand reweighted away, contradicting the updated description that no changeset data scores 0/15. Make responsiveness available for included Gerrit repos even when the PR aggregate is absent, so this broader classification does not turn a zero into a data gap.
(i.platform = 'gerrit') AS isGerrit,
services/libs/tinybird/pipes/health_score_v2_signal_detail.pipe:76
- Archived repositories are still materialized in the signal-detail datasource, contrary to the PR's eligibility rule. Because all three component pipes now omit archived repos, these rows contain only null/default signal values rather than useful detail; filter them from this base as well.
WHERE deletedAt IS NULL AND excluded = false
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
services/libs/tinybird/pipes/health_score_v2.pipe:68
- The PR states that archived repositories are excluded from the final aggregator, but this base only filters
excluded. As a result, the replace job still emits archived repo rows (with lifecycle/impact data and unavailable category scores). Add the archived predicate so the materialized output matches the declared eligibility policy.
WHERE deletedAt IS NULL AND excluded = false
services/libs/tinybird/pipes/health_score_v2_signal_detail.pipe:76
- Archived repositories are also supposed to be removed from the signal-detail datasource, but this base still includes them. Because all three upstream category pipes now exclude archived repos, this produces archived rows whose signal columns are entirely NULL instead of removing those repos.
WHERE deletedAt IS NULL AND excluded = false
Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (3)
services/libs/tinybird/pipes/health_score_v2.pipe:68
- The PR states that archived repositories are removed from the aggregator, but this base only filters
excluded. Because lifecycle and impact intentionally retain archived repositories, the joins still emit an aggregate row for each archived repo (with null category scores). Add the archived predicate here to enforce the documented eligibility rule.
WHERE deletedAt IS NULL AND excluded = false
services/libs/tinybird/pipes/health_score_v2_signal_detail.pipe:76
- Archived repositories are still included in the signal-detail base, contrary to the PR's stated rule that both the aggregator and signal-detail outputs exclude them. The three category datasets omit these repos, so this currently materializes misleading all-null detail rows.
WHERE deletedAt IS NULL AND excluded = false
services/libs/tinybird/pipes/health_score_v2_maintainer.pipe:191
- This new eligibility filter removes excluded repositories entirely, but the file description still says excluded repositories remain present with responsiveness marked
blocked(lines 12–14). That methodology documentation is now incorrect and should be removed or updated to describe output-level exclusion.
WHERE r.deletedAt IS NULL AND r.archived = false AND r.excluded = false
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
services/libs/tinybird/pipes/health_score_v2.pipe:68
- Archived repositories are still included in the final aggregator base. Since only the three category pipes filter them, this emits archived rows with null category scores (and potentially lifecycle/impact values), contrary to the PR's stated eligibility rule that the aggregator excludes archived repos. Add the archived predicate here.
WHERE deletedAt IS NULL AND excluded = false
services/libs/tinybird/pipes/health_score_v2_signal_detail.pipe:76
- The signal-detail base still admits archived repositories, so it materializes null-only detail rows for repos that the three category datasources now exclude. This contradicts the PR's stated requirement to filter archived repos from signal detail; add the same archived predicate used by the category pipes.
WHERE deletedAt IS NULL AND excluded = false

Summary
repos_channels_dsdatasource +repos_channels_copy.pipe(daily at 00:00 UTC) that materializes the full channel expansion for every repo: one row per non-Gerrit repo (channel = url) and two rows per Gerrit repo (plain URL +/q/project:variant used by Gerrit activity ingestion). UsesFINALonrepositories/integrations(both ReplacingMergeTree) to avoid stale-row duplicateshealth_score_v2_maintainer.pipe,health_score_v2_development.pipe,health_score_v2_lifecycle.pipe, andhealth_score_v2_raw_inputs_snapshot.pipethroughrepos_channels_dsso Gerrit repos onreview.opendev.org(Airship, StarlingX, Zuul, OpenStack, AGL) and other Gerrit hosts correctly match their activity dataisGerritdomain regex (review.opendev.org+gerrit.*) with anintegrations.platform = 'gerrit'lookup — correctly classifiesgit.opendaylight.organd any custom-domain Gerrit instancechangeset-mergedtoobservedActorsCounttype filter in maintainer + snapshot pipes so admin and cherry-pick merges count toward bus factorexcludedrepos are now filtered from all v2 score components (maintainer,security,development), from lifecycle, from impact, and from the aggregator + signal-detail pipes.archivedrepos are additionally filtered from the three v2 score components and from the aggregator + signal-detail pipes; lifecycle keeps them so it can emit thearchivedstate label, and impact keeps them so packages published by archived repos still get an impact scoreJira: IN-1229