Repository navigation
speaker情報をSQLiteに保存するようにした - #30
Conversation
📝 WalkthroughWalkthroughディスコードボットのスピーカー設定を、メモリ内ストレージからSQLiteデータベース(Drizzle ORM経由)への永続化に移行し、エラーハンドリングを例外からタイプセーフな結果型(neverthrow)に変更しました。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as Discord User
participant Command as Speaker<br/>Command
participant DB as SQLite Database<br/>(via Drizzle)
participant Queue as Message Queue
participant VoiceGen as Voice Generation
User->>Command: Select speaker
Command->>DB: setSpeakerPreference(userId, speakerId)
DB-->>Command: Done
User->>Command: Send message
Command->>Queue: Enqueue message with userId
Queue->>DB: getSpeakerPreference(userId)
DB-->>Queue: Stored speaker preference
Queue->>VoiceGen: Generate voice with speaker
alt Speaker found
VoiceGen-->>Queue: ok(Readable stream)
else Speaker not found
VoiceGen-->>Queue: error(speaker_not_found)
Queue->>DB: resetSpeakerPreference(userId)
DB-->>Queue: Done
Queue->>DB: getSpeakerPreference(userId)<br/>(default speaker)
DB-->>Queue: Default speaker preference
Queue->>VoiceGen: Generate voice with default
VoiceGen-->>Queue: ok(Readable stream)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9ba6c8537
ℹ️ 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 sqlite = new Database("./mydb.sqlite"); | ||
| export const db = drizzle({ client: sqlite, schema }); |
There was a problem hiding this comment.
Create the preferences table before first DB access
This commit starts querying and writing user_speaker_preferences on normal bot flows, but the DB bootstrap only opens SQLite and registers schema metadata; there is no migration/DDL execution anywhere in the repo to actually create the table. On a fresh checkout, the first call to getSpeakerPreference/setSpeakerPreference will fail with no such table, breaking message handling until manual setup is done.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/bot/db/index.ts (1)
5-6: 相対パス./mydb.sqliteは実行時のカレントディレクトリに依存します。現在の実装では、データベースファイルのパスが実行時のカレントディレクトリに依存するため、異なるディレクトリからボットを起動した場合、データベースファイルが予期しない場所に作成される可能性があります。
import.meta.dirを使用してスクリプトのディレクトリからの相対パスを指定することを検討してください。♻️ 提案する修正
import { drizzle } from "drizzle-orm/bun-sqlite"; import { Database } from "bun:sqlite"; +import { join } from "node:path"; import * as schema from "./schema/index.js"; -const sqlite = new Database("./mydb.sqlite"); +const sqlite = new Database(join(import.meta.dir, "mydb.sqlite")); export const db = drizzle({ client: sqlite, schema });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/db/index.ts` around lines 5 - 6, The code constructs the SQLite Database with a relative path ("./mydb.sqlite") which depends on the process CWD; change the path construction in the module that defines the sqlite constant (where `const sqlite = new Database(...)` and `export const db = drizzle(...)`) to resolve the DB path from the script directory instead of the CWD by using import.meta.dir (or, if unavailable, derive it via fileURLToPath(import.meta.url)) and path.join(import.meta.dir, "mydb.sqlite"); update imports accordingly so the Database always opens the DB file relative to the module location.packages/bot/db/speaker-preferences.ts (1)
5-5: デフォルト話者IDのハードコードは設定化を推奨します。話者の入れ替え時にこの値だけ古くなると、既定値利用ユーザーが継続的に失敗します。環境変数または設定ファイル経由にしておく方が運用しやすいです。
💡 修正案
-const DEFAULT_SPEAKER_ID = "874568803256786945"; +const DEFAULT_SPEAKER_ID = + process.env.DEFAULT_SPEAKER_ID ?? "874568803256786945";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/db/speaker-preferences.ts` at line 5, Replace the hardcoded DEFAULT_SPEAKER_ID constant with a configurable value sourced from environment/config (e.g., process.env.DEFAULT_SPEAKER_ID or a shared config module) and keep the original literal as a fallback; update the declaration of DEFAULT_SPEAKER_ID to read from that config and ensure any code that imports/uses DEFAULT_SPEAKER_ID (in speaker-preferences.ts and callers) still works with the string fallback, adding validation/logging if the env value is missing or invalid.
🤖 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/bot/commands/speaker.ts`:
- Around line 53-56: The interaction acknowledgment must happen before the DB
write to avoid "Interaction failed" when DB is slow: move the call to
interaction.deferUpdate() to execute immediately (before calling
setSpeakerPreference), then perform setSpeakerPreference(interaction.user.id,
speakerId) afterwards and handle its failure separately (wrap
setSpeakerPreference in try/catch, log errors and/or send a follow-up message)
so the ACK is always sent within 3 seconds while still persisting the
preference.
In `@packages/bot/lib/generate.ts`:
- Around line 15-19: The fetch call that posts to
"http://localhost:8000/generate" in generate.ts lacks a timeout and can hang;
wrap the request in an AbortController with a setTimeout (e.g., 10s or
configurable) that calls controller.abort(), pass controller.signal into fetch,
and clear the timeout after fetch resolves; update the surrounding function (the
code that defines const res = await fetch(...)) to catch an AbortError and
handle/throw a clear timeout-specific error so the queue does not stall
indefinitely.
In `@packages/bot/lib/voice-queue.ts`:
- Around line 54-56: When handling error.reason === "speaker_not_found" wrap the
call to resetSpeakerPreference(item.userId) in its own try/catch so any failure
is logged but not rethrown, and ensure currentFinished() is still called (move
it to finally or after the catch) so the queue continues; apply the same pattern
to the other identical block (lines 57-62) to swallow/reset errors safely while
advancing the queue.
---
Nitpick comments:
In `@packages/bot/db/index.ts`:
- Around line 5-6: The code constructs the SQLite Database with a relative path
("./mydb.sqlite") which depends on the process CWD; change the path construction
in the module that defines the sqlite constant (where `const sqlite = new
Database(...)` and `export const db = drizzle(...)`) to resolve the DB path from
the script directory instead of the CWD by using import.meta.dir (or, if
unavailable, derive it via fileURLToPath(import.meta.url)) and
path.join(import.meta.dir, "mydb.sqlite"); update imports accordingly so the
Database always opens the DB file relative to the module location.
In `@packages/bot/db/speaker-preferences.ts`:
- Line 5: Replace the hardcoded DEFAULT_SPEAKER_ID constant with a configurable
value sourced from environment/config (e.g., process.env.DEFAULT_SPEAKER_ID or a
shared config module) and keep the original literal as a fallback; update the
declaration of DEFAULT_SPEAKER_ID to read from that config and ensure any code
that imports/uses DEFAULT_SPEAKER_ID (in speaker-preferences.ts and callers)
still works with the string fallback, adding validation/logging if the env value
is missing or invalid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6a0a2822-e1a2-462f-b1d3-a5d1832007f2
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.gitignorepackages/bot/commands/speaker.tspackages/bot/db/index.tspackages/bot/db/schema/index.tspackages/bot/db/schema/user-speaker-preference.tspackages/bot/db/speaker-preferences.tspackages/bot/drizzle.config.tspackages/bot/index.tspackages/bot/lib/generate.tspackages/bot/lib/message-queue.tspackages/bot/lib/voice-queue.tspackages/bot/package.jsonpackages/message_queue/src/message_queue.gleampackages/message_queue/test/message_queue_test.gleam
| await setSpeakerPreference(interaction.user.id, speakerId); | ||
| } | ||
|
|
||
| await interaction.deferUpdate(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
discord.js v14 の StringSelectMenuInteraction は、deferUpdate/reply を何秒以内に返す必要がありますか?公式ドキュメントと Discord API の制約を確認してください。
💡 Result:
discord.js v14のStringSelectMenuInteraction(メッセージコンポーネントインタラクション)に対して、deferUpdate、deferReply、replyなどの応答は、Discord APIの制約により、インタラクション受信から3秒以内に実行する必要があります。それを超えるとトークンが無効になり、応答が失敗します。deferを使用すると、15分以内にeditReplyなどで最終応答可能です。
Citations:
- 1: Raise limit of time to responding to a interaction discord/discord-api-docs#2389
- 2: https://docs.discord.com/developers/interactions/receiving-and-responding
- 3: https://discordjs.guide/interactive-components/interactions
- 4: https://discordjs.guide/slash-commands/response-methods
🏁 Script executed:
# Find the speaker.ts file in bot commands
fd -t f "speaker.ts" packages/bot/commands/Repository: akazdayo/omni-tts-discord
Length of output: 102
🏁 Script executed:
# If found, read the file to verify the code structure
if [ -f "packages/bot/commands/speaker.ts" ]; then
wc -l packages/bot/commands/speaker.ts
fiRepository: akazdayo/omni-tts-discord
Length of output: 105
🏁 Script executed:
# Read the handleSpeakerSelect function around lines 53-56
if [ -f "packages/bot/commands/speaker.ts" ]; then
cat -n packages/bot/commands/speaker.ts | sed -n '40,70p'
fiRepository: akazdayo/omni-tts-discord
Length of output: 765
deferUpdate() を setSpeakerPreference() より先に呼び出してください。DB遅延時のインタラクション失敗を防ぐため。
Discord APIはインタラクション受信から3秒以内に応答(deferUpdate/reply)を返す必要があります。現在のコードでは、DB操作が先に実行されるため、その処理が3秒を超えると、deferUpdate()の呼び出し時にはトークンが無効になり「Interaction failed」エラーが発生します。
ACKを優先し、DB操作の失敗はその後で個別にハンドリングしてください。
修正案
export const handleSpeakerSelect = async (
interaction: StringSelectMenuInteraction,
): Promise<void> => {
const [speakerId] = interaction.values;
+ await interaction.deferUpdate();
if (speakerId) {
- await setSpeakerPreference(interaction.user.id, speakerId);
+ try {
+ await setSpeakerPreference(interaction.user.id, speakerId);
+ } catch (error) {
+ console.error("failed to persist speaker preference", {
+ error,
+ speakerId,
+ userId: interaction.user.id,
+ });
+ }
}
-
- await interaction.deferUpdate();
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/commands/speaker.ts` around lines 53 - 56, The interaction
acknowledgment must happen before the DB write to avoid "Interaction failed"
when DB is slow: move the call to interaction.deferUpdate() to execute
immediately (before calling setSpeakerPreference), then perform
setSpeakerPreference(interaction.user.id, speakerId) afterwards and handle its
failure separately (wrap setSpeakerPreference in try/catch, log errors and/or
send a follow-up message) so the ACK is always sent within 3 seconds while still
persisting the preference.
| const res = await fetch("http://localhost:8000/generate", { | ||
| body: JSON.stringify({ speaker, text }), | ||
| headers: { "Content-Type": "application/json" }, | ||
| method: "POST", | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Node.js / Bun の fetch におけるデフォルトタイムアウト挙動と、AbortController によるタイムアウト実装の推奨方法を公式ドキュメントで確認してください。
💡 Result:
Node.jsとBunのfetchにおけるデフォルトタイムアウト挙動とAbortController(AbortSignal)によるタイムアウト実装の推奨方法は以下の通りです。 ## Node.js fetch - デフォルトタイムアウト: 公式ドキュメント(nodejs.org/api/globals.html, nodejs.dev)ではfetchに明示的なデフォルトタイムアウトは記載されていません。Undici(Node.jsのfetch実装)では接続タイムアウトが約10秒、ヘッダー/ボディタイムアウトが約5分(300秒)などの内部デフォルトがありますが、標準fetchオプションとして無限(no timeout)と等価です。ただし、Undiciの挙動により実質的な制限が存在します。 - 推奨タイムアウト実装: AbortSignal.timeoutを使用してsignalオプションで指定。Node.js 20以降でネイティブサポート。 const response = await fetch(url, { signal: AbortSignal.timeout(5000) }); エラーはTimeoutErrorとしてキャッチ可能。 ## Bun fetch - デフォルトタイムアウト: 公式ドキュメント(bun.sh/docs/runtime/networking/fetch)では明示されていませんが、5分(300秒)に設定されています(過去30秒から変更、PR #6217)。 - 推奨タイムアウト実装: AbortSignal.timeoutを公式に推奨。 const response = await fetch("http://example.com", { signal: AbortSignal.timeout(1000), }); 同時リクエスト上限は256。 両者ともWeb標準のAbortSignalを使用し、setTimeout + AbortController.abortの手動実装も可能ですが、AbortSignal.timeoutが簡潔で推奨されます。
Citations:
- 1: https://nodejs.org/api/globals.html
- 2: https://bun.com/docs/runtime/networking/fetch
- 3: https://bun.sh/docs/runtime/networking/fetch
- 4: Default fetch timeout too short nodejs/undici#1373
- 5: https://stackoverflow.com/questions/76755913/increase-timeout-in-nodejs-built-in-fetch-api
- 6: Set
fetchtimeout to 5 minutes oven-sh/bun#6217 - 7: Respect timeout passed to fetch oven-sh/bun#16682
- 8: https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout
🏁 Script executed:
# First, find the file and check if it exists
find . -name "generate.ts" -type f | grep -E "packages/bot/lib"Repository: akazdayo/omni-tts-discord
Length of output: 100
🏁 Script executed:
# Check the repository structure and identify the runtime environment
ls -la packages/bot/lib/ 2>/dev/null || echo "Directory might not exist"Repository: akazdayo/omni-tts-discord
Length of output: 641
🏁 Script executed:
# Check package.json for Node.js version and runtime details
cat package.json | head -50Repository: akazdayo/omni-tts-discord
Length of output: 743
🏁 Script executed:
# Read the specific file to understand the context
if [ -f "packages/bot/lib/generate.ts" ]; then
cat -n packages/bot/lib/generate.ts
else
echo "File not found at expected location"
fiRepository: akazdayo/omni-tts-discord
Length of output: 1512
🏁 Script executed:
# Check Bun version support for AbortSignal.timeout()
# Also search for any existing timeout implementations in the codebase
rg "AbortSignal|setTimeout|timeout" packages/bot/ --type tsRepository: akazdayo/omni-tts-discord
Length of output: 51
🏁 Script executed:
# Let's do a broader search for timeout patterns
rg "timeout|abort" --type ts packages/Repository: akazdayo/omni-tts-discord
Length of output: 51
🏁 Script executed:
# Check if there are any other files in the bot package that might have timeout patterns
find packages/bot -name "*.ts" -type f -exec grep -l "timeout\|abort\|signal" {} \;Repository: akazdayo/omni-tts-discord
Length of output: 51
🏁 Script executed:
# Check the exact Bun version and look up its AbortSignal.timeout() support
grep -A 5 "packageManager" package.jsonRepository: akazdayo/omni-tts-discord
Length of output: 104
🏁 Script executed:
# Check if there are any comments or documentation about the 10-second timeout choice
grep -r "10" packages/bot/lib/generate.tsRepository: akazdayo/omni-tts-discord
Length of output: 51
🏁 Script executed:
# Look at the full context of error handling to understand timeout error handling
cat -n packages/bot/lib/generate.ts | grep -A 10 "catch"Repository: akazdayo/omni-tts-discord
Length of output: 301
voice生成APIにタイムアウトがなく、サーバがハングした場合にキューが無期限に停滞するリスクがあります。
現在の実装では明示的なタイムアウトがないため、生成サーバが応答しない場合に fetch が永遠に待ち続けます。Bun 1.3.11 では AbortController を使用して明示的なタイムアウトを設定してください。
修正案
export const generateVoice = async (
text: string,
speaker: string,
): Promise<Result<Readable, GenerateError>> => {
try {
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 10_000);
const res = await fetch("http://localhost:8000/generate", {
body: JSON.stringify({ speaker, text }),
headers: { "Content-Type": "application/json" },
method: "POST",
+ signal: controller.signal,
});
+ clearTimeout(timeoutId);
if (res.status === 404) {
return err({ reason: "speaker_not_found", speaker });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const res = await fetch("http://localhost:8000/generate", { | |
| body: JSON.stringify({ speaker, text }), | |
| headers: { "Content-Type": "application/json" }, | |
| method: "POST", | |
| }); | |
| export const generateVoice = async ( | |
| text: string, | |
| speaker: string, | |
| ): Promise<Result<Readable, GenerateError>> => { | |
| try { | |
| const controller = new AbortController(); | |
| const timeoutId = setTimeout(() => controller.abort(), 10_000); | |
| const res = await fetch("http://localhost:8000/generate", { | |
| body: JSON.stringify({ speaker, text }), | |
| headers: { "Content-Type": "application/json" }, | |
| method: "POST", | |
| signal: controller.signal, | |
| }); | |
| clearTimeout(timeoutId); | |
| if (res.status === 404) { | |
| return err({ reason: "speaker_not_found", speaker }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/lib/generate.ts` around lines 15 - 19, The fetch call that posts
to "http://localhost:8000/generate" in generate.ts lacks a timeout and can hang;
wrap the request in an AbortController with a setTimeout (e.g., 10s or
configurable) that calls controller.abort(), pass controller.signal into fetch,
and clear the timeout after fetch resolves; update the surrounding function (the
code that defines const res = await fetch(...)) to catch an AbortError and
handle/throw a clear timeout-specific error so the queue does not stall
indefinitely.
| if (error.reason === "speaker_not_found") { | ||
| await resetSpeakerPreference(item.userId); | ||
| } |
There was a problem hiding this comment.
speaker_not_found 後のDBリセット失敗でキュー処理が止まる可能性があります。
resetSpeakerPreference が失敗すると currentFinished まで到達せず、現在アイテムが進まないまま終了します。ここは失敗を握って次アイテムへ進める方が安全です。
💡 修正案
if (!voiceResult.ok) {
const { error } = voiceResult;
if (error.reason === "speaker_not_found") {
- await resetSpeakerPreference(item.userId);
+ try {
+ await resetSpeakerPreference(item.userId);
+ } catch (resetError) {
+ console.error("failed to reset speaker preference", {
+ resetError,
+ userId: item.userId,
+ });
+ }
}
console.error(error);
if (queueByGuild.get(guildId) !== queue) {
return;
}
await runQueueCommands(guildId, queue, queue.currentFinished(item.id));
continue;
}Also applies to: 57-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/bot/lib/voice-queue.ts` around lines 54 - 56, When handling
error.reason === "speaker_not_found" wrap the call to
resetSpeakerPreference(item.userId) in its own try/catch so any failure is
logged but not rethrown, and ensure currentFinished() is still called (move it
to finally or after the catch) so the queue continues; apply the same pattern to
the other identical block (lines 57-62) to swallow/reset errors safely while
advancing the queue.
Closes #11
Summary by CodeRabbit
リリースノート
新機能
改善