Add translations to discord bot #100
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
WalkthroughThis pull request integrates Sayable, an internationalization library, into the Discord bot. Changes include adding Sayable dependencies, wrapping Command and Modal classes with sayable, creating an i18n module with locale loading, updating component constructors to accept Sayable instances, and adding English locale translations. The Discord service is updated to inject Sayable across commands and components. Changes
Sequence DiagramsequenceDiagram
participant Client as Discord Client
participant Service as Discord Service
participant Say as Sayable i18n
participant Command as EvaluateCommand
participant Modal as EvaluateModal
participant Handler as Evaluation Handler
Client->>Service: Initialize
Service->>Say: Load say instance from i18n
Service->>Command: new EvaluateCommand(say)
Service->>Modal: new EvaluateModal(say)
Service->>Client: Register with SayablePlugin
Client->>Command: User executes /evaluate
Command->>Modal: new EvaluateModal(interaction.say, values)
Command->>Command: Show modal via interaction.showModal
Client->>Modal: User submits form
Modal->>Handler: handleEvaluating(interaction)
Handler->>Say: Generate message via interaction.say
Handler->>Handler: Build embed with localized labels
Handler->>Say: interaction.say for field names
Handler->>Client: Return decorated message response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes follow a consistent Sayable integration pattern across multiple file types (commands, components, modals, handlers), which provides repetition that reduces per-file complexity. However, the breadth of affected files, constructor signature updates, logic changes in handlers, and new configuration requirements demand verification across the entire integration layer. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/discord-bot/src/handlers/evaluate.ts (1)
66-83: I'll verify the Discord interaction behaviour to confirm the review comment's technical accuracy.Kia ora, I've verified the Discord interaction handling behaviour. It is not possible to edit a reply to change its ephemeral state once sent. More critically, the initial defer sets and fixates the ephemeral flag, meaning subsequent
followUp()calls respect that initial setting even if you specify a different ephemeral value.Your review comment is correct that deferring first then trying to set ephemeral afterward won't work as intended. However, there's a significant issue with the proposed fix: the suggested code calls
interaction.reply()after determining the runtime is missing, but reply() and deferReply() are both initial responses, which cannot be used to send additional messages. Once you've called defer or acknowledge (on theisEditbranch), you cannot callreply()again—you'd neededitReply()orfollowUp()instead.The correct approach is what the review suggests conceptually: check the runtime before deferring, and only defer if you plan to proceed with processing. For the error case, use
reply()immediately with ephemeral. For success, defer as needed.However, since the code uses conditional logic for
isNewvsisEdit, andisEditcallsacknowledge()(which also establishes the initial response), the error path must use the appropriate method:reply()forisNew, butfollowUp()forisEdit(which it already does correctly in the snippet).Reorder defer/acknowledge to occur only after confirming the runtime is found. Move the defer and acknowledge calls to after the runtime validity check. The error replies (with ephemeral) must occur before any defer/acknowledge call. The current structure deferring upfront prevents setting ephemeral properly on the error response.
🧹 Nitpick comments (12)
apps/discord-bot/tsup.config.ts (1)
2-2: Sayable plugin wiring looks fine; consider guarding by env.To avoid surprises in non-prod invocations, optionally enable the plugin only in builds (not tests/dev scripts).
-export default defineConfig({ +export default defineConfig({ entry: ['src/index.ts', 'src/env.ts'], format: 'esm', dts: true, - esbuildPlugins: [sayable()], + esbuildPlugins: process.env.NODE_ENV === 'production' ? [sayable()] : [], });Please confirm the plugin resolves apps/discord-bot/sayable.config.ts automatically (no inline options needed).
Also applies to: 8-8
apps/discord-bot/src/locales/en/messages.po (2)
1-8: Add Plural-Forms to header for correct pluralisation.Required by many tooling chains; prevents fallback mistakes.
msgid "" msgstr "" "POT-Creation-Date: 2025-10-01 15:21+1300\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" "Language: en\n" +"Plural-Forms: nplurals=2; plural=(n != 1);\n" "X-Generator: sayable\n"
38-40: Align on “STDIN” terminology.Current strings mix “STD Input” and “STDIN”. Prefer “STDIN” for consistency.
- msgid "STD Input" - msgstr "STD Input" + msgid "STD Input" + msgstr "STDIN"If you also change the source strings to “STDIN”, re‑extract to update msgids.
Also applies to: 84-88
apps/discord-bot/src/helpers/resolve-emoji.ts (1)
13-15: Remove unnecessary placeholder.The
_: ''placeholder appears unnecessary. TypeScript handles empty objects correctly, and the empty string value could cause runtime issues if accidentally referenced. If custom emojis are no longer needed, consider removing the CustomEmojis object entirely or leaving it as an empty object{}.export const CustomEmojis = { - // pencil: '1278931843986620428', - // globe: '1278931860805779517', - _: '', -} as const; +} as const;apps/discord-bot/src/components/open-evaluation-button.ts (1)
12-12: Normalise WEBSITE_URL to avoid missing slashIf
env.WEBSITE_URLlacks a trailing slash,${env.WEBSITE_URL}api/...elsewhere and this default here can concatenate incorrectly. Normalise once.- url = `${env.WEBSITE_URL}`; + // Ensure trailing slash for consistent concatenation elsewhere + url = `${(env.WEBSITE_URL ?? '').replace(/\/?$/, '/')}`;Also consider validating that
WEBSITE_URLis set at startup (see services/discord.ts).apps/discord-bot/src/components/evaluate-modal.ts (2)
16-25: Prefer strongly‑typed keys for modal valuesAvoid
Record<string, string | undefined>; constrain to known fields for safety and autocompletion.-import type { Sayable } from 'sayable'; +import type { Sayable } from 'sayable'; +type EvaluateFields = Partial<Record<'runtime' | 'code' | 'args' | 'input', string>>; @@ - constructor(say: Sayable, values?: Record<string, string | undefined>) { + constructor(say: Sayable, values?: EvaluateFields) {Apply the same alias to
EvaluateModalEdit.
44-47: Avoid Reflect.set for title if a direct API existsOverriding
titleviaReflect.setis brittle. If the base class exposestitleor accepts it insuper(...), prefer that. Alternatively, allowEvaluateModalto accept an optionaltitleparameter to avoid mutation.apps/discord-bot/src/services/discord.ts (1)
22-39: Validate WEBSITE_URL and normalise baseUrlIf
WEBSITE_URLis missing or lacks a trailing slash,baseUrlcan be malformed. Include it in theenabledcheck and normalise.-const enabled = Boolean( - env.DISCORD_CLIENT_ID && env.DISCORD_PUBLIC_KEY && env.DISCORD_TOKEN, -); +const enabled = Boolean( + env.DISCORD_CLIENT_ID && + env.DISCORD_PUBLIC_KEY && + env.DISCORD_TOKEN && + env.WEBSITE_URL +); @@ - baseUrl: `${env.WEBSITE_URL}api/discord`, + baseUrl: `${env.WEBSITE_URL!.replace(/\/?$/, '/') }api/discord`,apps/discord-bot/src/commands/evaluate.ts (1)
79-84: Consistent import style for ESM/TS resolutionHere you import EvaluateModal without an extension, while other files import local modules with “.js”. Pick one convention repo‑wide (ideally extensionless in TS and resolved by the build) to avoid Node ESM runtime resolution surprises.
apps/discord-bot/src/handlers/evaluate.ts (3)
103-108: Dynamic link inside a translation — confirm placeholder handlingThe string embeds a dynamic URL inside a say template. Verify the i18n layer preserves Markdown links and doesn’t escape or strip them, and that translators won’t break the
(${url})placeholder. If uncertain, prefer interpolation placeholders (e.g., {url}) or build the link text separately from the translated sentence.
128-139: Author fields: null assertions
interaction.user?.avatarUrl!andusername!use non‑null assertions. If the interaction can originate without a user (e.g., webhooks), consider guarding or providing a fallback to avoid runtime errors.
141-156: Minor UX: standardise terminology“STD Input” is uncommon; “STDIN” or “Standard input” reads clearer. Likewise consider “Command‑line arguments (CLI)” for consistency across locales.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.gitignore(1 hunks)apps/discord-bot/package.json(1 hunks)apps/discord-bot/sayable.config.ts(1 hunks)apps/discord-bot/src/commands/evaluate.ts(2 hunks)apps/discord-bot/src/components/edit-evaluation-button.ts(1 hunks)apps/discord-bot/src/components/evaluate-modal.ts(3 hunks)apps/discord-bot/src/components/open-evaluation-button.ts(1 hunks)apps/discord-bot/src/events/application-authorised.ts(1 hunks)apps/discord-bot/src/handlers/evaluate.ts(5 hunks)apps/discord-bot/src/helpers/embed-helpers.ts(1 hunks)apps/discord-bot/src/helpers/evaluate-helpers.ts(2 hunks)apps/discord-bot/src/helpers/resolve-emoji.ts(1 hunks)apps/discord-bot/src/helpers/session-context.ts(0 hunks)apps/discord-bot/src/i18n.ts(1 hunks)apps/discord-bot/src/index.ts(1 hunks)apps/discord-bot/src/locales/en/messages.po(1 hunks)apps/discord-bot/src/locales/fr/messages.po(1 hunks)apps/discord-bot/src/services/discord.ts(3 hunks)apps/discord-bot/tsup.config.ts(1 hunks)biome.json(2 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/discord-bot/src/helpers/session-context.ts
🔇 Additional comments (16)
apps/discord-bot/src/locales/en/messages.po (1)
25-28: Confirm interpolation style for {url}.Ensure the runtime uses the same placeholder syntax (e.g.,
{url}) and supplies a safe, already-escaped URL when formatting this message.package.json (1)
18-18: Biome upgrade is safe—no breaking changes documented.The upgrade from 2.1.x to 2.2.5 introduces no breaking changes; 2.2.0 is a minor release with feature additions and bugfixes, including a new patterns option for the noRestrictedImports rule, and subsequent 2.2.x releases are patch fixes and performance/bug fixes. Biome requires Node.js v14.18 or newer for npm install. Verify your CI's Node version meets this minimum, but no config or lint/format expectation adjustments are required.
apps/discord-bot/package.json (2)
22-34: Sayable packages confirmed resolved in lockfile—no action needed.The pnpm-lock.yaml demonstrates all referenced sayable packages (@sayable/[email protected], @sayable/[email protected], @sayable/[email protected], @sayable/[email protected], [email protected]) have already been successfully resolved with integrity hashes and complete dependency chains. The packages are actively imported throughout the codebase. No .npmrc is checked into the repository, which is expected—registry access and credentials are typically configured at user or CI level. The lockfile proves registry access was working at lock time.
36-39: Verify tsup 8 + esbuild plugin configuration is working correctly in CI.The codebase confirms use of tsup 8.5.0 with
esbuildPlugins: [sayable()]and unplugin-sayable 0.0.0-alpha.4 (alpha). tsup v8 has known reported issues where esbuild plugins via esbuildPlugins may not invoke correctly (e.g., onLoad filters not firing), and this alpha version compounds the risk. Before merging:
- Verify the build completes and unplugin-sayable transformations are applied (check build output or add logging to confirm plugin execution)
- Pin Node 20 LTS in CI to ensure consistent tooling behaviour (tsup 8 requires Node 14+ but Node 20 is safer)
- Test on the target Node version to rule out runtime incompatibilities
biome.json (2)
9-10: LGTM!Adding
.turboto the ignore list is standard practice for Turborepo projects to exclude build cache directories from linting and formatting.
60-63: LGTM!Disabling CSS linter and formatter is appropriate if the project doesn't use CSS or handles it with different tooling.
apps/discord-bot/src/helpers/evaluate-helpers.ts (1)
19-26: Emoji-based field lookup is well-structured.The migration from hardcoded strings to emoji-based lookups via
resolveEmojialigns well with the i18n changes and provides consistent labelling. However, the correctness of this code depends on the partial matching behaviour ingetEmbedField, which has ambiguity risks (see comment on embed-helpers.ts).apps/discord-bot/sayable.config.ts (1)
1-12: LGTM!The Sayable configuration is well-structured with appropriate source locale, supported locales, and output path patterns. The catalogue inclusion pattern correctly targets all TypeScript sources.
apps/discord-bot/src/locales/fr/messages.po (1)
1-119: French translations look good.The translations are clear, concise, and appropriate for the context. The PO file structure is correct with proper metadata headers.
apps/discord-bot/src/events/application-authorised.ts (1)
9-9: LGTM!Using indexed access type
this['type']is a more generic and type-safe approach that improves consistency with the Carbon framework's event typing patterns.apps/discord-bot/src/helpers/resolve-emoji.ts (1)
5-9: LGTM!The new Unicode emoji mappings support the emoji-based field labelling introduced in the i18n changes. Using Unicode emojis is preferable to custom emojis for broader compatibility.
apps/discord-bot/src/helpers/embed-helpers.ts (1)
10-10: The review comment's concern does not apply to the actual codebase.The
getEmbedFieldfunction is called with unique emoji characters (resolveEmoji('puzzle'),resolveEmoji('keyboard'),resolveEmoji('printer')), which resolve to distinct emojis (🧩, ⌨️, 🖨️). These emojis are embedded at the start of each field name (e.g.,🧩 CLI Arguments,⌨️ STD Input,🖨️ Output). Since each emoji is unique, there is no ambiguity or collision risk with the current.includes()implementation. The hypothetical example in the review ("searching for 'Input' would match both 'Input' and 'STD Input'") does not reflect how the function is actually used.Likely an incorrect or invalid review comment.
apps/discord-bot/src/components/edit-evaluation-button.ts (1)
11-16: Confirm customId uniqueness across component registriesThis button uses
customId = 'evaluate,edit'. The modal classEvaluateModalEditalso uses the same customId. If the framework dispatch tables are segregated by interaction type, this is fine; if not, it could cause handler clashes. Please confirm.apps/discord-bot/src/index.ts (1)
1-9: Verification confirmed — no action requiredThe web search results confirm that
createHandlerfrom@buape/carbon/adapters/fetchdoes handle both Discord signature verification and route dispatch automatically. The adapter performs signature verification using theDISCORD_PUBLIC_KEYenvironment variable and automatically routes interaction requests to the client through expected endpoints (e.g./interactions). Your code changes are correct.apps/discord-bot/src/commands/evaluate.ts (2)
15-47: Nice localisation surface — constructor LGTMThe sayable-wrapped Command and localised name/description/options are clean and idiomatic.
71-74: Verify option name lookups remain stable with localisationYou define option names via say
…, but read them using literal keys ('runtime' | 'code' | 'input' | 'arguments'). Ensure sayable(Command) registers these literals as the base command keys while emitting name_localizations, so getString('…') works regardless of user locale. If not, consider storing canonical keys separately and mapping localised labels.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/discord-bot/src/components/edit-evaluation-button.ts (1)
20-29: Handle missing/invalid embed and surface a friendly error.
getEvaluateOptions(embed)can throw; and when no embed exists you still open the modal with empty values. Guard and catch to avoid failing the interaction and to reply ephemerally with a localised error.Apply:
-import { Button, type ButtonInteraction, ButtonStyle } from '@buape/carbon'; +import { Button, type ButtonInteraction, ButtonStyle, MessageFlags } from '@buape/carbon'; @@ async run(interaction: ButtonInteraction) { @@ - const embed = interaction.embeds?.[0]; - const options = embed && getEvaluateOptions(embed); - return interaction.showModal( - new EvaluateModalEdit(interaction.say, options), - ); + const embed = interaction.embeds?.[0]; + if (!embed) { + return interaction.reply({ + content: interaction.say`I couldn't find the evaluation to edit.`, + flags: MessageFlags.Ephemeral, + }); + } + try { + const options = getEvaluateOptions(embed); + return interaction.showModal(new EvaluateModalEdit(interaction.say, options)); + } catch (err) { + captureEvent(getInteractionContext(interaction.rawData), 'evaluation_edit_parse_failed', { + button_id: this.customId, + error: String(err), + }); + return interaction.reply({ + content: interaction.say`I couldn't parse the evaluation to edit.`, + flags: MessageFlags.Ephemeral, + }); + }Based on learnings.
apps/discord-bot/src/handlers/evaluate.ts (1)
98-99: Possible crash when run is absent; use optional chaining.
executeResult.runcan be undefined (e.g., compile failure). Accessing.expiredwill throw.- else if (executeResult.run.expired) + else if (executeResult.run?.expired)apps/discord-bot/src/services/discord.ts (1)
29-30: Security: don’t reuse the OAuth client secret as the deploy webhook secret.Use a dedicated random secret (rotate if needed) to limit blast radius if leaked.
- deploySecret: env.DISCORD_CLIENT_SECRET!, + // Use a dedicated, random secret for deploy webhook auth + deploySecret: env.DISCORD_DEPLOY_SECRET!,Optionally fail fast if missing:
); if (!enabled) console.warn( 'Missing Discord bot environment variables, bot will be disabled.', ); +if (enabled && !env.DISCORD_DEPLOY_SECRET) + throw new Error('DISCORD_DEPLOY_SECRET is required for secure deploy webhooks.');
🧹 Nitpick comments (3)
apps/discord-bot/src/components/evaluate-modal.ts (1)
42-47: Avoid Reflect.set for title; expose a supported override.Using Reflect to mutate
titleis brittle. Prefer a constructor option so subclasses can set titles cleanly.-export class EvaluateModal extends sayable(Modal) { +export class EvaluateModal extends sayable(Modal) { customId = 'evaluate,new'; - constructor(say: Sayable, values?: Record<string, string | undefined>) { + constructor( + say: Sayable, + values?: Record<string, string | undefined>, + opts?: { title?: string }, + ) { super({ - title: say`Evaluate`, + title: opts?.title ?? say`Evaluate`, components: [ new RuntimeLabel(say, values?.runtime), new CodeLabel(say, values?.code), new ArgumentsLabel(say, values?.args), new InputLabel(say, values?.input), ], }); } @@ export class EvaluateModalEdit extends EvaluateModal { customId = 'evaluate,edit'; - constructor(say: Sayable, values?: Record<string, string | undefined>) { - super(say, values); - Reflect.set(this, 'title', say`Edit Evaluation`); - } + constructor(say: Sayable, values?: Record<string, string | undefined>) { + super(say, values, { title: say`Edit Evaluation` }); + }apps/discord-bot/src/handlers/evaluate.ts (2)
102-108: Wording vs threshold mismatch (optional).You call it “too large to display” at >900, but then allow up to 1000 when in a code block. Align thresholds to avoid confusion or bump the code‑block cap slightly lower for embed headroom.
- else if (output.length > 900) { + else if (output.length > 1000) { @@ - } else output = codeBlock(output, 1000); + } else output = codeBlock(output, 950);
121-173: Minor: author block can be simplified.Nit: you can guard inline to avoid the ternary.
- const embed = new Embed({ + const embed = new Embed({ title: interaction.say`Code Evaluation`, description: `**${runtime.name}** (v${runtime.version})\n${codeBlock(runtime.aliases[0]!, options.files[options.entry]!, 4000)}`, color: result.success ? 0x2fc086 : 0xff0000, fields: [], - author: interaction.user - ? { - name: interaction.user.username!, - icon_url: interaction.user.avatarUrl!, - } - : undefined, + author: interaction.user && { + name: interaction.user.username!, + icon_url: interaction.user.avatarUrl!, + }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/discord-bot/package.json(1 hunks)apps/discord-bot/sayable.config.ts(1 hunks)apps/discord-bot/src/components/edit-evaluation-button.ts(1 hunks)apps/discord-bot/src/components/evaluate-modal.ts(2 hunks)apps/discord-bot/src/handlers/evaluate.ts(5 hunks)apps/discord-bot/src/i18n.ts(1 hunks)apps/discord-bot/src/index.ts(1 hunks)apps/discord-bot/src/locales/en/messages.po(1 hunks)apps/discord-bot/src/services/discord.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/discord-bot/src/locales/en/messages.po
- apps/discord-bot/src/i18n.ts
- apps/discord-bot/package.json
🧰 Additional context used
🧬 Code graph analysis (3)
apps/discord-bot/src/services/discord.ts (3)
apps/discord-bot/src/commands/evaluate.ts (3)
EvaluateCommand(15-86)EvaluateCommand(14-79)run(62-78)apps/discord-bot/src/components/edit-evaluation-button.ts (1)
EditEvaluationButton(10-31)apps/discord-bot/src/components/evaluate-modal.ts (2)
EvaluateModal(14-40)EvaluateModalEdit(42-48)
apps/discord-bot/src/components/edit-evaluation-button.ts (5)
apps/discord-bot/src/helpers/resolve-emoji.ts (1)
resolveEmoji(80-94)apps/discord-bot/src/services/posthog.ts (1)
captureEvent(35-108)apps/discord-bot/src/helpers/session-context.ts (1)
getInteractionContext(9-20)apps/discord-bot/src/helpers/evaluate-helpers.ts (1)
getEvaluateOptions(12-29)apps/discord-bot/src/components/evaluate-modal.ts (1)
EvaluateModalEdit(42-48)
apps/discord-bot/src/handlers/evaluate.ts (5)
packages/execute/src/url.ts (1)
makeEditCodePathname(14-20)apps/discord-bot/src/helpers/discord-formatting.ts (1)
codeBlock(71-89)apps/discord-bot/src/helpers/resolve-emoji.ts (1)
resolveEmoji(80-94)apps/discord-bot/src/components/edit-evaluation-button.ts (1)
EditEvaluationButton(10-31)apps/discord-bot/src/components/open-evaluation-button.ts (1)
OpenEvaluationButton(7-14)
🔇 Additional comments (6)
apps/discord-bot/sayable.config.ts (4)
1-1: LGTM!The import is correct and aligns with the Sayable integration described in the PR summary.
4-5: Verify that only English locale is intentional.The configuration currently includes only 'en' as the locale. Given that the PR title is "Add translations to discord bot", this seems like it might be setting up the translation infrastructure without actually adding additional language support yet. Please confirm this is intentional, or if additional locales (such as 'fr' mentioned in the enriched summary) should be included.
6-11: LGTM!The catalogues configuration is correct. The include pattern appropriately covers all TypeScript source files, and the output path pattern follows standard conventions for organizing locale-specific message files.
3-12: LGTM!The overall configuration structure is correct and follows Sayable conventions. The integration with the unplugin-sayable in tsup.config.ts and the i18n.ts loader mentioned in the AI summary should work seamlessly with this configuration.
apps/discord-bot/src/index.ts (1)
11-15: Good fix: always flush analytics in finally.Wrapping the handler in try/finally ensures PostHog shutdown runs on both success and error. LGTM.
apps/discord-bot/src/services/discord.ts (1)
37-38: Verify TypeScript type resolution for @sayable/carbon augmentation.The review concern is substantiable: SayablePlugin from @sayable/carbon is instantiated in discord.ts without explicit tsconfig configuration to ensure type augmentation is picked up. The
.say()method is used extensively across handlers and components (evaluate.ts, edit-evaluation-button.ts, evaluate-modal.ts, commands/evaluate.ts), and there is currently no"types"array in tsconfig.base.json or triple-slash references to guarantee TypeScript recognises the augmentation.However, we cannot conclusively verify whether @sayable/carbon (currently at 0.0.0-alpha.4) declares types via package exports that TypeScript automatically discovers. Manually confirm that
interaction.saytype-checks correctly in your IDE and CI, or add@sayable/carbonto tsconfig"types"as a precautionary measure.
k0d13
force-pushed
the
translate/discord-bot
branch
from
925dfac to
dcbb25a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/discord-bot/src/services/discord.ts (2)
25-25: baseUrl concatenation still fragile.The baseUrl construction via template literal concatenation remains vulnerable to malformed URLs if
WEBSITE_URLlacks a trailing slash. This was previously flagged. Use the URL constructor for robust path joining.- baseUrl: `${env.WEBSITE_URL}api/discord`, + baseUrl: new URL('api/discord', env.WEBSITE_URL).toString(),
37-37: Verify TypeScript picks up @sayable/carbon type augmentations.The
SayablePlugin(say)integration requires TypeScript to recognise augmented types (e.g.,interaction.say). Per the previous review, ensuretsconfig.base.jsonincludes"types": ["@sayable/carbon"]incompilerOptionsso type augmentations are loaded andinteraction.sayusage doesn't cause type errors in editors/CI.
🧹 Nitpick comments (4)
apps/discord-bot/src/helpers/embed-helpers.ts (1)
10-10: Substring matching may introduce ambiguity.Changing from exact equality (
===) to substring matching (includes()) enables more flexible field lookups (e.g., emoji-prefixed fields), but introduces risk: if multiple fields share a common substring, the function returns the first match rather than the intended field.For safety, consider either:
- Documenting that field names must be unique substrings, or
- Using a more precise matching strategy (e.g., prefix matching with a delimiter).
apps/discord-bot/src/helpers/resolve-emoji.ts (2)
15-15: Remove or document the placeholder entry.The
_: ''entry inCustomEmojiscreates an edge case: ifresolveEmoji('_')is called, the function would return<:_:>, which is not a valid Discord emoji format. If this placeholder is intentional (e.g., to maintain object structure), document its purpose; otherwise, remove it.
13-14: Consider removing commented-out code.The commented custom emoji mappings for
pencilandglobeappear to be superseded by the Unicode emoji entries. If these are no longer needed, remove them to reduce clutter.apps/discord-bot/src/components/evaluate-modal.ts (1)
44-47: Using Reflect.set to override readonly property is fragile.Line 46 uses
Reflect.setto override thetitleproperty, likely because it's readonly in the parentModalclass. This workaround bypasses TypeScript's type safety and could break if theModalimplementation changes (e.g., property descriptors, getters/setters).Consider one of these safer alternatives:
- Pass the title override as a parameter to the parent constructor (preferred).
- If the
sayable(Modal)wrapper supports title override, use that mechanism.- Document why Reflect is necessary if no alternative exists.
constructor(say: Sayable, values?: Record<string, string | undefined>) { - super(say, values); - Reflect.set(this, 'title', say`Edit Evaluation`); + super({ + title: say`Edit Evaluation`, + components: [ + new RuntimeLabel(say, values?.runtime), + new CodeLabel(say, values?.code), + new ArgumentsLabel(say, values?.args), + new InputLabel(say, values?.input), + ], + }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
apps/discord-bot/package.json(1 hunks)apps/discord-bot/sayable.config.ts(1 hunks)apps/discord-bot/src/commands/evaluate.ts(2 hunks)apps/discord-bot/src/components/edit-evaluation-button.ts(1 hunks)apps/discord-bot/src/components/evaluate-modal.ts(2 hunks)apps/discord-bot/src/components/open-evaluation-button.ts(1 hunks)apps/discord-bot/src/events/application-authorised.ts(1 hunks)apps/discord-bot/src/handlers/evaluate.ts(5 hunks)apps/discord-bot/src/helpers/embed-helpers.ts(1 hunks)apps/discord-bot/src/helpers/evaluate-helpers.ts(2 hunks)apps/discord-bot/src/helpers/resolve-emoji.ts(1 hunks)apps/discord-bot/src/helpers/session-context.ts(0 hunks)apps/discord-bot/src/i18n.ts(1 hunks)apps/discord-bot/src/index.ts(1 hunks)apps/discord-bot/src/locales/en/messages.po(1 hunks)apps/discord-bot/src/services/discord.ts(3 hunks)apps/discord-bot/tsup.config.ts(1 hunks)biome.json(2 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/discord-bot/src/helpers/session-context.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/discord-bot/src/i18n.ts
- apps/discord-bot/src/locales/en/messages.po
- apps/discord-bot/src/events/application-authorised.ts
- biome.json
- apps/discord-bot/package.json
- apps/discord-bot/src/components/edit-evaluation-button.ts
- apps/discord-bot/sayable.config.ts
🧰 Additional context used
🧬 Code graph analysis (9)
apps/discord-bot/src/helpers/embed-helpers.ts (1)
apps/website/src/app/[locale]/(playgrounds)/playgrounds/playground-card-list.tsx (1)
alias(45-46)
apps/discord-bot/src/index.ts (1)
apps/discord-bot/src/services/posthog.ts (1)
captureEvent(35-108)
apps/discord-bot/src/helpers/evaluate-helpers.ts (3)
apps/discord-bot/src/helpers/embed-helpers.ts (1)
getEmbedField(9-11)apps/discord-bot/src/helpers/resolve-emoji.ts (1)
resolveEmoji(80-94)apps/discord-bot/src/helpers/discord-formatting.ts (1)
extractCodeBlocks(16-36)
apps/discord-bot/src/components/open-evaluation-button.ts (1)
apps/website/src/components/editor/opened-files/button.tsx (1)
file(39-43)
apps/discord-bot/src/components/evaluate-modal.ts (1)
packages/components/src/label.tsx (1)
Label(22-22)
apps/discord-bot/tsup.config.ts (3)
apps/website/src/app/manifest.ts (1)
say(7-12)apps/website/src/app/[locale]/(editor)/playgrounds/[playground]/page.tsx (1)
say(29-35)apps/website/sitemap.config.ts (1)
_(8-20)
apps/discord-bot/src/services/discord.ts (3)
apps/discord-bot/src/commands/evaluate.ts (1)
EvaluateCommand(15-86)apps/discord-bot/src/components/edit-evaluation-button.ts (1)
EditEvaluationButton(10-31)apps/discord-bot/src/components/evaluate-modal.ts (2)
EvaluateModal(14-40)EvaluateModalEdit(42-48)
apps/discord-bot/src/commands/evaluate.ts (4)
apps/discord-bot/src/services/posthog.ts (1)
captureEvent(35-108)apps/discord-bot/src/helpers/session-context.ts (1)
getInteractionContext(9-20)apps/discord-bot/src/handlers/evaluate.ts (1)
handleEvaluating(53-119)apps/discord-bot/src/components/evaluate-modal.ts (1)
EvaluateModal(14-40)
apps/discord-bot/src/handlers/evaluate.ts (7)
packages/execute/src/url.ts (1)
makeEditCodePathname(14-20)apps/discord-bot/src/helpers/discord-formatting.ts (1)
codeBlock(71-89)packages/runtimes/src/shapes.ts (2)
Runtime(3-10)Runtime(11-11)packages/execute/src/shapes.ts (4)
ExecuteOptions(48-92)ExecuteOptions(94-94)ExecuteResult(110-120)ExecuteResult(122-122)apps/discord-bot/src/helpers/resolve-emoji.ts (1)
resolveEmoji(80-94)apps/discord-bot/src/components/edit-evaluation-button.ts (1)
EditEvaluationButton(10-31)apps/discord-bot/src/components/open-evaluation-button.ts (1)
OpenEvaluationButton(7-14)
🔇 Additional comments (13)
apps/discord-bot/src/helpers/evaluate-helpers.ts (1)
19-26: Emoji-based field matching may be fragile across locales.The field extraction now relies on
resolveEmoji()returning specific Unicode emoji strings (⌨️, 🧩, 🖨️) andgetEmbedFieldusing substring matching. If field names are localized or formatted differently in future i18n updates, this extraction will fail silently—returningundefinedfor optional fields without alerting the caller.Consider adding validation or a fallback mechanism to ensure critical fields are found, or document that field names must always include the exact emoji returned by
resolveEmoji().apps/discord-bot/src/index.ts (1)
11-15: LGTM! Analytics shutdown now guaranteed.The
try/finallyblock ensuresposthog.shutdown()is always called, even when the handler throws. This addresses the previous review concern about analytics not being flushed on errors.apps/discord-bot/src/components/open-evaluation-button.ts (1)
7-11: LGTM! Sayable integration is consistent.The migration to
sayable(LinkButton)with a Sayable-based constructor aligns with the broader i18n integration pattern used across commands and modals. The optionalurlparameter maintains backward compatibility with the default.apps/discord-bot/src/components/evaluate-modal.ts (1)
50-122: LGTM! Internal component structure is well-organised.The internal label and input classes follow a consistent pattern, properly encapsulate modal composition, and correctly integrate the Sayable API for localised placeholders and labels.
apps/discord-bot/src/services/discord.ts (2)
29-29: deploySecret updated, but verify the secret is correct.The
deploySecretnow usesenv.DISCORD_CLIENT_SECRET!instead of the non-secretDISCORD_CLIENT_ID, which addresses the previous security concern. However, verify thatDISCORD_CLIENT_SECRETis the correct secret for deploy webhook authentication, as this name typically refers to OAuth client secrets rather than webhook secrets.
33-40: LGTM! Sayable wiring is consistent across the bot.All command, component, and modal constructors correctly receive the
sayinstance, establishing a coherent i18n integration pattern throughout the bot.apps/discord-bot/src/commands/evaluate.ts (2)
15-48: LGTM! Comprehensive Sayable integration.The command class correctly extends
sayable(Command)and uses the Sayable instance throughout metadata generation. The constructor pattern with a factory function ensures all command names, descriptions, and option metadata are localised via thesaytemplate literals.
66-84: LGTM! Parameter renamed for clarity.Renaming the parameter from
usetointeractionimproves code readability and aligns with conventional naming. The usage ofinteraction.sayto access the Sayable instance (Line 80) correctly leverages the augmented type provided bySayablePlugin.apps/discord-bot/src/handlers/evaluate.ts (3)
2-3: LGTM! Clean import additions for localization support.The new imports support the interaction-based localization pattern and emoji resolution used throughout the file.
Also applies to: 23-23
71-71: Excellent localization integration throughout the function.All user-facing messages now use
interaction.say, and the URL construction for long output correctly builds a full URL to the evaluation playground. The change to pass the fullinteractioninstead of justinteraction.userproperly supports the updated signature ofcreateEvaluationPayload.Also applies to: 97-97, 99-99, 101-108, 112-112
122-122: Well-structured signature change and comprehensive localization.The updated function signature accepting
interactioninstead ofuserprovides proper context for localization throughout embed and component construction. The emoji-prefixed field names are a nice touch, and all component instantiations correctly pass the localization callback.Also applies to: 129-129, 133-138, 144-144, 150-150, 154-154, 162-162, 164-164
package.json (1)
18-18: I'll search for information about Biome 2.2.5 and any breaking changes:Biome 2.2.5 update is safe; minimal breaking changes within v2.
The noImportCycles rule no longer detects import cycles that include one or more type-only imports by default, but you can restore the previous behaviour if needed. Several lint rules are now promoted to stable in v2.2. The migrate command takes care of breaking changes of the configuration, so run
biome migrateafter upgrading to ensure your configuration remains compatible.apps/discord-bot/tsup.config.ts (1)
8-8: Plugin configuration is syntactically correct and dependencies are properly declared.Static analysis confirms the sayable plugin integration follows correct unplugin patterns:
- Import path is correct:
unplugin-sayable/esbuild- Plugin instantiation is correct:
sayable()passed toesbuildPluginsarray- Both
sayable(0.0.0-alpha.6) andunplugin-sayable(0.0.0-alpha.4) are declared in package.json- Prebuild script (
sayable extract && sayable compile) and build pipeline are in placeYou should manually verify that the build completes successfully and transforms apply correctly once dependencies are installed in your environment. The configuration itself is sound and ready.
| @@ -1,7 +1,9 @@ | |||
| import { defineConfig } from 'tsup'; | |||
| import sayable from 'unplugin-sayable/esbuild'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
Consider the risks of using an alpha package without public documentation.
The unplugin-sayable package is at version 0.0.0-alpha.4 with no public documentation or stable release. This raises concerns about API stability, potential breaking changes, and maintainability.
Based on learnings.
Please verify:
- The package is functioning correctly in your build pipeline
- You have access to internal documentation or source code for troubleshooting
- Consider pinning the exact version and documenting the integration internally until a stable release is available
Run the following to verify the package installation and exports:
🏁 Script executed:
#!/bin/bash
# Verify unplugin-sayable package and esbuild export
cd apps/discord-bot
# Check if package is installed and inspect its structure
if [ -d "node_modules/unplugin-sayable" ]; then
echo "=== Package found ==="
cat node_modules/unplugin-sayable/package.json | jq '{name, version, exports, main}'
echo ""
echo "=== Checking for esbuild export ==="
ls -la node_modules/unplugin-sayable/ | grep -E "(esbuild|dist)"
else
echo "Package not found in node_modules"
fiLength of output: 33
I need to gather additional information to properly verify this review comment.
🏁 Script executed:
#!/bin/bash
# Check if unplugin-sayable is in package.json and examine lock files
echo "=== Checking apps/discord-bot/package.json ==="
if [ -f "apps/discord-bot/package.json" ]; then
jq '.dependencies, .devDependencies' apps/discord-bot/package.json 2>/dev/null | grep -A2 -B2 "sayable" || echo "No 'sayable' reference found"
fi
echo ""
echo "=== Checking for lock files ==="
ls -la apps/discord-bot/ | grep -E "package-lock|yarn.lock|pnpm-lock"
echo ""
echo "=== Checking if unplugin-sayable appears anywhere in workspace ==="
rg "unplugin-sayable" --type json apps/Length of output: 655
🌐 Web query:
unplugin-sayable npm package esbuild plugin
💡 Result:
I couldn't find an official or published package named "unplugin-sayable" on npm or GitHub. A Libraries.io stub exists for "[email protected]" but there is no standard npm/GitHub package page or documentation to use as an esbuild/unplugin. [1][2]
If you meant to:
- find an unplugin compatible with esbuild: unplugin-based packages expose an esbuild entry (e.g. myPlugin/esbuild) — see Unplugin docs. [2]
- create your own unplugin for esbuild: use the unplugin factory (createUnplugin or createEsbuildPlugin) and export the esbuild build variant. The unplugin docs show usage and the esbuild-specific options. [2]
Would you like me to:
- search again for a specific repo or author name, or
- show a short example of how to write an unplugin that works with esbuild?
Sources:
- Libraries.io — unplugin-sayable page.
- Unplugin documentation / npm (general unplugin usage and esbuild support).
---
🏁 Script executed:
```shell
#!/bin/bash
# Determine if @sayable is an internal monorepo package
echo "=== Checking for @sayable packages in monorepo ==="
find . -name "package.json" -type f | head -20 | xargs grep -l '"name".*@sayable' 2>/dev/null
echo ""
echo "=== Checking root monorepo config ==="
if [ -f "package.json" ]; then
echo "Root package.json:"
jq '.workspaces // .packages // empty' package.json
fi
echo ""
echo "=== Checking for unplugin-sayable in repo ==="
find . -type d -name "unplugin-sayable" 2>/dev/null | head -5
echo ""
echo "=== Checking tsup.config.ts for actual usage ==="
cat apps/discord-bot/tsup.config.ts | head -20
Length of output: 458
---
🏁 Script executed:
```shell
#!/bin/bash
# Final check: explicitly search packages/ directory
echo "=== Contents of packages/ directory ==="
ls -la packages/ 2>/dev/null || echo "No packages/ directory found"
echo ""
echo "=== Search packages/ for unplugin-sayable or sayable ==="
find packages/ -type d -name "*sayable*" 2>/dev/null
echo ""
echo "=== Verify if packages are actually listed in workspace ==="
grep -r "unplugin-sayable" packages/*/package.json 2>/dev/null || echo "Not found in packages/"
Length of output: 745
The import references an unresolvable package that will break the build.
The unplugin-sayable package at version 0.0.0-alpha.4 is not available on npm, not present as a local monorepo package, and not installed in node_modules. The tsup configuration will fail to resolve this import during the build process.
Before merging, you must:
- Locate where
unplugin-sayableis published or hosted (internal registry, local package, git dependency) - Ensure it's properly installed and accessible in the build environment, or
- Replace it with an available alternative if it's been deprecated
🤖 Prompt for AI Agents
In apps/discord-bot/tsup.config.ts around line 2, the import of
"unplugin-sayable/esbuild" points to a package ([email protected])
that is not resolvable and will break the build; either (A) locate the package
source (internal registry, monorepo workspace package, or git URL) and add it as
a proper dependency (or workspace link) so it is installed and resolvable at
build time, or (B) if the package is deprecated/unavailable, remove this import
and replace it with a supported alternative plugin or equivalent logic (update
tsup config accordingly) and run the build to verify resolution; ensure
package.json and lockfile are updated and CI builds succeed.
Summary by CodeRabbit
New Features
Chores