mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-27 07:55:07 +00:00
fix(schema): warn that only a subset of draft 2020-12 is enforced; cover the gaps it cannot
Both review findings taken. MED — the schema declared `$schema: draft/2020-12`, which promises full draft semantics, while the validator that actually runs it enforces only a subset. Someone adding `"minimum": 1` to contextWindow would get SILENT no-op with a green suite — a trap made worse by the file looking authoritative. The description now enumerates exactly what is enforced (type, required, const, enum, additionalProperties in both forms, items, minItems/maxItems, anyOf/allOf/oneOf, $ref, nullable) and names what is ignored (minimum, maxLength, pattern, uniqueItems, propertyNames, not), with the instruction to add such a constraint as a test instead. LOW — three corruptions neither the schema nor any sibling test caught: duplicate models[].id, empty-string id/displayName/openclawName, and non-positive contextWindow/maxTokens. They are structurally NOT expressible with the supported keyword set (no uniqueItems, no minLength, no minimum), so they are asserted directly rather than by pretending the schema covers them. Mutation-proven, all three previously slipped through: duplicate id -> CAUGHT empty displayName -> CAUGHT zero contextWindow -> CAUGHT Tests: 461 passed, 0 failed (460 + 1). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017gbqUZ8HfBZpjjbzQ85oH8
This commit is contained in:
+1
-1
@@ -2,7 +2,7 @@
|
|||||||
"$schema": "https://json-schema.org/draft/2020-12/schema",
|
"$schema": "https://json-schema.org/draft/2020-12/schema",
|
||||||
"$id": "https://github.com/dtzp555-max/ocp/blob/main/models.schema.json",
|
"$id": "https://github.com/dtzp555-max/ocp/blob/main/models.schema.json",
|
||||||
"title": "OCP models.json",
|
"title": "OCP models.json",
|
||||||
"description": "Schema for models.json, the single source of truth for model metadata (ADR 0003). Enforced in CI by test-features.mjs, which validates models.json against this file using the repo's own validateJsonSchema (lib/structured-output.mjs) — no new dependency. NOTE: referential integrity (every aliases/legacyAliases target must exist as a models[].id) is NOT expressible here and is covered by separate tests.",
|
"description": "Schema for models.json, the single source of truth for model metadata (ADR 0003). Enforced in CI by test-features.mjs, which validates models.json against this file using the repo's own validateJsonSchema (lib/structured-output.mjs) — no new dependency. ⚠ ONLY A SUBSET OF DRAFT 2020-12 IS ENFORCED: type, required, const, enum, additionalProperties (boolean and schema forms), items, minItems, maxItems, anyOf/allOf/oneOf, $ref, nullable. Anything else — minimum, maxLength, pattern, uniqueItems, propertyNames, not — is SILENTLY IGNORED by that validator, so adding one here buys nothing and misleads the next reader. Add such a constraint as a test in test-features.mjs instead. NOTE: referential integrity (every aliases/legacyAliases target must exist as a models[].id) is likewise not expressible here and is covered by separate tests.",
|
||||||
"type": "object",
|
"type": "object",
|
||||||
"required": ["version", "models", "aliases", "legacyAliases"],
|
"required": ["version", "models", "aliases", "legacyAliases"],
|
||||||
"additionalProperties": false,
|
"additionalProperties": false,
|
||||||
|
|||||||
@@ -4330,6 +4330,21 @@ test("models.json validates against models.schema.json (strict)", () => {
|
|||||||
assert.deepEqual(errors, [], `models.json violates its own schema:\n ${errors.join("\n ")}`);
|
assert.deepEqual(errors, [], `models.json violates its own schema:\n ${errors.join("\n ")}`);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Three corruptions the SCHEMA structurally cannot catch, asserted directly instead of pretending
|
||||||
|
// the schema covers them: the validator has no uniqueItems, no minLength, and no minimum. Adding
|
||||||
|
// those keywords to the schema would be silently ignored (see its description), so they live here.
|
||||||
|
test("models.json: ids are unique, non-empty, and windows are positive (not schema-expressible)", () => {
|
||||||
|
const ids = _spotModels.models.map(m => m.id);
|
||||||
|
assert.equal(new Set(ids).size, ids.length, `duplicate models[].id: ${ids.filter((v, i) => ids.indexOf(v) !== i)}`);
|
||||||
|
for (const m of _spotModels.models) {
|
||||||
|
for (const f of ["id", "displayName", "openclawName"]) {
|
||||||
|
assert.ok(typeof m[f] === "string" && m[f].trim().length > 0, `${m.id}: ${f} must be a non-empty string`);
|
||||||
|
}
|
||||||
|
assert.ok(m.contextWindow > 0, `${m.id}: contextWindow must be positive`);
|
||||||
|
assert.ok(m.maxTokens > 0, `${m.id}: maxTokens must be positive`);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
// Guards the guard: if the schema were vacuous (e.g. `{}` or a typo'd `properties`), the test
|
// Guards the guard: if the schema were vacuous (e.g. `{}` or a typo'd `properties`), the test
|
||||||
// above would pass on anything. Each corruption below must be caught.
|
// above would pass on anything. Each corruption below must be caught.
|
||||||
test("models.schema.json actually rejects malformed entries (guard is not vacuous)", () => {
|
test("models.schema.json actually rejects malformed entries (guard is not vacuous)", () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user