validate computed field configuration on startup

[lsmith77]

Note on the changes to the existing tests:

The type-checking tests contain duplicate configurations:

Outer config (required by createTestClient): Needed to instantiate the actual client. Without it, the new runtime validation throws an error before the test can run.

Inner config (in extraSourceFiles): Example code that demonstrates proper usage and ensures the generated TypeScript file compiles with correct type hints.

This duplication is a consequence of using createTestClient for type-checking. The outer config satisfies the runtime validation, while the inner config serves as realistic documentation of how users should structure their code.

Let me know if you would prefer adding a helper to side-step createTestClient for these cases.

Summary by CodeRabbit

• Bug Fixes

  • ORM client now validates computed-field configurations during initialization and raises clear errors if a computed field is missing or misconfigured.

• New Features

  • Test client options now accept an optional computed-fields configuration to provide computed-field behavior in tests.

• Tests

  • Added end-to-end tests for computed-field validation, error cases (missing or non-function configs), and typed/runtime behaviors across workflows.

[coderabbitai]

📝 Walkthrough

Walkthrough

Added root-client validation that ensures every computed field declared in the schema has a corresponding computedFields configuration and that each configuration is a function; throws descriptive config errors for violations. New e2e tests exercise missing and non-function configurations and workflow scenarios. Validation was implemented twice (duplicate definitions) in the same class.

Changes

Cohort / File(s)	Summary
Client implementation packages/orm/src/client/client-impl.ts	Added private validateComputedFieldsConfig() and call during root client initialization (only when no baseClient); iterates models and their computedFields, throwing errors for missing or non-function entries. Note: the validation method appears twice (duplicate definitions) in the class.
End-to-end tests tests/e2e/orm/client-api/computed-fields.test.ts	Added tests covering missing computed-field configuration, computed-field present in schema but absent from config, non-function configurations (string/object), and various computed-field workflows (non-optional, optional, relation-read, delegation). Adjusted test scaffolding to supply computedFields where required.
Test client options packages/testtools/src/client.ts	Extended ExtraTestClientOptions with optional computedFields?: import('@zenstackhq/orm').ComputedFieldsOptions<any> to allow supplying computed-field configs via test client factory.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibble through schemas, name by name,
If a computed field lacks its guiding flame.
I squeak an error, spot duplicates twice,
Hop back to the root, make the configs precise. ✨

🚥 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 'validate computed field configuration on startup' directly and clearly describes the main change: adding runtime validation of computed field configuration at startup, which matches the primary objective of the PR.
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

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

Adds runtime validation to ensure all @computed fields declared in the schema have corresponding computedFields implementations configured when the ORM client starts.

Changes:

• Add startup-time validation in ClientImpl constructor to verify computed field config completeness.
• Add E2E tests covering missing computed field configuration scenarios.
• Update existing type-checking computed-field tests to include runtime computedFields config required by the new validation.

Reviewed changes

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

File	Description
tests/e2e/orm/client-api/computed-fields.test.ts	Adds new negative tests for missing computedFields config and updates type-checking cases to satisfy new startup validation.
packages/orm/src/client/client-impl.ts	Introduces validateComputedFieldsConfig() and invokes it during client construction to fail fast on misconfiguration.

---

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

tests/e2e/orm/client-api/computed-fields.test.ts (2)

183-226: ⚠️ Potential issue | 🔴 Critical

Pipeline failure: TS2769 at line 184 — the added computedFields option breaks the createTestClient overload resolution.

The CI reports TS2769: No overload matches this call at line 184. The newly added outer computedFields (lines 193–197) doesn't have an as any cast on the options object, unlike the error-handling tests above. Since the schema is passed as a raw string, TypeScript can't infer the correct computedFields shape and the call doesn't match any createTestClient overload.

Add as any to the options object (consistent with lines 110, 243, 326, 359) or type-assert just the computedFields property.

Proposed fix

         {
+            computedFields: {
+                User: {
+                    upperName: (eb: any) => eb.fn('upper', ['name']),
+                },
+            },
             extraSourceFiles: {
                 main: `
-                computedFields: {
-                    User: {
-                        upperName: (eb: any) => eb.fn('upper', ['name']),
-                    },
-                },

The simplest fix is to add as any to the entire options argument, matching the pattern used by the other tests:

         },
-        },
+        } as any,
     );

---

255-297: ⚠️ Potential issue | 🔴 Critical

Same as any issue likely applies here for the optional-fields type-checking test.

Lines 264–269 add the outer computedFields without an as any on the options object. This will likely cause the same TS2769 overload error as line 184. Apply the same fix.

Proposed fix

         },
-        },
+        } as any,
     );

[ymc9]

Hi @lsmith77 , it seems the updated test file is failing type checking.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@packages/testtools/src/client.ts`:
- Around line 107-111: The computedFields type currently references the internal
path import('../../orm/src/client/options').ComputedFieldsOptions<any>; update
this to use the package public API by importing ComputedFieldsOptions from
'@zenstackhq/orm' and changing the field declaration to use that imported symbol
(keep the property name computedFields and generic any as before); locate the
type usage in client.ts (computedFields?: ComputedFieldsOptions<any>;) and
adjust the top-of-file import list to include ComputedFieldsOptions from
'@zenstackhq/orm' instead of the relative internal path.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/orm/src/client/client-impl.ts (1)

392-397: ⚠️ Potential issue | 🟡 Minor

Validation bypass via $setOptions.

$setOptions creates a derived client (baseClient = this), so validateComputedFieldsConfig won't re-run. If a caller passes options with missing/invalid computedFields, the error surfaces at query time rather than at client creation. This is a minor gap since the type system should prevent it in most cases, but worth being aware of.

🧹 Nitpick comments (1)

packages/orm/src/client/client-impl.ts (1)

150-154: Nit: consider using this.$options type narrowing instead of in check + cast.

The 'computedFields' in this.$options guard with a cast to Record<string, any> works, but it bypasses type safety. This is acceptable given the generic nature of ClientOptions<Schema>, but worth noting that if the options type is ever refined to always include computedFields, this cast can be removed.

[lsmith77]

Hi @lsmith77 , it seems the updated test file is failing type checking.

should be fixed now.