fix: correct property PINs, addresses, and add missing properties#54
Merged
chitcommit merged 1 commit intomainfrom Mar 28, 2026
Merged
fix: correct property PINs, addresses, and add missing properties#54chitcommit merged 1 commit intomainfrom
chitcommit merged 1 commit intomainfrom
Conversation
Corrective migration for data errors in 0004 and 0007: - Fix Addison PIN: 14-21-307-032-1006 → 14-21-111-008-1006 - Fix Surf 504 PIN: 14-28-200-011-1042 → 14-28-122-017-1091 - Fix Surf 504 address: 550 → 559 W Surf St (per deed) - Remove wrong properties (Park Forest, Clarendon Hills) - Add correct: Surf 211 (550 W Surf #C211), Clarendon (4343 N Clarendon #1610), Medellín - Update related obligation and account metadata Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
📝 WalkthroughWalkthroughA SQL migration corrects property data by fixing tax PIN values, updating property metadata, deleting erroneous records, inserting corrected condo properties, and amending account and obligation metadata in the database. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
|
To use Codex here, create a Codex account and connect to github. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
migrations/0014_fix_properties.sql (2)
78-81: Metadata replacement may discard existing fields.Using
SET metadata = '{...}'::jsonbreplaces the entire metadata object, discarding any existing fields. Other updates in this migration (lines 86, 94, 98, 102) usemetadata || '{...}'::jsonbto merge instead.If intentional (e.g., the existing metadata was entirely wrong), this is fine. Otherwise, consider using merge:
Use merge operator to preserve existing metadata
UPDATE cc_accounts - SET metadata = '{"property": "559 W Surf St `#C504`"}'::jsonb + SET metadata = metadata || '{"property": "559 W Surf St `#C504`"}'::jsonb WHERE source = 'mr_cooper' AND account_name = '550 W Surf Mortgage';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@migrations/0014_fix_properties.sql` around lines 78 - 81, The UPDATE for cc_accounts where source = 'mr_cooper' AND account_name = '550 W Surf Mortgage' currently replaces the entire metadata JSON; change it to merge the new property into existing metadata (use the jsonb merge operator as used elsewhere in this migration) so existing fields are preserved—for consistency, update the SET clause for that UPDATE to merge rather than assign, or if full replacement was intentional, add a clear comment explaining why.
89-91: Same metadata replacement pattern as noted above.Lines 90 and 106 use full replacement (
=) while adjacent updates (lines 86, 94, 98, 102) use merge (||). If intentional, consider adding a comment explaining why these specific obligations need full metadata replacement.Use merge for consistency (if appropriate)
UPDATE cc_obligations - SET metadata = '{"property": "559 W Surf St `#C504`", "pin": "14-28-122-017-1091"}'::jsonb + SET metadata = metadata || '{"property": "559 W Surf St `#C504`", "pin": "14-28-122-017-1091"}'::jsonb WHERE category = 'mortgage' AND payee ILIKE '%Surf%';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@migrations/0014_fix_properties.sql` around lines 89 - 91, The UPDATE on cc_obligations that sets metadata = '{"property": "559 W Surf St `#C504`", "pin": "14-28-122-017-1091"}'::jsonb for category = 'mortgage' AND payee ILIKE '%Surf%' uses full replacement of the metadata column while nearby updates use JSON merge (metadata || ...); either change this statement to use the same merge pattern (metadata = metadata || '{"property": "...", "pin": "..."}'::jsonb) to preserve existing metadata, or add an inline SQL comment above this UPDATE explaining why a full replacement (metadata =) is intentionally required for this specific record to make the intent explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@migrations/0014_fix_properties.sql`:
- Around line 42-52: The HOA payee spelling is inconsistent between
cc_properties inserts (the INSERT for property_name 'Surf 211' sets hoa_payee to
'Commodore/Greenbriar Landmark Condo Association' while another record uses
'Commodore/Greenbrier Landmark Condo Association'); verify the authoritative
spelling and update the hoa_payee value in this INSERT statement (and any other
cc_properties rows) so the hoa_payee field is identical across records to avoid
data-quality issues when querying/displaying HOA information.
- Around line 66-74: The INSERT into cc_properties for 'Medellín' is
non-idempotent (tax_pin is NULL so unique constraints won’t prevent duplicates);
make it idempotent by either adding a NOT EXISTS guard around the insert (check
cc_properties for property_name = 'Medellín') or by adding a UNIQUE constraint
on property_name and converting the INSERT into an ON CONFLICT DO NOTHING using
property_name as the conflict target; update the migration to use one of these
approaches so repeated runs do not create duplicate Medellín rows.
---
Nitpick comments:
In `@migrations/0014_fix_properties.sql`:
- Around line 78-81: The UPDATE for cc_accounts where source = 'mr_cooper' AND
account_name = '550 W Surf Mortgage' currently replaces the entire metadata
JSON; change it to merge the new property into existing metadata (use the jsonb
merge operator as used elsewhere in this migration) so existing fields are
preserved—for consistency, update the SET clause for that UPDATE to merge rather
than assign, or if full replacement was intentional, add a clear comment
explaining why.
- Around line 89-91: The UPDATE on cc_obligations that sets metadata =
'{"property": "559 W Surf St `#C504`", "pin": "14-28-122-017-1091"}'::jsonb for
category = 'mortgage' AND payee ILIKE '%Surf%' uses full replacement of the
metadata column while nearby updates use JSON merge (metadata || ...); either
change this statement to use the same merge pattern (metadata = metadata ||
'{"property": "...", "pin": "..."}'::jsonb) to preserve existing metadata, or
add an inline SQL comment above this UPDATE explaining why a full replacement
(metadata =) is intentionally required for this specific record to make the
intent explicit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d1b47cc-32be-40f1-9440-77fe37dfd1f9
📒 Files selected for processing (1)
migrations/0014_fix_properties.sql
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Changes
Also updates related mortgage account and obligation metadata.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit