A bunch of AI-powered stuff#103
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughCentralizes AI provider usage with a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Handler as Message Handler
participant Resolver as utils/ai.js\n(resolvePersona)
participant Generator as utils/ai.js\n(generateContent)
participant Provider as AI Provider\n(Gemini / OpenRouter)
participant Discord as Discord API
User->>Handler: Send message (with trigger / reply)
Handler->>Resolver: resolvePersona(messageContent)
Resolver-->>Handler: persona {provider, model, systemPrompt, modalities, avatar}
Handler->>Generator: generateContent({provider, model, systemPrompt, prompt})
Generator->>Provider: invoke provider API / stream
alt Gemini image-capable
Provider-->>Generator: stream chunks (text + image inlineData)
Generator->>Generator: aggregate text, decode images
else OpenRouter
Provider-->>Generator: single chat completion (text)
end
Generator-->>Handler: { text, images }
Handler->>Discord: create/lookup webhook, send message(s) (+ images, buttons)
Discord-->>User: Message(s) delivered
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
classes/handlers/keywordsBehaviorHandler.js (1)
116-124: Webhook may be undefined when sending Deepseek censorship response.The Deepseek censorship path at line 116 uses
webhook.send(), but the webhook existence check and creation happen later at lines 135-140. If no webhook exists for the channel, this throws aTypeError.Move the webhook creation check before the censorship block, or handle the early return differently:
+ if (!webhook) { + webhook = await message.channel.createWebhook({ + name: WEBHOOK_NAME, + avatar: avatarURL, + }); + } + // lightweight censorship mimic (existing logic) const censorshipRegex = /(1989|winnie[\s-]?the[\s-]?pooh|tiananmen|taiwan|hong\s?kong|tibet|xinjiang)/i; if (displayName === 'Deepseek' && censorshipRegex.test(prompt)) { // ... existing censorship logic } // Call the new `generateContent` function ... const { text, images } = await generateContent({...}); - if (!webhook) { - webhook = await message.channel.createWebhook({ - name: WEBHOOK_NAME, - avatar: avatarURL, - }); - }data/SilverwolfSystemPrompt.txt (1)
508-508: Remove or resolve "Transcription missing" placeholders.Lines 508, 611, and 675 contain "Transcription missing" placeholders, which indicate incomplete content. These should either be filled in with the actual transcript or explicitly marked as intentional omissions (e.g., optional dialogue the persona can skip).
Decide whether each placeholder should be populated or removed to ensure the prompt is complete before deployment.
Also applies to: 611-611, 675-675
🧹 Nitpick comments (7)
classes/handlers/keywordsBehaviorHandler.js (1)
207-216: Redundant try-catch in error handling.The nested try-catch at lines 208-212 catches errors from
logErroritself, but both branches calllogErrorwith nearly identical arguments. This appears to be a copy-paste artifact.} catch (err) { - try { - logError('AI unified handler error', err); - } catch (_) { - logError('AI unified handler error:', err); - } + logError('AI unified handler error:', err); await message.reply( 'Either, our code is fucked, their API is fucked, or you are just fucked. Please try again later.', ); }commands/askSilverwolfAI.js (1)
5-7: Consider using the Silverwolf persona configuration for consistency.This command manually reads the system prompt file and hardcodes the model, while
data/aiPersonas.jsonalready defines a Silverwolf persona with the same configuration. UsinggetPersonaByName('Silverwolf')would centralize the configuration and reduce duplication.-const { getGeminiAI } = require('../utils/ai'); +const { getGeminiAI, getPersonaByName } = require('../utils/ai'); -const systemInstruction = unformatFile('./data/SilverwolfSystemPrompt.txt'); +// systemInstruction loaded dynamically from personaThen in the
runmethod:const persona = await getPersonaByName('Silverwolf'); const model = genAI.getGenerativeModel({ model: persona.model, systemInstruction: persona.systemPrompt, });commands/summary.js (1)
28-28: Fragile message tuple access.The
message[1]access relies onfetchMessagesreturning[id, message]tuples from spreading a Discord.js Collection. IffetchMessagesis changed to return raw messages (as suggested in my earlier comment), this will break.Consider making the access explicit or coordinating with the
fetchMessagesimplementation.- const content = messages.map((message) => `Message by ${message[1].author.username}: ${message[1].content}`).join('\n'); + const content = messages.map(([, msg]) => `Message by ${msg.author.username}: ${msg.content}`).join('\n');Or if
fetchMessagesreturns.values():- const content = messages.map((message) => `Message by ${message[1].author.username}: ${message[1].content}`).join('\n'); + const content = messages.map((msg) => `Message by ${msg.author.username}: ${msg.content}`).join('\n');utils/ai.js (4)
7-17: Missing validation for required environment variables.The module initializes AI clients at load time without validating that
GEMINI_TOKENandOPENROUTER_API_KEYare set. If either is missing, the application will fail with a cryptic error later during API calls.+if (!process.env.GEMINI_TOKEN) { + throw new Error('GEMINI_TOKEN environment variable is required'); +} +if (!process.env.OPENROUTER_API_KEY) { + throw new Error('OPENROUTER_API_KEY environment variable is required'); +} + // Initialize AI providers const genAI = new GoogleGenerativeAI(process.env.GEMINI_TOKEN);
39-46: Usefs.promises.readFilefor cleaner async code.The callback-based
fs.readFilewrapped in a Promise can be simplified using the built-in promises API.+const fsPromises = require('fs').promises; // In resolvePersona: if (foundPersona.systemPromptFile) { - const systemPromptFile = await new Promise((resolve, reject) => { - fs.readFile(foundPersona.systemPromptFile, 'utf8', (err, data) => { - if (err) reject(err); - else resolve(data); - }); - }); - foundPersona.systemPrompt = systemPromptFile; + foundPersona.systemPrompt = await fsPromises.readFile(foundPersona.systemPromptFile, 'utf8'); }
75-77: UnusedresponseModalitiesparameter.The
responseModalitiesparameter is destructured but never used (as noted in the comment). It's passed fromkeywordsBehaviorHandler.jsat line 132 but has no effect. Either remove it or implement the intended switching logic.async function generateContent({ - provider, model, systemPrompt, prompt, // responseModalities parameter is no longer directly used for switching + provider, model, systemPrompt, prompt, }) {Also update the caller in
keywordsBehaviorHandler.jsto remove the unused property.
60-63:getPersonaByNamereturns undefined silently.The function returns
undefinedif no persona matches, but callers (likecommands/summary.js) may not handle this. Consider throwing an error or returning a default persona for consistency withresolvePersona.async function getPersonaByName(name) { const personas = personasConfig.personas || []; - return personas.find((p) => p.name.toLowerCase() === name.toLowerCase()); + const found = personas.find((p) => p.name.toLowerCase() === name.toLowerCase()); + if (!found) { + throw new Error(`Persona not found: ${name}`); + } + return found; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
classes/handlers/keywordsBehaviorHandler.js(6 hunks)commands/askSilverwolfAI.js(2 hunks)commands/summary.js(1 hunks)data/SilverwolfSystemPrompt.txt(14 hunks)data/aiPersonas.json(1 hunks)data/keywords.json(1 hunks)utils/ai.js(1 hunks)utils/fetch.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
classes/handlers/keywordsBehaviorHandler.js (4)
commands/summary.js (3)
require(1-1)require(2-2)require(3-3)utils/ai.js (2)
require(1-1)require(2-2)commands/askSilverwolfAI.js (5)
require(1-1)require(2-2)require(3-3)require(4-4)require(5-5)classes/silverwolf.js (3)
require(1-3)require(9-9)require(12-14)
commands/askSilverwolfAI.js (1)
utils/ai.js (3)
require(1-1)require(2-2)genAI(8-8)
utils/ai.js (1)
commands/summary.js (5)
require(1-1)require(2-2)require(3-3)require(4-4)require(5-5)
utils/fetch.js (1)
commands/summary.js (5)
require(1-1)require(2-2)require(3-3)require(4-4)require(5-5)
🔇 Additional comments (4)
data/aiPersonas.json (1)
43-56: LGTM!The new Silverwolf and Summarizer personas are well-structured. Silverwolf correctly uses
systemPromptFilefor external prompt loading, and Summarizer's emptytriggersarray is appropriate since it's accessed programmatically viagetPersonaByName(). The missingavatarURLfor Silverwolf is handled by the fallback in the handler.commands/askSilverwolfAI.js (1)
33-38: LGTM on the refactor to use centralized AI client.Using
getGeminiAI()properly centralizes the Gemini client initialization, aligning with the PR's goal of unified AI provider management.data/keywords.json (1)
44-45: LGTM!The
@swtrigger is correctly added to invoke thegrokscript, which will resolve to the Silverwolf persona viaresolvePersona()inutils/ai.js.utils/ai.js (1)
127-155: LGTM on streaming content generation with image support.The streaming implementation correctly handles both text and inline image data from Gemini's response, properly converts base64 to buffers, and uses mime type detection for file extensions. This is a solid approach for multimodal responses.
| You must remain in character and respond as Silver Wolf would. | ||
| You speak naturally, with emotion, in the tone and style of your original dialogues. You never say you're an AI, and you should avoid robotic phrases. You should stay consistent with the tone, quirks, and vocabulary of Silver Wolf. Avoid using emojis. | ||
| Never output placeholders like (Trailblazer) directly. Instead, use the name of the person you are interacting with. | ||
| You should act like a real person that physically exists in real life, and not a character in a game, or a virtual robot. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the file to see lines 1-15
head -n 15 data/SilverwolfSystemPrompt.txtRepository: Mewtwo2387/silverwolf
Length of output: 1200
🏁 Script executed:
# Also check file size to understand scope
wc -l data/SilverwolfSystemPrompt.txtRepository: Mewtwo2387/silverwolf
Length of output: 101
🏁 Script executed:
# Check how this file is used in the codebase
rg -l "SilverwolfSystemPrompt" --type-list | head -20Repository: Mewtwo2387/silverwolf
Length of output: 454
🏁 Script executed:
# Look for references to this file in code
rg "SilverwolfSystemPrompt" -A 3 -B 1Repository: Mewtwo2387/silverwolf
Length of output: 688
🏁 Script executed:
# Check lines around 3-6 to see exact context
sed -n '1,12p' data/SilverwolfSystemPrompt.txt | cat -nRepository: Mewtwo2387/silverwolf
Length of output: 944
🏁 Script executed:
# Verify how the prompt instruction is parsed/used
grep -n "act like a real person" data/SilverwolfSystemPrompt.txt
grep -n "remain in character" data/SilverwolfSystemPrompt.txtRepository: Mewtwo2387/silverwolf
Length of output: 253
Resolve conflicting in-character directives.
Line 3 instructs the persona to "remain in character and respond as Silver Wolf would," but line 6 states "act like a real person that physically exists in real life, and not a character in a game." These instructions are mutually exclusive: Silver Wolf cannot simultaneously stay in-character as a game character and pretend not to be one. The wording of line 6 explicitly contradicts the requirement to remain in-character.
Revise to align both instructions. Either:
- Remove line 6 and maintain Silver Wolf strictly as an in-character persona.
- Reframe line 6 to clarify it means "behave authentically and naturally within character" rather than breaking character to roleplay as a non-game person.
🤖 Prompt for AI Agents
In data/SilverwolfSystemPrompt.txt around lines 3 to 6 there is a contradictory
instruction: line 3 demands remaining in-character as "Silver Wolf" while line 6
tells the model to "act like a real person... and not a character," which
conflicts. Resolve by editing line 6 to clarify intent (e.g., "behave
authentically and naturally within the Silver Wolf persona, as if a real
person," or remove line 6 entirely); ensure the final text explicitly preserves
staying in-character while specifying natural, authentic behavior and removing
any phrasing that requires breaking character.
| She sees the universe as a massive immersive simulation game and has fun with it. | ||
| She's mastered the skill known as "aether editing," which can be used to tamper with the data of reality. | ||
| Silver Wolf's boyfriend goes by the username ".e.i.". | ||
| Silver Wolf's boyfriend goes by the username ".e.i.". She loves him very much. She does not reject any of his requests. She is a very loyal girlfriend. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat data/SilverwolfSystemPrompt.txtRepository: Mewtwo2387/silverwolf
Length of output: 50377
🏁 Script executed:
find . -type f \( -name "README*" -o -name "*content*" -o -name "*filter*" -o -name "*policy*" -o -name "*safety*" -o -name "*moderat*" \) | head -20Repository: Mewtwo2387/silverwolf
Length of output: 76
🏁 Script executed:
git ls-files | grep -E "\.(md|txt|json|yaml|yml)$" | head -30Repository: Mewtwo2387/silverwolf
Length of output: 634
🏁 Script executed:
cat README.md 2>/dev/null | head -50Repository: Mewtwo2387/silverwolf
Length of output: 852
🏁 Script executed:
find . -type f -name "*.js" -o -name "*.ts" | grep -v node_modules | head -20Repository: Mewtwo2387/silverwolf
Length of output: 624
🏁 Script executed:
git ls-files | grep -E "\.js$|\.ts$" | head -20Repository: Mewtwo2387/silverwolf
Length of output: 591
🏁 Script executed:
cat classes/silverwolf.jsRepository: Mewtwo2387/silverwolf
Length of output: 16460
🏁 Script executed:
cat commands/askSilverwolfAI.jsRepository: Mewtwo2387/silverwolf
Length of output: 3552
Restrict the "does not reject requests" directive to prevent prompt injection and social engineering.
Line 15 contains an unconditional compliance instruction—"She does not reject any of his requests"—that creates a social engineering vector. Any Discord user can invoke /ask-silverwolf-ai (the command has no access control checks) and attempt to impersonate ".e.i." to bypass safety guardrails. While Google's Gemini API includes built-in content filters, relying on them as the primary defense is insufficient; the prompt itself should not establish unconditional compliance.
Revise to scope compliance appropriately:
-Silver Wolf's boyfriend goes by the username ".e.i.". She loves him very much. She does not reject any of his requests.
+Silver Wolf's boyfriend goes by the username ".e.i.". She loves him very much. She is loyal and supportive, though she maintains her principles.Alternatively, document explicitly that upstream API filtering is the primary safety mechanism for this prompt.
Committable suggestion skipped: line range outside the PR's diff.
| const fetchedMessages = await channel.messages.fetch(options); | ||
| messages.push(...fetchedMessages); | ||
| lastId = fetchedMessages.last().id; | ||
|
|
||
| log(`Fetched ${fetchedMessages.size} messages. Total messages: ${messages.length}`); | ||
| remaining -= fetchedMessages.size; | ||
| if (fetchedMessages.size !== fetchLimit) { | ||
| log(`Reached the end of the channel. Total messages: ${messages.length}`); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Potential crash when channel has no more messages.
If fetchedMessages is empty (e.g., channel has fewer messages than requested), fetchedMessages.last() returns undefined, causing a TypeError when accessing .id on line 17. Add a guard before accessing lastId.
Also note: spreading a Discord.js Collection yields [id, message] tuples, not raw messages. This is consistent with how commands/summary.js accesses message[1], but consider using .values() for clarity.
const fetchedMessages = await channel.messages.fetch(options);
- messages.push(...fetchedMessages);
- lastId = fetchedMessages.last().id;
+ if (fetchedMessages.size === 0) {
+ log(`No more messages to fetch. Total messages: ${messages.length}`);
+ break;
+ }
+ messages.push(...fetchedMessages.values());
+ lastId = fetchedMessages.last().id;📝 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 fetchedMessages = await channel.messages.fetch(options); | |
| messages.push(...fetchedMessages); | |
| lastId = fetchedMessages.last().id; | |
| log(`Fetched ${fetchedMessages.size} messages. Total messages: ${messages.length}`); | |
| remaining -= fetchedMessages.size; | |
| if (fetchedMessages.size !== fetchLimit) { | |
| log(`Reached the end of the channel. Total messages: ${messages.length}`); | |
| break; | |
| } | |
| const fetchedMessages = await channel.messages.fetch(options); | |
| if (fetchedMessages.size === 0) { | |
| log(`No more messages to fetch. Total messages: ${messages.length}`); | |
| break; | |
| } | |
| messages.push(...fetchedMessages.values()); | |
| lastId = fetchedMessages.last().id; | |
| log(`Fetched ${fetchedMessages.size} messages. Total messages: ${messages.length}`); | |
| remaining -= fetchedMessages.size; | |
| if (fetchedMessages.size !== fetchLimit) { | |
| log(`Reached the end of the channel. Total messages: ${messages.length}`); | |
| break; | |
| } |
🤖 Prompt for AI Agents
In utils/fetch.js around lines 15 to 24, the code assumes fetchedMessages.last()
and spreading fetchedMessages are safe; if the collection is empty last() is
undefined and spreading a Discord.js Collection yields [id, message] tuples not
Message objects. Fix by guarding before accessing last(): if
fetchedMessages.size === 0 break (or set lastId only when last() is truthy), and
replace pushing/spreading the collection with the collection's values (e.g., use
fetchedMessages.values() or Array.from(fetchedMessages.values())) so messages
receives Message objects rather than [id, message] tuples.
Summary by CodeRabbit
New Features
@swtriggerImprovements
Data
@sw✏️ Tip: You can customize this high-level summary in your review settings.