Skip to content

fix(bot): update embeds after flag edits #94

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
rosethornbush wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(bot): update embeds after flag edits #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/bot/src/commands/embed.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class EmbedCommand extends Command {
await handleUrls(
extractURLs(msg.content).map(({ url }) => ({ url })),
interaction,
"context_menu",
{ source: "context_menu" },
);
}

Expand All @@ -88,7 +88,7 @@ export class EmbedCommand extends Command {
force: interaction.options.getBoolean("force") ?? false,
})),
interaction,
"command",
{ source: "command" },
);
}
}
106 changes: 88 additions & 18 deletions apps/bot/src/lib/handleUrls.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
recordError,
span,
} from "./observability";
import { extractURLs, isSpoiler } from "./utils";

type EmbedSource = "message" | "command" | "context_menu";
type EmbedInteraction = Command.ChatInputCommandInteraction | Command.ContextMenuCommandInteraction;
Expand All @@ -35,6 +36,51 @@ export interface EmbedURLRequest {
force?: boolean;
}

interface HandleUrlsOptions {
source?: EmbedSource;
updateTargets?: ReadonlyMap<number, string>;
}

export function parseMessageURLs(content: string) {
const urls: EmbedURLRequest[] = [];
let suppressNativeEmbeds = true;

for (const match of extractURLs(content)) {
const before = content.slice(0, match.index);
const after = content.slice(match.endIndex);

if (before.endsWith("<") && after.startsWith(">")) continue;

if (before.endsWith("~")) {
suppressNativeEmbeds = false;
continue;
}

const flags: Partial<EmbedFlags> = {
Spoiler: isSpoiler(match.url, content),
};
let force = false;

if (before.endsWith("?@")) {
flags.SourceOnly = true;
force = true;
} else if (before.endsWith("?!")) {
flags.MediaOnly = true;
force = true;
} else if (before.endsWith("@")) {
flags.SourceOnly = true;
} else if (before.endsWith("!")) {
flags.MediaOnly = true;
} else if (before.endsWith("?")) {
force = true;
}

urls.push({ url: match.url, flags, force });
}

return { urls, suppressNativeEmbeds };
}

function canReactToMessage(msg: Message) {
if (!msg.channel) return false;
if (!msg.inGuild()) return true;
Expand All @@ -48,7 +94,7 @@ function canReactToMessage(msg: Message) {
export async function handleUrls(
urls: EmbedURLRequest[],
interactionOrMessage: EmbedInteraction | Message,
source?: EmbedSource,
options: HandleUrlsOptions = {},
) {
let msg: Message | null = null;
let interaction: EmbedInteraction | null = null;
Expand All @@ -59,7 +105,7 @@ export async function handleUrls(
interaction = interactionOrMessage;
}

const embedSource = source ?? (msg ? "message" : "command");
const embedSource = options.source ?? (msg ? "message" : "command");
let sentEmbed = false;
let sentFailureReaction = false;
const reactToFailure = async () => {
Expand Down Expand Up @@ -107,12 +153,19 @@ export async function handleUrls(

const matches = (
await Promise.all(
urls.map(async (request) => {
urls.map(async (request, requestIndex) => {
const match = await matchURL(request.url);
return match ? { ...request, ...match } : null;
return match ? { ...request, ...match, requestIndex } : null;
}),
)
).filter((m) => m !== null);
).filter(
(match) =>
match !== null && (!options.updateTargets || options.updateTargets.has(match.requestIndex)),
);
Comment on lines 154 to +164

@greptile-apps greptile-apps Bot Jul 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unnecessary matchURL calls for non-target URLs

When called with updateTargets, handleUrls still invokes matchURL for every URL in the full urls array and then filters down to only the update targets. For a message with N URLs where only one flag changed, N−1 unnecessary matchURL evaluations happen in parallel. Consider pre-filtering urls to only entries whose index is in updateTargets before the Promise.all.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/bot/src/lib/handleUrls.ts
Line: 154-164

Comment:
**Unnecessary `matchURL` calls for non-target URLs**

When called with `updateTargets`, `handleUrls` still invokes `matchURL` for every URL in the full `urls` array and then filters down to only the update targets. For a message with N URLs where only one flag changed, N−1 unnecessary `matchURL` evaluations happen in parallel. Consider pre-filtering `urls` to only entries whose index is in `updateTargets` before the `Promise.all`.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex


if (msg && options.updateTargets && matches.length !== options.updateTargets.size) {
await reactToFailure();
}

if (interaction && matches.length === 0) {
const requestId = `${embedSource}:${interaction.id}`;
Expand All @@ -132,15 +185,19 @@ export async function handleUrls(
return sentEmbed;
}

for (const [i, { platform, id, flags, force }] of matches.entries()) {
for (const [i, { platform, id, flags, force, requestIndex }] of matches.entries()) {
const startedAt = Date.now();
const requestId = msg ? `message:${msg.id}:${i}` : `${embedSource}:${interaction!.id}:${i}`;
const requestId = msg
? `message:${msg.id}:${requestIndex}`
: `${embedSource}:${interaction!.id}:${requestIndex}`;
const targetBotMessageId = options.updateTargets?.get(requestIndex);
const logContext: Record<string, unknown> = {
request_id: requestId,
source: embedSource,
platform,
post_id: id,
force: force ?? false,
operation: targetBotMessageId ? "update" : "create",
outcome: "success",
status_code: 200,
...(msg
Expand Down Expand Up @@ -297,26 +354,34 @@ export async function handleUrls(

try {
const botMessage = await span(
"discord.send",
targetBotMessageId ? "discord.edit" : "discord.send",
{
...spanAttributes,
"discord.guild_id": String(logContext.guild_id),
"discord.channel_id": String(logContext.channel_id),
},
async (sendSpan) => {
const message = msg
? i > 0 && msg.channel.isSendable()
? await msg.channel.send(response)
: await msg.reply(response)
: i === 0
? await interaction!.editReply(response)
: await interaction!.followUp(response);
let message: Message;
if (msg && targetBotMessageId) {
message = await msg.channel.messages.fetch(targetBotMessageId);
await message.edit(response);
} else if (msg) {
message =
i > 0 && msg.channel.isSendable()
? await msg.channel.send(response)
: await msg.reply(response);
} else {
message =
i === 0
? await interaction!.editReply(response)
: await interaction!.followUp(response);
}
sendSpan.setAttribute("discord.bot_message_id", message.id);
return message;
},
);
logContext.bot_message_id = botMessage.id;
if (msg) {
if (msg && !targetBotMessageId) {
await span(
"message_cache.save",
{
Expand All @@ -325,11 +390,16 @@ export async function handleUrls(
"discord.bot_message_id": botMessage.id,
},
async () => {
await container.messageCache.save(msg.id, botMessage.id, msg.author.id);
await container.messageCache.save(
msg.id,
botMessage.id,
msg.author.id,
requestIndex,
);
},
);
}
botEmbedsCreated.add(1, metricContext);
if (!targetBotMessageId) botEmbedsCreated.add(1, metricContext);
sentEmbed = true;
} catch (error) {
recordError(requestSpan, error);
Expand Down
28 changes: 25 additions & 3 deletions apps/bot/src/lib/messageCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const CACHE_URL = process.env.CACHE_URL ?? "redis://localhost:6379";

interface SourceMessageCache {
botMessageIds: string[];
botMessageIndexes?: Record<string, number>;
}

export class MessageCache {
Expand All @@ -24,20 +25,29 @@ export class MessageCache {
return new MessageCache(client as RedisClientType);
}

public async save(sourceMessageId: string, botMessageId: string, authorId: string) {
public async save(
sourceMessageId: string,
botMessageId: string,
authorId: string,
requestIndex: number,
) {
const messageKey = this.getSourceMessageKey(sourceMessageId);
const authorKey = this.getBotMessageAuthorKey(botMessageId);
const sourceKey = this.getBotMessageSourceKey(botMessageId);
const existing = await this.getSourceMessage(sourceMessageId);
const botMessageIds = existing?.botMessageIds ?? [];
const botMessageIndexes = existing?.botMessageIndexes ?? {};

if (!botMessageIds.includes(botMessageId)) {
botMessageIds.push(botMessageId);
}
botMessageIndexes[botMessageId] = requestIndex;

await this.client
.multi()
.set(messageKey, JSON.stringify({ botMessageIds }), { EX: MESSAGE_CACHE_TTL_SECONDS })
.set(messageKey, JSON.stringify({ botMessageIds, botMessageIndexes }), {
EX: MESSAGE_CACHE_TTL_SECONDS,
})
.set(authorKey, authorId, { EX: MESSAGE_CACHE_TTL_SECONDS })
.set(sourceKey, sourceMessageId, { EX: MESSAGE_CACHE_TTL_SECONDS })
.exec();
Expand All @@ -61,14 +71,16 @@ export class MessageCache {

const sourceMessage = await this.getSourceMessage(sourceMessageId);
const botMessageIds = sourceMessage?.botMessageIds.filter((id) => id !== botMessageId) ?? [];
const botMessageIndexes = sourceMessage?.botMessageIndexes ?? {};
delete botMessageIndexes[botMessageId];
const transaction = this.client.multi().del(keys);

if (botMessageIds.length === 0) {
transaction.del(this.getSourceMessageKey(sourceMessageId));
} else {
transaction.set(
this.getSourceMessageKey(sourceMessageId),
JSON.stringify({ botMessageIds }),
JSON.stringify({ botMessageIds, botMessageIndexes }),
{
EX: MESSAGE_CACHE_TTL_SECONDS,
},
Expand All @@ -83,6 +95,16 @@ export class MessageCache {
return sourceMessage?.botMessageIds ?? [];
}

public async getBotMessages(sourceMessageId: string) {
const sourceMessage = await this.getSourceMessage(sourceMessageId);
if (!sourceMessage) return [];

return sourceMessage.botMessageIds.map((id) => ({
id,
requestIndex: sourceMessage.botMessageIndexes?.[id],
}));
}

public async deleteSourceMessage(sourceMessageId: string) {
const botMessageIds = await this.getBotMessageIds(sourceMessageId);
const keys = [
Expand Down
5 changes: 3 additions & 2 deletions apps/bot/src/lib/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ export interface URLMatch {
export function extractURLs(content: string): URLMatch[] {
return [...content.matchAll(URL_RE)].map((match) => {
const index = match.index ?? 0;
const url = match[0].endsWith("||") ? match[0].slice(0, -2) : match[0];
return {
url: match[0],
url,
index,
endIndex: index + match[0].length,
endIndex: index + url.length,
};
});
}
Expand Down
44 changes: 1 addition & 43 deletions apps/bot/src/listeners/messageCreate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,49 +1,7 @@
import { Events, Listener } from "@sapphire/framework";
import { MessageFlags, type Message } from "discord.js";

import type { EmbedFlags } from "../lib/builder";
import { handleUrls, type EmbedURLRequest } from "../lib/handleUrls";
import { extractURLs, isSpoiler } from "../lib/utils";

function parseMessageURLs(content: string) {
const urls: EmbedURLRequest[] = [];
let suppressNativeEmbeds = true;

for (const match of extractURLs(content)) {
const before = content.slice(0, match.index);
const after = content.slice(match.endIndex);

if (before.endsWith("<") && after.startsWith(">")) continue;

if (before.endsWith("~")) {
suppressNativeEmbeds = false;
continue;
}

const flags: Partial<EmbedFlags> = {
Spoiler: isSpoiler(match.url, content),
};
let force = false;

if (before.endsWith("?@")) {
flags.SourceOnly = true;
force = true;
} else if (before.endsWith("?!")) {
flags.MediaOnly = true;
force = true;
} else if (before.endsWith("@")) {
flags.SourceOnly = true;
} else if (before.endsWith("!")) {
flags.MediaOnly = true;
} else if (before.endsWith("?")) {
force = true;
}

urls.push({ url: match.url, flags, force });
}

return { urls, suppressNativeEmbeds };
}
import { handleUrls, parseMessageURLs } from "../lib/handleUrls";

export class MessageCreateListener extends Listener<typeof Events.MessageCreate> {
public constructor(context: Listener.LoaderContext, options: Listener.Options) {
Expand Down
Loading
Loading