Skip to content

feat: collections groundwork - #2337

Merged
epipav merged 5 commits into
mainfrom
feat/IN-1403
Oct 1, 2026
Merged

epipav merged 5 commits into
mainfrom
feat/IN-1403

Conversation

@epipav

@epipav epipav commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

First part of the shared groundwork for the Collections endpoints (IN-1403, epic IN-1146). The API gets read access to the CM database, which holds collections, plus the shared response schemas the collection routes will use.

  • libs/postgres-client: a new workspace lib with createPostgresPool. It carries the pool settings the frontend already used, so both apps connect the same way. Queries stay in each app, per ADR-0017.
  • Frontend: server/utils/db.ts builds both pools through the lib. Settings and repos are unchanged.
  • API client: src/clients/postgres.ts builds the pool on first use from API_CM_DB_* and exposes queryCm, which turns driver and config failures into a 503, like Tinybird failures.
  • Health routes: /health/live always answers 200. /health/ready answers 503 only when the pod lacks required config (API_CM_DB_* or API_TB_*), naming the missing variables, so a misconfigured rollout stops. It ignores upstream health: every pod shares the CM database and Tinybird, so an outage would pull all pods at once, and each route already answers 503 for its own upstream. Both are the unversioned probes ADR-0009 reserves, and both stay out of the OpenAPI documents.
  • Env helpers: requiredEnv and missingEnv in src/env.ts, used by both clients and the readiness check.
  • Shared schemas: src/lib/collections.ts defines Collection and CollectionProject with their row guards and mappers. The list stays Postgres-only; collection-level Tinybird numbers belong to /collections/{slug}/metrics.
  • Build and CI: both Dockerfiles and the lint and test workflows install and build the new lib.

Configuration

The API needs these variables before this deploys, or /health/ready answers 503 and the rollout stops:

Variable Notes
API_CM_DB_HOST Read replica of the CM database
API_CM_DB_PORT Defaults to 5432
API_CM_DB_DATABASE
API_CM_DB_USERNAME Read-only user
API_CM_DB_PASSWORD
API_CM_DB_SSL false turns TLS off for local databases

Notes

  • pnpm-lock.yaml also re-pairs the pg-promise peer between two crowd.dev importers. pnpm does this once a second workspace package depends on pg.

Still to come on IN-1403

  • The shared public-only collections query
  • Collection-scope routing and the collections, categories and oss-index autoload groups
  • Route-suite variants and pagination

Tests

  • libs/postgres-client: pool options.
  • api/tests/health.test.ts: live, ready with full config, ready while the CM database is down, missing config, env parsing, and queryCm failures.
  • api/tests/collections.test.ts: row guards and mappers.

Signed-off-by: anilb <epipav@gmail.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 12:10
Comment thread api/package.json

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 Collections guards accept out-of-contract values, and the required database variables are missing from the local environment template.

Review effort: Balanced
Findings: 2 Medium severity · 1 Low severity

Open (3)
What changed in this PR

Adds shared PostgreSQL infrastructure and Collections schemas for upcoming API endpoints.

Changes:

  • Introduces a shared PostgreSQL pool library.
  • Adds API health probes and Collections schemas/mappers.
  • Updates frontend, Docker, CI, and tests for the new library.
File Description
pnpm-lock.yaml Resolves new workspace dependencies.
libs/​postgres-client/​package.json Defines the PostgreSQL client package.
libs/​postgres-client/​src/​index.ts Implements pool creation and health checks.
libs/​postgres-client/​tests/​index.test.ts Tests pool options and ping behavior.
libs/​postgres-client/​tsconfig.json Configures TypeScript.
libs/​postgres-client/​tsconfig.build.json Configures production compilation.
libs/​postgres-client/​vitest.config.ts Configures library tests.
frontend/​server/​utils/​db.ts Migrates frontend pools to the shared client.
frontend/​package.json Adds the shared dependency.
frontend/​Dockerfile Builds the shared library.
api/​src/​clients/​postgres.ts Adds lazy CM database access.
api/​src/​health.ts Adds liveness and readiness probes.
api/​src/​app.ts Registers unversioned health routes.
api/​src/​lib/​collections.ts Defines Collections schemas, guards, and mappers.
api/​tests/​health.test.ts Tests health and database behavior.
api/​tests/​collections.test.ts Tests Collections mapping and validation.
api/​package.json Adds the shared dependency.
api/​Dockerfile Builds and packages the shared library.
.github/​workflows/​tests.yml Builds the library for frontend tests.
.github/​workflows/​lint-test-api-libs.yml Adds library checks and tests.
.github/​workflows/​lint-frontend.yml Builds the library before frontend checks.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

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

Comment thread api/src/lib/collections.ts Outdated
Comment thread api/src/lib/collections.ts
Comment thread api/src/clients/postgres.ts
Signed-off-by: anilb <epipav@gmail.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 14:03

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

Configuration validation, schema enforcement, and supporting documentation contain unresolved inconsistencies.

Review effort: Balanced
Findings: 1 High severity · 3 Medium severity · 1 Low severity

Open (5)
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Previously missed (1)

In code that hasn't changed since last review

Low severity Document null owner behavior for community collections

api/​src/​lib/​collections.ts:51

This contract says owner is null only for curated collections, but toCollection also returns null for a community collection whose ownerName is unavailable, as the new test explicitly covers. Document that case so API consumers do not infer owner: null means type: curated.

This issue also appears on line 103 of the same file.

Comment thread api/src/clients/postgres.ts Outdated
Comment thread api/src/health.ts
@epipav
epipav requested review from gaspergrom and themarolt October 1, 2026 08:30
@epipav epipav self-assigned this Oct 1, 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.

Two things to fix before this merges. The rest of the groundwork looks solid.

Comment thread api/src/clients/postgres.ts Outdated
Comment thread api/src/clients/postgres.ts
Signed-off-by: anilb <epipav@gmail.com>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 08:39

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 previously requested API_CM_DB_* additions are still absent from api/.env.dist, leaving documented local setup unready.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
Resolved since last review (4)
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Signed-off-by: anilb <epipav@gmail.com>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 09:04
@epipav
epipav requested a review from gaspergrom October 1, 2026 09:07
@epipav
epipav marked this pull request as ready for review October 1, 2026 09:07
@cursor

cursor Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Adds required deploy config (API_CM_DB_*) and new read paths to the shared CM Postgres; misconfiguration blocks rollout via readiness, but query/SQL bugs would affect future collection endpoints.

Overview
Introduces @lfx-insights/postgres-client so the API and Nuxt app share the same Postgres pool settings; the frontend switches server/utils/db.ts to that helper, and CI/Docker builds now include the new workspace package.

The standalone API gains CM database read access via API_CM_DB_* and queryCm (driver/config errors become 503 like Tinybird). /health/live and /health/ready are registered on the app; readiness follows ADR-0022—it only validates required env (CM + Tinybird), not upstream connectivity—documented in a new ADR and reflected in the public API plan.

Collections groundwork adds TypeBox schemas and row mappers in collections.ts, plus collections-db with public-only list/slug/member SQL (filters, sort, pagination bind params). v1-alpha pre-registers autoload groups for collections, categories, and oss-index when those route folders exist; HTTP handlers are not in this diff. Env helpers requiredEnv / missingEnv centralize config checks for clients and readiness.

Reviewed by Cursor Bugbot for commit 7c9960e. 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

🟡 Changes recommended

Database TLS verification, Tinybird host readiness validation, and percentage bounds require correction.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Previously missed (2)

In code that hasn't changed since last review

Medium severity Readiness probe accepts malformed API_TB_HOST URLs

api/​src/​health.ts:23

Readiness reports malformed API_TB_HOST values as ready, although the Tinybird client later builds request URLs from this value and every affected route fails. Validate an absolute HTTP(S) URL as part of the config-only probe.

Medium severity Percentage validation allows values outside the 0–100 range

api/​src/​lib/​collections.ts:243

This accepts every finite number, including negative values and values above 100, even though both callers represent percentages. Bound the value to 0–100 so malformed pipe rows cannot pass validation.

Comment thread libs/postgres-client/src/index.ts

@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.

Looks good, thanks for the update.

Signed-off-by: anilb <epipav@gmail.com>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 09:29

@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

@epipav
epipav merged commit 3df53b5 into main Oct 1, 2026
13 checks passed
@epipav
epipav deleted the feat/IN-1403 branch October 1, 2026 11:24
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