Skip to content

Fix admin deletion cascade and valuation classification - #7

Draft
cursor[bot] wants to merge 2 commits into
mainfrom
cursor/critical-bug-investigation-9619
Draft

Fix admin deletion cascade and valuation classification#7
cursor[bot] wants to merge 2 commits into
mainfrom
cursor/critical-bug-investigation-9619

Conversation

@cursor

@cursor cursor Bot commented Jun 21, 2026

Copy link
Copy Markdown

Bug and impact

  • Admin user deletion relied on database-level ON DELETE CASCADE, but the default SQLite configuration does not enable FK cascades. Deleting a user could leave executions, reports, chats, token usage, API keys, schedules, and other user-owned rows behind.
  • The ETF/index valuation classifier treated ordinary equity names containing broad terms like Trust as ETFs/indexes. REIT-style equities such as Digital Realty Trust could skip DCF/comps and receive aggregate ETF-style valuation output.

Root cause

  • delete_user only deleted the User ORM object and assumed cascades would remove all related rows.
  • _is_index_or_etf used broad substring matching across metadata and names (FUND, TRUST, provider names), causing false positives.

Fix

  • Explicitly delete user-owned dependent rows in dependency order before deleting the user, independent of DB cascade enforcement.
  • Tightened ETF/index detection to exact instrument metadata values and specific fund/ETF name phrases, avoiding broad Trust/Fund false positives.
  • Added regression coverage for SQLite user deletion and Trust-named equity valuation.
  • Aligned an existing valuation bridge assertion with the cent-rounded bridge components returned by the implementation.

Validation

  • python3 -m unittest backend.tests.test_admin_service backend.tests.test_valuation_summary
Open in Web View Automation 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant