chore(project): move to dist structure - #866
Conversation
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 82 files out of 189 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughRemoved NODE_OPTIONS memory flags from multiple scripts, deleted the "@llmgateway/*" TypeScript path mapping across apps/packages, updated many model files to import ModelDefinition via "@/models", added workspace deps to API/UI, and changed several CI workflow job commands to run build earlier. Changes
Sequence Diagram(s)mermaid Dev->>GH: push PR Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
apps/worker/package.json (1)
11-11: Avoid double JS emission — type‑check or emit types only before bundlingroot tsup.config.ts has dts: true and format: ["esm"] and root tsconfig.json sets outDir: "./dist/". Running "tsc && tsup" in apps/worker will emit JS into dist and then tsup will also write bundles to dist, producing duplicate JS.
Apply one of these to apps/worker/package.json (scripts.build):
Option A (preferred — type‑check only)
- "build": "tsc && tsup", + "build": "tsc --noEmit && tsup",(If using project references use
tsc -b --noEmit.)Option B (emit declarations only — usually redundant because tsup.dts = true)
- "build": "tsc && tsup", + "build": "tsc --emitDeclarationOnly --declaration --outDir dist/types && tsup",Confirmed: tsup config emits d.ts and ESM. Verify tsup resolves tsconfig "paths" (no tsconfig-paths plugin seen in tsup config); add a tsconfig-paths/esbuild plugin or set tsup.tsconfig if bundling fails.
apps/gateway/package.json (1)
12-15: Optional: align generate step with the NODE_OPTIONS removal (if memory allows).If OpenAPI generation runs fine without extra heap, consider removing it for consistency.
"scripts": { - "build": "tsc && tsup && pnpm run generate", + "build": "tsc && tsup && pnpm run generate", "dev": "nodemon -e ts,json --exec \"tsx src/serve.ts\"", - "generate": "NODE_ENV=production NODE_OPTIONS=\"--max-old-space-size=4096\" tsx ./src/scripts/generate-openapi.ts", + "generate": "NODE_ENV=production tsx ./src/scripts/generate-openapi.ts", "start": "node --enable-source-maps dist/serve.js" },apps/ui/package.json (1)
72-72: Duplicate tailwindcss in deps and devDeps.Keep Tailwind as a devDependency only to prevent unnecessary prod installs.
"tailwind-merge": "^3.2.0", - "tailwindcss": "^4.1.12", "tailwindcss-animate": "1.0.7"Also applies to: 82-82
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
apps/api/package.json(2 hunks)apps/api/tsconfig.json(1 hunks)apps/docs/tsconfig.json(1 hunks)apps/gateway/package.json(1 hunks)apps/gateway/tsconfig.json(1 hunks)apps/ui/package.json(2 hunks)apps/ui/tsconfig.json(0 hunks)apps/worker/package.json(1 hunks)apps/worker/tsconfig.json(1 hunks)package.json(1 hunks)packages/cache/tsconfig.json(1 hunks)packages/db/tsconfig.json(1 hunks)packages/instrumentation/tsconfig.json(1 hunks)packages/logger/tsconfig.json(1 hunks)packages/models/src/models/alibaba.ts(1 hunks)packages/models/src/models/anthropic.ts(1 hunks)packages/models/src/models/deepseek.ts(1 hunks)packages/models/src/models/google.ts(1 hunks)packages/models/src/models/llmgateway.ts(1 hunks)packages/models/src/models/meta.ts(1 hunks)packages/models/src/models/microsoft.ts(1 hunks)packages/models/src/models/mistral.ts(1 hunks)packages/models/src/models/moonshot.ts(1 hunks)packages/models/src/models/nousresearch.ts(1 hunks)packages/models/src/models/openai.ts(1 hunks)packages/models/src/models/perplexity.ts(1 hunks)packages/models/src/models/routeway.ts(1 hunks)packages/models/src/models/xai.ts(1 hunks)packages/models/src/models/zai.ts(1 hunks)packages/models/tsconfig.json(1 hunks)packages/shared/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/ui/tsconfig.json
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use top-level import; never use require() or dynamic imports
Files:
packages/models/src/models/openai.tspackages/models/src/models/google.tspackages/models/src/models/routeway.tspackages/models/src/models/mistral.tspackages/models/src/models/llmgateway.tspackages/models/src/models/alibaba.tspackages/models/src/models/anthropic.tspackages/models/src/models/nousresearch.tspackages/models/src/models/zai.tspackages/models/src/models/microsoft.tspackages/models/src/models/xai.tspackages/models/src/models/moonshot.tspackages/models/src/models/meta.tspackages/models/src/models/deepseek.tspackages/models/src/models/perplexity.ts
{apps/api,apps/gateway,apps/ui,apps/docs,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level import; never use require() or dynamic imports (e.g., import(), next/dynamic)
Files:
packages/models/src/models/openai.tspackages/models/src/models/google.tspackages/models/src/models/routeway.tspackages/models/src/models/mistral.tspackages/models/src/models/llmgateway.tspackages/models/src/models/alibaba.tspackages/models/src/models/anthropic.tspackages/models/src/models/nousresearch.tspackages/models/src/models/zai.tspackages/models/src/models/microsoft.tspackages/models/src/models/xai.tspackages/models/src/models/moonshot.tspackages/models/src/models/meta.tspackages/models/src/models/deepseek.tspackages/models/src/models/perplexity.ts
🧠 Learnings (2)
📚 Learning: 2025-09-15T13:16:05.355Z
Learnt from: CR
PR: theopenco/llmgateway#0
File: AGENTS.md:0-0
Timestamp: 2025-09-15T13:16:05.355Z
Learning: Applies to {apps/api,apps/gateway,apps/ui,apps/docs,packages}/**/*.{ts,tsx} : Always use top-level import; never use require() or dynamic imports (e.g., import(), next/dynamic)
Applied to files:
apps/docs/tsconfig.json
📚 Learning: 2025-09-15T13:15:00.724Z
Learnt from: CR
PR: theopenco/llmgateway#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T13:15:00.724Z
Learning: Always run pnpm format after code changes
Applied to files:
package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build-split (gateway, linux/amd64)
- GitHub Check: build-split (api, linux/amd64)
- GitHub Check: build-split (ui, linux/amd64)
- GitHub Check: build-split (worker, linux/amd64)
- GitHub Check: build-split (docs, linux/amd64)
- GitHub Check: build-unified (linux/amd64)
🔇 Additional comments (27)
packages/models/src/models/deepseek.ts (1)
1-1: Import path update LGTM.
Type-only, top-level import; consistent with alias change.packages/models/src/models/nousresearch.ts (1)
1-1: Import path update LGTM.
Consistent with repo-wide alias migration.packages/models/src/models/zai.ts (1)
1-1: Import path update LGTM.
No runtime impact; aligns with@alias.packages/models/src/models/alibaba.ts (1)
1-1: Import path update LGTM.
Top-level, type-only import; matches new aliasing.packages/logger/tsconfig.json (1)
5-5: Logger paths LGTM; quick sweep for old alias recommended. Ran rg -nP '@llmgateway/' packages/logger/src — no matches; run a repo-wide search to confirm no remaining alias usages.apps/docs/tsconfig.json (1)
11-11: Alias cleanup LGTM — unable to confirm no remaining @llmgateway imports in apps/docs
Sandbox ripgrep skipped files; run locally to verify:
rg -n '@llmgateway' apps/docs || git grep -n '@llmgateway' -- apps/docspackages/models/src/models/openai.ts (1)
1-1: Approve: "@/models" alias and ModelDefinition export verified.
packages/models/tsconfig.json defines"@/*": ["./src/*"]; packages/models/src/models.ts exportsModelDefinition(packages/models/src/index.ts re-exports./models); no@llmgateway/modelsimports found; type-only top-level import is correct.packages/models/src/models/llmgateway.ts (1)
1-1: LGTM: alias import conforms to repo path strategy.packages/models/src/models/anthropic.ts (1)
1-1: LGTM: import path migration only; no runtime changes.packages/models/src/models/moonshot.ts (1)
1-1: LGTM: consistent alias usage.packages/models/src/models/perplexity.ts (1)
1-1: LGTM — ModelDefinition re-export verifiedpackages/models/src/models.ts declares
export interface ModelDefinitionand packages/models/src/index.ts re-exports./models, soimport type { ModelDefinition } from "@/models"is valid.apps/worker/tsconfig.json (1)
5-5: tsx resolves tsconfig "paths" — no tsconfig-paths preload neededapps/worker's dev script runs
nodemon ... tsx src/index.tsand tsx has built‑in support for tsconfig.json "paths", so the@/*mapping in apps/worker/tsconfig.json should work in dev; I found no tsconfig-paths usage in the repo. (npmjs.com)packages/models/src/models/routeway.ts (1)
1-1: Consistent alias update.Matches repo‑wide move to "@/". No issues spotted.
packages/models/src/models/google.ts (1)
1-1: Alias switch looks correct.Top‑level import maintained; no runtime impact.
packages/models/src/models/mistral.ts (1)
1-1: Import path update LGTM.Consistent with other model files.
apps/gateway/package.json (1)
12-12: Build script simplification LGTM — remaining NODE_OPTIONS found.apps/gateway/package.json (line 14) contains:
"generate": "NODE_ENV=production NODE_OPTIONS="--max-old-space-size=4096" tsx ./src/scripts/generate-openapi.ts"
Remove the hardcoded NODE_OPTIONS or ensure CI provides enough memory for tsc/tsup/generate.apps/gateway/tsconfig.json (1)
5-5: tsconfig paths trimmed to "@/*" but @llmgateway imports still presentapps/gateway/tsconfig.json now maps only "@/", yet ripgrep shows many "@llmgateway/" imports under apps/gateway (e.g. apps/gateway/package.json; apps/gateway/src/index.ts, serve.ts, models/models.ts, lib/, tests). Either add a '@llmgateway/' path mapping back to apps/gateway/tsconfig.json or update imports / rely on workspace package resolution; verify TypeScript build/compile after the change.
packages/models/src/models/microsoft.ts (1)
1-1: Import path migration LGTM — confirm '@/models' resolves at build time.packages/models/tsconfig.json defines paths { "@/": ["./src/"] } and packages/models/src/models.ts exports ModelDefinition; the repo tsconfig (../../tsconfig.json) does not set compilerOptions.baseUrl. Confirm your toolchain/CI resolves the "@/models" alias; if not, add a baseUrl (e.g., ".") to packages/models/tsconfig.json.
apps/ui/package.json (2)
11-11: Build script simplification looks good.Dropping NODE_OPTIONS here is fine; please keep an eye on CI memory during next build runs.
21-21: UI depending on @llmgateway/db — ensure server-only usage.
rg found no imports, but it reported "unrecognized file type: tsx", so the scan may be incomplete — manually confirm no client components import @llmgateway/db. If imported, confine those imports to server files/routes and add "server-only" to avoid accidental client bundling.
Location: apps/ui/package.json (line 21)packages/db/tsconfig.json (1)
5-5: Path alias cleanup LGTM.Removing @llmgateway/* in favor of @/* is consistent with the repo-wide change.
package.json (2)
14-14: Format script change LGTM.Noted prior learning: remember to run "pnpm format" after merging.
17-17: Lint script change LGTM.Removing NODE_OPTIONS here is fine.
packages/models/src/models/xai.ts (1)
1-1: Import path migration LGTM.Consistent with the alias removal; top-level import respected.
apps/api/package.json (2)
24-24: Workspace models dependency addition LGTM.Matches alias removal strategy.
11-11: Confirm OpenAPI generation timing & path
- apps/api writes openapi.json to apps/api/openapi.json (apps/api/src/scripts/generate-openapi.ts) and apps/ui generate expects ../api/openapi.json (apps/ui/package.json) — path matches.
- turbo.json defines a "generate" pipeline (dependsOn ["^generate"], outputs include openapi.json and **/v1.d.ts) and ui lists api as a workspace dependency, so running root "pnpm generate" (turbo run generate) will run api.generate before ui.generate — ordering OK when using the generate pipeline.
- Caveat: running only "pnpm build" (turbo run build) does not run the separate generate tasks; api.build calls pnpm run generate at its end (so apps/api/openapi.json is produced during api build) but UI’s v1.d.ts will not be produced unless you run the generate pipeline first. Ensure CI/workflow runs "pnpm generate" before "pnpm build" or make build depend on generate (update turbo.json).
packages/models/src/models/meta.ts (1)
1-1: Import path migration LGTM — approved. Verified no '@llmgateway/models' imports remain under packages/models/src; '@/models' top-level alias matches guidelines.
| "paths": { | ||
| "@/*": ["./src/*"], | ||
| "@llmgateway/*": ["../../packages/*/src"] | ||
| "@/*": ["./src/*"] |
There was a problem hiding this comment.
🧩 Analysis chain
Paths change LGTM; check callers and import style in API.
Verify no @llmgateway/ usage remains and no dynamic imports/require():
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -nP '@llmgateway/' apps/api/src || true
rg -nP '\brequire\s*\(' apps/api/src || true
rg -nP '\bimport\s*\(' apps/api/src || trueLength of output: 3201
Paths change LGTM — API still imports @llmgateway; fix callers
Multiple files in apps/api/src still import from @llmgateway/* (e.g. apps/api/src/testing.ts, apps/api/src/serve.ts, apps/api/src/stripe.ts and many files under apps/api/src/routes). Update those imports to the new alias or restore an equivalent path mapping in apps/api/tsconfig.json. No require() or dynamic import() were detected by the check.
🤖 Prompt for AI Agents
In apps/api/tsconfig.json around line 5, the paths mapping was changed to "@/*":
["./src/*"] but many source files still import from "@llmgateway/*"; update
callers or restore mapping: either (A) add a paths entry mapping
"@llmgateway/*": ["./src/*"] to tsconfig so existing imports continue to
resolve, or (B) update all imports in apps/api/src (e.g. testing.ts, serve.ts,
stripe.ts and routes) to use the new alias "@/..." instead of "@llmgateway/...",
then run typecheck/tsserver to verify no unresolved modules remain.
| "paths": { | ||
| "@/*": ["./src/*"], | ||
| "@llmgateway/*": ["../../packages/*/src"] | ||
| "@/*": ["./src/*"] |
There was a problem hiding this comment.
🧩 Analysis chain
Alias cleanup looks good.
Removing "@llmgateway/*" is consistent. Please verify no remaining references in this package.
🏁 Script executed:
#!/bin/bash
rg -n --type=ts --type=tsx '@llmgateway/' packages/cacheLength of output: 27
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for '@llmgateway' in packages/cache (fixed string)..."
rg -nF --hidden --no-ignore-vcs --glob '!node_modules/**' '@llmgateway' packages/cache || true
echo "Searching for '@llmgateway' across repo (fixed string)..."
rg -nF --hidden --no-ignore-vcs --glob '!node_modules/**' '@llmgateway' || true
echo "Fallback grep in packages/cache..."
grep -RIn --exclude-dir=node_modules --exclude-dir=.git '@llmgateway' packages/cache || trueLength of output: 817
Remaining @llmgateway references found — alias removal is incomplete
The package still imports and depends on @llmgateway packages; restore the tsconfig alias or update these imports to relative/monorepo paths.
- packages/cache/package.json — name and deps reference @llmgateway (lines 2, 21, 22).
- packages/cache/src/cache.ts — imports at lines 3, 4, 8 (["@llmgateway/db", "@llmgateway/logger"]).
- packages/cache/src/redis.ts — import at line 3 ("@llmgateway/logger").
🤖 Prompt for AI Agents
In packages/cache/tsconfig.json around line 5, the project removed the "@/*"
path alias but related files still import @llmgateway packages; update
references or restore the alias: either re-add the "@/*": ["./src/*"] (or add a
mapping for "@llmgateway/*": ["../<appropriate-package>/src/*"]) so TypeScript
resolves @llmgateway imports, or change all imports and package.json entries to
monorepo-relative paths and package names. Specifically, in
packages/cache/package.json update "name" and dependency entries to the correct
monorepo package names or remove @llmgateway prefixes (lines ~2, 21, 22), and in
packages/cache/src/cache.ts (imports at lines 3,4,8) and
packages/cache/src/redis.ts (import at line 3) replace "@llmgateway/db" and
"@llmgateway/logger" with either the restored path-alias-resolvable names or
with relative/monorepo import paths to the actual packages; ensure tsconfig,
package.json, and the import statements remain consistent and run a TypeScript
build to verify resolution.
Ensure all steps build the project before running additional commands such as linting, generating files, and unit tests.
a2d024e to
6bea432
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/e2e.yml (1)
46-52: Build added before setup — consider env ordering and build scope
- Adding
pnpm buildis reasonable. Two checks:
- If
pnpm run setupneedsDATABASE_URL, export it before running setup. Otherwise migrations/seeding may target the wrong DB.- If root build triggers heavy UI/Next builds that require extra env, consider scoping the build to what's needed for e2e (e.g., only gateway/models) to keep the job fast and robust.
Suggested tweak if setup uses the DB URL:
set -eux - pnpm build - pnpm run setup - export DATABASE_URL=postgres://postgres:pw@localhost:5432/test + pnpm build + export DATABASE_URL=postgres://postgres:pw@localhost:5432/test + pnpm run setup E2E_TEST=true pnpm vitest run -c vitest/vitest.e2e.config.mts --no-file-parallelism --shard=${{ matrix.shard }}/5 --reporter=blob --outputFile=e2e-results-${{ matrix.shard }}.blob || trueIf the full workspace build is unnecessary, we can propose a filtered build once we confirm which workspaces the e2e rely on.
.github/workflows/ci.yml (2)
15-15: Lint after build increases cycle timeRunning
pnpm buildbeforepnpm lintslows feedback. Linting can run on sources without a successful build.Apply:
- cmd: pnpm build && pnpm lint + cmd: pnpm lintIf TypeScript type-checking is desired in this job, consider
pnpm -w ts:check(or equivalent) instead of a full build.
21-21: Confirm generate’s dependency on build
pnpm build && pnpm generate && git diff --exit-codeis fine if codegen requires built artifacts; otherwise the build is redundant and adds minutes.If build isn’t required:
- cmd: pnpm build && pnpm generate && git diff --exit-code + cmd: pnpm generate && git diff --exit-code
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks).github/workflows/e2e.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-15T13:15:00.724Z
Learnt from: CR
PR: theopenco/llmgateway#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T13:15:00.724Z
Learning: Run pnpm test:unit and pnpm test:e2e after adding features
Applied to files:
.github/workflows/e2e.yml
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
27-27: Build job order looks okay
pnpm build && pnpm run setup && bash .github/start.shis coherent if setup prepares runtime deps post-build.Confirm that
setupdoesn’t mutate sources in a way that invalidates the preceding build (e.g., re-generating Prisma clients).
Ensure `pnpm build` runs before `pnpm run setup` for correct workflow execution.
83cb854 to
459d650
Compare
Updated `dev` scripts in multiple packages to ensure compilation with `tsup` before running `nodemon`. Added new `watch` command in the `models` package for better development workflow.
Removed redundant dynamic imports of `allBlogs` in blog pages to simplify code and improve readability.
Revised TypeScript configurations across packages by adding `outDir` and updating `module/moduleResolution`. Replaced `tsup` with `tsc` in build scripts for consistency.
Updated import statements across the project to include `.js` extensions for better compatibility with ES module resolution. Adjusted related TypeScript configurations and lockfile changes.
Removed `tsup` usage across all packages and deleted its configurations. Adjusted package.json and lockfile to remove `tsup` as a dependency.
Added `dotenv` library to `apps/api` dependencies. Replaced `@steebchen/lint-next` with `@steebchen/lint-base` and removed unnecessary packages from `pnpm-lock.yaml`. Added `format` and `lint` scripts to package.json files for consistent code formatting. Updated some dependency resolutions in the lockfile for better compatibility.
Updated import paths to use base "@/..." syntax for consistency across packages. Adjusted seed script to reference the correct JS build output. Added `format` and `lint` scripts to package.json for standardized workflows.
Added `.prettierignore` files to ignore `dist` directories in `apps/gateway` and `apps/api`.
Applied `eslint-disable-next-line import/order` to prevent import order warnings for specific files across changelog and blog pages. No functional changes.
steebchen
changed the title
Updated `generate` task in `turbo.json` to include `build` as a dependency, ensuring the build step is executed before generating outputs.
43cb6f3 to
1a86656
Compare
Updated `dev` scripts in `apps/api` and `apps/worker` to use `tsc-watch` for triggering `resolve-tspaths` on successful builds. Added `tsc-watch` dependency to `package.json`. Cleaned up `pnpm-lock.yaml` by removing unused dependency mappings.
Removed unnecessary `console.log` statements from `waitForLogWithRequestId` helper to clean up output and improve readability. No functional changes.
Updated `getCheapestModelForProvider` to exclude free models from consideration as they may be less stable. Simplified null checks for `inputPrice` and `outputPrice`.
This reverts commit c924862.
Removed `sync-editorconfig` from the `postinstall` script in `package.json` for clarity and to streamline the configuration. No functional impact expected.
1 participant
Summary by CodeRabbit