Roam: Eng-450 remove unused db functions #315
Merged
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
|
Updates to Preview Branch (eng-450-remove-unused-db-functions) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThis change removes multiple legacy PostgreSQL functions and their associated type declarations related to discourse node upsert, synchronization, and embedding similarity search from the database schema, migration scripts, and generated TypeScript types. The schema file containing these functions is deleted, and references to it are removed from configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Database
Note over Client,Database: (Old flow - now removed)
Client->>Database: Call upsert_discourse_nodes / alpha_upsert_discourse_nodes
Database-->>Client: Upsert nodes, return results
Client->>Database: Call get_nodes_needing_sync
Database-->>Client: Return nodes needing sync
Client->>Database: Call match_content_embeddings
Database-->>Client: Return similar content
Note over Client,Database: (All above functions are now removed)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/database/supabase/schemas/sync.sql (1)
222-222: Schema file now dropsget_nodes_needing_sync– keep schema & migrations in sync.Good to see the schema reflecting the drop. Just make sure the corresponding migration (
20250803100134_drop-get_nodes_needing_sync.sql) is always applied after this schema file during fresh-schema bootstrap so aCREATE …; DROP …;ordering conflict cannot occur for new environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/database/supabase/config.toml(0 hunks)packages/database/supabase/migrations/20250803092544_drop-unused-functions.sql(1 hunks)packages/database/supabase/migrations/20250803100134_drop-get_nodes_needing_sync.sql(1 hunks)packages/database/supabase/schemas/embedding.sql(1 hunks)packages/database/supabase/schemas/sync.sql(1 hunks)packages/database/supabase/schemas/upload_temp.sql(0 hunks)packages/database/types.gen.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/database/supabase/config.toml
- packages/database/supabase/schemas/upload_temp.sql
🧰 Additional context used
🧠 Learnings (22)
📓 Common learnings
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#0
File: :0-0
Timestamp: 2025-07-13T16:47:14.352Z
Learning: When reviewing SQL functions in the discourse-graph codebase, be mindful that suggesting additional explicit error handling may introduce unnecessary database queries. Functions that use appropriate ON CONFLICT clauses may already handle errors sufficiently without needing extra validation queries.
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#0
File: :0-0
Timestamp: 2025-07-13T16:47:14.352Z
Learning: In the discourse-graph codebase, database functions that use STRICT clauses and ON CONFLICT statements already have appropriate error handling. Suggesting additional explicit error handling may be unnecessary and could introduce performance overhead through extra database queries.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#0
File: :0-0
Timestamp: 2025-07-13T16:47:14.352Z
Learning: In the discourse-graph codebase, types.gen.ts contains automatically generated database function type definitions that may have reordered signatures between versions. This reordering is expected behavior from the code generation process and should not be flagged as an issue requiring fixes.
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250504202930_content_tables.sql:37-45
Timestamp: 2025-05-20T03:11:07.917Z
Learning: When reviewing SQL migrations in the discourse-graph codebase, understand that historical migration files should not be modified once they're applied. Instead, issues should be fixed in subsequent migrations or schema definition files. This is why maparent indicated "migration, not changing. corrected in schema."
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#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/20250504202930_content_tables.sql:37-45
Timestamp: 2025-05-20T03:11:07.917Z
Learning: When reviewing SQL migrations in this codebase, understand that historical migration files should not be modified once they're applied. Instead, issues should be fixed in subsequent migrations or schema definition files. This is why maparent indicated "migration, not changing. corrected in schema."
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: maparent
PR: DiscourseGraphs/discourse-graph#291
File: packages/database/supabase/functions/create-space/index.ts:0-0
Timestamp: 2025-07-21T14:22:20.752Z
Learning: In the discourse-graph codebase, types.gen.ts is not accessible from Supabase edge functions, requiring duplication of types and utilities when needed in the edge function environment at packages/database/supabase/functions/.
📚 Learning: migration files in packages/database/supabase/migrations/ are historical snapshots that preserve dat...
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.
Applied to files:
packages/database/supabase/migrations/20250803100134_drop-get_nodes_needing_sync.sqlpackages/database/supabase/schemas/sync.sqlpackages/database/supabase/schemas/embedding.sqlpackages/database/supabase/migrations/20250803092544_drop-unused-functions.sql
📚 Learning: for the discourse-graph project, database schema management focuses on the final state in the supaba...
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.
Applied to files:
packages/database/supabase/migrations/20250803100134_drop-get_nodes_needing_sync.sqlpackages/database/supabase/schemas/sync.sqlpackages/database/supabase/migrations/20250803092544_drop-unused-functions.sql
📚 Learning: database migration files (like the sql files in packages/database/supabase/migrations/) should not b...
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.
Applied to files:
packages/database/supabase/migrations/20250803100134_drop-get_nodes_needing_sync.sqlpackages/database/supabase/schemas/sync.sqlpackages/database/supabase/migrations/20250803092544_drop-unused-functions.sql
📚 Learning: supabase automatically creates necessary schemas like the "extensions" schema before migrations run,...
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.
Applied to files:
packages/database/supabase/migrations/20250803100134_drop-get_nodes_needing_sync.sqlpackages/database/supabase/schemas/sync.sqlpackages/database/supabase/migrations/20250803092544_drop-unused-functions.sql
📚 Learning: the necessary schemas for supabase extensions (like 'extensions', 'graphql', and 'vault') are automa...
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.
Applied to files:
packages/database/supabase/migrations/20250803100134_drop-get_nodes_needing_sync.sqlpackages/database/supabase/migrations/20250803092544_drop-unused-functions.sql
📚 Learning: in the `match_embeddings_for_subset_nodes` sql function in packages/database/supabase/schemas/embedd...
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)`.
Applied to files:
packages/database/supabase/schemas/sync.sqlpackages/database/supabase/schemas/embedding.sqlpackages/database/types.gen.ts
📚 Learning: when reviewing sql migrations in this codebase, understand that historical migration files should no...
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#165
File: packages/database/supabase/migrations/20250504202930_content_tables.sql:37-45
Timestamp: 2025-05-20T03:11:07.917Z
Learning: When reviewing SQL migrations in this codebase, understand that historical migration files should not be modified once they're applied. Instead, issues should be fixed in subsequent migrations or schema definition files. This is why maparent indicated "migration, not changing. corrected in schema."
Applied to files:
packages/database/supabase/schemas/sync.sqlpackages/database/supabase/migrations/20250803092544_drop-unused-functions.sql
📚 Learning: in apps/website/app/utils/supabase/dbutils.ts, expanding the known_embeddings and default_dimensions...
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.
Applied to files:
packages/database/supabase/schemas/embedding.sqlpackages/database/types.gen.tspackages/database/supabase/migrations/20250803092544_drop-unused-functions.sql
📚 Learning: when reviewing sql functions in the discourse-graph codebase, be mindful that suggesting additional ...
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#0
File: :0-0
Timestamp: 2025-07-13T16:47:14.352Z
Learning: When reviewing SQL functions in the discourse-graph codebase, be mindful that suggesting additional explicit error handling may introduce unnecessary database queries. Functions that use appropriate ON CONFLICT clauses may already handle errors sufficiently without needing extra validation queries.
Applied to files:
packages/database/supabase/schemas/embedding.sqlpackages/database/supabase/migrations/20250803092544_drop-unused-functions.sql
📚 Learning: the search-content route (apps/website/app/api/supabase/rpc/search-content/route.ts) is intentionall...
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.
Applied to files:
packages/database/supabase/schemas/embedding.sqlpackages/database/types.gen.ts
📚 Learning: in the discourse-graph codebase, types.gen.ts contains automatically generated database function typ...
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#0
File: :0-0
Timestamp: 2025-07-13T16:47:14.352Z
Learning: In the discourse-graph codebase, types.gen.ts contains automatically generated database function type definitions that may have reordered signatures between versions. This reordering is expected behavior from the code generation process and should not be flagged as an issue requiring fixes.
Applied to files:
packages/database/types.gen.ts
📚 Learning: in the discourse-graph codebase, types.gen.ts is not accessible from supabase edge functions, requir...
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#291
File: packages/database/supabase/functions/create-space/index.ts:0-0
Timestamp: 2025-07-21T14:22:20.752Z
Learning: In the discourse-graph codebase, types.gen.ts is not accessible from Supabase edge functions, requiring duplication of types and utilities when needed in the edge function environment at packages/database/supabase/functions/.
Applied to files:
packages/database/types.gen.ts
📚 Learning: applies to **/*.{ts,tsx} : use explicit return types for functions...
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-07-19T22:34:08.725Z
Learning: Applies to **/*.{ts,tsx} : Use explicit return types for functions
Applied to files:
packages/database/types.gen.ts
📚 Learning: the team prefers not to manually edit generated files (like supabase typescript types) even when the...
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.
Applied to files:
packages/database/types.gen.ts
📚 Learning: the packages/database directory in the discourse-graph repository has a unique structure as a databa...
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.
Applied to files:
packages/database/types.gen.tspackages/database/supabase/migrations/20250803092544_drop-unused-functions.sql
📚 Learning: currently, there are no obsidian spaces in the database - all existing spaces are roam spaces. this ...
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.
Applied to files:
packages/database/types.gen.ts
📚 Learning: applies to apps/roam/**/*.{js,jsx,ts,tsx} : use the roamalphaapi documentation from https://roamrese...
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use the roamAlphaApi documentation from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when working with the Roam API
Applied to files:
packages/database/types.gen.ts
📚 Learning: in apps/roam/src/utils/getalldiscoursenodessince.ts, the user confirmed that querying for `?title` w...
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.
Applied to files:
packages/database/types.gen.ts
📚 Learning: in the discourse-graph project's upsert_content function, null creator_id values are tolerated as an...
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.
Applied to files:
packages/database/types.gen.ts
📚 Learning: in the discoursenode interface from apps/roam/src/utils/getdiscoursenodes.ts, the field `type` serve...
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#220
File: apps/roam/src/utils/conceptConversion.ts:42-56
Timestamp: 2025-06-17T23:37:45.289Z
Learning: In the DiscourseNode interface from apps/roam/src/utils/getDiscourseNodes.ts, the field `type` serves as the unique identifier field, not a type classification field. The interface has no `uid` or `id` field, making `node.type` the correct field to use for UID-related operations.
Applied to files:
packages/database/types.gen.ts
📚 Learning: in the discoursenode interface from apps/roam/src/utils/getdiscoursenodes.ts, the field `node.type` ...
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#220
File: apps/roam/src/utils/conceptConversion.ts:42-56
Timestamp: 2025-06-17T23:37:45.289Z
Learning: In the DiscourseNode interface from apps/roam/src/utils/getDiscourseNodes.ts, the field `node.type` serves as the UID field rather than having a conventional `node.uid` field. This is an unusual naming convention where the type field actually contains the unique identifier.
Applied to files:
packages/database/types.gen.ts
⏰ 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: Supabase Preview
🔇 Additional comments (4)
packages/database/supabase/schemas/embedding.sql (1)
75-77: Update migrations before droppingmatch_content_embeddingsThe function is still being re-created by two existing migrations, so simply dropping it in the schema will not fully remove it and may lead to unexpected behavior when replaying or reverting migrations.
• packages/database/supabase/migrations/20250513173724_content_concept_key.sql:43 –
CREATE OR REPLACE FUNCTION public.match_content_embeddings(...)
• packages/database/supabase/migrations/20250520132747_restrict_search_by_document.sql:1 –CREATE OR REPLACE FUNCTION public.match_content_embeddings(...)Please remove or adjust these migration definitions (or consolidate them) to fully deprecate the function before running the DROP in your schema.
⛔ Skipped due to 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#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/20250504202930_content_tables.sql:37-45 Timestamp: 2025-05-20T03:11:07.917Z Learning: When reviewing SQL migrations in this codebase, understand that historical migration files should not be modified once they're applied. Instead, issues should be fixed in subsequent migrations or schema definition files. This is why maparent indicated "migration, not changing. corrected in schema."Learnt from: maparent PR: DiscourseGraphs/discourse-graph#165 File: packages/database/supabase/migrations/20250504202930_content_tables.sql:37-45 Timestamp: 2025-05-20T03:11:07.917Z Learning: When reviewing SQL migrations in the discourse-graph codebase, understand that historical migration files should not be modified once they're applied. Instead, issues should be fixed in subsequent migrations or schema definition files. This is why maparent indicated "migration, not changing. corrected in schema."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#0 File: :0-0 Timestamp: 2025-07-13T16:47:14.352Z Learning: When reviewing SQL functions in the discourse-graph codebase, be mindful that suggesting additional explicit error handling may introduce unnecessary database queries. Functions that use appropriate ON CONFLICT clauses may already handle errors sufficiently without needing extra validation queries.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#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/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#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.packages/database/supabase/migrations/20250803100134_drop-get_nodes_needing_sync.sql (1)
1-2: No lingering callers ofget_nodes_needing_syncfound
All references to this function are confined to SQL migration and schema files. A repo-wide search excluding.sqlfiles returned zero hits in TypeScript RPC wrappers, cron jobs, or Supabase edge functions:
- packages/database/supabase/migrations/20250520133551_nodes_needing_sync.sql
- packages/database/supabase/schemas/sync.sql
It’s safe to drop this function in the new migration.
packages/database/types.gen.ts (1)
637-641: Generated file – field order change only. No action required.packages/database/supabase/migrations/20250803092544_drop-unused-functions.sql (1)
1-10: No references to dropped functions remain
A full‐repo search for both function calls and string literals confirmed that none of the following are used outside of the drop migration:
- alpha_delete_by_source_local_ids
- alpha_get_last_update_time
- alpha_upsert_discourse_nodes
- match_content_embeddings
- upsert_discourse_nodes
Safe to merge.
maparent
reviewed
Aug 4, 2025
maparent
reviewed
Aug 4, 2025
packages/database/supabase/migrations/20250803100134_drop-get_nodes_needing_sync.sql
Outdated
Show resolved
Hide resolved
maparent
reviewed
Aug 4, 2025
|
|
||
| set search_path to public, extensions ; | ||
|
|
||
| CREATE OR REPLACE FUNCTION public.match_content_embeddings ( |
There was a problem hiding this comment.
I don't know if this function should be removed.
On the one hand you are correct that it is not currently used. OTOH It allows functionality that is not there otherwise, and could be used.
That said... Search architecture will be rethought soon, so it's not like we're going to get to use it in this exact shape.
I guess I'm a -0 on this. Not going to fight it.
There was a problem hiding this comment.
I mis-remembered and thought I created both these functions and now the other one is being used, I don't have any opinions on which one to keep. Since you see a use for this in future I will keep this one and remove the other one
There was a problem hiding this comment.
I would not remove the other one! It fits a real and distinct use case.
I originally had a single function which handled both use cases, but @mdroidian thought it was confusing.
There was a problem hiding this comment.
Ok no embedding function is being removed now.
maparent
reviewed
Aug 4, 2025
packages/database/supabase/migrations/20250803092544_drop-unused-functions.sql
Show resolved
Hide resolved
maparent
requested changes
Aug 4, 2025
|
The failure is due to merging the migrations. Trying to solve this. |
|
For some reason I get errors with supabase when trying to do a |
|
(Or we could ignore the failing check, but that's probably bad practice.) |
|
@maparent @mdroidian build is passing now |
mdroidian
approved these changes
Aug 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary by CodeRabbit
Bug Fixes
Chores
These changes streamline the database and remove unused or obsolete features, improving maintainability. No user-facing features or workflows are affected.