Update session lookup for party model

[creatorrr]

User description

Summary

• add migration to normalize session lookup table using party ids
• include down migration for reversal

Testing

• poe check (fails: command not found)

---

PR Type

Enhancement

---

Description

• Normalize session_lookup table to use party_id instead of participant columns

  • Add and populate party_id column, update constraints and indexes
  • Remove participant-based columns, triggers, and types

• Clean up and rename document and file owner tables

  • Drop legacy owner tables and triggers, rename new tables

• Provide full down migration to revert all changes

  • Restore participant-based model and legacy tables

---

Changes walkthrough 📝

Relevant files

Enhancement

000043_session_lookup_party_model.up.sql Normalize session_lookup to party-based model, clean up owner tables memory-store/migrations/000043_session_lookup_party_model.up.sql Adds migration to normalize session_lookup using party_id Populates new column from users/agents, updates constraints and indexes Removes participant columns, triggers, types, and legacy owner tables Renames and cleans up file owner tables +48/-0    000043_session_lookup_party_model.down.sql Down migration to restore participant-based session_lookup memory-store/migrations/000043_session_lookup_party_model.down.sql Adds down migration to revert to participant-based model Restores participant columns, types, and legacy owner tables Recreates indexes and migrates data back Drops party-based columns and structures +55/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

---

Important

Add migration to normalize session_lookup using party_id and provide a down migration to revert changes.

• Migration:
  • 000043_session_lookup_party_model.up.sql: Adds party_id to session_lookup, populates it from users and agents, enforces not null, adds foreign key, updates primary key and indexes, removes old triggers and columns, and drops legacy owner tables.
  • 000043_session_lookup_party_model.down.sql: Reverts to participant-based model, recreates participant_type enum, restores columns, updates data from parties, recreates legacy owner tables, and renames temporary tables.

^{This description was created by for e0009d5. You can customize this summary. It will automatically update as commits are pushed.}

[qodo-merge-for-open-source]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Data Loss Risk The down migration updates session_lookup with data from parties table but doesn't handle the case where a party might not exist anymore, potentially causing data loss during rollback. UPDATE session_lookup sl SET participant_type = p.party_type, participant_id = p.party_id FROM parties p WHERE sl.party_id = p.party_id AND sl.developer_id = p.developer_id; Migration Ordering The migration drops constraints and triggers before ensuring all data is properly migrated, which could lead to integrity issues if the migration is interrupted. DROP TRIGGER IF EXISTS trg_validate_participant_before_insert ON session_lookup; DROP TRIGGER IF EXISTS trg_validate_participant_before_update ON session_lookup; DROP FUNCTION IF EXISTS validate_participant(); ALTER TABLE session_lookup DROP COLUMN IF EXISTS participant_type; ALTER TABLE session_lookup DROP COLUMN IF EXISTS participant_id; DROP TYPE IF EXISTS participant_type;

[qodo-merge-for-open-source]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Validate data before constraints The migration attempts to set NOT NULL constraints and create a primary key without first verifying that all rows have valid data. Add a validation step before applying these constraints to prevent migration failure. memory-store/migrations/000043_session_lookup_party_model.down.sql [23-25] +-- Verify data integrity before applying constraints +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM session_lookup WHERE participant_type IS NULL OR participant_id IS NULL) THEN + RAISE EXCEPTION 'Cannot apply NOT NULL constraints - null values exist in participant columns'; + END IF; +END $$; + ALTER TABLE session_lookup ALTER COLUMN participant_type SET NOT NULL; ALTER TABLE session_lookup ALTER COLUMN participant_id SET NOT NULL; ALTER TABLE session_lookup ADD PRIMARY KEY (developer_id, session_id, participant_type, participant_id); Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion introduces a validation step to ensure there are no NULLs in participant_type or participant_id before applying NOT NULL constraints and creating a primary key. This is important for migration robustness, preventing failures or partial migrations, and thus deserves a high score.	Medium
	Handle NULL values properly The migration is missing a check for NULL party_id values in the session_lookup table, which could lead to data loss. Add a WHERE condition to ensure only rows with non-NULL party_id values are processed. memory-store/migrations/000043_session_lookup_party_model.down.sql [16-21] UPDATE session_lookup sl SET participant_type = p.party_type, participant_id = p.party_id FROM parties p WHERE sl.party_id = p.party_id + AND sl.party_id IS NOT NULL AND sl.developer_id = p.developer_id; Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion adds a check for NULL party_id values in the session_lookup table, which helps prevent data loss or incorrect updates during migration. This is a prudent safety measure, though not strictly critical if upstream logic guarantees non-NULLs, so it merits a moderate-to-high score.	Medium
More

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to e0009d5 in 2 minutes and 45 seconds. Click for details.

• Reviewed 115 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 4 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. memory-store/migrations/000043_session_lookup_party_model.down.sql:38

• Draft comment:
  The INSERT into 'doc_owners' pulls data from 'document_owners'. Ensure the source table 'document_owners' exists in the target schema, or this step may fail.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to ensure that a table exists, which is similar to asking them to double-check something. This violates the rule against asking the author to confirm or ensure things. The comment does not provide a specific code suggestion or ask for a specific test to be written.

2. memory-store/migrations/000043_session_lookup_party_model.up.sql:1

• Draft comment:
  Consider wrapping all migration statements in a transaction block to ensure atomicity and rollback safety in case of errors.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

3. memory-store/migrations/000043_session_lookup_party_model.up.sql:6

• Draft comment:
  Ensure that the join columns (developer_id and user_id/agent_id) are properly indexed to optimize the update queries on large tables.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

4. memory-store/migrations/000043_session_lookup_party_model.up.sql:48

• Draft comment:
  Renaming 'file_owners_party' to 'file_owners' here may conflict with the down migration logic that expects 'file_owners_party'. Ensure consistent table naming for easy reversibility.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a speculative comment about what might happen in the down migration. Without seeing the down migration file, we can't verify if this is actually an issue. The comment is asking the author to "ensure" something, which violates our rules. It's making assumptions about migration reversibility requirements that may not be valid. The comment could be raising a legitimate concern about migration reversibility. Database migrations often need careful handling of renames. While migration reversibility is important, this comment is speculative without seeing the down migration, and asks for verification rather than pointing out a concrete issue. Delete this comment as it's speculative, asks for verification, and requires context we don't have to validate the concern.

Workflow ID: wflow_QizU0qcSfCSUu1zU

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Down migration renames file_owners_party to file_owners_tmp, but up migration already renames it to file_owners. This mismatch can cause the down script to fail.

Suggested change

	ALTER TABLE file_owners_party RENAME TO file_owners_tmp;
	ALTER TABLE file_owners RENAME TO file_owners_tmp;