Skip to content

validate computed field configuration on startup#653

Open
lsmith77 wants to merge 1 commit intozenstackhq:devfrom
lsmith77:validate-computed-fields
Open

validate computed field configuration on startup#653
lsmith77 wants to merge 1 commit intozenstackhq:devfrom
lsmith77:validate-computed-fields

Conversation

@lsmith77
Copy link
Contributor

@lsmith77 lsmith77 commented Feb 2, 2026

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 root client initialization and raises clear errors if any computed field is missing or misconfigured, preventing silent misconfiguration.
  • Tests

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

Copilot AI review requested due to automatic review settings February 2, 2026 15:55
@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

📝 Walkthrough

Walkthrough

Added root-client validation that every computed field declared in the schema has a corresponding computedFields configuration and that each configuration is a function; throws descriptive config errors when violations are found. New e2e tests cover missing, non-function, and workflow scenarios for computed fields.

Changes

Cohort / File(s) Summary
Computed Field Validation
packages/orm/src/client/client-impl.ts
Added private validateComputedFieldsConfig() and invoke it during root client initialization; iterates models and their computed fields, throwing descriptive errors for missing or non-function computedFields entries.
Validation Tests
tests/e2e/orm/client-api/computed-fields.test.ts
Added tests for missing computed-field configuration, non-function configurations, and various computed-field workflows (non-optional, optional, relation-read, delegation); updated test scaffolding to supply computedFields where needed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hop through schemas, sniff each name,
No stray computed field escapes my game.
I nudge the roots and check each rule,
So configs are tidy — neat and cool. ✨

🚥 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 describes the main change: adding runtime validation of computed field configuration during client initialization.
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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@lsmith77 lsmith77 force-pushed the validate-computed-fields branch 2 times, most recently from feaecb9 to 1212adc Compare February 3, 2026 09:07
@lsmith77 lsmith77 requested a review from ymc9 February 3, 2026 13:59
@lsmith77 lsmith77 force-pushed the validate-computed-fields branch from 1212adc to 5d8b09a Compare February 6, 2026 09:15
@lsmith77 lsmith77 force-pushed the validate-computed-fields branch from 5d8b09a to 5aee416 Compare February 6, 2026 09:16
@lsmith77 lsmith77 requested a review from ymc9 February 6, 2026 09:17
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

ymc9 commented Feb 6, 2026

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

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.

2 participants