Skip to content

feat: finalize RAG integration for tasks, tickets, and meetings - #186

Open
dolliecoder wants to merge 3 commits into
AOSSIE-Org:mainfrom
dolliecoder:feat/rag-integration-final
Open

dolliecoder wants to merge 3 commits into
AOSSIE-Org:mainfrom
dolliecoder:feat/rag-integration-final

Conversation

@dolliecoder

@dolliecoder dolliecoder commented Feb 22, 2026

Copy link
Copy Markdown
Contributor

Closes #65

📝 Description
This PR completes the end-to-end RAG (Retrieval-Augmented Generation) integration for tasks, tickets, and meetings.

Building on the previous work:
#160 PR1 introduced embedding schema support by adding vector columns for tasks and tickets.
#166 PR2 implemented vector similarity search functions at the database level.

This PR finalizes the pipeline by:

Extending the generate-embeddings Edge Function to support tasks and tickets (in addition to meetings).
Adding background processing functions using pg_net and pg_cron to generate embeddings for tasks and tickets with missing vectors.

Introducing a generic process_missing_embeddings() function to reduce duplication and ensure maintainability.
Updating AIService to safely handle Gemini responses, improve defensive parsing, and maintain full function-calling support for task, ticket, and meeting operations.

Adding request timeouts and robust type checks to prevent runtime crashes due to malformed API responses.
With this PR, the RAG system is now complete and consistent across meetings, tasks, and tickets.

🔧 Changes Made

Extended Edge Function to handle multiple entity types (meeting, task, ticket).
Added background embedding processors for tasks and tickets.
Implemented generic SQL processor to avoid duplicate logic.
Scheduled embedding processors using pg_cron.
Restored and validated full Gemini tool-calling support.
Added defensive JSON parsing and response validation in AIService.
Added HTTP timeout protection for Gemini API calls.
Improved safe date parsing and RPC response handling.

✅ Checklist

I have read the contributing guidelines.
I have added tests that prove my fix is effective or that my feature works.
I have added necessary documentation (if applicable).
Any dependent changes have been merged and published in downstream modules.

Summary by CodeRabbit

  • New Features

    • Automatic embedding generation for tasks and tickets with a controllable background processor.
    • Expanded support for multiple entity types and dual input formats for embedding requests.
    • Improved text extraction and trimming (8000-character limit) to ensure consistent embeddings.
  • Bug Fixes

    • More robust embedding requests with timeouts, clearer structured error responses, and validation.
    • Reliable and dynamic updating of embeddings in the database after generation.

@coderabbitai

coderabbitai Bot commented Feb 22, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Refactors the generate-embeddings Edge Function to be config-driven for multiple entity types, adds robust input validation, flattening/truncation, timeout-safe Gemini embedding calls, and structured JSON errors; also adds a PostgreSQL function to batch-request embeddings for tasks/tickets via HTTP calls.

Changes

Cohort / File(s) Summary
Embedding Function Refactor
supabase/functions/generate-embeddings/index.ts
Replaces hard-coded table/field usage with ENTITY_CONFIG; adds flattenObject, JSON body validation, backward-compatible input handling, dynamic Supabase fetch/update, 8000-char truncation, timeout/AbortController for Gemini calls, embedding response validation, and structured JSON error responses.
DB Migration: Embedding Processor
supabase/migrations/20251022_task_ticket_vector_processing.sql
Adds process_missing_embeddings(p_table, p_entity_type, p_limit) PL/pgSQL function that validates inputs, clamps limits, iterates records missing embeddings, issues HTTP POSTs to the embedding endpoint per record, logs responses, and throttles with pg_sleep.

Sequence Diagram

sequenceDiagram
    participant Cron as Cron Scheduler
    participant DB as Supabase DB
    participant EdgeFunc as Embedding Edge Function
    participant Gemini as Gemini Embedding API

    Cron->>DB: CALL process_missing_embeddings(table, entity_type, limit)
    DB->>DB: SELECT rows WHERE description IS NOT NULL AND description_embedding IS NULL
    DB->>EdgeFunc: HTTP POST (entity_type, entity_id)
    EdgeFunc->>DB: SELECT row FROM table (table from ENTITY_CONFIG)
    EdgeFunc->>EdgeFunc: flattenObject -> prepare text (truncate to 8000 chars)
    EdgeFunc->>Gemini: POST embedding request (with timeout/AbortController)
    Gemini-->>EdgeFunc: Return embedding payload
    EdgeFunc->>EdgeFunc: Validate embedding structure/values
    EdgeFunc->>DB: UPDATE table SET embedding_field = embedding WHERE id = entity_id
    DB-->>EdgeFunc: Confirm update
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰
I munched through nested JSON, teeth so spry,
Flattened each secret, sent vectors to the sky,
Cron hums, pg sleeps, requests take flight,
Tasks and tickets wake up wise and bright,
A tiny hop, a code—embeddings alight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: finalize RAG integration for tasks, tickets, and meetings' accurately summarizes the main objective of extending the RAG system to support tasks and tickets alongside meetings.
Linked Issues check ✅ Passed The PR addresses all primary objectives from issue #65: adds description_embedding columns to tasks/tickets schemas, implements automatic embedding generation via Edge Function, adds background processors for embedding backfill, and updates the AI service with defensive parsing and timeouts.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the RAG integration objectives: the index.ts updates support tasks/tickets in the Edge Function, and the migration adds the process_missing_embeddings function for background embedding generation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (4)
supabase/functions/generate-embeddings/index.ts (2)

60-74: Legacy meeting_id silently overrides entity_type/entity_id if both are provided.

If a caller passes { meeting_id: "x", entity_type: "ticket", entity_id: "y" }, the meeting path wins silently. This is fine for backward compatibility, but consider logging a warning or explicitly documenting that meeting_id takes precedence.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/generate-embeddings/index.ts` around lines 60 - 74, The
code silently prefers meeting_id over entity_type/entity_id in the request
parsing block; update the logic in the try block that reads req.json() (the
section that assigns entity_type/entity_id and checks body.meeting_id) to detect
when both meeting_id and entity_type/entity_id are provided and emit a warning
(e.g., processLogger.warn or console.warn) indicating that meeting_id takes
precedence and showing the provided values; keep the existing override behavior
and still validate against ENTITY_CONFIG[entity_type].

175-187: All errors return HTTP 400 — internal failures should return 500.

Errors like a missing Gemini API key, a failed embedding API call, or a database update failure are server-side issues, not bad client requests. Returning 400 for everything makes it harder for callers (including the pg_net cron processor) to distinguish retriable server errors from permanent client errors.

Consider classifying errors: keep 400 for validation failures (invalid entity_type/entity_id) and use 500 for upstream/internal failures.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/generate-embeddings/index.ts` around lines 175 - 187, The
catch block in the generate-embeddings handler currently maps all exceptions to
HTTP 400; change it to return 400 only for client/validation errors (e.g.,
invalid entity_type/entity_id) and 500 for server/upstream/internal failures
(missing Gemini API key, embedding API call failure, DB update errors).
Implement this by setting a status variable inside the handler or by throwing a
custom ValidationError for input validation and checking error type in the
catch; use status = 400 for ValidationError (or when validation failed in the
code path that currently does input checks) and status = 500 for all other Error
cases, then return the Response with that status and the same JSON body and
headers (including corsHeaders and "Content-Type": "application/json"). Ensure
the final catch uses error instanceof ValidationError (or an exported validation
error class) before deciding between 400 and 500.
supabase/migrations/20251022_task_ticket_vector_processing.sql (2)

6-39: The "generic" function hardcodes description and description_embedding column names.

Despite accepting p_table and p_entity_type as parameters, the SQL query on line 17 hardcodes description IS NOT NULL AND description_embedding IS NULL. This means the function cannot be reused for meetings (which use meeting_summary_json / summary_embedding). Consider parameterizing the column names if true generality is a goal, or rename the function to reflect its actual scope (e.g., process_missing_description_embeddings).

♻️ Parameterized version
 CREATE OR REPLACE FUNCTION process_missing_embeddings(
     p_table TEXT,
     p_entity_type TEXT,
-    p_limit INT DEFAULT 50
+    p_limit INT DEFAULT 50,
+    p_text_field TEXT DEFAULT 'description',
+    p_embedding_field TEXT DEFAULT 'description_embedding'
 )
 RETURNS void AS $$
 DECLARE
     record_id TEXT;
     embedding_function_url TEXT := current_setting('app.embedding_function_url');
 BEGIN
     FOR record_id IN EXECUTE format(
-        'SELECT id::text FROM %I WHERE description IS NOT NULL AND description_embedding IS NULL LIMIT %s',
+        'SELECT id::text FROM %I WHERE %I IS NOT NULL AND %I IS NULL LIMIT %s',
         p_table,
+        p_text_field,
+        p_embedding_field,
         p_limit
     )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/migrations/20251022_task_ticket_vector_processing.sql` around lines
6 - 39, The function process_missing_embeddings misleadingly hardcodes the
columns description and description_embedding making it not reusable; either add
two new parameters (e.g., p_text_col TEXT, p_embedding_col TEXT) and use them as
identifier placeholders in the EXECUTE/format call (use %I for column
identifiers) so the query becomes 'SELECT id::text FROM %I WHERE %I IS NOT NULL
AND %I IS NULL LIMIT %s' or, if generality isn't required, rename the function
to process_missing_description_embeddings and leave the hardcoded column names;
update all references and the function signature accordingly.

76-87: Consider staggering the two cron jobs to avoid concurrent bursts.

Both jobs are scheduled at */5 * * * *, so they fire simultaneously. If there's a backlog in both tasks and tickets, the edge function will receive a burst of up to 100 concurrent HTTP requests. Consider offsetting one by a minute or two:

 SELECT cron.schedule(
     'process-task-embeddings',
     '*/5 * * * *',
     $$SELECT process_tasks_missing_embeddings();$$
 );

 SELECT cron.schedule(
     'process-ticket-embeddings',
-    '*/5 * * * *',
+    '2-57/5 * * * *',
     $$SELECT process_tickets_missing_embeddings();$$
 );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/migrations/20251022_task_ticket_vector_processing.sql` around lines
76 - 87, The two cron.schedule entries ('process-task-embeddings' and
'process-ticket-embeddings') run at the same cron expression and will fire
concurrently; modify one of the cron expressions (for example the
'process-ticket-embeddings' schedule) to offset by 1–2 minutes (e.g., use a
"start at minute 1 and then every 5 minutes" style expression) so the jobs are
staggered and avoid simultaneous bursts to the edge function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@supabase/functions/generate-embeddings/index.ts`:
- Line 18: GEMINI_API_KEY is read with Deno.env.get("GEMINI_API_KEY") without
validation causing "undefined" to be sent in requests; update the module
initialization to check the GEMINI_API_KEY value (same pattern used for
SUPABASE_URL/SUPABASE_SERVICE_ROLE_KEY) and throw a clear error or exit early if
it's missing, and add the same guard before any code paths that use
GEMINI_API_KEY (e.g., the code around where it's interpolated at the request URL
near lines 115–116) so requests never proceed with an invalid key.
- Around line 115-116: Update the Google Generative Language model name used
when requesting embeddings: in the fetch call that assigns embeddingResponse
(the URL string passed to fetch), replace the deprecated model identifier
"embedding-001" with "gemini-embedding-001" so the request uses
`models/gemini-embedding-001:embedContent` while keeping the existing
GEMINI_API_KEY query parameter and surrounding fetch logic unchanged.

In `@supabase/migrations/20251022_task_ticket_vector_processing.sql`:
- Line 14: The migration embeds a literal placeholder in embedding_function_url
which will break net.http_post calls; change the function to read the real URL
at runtime instead of hardcoding it (replace the literal embedding_function_url
initialization in the function that calls net.http_post with a runtime lookup
such as using current_setting('app.embedding_function_url') or fetching from
Supabase Vault), and ensure you set that GUC or vault secret at deploy time so
the migration remains deployable and net.http_post targets the real endpoint.
- Around line 30-33: Replace the hardcoded Authorization value in the
jsonb_build_object that assigns headers (the headers variable) so the service
role key is not embedded in the function definition; instead retrieve the secret
at runtime (e.g., via the PostgreSQL GUC accessed by current_setting with a key
like app.supabase_service_role_key or from Supabase Vault) and concatenate it
with the "Bearer " prefix when building the Authorization header; update any
deployment docs to set the GUC or vault secret and ensure the function does not
contain the raw secret.

---

Nitpick comments:
In `@supabase/functions/generate-embeddings/index.ts`:
- Around line 60-74: The code silently prefers meeting_id over
entity_type/entity_id in the request parsing block; update the logic in the try
block that reads req.json() (the section that assigns entity_type/entity_id and
checks body.meeting_id) to detect when both meeting_id and entity_type/entity_id
are provided and emit a warning (e.g., processLogger.warn or console.warn)
indicating that meeting_id takes precedence and showing the provided values;
keep the existing override behavior and still validate against
ENTITY_CONFIG[entity_type].
- Around line 175-187: The catch block in the generate-embeddings handler
currently maps all exceptions to HTTP 400; change it to return 400 only for
client/validation errors (e.g., invalid entity_type/entity_id) and 500 for
server/upstream/internal failures (missing Gemini API key, embedding API call
failure, DB update errors). Implement this by setting a status variable inside
the handler or by throwing a custom ValidationError for input validation and
checking error type in the catch; use status = 400 for ValidationError (or when
validation failed in the code path that currently does input checks) and status
= 500 for all other Error cases, then return the Response with that status and
the same JSON body and headers (including corsHeaders and "Content-Type":
"application/json"). Ensure the final catch uses error instanceof
ValidationError (or an exported validation error class) before deciding between
400 and 500.

In `@supabase/migrations/20251022_task_ticket_vector_processing.sql`:
- Around line 6-39: The function process_missing_embeddings misleadingly
hardcodes the columns description and description_embedding making it not
reusable; either add two new parameters (e.g., p_text_col TEXT, p_embedding_col
TEXT) and use them as identifier placeholders in the EXECUTE/format call (use %I
for column identifiers) so the query becomes 'SELECT id::text FROM %I WHERE %I
IS NOT NULL AND %I IS NULL LIMIT %s' or, if generality isn't required, rename
the function to process_missing_description_embeddings and leave the hardcoded
column names; update all references and the function signature accordingly.
- Around line 76-87: The two cron.schedule entries ('process-task-embeddings'
and 'process-ticket-embeddings') run at the same cron expression and will fire
concurrently; modify one of the cron expressions (for example the
'process-ticket-embeddings' schedule) to offset by 1–2 minutes (e.g., use a
"start at minute 1 and then every 5 minutes" style expression) so the jobs are
staggered and avoid simultaneous bursts to the edge function.

Comment thread supabase/functions/generate-embeddings/index.ts
Comment thread supabase/functions/generate-embeddings/index.ts Outdated
Comment thread supabase/migrations/20251022_task_ticket_vector_processing.sql Outdated
Comment thread supabase/migrations/20251022_task_ticket_vector_processing.sql

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
supabase/functions/generate-embeddings/index.ts (2)

95-98: Supabase client is created per-request inside the handler.

This works correctly but creates a new client (and potentially a new connection) on every invocation. For a low-traffic Edge Function this is fine, but if call volume increases, hoisting the client outside the serve handler would be more efficient. Low priority given the MVP phase.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/generate-embeddings/index.ts` around lines 95 - 98, The
Supabase client (supabaseClient created via createClient) is currently
instantiated inside the serve handler on every request; hoist the
createClient(...) call to module scope (outside the serve/handler function) so a
single supabaseClient is reused across invocations, keeping the same
configuration (SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY) and updating only
request-specific logic inside the handler.

209-219: All errors return HTTP 400, but some are server-side faults.

Database errors, embedding API timeouts, and upstream API failures are not client errors — they should return 500. Returning 400 for everything makes it harder to debug and monitor, and misleads callers (including the pg_net background processor) about retry-ability.

Proposed approach — use a simple custom error class
class ClientError extends Error {
  constructor(message: string) {
    super(message);
    this.name = "ClientError";
  }
}

Then throw ClientError for input validation failures and use a plain Error for everything else. In the catch block:

   } catch (error) {
+    const status = error instanceof ClientError ? 400 : 500;
     return new Response(
       JSON.stringify({
         error: error instanceof Error ? error.message : "Unknown error",
       }),
       {
-        status: 400,
+        status,
         headers: { ...corsHeaders, "Content-Type": "application/json" },
       }
     );
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/generate-embeddings/index.ts` around lines 209 - 219, Add
a ClientError class (e.g., class ClientError extends Error { name =
"ClientError" }) and throw ClientError for input/validation failures in
functions like the request validation logic, leaving all other errors as plain
Error; then update the catch block in the generate-embeddings handler to inspect
the caught error (e.g., if (error instanceof ClientError) return status 400 else
return status 500), returning the error.message in the JSON body and preserving
the existing headers (and optionally log server-side errors before returning
500).
supabase/migrations/20251022_task_ticket_vector_processing.sql (1)

54-57: pg_sleep(0.2) inside the loop blocks the database connection for up to 100 seconds at max batch size.

With p_limit = 500, the total sleep time is 500 × 0.2 = 100 seconds. Since net.http_post is asynchronous (fire-and-forget via pg_net), the sleep is a simple rate limiter, but it holds a backend connection for the entire duration. This is fine for a low-frequency cron job, but worth noting if the batch size grows or concurrency increases.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/migrations/20251022_task_ticket_vector_processing.sql` around lines
54 - 57, The loop currently calls PERFORM pg_sleep(0.2) to rate-limit async
net.http_post calls (with p_limit up to 500), which blocks the backend for the
entire batch; remove the in-loop PERFORM pg_sleep(0.2) and instead implement
non-blocking dispatch: either (a) write the requests to a queue table and have a
separate background worker/process (pg_background, pg_cron, or an external
worker) pop and rate-limit deliveries, or (b) launch the entire dispatch loop
via a background job (pg_background.launch) so the caller connection is freed
while the background job can sleep as needed; update references in this
migration to stop sleeping inside the loop around net.http_post and ensure
p_limit-driven batching still writes queued requests (use the same
req_id/record_id/p_entity_type fields) for the async dispatcher to consume.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@supabase/functions/generate-embeddings/index.ts`:
- Around line 84-91: The guard incorrectly treats valid falsy IDs (like 0 or "")
as missing; update the check around entity_type/entity_id so you test entity_id
for null/undefined rather than falsiness and also validate its type matches the
declared signature (string | number). Specifically, in the block that references
ENTITY_CONFIG and variables entity_type and entity_id (the same area where the
function accepts string | number), replace the "!entity_id" falsy check with an
explicit null/undefined check (e.g., entity_id === null || entity_id ===
undefined) and/or add a typeof check (typeof entity_id === "string" || typeof
entity_id === "number") so only truly missing values are rejected.
- Around line 119-122: The current logic sets textToEmbed to String(rawValue ??
"") for non-meeting entities which yields "[object Object]" for object-typed
descriptions; change it to use flattenObject for all cases (replace the
conditional so textToEmbed = flattenObject(rawValue) regardless of entity_type)
or, if meetings must differ, add a guard before calling String that checks
typeof rawValue === "string" and falls back to flattenObject(rawValue) when
rawValue is an object; update the assignment for textToEmbed and reference
flattenObject, rawValue, and entity_type in your change.

In `@supabase/migrations/20251022_task_ticket_vector_processing.sql`:
- Around line 16-22: Add a consistency check between p_table and p_entity_type
so callers cannot pass mismatched pairs: validate that when p_table = 'tasks'
then p_entity_type = 'task' and when p_table = 'tickets' then p_entity_type =
'ticket' (and RAISE EXCEPTION on mismatch), or alternately derive p_entity_type
from p_table inside the function (e.g., compute entity_type := rtrim(p_table,
's')) and remove/ignore the p_entity_type parameter; update the existing IF ...
RAISE EXCEPTION logic to enforce the mapping using the p_table and p_entity_type
symbols.

---

Duplicate comments:
In `@supabase/functions/generate-embeddings/index.ts`:
- Around line 139-140: The fetch call constructing embeddingResponse uses the
deprecated model name embedding-001; update the model path in the URL to
gemini-embedding-001 so the request uses the current stable model (i.e., change
the string in the fetch call that currently contains
models/embedding-001:embedContent to models/gemini-embedding-001:embedContent),
keep the GEMINI_API_KEY usage unchanged, and scan adjacent code that references
embeddingResponse or the same endpoint to ensure all occurrences are migrated.

In `@supabase/migrations/20251022_task_ticket_vector_processing.sql`:
- Around line 47-50: The headers JSON currently built in the block (variable
headers using jsonb_build_object) omits the Authorization header which will
cause net.http_post calls to 401 when verify_jwt is true; update the code that
constructs headers to include an Authorization entry sourced at runtime (not
hard-coded) — e.g., read the token from a runtime GUC/current_setting or a
secrets store and add 'Authorization' => format('Bearer %s', <runtime_token>)
into the jsonb_build_object before calling net.http_post, and ensure the key
name (e.g., task_ticket.edge_function_jwt or similar) is documented for
maintainers so the function works without manual edits.
- Around line 12-13: The embedding_function_url currently contains a hardcoded
placeholder 'https://PROJECT_REF.supabase.co/functions/v1/generate-embeddings'
which must be replaced with a runtime configurable value; change the
initialization of embedding_function_url to read from the Postgres setting
(e.g., current_setting('app.embedding_function_url')) and fall back or raise if
missing, and adjust the existing fail-fast guard around embedding_function_url
to validate the retrieved setting so deployment will fail early if the
app.embedding_function_url setting is not configured. Use the symbol
embedding_function_url to locate the variable and
current_setting('app.embedding_function_url') as the recommended source.

---

Nitpick comments:
In `@supabase/functions/generate-embeddings/index.ts`:
- Around line 95-98: The Supabase client (supabaseClient created via
createClient) is currently instantiated inside the serve handler on every
request; hoist the createClient(...) call to module scope (outside the
serve/handler function) so a single supabaseClient is reused across invocations,
keeping the same configuration (SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY) and
updating only request-specific logic inside the handler.
- Around line 209-219: Add a ClientError class (e.g., class ClientError extends
Error { name = "ClientError" }) and throw ClientError for input/validation
failures in functions like the request validation logic, leaving all other
errors as plain Error; then update the catch block in the generate-embeddings
handler to inspect the caught error (e.g., if (error instanceof ClientError)
return status 400 else return status 500), returning the error.message in the
JSON body and preserving the existing headers (and optionally log server-side
errors before returning 500).

In `@supabase/migrations/20251022_task_ticket_vector_processing.sql`:
- Around line 54-57: The loop currently calls PERFORM pg_sleep(0.2) to
rate-limit async net.http_post calls (with p_limit up to 500), which blocks the
backend for the entire batch; remove the in-loop PERFORM pg_sleep(0.2) and
instead implement non-blocking dispatch: either (a) write the requests to a
queue table and have a separate background worker/process (pg_background,
pg_cron, or an external worker) pop and rate-limit deliveries, or (b) launch the
entire dispatch loop via a background job (pg_background.launch) so the caller
connection is freed while the background job can sleep as needed; update
references in this migration to stop sleeping inside the loop around
net.http_post and ensure p_limit-driven batching still writes queued requests
(use the same req_id/record_id/p_entity_type fields) for the async dispatcher to
consume.

Comment thread supabase/functions/generate-embeddings/index.ts
Comment thread supabase/functions/generate-embeddings/index.ts Outdated
Comment thread supabase/migrations/20251022_task_ticket_vector_processing.sql

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
supabase/functions/generate-embeddings/index.ts (1)

228-243: All errors uniformly return HTTP 400 — consider differentiating status codes.

Upstream/infrastructure failures (database errors, Gemini API errors, timeouts) are returned as 400 Bad Request, which is semantically incorrect and makes it harder for callers (e.g., pg_net processors) to distinguish retriable server-side failures from permanent client errors.

A lightweight improvement: return 500 for internal/upstream errors and reserve 400 for input validation failures. This is not blocking for MVP but would improve operational debuggability.

Sketch: error differentiation

One approach is a small custom error class:

class HttpError extends Error {
  constructor(public statusCode: number, message: string) {
    super(message);
  }
}

Then throw with the appropriate status:

// Validation errors
throw new HttpError(400, "Invalid entity_type");

// Upstream failures
throw new HttpError(502, `Embedding API error: ${errorText}`);
throw new HttpError(504, "Embedding API request timed out");

// Internal failures
throw new HttpError(500, `Database fetch error: ${error.message}`);

And in the catch block:

const statusCode = error instanceof HttpError ? error.statusCode : 500;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@supabase/functions/generate-embeddings/index.ts` around lines 228 - 243, The
current catch block in generate-embeddings/index.ts always returns status 400;
introduce a lightweight HttpError class (e.g., class HttpError {
constructor(public statusCode:number, message:string){...} }) and throw
HttpError(400, ...) for validation problems and appropriate 5xx/502/504
HttpError instances for upstream/database/timeouts inside functions that call
the Gemini API and DB; then update the catch handling in the existing catch
(error) { ... } to compute const statusCode = error instanceof HttpError ?
error.statusCode : 500 and return the Response with that statusCode (and the
same JSON body and headers) so client vs server errors are differentiated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@supabase/functions/generate-embeddings/index.ts`:
- Around line 155-168: The fetch call that requests embeddings uses the
deprecated model name "embedding-001" in both the URL and the request body;
update the request path and the body to use "gemini-embedding-001" instead of
"embedding-001" (i.e., change the URL ending and the model field in the JSON
sent when building embeddingResponse in the try block where fetch is called,
which references controller.signal and textToEmbed).

---

Nitpick comments:
In `@supabase/functions/generate-embeddings/index.ts`:
- Around line 228-243: The current catch block in generate-embeddings/index.ts
always returns status 400; introduce a lightweight HttpError class (e.g., class
HttpError { constructor(public statusCode:number, message:string){...} }) and
throw HttpError(400, ...) for validation problems and appropriate 5xx/502/504
HttpError instances for upstream/database/timeouts inside functions that call
the Gemini API and DB; then update the catch handling in the existing catch
(error) { ... } to compute const statusCode = error instanceof HttpError ?
error.statusCode : 500 and return the Response with that statusCode (and the
same JSON body and headers) so client vs server errors are differentiated.

@SharkyBytes

Copy link
Copy Markdown
Collaborator

Keepin as it is as #166 still about to merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FEATURE REQUEST: Implement RAG for tasks/tickets using vector embeddings to reduce token costs

2 participants