Eng 417 update alpha functions #198
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis change implements a major schema and API refactor, replacing the previous agent-centric model with a platform account-centric model. It introduces new database entities and enums, updates API endpoints to align with the new structure, removes legacy agent and person logic, and migrates all relevant foreign keys and validation logic to use platform accounts and identifiers. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant SupabaseDB
Client->>API: POST /api/supabase/platform_account {name, platform, ...}
API->>API: Validate input (name, platform, account_local_id, etc.)
API->>SupabaseDB: Upsert PlatformAccount (by account_local_id + platform)
SupabaseDB-->>API: PlatformAccount record
API-->>Client: JSON response (success or error)
sequenceDiagram
participant Client
participant API
participant SupabaseDB
Client->>API: POST /api/supabase/agent_identifier {identifier_type, account_id, value, ...}
API->>API: Validate input (identifier_type, account_id, value, trusted)
API->>SupabaseDB: Upsert AgentIdentifier (by value + identifier_type + account_id)
SupabaseDB-->>API: AgentIdentifier record
API-->>Client: JSON response (success or error)
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@CodeRabbit 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: 18
🔭 Outside diff range comments (2)
packages/database/supabase/schemas/account.sql (1)
97-99:⚠️ Potential issueCritical: Access control table should have restricted permissions.
The
SpaceAccesstable controls authorization, yet grantsALLpermissions to anonymous users. This could allow unauthorized users to grant themselves access to spaces.Implement strict access controls:
-GRANT ALL ON TABLE public."SpaceAccess" TO anon; +-- Anonymous users should not have any access to the access control table GRANT ALL ON TABLE public."SpaceAccess" TO authenticated; GRANT ALL ON TABLE public."SpaceAccess" TO service_role;packages/database/schema.yaml (1)
448-452:⚠️ Potential issueInvalid default syntax for validation & epistemic_status
ifabsent: Validation(suggested)andifabsent: EpistemicStatus(unknown)should use literal enum values (e.g.ifabsent: 'suggested',ifabsent: 'unknown').
🧹 Nitpick comments (11)
apps/website/app/api/supabase/agent_identifier/route.ts (1)
29-30: Specify radix parameter for parseInt.When using
parseInt, always specify the radix parameter to avoid unexpected behavior with strings starting with "0".- if (!account_id || Number.isNaN(Number.parseInt(account_id))) + if (!account_id || Number.isNaN(Number.parseInt(account_id, 10)))apps/website/app/api/supabase/platform_account/route.ts (2)
34-38: Improve metadata validation error handling.The current error message doesn't specify what went wrong with JSON parsing.
else try { JSON.parse(metadata) } catch (error) { - return "metadata should be a JSON string"; + return `metadata should be a valid JSON string: ${error instanceof Error ? error.message : 'Invalid JSON'}`; }
45-47: Extract UUID regex to a constant.The UUID regex pattern should be extracted to a reusable constant for better maintainability.
+const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + const accountValidator: ItemValidator<PlatformAccountDataInput> = (account: any) => { // ... existing code ... if (dg_account != undefined) { if (typeof dg_account != "string") return "dg_account should be a UUID string"; - const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; - if (!uuidRegex.test(dg_account)) + if (!UUID_REGEX.test(dg_account)) return "dg_account must be a valid UUID"; }packages/database/supabase/schemas/account.sql (1)
11-14: Consider future extensibility of identifier types.The
AgentIdentifierTypeenum currently only supportsorcid. Consider whether additional identifier types might be needed in the future (e.g.,github,phone, etc.). Adding new enum values later requires a migration.packages/database/schema.yaml (6)
167-173: Use boolean literals fortrusteddefault
ifabsent: "false"should be an unquoted boolean (false).
301-301: Boolean default forobsolete
Changeifabsent: 'false'to the boolean literalfalse.
318-318: Arity default as string
Use numeric0forifabsentinstead of the string'0'.
329-329:is_schemadefault value
Switchifabsent: 'false'to the boolean literalfalse.
438-440: Position default as string
Convertifabsent: '0'forpositioninto numeric0.
444-445: Character position default
Replaceifabsent: '0'with numeric0forchar_position.packages/database/supabase/migrations/20250603144146_account_centric.sql (1)
92-99: CreateAgentIdentifiertable
Consider adding an index onaccount_idto improve lookup performance by account.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/database/schema.svgis excluded by!**/*.svg
📒 Files selected for processing (23)
apps/website/app/api/supabase/account/route.ts(0 hunks)apps/website/app/api/supabase/agent_identifier/[id].ts(1 hunks)apps/website/app/api/supabase/agent_identifier/route.ts(1 hunks)apps/website/app/api/supabase/person/[id].ts(0 hunks)apps/website/app/api/supabase/person/route.ts(0 hunks)apps/website/app/api/supabase/platform/route.ts(0 hunks)apps/website/app/api/supabase/platform_account/[id].ts(1 hunks)apps/website/app/api/supabase/platform_account/route.ts(1 hunks)apps/website/app/api/supabase/space/route.ts(2 hunks)packages/database/schema.puml(3 hunks)packages/database/schema.yaml(11 hunks)packages/database/supabase/config.toml(0 hunks)packages/database/supabase/migrations/20250603144146_account_centric.sql(1 hunks)packages/database/supabase/migrations/20250605083319_alpha_upload.sql(1 hunks)packages/database/supabase/migrations/20250605133006_update_alpha_functions.sql(1 hunks)packages/database/supabase/schemas/account.sql(2 hunks)packages/database/supabase/schemas/agent.sql(0 hunks)packages/database/supabase/schemas/base.sql(2 hunks)packages/database/supabase/schemas/concept.sql(1 hunks)packages/database/supabase/schemas/content.sql(2 hunks)packages/database/supabase/schemas/contributor.sql(2 hunks)packages/database/supabase/schemas/space.sql(1 hunks)packages/database/types.gen.ts(14 hunks)
💤 Files with no reviewable changes (6)
- packages/database/supabase/config.toml
- apps/website/app/api/supabase/platform/route.ts
- apps/website/app/api/supabase/person/[id].ts
- apps/website/app/api/supabase/person/route.ts
- apps/website/app/api/supabase/account/route.ts
- packages/database/supabase/schemas/agent.sql
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/website/app/api/supabase/agent_identifier/[id].ts (2)
apps/website/app/utils/supabase/apiUtils.ts (3)
makeDefaultGetHandler(92-113)defaultOptionsHandler(79-84)makeDefaultDeleteHandler(118-136)apps/website/app/api/supabase/agent_identifier/route.ts (1)
OPTIONS(65-65)
apps/website/app/api/supabase/platform_account/[id].ts (2)
apps/website/app/api/supabase/agent_identifier/[id].ts (3)
GET(7-7)OPTIONS(9-9)DELETE(11-11)apps/website/app/utils/supabase/apiUtils.ts (3)
makeDefaultGetHandler(92-113)defaultOptionsHandler(79-84)makeDefaultDeleteHandler(118-136)
apps/website/app/api/supabase/agent_identifier/route.ts (3)
apps/website/app/utils/supabase/server.ts (1)
createClient(5-47)apps/website/app/api/supabase/agent_identifier/[id].ts (1)
OPTIONS(9-9)apps/website/app/utils/supabase/apiUtils.ts (1)
defaultOptionsHandler(79-84)
🔇 Additional comments (49)
packages/database/supabase/schemas/base.sql (2)
27-27: LGTM! Enum value updated correctly for schema migration.The change from 'Account' to 'PlatformAccount' aligns with the broader schema refactor described in the AI summary.
40-40: Good documentation improvement.Adding a descriptive comment to the EntityType enum enhances code documentation and clarity.
packages/database/supabase/schemas/concept.sql (1)
73-73: LGTM! Foreign key correctly updated for schema migration.The change from referencing
AgenttoPlatformAccountis consistent with the broader schema refactor. The cascade behaviors are appropriately preserved.apps/website/app/api/supabase/agent_identifier/[id].ts (1)
7-7:✅ Verification successful
Verify AgentIdentifier table exists in database schema.
The API endpoint changes from "Account" to "AgentIdentifier" align with the schema migration. However, ensure that the
AgentIdentifiertable is properly defined in the database schema.Also applies to: 11-11
🏁 Script executed:
#!/bin/bash # Description: Verify that AgentIdentifier table exists in the database schema # Expected: Find AgentIdentifier table definition rg -A 10 'CREATE TABLE.*AgentIdentifier' packages/database/supabase/schemas/Length of output: 972
AgentIdentifier table confirmed in database schema
The
public."AgentIdentifier"table is defined inpackages/database/supabase/schemas/account.sql, so the updated GET and DELETE handlers using"AgentIdentifier"are valid and aligned with the schema.packages/database/supabase/schemas/contributor.sql (1)
19-19: LGTM! Foreign keys correctly updated for schema migration.Both contributor tables now properly reference
PlatformAccountinstead ofAgent, maintaining consistency with the broader schema refactor. The cascade behaviors are appropriately preserved.Also applies to: 39-39
apps/website/app/api/supabase/platform_account/[id].ts (1)
7-7: LGTM! Correct table name update for the new schema.The changes correctly update the API handlers to use "PlatformAccount" instead of "Platform", aligning with the new account-centric data model. The utility functions will handle this table name appropriately.
Also applies to: 11-11
apps/website/app/api/supabase/space/route.ts (2)
20-32: LGTM! Correct migration from foreign key to enum approach.The validation logic correctly updates from numeric
platform_idto stringplatformenum, with proper validation for the "Roam" and "Obsidian" values that match the SQL schema definition.
38-38: LGTM! Consistent payload updates.The destructuring and insertion payload correctly use the new
platformenum field instead ofplatform_id, maintaining consistency with the validation changes.Also applies to: 52-52
packages/database/supabase/schemas/space.sql (2)
1-6: LGTM! Good design pattern using enum for fixed values.Creating a Platform enum with 'Roam' and 'Obsidian' values is an excellent design choice for fixed platform types, eliminating the need for a separate lookup table and foreign key constraints.
14-14: LGTM! Consistent schema update.The Space table correctly uses the Platform enum type directly, which aligns perfectly with the API validation logic and simplifies the overall data model.
packages/database/supabase/schemas/content.sql (2)
34-36: LGTM! Correct foreign key migration for Document.author_id.The foreign key constraint correctly updates to reference PlatformAccount instead of Agent, maintaining the CASCADE behavior for referential integrity.
86-88: LGTM! Consistent foreign key updates for Content table.Both author_id and creator_id foreign key constraints correctly reference PlatformAccount instead of Agent, maintaining the appropriate SET NULL cascade behavior for optional relationships.
Also applies to: 91-93
packages/database/schema.puml (1)
5-95: Schema changes align with the platform-account-centric refactor.The PlantUML schema correctly reflects the migration from the agent-centric model to the platform-account-centric model, with proper relationships and field definitions.
packages/database/types.gen.ts (1)
1-731: Generated types correctly reflect the schema changes.The TypeScript type definitions properly represent the new
PlatformAccountandAgentIdentifiertables, updated foreign key relationships, and new enum types. The migration from agent-centric to platform-account-centric model is accurately reflected.packages/database/supabase/migrations/20250605083319_alpha_upload.sql (2)
3-137: Well-implemented deletion function with proper cascade handling.The
alpha_delete_by_source_local_idsfunction correctly handles cascading deletions across related tables and provides comprehensive error handling and progress reporting.
139-155: Simple and effective sync time retrieval.The function correctly retrieves the last update time for a given space.
packages/database/schema.yaml (12)
69-70: New EntityType values for account-centric model
The additions ofPlatformAccountandAgentIdentifierto theEntityTypeenum correctly reflect the refactor.
122-123: Introduce Users class
Defining aUsersclass for auth-managed accounts is appropriate and required for linkingdg_account.
134-137: Define account_local_id slot
Theaccount_local_idattribute is required and its description is clear.
154-158: Unique key for PlatformAccount
The composite unique key on(account_local_id, platform)aligns with the migration constraints.
158-160: Define AgentIdentifier class
IntroducingAgentIdentifierwith its description is correct.
161-166: AgentIdentifier attributesidentifier_type&account
The required typing and foreign-key reference toPlatformAccountare accurate.
175-179: AgentIdentifier unique key
The primary key on(value, identifier_type, account)is well-defined.
186-186: Addplatformslot to Space
EnsuringSpace.platformreferences thePlatformenum supports the new model.
197-197: Update SpaceAccessaccountreference
MappingaccounttoPlatformAccountcorrectly adapts the access control model.
252-252: Enhance Document description
The newDocumentdescription clarifies its role in the model.
351-351: Add ConceptSchema description
TheConceptSchemadescription addition is clear and valuable.
395-405: Update content-owner slots to PlatformAccount
Changingauthor,creator, andcontributorsslots to referencePlatformAccountfully implements the account-centric model.packages/database/supabase/migrations/20250603144146_account_centric.sql (21)
10-18: Create AgentType enum
DefiningAgentTypewithperson,organization, andautomated_agentcorrectly supports the new model.
20-29: Rename Person → PlatformAccount and update defaults
The table and index renames plus theiddefault are correct; verify thatentity_id_seqcontinues sequencing without gaps.
30-30: Dropping obsolete tableAutomatedAgent
Ensure no dependencies remain and backups are in place before this destructive change.
32-44: Add new columns toPlatformAccount
Columnsplatform,dg_account, andaccount_local_idlack defaults or NOT NULL until backfill; confirm null-handling and sequencing.
62-62: Deleting accounts without local IDs
Rows deleted byaccount_local_id IS NULLmay include valid records. Confirm cleanup intent.
64-64: Unique index on(account_local_id, platform)
This enforces the intended uniqueness constraint.
67-67: Foreign key toauth.users
Addingdg_accountFK withSET NULLandCASCADEis appropriate.
70-79: UpdateSpaceAccessforeign key
Re-pointingSpaceAccess.account_idtoPlatformAccountis correct.
80-80: Dropping legacyAccounttable
Confirm all references are removed and data migration is complete.
83-91: Create AgentIdentifierType enum
DefiningAgentIdentifierTypefororcidmatches the schema changes.
111-117: Migrateorcidinto AgentIdentifier
The INSERTs assume no duplicates and complete data; verify uniqueness and handle collisions.
119-123: Set NOT NULL on newPlatformAccountcolumns
Applying constraints post-backfill is the correct order.
134-137: Remove deprecatedplatform_idand enforce not-null
Droppingplatform_idand setting the new enum NOT NULL post-update is correct.
138-139: DropPlatformtable and rename enum
The transition from table to enumPlatformis handled properly.
141-146: Update Document FK to PlatformAccount
AdjustingDocument.author_idto referencePlatformAccountmaintains referential integrity.
148-153: Update Content author FK
RebindingContent.author_idtoPlatformAccountwith appropriate cascade.
154-159: Update Content creator FK
The new constraint onContent.creator_idis correctly defined.
160-164: Update Concept author FK
AligningConcept.author_idto the new model is accurate.
167-171: Update content_contributors FK
Updating the contributors join table to referencePlatformAccountis correct.
173-177: Update concept_contributors FK
The migration forconcept_contributorsis properly handled.
179-179: Dropping legacyAgenttable
Ensure no residual data or dependencies exist before this drop.
| COMMENT ON TYPE public."EntityType" IS 'The type of an entity'; | ||
|
|
||
| CREATE TYPE public."PlatformE" AS ENUM ( | ||
| 'Roam', | ||
| 'Obsidian' | ||
| ); | ||
|
|
||
| ALTER TYPE public."PlatformE" OWNER TO postgres; |
There was a problem hiding this comment.
PlatformE enum casing mismatches schema
The SQL defines PlatformE with 'Roam'/'Obsidian' but the LinkML schema uses lowercase. Harmonize enum definitions to avoid runtime errors.
🤖 Prompt for AI Agents
In packages/database/supabase/migrations/20250603144146_account_centric.sql
lines 1 to 8, the enum values for PlatformE are capitalized ('Roam', 'Obsidian')
but the LinkML schema uses lowercase. Update the enum values in the SQL to
lowercase ('roam', 'obsidian') to match the schema and prevent runtime
mismatches.
| WITH s AS (SELECT agent_id, write_permission, active, account_local_id | ||
| FROM public."Account" | ||
| JOIN public."Platform" AS p ON (platform_id=p.id) | ||
| WHERE url='https://roamresearch.com') | ||
| UPDATE public."PlatformAccount" AS pa | ||
| SET platform = 'Roam', | ||
| active = s.active, | ||
| write_permission = s.write_permission, | ||
| account_local_id = s.account_local_id | ||
| FROM s | ||
| WHERE pa.id = s.agent_id; | ||
|
|
There was a problem hiding this comment.
Backfill PlatformAccount data only for Roam
This update filters on url='https://roamresearch.com', leaving other platforms unpopulated. Extend logic or document support for multiple platforms.
🤖 Prompt for AI Agents
In packages/database/supabase/migrations/20250603144146_account_centric.sql
around lines 50 to 61, the update query only backfills PlatformAccount data for
the Roam platform by filtering on url='https://roamresearch.com'. To fix this,
either extend the query logic to handle multiple platforms by including
additional platform URLs and corresponding updates, or add documentation
clarifying that this migration currently supports only the Roam platform and
other platforms require separate handling.
| ALTER TABLE public."Space" ADD COLUMN platform public."PlatformE"; | ||
|
|
||
| WITH s AS (SELECT id | ||
| FROM public."Platform" | ||
| WHERE url='https://roamresearch.com') | ||
| UPDATE public."Space" AS space | ||
| SET platform = 'Roam' | ||
| FROM s | ||
| WHERE platform_id = s.id; |
There was a problem hiding this comment.
Backfill Space.platform only for Roam
Mirroring the account backfill issue, this update only covers one platform. Extend for full support.
🤖 Prompt for AI Agents
In packages/database/supabase/migrations/20250603144146_account_centric.sql
around lines 124 to 132, the backfill update for the new Space.platform column
only sets the platform for the Roam platform. To fix this, extend the update
logic to backfill the platform column for all existing platforms by joining the
Space table with the Platform table on platform_id and setting Space.platform to
the corresponding PlatformE value for each platform, ensuring full coverage
instead of just one platform.
|
@CodeRabbit apology I should have set the base properly. The only file to analyze here is |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@maparent, no worries! Let me focus specifically on the migration file you mentioned. 🏁 Script executed: Length of output: 7671 @maparent, thanks for clarifying! I've reviewed the Critical Issues🚨 Major Bug - Undefined Variable: -- Line 26: Declares v_caller_id
SELECT account_id INTO v_caller_id FROM public."AgentIdentifier" ...
-- Lines 84, 91: Uses undefined v_person_id
VALUES (v_space_id, v_person_id, v_node_created_at, ...)
VALUES (..., v_person_id, v_person_id, ...)Fix needed: -- Replace v_person_id with v_caller_id throughout the function
VALUES (v_space_id, v_caller_id, v_node_created_at, ...)
VALUES (..., v_caller_id, v_caller_id, ...)Potential Issues
Good Practices Observed✅ Proper Error Handling: Good use of exception blocks for timestamp and vector parsing Suggestions
The function shows good understanding of the new schema structure and includes thoughtful error handling, but the undefined variable bug would prevent it from executing successfully. |
maparent
force-pushed
the
ENG-417-update-alpha-functions
branch
from
cbac1e4 to
f928c4d
Compare
maparent
force-pushed
the
ENG-417-update-alpha-functions
branch
from
f928c4d to
eeda92f
Compare
maparent
force-pushed
the
ENG-417-update-alpha-functions
branch
from
eeda92f to
96c9edd
Compare
maparent
force-pushed
the
ENG-417-update-alpha-functions
branch
from
96c9edd to
697fd35
Compare
maparent
force-pushed
the
ENG-417-update-alpha-functions
branch
from
697fd35 to
9e489d1
Compare
2 participants
Summary by CodeRabbit
New Features
Improvements
Removals
Database