fix(orm): use id-only filter for update read-back

[mwillbanks]

• Prevents false cannot-read-back errors when update returning values don’t round-trip cleanly into an equality where filter
• Fixes update read-back to derive the filter from model id fields instead of the full returned row
• Addresses an issue that surfaces when multiple plugins are installed alongside PolicyPlugin (entity-mutation hooks can change mutation returning behavior and trigger policy read-back)
• Adds a regression in the policy update CRUD suite covering JSON array field update with an additional entity-mutation plugin (runs on Postgres; skipped otherwise)

Summary by CodeRabbit

• Bug Fixes
  • Fixed an issue where reading back updated entities would not correctly identify the target record, particularly when using plugins with JSON array fields. The read-back mechanism now reliably uses entity IDs from the update result to fetch the latest data.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request modifies how read-back filters are computed during update operations by extracting ID values from the update result using getIdValues() instead of relying directly on the update result when available. A new test validates this behavior for JSON array field updates with plugins on PostgreSQL.

Changes

Cohort / File(s)	Summary
Update read-back filter logic packages/orm/src/client/crud/operations/update.ts	Changed read-back filter computation to use getIdValues() on the update result when truthy, falling back to args.where otherwise. This affects which entity is fetched after an update.
Read-back regression tests tests/e2e/orm/policy/crud/update.test.ts	Added import for getTestDbProvider and new PostgreSQL-gated test block "Read-back regressions" validating JSON array field updates with mutation plugins and proper data return.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• PR #252: Both PRs modify update/read-back logic to prefer deriving ID objects via getIdValues()/getEntityIds() instead of full entity reads.
• PR #477: Both PRs alter update read-back/return logic and how the target entity filter is determined and loaded.
• PR #6: Both PRs modify read-back behavior after updates, with the main change affecting update result ID extraction and policy handling.

Poem

🐰 A filter refined, now IDs lead the way,
No longer the whole result holds sway,
In JSON fields we dance and play,
PostgreSQL tests ensure we don't stray,
Read-backs precise, hip hip hooray! 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and concisely summarizes the main change: using id-only filters for update read-back operations instead of the full returned row.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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 and usage tips.}

[Copilot]

Pull request overview

This PR fixes false cannot-read-back errors in ORM update operations by using ID-only filters for read-back instead of the full updated row. This prevents issues when update returning values don't cleanly round-trip into equality filters, particularly when multiple plugins (including PolicyPlugin) are installed and affect mutation returning behavior.

Key changes:

• Modified update read-back to derive filter from model ID fields only
• Added regression test for JSON array field updates with mutation plugins on PostgreSQL

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
packages/orm/src/client/crud/operations/update.ts	Changed runUpdate to use getIdValues() for extracting ID fields from update result for read-back filter, consistent with runUpsert and create operations
tests/e2e/orm/policy/crud/update.test.ts	Added PostgreSQL-specific regression test covering JSON array field updates with an entity-mutation plugin to ensure no false cannot-read-back errors occur

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

tests/e2e/orm/policy/crud/update.test.ts (1)

1264-1349: Strong regression test for JSON array read-back behavior.

The test validates that updating a JSON array field with a mutation plugin installed does not trigger a false cannot-read-back error. The PostgreSQL-gating is appropriate (JSON arrays are database-specific), and the test properly includes entity-mutation hooks to reproduce the scenario described in the PR objectives. The cleanup with $disconnect() in the finally block is correct.

Optional refactor: Line 1341 includes rootId: root.id in the update data, but foreign keys typically don't change during updates. Consider omitting it for clarity:

♻️ Minor simplification

                 await expect(
                     authed.jsonArrayField.update({
                         where: { id: created.id },
                         data: {
                             data: updateData,
-                            rootId: root.id,
                         },
                     }),
                 ).resolves.toMatchObject({ data: updateData });

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c4ee20a and 46cb219.

📒 Files selected for processing (2)

• packages/orm/src/client/crud/operations/update.ts
• tests/e2e/orm/policy/crud/update.test.ts

🧰 Additional context used

📓 Path-based instructions (1)

tests/e2e/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

E2E tests should validate real-world schema compatibility with established projects

Files:

• tests/e2e/orm/policy/crud/update.test.ts

🧠 Learnings (4)

📚 Learning: 2025-11-26T01:55:04.540Z

Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests

Applied to files:

• tests/e2e/orm/policy/crud/update.test.ts

📚 Learning: 2025-11-26T01:55:04.540Z

Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects

Applied to files:

• tests/e2e/orm/policy/crud/update.test.ts

📚 Learning: 2025-11-26T01:55:04.540Z

Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization

Applied to files:

• tests/e2e/orm/policy/crud/update.test.ts
• packages/orm/src/client/crud/operations/update.ts

📚 Learning: 2025-12-30T15:07:06.254Z

Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:06.254Z
Learning: Do not use ts-expect-error in production code within the zenstackhq/zenstack-v3 repo (e.g., packages/*). Use explicit type annotations, targeted type assertions, or refactor to resolve the type error. ts-expect-error may be acceptable only in test files for stubbing or temporary silencing. Ensure production code is type-safe and maintainable.

Applied to files:

• packages/orm/src/client/crud/operations/update.ts

🧬 Code graph analysis (1)

packages/orm/src/client/crud/operations/update.ts (1)

packages/orm/src/client/query-utils.ts (1)

• getIdValues (181-187)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Agent
• GitHub Check: build-test (22.x, postgresql)
• GitHub Check: build-test (22.x, sqlite)

🔇 Additional comments (1)

packages/orm/src/client/crud/operations/update.ts (1)

46-46: Excellent fix for read-back filter robustness.

Using getIdValues() to derive the read-back filter from ID fields only (instead of the full updateResult) prevents false cannot-read-back errors when returned values don't round-trip cleanly into equality filters (e.g., JSON arrays, floating-point precision). This approach is consistent with runUpdateManyAndReturn (line 114) and runUpsert (line 163).