Replace misleading Promise.all with sequential sync calls

[ng]

Summary

• Replaced Promise.all wrapping synchronous better-sqlite3/drizzle queries with sequential .all() calls in getAll and getByDay handlers
• better-sqlite3 is synchronous and single-connection — Promise.all provided no parallelism, only a false suggestion of concurrency

Closes #201

Test plan

• TypeScript type check passes
• All 9 schedules tests pass
• Manual: verify getAll and getByDay endpoints return correct data

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor
  • Optimized internal query execution for schedule data retrieval to improve performance.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refactors query execution in the schedules router from Promise.all() to sequential awaiting. Since better-sqlite3 is synchronous with a single connection, the parallel-style Promise.all() construct was misleading—queries were already serialized. The refactor changes control flow to explicitly sequential while maintaining the same returned data structure.

Changes

Cohort / File(s)	Summary
Schedule Query Refactoring src/server/routers/schedules.ts	Replaced Promise.all([...]) concurrent-style query execution with sequential db.select().from(...).where(...).all() calls in getAll and getByDay procedures. Query results for temperature, power, and alarm schedules are now awaited individually rather than in parallel, better reflecting the synchronous nature of better-sqlite3.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰✨ No more promises in a line,
Sequential queries now align,
Better-sqlite's truthful way,
No false parallelism at play! 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: replacing Promise.all with sequential calls for better-sqlite3 synchronous queries.
Linked Issues check	✅ Passed	The PR successfully addresses issue #201 by removing Promise.all and replacing it with sequential query execution, eliminating misleading concurrency implications.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing issue #201; only the Promise.all replacement in schedules.ts was modified, maintaining scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/201-remove-misleading-promise-all

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/server/routers/schedules.ts (1)

50-64: Correctly addresses the misleading Promise.all pattern.

The refactor properly replaces Promise.all with sequential .all() calls, accurately reflecting that better-sqlite3 queries execute synchronously. The code is functionally equivalent and more honest about the actual execution model.

However, src/server/routers/health.ts (lines 149-165) still uses await Promise.all([...]) for fetching the same three schedule types. For codebase consistency, consider applying the same refactor there.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/server/routers/schedules.ts` around lines 50 - 64, The health router
still uses await Promise.all([...]) to fetch schedule rows even though
better-sqlite3 queries are synchronous; update the health handler to mirror the
change in schedules.ts by replacing the Promise.all call with three synchronous
queries that call .select().from(...).where(eq(..., input.side)).all() and
assign them to temperatureSchedulesList, powerSchedulesList, and
alarmSchedulesList (or the local variable names used in that file) so the code
is consistent and avoids the misleading async pattern.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/server/routers/schedules.ts`:
- Around line 50-64: The health router still uses await Promise.all([...]) to
fetch schedule rows even though better-sqlite3 queries are synchronous; update
the health handler to mirror the change in schedules.ts by replacing the
Promise.all call with three synchronous queries that call
.select().from(...).where(eq(..., input.side)).all() and assign them to
temperatureSchedulesList, powerSchedulesList, and alarmSchedulesList (or the
local variable names used in that file) so the code is consistent and avoids the
misleading async pattern.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 761b468f-e095-49a7-9ad7-b1bb89dfd650

📥 Commits

Reviewing files that changed from the base of the PR and between 2be5b15 and 5cdd309.

📒 Files selected for processing (1)

• src/server/routers/schedules.ts

[github-actions]

🎉 This PR is included in version 1.6.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀