feat(api): use camelCase for TypeScript SDK#4622
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (18)
📝 WalkthroughWalkthroughThis PR adds a camelCase SDK surface mapped to snake_case wire payloads, with build-time casing checks, runtime wire conversion/validation, regenerated SDK request/response handling, and matching docs, tests, and tooling updates. ChangesWire mapping boundary feature
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant funcs
participant toWire
participant assertValid
participant HTTP
participant fromWire
Caller->>funcs: call createMeter(req)
funcs->>toWire: convert camelCase body to snake_case
funcs->>assertValid: validate wire body (if options.validate)
funcs->>HTTP: POST wire body
HTTP-->>funcs: JSON response
funcs->>assertValid: validate response wire (if options.validate)
funcs->>fromWire: convert response to camelCase
funcs-->>Caller: typed camelCase result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR changes the TypeScript SDK to expose camelCase shapes while keeping snake_case on the wire. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "chore: review comments" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
api/spec/packages/aip-client-javascript/.npmignore (1)
14-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlso ignore
tests/in the npm package.Nice catch adding the Vitest config and coverage excludes. The new
tests/tree from this PR is still publishable, though, so consumers will get dev-only specs/helpers unless that folder is ignored too.Suggested tweak
vitest.config.ts .gitignore +tests/ # Test coverage output coverage/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/spec/packages/aip-client-javascript/.npmignore` around lines 14 - 19, The npm package ignore list is missing the new tests tree, so dev-only specs/helpers would still be published. Update the .npmignore entry set alongside vitest.config.ts and coverage/ to also exclude tests/, keeping the package publishable only with runtime files.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/spec/AGENTS.md`:
- Around line 200-213: The Markdown in this contract section is broken because
the asterisks in snake*case and \_inside* are being interpreted as emphasis
instead of literal text. Update the AGENTS.md prose near SDKOptions.validate and
request() to use proper inline code or plain Markdown emphasis for the intended
terms, and verify the surrounding wording still reads correctly after replacing
those malformed fragments.
In `@api/spec/packages/aip-client-javascript/src/funcs/apps.ts`:
- Around line 31-35: The listApps response mapping in apps.ts is dropping
server-added fields because fromWire() only keeps schema-defined keys. Update
the listApps transformation to preserve unknown response properties while still
camelizing and validating the known ones via schemas.listAppsResponseWire and
schemas.listAppsResponse, so additive fields remain available to consumers when
client._options.validate is off.
In `@api/spec/packages/aip-client-javascript/src/funcs/plans.ts`:
- Around line 29-38: The listPlans query path is serializing the wire object
without runtime validation, so invalid page/sort/filter values can slip through
when client._options.validate is enabled. Update the listPlans request
construction to validate the wire query object against
schemas.listPlansQueryParams before calling toURLSearchParams, using the same
validation flow used for request bodies so bad inputs fail fast. Use the
existing listPlans query builder, toWire, encodeSort, and
schemas.listPlansQueryParams symbols to place the check in the right spot.
In `@api/spec/packages/aip-client-javascript/src/lib/config.ts`:
- Around line 22-27: The JSDoc for the validation setting currently says
failures “reject,” but the SDK actually surfaces them as a failed Result via
assertValid(...) inside request(() => ...), so callers won’t hit catch handlers.
Update the wording in the config documentation and the emitting template to
describe that validation failures return Result.error/failed Result instead of
rejecting, and keep the description aligned with the assertValid and request
flow.
In `@api/spec/packages/aip-client-javascript/src/lib/wire.ts`:
- Around line 20-22: The helper def currently reads the wrong Zod internals, so
it never returns the schema definition for Zod 4 types. Update def in wire.ts to
read from schema._zod.def instead of schema.def, and keep the rest of the walk()
logic unchanged so object, union, and record payload key renaming paths are
reached.
In `@api/spec/packages/aip-client-javascript/tests/wire-helpers.ts`:
- Around line 115-121: The key collection logic in collectFieldKeys is skipping
the entire subtree for user_key_* entries, which causes nested schema fields to
be missed. Update the loop over Object.entries(value) so preserved user record
keys are not added as schema fields themselves, but their nested values are
still traversed for leak-checking; keep the existing handling in
collectFieldKeys and any caller that relies on the generated keys list.
In `@api/spec/packages/typespec-typescript/src/casing-gate.ts`:
- Around line 68-70: The casing gate is only checking namespace-owned unions
because it iterates userUnions(program), so inline request/response unions are
skipped. Update the union scan in mappedReachableUnions/casing-gate logic to
iterate mappedUnions instead, and keep the existing ambiguous-union and
discriminator/envelope casing checks applied to every mapped union.
In `@api/spec/packages/typespec-typescript/src/emitter.tsx`:
- Around line 102-105: The current casing check in assertCasingDerivable only
verifies that each wire key can be deterministically recovered, but it does not
catch collisions where two different wire names map to the same camelCase public
name. Extend the gate around assertCasingDerivable in emitter.tsx to also detect
duplicate camelized names before emission, and fail the build when a model’s
fields or a query object’s parameters would collapse to the same public key. Use
the existing model and operation traversal to compare the derived public names
for each group and reject any duplicates before the public emitters run.
In `@api/spec/packages/typespec-typescript/src/runtime/wire.ts`:
- Around line 20-22: The literal discriminator lookup is still reading the old
Zod shape, so discriminated union renaming won’t work. Update the discriminator
extraction in the runtime wire helpers by adjusting the `literalValue()` logic
to read from `def.values` via the existing `def()` helper instead of
`schema.value`, keeping the change aligned with `ZodDef` and the `def()`
function.
In `@api/spec/packages/typespec-typescript/src/sdk-files.ts`:
- Around line 6-17: The path templating in pathExpr() no longer preserves the
fail-fast missing-parameter guard from encodePath, so a missing runtime path arg
can be sent as an “undefined”/“null” segment. Update pathExpr() to keep the
inline template literal behavior while still validating required path params
before building the URL, using the same SdkOperation path-param handling that
encodePath previously enforced.
In `@api/spec/packages/typespec-typescript/src/ZodOperations.tsx`:
- Around line 18-23: Make the query-parameter schema generation in ZodOperations
wire-mode aware so the wire pass does not camelCase keys for query params.
Update the logic around paramObject(queryParams, true) and the related query
schema builders (including *QueryParamsWire and the helpers in ZodOperations) to
preserve wire-format names when generating wire schemas, while still using
toCamelCase only for the non-wire schema path. Ensure the fix covers the
query-params validation flow used after toWire(...).
---
Nitpick comments:
In `@api/spec/packages/aip-client-javascript/.npmignore`:
- Around line 14-19: The npm package ignore list is missing the new tests tree,
so dev-only specs/helpers would still be published. Update the .npmignore entry
set alongside vitest.config.ts and coverage/ to also exclude tests/, keeping the
package publishable only with runtime files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b7655d9-228b-4cd6-8ff4-9c0a6feae491
⛔ Files ignored due to path filters (1)
api/spec/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
api/spec/AGENTS.mdapi/spec/package.jsonapi/spec/packages/aip-client-javascript/.gitignoreapi/spec/packages/aip-client-javascript/.npmignoreapi/spec/packages/aip-client-javascript/README.mdapi/spec/packages/aip-client-javascript/src/funcs/addons.tsapi/spec/packages/aip-client-javascript/src/funcs/apps.tsapi/spec/packages/aip-client-javascript/src/funcs/billing.tsapi/spec/packages/aip-client-javascript/src/funcs/currencies.tsapi/spec/packages/aip-client-javascript/src/funcs/customers.tsapi/spec/packages/aip-client-javascript/src/funcs/defaults.tsapi/spec/packages/aip-client-javascript/src/funcs/entitlements.tsapi/spec/packages/aip-client-javascript/src/funcs/events.tsapi/spec/packages/aip-client-javascript/src/funcs/features.tsapi/spec/packages/aip-client-javascript/src/funcs/governance.tsapi/spec/packages/aip-client-javascript/src/funcs/invoices.tsapi/spec/packages/aip-client-javascript/src/funcs/llmCost.tsapi/spec/packages/aip-client-javascript/src/funcs/meters.tsapi/spec/packages/aip-client-javascript/src/funcs/planAddons.tsapi/spec/packages/aip-client-javascript/src/funcs/plans.tsapi/spec/packages/aip-client-javascript/src/funcs/subscriptions.tsapi/spec/packages/aip-client-javascript/src/funcs/tax.tsapi/spec/packages/aip-client-javascript/src/index.tsapi/spec/packages/aip-client-javascript/src/lib/config.tsapi/spec/packages/aip-client-javascript/src/lib/encodings.tsapi/spec/packages/aip-client-javascript/src/lib/wire.tsapi/spec/packages/aip-client-javascript/src/models/operations/tax.tsapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip-client-javascript/tests/meters.spec.tsapi/spec/packages/aip-client-javascript/tests/wire-helpers.tsapi/spec/packages/aip-client-javascript/tests/wire.generated.spec.tsapi/spec/packages/aip-client-javascript/tests/wire.spec.tsapi/spec/packages/aip-client-javascript/vitest.config.tsapi/spec/packages/typespec-typescript/package.jsonapi/spec/packages/typespec-typescript/src/ZodOperations.tsxapi/spec/packages/typespec-typescript/src/casing-gate.tsapi/spec/packages/typespec-typescript/src/casing.tsapi/spec/packages/typespec-typescript/src/components/ZodSchema.tsxapi/spec/packages/typespec-typescript/src/components/ZodSchemaDeclaration.tsxapi/spec/packages/typespec-typescript/src/emitter.tsxapi/spec/packages/typespec-typescript/src/interface-types.tsapi/spec/packages/typespec-typescript/src/readme.tsapi/spec/packages/typespec-typescript/src/request-types.tsapi/spec/packages/typespec-typescript/src/runtime-templates.tsapi/spec/packages/typespec-typescript/src/runtime/wire.tsapi/spec/packages/typespec-typescript/src/sdk-files.tsapi/spec/packages/typespec-typescript/src/ts-types.tsapi/spec/packages/typespec-typescript/src/utils.tsxapi/spec/packages/typespec-typescript/src/wire-runtime.tsapi/spec/packages/typespec-typescript/src/zodBaseSchema.tsxapi/spec/packages/typespec-typescript/test/casing.test.ts
| `SDKOptions.validate` (default **off**) turns on schema validation of the actual | ||
| snake*case wire payload: the request body after `toWire` (before sending) and the | ||
| raw response body before `fromWire`. Validation uses the generated **`…Wire` | ||
| schemas** in `models/schemas.ts` — every model and per-op body/response is emitted a | ||
| second time in a snake_case "wire" pass (`WireModeContext` in the emitter), keyed by | ||
| the raw JSON wire name and made `z.strictObject`, so a wrong-shaped or | ||
| leaked-camelCase wire field is **rejected, not silently stripped**. Open models | ||
| (record spread, `emitsAsIntersection`, e.g. `baseError`) stay non-strict — strict | ||
| would defeat the record arm that exists to accept them. Because the wire pass is the | ||
| same emitter walk as the camelCase pass (parameterized by key-casing + strictness + | ||
| a separate refkey namespace), the two are structurally identical except for casing, | ||
| **by construction** — no runtime schema derivation. A failure throws | ||
| `ValidationError`, which `request()` surfaces as `Result.error` (request validation | ||
| runs \_inside* the `request()` closure so it does not throw synchronously). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the broken Markdown formatting here.
snake*case and \_inside* currently render like malformed emphasis, so this part of the contract reads oddly in the docs. I'd switch them to snake_case and _inside_.
As per path instructions, **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness".
Suggested tweak
-`SDKOptions.validate` (default **off**) turns on schema validation of the actual
-snake*case wire payload: the request body after `toWire` (before sending) and the
+`SDKOptions.validate` (default **off**) turns on schema validation of the actual
+`snake_case` wire payload: the request body after `toWire` (before sending) and the
@@
-`ValidationError`, which `request()` surfaces as `Result.error` (request validation
-runs \_inside* the `request()` closure so it does not throw synchronously).
+`ValidationError`, which `request()` surfaces as `Result.error` (request validation
+runs _inside_ the `request()` closure so it does not throw synchronously).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `SDKOptions.validate` (default **off**) turns on schema validation of the actual | |
| snake*case wire payload: the request body after `toWire` (before sending) and the | |
| raw response body before `fromWire`. Validation uses the generated **`…Wire` | |
| schemas** in `models/schemas.ts` — every model and per-op body/response is emitted a | |
| second time in a snake_case "wire" pass (`WireModeContext` in the emitter), keyed by | |
| the raw JSON wire name and made `z.strictObject`, so a wrong-shaped or | |
| leaked-camelCase wire field is **rejected, not silently stripped**. Open models | |
| (record spread, `emitsAsIntersection`, e.g. `baseError`) stay non-strict — strict | |
| would defeat the record arm that exists to accept them. Because the wire pass is the | |
| same emitter walk as the camelCase pass (parameterized by key-casing + strictness + | |
| a separate refkey namespace), the two are structurally identical except for casing, | |
| **by construction** — no runtime schema derivation. A failure throws | |
| `ValidationError`, which `request()` surfaces as `Result.error` (request validation | |
| runs \_inside* the `request()` closure so it does not throw synchronously). | |
| `SDKOptions.validate` (default **off**) turns on schema validation of the actual | |
| `snake_case` wire payload: the request body after `toWire` (before sending) and the | |
| raw response body before `fromWire`. Validation uses the generated **`…Wire` | |
| schemas** in `models/schemas.ts` — every model and per-op body/response is emitted a | |
| second time in a snake_case "wire" pass (`WireModeContext` in the emitter), keyed by | |
| the raw JSON wire name and made `z.strictObject`, so a wrong-shaped or | |
| leaked-camelCase wire field is **rejected, not silently stripped**. Open models | |
| (record spread, `emitsAsIntersection`, e.g. `baseError`) stay non-strict — strict | |
| would defeat the record arm that exists to accept them. Because the wire pass is the | |
| same emitter walk as the camelCase pass (parameterized by key-casing + strictness + | |
| a separate refkey namespace), the two are structurally identical except for casing, | |
| **by construction** — no runtime schema derivation. A failure throws | |
| `ValidationError`, which `request()` surfaces as `Result.error` (request validation | |
| runs _inside_ the `request()` closure so it does not throw synchronously). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/spec/AGENTS.md` around lines 200 - 213, The Markdown in this contract
section is broken because the asterisks in snake*case and \_inside* are being
interpreted as emphasis instead of literal text. Update the AGENTS.md prose near
SDKOptions.validate and request() to use proper inline code or plain Markdown
emphasis for the intended terms, and verify the surrounding wording still reads
correctly after replacing those malformed fragments.
Source: Path instructions
| .then((data) => { | ||
| if (client._options.validate) { | ||
| assertValid(schemas.listAppsResponseWire, data) | ||
| } | ||
| return fromWire(data, schemas.listAppsResponse) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
This now strips additive response fields from the runtime payload.
Because fromWire() only materializes schema-declared keys, any server-added fields that used to survive raw .json<T>() are silently lost here even when validation is off. That’s a wire-contract change across the SDK surface for consumers that proxy responses or reach for newly added fields before regenerating. Unless this is intentionally semver-major, the mapper should preserve unknown response keys and only camelize the known ones.
Also applies to: 50-54
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/spec/packages/aip-client-javascript/src/funcs/apps.ts` around lines 31 -
35, The listApps response mapping in apps.ts is dropping server-added fields
because fromWire() only keeps schema-defined keys. Update the listApps
transformation to preserve unknown response properties while still camelizing
and validating the known ones via schemas.listAppsResponseWire and
schemas.listAppsResponse, so additive fields remain available to consumers when
client._options.validate is off.
| function def(schema: ZodType | undefined): ZodDef | undefined { | ||
| return (schema as { def?: ZodDef } | undefined)?.def | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
# Inspect the target file with line numbers.
sed -n '1,220p' api/spec/packages/aip-client-javascript/src/lib/wire.ts
printf '\n--- search for schema.def and zod internals ---\n'
rg -n "\.def\b|_def\b|schema\.def|schema\._def|ZodType" api/spec/packages/aip-client-javascript -S
printf '\n--- locate package metadata for zod version ---\n'
rg -n '"zod"|zod' api/spec/packages/aip-client-javascript/package.json api/spec/packages/aip-client-javascript/**/package.json -S || trueRepository: openmeterio/openmeter
Length of output: 11966
🏁 Script executed:
set -euo pipefail
# Find the Zod package version and any local wrappers around it.
git ls-files | rg '(^|/)package\.json$|(^|/)wire\.ts$|(^|/)zod' -n
printf '\n--- package.json files mentioning zod ---\n'
for f in $(git ls-files '**/package.json'); do
if rg -q '"zod"|zod' "$f"; then
echo "### $f"
sed -n '1,220p' "$f"
echo
fi
done
printf '\n--- broader search for definition access patterns ---\n'
rg -n "\.def\b|_def\b|definition\b" api/spec/packages/aip-client-javascript -SRepository: openmeterio/openmeter
Length of output: 7878
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
p = Path('api/spec/packages/aip-client-javascript/src/lib/wire.ts')
print(p.read_text())
PYRepository: openmeterio/openmeter
Length of output: 9608
🌐 Web query:
Zod 4 schema internals def _def public property docs
💡 Result:
In Zod 4, the internal structure of schemas has been significantly refactored compared to Zod 3. All Zod 4 schema instances contain a private _zod property [1][2], which acts as the container for the schema's internals [1]. The _def property, which was the primary way to access schema internals in Zod 3, has been deprecated in Zod 4 in favor of a cleaner internal structure [3]. While _def may still be present in some instances for backward compatibility, it is officially deprecated and not guaranteed to be consistent across all schema types [3]. Key points regarding schema internals in Zod 4: 1. The _zod Property: All zod/v4/core classes use a single, private _zod property [1]. This object encapsulates the schema's definition, traits, and other metadata [2]. 2. The def Property: The canonical way to access a schema's definition within the _zod object is through ._zod.def [1]. This property contains the configuration object passed to the schema constructor and is designed to be JSON-serializable [1]. It allows for programmatic inspection of schema types (e.g., def.type) and configuration [1]. 3. Deprecation of _def: Because of the move to the _zod internal object, accessing schema._def is officially discouraged [3]. In some complex cases or specific schema types (like ZodDefault), _def may still be the only way to retrieve certain properties, but reliance on it is considered brittle and not a stable public API [3]. 4. Schema Identification: Developers are advised against using _def identity checks (e.g., schemaA._def === schemaB._def) to identify schemas, especially since methods like .describe() or .meta() often clone the schema and create new _def objects [4]. Instead, metadata should be handled using the dedicated .meta() and .register() methods [5]. For developers building tools or libraries that need to traverse schemas, the recommended pattern is to cast the schema to $ZodTypes and inspect the ._zod.def property to discriminate between types [1].
Citations:
- 1: https://zod.dev/packages/core
- 2: https://github.com/colinhacks/zod/blob/c7805073/packages/zod/src/v4/core/core.ts
- 3: v4 field.def is broken colinhacks/zod#5020
- 4: .describe() is not wrapping, but creates a "new" _def colinhacks/zod#3832
- 5: https://zod.dev/packages/zod
Read Zod internals from schema._zod.def here. Zod 4 stores schema definitions under _zod.def, so schema.def will stay undefined and walk() will skip the object/union/record paths that rename payload keys.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/spec/packages/aip-client-javascript/src/lib/wire.ts` around lines 20 -
22, The helper def currently reads the wrong Zod internals, so it never returns
the schema definition for Zod 4 types. Update def in wire.ts to read from
schema._zod.def instead of schema.def, and keep the rest of the walk() logic
unchanged so object, union, and record payload key renaming paths are reached.
| // Fail the build if any wire key is not recoverable from its camelCase public | ||
| // form by the deterministic casing rule, before emitting anything that relies | ||
| // on it. | ||
| assertCasingDerivable(context.program, models, operations) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The casing gate still misses camelCase collisions.
Right now this only proves each wire key round-trips on its own. It still allows two distinct wire names to collapse onto the same public key — e.g. foo_bar and fooBar both become fooBar — and the new public emitters now use that camelized name for model fields and query params. In that case the generated types/schemas will silently duplicate or shadow one side instead of failing the build. Please extend the gate to reject duplicate camelized names per model and per query object before emission.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/spec/packages/typespec-typescript/src/emitter.tsx` around lines 102 -
105, The current casing check in assertCasingDerivable only verifies that each
wire key can be deterministically recovered, but it does not catch collisions
where two different wire names map to the same camelCase public name. Extend the
gate around assertCasingDerivable in emitter.tsx to also detect duplicate
camelized names before emission, and fail the build when a model’s fields or a
query object’s parameters would collapse to the same public key. Use the
existing model and operation traversal to compare the derived public names for
each group and reject any duplicates before the public emitters run.
| function def(schema: ZodType | undefined): ZodDef | undefined { | ||
| return (schema as { def?: ZodDef } | undefined)?.def | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
node --input-type=module <<'NODE'
import { z } from 'zod'
const objectSchema = z.object({ fooBar: z.string() })
const literalSchema = z.literal('customer')
console.log({
objectDef: objectSchema.def?.type,
objectZodDef: objectSchema._zod?.def?.type,
literalValue: literalSchema.value,
literalDefValue: literalSchema.def?.value,
literalDefValues: literalSchema.def?.values,
literalZodDefValue: literalSchema._zod?.def?.value,
literalZodDefValues: literalSchema._zod?.def?.values,
})
NODERepository: openmeterio/openmeter
Length of output: 1159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== package references ==\n'
rg -n '"zod"|zod' api/spec/packages/typespec-typescript/package.json api/spec/packages/typespec-typescript -g '!**/dist/**' -g '!**/build/**' || true
printf '\n== relevant source ==\n'
sed -n '1,260p' api/spec/packages/typespec-typescript/src/runtime/wire.ts
printf '\n== nearby callers ==\n'
rg -n "literalValue\\(|def\\(" api/spec/packages/typespec-typescript/src/runtime -g '!**/dist/**' || trueRepository: openmeterio/openmeter
Length of output: 22040
🌐 Web query:
Zod 4.4.3 schema def _zod.def literal value internal property classic runtime
💡 Result:
In Zod 4.4.3, accessing internal properties of a schema, such as the literal values of a z.literal() schema, is considered an advanced or internal use case rather than a supported public API [1][2]. Key points regarding schema introspection and internal properties in Zod 4: 1. Introspection APIs: Zod does not provide a single, unified public API for schema introspection. Instead, it relies on specific instance properties (e.g., .shape for objects, .element for arrays, .unwrap() for optional/nullable wrappers) [3][2]. 2. Internal Definitions (.def vs ._def): Zod schemas maintain their configuration within a def property [4][5]. In Zod 4, the primary property is .def, and ._def is deprecated (often retained for internal compatibility) [5][6]. These properties contain the schema's type definition and metadata, such as the values set for literals [4][7]. 3. Breaking Changes for Literals: In Zod 4, z.literal() schemas no longer expose a single .value property. This was replaced by a .values property, which is a Set to support cases where a literal schema might represent multiple values [8]. 4. Accessing Literal Values: Because these internal structures are subject to change and not part of the stable public API, code that relies on accessing schema.def.values is fragile and may break across minor or patch versions [9][2]. When introspection is required, the recommended approach is often to perform custom logic (such as checking instanceof on specific Zod classes) or to write a utility function that traverses the schema structure using these internal properties with appropriate checks [1][9][2]. For Zod 4.4.3 specifically, you can inspect the schema's internal definition structure by exploring the def object [4][7], but you should be aware that these properties are explicitly marked as @internal or are considered implementation details not guaranteed to remain stable [4][7].
Citations:
- 1: Retrieve validations type from schema colinhacks/zod#4621
- 2: Improve Zod Type Inspection colinhacks/zod#1833
- 3: https://github.com/colinhacks/zod/blob/9977fb08/packages/docs/content/api.mdx
- 4: https://github.com/colinhacks/zod/blob/c7805073/packages/zod/src/v4/core/schemas.ts
- 5: https://github.com/colinhacks/zod/blob/76e8f706/packages/zod/src/v4/classic/schemas.ts
- 6: https://github.com/colinhacks/zod/blob/912f0f51/packages/zod/src/v4/classic/schemas.ts
- 7: https://cdn.jsdelivr.net/npm/@zod/core@0.11.6/dist/esm/schemas.d.ts
- 8: v4: breaking change to schema introspection:
z.literal().valueno longer exists, is nowz.literal().valuescolinhacks/zod#4497 - 9: [Question] How can I get field type from Zod object (reflection like)? colinhacks/zod#371
Use def.values for literal discriminators
def() matches Zod 4.4.3, but literalValue() is still reading schema.value. Zod 4 literals keep their allowed values on def.values, so the discriminator map stays empty and discriminated unions won’t be renamed correctly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/spec/packages/typespec-typescript/src/runtime/wire.ts` around lines 20 -
22, The literal discriminator lookup is still reading the old Zod shape, so
discriminated union renaming won’t work. Update the discriminator extraction in
the runtime wire helpers by adjusting the `literalValue()` logic to read from
`def.values` via the existing `def()` helper instead of `schema.value`, keeping
the change aligned with `ZodDef` and the `def()` function.
| function literalValue(schema: ZodType | undefined): unknown { | ||
| const s = unwrap(schema) | ||
| if (def(s)?.type === 'literal') { | ||
| return (s as { value?: unknown }).value |
There was a problem hiding this comment.
literalValue still reads the discriminator literal from the old top-level schema.value slot. The generated Zod v4 discriminated schemas use z.literal(...) inside the variant shape, so this lookup can still return undefined and leave variantsByDiscriminator empty. When fromWire maps a valid discriminated response such as an app response, selectVariant cannot choose the branch and returns the raw object, leaving wire keys like account_id on the public SDK result instead of accountId.
Context Used: api/spec/AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/spec/packages/typespec-typescript/src/runtime/wire.ts
Line: 255
Comment:
**Literal Lookup Still Fails**
`literalValue` still reads the discriminator literal from the old top-level `schema.value` slot. The generated Zod v4 discriminated schemas use `z.literal(...)` inside the variant shape, so this lookup can still return `undefined` and leave `variantsByDiscriminator` empty. When `fromWire` maps a valid discriminated response such as an app response, `selectVariant` cannot choose the branch and returns the raw object, leaving wire keys like `account_id` on the public SDK result instead of `accountId`.
**Context Used:** api/spec/AGENTS.md ([source](https://app.greptile.com/openmeter/github/openmeterio/openmeter/-/custom-context?memory=28ba6068-00f9-4629-9b78-8e49cc802858))
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/spec/packages/aip-client-javascript/src/funcs/customers.ts`:
- Around line 75-80: The path-building IIFE in the customer SDK methods throws
before request(...) is invoked, so missing customerId/creditGrantId escapes the
SDK error wrapper. Move the missing-path validation and path construction inside
the request callback for the affected customer-related operations in
customers.ts so request(...) can return a Promise<Result<...>> consistently. Use
the existing request helper and the generated method bodies as the lookup
points, and apply the same pattern to all other listed endpoint builders in this
file.
In `@api/spec/packages/typespec-typescript/src/runtime/wire.ts`:
- Around line 151-155: The wire mapper is returning null-prototype records
instead of plain objects. Update the record-building logic in the wire
conversion path (the `wire.ts` code that iterates `Object.entries(record)` and
any similar helper around the other flagged location) to use normal plain
objects, ideally by building from entries rather than `Object.create(null)`, so
callers keep standard `Object.prototype` behavior while still preserving
`__proto__` as data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0d601d8-ef95-439a-9285-2514d445fb3c
📒 Files selected for processing (22)
api/spec/packages/aip-client-javascript/src/funcs/addons.tsapi/spec/packages/aip-client-javascript/src/funcs/apps.tsapi/spec/packages/aip-client-javascript/src/funcs/billing.tsapi/spec/packages/aip-client-javascript/src/funcs/currencies.tsapi/spec/packages/aip-client-javascript/src/funcs/customers.tsapi/spec/packages/aip-client-javascript/src/funcs/entitlements.tsapi/spec/packages/aip-client-javascript/src/funcs/features.tsapi/spec/packages/aip-client-javascript/src/funcs/invoices.tsapi/spec/packages/aip-client-javascript/src/funcs/llmCost.tsapi/spec/packages/aip-client-javascript/src/funcs/meters.tsapi/spec/packages/aip-client-javascript/src/funcs/planAddons.tsapi/spec/packages/aip-client-javascript/src/funcs/plans.tsapi/spec/packages/aip-client-javascript/src/funcs/subscriptions.tsapi/spec/packages/aip-client-javascript/src/funcs/tax.tsapi/spec/packages/aip-client-javascript/src/index.tsapi/spec/packages/aip-client-javascript/src/lib/wire.tsapi/spec/packages/aip-client-javascript/src/models/schemas.tsapi/spec/packages/aip-client-javascript/tests/wire.spec.tsapi/spec/packages/typespec-typescript/src/ZodOperations.tsxapi/spec/packages/typespec-typescript/src/casing-gate.tsapi/spec/packages/typespec-typescript/src/runtime/wire.tsapi/spec/packages/typespec-typescript/src/sdk-files.ts
✅ Files skipped from review due to trivial changes (1)
- api/spec/packages/aip-client-javascript/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- api/spec/packages/aip-client-javascript/src/funcs/entitlements.ts
- api/spec/packages/aip-client-javascript/src/funcs/invoices.ts
- api/spec/packages/aip-client-javascript/src/funcs/apps.ts
- api/spec/packages/typespec-typescript/src/ZodOperations.tsx
- api/spec/packages/aip-client-javascript/src/funcs/addons.ts
- api/spec/packages/typespec-typescript/src/casing-gate.ts
- api/spec/packages/aip-client-javascript/src/funcs/billing.ts
- api/spec/packages/aip-client-javascript/tests/wire.spec.ts
- api/spec/packages/aip-client-javascript/src/funcs/currencies.ts
- api/spec/packages/aip-client-javascript/src/funcs/tax.ts
- api/spec/packages/aip-client-javascript/src/funcs/features.ts
- api/spec/packages/aip-client-javascript/src/funcs/planAddons.ts
- api/spec/packages/aip-client-javascript/src/funcs/subscriptions.ts
- api/spec/packages/aip-client-javascript/src/funcs/llmCost.ts
- api/spec/packages/aip-client-javascript/src/funcs/plans.ts
- api/spec/packages/typespec-typescript/src/sdk-files.ts
- api/spec/packages/aip-client-javascript/src/funcs/meters.ts
| const valueSchema = hasRenamableShape(d.valueType) ? d.valueType : undefined | ||
| const out: Record<string, unknown> = Object.create(null) | ||
| for (const [key, value] of Object.entries(record)) { | ||
| out[key] = valueSchema ? walk(value, valueSchema, dir, depth + 1) : value | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
node --input-type=module <<'NODE'
const out = Object.fromEntries([['__proto__', 'safe'], ['constructor', 1]])
console.log(Object.getPrototypeOf(out) === Object.prototype)
console.log(Object.prototype.hasOwnProperty.call(out, '__proto__'))
console.log(typeof out.hasOwnProperty)
const nullProto = Object.create(null)
nullProto.foo = 1
console.log(typeof nullProto.hasOwnProperty)
NODERepository: openmeterio/openmeter
Length of output: 188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure first
ast-grep outline api/spec/packages/typespec-typescript/src/runtime/wire.ts --view expanded
# Read the relevant region around the reported lines
sed -n '120,210p' api/spec/packages/typespec-typescript/src/runtime/wire.ts
# Find other uses of Object.create(null) in this runtime file
rg -n "Object\.create\(null\)|Object\.fromEntries|hasOwnProperty|instanceof Object" api/spec/packages/typespec-typescript/src/runtime/wire.tsRepository: openmeterio/openmeter
Length of output: 4934
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,360p' api/spec/packages/typespec-typescript/src/runtime/wire.ts
printf '\n--- usages ---\n'
rg -n "\btoWire\(|\bfromWire\(|assertValid\(|wire\.ts" api/spec/packages/typespec-typescript/src -g '!**/dist/**' -g '!**/build/**'Repository: openmeterio/openmeter
Length of output: 12679
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for generated-client guidance and for places where these helpers are used in serialization paths.
rg -n "generated SDK|regenerate it from TypeSpec|toWire\(|fromWire\(" api/spec/packages -g '!**/dist/**' -g '!**/build/**' -g '!**/node_modules/**'Repository: openmeterio/openmeter
Length of output: 21601
Return plain objects from the wire mapper api/spec/packages/typespec-typescript/src/runtime/wire.ts:152,175
Object.create(null) turns these SDK results into null-prototype values, which drops normal Object.prototype behavior for callers. Object.fromEntries(...) keeps __proto__ as data while preserving a standard object prototype.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@api/spec/packages/typespec-typescript/src/runtime/wire.ts` around lines 151 -
155, The wire mapper is returning null-prototype records instead of plain
objects. Update the record-building logic in the wire conversion path (the
`wire.ts` code that iterates `Object.entries(record)` and any similar helper
around the other flagged location) to use normal plain objects, ideally by
building from entries rather than `Object.create(null)`, so callers keep
standard `Object.prototype` behavior while still preserving `__proto__` as data.
Source: Coding guidelines
1 participant
Summary by CodeRabbit
SDKOptions.validate) with validation failures returned asResult.error(viaValidationError).DepthLimitExceededError).