Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions skills/rig/rig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* T:JsonSchemaObject type {[key:string]:unknown} plain JSON Schema object
* s.string/number/integer/boolean/null SchemaHelperFactory primitives; call as value or fn(desc)
* s.array(items,desc?) ArraySchema; use for homogeneous lists, e.g. s.array(s.string)
* s.object(props,desc?) ObjectSchema; s.optional(inner) marks field optional; use for fixed-key shapes
* s.object(props,desc?) ObjectSchema; s.optional(inner) marks field optional; s.nullable(inner) accepts inner|null; use for fixed-key shapes
* s.record(valSchema,desc?) RecordSchema keyed by string; use for open-ended key→value maps
* s.enum(...values|values,desc) EnumSchema
* s.unknown unconstrained JSON; call as value or s.unknown("description")
Expand Down Expand Up @@ -82,6 +82,7 @@ const OPTIONAL_SYMBOL: unique symbol = Symbol("rig.optional");
type OptionalMarker = { readonly [OPTIONAL_SYMBOL]: true };
type UnwrapOptional<T> = Omit<T, typeof OPTIONAL_SYMBOL>;
export type OptionalSchema<Inner extends Schema = Schema> = Inner & OptionalMarker;
export type NullableSchema<Inner extends Schema = Schema> = { nullable: true; inner: Inner; description?: string };

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[/grill-with-docs] NullableSchema is a plain object shape, unlike every other Schema member which carries SCHEMA_SYMBOL via markAsSchema. This means isSchema(s.nullable(s.string)) returns false, and any guard that relies on the schema-symbol invariant (INV:schema-symbol, line 54) will silently miss nullable values.

Suggested fix

Call markAsSchema the same way s.optional does:

nullable<Inner extends Schema>(schema: Inner, description?: string): NullableSchema<Inner> {
  const obj = description !== undefined
    ? { nullable: true as const, inner: schema, description }
    : { nullable: true as const, inner: schema };
  return markAsSchema(obj) as NullableSchema<Inner>;
}

export type Schema =
| StringSchema
Expand All @@ -94,7 +95,8 @@ export type Schema =
| ObjectSchema<any>
| RecordSchema<any>
| EnumSchema<any>
| OptionalSchema<any>;
| OptionalSchema<any>
| NullableSchema<any>;

type SchemaHelperFactory<T extends Schema> = T & ((description?: string) => T);

Expand Down Expand Up @@ -174,6 +176,7 @@ export type AgentInputValue<T> =
T | PromptIntent | PromptBuilder;

export type InferSchema<T> =
T extends { nullable: true; inner: infer Inner extends Schema } ? InferSchema<Inner> | null :

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[/tdd] The InferSchema branch for nullable checks structural duck-typing ({ nullable: true; inner: infer Inner }). Any object literal that coincidentally has nullable: true and inner will match — a potential false positive. s.optional avoids this via a unique symbol (OPTIONAL_SYMBOL). Consider adding a NULLABLE_SYMBOL guard the same way to make the type discriminant robust.

Why this matters

TypeScript structural typing means { nullable: true, inner: s.string, someExtraField: 1 } would match the InferSchema branch, which could produce confusing inferred types for unrelated objects that happen to have these keys.

T extends OptionalMarker ? InferSchema<UnwrapOptional<T>> | undefined :
T extends { type: "string" } ? string :
T extends { type: "number" } ? number :
Expand Down Expand Up @@ -257,6 +260,20 @@ export const s = {
optional<Inner extends Schema>(schema: Inner, description?: string): OptionalSchema<Inner> {
return markAsOptional(cloneSchema(schema, description));
},
/**
* Wraps a schema to also accept `null`. The serialized JSON Schema uses `anyOf`
* with the inner schema and `{"type":"null"}`. The inferred TypeScript type is
* `InferSchema<Inner> | null`.
*
* @example
* s.nullable(s.string) // string | null
* s.nullable(s.number, "score or null") // number | null with description
*/
nullable<Inner extends Schema>(schema: Inner, description?: string): NullableSchema<Inner> {
return markAsSchema(description !== undefined
? { nullable: true, inner: schema, description }
: { nullable: true, inner: schema });
},
/** Converts a rig `Schema` to a plain JSON Schema object. */
toJsonSchema,
};
Expand All @@ -268,9 +285,13 @@ export function toJsonSchema(schema: Schema): JsonSchemaObject {
}

function serializeSchema(schema: Schema): JsonSchemaObject {
const { description } = schema;
const { description } = schema as { description?: string };
const withDescription = (obj: JsonSchemaObject): JsonSchemaObject =>
description === undefined ? obj : { ...obj, description };
if ("nullable" in schema && schema.nullable === true) {
const inner = serializeSchema((schema as NullableSchema).inner);
return withDescription({ anyOf: [inner, { type: "null" }] });
}
if ("enum" in schema) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[/tdd] The serializeSchema description extraction uses a cast (schema as { description?: string }) because NullableSchema stores description on itself, not via the shared description?: string that most schema types carry. This cast will silently break if the property is ever renamed or moved. Consider adding description?: string directly to the Schema base union (or an interface) so the cast isn't needed.

const enumValues = schema.enum as readonly unknown[];
const allStrings = enumValues.length > 0 && enumValues.every((v) => typeof v === "string");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[/grill-with-docs] SKILL.md documents every s.* helper in the schema reference table (lines 130–146) but s.nullable is not listed there. Callers reading the docs won't discover the new helper until they inspect the source. A one-liner addition to the table (s.nullable(inner) / s.nullable(inner, "desc")) would bring docs in sync with the implementation.

Expand Down Expand Up @@ -1381,6 +1402,10 @@ function validateSchema(value: unknown, schema: Schema, path: string, optional:
if ((optional || isOptionalSchema(schema)) && value === undefined) {
return { ok: true };
}
if ("nullable" in schema && schema.nullable === true) {
if (value === null) return ok();
return validateSchema(value, (schema as NullableSchema).inner, path, false);
}
if ("enum" in schema) {
return schema.enum.some((item: Json) => deepEqual(item, value))
? ok()
Expand Down
34 changes: 34 additions & 0 deletions src/rig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,40 @@ describe("toJsonSchema", () => {
});
});

describe("s.nullable", () => {
it("serializes to anyOf with null", () => {
expect(toJsonSchema(s.nullable(s.string))).toEqual({ anyOf: [{ type: "string" }, { type: "null" }] });
expect(toJsonSchema(s.nullable(s.number))).toEqual({ anyOf: [{ type: "number" }, { type: "null" }] });
});

it("includes description when provided", () => {
expect(toJsonSchema(s.nullable(s.string, "maybe text"))).toEqual({
anyOf: [{ type: "string" }, { type: "null" }],
description: "maybe text",
});
});

it("accepts null values during validation", async () => {
mocks.setSendAndWaitImpl(async () => JSON.stringify({ score: null }));
const a = agent({ output: s.object({ score: s.nullable(s.number) }) });
const result = await a("");
expect(result).toEqual({ score: null });
});

it("accepts non-null values during validation", async () => {
mocks.setSendAndWaitImpl(async () => JSON.stringify({ score: 42 }));
const a = agent({ output: s.object({ score: s.nullable(s.number) }) });
const result = await a("");
expect(result).toEqual({ score: 42 });
});

it("rejects invalid values that are neither null nor inner type", async () => {
mocks.setSendAndWaitImpl(async () => JSON.stringify({ score: "not-a-number" }));
const a = agent({ output: s.object({ score: s.nullable(s.number) }), maxTurns: 1 });
await expect(a("")).rejects.toBeInstanceOf(AgentError);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[/tdd] Missing edge case: s.nullable nested inside s.optional (and vice versa). Both modifiers interact at the validation level — optional short-circuits on undefined, nullable short-circuits on null. A combined test like s.object({ x: s.optional(s.nullable(s.number)) }) would confirm the composability of the two helpers and prevent a silent regression if either dispatch order changes.

});
});

describe("s.integer validation", () => {
it("accepts whole numbers", () => {
const intAgent = agent({ output: s.integer });
Expand Down
Loading