Skip to content

feat: add pull requests endpoint under v1-alpha - #2240

Merged
epipav merged 5 commits into
mainfrom
feat/IN-1333-pull-requests
Sep 21, 2026
Merged

epipav merged 5 commits into
mainfrom
feat/IN-1333-pull-requests

Conversation

@epipav

@epipav epipav commented Sep 21, 2026 •

Copy link
Copy Markdown
Collaborator

What

Adds GET /v1-alpha/projects/{slug}/development/pull-requests, the port of the Nuxt pull-requests widget handler (frontend/server/api/widget/development/pull-requests.get.ts) to the api package.

Query: repos[], startDate, endDate (UTC calendar days, YYYY-MM-DD) and granularity (daily | weekly | monthly | quarterly | yearly, required).

Response:

  • openedSummary, mergedSummary, closedSummary: PeriodSummary blocks (current and previous counts, signed percentageChange, changeValue, ISO periodFrom/periodTo)
  • avgResolveTimeSeconds: average time to resolve a pull request in the period, in seconds; null when nothing was resolved
  • data[]: one { startDate, endDate, open, merged, closed } per granularity bucket, dates as ISO-8601 UTC

Changes against the Nuxt handler

  • The top-level summary is dropped; it duplicated openedSummary.
  • avgVelocityInDays is renamed avgResolveTimeSeconds; the pipe value was already in seconds.
  • percentageChange is signed (Nuxt used Math.abs).
  • Summary and bucket dates are ISO-8601 UTC; Nuxt returned '' and bare YYYY-MM-DD.
  • An unknown slug returns 200 with zero summaries and an empty series, per the epic's definition of done; Nuxt returned 404.
  • Every Tinybird failure, the bucket lookup included, returns 503 upstream_unavailable; Tinybird's own status never reaches the caller.
  • An empty repos= is dropped instead of being forwarded as a filter that matches nothing (Ajv coerces it to ['']); Nuxt dropped it too.

Decisions

  • Bucket resolved once per request. The Tinybird client resolves a bucketId before every query that carries project, so a naive port would make 20 HTTP calls. The route calls getBucketIdForProject once and passes the id to the ten pipe queries, which run concurrently with Promise.all as in Nuxt. A null bucket is the unknown-slug case. No response caching in this PR (T-090 load test tracks this endpoint); the route description states the 11-call cost.
  • granularity is required. Without one the activities_count pipe returns a single summary row instead of a series, so a missing value is a 400. This matches IN-1331, IN-1332 and IN-1334.
  • The resolved current range is always sent. With dates omitted the route sends 2010-01-01 through today, so periodFrom/periodTo describe the range that was queried.
  • Bucket key stays open. The ticket lists the renames explicitly; the field description states it is the count of pull requests opened in the bucket. Renaming it to opened is a candidate for the /v1 graduation review.
  • Series merged by bucket start over their union. Whether the pipe emits a row for an empty bucket is undocumented, so data covers every bucket start any of the three series reports, matched by the normalised start instant and ordered by it; a count missing from a series is 0. Nuxt anchored on the opened series and joined by index, which could misplace or drop merged and closed activity.

Helpers (Tinybird date formatting, ISO conversion, granularity query schema, 503 wrapper, bucket-once fan-out) live in the route file per the parallel-PR rules; the consolidation chore after the five Development PRs merge moves them to a shared module.

Tests

api/tests/development-pull-requests.test.ts: 33 cases, all green. Stubs global fetch at the HTTP boundary and routes by pipe, so the real Tinybird client builds the URLs, resolves the bucket and classifies the errors. Covers the exact 11 calls and their params, concurrency, the repos filter, defaults, unknown slug, empty data, validation (400), every Tinybird failure class (503), Cache-Control, the OpenAPI entry (tag, descriptions, nullable, required granularity) and field stripping.

pnpm test, pnpm tsc-check, pnpm lint and pnpm format:check pass in api/.

Includes the routing-test parser fix from the base branch (62b136b).

Based on main, which includes #2241 (IN-1348, directory-based route registration) as dd37a7a. Jira: IN-1333

Copilot AI balanced review requested due to automatic review settings September 21, 2026 11:31

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

🟡 Changes recommended

Index-based series merging can associate counts with incorrect buckets when a non-trailing row is absent.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds the v1-alpha pull-request analytics endpoint backed by Tinybird.

Changes:

  • Registers the new development route.
  • Adds summaries, resolution time, and bucketed activity mapping.
  • Adds comprehensive integration and OpenAPI tests.
File Description
api/​src/​versions/​v1-alpha/​index.ts Registers pull-request routes.
api/​src/​versions/​v1-alpha/​development/​pull-requests.ts Implements the endpoint and response schema.
api/​tests/​development-pull-requests.test.ts Tests behavior, failures, validation, and OpenAPI.

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

Comment thread api/src/versions/v1-alpha/development/pull-requests.ts
Copilot AI review requested due to automatic review settings September 21, 2026 11:44

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

🟡 Changes recommended

The response drops merged- or closed-only buckets when the opened series omits them.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Comment thread api/src/versions/v1-alpha/development/pull-requests.ts Outdated
Copilot AI review requested due to automatic review settings September 21, 2026 11:55

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 implementation is consistent with the stated contract and has comprehensive integration coverage.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Copilot AI review requested due to automatic review settings September 21, 2026 12:06

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

🔵 Needs a closer look

The response documentation misstates sparse bucket and call-count behavior, and the default-date test can fail across UTC midnight.

Review effort: Balanced
Findings: None

Previously missed (3)

In code that hasn't changed since last review

Medium severity Avoid UTC date mismatch when test crosses midnight

api/​tests/​development-pull-requests.test.ts:431

This captures the UTC day when the test module loads, while the handler derives it during the request. A run crossing UTC midnight can compare different dates; freeze time or accept the before/after days as period.test.ts:131-135 does.

Low severity Document that empty granularity buckets may be omitted

api/​src/​versions/​v1-alpha/​development/​pull-requests.ts:104

This promises an entry for every granularity bucket, but the merger only emits bucket starts returned by at least one series, so fully empty buckets can be omitted. Document the series as sparse so clients do not assume continuity.

Low severity Qualify call count as applying only to known projects

api/​src/​versions/​v1-alpha/​development/​pull-requests.ts:267

The call-count statement excludes the unknown-slug path: queryPipes returns after one bucket lookup, as the test at lines 471-480 asserts. Qualify the 11-call cost as applying to known projects.

@epipav epipav self-assigned this Sep 21, 2026

@gaspergrom gaspergrom left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Traced the bucket-merge logic and the Tinybird failure mapping against the tests, both hold up. Good coverage on the union-merge edge cases and the concurrency check.

Base automatically changed from feat/IN-1345-project-endpoint to main September 21, 2026 12:26
@epipav
epipav force-pushed the feat/IN-1333-pull-requests branch from b489cf7 to ce32033 Compare September 21, 2026 12:30
Copilot AI review requested due to automatic review settings September 21, 2026 12:40
@epipav
epipav force-pushed the feat/IN-1333-pull-requests branch from ce32033 to b00c15b Compare September 21, 2026 12:40

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

🟡 Changes recommended

The generated OpenAPI documentation contains incomplete and inaccurate comparison-period and call-count descriptions.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 Low severity

Open (2)

Comment thread api/src/versions/v1-alpha/development/pull-requests.ts Outdated
Comment thread api/src/versions/v1-alpha/development/pull-requests.ts Outdated
Copilot AI review requested due to automatic review settings September 21, 2026 12:54
@epipav
epipav force-pushed the feat/IN-1333-pull-requests branch from b00c15b to 2016746 Compare September 21, 2026 12:54
@epipav
epipav changed the base branch from main to refactor/IN-1348-autoload-routes September 21, 2026 12:55

@themarolt themarolt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm - the bucket-once fan-out and the tinybird date formatting line up with the nuxt handler the way the description says, nothing from me

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

🔵 Needs a closer look

The API documentation incorrectly claims comparison periods always have the same elapsed length.

Review effort: Balanced
Findings: 2 Low severity

Open (2)

Base automatically changed from refactor/IN-1348-autoload-routes to main September 21, 2026 14:16
Copilot AI review requested due to automatic review settings September 21, 2026 14:17
@epipav
epipav force-pushed the feat/IN-1333-pull-requests branch from 2016746 to 423837f Compare September 21, 2026 14:17

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

🔵 Needs a closer look

The OpenAPI descriptions inaccurately characterize comparison-period lengths and Tinybird call counts.

Review effort: Balanced
Findings: 2 Low severity

Open (2)

Signed-off-by: anilb <epipav@gmail.com>
Copilot AI review requested due to automatic review settings September 21, 2026 14:50

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 implementation matches the documented contract, addresses prior feedback, and has comprehensive integration coverage.

Review effort: Balanced
Findings: None

Resolved since last review (2)

@epipav
epipav merged commit cf83f27 into main Sep 21, 2026
11 checks passed
@epipav
epipav deleted the feat/IN-1333-pull-requests branch September 21, 2026 15:07
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.

4 participants