Uh oh!
There was an error while loading. Please reload this page.
fix(db): add missing FK constraints on graph_edges + notes (#179, #180) - #245
fix(db): add missing FK constraints on graph_edges + notes (#179, #180)#245Jose-Gael-Cruz-Lopez wants to merge 15 commits into
Conversation
Deploying with |
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs | frontend | 75d3729 | Jun 22 2026, 03:55 AM |
Warning Review limit reached
More reviews will be available in 43 minutes and 55 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds inline ChangesFK Integrity for graph_edges and notes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/db/supabase_schema.sql`:
- Around line 495-496: The comment explaining why note_concepts.concept_node_id
is intentionally not a hard FK states it's because graph_nodes uses TEXT ids
managed by application code, but this reasoning is inconsistent since
graph_edges.source_node_id and graph_edges.target_node_id also reference
graph_nodes(id) with TEXT ids yet have hard FKs defined. Update the comment at
lines 495-496 to clarify the actual rationale for not adding the FK constraint
to note_concepts.concept_node_id, such as whether concepts can exist before
their corresponding graph nodes are created or if there's a specific
application-level design consideration that necessitates this approach.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b17fdcab-4e71-4e4f-83a4-8009fa8e242a
📒 Files selected for processing (3)
backend/db/migration_fk_integrity.sqlbackend/db/supabase_schema.sqlbackend/tests/test_fk_integrity_migration.py
| -- links. Only the note_concepts.concept_node_id link is intentionally NOT a | ||
| -- hard FK, because graph_nodes uses TEXT ids managed by application code. |
There was a problem hiding this comment.
Clarify the rationale for not adding FK to note_concepts.concept_node_id.
The comment states the link is "intentionally NOT a hard FK, because graph_nodes uses TEXT ids managed by application code." However, graph_edges.source_node_id and graph_edges.target_node_id (lines 98-99) both have hard FKs to graph_nodes(id), so the stated reason is inconsistent.
If the real reason is different (e.g., concepts can exist before graph nodes are created, or there's an application-level design consideration), please update the comment to explain the actual rationale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/db/supabase_schema.sql` around lines 495 - 496, The comment
explaining why note_concepts.concept_node_id is intentionally not a hard FK
states it's because graph_nodes uses TEXT ids managed by application code, but
this reasoning is inconsistent since graph_edges.source_node_id and
graph_edges.target_node_id also reference graph_nodes(id) with TEXT ids yet have
hard FKs defined. Update the comment at lines 495-496 to clarify the actual
rationale for not adding the FK constraint to note_concepts.concept_node_id,
such as whether concepts can exist before their corresponding graph nodes are
created or if there's a specific application-level design consideration that
necessitates this approach.
Postgres does not auto-index the referencing side of a foreign key, and sibling tables index this access path. Add the index in both the migration and the canonical schema.
Deploying with |
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs | frontend-staging | 4f8655e | Commit Preview URL Branch Preview URL | Jun 24 2026, 02:50 PM |
main restructured db/ into ordered migrations applied by migrate.py, deleting the flat migration_*.sql files. Move the FK-integrity DDL into migrations/0020_fk_integrity.sql so existing databases get the graph_edges/notes FK constraints. The DDL is already idempotent (pg_constraint-guarded ADD CONSTRAINT, orphan DELETEs first, CREATE INDEX IF NOT EXISTS), so it is also a no-op on fresh DBs that already have the inline FKs from 0001_baseline_schema.
…iles supabase_schema.sql was deleted when main restructured db/. Assert the FK DDL invariants against migrations/0020_fk_integrity.sql and the inline REFERENCES against migrations/0001_baseline_schema.sql instead, preserving the original verification intent (guarded ADD CONSTRAINT, orphan cleanup ordering, inline FKs on fresh DBs). Also assert the idx_graph_edges_user_id index this PR adds.
graph_edges.user_id,notes.user_id, andnotes.course_idwere bareTEXTcolumns with noREFERENCES, inconsistent with every sibling table in the learning schema. An edge/note could be written for a non-existent user, and deleting a course left dangling notes thatlist_notessurfaces but can't resolve.Changes
backend/db/migration_fk_integrity.sql— new hand-applied migration:ALTER TABLEcan validate),DO $$ … IF NOT EXISTS (SELECT 1 FROM pg_constraint …)guard already used inmigration_gradebook.sql(Postgres has noADD CONSTRAINT IF NOT EXISTS), so it is safely re-runnable.backend/db/supabase_schema.sql— inlineREFERENCESon all three columns so fresh databases are born with the FKs; comment block updated to note only thenote_concepts.concept_node_idlink stays soft.backend/tests/test_fk_integrity_migration.py— drift guard: each constraint is guarded, orphans are cleaned before the ALTER, schema declares the inline references.Verification
ruff check .clean; gated suite green (+3 new).Closes#179, #180.
Summary by CodeRabbit
Bug Fixes
Tests