You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
refactor: consolidate development endpoint helpers - #2242
IN-1331 to IN-1335 (#2236, #2237, #2238, #2239, #2240) kept their helpers route-local so five parallel PRs would not conflict. This PR hoists each duplicated piece into the shared modules and switches all six /v1-alpha routes (projects.ts and the five under development/) to them. Routes stay autoloaded from development/; every shared piece lives in lib/, schemas/ or clients/, and the routes no longer import the Tinybird client at all.
Wire behaviour is unchanged except where the ticket owner decided otherwise (below). The OpenAPI diff is limited to descriptions and the integer/number types listed under "OpenAPI diff".
Query-bound semantics: keep what is sent to Tinybird. Both bounds go out at 00:00:00 and the pipes compare with > / <. The docs now say so: DateRangeQuery.startDate is inclusive from 00:00 UTC, endDate is exclusive at 00:00 UTC of that day, matching the Insights widgets; each route description states the same rule. No numeric change.
ISO bucket bounds keep the time when a pipe returns a DateTime (pull-requests' toIsoUtc behaviour, the more general one): 2025-01-05 -> 2025-01-05T00:00:00Z, 2025-01-05 23:59:59 -> 2025-01-05T23:59:59Z. issues-resolution and active-days used to truncate; their pipes return Date columns, so real output is unchanged and their "at UTC midnight" bucket descriptions stay accurate.
Logging: one request.log.error per failed pipe call, with the pipe path in the message. The shared wrapper owns it. pull-requests moves from one log per request to one per failed pipe, like the other four.
Count summaries are Type.Integer; the percent share in contributions-outside-work-hours stays Type.Number.periodSummary takes the unit and the numeric kind. issues-resolution, pull-requests and active-days flip current/previous/changeValue from number to integer in the spec; the values were always whole numbers.
fetchPipe maps every failure to UpstreamUnavailableError (any status, network error, non-JSON body) and adopts commit-activities' row check: a non-array data, or a row failing the optional isRow predicate, becomes TinybirdInvalidResponseError inside the wrapper, so 503 too. Two consequences: projects.ts now answers 503 for a malformed projects_list body (was a TypeError 500), and a series row without bucket bounds is 503 on issues-resolution and pull-requests as well (was a TypeError 500 in their reshaping). withBucket resolves the bucket once per request; the in-memory cache makes it once per process per slug (TTL from the client library, 24h). The frontend uses a Nitro Redis adapter; a shared cache for the api can follow once it runs more than one instance.
InvalidDateRangeError sets this.name.
OpenAPI diff (/v1-alpha/openapi.json, main dfd5f4d9 vs this branch)
39 leaf differences, every one a description (24) or a type (15). No path, parameter, required, enum or ordering change.
5 route descriptions: one added sentence, "The period runs from 00:00 UTC on startDate up to, and excluding, 00:00 UTC on endDate." (folded into the existing UTC-timestamp sentence on contributions-outside-work-hours). pull-requests also states its call count as "up to 11 Tinybird calls", with the project lookup only when the process has no cached bucket for the slug (Copilot review, 150d995).
10 startDate / endDate parameter descriptions (all five routes): inclusive / exclusive wording from decision 1.
2 granularity descriptions (issues-resolution, pull-requests) now read "Width of each bucket in data." like commit-activities and active-days already did.
6 summary field descriptions from the factory wording: active-days previous (gains the comparison-period sentence) and changeValue; commit-activities changeValue ("(count of commits)" -> "(count)"); contributions-outside-work-hours current/previous ("in percent" -> "(percent)") and changeValue ("(percentage points)").
Dumped with scripts/export-openapi.ts on both commits and compared leaf by leaf; re-checked after 150d995.
Verification (in api/)
pnpm test: 20 files, 412 tests (main: 19 files, 340). New: tests/tinybird-client.test.ts (fetchPipe, withBucket, cache wiring, in-memory adapter), plus periodSummary, describe, SeriesQuery, DateRangeQuery wording, toTinybirdRange, toIsoUtc, utcMidnight, InvalidDateRangeError.name, and a check on the served /v1-alpha spec for the wording and integer types above.
Existing route tests: assertions unchanged. The only edit is a beforeEach that clears the bucket cache in the five development test files, so each test still counts its own project_buckets lookup regardless of order.
Size
1732 lines: 1038 in api/src (six routes shrink; three shared modules and the 41-line adapter grow; pull-requests moves its pipe fan-out into the withBucket callback, which re-indents about 60 lines) and 694 in api/tests (unit tests for every new shared helper). Over the 1000-line target, one concern: the runtime consolidation. Splitting it per piece would leave the routes half-switched between PRs.
Left for follow-ups
Test scaffolding (ticket piece 7, api/tests/helpers/tinybird.ts: app builder, env stubs, fetch stub routed by pipe path, response builder, pipe-call recorder, OpenAPI types): second PR after this one merges. That PR should also pin the two new 503 paths from decision 5 at route level (issues-resolution and pull-requests series row without bounds; projects_list non-array data); the mechanism is unit-tested in tests/tinybird-client.test.ts today.
issues-resolution is the only route that forwards omitted startDate/endDate as absent to its current-period pipe calls (its test pins that); the other four send the resolved default range. Kept as is.
repoFilter is applied on all five development routes (b48e818, after review). A bare ?repos= reached Ajv as [''] and went to the pipes as repos=, a filter matching nothing; each route now drops empty values and pins it with two tests.
describe() has no route call site left once SeriesQuery and periodSummary exist; kept per the ticket with a unit test. Drop it if no endpoint picks it up.
Library-level: give the Tinybird client the Fastify logger (register it as a plugin) so its own console lines go through pino; move the in-memory adapter into libs/tinybird-client and use it as the frontend's local fallback; move the Array.isArray(data) check into the library's parseResponse.
This newly maps a series row without bucket bounds to 503, but the route suite does not exercise that behavior. Add a route-level malformed-row case so removing or miswiring this predicate cannot pass the shared helper tests unnoticed.
Verify route passes missing-bounds predicate to series requests
The new 503 path for a series row without bucket bounds is only covered at the shared-helper level. Add a route-level malformed-row case to verify this endpoint passes the predicate to each series request.
Add project route test for non-array projects data
api/src/versions/v1-alpha/projects.ts:51
Switching to fetchPipe newly maps a non-array projects_list.data response to 503, but the project endpoint failure suite does not cover that path. Add a route-level malformed-data regression test alongside its existing Tinybird failure cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
themarolt
left a comment
Summary
IN-1331 to IN-1335 (#2236, #2237, #2238, #2239, #2240) kept their helpers route-local so five parallel PRs would not conflict. This PR hoists each duplicated piece into the shared modules and switches all six
/v1-alpharoutes (projects.tsand the five underdevelopment/) to them. Routes stay autoloaded fromdevelopment/; every shared piece lives inlib/,schemas/orclients/, and the routes no longer import the Tinybird client at all.Wire behaviour is unchanged except where the ticket owner decided otherwise (below). The OpenAPI diff is limited to descriptions and the integer/number types listed under "OpenAPI diff".
What moved where
YYYY-MM-DD 00:00:00range formatter (5:toTinybirdDay,toTinybirdRangex3,toTinybirdDateTime+tinybirdRange)toTinybirdRange(range)inapi/src/lib/period.ts; takes aPartial<DateRange>so issues-resolution can keep forwarding an omitted date as absenttoUtcMidnightx2,toUtcDateTime,toIsoUtc)toIsoUtc(value)inlib/period.ts;utcMidnight(day)is exported next to ithasBounds,hasBucketDates)hasBucketBounds(row)inlib/period.ts; active-days filters with it, the threeactivities_countseries routes pass it tofetchPipefromTinybirdx3,rowsOf+ inline catch, inline catch x2)fetchPipe<T>(request, path, params, isRow?)inapi/src/clients/tinybird.tswithBucket(request, slug, query)inclients/tinybird.ts; resolvesnullfor a slug without a bucket so the route answers zeros without a pipe call?repos=guard (2 had it, 3 forwardedrepos=raw)repoFilter(repos)inclients/tinybird.ts, applied on all five routesproject_bucketson every request)createInMemoryBucketCache()inapi/src/clients/bucket-cache.ts, wired intogetTinybirdClient()PeriodSummary(5) +describe()(2)periodSummary({ measure, unit, changeUnit?, kind, title?, description? })anddescribe()inapi/src/schemas/common.tsgranularityquery param (4)descriptionon the sharedGranularity;SeriesQuery=DateRangeQuery+ requiredgranularityInvalidDateRangeErrornever setthis.name(nit from IN-1330)Decisions (settled once here; raised on #2237, #2238, #2239, #2240)
00:00:00and the pipes compare with>/<. The docs now say so:DateRangeQuery.startDateis inclusive from 00:00 UTC,endDateis exclusive at 00:00 UTC of that day, matching the Insights widgets; each route description states the same rule. No numeric change.toIsoUtcbehaviour, the more general one):2025-01-05->2025-01-05T00:00:00Z,2025-01-05 23:59:59->2025-01-05T23:59:59Z. issues-resolution and active-days used to truncate; their pipes return Date columns, so real output is unchanged and their "at UTC midnight" bucket descriptions stay accurate.request.log.errorper failed pipe call, with the pipe path in the message. The shared wrapper owns it. pull-requests moves from one log per request to one per failed pipe, like the other four.Type.Integer; the percent share in contributions-outside-work-hours staysType.Number.periodSummarytakes the unit and the numeric kind. issues-resolution, pull-requests and active-days flipcurrent/previous/changeValuefromnumbertointegerin the spec; the values were always whole numbers.fetchPipemaps every failure toUpstreamUnavailableError(any status, network error, non-JSON body) and adopts commit-activities' row check: a non-arraydata, or a row failing the optionalisRowpredicate, becomesTinybirdInvalidResponseErrorinside the wrapper, so 503 too. Two consequences:projects.tsnow answers 503 for a malformedprojects_listbody (was a TypeError 500), and a series row without bucket bounds is 503 on issues-resolution and pull-requests as well (was a TypeError 500 in their reshaping).withBucketresolves the bucket once per request; the in-memory cache makes it once per process per slug (TTL from the client library, 24h). The frontend uses a Nitro Redis adapter; a shared cache for the api can follow once it runs more than one instance.InvalidDateRangeErrorsetsthis.name.OpenAPI diff (
/v1-alpha/openapi.json, maindfd5f4d9vs this branch)39 leaf differences, every one a
description(24) or atype(15). No path, parameter,required,enumor ordering change.startDateup to, and excluding, 00:00 UTC onendDate." (folded into the existing UTC-timestamp sentence on contributions-outside-work-hours). pull-requests also states its call count as "up to 11 Tinybird calls", with the project lookup only when the process has no cached bucket for the slug (Copilot review, 150d995).startDate/endDateparameter descriptions (all five routes): inclusive / exclusive wording from decision 1.granularitydescriptions (issues-resolution, pull-requests) now read "Width of each bucket indata." like commit-activities and active-days already did.previous(gains the comparison-period sentence) andchangeValue; commit-activitieschangeValue("(count of commits)" -> "(count)"); contributions-outside-work-hourscurrent/previous("in percent" -> "(percent)") andchangeValue("(percentage points)").number->integer: issues-resolution summary (3), pull-requests opened/merged/closed summaries (9), active-days summary (3). Decision 4.Dumped with
scripts/export-openapi.tson both commits and compared leaf by leaf; re-checked after 150d995.Verification (in
api/)pnpm test: 20 files, 412 tests (main: 19 files, 340). New:tests/tinybird-client.test.ts(fetchPipe,withBucket, cache wiring, in-memory adapter), plusperiodSummary,describe,SeriesQuery,DateRangeQuerywording,toTinybirdRange,toIsoUtc,utcMidnight,InvalidDateRangeError.name, and a check on the served/v1-alphaspec for the wording and integer types above.pnpm tsc-check,pnpm lint,pnpm format:check: green.beforeEachthat clears the bucket cache in the five development test files, so each test still counts its ownproject_bucketslookup regardless of order.Size
1732 lines: 1038 in
api/src(six routes shrink; three shared modules and the 41-line adapter grow; pull-requests moves its pipe fan-out into thewithBucketcallback, which re-indents about 60 lines) and 694 inapi/tests(unit tests for every new shared helper). Over the 1000-line target, one concern: the runtime consolidation. Splitting it per piece would leave the routes half-switched between PRs.Left for follow-ups
api/tests/helpers/tinybird.ts: app builder, env stubs, fetch stub routed by pipe path, response builder, pipe-call recorder, OpenAPI types): second PR after this one merges. That PR should also pin the two new 503 paths from decision 5 at route level (issues-resolution and pull-requests series row without bounds;projects_listnon-arraydata); the mechanism is unit-tested intests/tinybird-client.test.tstoday.startDate/endDateas absent to its current-period pipe calls (its test pins that); the other four send the resolved default range. Kept as is.repoFilteris applied on all five development routes (b48e818, after review). A bare?repos=reached Ajv as['']and went to the pipes asrepos=, a filter matching nothing; each route now drops empty values and pins it with two tests.describe()has no route call site left onceSeriesQueryandperiodSummaryexist; kept per the ticket with a unit test. Drop it if no endpoint picks it up.consolelines go through pino; move the in-memory adapter intolibs/tinybird-clientand use it as the frontend's local fallback; move theArray.isArray(data)check into the library'sparseResponse.Jira: IN-1347