Repository navigation
fix: production bugs in stars_cumulative_count pipe (CM-1785) - #4768
Conversation
… name collision (CM-1785) tb push rejects a node with the exact same name as its pipe - "Nodes can't have the same exact name as the Pipe they belong to." This pipe's last node was named stars_cumulative_count, identical to the pipe file, so it never actually deployed until now. Renamed to stars_cumulative_count_final, matching the convention every other multi-node pipe in this repo already follows (e.g. activities_count.pipe's activity_count_merged, activities_cumulative_count.pipe's cumulative_activity_count). No other file references the node by name - cross-pipe callers (stars_count.pipe's stars_count_bucketed) resolve by pipe name, not node name, so they're unaffected. Signed-off-by: Uroš Marolt <uros@marolt.me>
Granularity mode for `stars_cumulative_count` failed in production with `Unexpected type of filter column: Nullable ... ILLEGAL_COLUMN`. `ds.endDate` comes from `generate_timeseries.pipe`'s CASE WHEN block, which has no ELSE branch - ClickHouse types the column as Nullable(Date) even though it's always populated for a valid granularity value. The `arrayFirst` lambda in `stars_github_bucketed` compares against it and requires a strict, non-nullable UInt8 boolean result, which a Nullable comparison can't produce. Wrapped just this comparison in assumeNotNull() instead of touching generate_timeseries.pipe itself, since that pipe is shared infrastructure for every activity/contributor/metric pipe in the repo and widening its blast radius isn't warranted for a one-node fix. Verified against production data (opentf, longhorn, keystone projects, monthly granularity) after pushing to lfx_insights - previously failing queries now return correct, monotonic per-bucket totals in under 2s. Signed-off-by: Uroš Marolt <uros@marolt.me>
PR SummaryLow Risk Overview The terminal node is renamed from Pipe DESCRIPTION text is lightly edited (ticket refs removed); response shape and callers are unchanged. Reviewed by Cursor Bugbot for commit 7b0c981. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
Both targeted fixes are consistent with Tinybird behavior and have production validation.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes two deployment-blocking issues in the cumulative stars Tinybird pipe.
Changes:
- Handles nullable generated bucket end dates.
- Renames the terminal node to avoid a pipe-name collision.
| File | Description |
|---|---|
services/libs/tinybird/pipes/stars_cumulative_count.pipe |
Fixes nullable comparison and terminal node naming. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ulative-count-node-name-collision
…scription (CM-1785) Per review feedback: pipe DESCRIPTION blocks shouldn't carry ticket numbers that go stale once the work ships. Reworded the same points without the CM-XXX references. Signed-off-by: Uroš Marolt <uros@marolt.me>
Summary
Two bugs found while performance-testing
stars_cumulative_count.pipeagainst real projects in production (per request), both blocking the pipe entirely:stars_cumulative_count, identical to the pipe itself. Tinybird rejects this at push time (Nodes can't have the same exact name as the Pipe they belong to), so the pipe had never actually deployed until this fix. Renamed tostars_cumulative_count_final, matching the convention already used by every other multi-node pipe in this repo (e.g.activities_count.pipe'sactivity_count_merged). Cross-pipe callers resolve by pipe name, not node name, so nothing else needed to change.Unexpected type of filter column: Nullable ... ILLEGAL_COLUMN.generate_timeseries.pipe'sCASE WHENhas noELSE, soendDateis typedNullable(Date)even though it's always populated for a valid granularity. ThearrayFirstlambda instars_github_bucketedrequires a strict, non-nullableUInt8boolean, which aNullablecomparison can't produce. Fixed narrowly withassumeNotNull()on just that comparison, rather than touchinggenerate_timeseries.pipeitself (shared infra for every activity/contributor/metric pipe in the repo).Test plan
Both fixes pushed to
lfx_insights(production) and verified against real project data:opentf,longhorn,keystone- all return correct totals in 0.2-0.4s