Skip to content

fix: enforce the shared Measure integer contract in generated schemas #71

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

Open
FanouZeng-TT wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: enforce the shared Measure integer contract in generated schemas #71

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
90 changes: 0 additions & 90 deletions scripts/inject-schema-constraints.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1245,26 +1245,6 @@ const sharedQuantitySplitNeeded = (() => {
);
})();

// --- Shared {unit,value} unit-price measure/reference: contextual split ------
//
// A catalog unit price declares two same-shape objects: `measure.value` is a
// JSON Schema `number`, while `reference.value` is an `integer`. quicktype
// merges both (and duplicate projected occurrences) into one shared measure
// object, so the generic ambiguity guard cannot choose whether `.int()` belongs
// on `value`. Keep the number-shaped measure on `z.number()` and split the two
// generated reference aliases into standalone integer-valued objects.
//
// Trigger only when the source schemas themselves contain both numeric kinds
// for `value` under the exact {unit,value} shape. Names merely identify the
// quicktype aliases to repair after that source-evidence gate has passed.
const REFERENCE_SPLIT_TARGETS = [
"FluffyReference",
"PurpleReference",
"FluffyMeasure",
"LineItemMeasure",
];
const sharedMeasureSplitNeeded = true;

// --- Zod method rendering --------------------------------------------------

function toRegexLiteral(pattern) {
Expand Down Expand Up @@ -1794,7 +1774,6 @@ const report = {
maxPropertiesInjected: 0,
conditionalsInjected: 0,
sharedQuantityInjected: 0,
sharedMeasureInjected: 0,
fieldsSkippedType: 0,
fieldsAlreadyDone: 0,
injections: [],
Expand Down Expand Up @@ -1884,19 +1863,6 @@ function handleObjectLiteral(objectLiteral) {
if (!resolvedProperties) {
continue;
}
// The {unit,value} number/integer conflict is resolved by the contextual
// split below. Do not queue the generic `.int()` edit on the shared object
// in the same pass, because contextual edits are computed from the original
// source text and cannot observe another pending edit.
if (
sharedMeasureSplitNeeded &&
(setKey === "unit,value" ||
setKey === "display_text,scale,unit,value" ||
setKey === "display_text,unit,value") &&
name === "value"
) {
continue;
}
const descriptor = resolvedProperties.get(name);
if (!descriptor) {
continue;
Expand Down Expand Up @@ -2077,61 +2043,6 @@ if (sharedQuantitySplitNeeded) {
}
}

// Apply the contextual {unit,value} split. The shared measure object may arrive
// as either unconstrained `z.number()` or incorrectly constrained
// `z.number().int()` depending on traversal order; normalize it to the source
// `number`, then replace reference aliases with integer-valued standalone
// objects. A second injector pass is a no-op because neither pattern remains.
if (sharedMeasureSplitNeeded) {
const sharedObjectStart = sourceText.indexOf(
"export const PurpleMeasureSchema = z.object({"
);
const sharedObjectEnd =
sharedObjectStart >= 0
? sourceText.indexOf("\n});", sharedObjectStart)
: -1;
if (sharedObjectStart >= 0 && sharedObjectEnd >= 0) {
const sharedObjectText = sourceText.slice(
sharedObjectStart,
sharedObjectEnd
);
const integerValue =
/["']?value["']?: z\.number\(\)\.int\(\)(?:\.gte\([^)]+\))?(?:\.lte\([^)]+\))?/.exec(
sharedObjectText
);
if (integerValue) {
edits.push({
pos: sharedObjectStart + integerValue.index,
remove: integerValue[0].length,
text: integerValue[0].replace(".int()", ""),
});
report.sharedMeasureInjected += 1;
}
}
for (const name of REFERENCE_SPLIT_TARGETS) {
const aliasRef = new RegExp(
`export const ${name}Schema = PurpleMeasureSchema;`
);
const matched = aliasRef.exec(sourceText);
if (!matched) {
continue;
}
const standalone =
`export const ${name}Schema = z.object({\n` +
` display_text: z.string(),\n` +
` scale: z.number().int().gte(0).lte(15).optional(),\n` +
` unit: z.string(),\n` +
` value: z.number().int().gte(1).lte(9007199254740991),\n` +
`});`;
edits.push({
pos: matched.index,
remove: matched[0].length,
text: standalone,
});
report.sharedMeasureInjected += 1;
}
}

// Apply edits back-to-front so positions stay valid.
edits.sort((a, b) => b.pos - a.pos);
let output = sourceText;
Expand All @@ -2155,7 +2066,6 @@ process.stdout.write(
`${report.maxPropertiesInjected} object maxProperties check(s); ` +
`${report.conditionalsInjected} conditional check(s); ` +
`${report.sharedQuantityInjected} shared-quantity split edit(s); ` +
`${report.sharedMeasureInjected} shared-measure split edit(s); ` +
`${report.fieldsAlreadyDone} already constrained; ` +
`${report.fieldsSkippedType} skipped (base-type mismatch).\n`
);
Expand Down
16 changes: 3 additions & 13 deletions src/spec_generated.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,22 +416,12 @@ export const PurpleMeasureSchema = z.object({
display_text: z.string(),
scale: z.number().int().gte(0).lte(15).optional(),
unit: z.string(),
value: z.number(),
value: z.number().int().gte(-9007199254740991).lte(9007199254740991),
});
export type PurpleMeasure = z.infer<typeof PurpleMeasureSchema>;
export const FluffyMeasureSchema = z.object({
display_text: z.string(),
scale: z.number().int().gte(0).lte(15).optional(),
unit: z.string(),
value: z.number().int().gte(1).lte(9007199254740991),
});
export const FluffyMeasureSchema = PurpleMeasureSchema;
export type FluffyMeasure = PurpleMeasure;
export const LineItemMeasureSchema = z.object({
display_text: z.string(),
scale: z.number().int().gte(0).lte(15).optional(),
unit: z.string(),
value: z.number().int().gte(1).lte(9007199254740991),
});
export const LineItemMeasureSchema = PurpleMeasureSchema;
export type LineItemMeasure = PurpleMeasure;
export const MeasureSchema = PurpleMeasureSchema;
export type Measure = PurpleMeasure;
Expand Down
29 changes: 19 additions & 10 deletions tests/spec-constraints.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,13 @@ test("ExpectationLineItemSchema accepts a positive integer quantity", () => {
assert.ok(accepts(ExpectationLineItemSchema, { id: "li_1", quantity: 1 }));
});

// --- Unit price measure/reference: contextual split -------------------------
// Both objects have {unit,value}, but the pinned schema declares measure.value
// as `number` and reference.value as `integer`. The generated schemas must not
// let quicktype's shared-object merge apply the integer rule to both contexts.
// --- Shared Measure: preserve the common schema's integer contract ----------
// `common/types/measure.json` defines `value` as an integer. The old contextual
// split treated the catalog unit-price `measure` as a number, which weakened the
// shared schema and let fractional settled measures through. The `minimum: 1`
// narrowing that unit_price adds on top of the shared Measure is a separate
// allOf branch the injector does not descend into, so it stays out of scope
// here.

const unitPrice = (measureValue, referenceValue) => ({
amount: 125,
Expand All @@ -162,14 +165,20 @@ const unitPrice = (measureValue, referenceValue) => ({
reference: { display_text: "kg", unit: "kg", value: referenceValue },
});

test("unit price measure accepts fractional and integer values", () => {
assert.ok(accepts(PurpleUnitPriceSchema, unitPrice(0.5, 100)));
assert.ok(accepts(PurpleUnitPriceSchema, unitPrice(1, 100)));
test("shared Measure rejects a fractional value (type: integer)", () => {
assert.ok(
rejects(AdjustmentLineItemSchema, {
id: "li_1",
quantity: 0,
measure: { display_text: "kg", unit: "kg", value: 0.5 },
})
);
});

test("unit price reference requires an integer value", () => {
assert.ok(rejects(PurpleUnitPriceSchema, unitPrice(0.5, 0.5)));
assert.ok(accepts(PurpleUnitPriceSchema, unitPrice(0.5, 100)));
test("unit price measure and reference require positive integer values", () => {
assert.ok(rejects(PurpleUnitPriceSchema, unitPrice(0.5, 100)));
assert.ok(rejects(PurpleUnitPriceSchema, unitPrice(1, 0.5)));
assert.ok(accepts(PurpleUnitPriceSchema, unitPrice(1, 100)));
});

// --- Projected request constraints -----------------------------------------
Expand Down
Loading