ENG-529 Add a basic cucumber test for getContext. #267
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Collaborator
Author
|
@CodeRabbit review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
📝 WalkthroughWalkthroughThis change introduces a new shared module for Supabase context-related API functions, updates their usage in the Roam app, and adds Cucumber-based integration tests for database context creation and content access control. It also enhances environment variable handling, updates type imports, modifies several database function signatures, and adds a new PostgreSQL stored function for account creation in space. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant RoamPlugin
participant ContextFunctions (@repo/ui)
participant API Server
participant Database
User->>RoamPlugin: Opens plugin
RoamPlugin->>ContextFunctions: fetchOrCreateSpaceId({password, url, name, platform})
ContextFunctions->>API Server: POST /space
API Server->>Database: Create or fetch Space
Database-->>API Server: Space ID
API Server-->>ContextFunctions: Space ID
ContextFunctions-->>RoamPlugin: Space ID
RoamPlugin->>ContextFunctions: createLoggedInClient(platform, spaceId, password)
ContextFunctions->>API Server: Authenticate client
API Server-->>ContextFunctions: Authenticated client
RoamPlugin->>ContextFunctions: fetchOrCreatePlatformAccount({platform, accountLocalId, name, email, spaceId, password})
ContextFunctions->>API Server: RPC create_account_in_space
API Server->>Database: Insert/update PlatformAccount, SpaceAccess, AgentIdentifier
Database-->>API Server: Account ID
API Server-->>ContextFunctions: Account ID
ContextFunctions-->>RoamPlugin: Account ID
Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/database/features/step_definitions/stepdefs.ts (1)
4-4: Verify import path extension.The import path includes a
.tsextension which may not be necessary and could cause issues in some environments.
🧹 Nitpick comments (2)
packages/ui/src/lib/supabase/contextFunctions.ts (1)
56-56: Consider parameterizing the platform value.The platform is hardcoded as "Roam" but might need to be parameterized for future multi-platform support.
- platform: "Roam", + platform,And update the function signature to accept platform as a parameter.
packages/database/features/step_definitions/stepdefs.ts (1)
36-38: Address the Space table deletion security issue.The commented assertion indicates a permissions issue that should be resolved to ensure complete database cleanup.
Would you like me to help create a follow-up issue to address the Space table deletion security configuration?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
apps/roam/scripts/compile.ts(1 hunks)apps/roam/src/utils/supabaseContext.ts(2 hunks)packages/database/cucumber.json(1 hunks)packages/database/features/getContext.feature(1 hunks)packages/database/features/step_definitions/stepdefs.ts(1 hunks)packages/database/inputTypes.ts(1 hunks)packages/database/package.json(1 hunks)packages/database/tsconfig.json(1 hunks)packages/database/types.gen.ts(4 hunks)packages/ui/src/lib/supabase/contextFunctions.ts(1 hunks)packages/ui/src/lib/utils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#198
File: packages/database/supabase/migrations/20250605083319_alpha_upload.sql:157-337
Timestamp: 2025-06-09T16:57:14.681Z
Learning: Migration files in packages/database/supabase/migrations/ are historical snapshots that preserve database schema and functions as they existed at the time of creation. These files should not be updated to reflect current schema changes, even if they reference tables or columns that have since been modified or removed. Schema incompatibilities in migration files are expected and acceptable as they represent the valid state at the time of migration.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250513173724_content_concept_key.sql:37-60
Timestamp: 2025-05-22T23:50:23.771Z
Learning: For the discourse-graph project, database schema management focuses on the final state in the supabase/schemas directory, not on the individual migration files. When reviewing database changes, consider only the schema definitions in this directory, not potential duplications or conflicts across migration files.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250512142307_sync_table.sql:20-21
Timestamp: 2025-05-20T03:04:21.602Z
Learning: Database migration files (like the SQL files in packages/database/supabase/migrations/) should not be modified after they're created, even to fix minor issues like unused variables. Issues should be addressed in newer schema files instead to maintain migration history integrity.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250504195841_remote_schema.sql:1-7
Timestamp: 2025-05-20T03:01:11.220Z
Learning: Supabase automatically creates necessary schemas like the "extensions" schema before migrations run, so there's no need to explicitly create these schemas in migration files.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/schemas/extensions.sql:1-11
Timestamp: 2025-05-20T03:02:36.027Z
Learning: The necessary schemas for Supabase extensions (like 'extensions', 'graphql', and 'vault') are automatically created by Supabase before extensions are installed, so they don't need to be explicitly created in SQL migration files.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/api/supabase/rpc/search-content/route.ts:53-56
Timestamp: 2025-05-30T14:37:30.215Z
Learning: The search-content route (apps/website/app/api/supabase/rpc/search-content/route.ts) is intentionally designed to be platform-agnostic rather than Roam-specific. The generic "Platform" terminology (like subsetPlatformIds) is used because the route will support multiple platforms in the near future, not just Roam.
packages/database/features/getContext.feature (4)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/supabase/migrations/20250603144146_account_centric.sql:123-136
Timestamp: 2025-06-04T11:41:08.589Z
Learning: Currently, there are no Obsidian spaces in the database - all existing spaces are Roam spaces. This means database migrations targeting space platform assignments only need to handle Roam for now.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/supabase/migrations/20250603144146_account_centric.sql:50-63
Timestamp: 2025-06-04T11:41:34.951Z
Learning: In the discourse-graph database, all accounts currently stored are Roam platform accounts, making platform-specific migration logic safe for global operations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
packages/database/inputTypes.ts (4)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/types.gen.ts:336-371
Timestamp: 2025-06-04T11:54:37.239Z
Learning: The team prefers not to manually edit generated files (like Supabase TypeScript types) even when they contain inaccuracies, as manual changes would be overwritten during regeneration. They treat generated files as read-only artifacts.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Prefer `type` over `interface`
packages/database/cucumber.json (5)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{js,jsx,ts,tsx,json,md,mdx} : Use Prettier with the project's configuration
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: apps/website/eslint.config.mjs:6-13
Timestamp: 2025-06-25T18:11:58.352Z
Learning: In TypeScript ESLint parser options, `parserOptions.project: true` is a valid boolean value that has been supported since version 5.52.0. When set to `true`, it automatically finds the nearest `tsconfig.json` file for each source file being linted, eliminating the need to specify explicit paths like `"./tsconfig.json"`. This is especially useful for monorepos and projects with multiple TypeScript configurations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: apps/website/eslint.config.mjs:6-13
Timestamp: 2025-06-25T18:11:58.352Z
Learning: In TypeScript ESLint parser options, `project: true` is a valid boolean value that has been supported since version 5.52.0. When set to `true`, it automatically finds the nearest tsconfig file, eliminating the need to specify explicit paths like `"./tsconfig.json"`.
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Prefer util functions for reusable logic and common operations
packages/database/tsconfig.json (14)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: apps/website/eslint.config.mjs:6-13
Timestamp: 2025-06-25T18:11:58.352Z
Learning: In TypeScript ESLint parser options, `parserOptions.project: true` is a valid boolean value that has been supported since version 5.52.0. When set to `true`, it automatically finds the nearest `tsconfig.json` file for each source file being linted, eliminating the need to specify explicit paths like `"./tsconfig.json"`. This is especially useful for monorepos and projects with multiple TypeScript configurations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/scripts/deploy.ts:0-0
Timestamp: 2025-05-22T23:48:45.450Z
Learning: The database deployment script in packages/database/scripts/deploy.ts requires that the branch is 'main' and pristine (no uncommitted or unpushed changes) before allowing deployment to proceed.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: apps/website/eslint.config.mjs:6-13
Timestamp: 2025-06-25T18:11:58.352Z
Learning: In TypeScript ESLint parser options, `project: true` is a valid boolean value that has been supported since version 5.52.0. When set to `true`, it automatically finds the nearest tsconfig file, eliminating the need to specify explicit paths like `"./tsconfig.json"`.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/types.gen.ts:336-371
Timestamp: 2025-06-04T11:54:37.239Z
Learning: The team prefers not to manually edit generated files (like Supabase TypeScript types) even when they contain inaccuracies, as manual changes would be overwritten during regeneration. They treat generated files as read-only artifacts.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Maintain consistent naming conventions: PascalCase for components and types
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Maintain consistent naming conventions: UPPERCASE for constants
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Prefer `type` over `interface`
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250512142307_sync_table.sql:20-21
Timestamp: 2025-05-20T03:04:21.602Z
Learning: Database migration files (like the SQL files in packages/database/supabase/migrations/) should not be modified after they're created, even to fix minor issues like unused variables. Issues should be addressed in newer schema files instead to maintain migration history integrity.
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{js,jsx,ts,tsx,json,md,mdx} : Use Prettier with the project's configuration
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Prefer util functions for reusable logic and common operations
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Maintain consistent naming conventions: camelCase for variables and functions
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Avoid `any` types when possible
apps/roam/src/utils/supabaseContext.ts (9)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/api/supabase/rpc/search-content/route.ts:53-56
Timestamp: 2025-05-30T14:37:30.215Z
Learning: The search-content route (apps/website/app/api/supabase/rpc/search-content/route.ts) is intentionally designed to be platform-agnostic rather than Roam-specific. The generic "Platform" terminology (like subsetPlatformIds) is used because the route will support multiple platforms in the near future, not just Roam.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: sid597
PR: DiscourseGraphs/discourse-graph#232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:18-31
Timestamp: 2025-06-22T10:40:52.752Z
Learning: In apps/roam/src/utils/getAllDiscourseNodesSince.ts, the user confirmed that querying for `?title` with `:node/title` and mapping it to the `text` field in the DiscourseGraphContent type is the correct implementation for retrieving discourse node content from Roam Research, despite it appearing to query page titles rather than block text content.
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-06-30T18:58:53.332Z
Learning: Applies to apps/roam/**/* : Use BlueprintJS 3 components and Tailwind CSS for platform-native UI
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-06-30T18:58:53.332Z
Learning: Applies to apps/roam/**/* : Use the roamAlphaApi documentation from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-06-30T18:58:53.332Z
Learning: Applies to apps/roam/**/* : Use Roam Depot/Extension API documentation from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#0
File: :0-0
Timestamp: 2025-06-25T22:56:17.522Z
Learning: In the Roam discourse-graph system, the existence of the configuration page (identified by DISCOURSE_CONFIG_PAGE_TITLE) and its corresponding UID is a system invariant. The code can safely assume this page will always exist, so defensive null checks are not needed when using `getPageUidByPageTitle(DISCOURSE_CONFIG_PAGE_TITLE)`.
Learnt from: sid597
PR: DiscourseGraphs/discourse-graph#226
File: apps/roam/src/components/settings/HomePersonalSettings.tsx:123-149
Timestamp: 2025-06-19T19:43:43.380Z
Learning: The "Fetch Embeddings for nodes" button in HomePersonalSettings.tsx is for testing purposes only, so it doesn't require production-level error handling or user feedback improvements.
packages/ui/src/lib/utils.ts (7)
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Prefer util functions for reusable logic and common operations
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Maintain consistent naming conventions: UPPERCASE for constants
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to {apps/website,packages/ui}/**/*.tsx : Use platform-native UI components first with shadcn/ui as a fallback
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to {apps/website,packages/ui}/**/*.tsx : When refactoring inline styles, use tailwind classes
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/api.mdc:0-0
Timestamp: 2025-06-30T18:58:19.625Z
Learning: You are working on the api routes for Discourse Graph which uses NextJS app router.
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to {apps/website,packages/ui}/**/*.tsx : Use Tailwind CSS for styling where possible
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to {apps/website,packages/ui}/**/*.tsx : Maintain visual consistency with the host application's design system
packages/database/package.json (8)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/scripts/deploy.ts:0-0
Timestamp: 2025-05-22T23:48:45.450Z
Learning: The database deployment script in packages/database/scripts/deploy.ts requires that the branch is 'main' and pristine (no uncommitted or unpushed changes) before allowing deployment to proceed.
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/obsidian.mdc:0-0
Timestamp: 2025-06-30T18:58:43.737Z
Learning: Applies to apps/obsidian/package.json : Prefer existing dependencies from package.json when adding or using dependencies in the Obsidian plugin
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{js,jsx,ts,tsx,json,md,mdx} : Use Prettier with the project's configuration
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Maintain consistent naming conventions: PascalCase for components and types
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Maintain consistent naming conventions: UPPERCASE for constants
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Maintain consistent naming conventions: camelCase for variables and functions
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Prefer util functions for reusable logic and common operations
apps/roam/scripts/compile.ts (1)
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-06-30T18:58:53.332Z
Learning: Applies to apps/roam/**/* : Prefer existing dependencies from package.json when adding or using dependencies
packages/database/features/step_definitions/stepdefs.ts (3)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250513173724_content_concept_key.sql:37-60
Timestamp: 2025-05-22T23:50:23.771Z
Learning: For the discourse-graph project, database schema management focuses on the final state in the supabase/schemas directory, not on the individual migration files. When reviewing database changes, consider only the schema definitions in this directory, not potential duplications or conflicts across migration files.
packages/database/types.gen.ts (6)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.645Z
Learning: Applies to **/*.{ts,tsx} : Use explicit return types for functions
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/schemas/embedding.sql:66-95
Timestamp: 2025-05-20T14:04:19.632Z
Learning: In the `match_embeddings_for_subset_nodes` SQL function in packages/database/supabase/schemas/embedding.sql, the number of results is implicitly limited by the length of the input array parameter `p_subset_roam_uids` since the function filters content using `WHERE c.source_local_id = ANY(p_subset_roam_uids)`.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/api/supabase/rpc/search-content/route.ts:53-56
Timestamp: 2025-05-30T14:37:30.215Z
Learning: The search-content route (apps/website/app/api/supabase/rpc/search-content/route.ts) is intentionally designed to be platform-agnostic rather than Roam-specific. The generic "Platform" terminology (like subsetPlatformIds) is used because the route will support multiple platforms in the near future, not just Roam.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#0
File: :0-0
Timestamp: 2025-06-07T02:56:57.093Z
Learning: In the discourse-graph project's upsert_content function, null creator_id values are tolerated as an acceptable trade-off, even though they may be annoying, rather than failing the entire operation.
packages/ui/src/lib/supabase/contextFunctions.ts (2)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/api/supabase/rpc/search-content/route.ts:53-56
Timestamp: 2025-05-30T14:37:30.215Z
Learning: The search-content route (apps/website/app/api/supabase/rpc/search-content/route.ts) is intentionally designed to be platform-agnostic rather than Roam-specific. The generic "Platform" terminology (like subsetPlatformIds) is used because the route will support multiple platforms in the near future, not just Roam.
🧬 Code Graph Analysis (1)
apps/roam/src/utils/supabaseContext.ts (1)
packages/ui/src/lib/supabase/contextFunctions.ts (2)
fetchOrCreatePlatformAccount(41-88)fetchOrCreateSpaceId(8-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: deploy
🔇 Additional comments (27)
packages/database/inputTypes.ts (1)
1-1: Good improvement to import consistency.Using the workspace-scoped import path
@repo/database/types.geninstead of the relative path improves maintainability and follows monorepo best practices.packages/database/cucumber.json (1)
1-6: Well-structured Cucumber configuration.The configuration correctly uses
ts-node/esmloader for TypeScript support and properly targets the step definitions directory. This follows standard Cucumber practices for TypeScript projects.packages/database/tsconfig.json (1)
1-13: Appropriate TypeScript configuration for the database package.The configuration correctly extends the base config and includes the necessary paths for the database package structure, including the new Cucumber test directories. The compiler options are well-suited for this package.
apps/roam/scripts/compile.ts (1)
135-135: Consistent environment variable addition.The
NEXT_API_ROOTenvironment variable follows the same pattern as the existing environment variables and provides a reasonable fallback to an empty string when undefined.packages/ui/src/lib/utils.ts (3)
4-4: Necessary TypeScript declaration for process global.The process global declaration is required for TypeScript when accessing environment variables. The typing is appropriate for the build environment context.
13-13: Clean environment detection constant.The
IS_DEVconstant provides a clear abstraction for development environment detection.
15-24: Well-structured API root resolution function.The
nextApiRootfunction follows good logic flow:
- First checks for explicit
NEXT_API_ROOTenvironment variable- Falls back to appropriate development/production URLs based on environment
This provides flexible API endpoint configuration while maintaining sensible defaults.
packages/database/features/getContext.feature (1)
1-21: Clean and well-structured feature file.The feature file follows BDD conventions effectively, with clear user story and acceptance criteria. The scenario outline appropriately tests both service and user perspectives for database entity verification.
packages/database/package.json (3)
18-18: Appropriate test script for Cucumber integration.The test script correctly compiles TypeScript sources before running Cucumber tests, ensuring step definitions are available in the expected directory.
26-29: Well-chosen dependencies for Cucumber testing.The dependencies are appropriate:
@cucumber/cucumberfor the testing frameworkprettier-plugin-gherkinfor feature file formattingts-node-maintainedas a reliable fork for TypeScript execution
32-36: Prettier configuration enables Gherkin formatting.Adding the Gherkin plugin ensures consistent formatting for feature files, improving maintainability.
apps/roam/src/utils/supabaseContext.ts (3)
13-13: Simplified import path for createClient.The import path has been appropriately simplified by removing the
/srcsegment.
15-18: Successfully centralized context functions.The refactoring moves API interaction logic to a shared module, improving code reusability and maintainability.
60-74: Updated function call to match new signature.The
fetchOrCreateSpaceIdcall correctly uses the new object parameter style and explicitly defines the required properties.packages/ui/src/lib/supabase/contextFunctions.ts (3)
8-39: Well-structured fetchOrCreateSpaceId function.The function has proper error handling, type validation, and clear parameter structure. The error messages provide helpful context for debugging.
41-88: Robust fetchOrCreatePlatformAccount with email handling.The function handles both account creation and optional email association. The error handling appropriately logs email association failures without stopping the process.
1-1: Import path extension is intentional and consistentThe
.tsextension is used across the codebase for type imports (dozens of occurrences in@repo/database/types.gen.ts), and our build/config setup handles these paths without issue. No change is needed here—please disregard the suggestion to remove the.tsextension.Likely an incorrect or invalid review comment.
packages/database/features/step_definitions/stepdefs.ts (2)
13-22: Proper Supabase client setup.The client factory functions correctly configure anonymous and service role clients with appropriate environment variables.
41-57: Well-structured step for simulating user actions.The step effectively simulates opening the roam plugin by creating platform accounts and spaces using the shared context functions.
packages/database/types.gen.ts (8)
592-595: Confirm correct argument order incompute_arity_localcallsI ran a search for
compute_arity_localin TS/JS files and only found its type definition—no call sites were detected. Please ensure that every call (including those in generated code or other languages) matches the updated signature:
- New order:
(lit_content: Json, schema_id: number)- Definition location:
packages/database/types.gen.tslines 592–595
575-583: Verify call sites for reordered arguments inalpha_upsert_discourse_nodesA search in
.ts/.jsfiles did not reveal any direct invocations ofalpha_upsert_discourse_nodes. Please manually confirm that:
- All generated clients, resolvers, or other integration layers invoking this proc use the new argument order:
p_nodesp_space_namep_user_emailp_user_name
645-654: No action needed: RPC named parameters are order-agnosticThe Supabase RPC call uses an object literal with named parameters, so property order doesn’t matter. The only usage in
apps/website/app/api/supabase/sync-task/[fn]/[target]/route.tscorrectly passes all keys (s_worker,s_target,s_function,timeout,task_interval).Likely an incorrect or invalid review comment.
622-635: No action needed: property order in TypeScript return types has no impact
Property reordering inmatch_content_embeddings’s generated return type won’t affect any code that accesses its properties by name. TypeScript’s structural typing ensures consumers remain compatible, so no changes are required.Likely an incorrect or invalid review comment.
596-604: No updates needed for end_sync_task callsAll usages invoke
supabase.rpc("end_sync_task", { … })with named parameters in an object, so the order of keys is irrelevant. The only occurrence in the codebase is:
- apps/website/app/api/supabase/sync-task/[fn]/[target]/[worker]/route.ts
672-691: Verify and update all call sites of upsert_discourse_nodes
I ran a global search (rg -A15 "upsert_discourse_nodes"in .ts/.js) and only found its type definition—no actual invocations. Please ensure you:
- Search for calls in all client code (including .tsx/.jsx or other languages) that invoke
upsert_discourse_nodesand update them to match the new parameter order.- Update any tests or scripts that reference this stored procedure.
- Add or refresh integration tests around this function to catch argument‐order mismatches in the future.
636-644: No action needed: return type property reorder is non-breakingVerified usages of
match_embeddings_for_subset_nodesin the codebase:
- In
apps/website/app/api/supabase/similarity-rank/route.ts, the RPC result is forwarded viacreateApiResponsewithout any destructuring or index-based access.- No other callers were found that reference properties by position.
Since consumers rely on named fields rather than order, the updated property sequence in the generated type is safe.
668-671: Verify Supabase RPC calls for upsert_content_embedding argument orderI wasn’t able to locate any internal calls to
upsert_content_embeddingin TS/JS files—please manually check that every RPC invocation (including in.tsx/.jsxor other client code) passes arguments in the new order:
- model
- embedding_array
- content_id
maparent
force-pushed
the
ENG-529-minimal-backend-testing
branch
from
6b8f5ab to
fba1309
Compare
maparent
force-pushed
the
ENG-529-minimal-backend-testing
branch
9 times, most recently
from
241461b to
ccc3044
Compare
Collaborator
Author
|
@CodeRabbit review |
Contributor
✅ Actions performedReview triggered.
|
maparent
force-pushed
the
ENG-529-minimal-backend-testing
branch
from
1c50fbc to
ccc3044
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
packages/database/features/contentAccess.feature (1)
30-30: Add missing closing triple quotes.The code block is missing its closing delimiter.
And a user logged in space s1 should see 2 Document in the database +```packages/database/types.gen.ts (1)
565-605: Review Supabase RPC Call SignaturesReordering parameters in the generated types won’t break your supabase.rpc calls—RPC binds by named properties, not position. However, a few function signatures have changed and your code must pass the correct named args:
• apps/website/app/api/supabase/sync-task/[fn]/[target]/route.ts
– propose_sync_task now only takes{ timeout }but you’re passing{ s_target, s_function }. Update the call to match the new signature or restore the missing params.• apps/website/app/api/supabase/sync-task/[fn]/[target]/[worker]/route.ts
– end_sync_task’s Args include{ s_worker, s_status }in addition to{ s_target, s_function }. Ensure you supply all required fields.No other calls are affected by parameter order—supabase.rpc uses object destructuring. Verify each RPC call’s object matches its function’s current Args shape in packages/database/types.gen.ts.
🧹 Nitpick comments (5)
packages/database/features/contentAccess.feature (1)
26-26: Remove duplicate step definition.This step duplicates line 15 from the Background section. Background steps are automatically executed before each scenario, so this is redundant.
Scenario Outline: Per-space document access - When the user user1 opens the roam plugin in space s1 Then the database should contain 3 Documentpackages/database/supabase/schemas/account.sql (1)
118-118: Consider explicit error handling for invalid space_id.While
STRICTwill raise an exception if the space doesn't exist, consider adding explicit validation with a more user-friendly error message.BEGIN + -- Validate space exists + IF NOT EXISTS (SELECT 1 FROM public."Space" WHERE id = space_id_) THEN + RAISE EXCEPTION 'Space with id % does not exist', space_id_; + END IF; SELECT platform INTO platform_ STRICT FROM public."Space" WHERE id = space_id_;packages/ui/src/lib/supabase/contextFunctions.ts (1)
74-74: Clarify or remove misleading comment.The comment "account created but not connected, try again" is misleading since this error would indicate the RPC call itself failed, not a partial success.
- if (result.error) throw Error(result.error.message); // account created but not connected, try again + if (result.error) throw Error(result.error.message);packages/database/features/step_definitions/stepdefs.ts (2)
13-22: Add runtime validation for required environment variables.The non-null assertions on environment variables could cause runtime errors if they're not set. Consider adding validation to ensure a clearer error message.
const getAnonymousClient = () => + { + if (!process.env.SUPABASE_URL || !process.env.SUPABASE_ANON_KEY) { + throw new Error('Missing required environment variables: SUPABASE_URL and SUPABASE_ANON_KEY'); + } + return createClient<Database, "public", Database["public"]>( + process.env.SUPABASE_URL, + process.env.SUPABASE_ANON_KEY, + ); + } - createClient<Database, "public", Database["public"]>( - process.env.SUPABASE_URL!, - process.env.SUPABASE_ANON_KEY!, - ); const getServiceClient = () => + { + if (!process.env.SUPABASE_URL || !process.env.SUPABASE_SERVICE_ROLE_KEY) { + throw new Error('Missing required environment variables: SUPABASE_URL and SUPABASE_SERVICE_ROLE_KEY'); + } + return createClient<Database, "public", Database["public"]>( + process.env.SUPABASE_URL, + process.env.SUPABASE_SERVICE_ROLE_KEY, + ); + } - createClient<Database, "public", Database["public"]>( - process.env.SUPABASE_URL!, - process.env.SUPABASE_SERVICE_ROLE_KEY!, - );
96-119: Consider making the platform configurable.The step hardcodes "Roam" as the platform. For better test coverage, consider parameterizing it to support testing with different platforms.
When( - "the user {word} opens the roam plugin in space {word}", - async (userAccountId, spaceName) => { + "the user {word} opens the {word} plugin in space {word}", + async (userAccountId, platform, spaceName) => { // assumption: turbo dev is running. TODO: Make into hooks const localRefs: Record<string, any> = world.localRefs || {}; const spaceId = await fetchOrCreateSpaceId({ password: SPACE_ANONYMOUS_PASSWORD, url: `https://roamresearch.com/#/app/${spaceName}`, name: spaceName, - platform: "Roam", + platform: platform as Platform, }); localRefs[spaceName] = spaceId; const userId = await fetchOrCreatePlatformAccount({ - platform: "Roam", + platform: platform as Platform, accountLocalId: userAccountId, name: userAccountId, email: userEmail(userAccountId), spaceId, password: SPACE_ANONYMOUS_PASSWORD, }); localRefs[userAccountId] = userId; world.localRefs = localRefs; }, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
apps/roam/scripts/compile.ts(1 hunks)apps/roam/src/utils/supabaseContext.ts(3 hunks)apps/website/app/api/supabase/space/route.ts(1 hunks)packages/database/cucumber.json(1 hunks)packages/database/features/contentAccess.feature(1 hunks)packages/database/features/getContext.feature(1 hunks)packages/database/features/step_definitions/stepdefs.ts(1 hunks)packages/database/inputTypes.ts(1 hunks)packages/database/package.json(1 hunks)packages/database/supabase/migrations/20250710185603_create_account_in_space.sql(1 hunks)packages/database/supabase/schemas/account.sql(1 hunks)packages/database/tsconfig.json(1 hunks)packages/database/types.gen.ts(4 hunks)packages/ui/src/lib/supabase/contextFunctions.ts(1 hunks)packages/ui/src/lib/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/database/inputTypes.ts
- packages/database/tsconfig.json
- apps/roam/scripts/compile.ts
- packages/database/cucumber.json
- packages/ui/src/lib/utils.ts
- packages/database/package.json
- apps/roam/src/utils/supabaseContext.ts
- packages/database/features/getContext.feature
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#272
File: packages/ui/src/lib/supabase/contextFunctions.ts:50-111
Timestamp: 2025-07-08T14:48:37.989Z
Learning: The Supabase client does not offer transaction support, so idempotent upserts with proper conflict resolution (using onConflict with ignoreDuplicates: false) are the preferred approach for multi-step database operations in packages/ui/src/lib/supabase/contextFunctions.ts. This pattern prevents orphaned records when retrying operations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#198
File: packages/database/supabase/migrations/20250605083319_alpha_upload.sql:157-337
Timestamp: 2025-06-09T16:57:14.681Z
Learning: Migration files in packages/database/supabase/migrations/ are historical snapshots that preserve database schema and functions as they existed at the time of creation. These files should not be updated to reflect current schema changes, even if they reference tables or columns that have since been modified or removed. Schema incompatibilities in migration files are expected and acceptable as they represent the valid state at the time of migration.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250513173724_content_concept_key.sql:37-60
Timestamp: 2025-05-22T23:50:23.771Z
Learning: For the discourse-graph project, database schema management focuses on the final state in the supabase/schemas directory, not on the individual migration files. When reviewing database changes, consider only the schema definitions in this directory, not potential duplications or conflicts across migration files.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250512142307_sync_table.sql:20-21
Timestamp: 2025-05-20T03:04:21.602Z
Learning: Database migration files (like the SQL files in packages/database/supabase/migrations/) should not be modified after they're created, even to fix minor issues like unused variables. Issues should be addressed in newer schema files instead to maintain migration history integrity.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/schemas/extensions.sql:1-11
Timestamp: 2025-05-20T03:02:36.027Z
Learning: The necessary schemas for Supabase extensions (like 'extensions', 'graphql', and 'vault') are automatically created by Supabase before extensions are installed, so they don't need to be explicitly created in SQL migration files.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250504195841_remote_schema.sql:1-7
Timestamp: 2025-05-20T03:01:11.220Z
Learning: Supabase automatically creates necessary schemas like the "extensions" schema before migrations run, so there's no need to explicitly create these schemas in migration files.
packages/database/features/contentAccess.feature (1)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/supabase/migrations/20250603144146_account_centric.sql:123-136
Timestamp: 2025-06-04T11:41:08.589Z
Learning: Currently, there are no Obsidian spaces in the database - all existing spaces are Roam spaces. This means database migrations targeting space platform assignments only need to handle Roam for now.
packages/database/supabase/schemas/account.sql (4)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#198
File: packages/database/supabase/migrations/20250605083319_alpha_upload.sql:157-337
Timestamp: 2025-06-09T16:57:14.681Z
Learning: Migration files in packages/database/supabase/migrations/ are historical snapshots that preserve database schema and functions as they existed at the time of creation. These files should not be updated to reflect current schema changes, even if they reference tables or columns that have since been modified or removed. Schema incompatibilities in migration files are expected and acceptable as they represent the valid state at the time of migration.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/schemas/extensions.sql:1-11
Timestamp: 2025-05-20T03:02:36.027Z
Learning: The necessary schemas for Supabase extensions (like 'extensions', 'graphql', and 'vault') are automatically created by Supabase before extensions are installed, so they don't need to be explicitly created in SQL migration files.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250512142307_sync_table.sql:20-21
Timestamp: 2025-05-20T03:04:21.602Z
Learning: Database migration files (like the SQL files in packages/database/supabase/migrations/) should not be modified after they're created, even to fix minor issues like unused variables. Issues should be addressed in newer schema files instead to maintain migration history integrity.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250504195841_remote_schema.sql:1-7
Timestamp: 2025-05-20T03:01:11.220Z
Learning: Supabase automatically creates necessary schemas like the "extensions" schema before migrations run, so there's no need to explicitly create these schemas in migration files.
packages/database/supabase/migrations/20250710185603_create_account_in_space.sql (4)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#198
File: packages/database/supabase/migrations/20250605083319_alpha_upload.sql:157-337
Timestamp: 2025-06-09T16:57:14.681Z
Learning: Migration files in packages/database/supabase/migrations/ are historical snapshots that preserve database schema and functions as they existed at the time of creation. These files should not be updated to reflect current schema changes, even if they reference tables or columns that have since been modified or removed. Schema incompatibilities in migration files are expected and acceptable as they represent the valid state at the time of migration.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250512142307_sync_table.sql:20-21
Timestamp: 2025-05-20T03:04:21.602Z
Learning: Database migration files (like the SQL files in packages/database/supabase/migrations/) should not be modified after they're created, even to fix minor issues like unused variables. Issues should be addressed in newer schema files instead to maintain migration history integrity.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/schemas/extensions.sql:1-11
Timestamp: 2025-05-20T03:02:36.027Z
Learning: The necessary schemas for Supabase extensions (like 'extensions', 'graphql', and 'vault') are automatically created by Supabase before extensions are installed, so they don't need to be explicitly created in SQL migration files.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250504195841_remote_schema.sql:1-7
Timestamp: 2025-05-20T03:01:11.220Z
Learning: Supabase automatically creates necessary schemas like the "extensions" schema before migrations run, so there's no need to explicitly create these schemas in migration files.
packages/ui/src/lib/supabase/contextFunctions.ts (3)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#272
File: packages/ui/src/lib/supabase/contextFunctions.ts:50-111
Timestamp: 2025-07-08T14:48:37.989Z
Learning: The Supabase client does not offer transaction support, so idempotent upserts with proper conflict resolution (using onConflict with ignoreDuplicates: false) are the preferred approach for multi-step database operations in packages/ui/src/lib/supabase/contextFunctions.ts. This pattern prevents orphaned records when retrying operations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/api/supabase/rpc/search-content/route.ts:53-56
Timestamp: 2025-05-30T14:37:30.215Z
Learning: The search-content route (apps/website/app/api/supabase/rpc/search-content/route.ts) is intentionally designed to be platform-agnostic rather than Roam-specific. The generic "Platform" terminology (like subsetPlatformIds) is used because the route will support multiple platforms in the near future, not just Roam.
apps/website/app/api/supabase/space/route.ts (4)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#198
File: apps/website/app/api/supabase/agent_identifier/route.ts:0-0
Timestamp: 2025-06-09T16:54:29.659Z
Learning: In the AgentIdentifier API route (apps/website/app/api/supabase/agent_identifier/route.ts), the trusted field is intentionally optional and should accept undefined values. The validation logic `if (trusted !== undefined && ...)` is designed to allow undefined as expected behavior.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/api/supabase/rpc/search-content/route.ts:53-56
Timestamp: 2025-05-30T14:37:30.215Z
Learning: The search-content route (apps/website/app/api/supabase/rpc/search-content/route.ts) is intentionally designed to be platform-agnostic rather than Roam-specific. The generic "Platform" terminology (like subsetPlatformIds) is used because the route will support multiple platforms in the near future, not just Roam.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#272
File: packages/ui/src/lib/supabase/contextFunctions.ts:50-111
Timestamp: 2025-07-08T14:48:37.989Z
Learning: The Supabase client does not offer transaction support, so idempotent upserts with proper conflict resolution (using onConflict with ignoreDuplicates: false) are the preferred approach for multi-step database operations in packages/ui/src/lib/supabase/contextFunctions.ts. This pattern prevents orphaned records when retrying operations.
packages/database/features/step_definitions/stepdefs.ts (4)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#272
File: packages/ui/src/lib/supabase/contextFunctions.ts:50-111
Timestamp: 2025-07-08T14:48:37.989Z
Learning: The Supabase client does not offer transaction support, so idempotent upserts with proper conflict resolution (using onConflict with ignoreDuplicates: false) are the preferred approach for multi-step database operations in packages/ui/src/lib/supabase/contextFunctions.ts. This pattern prevents orphaned records when retrying operations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/supabase/migrations/20250603144146_account_centric.sql:123-136
Timestamp: 2025-06-04T11:41:08.589Z
Learning: Currently, there are no Obsidian spaces in the database - all existing spaces are Roam spaces. This means database migrations targeting space platform assignments only need to handle Roam for now.
packages/database/types.gen.ts (10)
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-06-30T18:58:32.668Z
Learning: Applies to **/*.{ts,tsx} : Use explicit return types for functions
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/schemas/embedding.sql:66-95
Timestamp: 2025-05-20T14:04:19.632Z
Learning: In the `match_embeddings_for_subset_nodes` SQL function in packages/database/supabase/schemas/embedding.sql, the number of results is implicitly limited by the length of the input array parameter `p_subset_roam_uids` since the function filters content using `WHERE c.source_local_id = ANY(p_subset_roam_uids)`.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/api/supabase/rpc/search-content/route.ts:53-56
Timestamp: 2025-05-30T14:37:30.215Z
Learning: The search-content route (apps/website/app/api/supabase/rpc/search-content/route.ts) is intentionally designed to be platform-agnostic rather than Roam-specific. The generic "Platform" terminology (like subsetPlatformIds) is used because the route will support multiple platforms in the near future, not just Roam.
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-06-30T18:58:53.362Z
Learning: Applies to apps/roam/**/* : Use the roamAlphaApi documentation from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-06-30T18:58:53.362Z
Learning: Applies to apps/roam/**/* : Use Roam Depot/Extension API documentation from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU
Learnt from: sid597
PR: DiscourseGraphs/discourse-graph#232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:18-31
Timestamp: 2025-06-22T10:40:52.752Z
Learning: In apps/roam/src/utils/getAllDiscourseNodesSince.ts, the user confirmed that querying for `?title` with `:node/title` and mapping it to the `text` field in the DiscourseGraphContent type is the correct implementation for retrieving discourse node content from Roam Research, despite it appearing to query page titles rather than block text content.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/supabase/migrations/20250603144146_account_centric.sql:50-63
Timestamp: 2025-06-04T11:41:34.951Z
Learning: In the discourse-graph database, all accounts currently stored are Roam platform accounts, making platform-specific migration logic safe for global operations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/supabase/migrations/20250603144146_account_centric.sql:123-136
Timestamp: 2025-06-04T11:41:08.589Z
Learning: Currently, there are no Obsidian spaces in the database - all existing spaces are Roam spaces. This means database migrations targeting space platform assignments only need to handle Roam for now.
🧬 Code Graph Analysis (2)
packages/ui/src/lib/supabase/contextFunctions.ts (3)
packages/database/types.gen.ts (1)
Enums(901-914)packages/ui/src/lib/utils.ts (2)
nextApiRoot(15-24)spaceAnonUserEmail(10-11)packages/ui/src/lib/supabase/client.ts (1)
DGSupabaseClient(9-13)
packages/database/features/step_definitions/stepdefs.ts (3)
packages/database/types.gen.ts (1)
Database(9-824)packages/ui/src/lib/supabase/contextFunctions.ts (2)
fetchOrCreateSpaceId(12-32)fetchOrCreatePlatformAccount(50-76)packages/ui/src/lib/utils.ts (1)
spaceAnonUserEmail(10-11)
🔇 Additional comments (8)
packages/database/supabase/schemas/account.sql (1)
102-136: Well-implemented idempotent function with proper conflict handling.The function correctly implements idempotent upserts with proper ON CONFLICT clauses, aligning with the learned best practices for Supabase operations without transaction support.
The use of
SECURITY DEFINERis appropriate here as the function needs elevated privileges to manage accounts and access control.packages/database/supabase/migrations/20250710185603_create_account_in_space.sql (1)
1-34: Migration correctly implements the account creation function.The migration file properly creates the
create_account_in_spacefunction with appropriate security settings and idempotent operations.apps/website/app/api/supabase/space/route.ts (2)
21-37: Clean refactoring with proper input validation.The simplified input structure and validation logic is cleaner and more maintainable. Good addition of password length validation.
43-46: Consistent simplification of space creation logic.The removal of account_id handling aligns well with the new architecture where account creation is handled by the
create_account_in_spacestored procedure. This separation of concerns improves modularity.packages/database/features/step_definitions/stepdefs.ts (3)
26-39: Implementation looks good.The database cleanup step correctly uses the service client and proper error assertions. The commented Space deletion is noted as a known issue to be addressed in another PR.
53-92: Well-designed test data management system.The implementation elegantly handles relational test data with local references, allowing tests to define relationships between records without knowing database-generated IDs. The use of @ prefix for primary keys and _ prefix for foreign key references is intuitive.
144-144: Use the space's platform instead of hardcoding "Roam".The anonymous user email generation hardcodes "Roam" as the platform, which may not match the actual platform of the space being tested.
Consider retrieving the platform from the space data:
Then( "a user logged in space {word} should see a {word} in the database", async (spaceName, tableName) => { const client = getAnonymousClient(); const spaceId = world.localRefs[spaceName]; + // TODO: Retrieve platform from space data instead of hardcoding + const serviceClient = getServiceClient(); + const { data: space } = await serviceClient + .from("Space") + .select("platform") + .eq("id", spaceId) + .single(); const loginResponse = await client.auth.signInWithPassword({ - email: spaceAnonUserEmail("Roam", spaceId), + email: spaceAnonUserEmail(space?.platform || "Roam", spaceId), password: SPACE_ANONYMOUS_PASSWORD, });Also applies to: 161-161
packages/database/types.gen.ts (1)
588-598: New RPC function properly typed.The
create_account_in_spacefunction signature correctly reflects the database stored procedure for managing platform accounts within spaces.
Comment on lines
18
to
29
| const response = await fetch(base_url + "/space", { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify(data), | ||
| }); | ||
| if (!response.ok) | ||
| throw new Error( | ||
| `Platform API failed: ${response.status} ${response.statusText} ${await response.text()}`, | ||
| ); | ||
| const space = await response.json(); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timeout to prevent hanging requests.
The fetch request has no timeout, which could cause the application to hang indefinitely if the server is unresponsive.
- const response = await fetch(base_url + "/space", {
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 30000); // 30 second timeout
+
+ try {
+ const response = await fetch(base_url + "/space", {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify(data),
+ signal: controller.signal,
});
+ } finally {
+ clearTimeout(timeoutId);
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/ui/src/lib/supabase/contextFunctions.ts around lines 18 to 29, the
fetch request lacks a timeout, risking indefinite hanging if the server is
unresponsive. To fix this, implement a timeout mechanism using AbortController
to abort the fetch request if it exceeds a reasonable time limit. Create an
AbortController instance, pass its signal to fetch, and set a timer to call
abort after the timeout duration. Handle the abort error appropriately to
prevent hanging.
Collaborator
Author
There was a problem hiding this comment.
This is probably useful, but we are not yet doing this in the codebase in general. Punting for now, we'll discuss it, do not take this as a learning.
Contributor
There was a problem hiding this comment.
@maparent Understood! I acknowledge your decision to defer the timeout implementation. I won't record this as a learning and will respect that this isn't a current pattern in your codebase. Feel free to revisit this when it aligns with your team's priorities.
🧠 Learnings used
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#272
File: packages/ui/src/lib/supabase/contextFunctions.ts:50-111
Timestamp: 2025-07-08T14:48:37.989Z
Learning: The Supabase client does not offer transaction support, so idempotent upserts with proper conflict resolution (using onConflict with ignoreDuplicates: false) are the preferred approach for multi-step database operations in packages/ui/src/lib/supabase/contextFunctions.ts. This pattern prevents orphaned records when retrying operations.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#182
File: apps/website/app/utils/supabase/dbUtils.ts:22-28
Timestamp: 2025-05-30T14:49:24.016Z
Learning: In apps/website/app/utils/supabase/dbUtils.ts, expanding the KNOWN_EMBEDDINGS and DEFAULT_DIMENSIONS mappings to support additional embedding models requires corresponding database model changes (creating new embedding tables), which should be scoped as separate work from API route implementations.
ee9687e to
653ee04
Compare
maparent
changed the title
maparent
force-pushed
the
ENG-529-minimal-backend-testing
branch
2 times, most recently
from
1708d89 to
61dd57c
Compare
Collaborator
Author
|
@CodeRabbit: Curious what gave you the idea that the gherkin code needed a delimiter. |
maparent
force-pushed
the
ENG-567-refactor-getContext
branch
from
43bea6a to
1fc1fb4
Compare
maparent
force-pushed
the
ENG-529-minimal-backend-testing
branch
from
c421ebe to
9e3df1e
Compare
maparent
force-pushed
the
ENG-567-refactor-getContext
branch
from
1fc1fb4 to
40f1d66
Compare
maparent
force-pushed
the
ENG-529-minimal-backend-testing
branch
from
9e3df1e to
2f9871f
Compare
maparent
force-pushed
the
ENG-529-minimal-backend-testing
branch
2 times, most recently
from
1cf51da to
2001bdd
Compare
Collaborator
Author
|
Re |
Contributor
|
If they aren't going to be part of CI/CD, why not add them to |
Collaborator
Author
|
Because
|
Contributor
|
re: |
Collaborator
Author
|
Ah, good point on |
Contributor
|
If it is intended for other's to use, instructions on how to use it should be included in this PR. If they are explicitly kept out, that should also be mentioned in the body of the PR, with a link to the task in which it is intended to be completed. |
Collaborator
Author
|
On second thought... Will turbo pick up the |
Collaborator
Author
Contributor
Collaborator
Author
|
added to README |
mdroidian
approved these changes
Jul 14, 2025
Factor out some of the functions that call next from roam to ui code. Also work on a generic way to know the Next endpoint.
maparent
force-pushed
the
ENG-529-minimal-backend-testing
branch
from
d73232d to
7b0a113
Compare
maparent
force-pushed
the
ENG-529-minimal-backend-testing
branch
from
7b0a113 to
87a6a83
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Install cucumber, write a first test.
(This showed a flaw in the function definition, which I'll fix in 424)
Factor out some of the functions that call next from roam to ui code.
Added a variable so I could vary the NEXT API endpoint independently of NODE_ENV.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Refactor