Skip to content

fix(orm): type parameterized computed field args from the field's params metadata - #2828

Merged
ymc9 merged 3 commits into
zenstackhq:devfrom
evgenovalov:fix/computed-field-args-typing
Sep 9, 2026
Merged

fix(orm): type parameterized computed field args from the field's params metadata#2828
ymc9 merged 3 commits into
zenstackhq:devfrom
evgenovalov:fix/computed-field-args-typing

Conversation

@evgenovalov

@evgenovalov evgenovalov commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Problem

args of a parameterized computed field were typed off the generated computedFields stub, whose non-scalar params (enums, type defs, models, Json) are emitted as unknown. So for

enum Status { ACTIVE INACTIVE }
type ViewFilter { minViews Int }

model User {
  postCountByStatus(status: Status) Int @computed
  popularPostCount(filter: ViewFilter) Int @computed
}

all of these compiled, and the implementation callback saw args.status / args.filter as unknown:

db.user.findMany({ select: { postCountByStatus: { args: { status: 'WRONG' } } } });
db.user.findMany({ where: { postCountByStatus: { args: { status: 'WRONG' }, gt: 0 } } });
const s: UserSelect = { popularPostCount: { args: { filter: { minViews: 'x' } } } };

Runtime zod validation was already precise (it builds the args schema from the field's params), so typing and validation had drifted apart.

Fix

  • orm: ComputedFieldArgs now derives from the field's params metadata, reusing the param mapping procedures already use (MapParamsObject / MapParam, renamed from the procedure-specific names): scalars → TS types, enums → value union, type defs → object shape, optional → optional key. ComputedFieldsOptions reads the same source, so the implementation signature and the query input types can't disagree. This also covers parameterized computed fields inherited by delegate sub-models (their field defs carry params).
  • sdk: the stub parameter is renamed to _args (the bare args tripped noUnusedParameters in consuming projects), and enum params in the stub are emitted as SchemaType["enums"]["X"]["values"][keyof ...] instead of unknown. Type-def params remain unknown in the stub only, since schema.ts has no TS type for type defs; the ORM/input.ts typing resolves them precisely.

Tests

  • e2e computed-fields.test.ts: a runtime test with enum + type-def params (including zod rejection of a wrong enum value and a wrong type-def payload), and a compile-time test asserting wrong args fail in select / where / orderBy, in the implementation callback, and in the generated UserSelect / UserWhereInput types.
  • typing schema: added hasStatus(status: Status) Boolean @computed with @ts-expect-error assertions in typecheck.ts.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Parameterized computed fields now derive query and implementation argument types from declared schema parameters.
    • Generated TypeScript typings support scalar, enum, array, and type-definition parameters.
    • Enum and structured arguments are validated for computed-field queries.
  • Bug Fixes

    • Improved type accuracy for optional and required computed-field arguments.
    • Computed fields inherited from delegate models are handled correctly.
  • Tests

    • Added coverage for runtime filtering, invalid arguments, optional parameters, and generated TypeScript typings.

…ams metadata

`ComputedFieldArgs` read the args type off the generated `computedFields`
stub, whose non-scalar params (enums, type defs, models, Json) were emitted
as `unknown`. A wrong enum value or type-def shape therefore compiled in
`select`/`where`/`orderBy`, the aggregate inputs and the generated
`input.ts` types, and the implementation callback saw `unknown` too, while
zod already validated the args precisely at runtime.

- derive `ComputedFieldArgs` from the field's `params` metadata with the
  same mapping procedures use (enums -> value union, type defs -> object
  shape, optional -> optional key), and make `ComputedFieldsOptions` read
  the same source so implementation and query typing can't drift
- generator: name the stub param `_args` (the old name tripped
  `noUnusedParameters` in consuming projects) and emit enum params as the
  enum value union read off the schema's own `enums` member
- tests: runtime + compile-time e2e coverage for enum and type-def params,
  and a parameterized enum-param field in the typing schema

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: eebf8958-88e2-4e64-a118-8a54b92dd5f9

📥 Commits

Reviewing files that changed from the base of the PR and between bd2c2e1 and 80b2d4f.

📒 Files selected for processing (12)
  • packages/orm/src/client/client-impl.ts
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • packages/orm/src/client/options.ts
  • packages/orm/test/schema/schema.ts
  • packages/schema/src/schema.ts
  • packages/sdk/src/ts-schema-generator.ts
  • packages/zod/test/schema/schema-lite.ts
  • packages/zod/test/schema/schema.ts
  • samples/orm/zenstack/schema.ts
  • samples/taskforge/zenstack/schema.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts
  • tests/e2e/orm/schemas/typing/schema.ts
💤 Files with no reviewable changes (7)
  • packages/schema/src/schema.ts
  • packages/zod/test/schema/schema-lite.ts
  • packages/orm/test/schema/schema.ts
  • samples/taskforge/zenstack/schema.ts
  • packages/zod/test/schema/schema.ts
  • samples/orm/zenstack/schema.ts
  • tests/e2e/orm/schemas/typing/schema.ts

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


📝 Walkthrough

Walkthrough

Changes

Schema-driven computed fields

Layer / File(s) Summary
Use field metadata without generated stubs
packages/sdk/src/ts-schema-generator.ts, packages/schema/src/schema.ts, packages/orm/test/schema/schema.ts, packages/zod/test/schema/*, samples/*, tests/e2e/orm/schemas/typing/*
Computed fields now use field metadata and parameter definitions instead of generated stub methods or model-level computedFields configuration.
Derive computed field types from schema
packages/orm/src/client/crud-types.ts, packages/orm/src/client/options.ts
ORM argument and result types now derive from schema metadata. Shared parameter helpers support computed fields and procedures.
Validate and route computed fields
packages/orm/src/client/client-impl.ts, packages/orm/src/client/crud/dialects/base-dialect.ts
Runtime validation and join detection inspect computed fields in model field definitions.
Validate parameterized computed fields
tests/e2e/orm/client-api/computed-fields.test.ts, tests/e2e/orm/schemas/typing/typecheck.ts
Tests cover parameter types, runtime behavior, optionality, result types, and generated query input types.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Merge Risk: ⚪ Minimal · up to 80b2d

The computed-field metadata transition is consistent across generation, typing, runtime validation, and query handling. No merge-blocking risk remains.

Suggested reviewers: ymc9

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: deriving parameterized computed-field argument types from field parameter metadata.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/orm/src/client/client-impl.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

packages/orm/src/client/crud/dialects/base-dialect.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

packages/orm/src/client/options.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

  • 1 others

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.

`count(*)` is a bigint on Postgres, which the `pg` driver returns as a
string, so the plain-select assertion compared `'2'` with `2` on the
postgresql CI matrix. Normalize via `Number()` before comparing.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread packages/sdk/src/ts-schema-generator.ts Outdated
The return type was the last thing read off the generated stub, and it is
available on the field definition too (`type`/`optional`/`array`), so drop
the stub from the generated schema entirely and derive everything about a
computed field from its field def:

- `ComputedFieldsOptions` keys off fields with `computed: true` (excluding
  delegate-inherited ones, which are configured on the base model) and maps
  the implementation's return type from the field's declared type with the
  same mapping the stub used (scalars to JS types, Decimal as number,
  everything else `unknown`, `| null` for optional, `[]` for lists)
- the runtime config validation and the "computed fields need an explicit
  select" join check read the field defs instead of `modelDef.computedFields`
- `ModelDef.computedFields` is removed from the schema type and the
  generator no longer emits the stub (nor the enum-typed `_args` for it)
- regenerate the checked-in schemas that carried the stub
- e2e: assert the implementation's return type is still enforced

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ymc9
ymc9 merged commit e192d65 into zenstackhq:dev Sep 9, 2026
13 checks passed
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