Skip to content

[integrations] Add open-brain-rest Edge Function#129

Closed
alanshurafa wants to merge 2 commits intoNateBJones-Projects:mainfrom
alanshurafa:contrib/alanshurafa/open-brain-rest
Closed

[integrations] Add open-brain-rest Edge Function#129
alanshurafa wants to merge 2 commits intoNateBJones-Projects:mainfrom
alanshurafa:contrib/alanshurafa/open-brain-rest

Conversation

@alanshurafa
Copy link
Copy Markdown
Contributor

Contribution Type

  • Recipe (/recipes)
  • Schema (/schemas)
  • Dashboard (/dashboards)
  • Integration (/integrations)
  • Repo improvement (docs, CI, templates)

What does this do?

Adds the open-brain-rest Edge Function — a REST API gateway that provides 12 required endpoints for the Next.js dashboard plus 4 optional ingest endpoints. This resolves issue #124, which reported that the dashboard's backend dependency was missing from the repo.

Includes 5 SQL migration files, the Deno/Hono Edge Function source, and utility modules for embedding generation, metadata classification, and sensitivity detection.

The integration introduces a serial_id BIGINT surrogate key on the thoughts table to support both UUID-based installs (from the getting-started guide) and BIGSERIAL-based installs (k8s variant) — the dashboard's numeric ID expectations work with either schema.

Requirements

  • Supabase project with Open Brain base setup
  • OpenAI API key (embeddings + classification) or OpenRouter API key (embeddings only)
  • Optional: Anthropic API key for richer metadata classification
  • Supabase CLI for deployment

Checklist

  • I've read CONTRIBUTING.md
  • My contribution has a README.md with prerequisites, step-by-step instructions, and expected outcome
  • My metadata.json has all required fields
  • I tested this on my own Open Brain instance
  • No credentials, API keys, or secrets are included

Closes #124 (together with the companion dashboard update PR)

alanshurafa and others added 2 commits March 27, 2026 16:58
REST API gateway providing 12+ endpoints for the Open Brain thoughts
database. Required backend for the Next.js dashboard (issue NateBJones-Projects#124).

Includes:
- Deno/Hono Edge Function with search, capture, browse, CRUD, stats,
  reflections, duplicates, and ingest endpoints
- 5 SQL migrations: schema extensions with serial_id surrogate key
  (supports both UUID and BIGSERIAL base schemas), reflections table,
  ingestion tables, 8 RPCs, and full-text search index
- Utility modules for embedding, classification, and sensitivity detection
- README with step badges, credential tracker, and troubleshooting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds the Deno/Hono REST API handler (index.ts) and shared utility
modules for embedding, classification, and sensitivity detection.

Files placed under function/ (not supabase/) to avoid the root
.gitignore rule that excludes supabase/ directories.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@shansen3
Copy link
Copy Markdown

Code review

Found 4 issues:

  1. upsert_thought returns action "created" but TypeScript checks for "inserted" -- every new thought capture will display "Updated" instead of "Captured new" in the response message.

SQL sets v_action := 'created':

v_action := 'created';
END IF;

TypeScript checks "inserted":

content_fingerprint: result.content_fingerprint,
message: `${result.action === "inserted" ? "Captured new" : "Updated"} thought #${result.thought_id} as ${prepared.type}`,
});

Same mismatch exists for upsert_reflection at line 704.

  1. get_thought_connections RPC RETURNS TABLE has no preview or overlap_type columns, but the TypeScript handler reads both -- they will always be undefined in API responses.

SQL return type (no preview or overlap_type):

RETURNS TABLE (
id BIGINT,
content TEXT,
type TEXT,
importance SMALLINT,
created_at TIMESTAMPTZ,
shared_topics JSONB,
shared_people JSONB,
overlap_count INT,
similarity FLOAT,
score FLOAT
) AS $$

TypeScript reads missing fields:

importance: row.importance,
preview: row.preview,
created_at: row.created_at,
shared_topics: row.shared_topics ?? [],
shared_people: row.shared_people ?? [],
overlap_count: row.overlap_count ?? 0,
score: row.score,
overlap_type: row.overlap_type,
}));

  1. serial_id SET NOT NULL runs as a separate statement after the backfill DO block. On a busy instance, rows inserted between the backfill and the NOT NULL constraint will have NULL serial_id, causing the migration to fail.

-- Step 5: Ensure NOT NULL
ALTER TABLE thoughts ALTER COLUMN serial_id SET NOT NULL;

  1. find_near_duplicates does an O(n^2) self-join on all thoughts with no WHERE clause limiting the scan. On a brain with 1,000+ thoughts this will time out the Edge Function (150s limit).

-- ── 6. find_near_duplicates ─────────────────────────────────────────
CREATE OR REPLACE FUNCTION find_near_duplicates(
p_threshold FLOAT DEFAULT 0.95,

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@matthallett1
Copy link
Copy Markdown
Collaborator

@claude review this PR

1 similar comment
@matthallett1
Copy link
Copy Markdown
Collaborator

@claude review this PR

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 30, 2026

Claude finished @matthallett1's task in 2m 25s —— View job


PR Review: [integrations] Add open-brain-rest Edge Function

  • Gather context (CLAUDE.md, CONTRIBUTING.md)
  • Read changed files (SQL, TypeScript, README, metadata.json)
  • Analyze existing review comments
  • Write comprehensive review

Summary

This is a well-structured, genuinely useful contribution — a REST API gateway that fills a real gap (resolves #124) and follows OB1's Edge Function pattern correctly. The README is thorough, the metadata is complete, and the SQL is idempotent and safe. That said, there are 6 bugs that need to be fixed before merge — 4 from the prior review (shansen3 caught the main ones), plus 2 new ones I found.


Alignment & Scope

✅ Fits OB1's integration category cleanly. Deploys as a remote Supabase Edge Function (not a local server), uses env vars for credentials, no hardcoded secrets, no destructive SQL. The design is appropriately scoped — it wraps the thoughts table rather than building its own data model.

The dependency on a smart-ingest Edge Function (for /ingest and /ingestion-jobs routes) should be noted — those 4 endpoints will error for anyone who hasn't deployed that function. The README mentions it; that's sufficient.


Bugs to Fix Before Merge

1. "created" vs "inserted" action mismatch — every new capture shows "Updated" (confirmed from prior review)

The SQL sets v_action := 'created' (sql/04-rpcs.sql:68) but TypeScript checks for "inserted" (function/index.ts:328, index.ts:704):

// index.ts:328 — will never match because SQL returns "created", not "inserted"
message: `${result.action === "inserted" ? "Captured new" : "Updated"} thought #${result.thought_id}`

Fix: change both TypeScript checks from "inserted" to "created", or change the SQL to return "inserted". Fix this →


2. content_fingerprint missing from upsert_thought payload — deduplication is silently broken (new)

prepareThoughtPayload correctly computes content_fingerprint and returns it, but handleCapture never includes it in p_payload (function/index.ts:302-313):

p_payload: {
  type: prepared.type,
  // ...
  // content_fingerprint is MISSING here
},

The SQL reads v_fingerprint := p_payload->>'content_fingerprint' and skips the duplicate check if it's NULL. As written, every /capture call inserts a new thought — the upsert/dedup logic never runs. Fix this →


3. Stats key mismatch — type counts and topics always return empty (new)

brain_stats_aggregate returns JSON keys "types" and "topics", but handleStats reads aggregate.top_types and aggregate.top_topics (function/index.ts:461,466):

// SQL returns: { total_count, types, topics }
// TypeScript reads: aggregate.top_types, aggregate.top_topics  ← wrong keys
parseAggregateCounts(aggregate.top_types, "type")   // always undefined → []
parseAggregateCounts(aggregate.top_topics, "topic") // always undefined → []

The /stats endpoint will always return types: {} and top_topics: [] regardless of what's in the database. Fix this →


4. get_thought_connections returns no preview or overlap_type columns (confirmed from prior review)

The SQL RETURNS TABLE for get_thought_connections (sql/04-rpcs.sql:251-262) has no preview or overlap_type columns, but the TypeScript handler reads both (function/index.ts:643,649). Both fields will always be undefined in API responses. The SQL does return truncated content (via LEFT(c.content, 200)) which could serve as the preview.

Fix: either add preview and overlap_type to the SQL return type, or remove them from the TypeScript mapping. Fix this →


5. serial_id NOT NULL migration race condition (confirmed from prior review)

In sql/01-schema-extensions.sql:58, the SET NOT NULL statement runs as a separate statement outside the DO $$ block. Any row inserted between the backfill and the constraint will have a NULL serial_id and cause the migration to fail. The fix is to move the SET NOT NULL inside the DO $$ block or wrap both in a single transaction.


6. find_near_duplicates O(n²) self-join — will time out on large brains (confirmed from prior review)

sql/04-rpcs.sql:353-370: the function joins thoughts to itself with no WHERE clause beyond embedding IS NOT NULL. At 1,000 thoughts that's ~500,000 pairs evaluated; at 5,000 thoughts it's ~12.5 million. The Edge Function has a 150s timeout.

A practical fix is to use pgvector's ANN index to find candidates close to each thought first (e.g. via match_thoughts) rather than the full cross-join, or at minimum add a created_at filter to limit the scan window.


Design Patterns

  • ✅ No DROP TABLE, DROP DATABASE, TRUNCATE, unqualified DELETE FROM
  • ✅ All functions use SECURITY DEFINER, env vars for credentials, no hardcoded keys
  • ✅ Deploys as remote Edge Function — no local server, no StdioServerTransport
  • CREATE ... IF NOT EXISTS throughout — migrations are idempotent
  • ⚠️ No Row Level Security on reflections, ingestion_jobs, ingestion_items. The service role bypasses RLS, so it won't break anything, but OB1's pattern (see docs/) is to enable RLS on all new tables. Worth adding ALTER TABLE ... ENABLE ROW LEVEL SECURITY even if policies are permissive for now.

Minor Observations

  • The sort query parameter in /thoughts browse is passed directly to .order() without validation against a whitelist of column names. Supabase's client handles this gracefully, but an invalid column name returns a 500 rather than a 400. Low impact, but easy to guard with a ALLOWED_SORT_COLUMNS set.
  • The ingest proxy (handleIngest) forwards the MCP_ACCESS_KEY to smart-ingest via x-brain-key. If the smart-ingest function uses a different access key, the proxy will silently pass auth failures through with a 401. Worth noting in the README troubleshooting section.

Verdict

The contribution is solid in structure and fills a genuine need. Fix the 6 bugs above before merge — especially bugs 2 and 3 (fingerprint dedup and stats keys) which are silent data-correctness issues that would be invisible to users during testing but would silently corrupt the dashboard's behavior in production. The prior review from @shansen3 is accurate; bugs 1, 4, 5, 6 are confirmed.

@alanshurafa
Copy link
Copy Markdown
Contributor Author

Closing this PR as part of the OB1 Alpha Milestone consolidation. This feature is being rebuilt as one of 12 clean, gate-compliant PRs that together form the alpha upgrade path. The consolidated PRs will be submitted once verified on the fork. See the full plan for details.

@alanshurafa alanshurafa closed this Apr 6, 2026
@alanshurafa alanshurafa reopened this Apr 6, 2026
@alanshurafa
Copy link
Copy Markdown
Contributor Author

Closing as part of the OB1 Alpha Milestone consolidation. This feature is being rebuilt as one of 12 clean, gate-compliant PRs that together form the alpha upgrade path. The consolidated PRs will be submitted once verified on the fork.

@alanshurafa alanshurafa closed this Apr 6, 2026
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.

[dashboards] Next.js dashboard missing open-brain-rest Edge Function dependency

3 participants