Skip to content

Initial feedback #1

Open
Open
@gregnr

Description

@gregnr

Some initial feedback in lieu of a PR.

  1. CREATE INDEX IF NOT EXISTS idx_messages_embedding ON public.messages
    USING ivfflat (embedding vector_cosine_ops) WITH (lists = 100);
    Let's swap this index to HNSW, which we recommend everyone use by default. Then also won't need this:
    // Set IVFFlat probes for better recall (balances speed vs accuracy)
    await connection.queryObject("SET ivfflat.probes = 3;");

  2. -- Create RLS policies for messages table
    CREATE POLICY "Users can view their own messages" ON public.messages
    FOR SELECT USING (auth.uid() = user_id);
    CREATE POLICY "Users can insert their own messages" ON public.messages
    FOR INSERT WITH CHECK (auth.uid() = user_id);
    CREATE POLICY "Users can update their own messages" ON public.messages
    FOR UPDATE USING (auth.uid() = user_id);
    CREATE POLICY "Users can delete their own messages" ON public.messages
    FOR DELETE USING (auth.uid() = user_id);

    We should wrap auth.uid() in select as described here.

  3. GRANT USAGE ON SCHEMA cron TO postgres;
    GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA cron TO postgres;
    GRANT USAGE ON SCHEMA cron TO service_role;
    GRANT ALL ON ALL TABLES IN SCHEMA cron TO service_role;
    -- Grant permissions for pg_net (http requests)
    GRANT USAGE ON SCHEMA net TO postgres;
    GRANT ALL ON ALL TABLES IN SCHEMA net TO postgres;
    GRANT USAGE ON SCHEMA net TO service_role;
    GRANT ALL ON ALL TABLES IN SCHEMA net TO service_role;

    Did the app break without these?

  4. return `[${embedding.join(",")}]`;

    JSON.stringify() would be cleaner here.

  5. // Parse webhook update
    if (body.message?.text && body.message?.from) {
    userPrompt = body.message.text;
    telegramUserId = body.message.from.id;
    chatId = body.message.chat.id;
    username = body.message.from.username;
    firstName = body.message.from.first_name;
    lastName = body.message.from.last_name;
    } else if (body.callback_query) {
    userPrompt = body.callback_query.data;
    telegramUserId = body.callback_query.from.id;
    chatId = body.callback_query.message.chat.id;
    username = body.callback_query.from.username;
    firstName = body.callback_query.from.first_name;
    lastName = body.callback_query.from.last_name;
    await answerCallbackQuery(body.callback_query.id);
    }

    Using zod to validate & type would be cleaner & more robust.

  6. id UUID PRIMARY KEY DEFAULT gen_random_uuid(),

    Any reason for choosing UUID as the ID datatype (over integer or bigint)?

  7. -- Create RLS policies for system_prompts table
    -- Note: Since system_prompts use chat_id instead of user_id, we rely on application-level security
    -- The service role (edge functions) will handle access control based on chat ownership
    -- Service role can access all system prompts (for functions to manage chat-specific prompts)
    CREATE POLICY "Service role can access all system prompts" ON public.system_prompts
    FOR ALL USING (current_setting('role') = 'service_role');

    Best practice would be to use postgres RLS for this instead of application. You would need to add user_id to this table, or track chats by ID in the DB and associate to a user_id. I didn't see auth checks in the edge function either btw, but might have missed it.

  8. const {
    userPrompt,
    id,
    userId,
    metadata = {},
    timezone,
    incomingMessageRole,
    callbackUrl
    } = body;
    if (!userPrompt || !id || !userId || !incomingMessageRole || !callbackUrl) {
    return new Response("Invalid request body", { status: 400 });
    }

    I would use zod here too.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions