Skip to content

fix: repair repos filter in issue/PR resolve-velocity pipes (DE-1063) - #4782

Merged
gaspergrom merged 2 commits into
mainfrom
fix/DE-1063-tinybird-repos-filter-drift
Sep 28, 2026
Merged

gaspergrom merged 2 commits into
mainfrom
fix/DE-1063-tinybird-repos-filter-drift

Conversation

@gaspergrom

@gaspergrom gaspergrom commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Production had drifted from git for issues_average_resolve_velocity.pipe and pull_requests_average_resolve_velocity.pipe: someone hand-edited the repos filter branch directly on the deployed pipes to lower(channel) IN arrayMap(x -> lower(x), [...]), which ClickHouse rejects (Function 'in' is supported only if second argument is constant or table expression).
  • This broke the "Issues resolution" and "Pull requests" widgets on the Insights Development page for every single-repo view (confirmed on cilium/cilium and cilium/tetragon; generalizes to any project with a repos filter).
  • Fix restores a valid IN (SELECT lower(arrayJoin(...))) subquery form, which ClickHouse accepts and which preserves IN-1307's lowercase channel matching (verified against 0 mixed-case rows across 16.9M + 39.4M rows in the underlying datasources, and re-verified against a real uppercase-URL repo, 10up/ElasticPress).

Root cause

Insights-layer code was correct throughout (getBucketIdForProject, activityTypes_filtered defaults, segments_filtered project resolution were all ruled out as contributing causes). The bug was isolated entirely to a drifted Tinybird pipe definition — production had a different repos filter branch than what was committed on origin/main.

Validation (already deployed to production by crowd-tinybird-manager prior to this PR)

  • Before: both pipes returned 400 for repos-filtered calls.
  • After: both pipes return correct non-null numeric responses for the same params.
  • Generalization: re-tested against a second repo (cilium/tetragon) — same fix, same result.
  • Regression check: IN-1307 lowercase channel matching still holds (verified against a real mixed-case GitHub URL).
  • Staging: could not fully validate issues_average_resolve_velocity.pipe — staging's issues_analyzed datasource is missing the channel column entirely (pre-existing gap, unrelated to this fix, flagged separately).

Test plan

  • Reproduced the exact 400 against prod Tinybird for both pipes before the fix
  • Confirmed prod pipes return valid data after redeploy
  • Confirmed fix generalizes to a second repo
  • Confirmed IN-1307 lowercase matching is preserved
  • Staging validation blocked by a pre-existing missing-column gap (tracked separately, out of scope for this ticket)

JIRA: DE-1063 (https://linuxfoundation.atlassian.net/browse/DE-1063)

linuxfoundation/insights#2301

Production had drifted from git with an invalid lower(channel) IN
arrayMap(...) rewrite that ClickHouse rejects (400: Function 'in' is
supported only if second argument is constant or table expression),
breaking the Issues resolution and Pull requests widgets on every
single-repo Development page. Restore a valid IN-subquery form that
also preserves IN-1307's lowercase channel matching.

Signed-off-by: Gašper Grom <gasper.grom@gmail.com>
Copilot AI balanced review requested due to automatic review settings September 28, 2026 08:16
@cursor

cursor Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes production analytics SQL for Insights Development widgets; scope is limited to the optional repos filter but incorrect matching would skew velocity metrics for filtered views.

Overview
Repairs the optional repos filter in the issues and pull requests average resolve-velocity Tinybird pipes so single-repo Insights Development widgets work again.

When repos is passed, filtering no longer uses a bare channel IN {{ Array(repos) }} expression. It now uses channel IN (SELECT lower(arrayJoin(...))), which ClickHouse accepts as a table expression and still normalizes repo URLs to lowercase for matching (same intent as IN-1307).

No other parameters or query logic change; only the repos branch in both pipes is updated.

Reviewed by Cursor Bugbot for commit 641ba4d. Bugbot is set up for automated code reviews on this repo. Configure here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The predicates are valid, consistently formatted, and align with upstream channel normalization.

Review effort: Balanced
Findings: None

What changed in this PR

Repairs repository filtering in issue and pull-request resolution velocity Tinybird pipes.

Changes:

  • Uses a valid IN subquery for repository filtering.
  • Lowercases repository parameters to match normalized channel data.
File Description
pull_requests_average_resolve_velocity.pipe Fixes repository filtering for PR velocity.
issues_average_resolve_velocity.pipe Fixes repository filtering for issue velocity.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI review requested due to automatic review settings September 28, 2026 08:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The focused changes use valid ClickHouse syntax and preserve lowercase channel matching.

Review effort: Balanced
Findings: None

@themarolt themarolt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@gaspergrom
gaspergrom merged commit ed43d2e into main Sep 28, 2026
15 checks passed
@gaspergrom
gaspergrom deleted the fix/DE-1063-tinybird-repos-filter-drift branch September 28, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants