Skip to content

[rig-sampler] Add s.percent schema helper for percentage values (0–100) #167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 1 commit into from
Jul 26, 2026
Merged

[rig-sampler] Add s.percent schema helper for percentage values (0–100) #167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion skills/rig/rig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
* T:JsonSchemaObject type {[key:string]:unknown} plain JSON Schema object

@github-actions github-actions Bot Jul 26, 2026

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] The header docstring and rig.ts JSDoc were updated, but SKILL.md's schema helper table (around line 62) still lists only s.number and doesn't mention s.percent, s.positiveInt, or s.nonNegativeInt. Agents using SKILL.md as prompt context won't know to prefer s.percent over s.number for percentage fields.

💡 Suggested SKILL.md addition

In the schema helpers table, add a row or note like:

| Constrained numbers | `s.positiveInt`, `s.nonNegativeInt`, `s.percent` (0–100) |

And in the prose guidance (around line 70), add: "Use s.percent instead of s.number for percentage fields to enforce the 0–100 range."

* s.string/number/integer/boolean/null SchemaHelperFactory primitives; call as value or fn(desc)
* s.int alias for s.integer; s.nonEmptyString string with minLength:1; s.url string with format:"uri"; s.path string with format:"path"; s.date string with format:"date" validated as YYYY-MM-DD
* s.positiveInt integer with minimum:1; s.nonNegativeInt integer with minimum:0; NumberSchema/IntegerSchema support minimum/maximum constraints
* s.positiveInt integer with minimum:1; s.nonNegativeInt integer with minimum:0; s.percent number with minimum:0,maximum:100; NumberSchema/IntegerSchema support minimum/maximum constraints
* s.array(items,desc?) ArraySchema; use for homogeneous lists, e.g. s.array(s.string)
* s.nonEmptyArray(items,desc?) ArraySchema with minItems:1; validates array has at least one element
* s.object(props,desc?) ObjectSchema; s.optional(inner) marks field optional; s.nullable(inner) accepts inner|null; use for fixed-key shapes
Expand Down Expand Up @@ -253,6 +253,8 @@ export const s = {
positiveInt: createConstrainedNumberSchema<IntegerSchema>({ type: "integer", minimum: 1 }),
/** Schema for a non-negative integer (minimum: 0). Call as `s.nonNegativeInt` or `s.nonNegativeInt("description")`. */
nonNegativeInt: createConstrainedNumberSchema<IntegerSchema>({ type: "integer", minimum: 0 }),

@github-actions github-actions Bot Jul 26, 2026

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.

[/codebase-design] The PR description explicitly identifies samples 41 and 43 as the motivation for s.percent, but neither sample was updated to use it. The helper provides value only when callers adopt it — leaving the motivating samples unchanged means the validation gap described in the PR body persists.

💡 What to update

In sample 41 (41-parse-coverage.md):

// Before
lines: s.number,
branches: s.number,

// After
lines: s.percent,
branches: s.percent,

In sample 43 (43-snapshot-test-updater.md): the generated sample doesn't seem to include percentage fields, but the PR description says overallPct: s.number — confirm which sample file contains it and update accordingly.

Additionally, samples 123, 143, and 153 each have overallPct: s.number and coverageByCategory: s.record(s.number) — these are candidates for s.percent and s.record(s.percent) as well.

/** Schema for a percentage value (number in the range 0–100, inclusive). Use instead of `s.number` when the value is a percentage; validates that the number is between 0 and 100. Call as `s.percent` or `s.percent("description")`. */
percent: createConstrainedNumberSchema<NumberSchema>({ type: "number", minimum: 0, maximum: 100 }),
/** Schema for a `boolean` value. Call as `s.boolean` or `s.boolean("description")`. */
boolean: createTypedPrimitiveSchema<BooleanSchema>("boolean"),
/** Schema for the JSON `null` literal. Call as `s.null` or `s.null("description")`. */
Expand Down
20 changes: 20 additions & 0 deletions src/rig.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1536,6 +1536,26 @@ describe("s.nonNegativeInt", () => {
});
});

describe("s.percent", () => {
it("serializes to {type:'number', minimum:0, maximum:100}", () => {
expect(toJsonSchema(s.percent)).toEqual({ type: "number", minimum: 0, maximum: 100 });
expect(toJsonSchema(s.percent("coverage percentage"))).toEqual({ type: "number", minimum: 0, maximum: 100, description: "coverage percentage" });
});

it("accepts numbers in range [0, 100]", () => {
expect(analyzeResponse(JSON.stringify(0), s.percent, "test", 1).ok).toBe(true);
expect(analyzeResponse(JSON.stringify(50.5), s.percent, "test", 1).ok).toBe(true);
expect(analyzeResponse(JSON.stringify(100), s.percent, "test", 1).ok).toBe(true);
});

it("rejects numbers outside [0, 100]", () => {
const below = analyzeResponse(JSON.stringify(-1), s.percent, "test", 1);
expect(below.ok).toBe(false);
const above = analyzeResponse(JSON.stringify(101), s.percent, "test", 1);
expect(above.ok).toBe(false);
});
});

@github-actions github-actions Bot Jul 26, 2026

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 rejection tests use integers only (-1, 101). Adding fractional near-boundary values would confirm the JSON Schema maximum/minimum exclusion works precisely.

💡 Suggested additions 
expect(analyzeResponse(JSON.stringify(100.001), s.percent, "test", 1).ok).toBe(false);
expect(analyzeResponse(JSON.stringify(-0.001), s.percent, "test", 1).ok).toBe(false);

JSON Schema minimum/maximum are inclusive, so these verify the validator does not drift at float precision.

describe("NumberSchema and IntegerSchema minimum/maximum", () => {
it("serializes minimum and maximum on number schema", () => {
const schema: import("rig").NumberSchema = { type: "number", minimum: 0, maximum: 1 };
Expand Down