store dashboard results locally instead of on mothership#229
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae18e38ff7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const startExec = startDashboardEvaluate(DashboardResult, dashboardId, $workspaceId, userId); | ||
| try { | ||
| result = await dashboard.evaluate(); |
There was a problem hiding this comment.
The promise returned by startDashboardEvaluate() is created and left without a rejection handler until after await dashboard.evaluate() completes. If DashboardResult.create() rejects quickly (for example during a transient studioConnection write failure) while evaluation is still in flight, this becomes an unhandled rejection; with modern Node defaults this can terminate the process before the later .then(...) is attached. Attach a catch immediately (or await the start promise before evaluation) to avoid the race.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR changes dashboard evaluation result storage to persist locally (via the studioConnection) instead of relying on the mothership, while still supporting fetching/merging mothership results for backwards compatibility.
Changes:
- Added a local
__Studio_DashboardResultmodel/schema and wired it into backend initialization. - Updated Dashboard actions to use
studioConnectionand adjustedgetDashboard()to persist evaluation status/results locally and merge with remote results when listing. - Updated and expanded test coverage to validate local persistence and local+remote merge behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
backend/index.js |
Registers the new __Studio_DashboardResult model on studioConnection and passes studioConnection into actions. |
backend/db/dashboardResultSchema.js |
Introduces schema/index for locally persisted dashboard evaluation results. |
backend/actions/Dashboard/getDashboard.js |
Persists evaluation results locally, fetches mothership results in parallel, and merges/sorts combined results. |
backend/actions/Dashboard/createDashboard.js |
Switches to studioConnection for dashboard creation. |
backend/actions/Dashboard/updateDashboard.js |
Switches to studioConnection for dashboard updates. |
backend/actions/Dashboard/deleteDashboard.js |
Switches to studioConnection and deletes associated local results on dashboard deletion. |
backend/actions/Dashboard/getDashboards.js |
Switches to studioConnection for listing dashboards. |
test/setup.test.js |
Exports studioConnection for tests that need direct model access on the studio DB. |
test/Dashboard.updateDashboard.test.js |
Uses studioConnection to validate dashboard updates against the correct DB. |
test/Dashboard.getDashboard.error.test.js |
Updates expectations to assert evaluation failures are persisted locally. |
test/Dashboard.getDashboard.results.test.js |
New tests validating local persistence, retrieval, and local+remote merge behavior. |
test/Dashboard.evaluate.objectid.test.js |
Uses studioConnection to define the Dashboard model for evaluation tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const startExec = startDashboardEvaluate(DashboardResult, dashboardId, $workspaceId, userId); | ||
| try { | ||
| result = await dashboard.evaluate(); |
There was a problem hiding this comment.
startExec is created without any rejection handler until after await dashboard.evaluate(). If DashboardResult.create() rejects before evaluation finishes, Node may emit an unhandled rejection (and potentially terminate the process, depending on runtime settings). Attach a .catch() immediately (or wrap with a helper that always resolves) and then await the handled promise later.
No description provided.