[ymc9]

Follow-up to #2872.

Problem

BaseOperationHandler.delete on a regular model with relations to delegate sub models deleted the children with a relation filter ({ [opposite]: where }) before evaluating the parent's own where. A filter that depends on those children then matched nothing after they were gone:

// Thread { notes Note[] }, Note extends Item, Note.thread onDelete: Cascade
await db.thread.deleteMany({ where: { notes: { some: {} } } });
// dev: deletes every Note and its Item base row, returns { count: 0 }, Thread survives

#2872 fixed the same ordering for delegate models by resolving the rows first, but kept the old order for regular models. This was pre-existing on dev before #2872 as well.

Fix

Remove the separate regular-model branch and route every model with delegate cascade relations through the same path: read the matching rows first, cascade into children by id, then delete the parents by id. The discriminator column is only selected when the model has one.

Behavior changes

• A regular model filtered by the rows it cascades to now deletes the right rows (new test).
• deleteMany with limit now works on a regular model with such relations instead of throwing.
• A regular model row that is deletable but hidden by the read policy is no longer deleted when it has such relations, since the rows are now resolved by a read. This was already the documented known limit for delegate models, so the two paths are now consistent. The existing policy test is updated to assert the new behavior.
• Regular models with such relations run the same extra queries delegates do: one read plus batched deletes by id.

Tests

• New deletes non-delegate rows filtered by the related rows it cascades to in delegate-cascade-delete.test.ts. Fails on dev with count: 0, passes here.
• Updated skips rows that can be deleted but not read.
• delegate-cascade-delete, delete, delegate, update, policy/delegate, policy/crud/delete e2e suites pass on SQLite (73 tests). Regression suite passes (268 tests). ORM unit tests pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved cascading deletion so filters and limits are applied before related records are removed.
  • Deleting records based on related data now affects only the matching records.
  • Records hidden by read-access policies are no longer deleted through cascading operations, and their related records remain intact.
  • Improved handling for models without discriminator fields.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: zenstackhq/zenstack/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: a11597b1-b1cf-4268-b7cc-170243c870d8

📥 Commits

Reviewing files that changed from the base of the PR and between 2c81cbd and 983b09d.

📒 Files selected for processing: 2

• packages/orm/src/client/crud/operations/base.ts
• tests/e2e/orm/client-api/delegate-cascade-delete.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Delete now reads rows matching the requested filter and limit, includes discriminator values when present, processes cascades for those rows, and deletes by ID batches. Tests cover filtered deletion and read-policy filtering of cascade targets.

Changes: Cascade Delete

Layer / File(s)	Summary
Resolve rows before cascading packages/orm/src/client/crud/operations/base.ts, tests/e2e/orm/client-api/delegate-cascade-delete.test.ts	The delete operation preloads matching rows before cascade processing and uses their IDs for deletion. Tests cover filtered non-delegate deletion and confirm that read-policy-hidden rows and their notes remain undeleted.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: erikdakoda

Merge Risk: ⚪ Minimal · up to 983b0

The reported partial-delete concern does not apply to the changed cascade path. No actionable merge-blocking risk remains after normal checks.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: resolving rows before delegate cascade processing for regular models.
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}