refactor: improve code structure for readability#11
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds dist entrypoints and types for packages, shifts shared exports to built artifacts, enables composite builds/tsbuildinfo for config, tightens apps/api TypeScript output/include, and changes top-level and apps/api build/typecheck scripts to run ordered workspace builds for config/shared/llm before compiling apps. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as Developer/CI
participant Config as packages/config
participant Shared as packages/shared
participant LLM as packages/llm
participant API as apps/api
participant Worker as apps/worker
CI->>Config: npm --workspace `@wa-chat/config` run build
Note right of Config: compile (composite) -> dist\nwrites tsbuildinfo
CI->>Shared: npm --workspace `@wa-chat/shared` run build
Note right of Shared: compile -> dist
CI->>LLM: npm --workspace `@wa-chat/llm` run build
Note right of LLM: compile -> dist (depends on shared)
CI->>API: npm --workspace apps/api run build
Note right of API: runs prebuilt config/shared then tsc -> dist
CI->>Worker: npm --workspace apps/worker run build
Note right of Worker: builds after packages
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can enforce grammar and style rules using `languagetool`.Configure the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api/package.json (1)
9-12: Keeptypechecknon-emitting.Using
run buildinsidetypecheckwrites dependency artifacts and can dirty the workspace for a check-only command. Both dependency packages already exposetypecheck, so reservebuildfor the emitting path only.Suggested change
- "typecheck": "npm --workspace `@wa-chat/config` run build && npm --workspace `@wa-chat/shared` run build && tsc --noEmit", + "typecheck": "npm --workspace `@wa-chat/config` run typecheck && npm --workspace `@wa-chat/shared` run typecheck && tsc --noEmit",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/package.json` around lines 9 - 12, The "typecheck" script currently runs emitting builds for dependencies; change it to call the dependencies' non-emitting checks instead (use npm --workspace `@wa-chat/config` run typecheck and npm --workspace `@wa-chat/shared` run typecheck) and keep the local tsc --noEmit step so the "typecheck" script remains purely check-only; update the package.json "typecheck" script accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/tsconfig.json`:
- Around line 8-9: The package export in packages/shared is pointing at
TypeScript source (./src/index.ts) which breaks NodeNext resolution; update
packages/shared/package.json's "exports" field so the package exports the built
artifacts (set "default" to ./dist/index.js and "types" to ./dist/index.d.ts)
instead of ./src/index.ts, rebuild the package so dist exists, and verify
apps/api can require/import `@wa-chat/shared` without loading TS source.
---
Nitpick comments:
In `@apps/api/package.json`:
- Around line 9-12: The "typecheck" script currently runs emitting builds for
dependencies; change it to call the dependencies' non-emitting checks instead
(use npm --workspace `@wa-chat/config` run typecheck and npm --workspace
`@wa-chat/shared` run typecheck) and keep the local tsc --noEmit step so the
"typecheck" script remains purely check-only; update the package.json
"typecheck" script accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc4e5c1a-ba9d-457e-99a9-1fd33fc1201e
📒 Files selected for processing (3)
apps/api/package.jsonapps/api/tsconfig.jsonapps/api/tsconfig.tsbuildinfo
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 14: The root package.json "build" script currently omits the dashboard
workspace; update the "build" script to include the dashboard workspace by
adding "-w apps/dashboard" alongside the existing workspaces (so the script runs
builds for packages/config, packages/shared, packages/llm, apps/api, apps/worker
and apps/dashboard). Ensure the modified "build" npm script preserves the
existing ordering and separators and matches the style of the other root
scripts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 297da07b-a777-40b3-a834-15cd77ad7e56
📒 Files selected for processing (7)
apps/api/tsconfig.tsbuildinfoapps/worker/tsconfig.tsbuildinfopackage.jsonpackages/config/tsconfig.jsonpackages/config/tsconfig.tsbuildinfopackages/llm/tsconfig.tsbuildinfopackages/shared/package.json
✅ Files skipped from review due to trivial changes (1)
- packages/config/tsconfig.tsbuildinfo
| "test:e2e": "node --import tsx tests/e2e/webhook-to-outbound.e2e.test.ts && node --import tsx tests/e2e/duplicate-replay.e2e.test.ts && node --import tsx tests/e2e/escalation-handoff.e2e.test.ts", | ||
| "test:load": "node --import tsx tests/load/load-test.ts", | ||
| "build": "npm run build --workspaces", | ||
| "build": "npm run build -w packages/config -w packages/shared -w packages/llm && npm run build -w apps/api -w apps/worker", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json
from pathlib import Path
for scope in ("apps", "packages"):
for pkg_json in sorted(Path(scope).glob("*/package.json")):
data = json.loads(pkg_json.read_text())
build = data.get("scripts", {}).get("build", "<no build script>")
print(f"{pkg_json.parent}: {build}")
PYRepository: Ryanakml/ChatFuse
Length of output: 358
Fix root build script to include apps/dashboard.
The root build script on line 14 omits apps/dashboard, which has a production build (next build). This workspace will not be built when running the root build, even though it appears to be a release target given the dedicated root scripts on lines 15–17. Add -w apps/dashboard to the root build script to ensure it builds alongside the other workspaces.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 14, The root package.json "build" script currently
omits the dashboard workspace; update the "build" script to include the
dashboard workspace by adding "-w apps/dashboard" alongside the existing
workspaces (so the script runs builds for packages/config, packages/shared,
packages/llm, apps/api, apps/worker and apps/dashboard). Ensure the modified
"build" npm script preserves the existing ordering and separators and matches
the style of the other root scripts.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/llm/package.json (1)
15-15: Prefer explicit versions or workspace specs over bare*for internal dependencies.
@wa-chat/sharedwith"*"can float to any published version, reducing reproducibility. While the root workspace is markedprivate: true, using an explicit version (e.g.,"1.0.0"or a narrow range) or a workspace-aware spec (if npm version supports it) makes intent clearer and dependencies more predictable across installs. This pattern appears across multiple packages in the monorepo—consider standardizing the approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/llm/package.json` at line 15, The dependency entry "@wa-chat/shared": "*" in package.json is too permissive; replace the bare "*" with either a concrete semver (e.g., "1.0.0" or "^1.0.0") or a workspace-aware spec (e.g., "workspace:^1.0.0" or "workspace:*" if your npm/yarn supports it) to pin intent and restore reproducibility; update the dependency string in the package.json where "@wa-chat/shared" is declared and apply the same pattern consistently across other packages that currently use "*" so the monorepo uses explicit versions or workspace specs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/shared/package.json`:
- Around line 5-11: Update the workspace TypeScript path mappings so imports
resolve to the new package entrypoint instead of the source; specifically remove
or change any tsconfig "paths" that map "@wa-chat/shared" (found in
tsconfig.base.json, apps/dashboard/tsconfig.json, tests/tsconfig.json) from
pointing to "packages/shared/src/*" to point at the built entry (or delete the
mapping so Node/TS uses the package "exports" and resolves to ./dist/index.js);
ensure downstream test and app configs no longer bypass the package manifest by
resolving the package name to its dist output.
---
Nitpick comments:
In `@packages/llm/package.json`:
- Line 15: The dependency entry "@wa-chat/shared": "*" in package.json is too
permissive; replace the bare "*" with either a concrete semver (e.g., "1.0.0" or
"^1.0.0") or a workspace-aware spec (e.g., "workspace:^1.0.0" or "workspace:*"
if your npm/yarn supports it) to pin intent and restore reproducibility; update
the dependency string in the package.json where "@wa-chat/shared" is declared
and apply the same pattern consistently across other packages that currently use
"*" so the monorepo uses explicit versions or workspace specs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19959125-44cd-46f3-91a7-eb6f4e6730e4
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
apps/api/package.jsonpackages/llm/package.jsonpackages/shared/package.json
| "main": "./dist/index.js", | ||
| "types": "./dist/index.d.ts", | ||
| "exports": { | ||
| ".": { | ||
| "types": "./src/index.ts", | ||
| "default": "./src/index.ts" | ||
| "types": "./dist/index.d.ts", | ||
| "import": "./dist/index.js", | ||
| "default": "./dist/index.js" |
There was a problem hiding this comment.
The new dist entrypoint is still bypassed by current TS path aliases.
tsconfig.base.json, apps/dashboard/tsconfig.json, and tests/tsconfig.json still map @wa-chat/shared to packages/shared/src..., so workspace builds/tests continue importing raw TS instead of ./dist/index.js. That means this manifest change is not actually exercised locally, and package-boundary mismatches can slip through until external consumption. Please update or remove those path mappings before relying on these dist exports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/package.json` around lines 5 - 11, Update the workspace
TypeScript path mappings so imports resolve to the new package entrypoint
instead of the source; specifically remove or change any tsconfig "paths" that
map "@wa-chat/shared" (found in tsconfig.base.json,
apps/dashboard/tsconfig.json, tests/tsconfig.json) from pointing to
"packages/shared/src/*" to point at the built entry (or delete the mapping so
Node/TS uses the package "exports" and resolves to ./dist/index.js); ensure
downstream test and app configs no longer bypass the package manifest by
resolving the package name to its dist output.
…ve outbound messages
Ryanakml
force-pushed
the
phase/release-go-live
branch
from
49c479b to
69be47e
Compare
…logic for embeddings
…mproved compatibility
…imilarity matching
…allback mechanisms and tests
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
1 participant
Summary by CodeRabbit