From 49cfdd43ed9e1f944840d865afee3bb5f780f486 Mon Sep 17 00:00:00 2001 From: Thomas Berdy Date: Tue, 4 Aug 2026 19:06:10 +0200 Subject: [PATCH] refactor(docs): read documented enum values through one shared reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engine is changing how it publishes per-value documentation for a schema's choice set, moving from an `x-enum-descriptions` map to a positional `x-mergify-enum` array that also carries a display title and a deprecation flag. Schema syncs land as direct pushes to main, so the docs repo cannot assume both sides move together. Introduce `readEnumChoices`, which tolerates every shape a synced schema can be in during that migration: values as a flat `enum`, a `const`, or the `anyOf` of branches a composed `Literal` produces, with the node — or any single branch of it — either inline or behind a `$ref`. Hoisting repeated enums into shared components is what removes the duplicated blocks from the published schema, and a reader that followed only the property node would silently render an empty table once that happens. Three things the shape change makes possible that the map could not, and which are handled explicitly because each fails silently: - Metadata can sit on the referring node as a `$ref` sibling rather than on the target, which is how pydantic publishes an annotation for a hoisted type. Both are consulted, nearest first. - A positional array can fall out of alignment with `enum` in a way a keyed map never could. A length mismatch is treated as unusable rather than shifting every description onto the wrong value. - The two shapes can coexist on one node mid-migration, so they are merged rather than treated as alternatives. Ref resolution uses the `jsonpointer` the sibling readers already use: hand-decoding each segment raises `URIError` on a pointer containing a stray `%`, which would break the never-throws contract this reader depends on to degrade instead of failing the build. The dequeue reason table moves onto it here; the batch status table is written against it from the start in a later commit. Rendered output is unchanged. Part of MRGFY-8330 Co-Authored-By: Claude Opus 5 (1M context) Change-Id: Ie539de06bf3391eb4390333810cda6e8f131b1e6 --- src/components/Tables/QueueDequeueReasons.tsx | 51 ++--- src/util/enumChoices.test.ts | 173 +++++++++++++++ src/util/enumChoices.ts | 207 ++++++++++++++++++ 3 files changed, 393 insertions(+), 38 deletions(-) create mode 100644 src/util/enumChoices.test.ts create mode 100644 src/util/enumChoices.ts diff --git a/src/components/Tables/QueueDequeueReasons.tsx b/src/components/Tables/QueueDequeueReasons.tsx index 0a6045f3fa..ca663b475b 100644 --- a/src/components/Tables/QueueDequeueReasons.tsx +++ b/src/components/Tables/QueueDequeueReasons.tsx @@ -1,17 +1,17 @@ +import { readEnumChoices } from '../../util/enumChoices'; import configSchema from '../../util/sanitizedConfigSchema'; import { renderMarkdown } from './utils'; // The `queue-dequeue-reason` condition attribute accepts one of the merge queue -// dequeue codes. The engine is the single source of truth: the codes come from -// the attribute's enum, and a one-line description for each is published -// alongside it under `x-enum-descriptions` (keyed by the raw code). Driving the -// code list from the enum keeps the table current even before a schema sync -// delivers the descriptions. +// dequeue codes. The engine is the single source of truth: the codes and their +// one-line descriptions are published on the attribute itself, so this table +// stays current without being hand-maintained. // -// `x-enum-descriptions` is not in the synced schema until the engine ships it, -// so it is absent from the JSON-derived types and read via a cast (it becomes a -// normal typed key once the sync bot lands it). +// `readEnumChoices` handles every shape the engine can publish here — a flat +// `enum`, the `anyOf` of `const`/`enum` branches a composed `Literal` produces +// today, or a `$ref` to a shared component — and reads the documentation from +// either `x-mergify-enum` or the older `x-enum-descriptions` map. // // The lookup is optional-chained through a loose cast so a future schema reshape // (renamed attribute or restructured `$defs`) degrades to an empty table rather @@ -22,28 +22,6 @@ const reasonProp: unknown = ( } ).$defs?.PullRequestAttributes?.properties?.['queue-dequeue-reason']; -type EnumNode = { anyOf?: EnumNode[]; enum?: string[]; const?: string }; - -function branchValues(node: EnumNode): string[] { - if (node.const !== undefined) { - return [node.const]; - } - return node.enum ?? []; -} - -// Collect the raw enum values the attribute accepts, tolerating the shapes the -// engine might emit: an `anyOf` of `{const}` / `{enum}` branches (today), or a -// flat `{enum}` / `{const}`. Returns [] for anything else (or a missing node), -// so a future schema-shape change degrades to an empty table rather than -// crashing the Astro build. -function enumValues(prop: unknown): string[] { - if (!prop || typeof prop !== 'object') { - return []; - } - const node = prop as EnumNode; - return node.anyOf ? node.anyOf.flatMap(branchValues) : branchValues(node); -} - // The engine stores codes as `UPPER_SNAKE`; conditions are written in kebab-case // (the parser normalizes `upper().replace('-', '_')`), so that is what to show. function toKebab(code: string): string { @@ -51,10 +29,7 @@ function toKebab(code: string): string { } export default function QueueDequeueReasons() { - const descriptions = - (reasonProp as { 'x-enum-descriptions'?: Record } | undefined)?.[ - 'x-enum-descriptions' - ] ?? {}; + const choices = readEnumChoices(configSchema, reasonProp); return (
@@ -66,12 +41,12 @@ export default function QueueDequeueReasons() { - {enumValues(reasonProp).map((code) => ( - + {choices.map((choice) => ( + - {toKebab(code)} + {toKebab(choice.value)} - + ))} diff --git a/src/util/enumChoices.test.ts b/src/util/enumChoices.test.ts new file mode 100644 index 0000000000..e3a6f57973 --- /dev/null +++ b/src/util/enumChoices.test.ts @@ -0,0 +1,173 @@ +import { describe, expect, it } from 'vitest'; +import { readEnumChoices, resolveRef } from './enumChoices'; + +// This reader spans an engine-side migration, so each shape it has to survive +// is pinned here. Every failure mode below is silent by nature — a shape it +// mishandles renders a header with an empty or subtly wrong table rather than +// failing the build — which is why they are tested rather than left to review. +describe('readEnumChoices', () => { + it('reads the target shape: x-mergify-enum aligned with enum', () => { + expect( + readEnumChoices( + {}, + { + enum: ['running', 'failed'], + 'x-mergify-enum': [ + { title: 'CI Running', description: 'Checks are running.' }, + { title: 'Failed', description: 'Checks failed.', deprecated: true }, + ], + } + ) + ).toEqual([ + { + value: 'running', + title: 'CI Running', + description: 'Checks are running.', + deprecated: false, + }, + { value: 'failed', title: 'Failed', description: 'Checks failed.', deprecated: true }, + ]); + }); + + it('falls back to the x-enum-descriptions map while the engine migrates', () => { + expect( + readEnumChoices( + {}, + { enum: ['a', 'b'], 'x-enum-descriptions': { a: 'First.', b: 'Second.' } } + ) + ).toEqual([ + { value: 'a', title: undefined, description: 'First.', deprecated: false }, + { value: 'b', title: undefined, description: 'Second.', deprecated: false }, + ]); + }); + + it('merges the two shapes so a half-migrated node keeps every description', () => { + // Both shapes can coexist on one node mid-migration; treating them as + // alternatives would blank values the schema still documents. + const choices = readEnumChoices( + {}, + { + enum: ['a', 'b', 'c'], + 'x-mergify-enum': [{ description: 'A new' }, {}, {}], + 'x-enum-descriptions': { a: 'A old', b: 'B old', c: 'C old' }, + } + ); + expect(choices.map((c) => c.description)).toEqual(['A new', 'B old', 'C old']); + }); + + it('flattens a composed Literal published as anyOf of const/enum branches', () => { + expect( + readEnumChoices( + {}, + { + anyOf: [{ const: 'NONE' }, { enum: ['MERGED', 'DEQUEUED'] }], + 'x-enum-descriptions': { NONE: 'Not queued.' }, + } + ).map((c) => c.value) + ).toEqual(['NONE', 'MERGED', 'DEQUEUED']); + }); + + it('resolves a $ref to a hoisted component', () => { + const root = { + components: { + schemas: { + Outcome: { + enum: ['success'], + 'x-mergify-enum': [{ title: 'Success', description: 'It passed.' }], + }, + }, + }, + }; + expect(readEnumChoices(root, { $ref: '#/components/schemas/Outcome' })).toEqual([ + { value: 'success', title: 'Success', description: 'It passed.', deprecated: false }, + ]); + }); + + it('resolves a $ref sitting inside an anyOf branch', () => { + // What an optional hoisted enum looks like: {anyOf: [{$ref}, {type: null}]}. + // Resolving only the top node would yield nothing at all. + const root = { + $defs: { + Reason: { + enum: ['A', 'B'], + 'x-mergify-enum': [{ description: 'a' }, { description: 'b' }], + }, + }, + }; + const choices = readEnumChoices(root, { + anyOf: [{ $ref: '#/$defs/Reason' }, { type: 'null' }], + }); + expect(choices.map((c) => c.value)).toEqual(['A', 'B']); + expect(choices.map((c) => c.description)).toEqual(['a', 'b']); + }); + + it('reads metadata published as a $ref sibling, not only on the target', () => { + // Pydantic publishes an annotation inline for an inlined type but as a + // sibling of `$ref` once the type is hoisted into `$defs`. + const root = { $defs: { Reason: { enum: ['A', 'B'] } } }; + const choices = readEnumChoices(root, { + $ref: '#/$defs/Reason', + 'x-mergify-enum': [{ description: 'first' }, { description: 'second' }], + }); + expect(choices.map((c) => c.description)).toEqual(['first', 'second']); + }); + + it('ignores a misaligned x-mergify-enum rather than shifting every description', () => { + // Positional metadata whose length disagrees with `enum` describes the + // wrong values from the first divergence onward. Publishing nothing beats + // publishing confidently wrong sentences. + const choices = readEnumChoices( + {}, + { + enum: ['b', 'c'], + 'x-mergify-enum': [ + { description: 'desc for a' }, + { description: 'desc for b' }, + { description: 'desc for c' }, + ], + } + ); + expect(choices.map((c) => c.description)).toEqual(['', '']); + }); + + it('leaves values undocumented rather than dropping them', () => { + const choices = readEnumChoices( + {}, + { enum: ['a', 'b'], 'x-mergify-enum': [{ title: 'A' }, {}] } + ); + expect(choices.map((c) => c.value)).toEqual(['a', 'b']); + expect(choices.map((c) => c.description)).toEqual(['', '']); + }); + + it('reads a single-value choice set published as a non-string const', () => { + // A one-value literal publishes `{const: 1}` where a two-value one + // publishes `{enum: [1, 2]}`; accepting only string consts would render + // the second and silently drop the first. + expect(readEnumChoices({}, { const: 1, 'x-mergify-enum': [{ description: 'one' }] })).toEqual([ + { value: '1', title: undefined, description: 'one', deprecated: false }, + ]); + }); + + it('degrades to an empty list instead of throwing on unusable input', () => { + expect(readEnumChoices({}, undefined)).toEqual([]); + expect(readEnumChoices({}, { type: 'string' })).toEqual([]); + expect(readEnumChoices({}, { $ref: '#/nope/missing' })).toEqual([]); + }); +}); + +describe('resolveRef', () => { + it('stops on a dangling ref rather than looping or throwing', () => { + expect(resolveRef({}, { $ref: '#/a/b' })).toEqual({ $ref: '#/a/b' }); + }); + + it('does not throw on a pointer containing a stray percent sign', () => { + // Hand-rolled `decodeURIComponent` on each segment raises URIError here, + // which would break the never-throws contract during SSR. + const root = { components: { schemas: { 'A%B': { enum: ['x'] } } } }; + expect(() => resolveRef(root, { $ref: '#/components/schemas/A%B' })).not.toThrow(); + }); + + it('returns non-ref nodes untouched', () => { + expect(resolveRef({}, { enum: ['x'] })).toEqual({ enum: ['x'] }); + }); +}); diff --git a/src/util/enumChoices.ts b/src/util/enumChoices.ts new file mode 100644 index 0000000000..cad8333ea7 --- /dev/null +++ b/src/util/enumChoices.ts @@ -0,0 +1,207 @@ +import jsonpointer from 'jsonpointer'; + +// Reading the documented values of a schema "choice set" (an enum and its +// per-value documentation). +// +// The engine is migrating how it publishes that documentation, so this reader +// deliberately understands every shape a synced schema can currently be in. +// Schema syncs land as direct pushes to main, so the docs repo cannot assume +// the engine side has migrated yet — and both shapes may coexist across the +// configuration schema and the OpenAPI spec for a while. +// +// Metadata shapes, which are *merged* rather than treated as alternatives so a +// half-migrated node keeps rendering every description it publishes: +// - `x-mergify-enum`: a positional array of `{title, description, deprecated}` +// aligned with `enum`. The target shape. +// - `x-enum-descriptions`: a map of raw value -> description sentence. The +// previous shape; carries no per-value title or deprecation. +// +// The values come from `enum`, from a `const`, or from an `anyOf`/`oneOf` of +// such branches — the shape a composed `Literal` (`Literal["A"] | OtherT`) +// produces. Branches are `$ref`-resolved too: hoisting a repeated enum into a +// shared component leaves `{anyOf: [{$ref: ...}, {type: "null"}]}`, and a +// reader that only resolved the top node would silently render nothing. + +export interface EnumChoice { + value: string; + /** Display label, when the schema publishes one. */ + title?: string; + /** May be empty: a value can be published before it is documented. */ + description: string; + deprecated: boolean; +} + +interface SchemaNode { + $ref?: unknown; + enum?: unknown; + const?: unknown; + anyOf?: unknown; + oneOf?: unknown; + 'x-mergify-enum'?: unknown; + 'x-enum-descriptions'?: unknown; +} + +const MAX_REF_HOPS = 10; + +function isObject(value: unknown): value is Record { + return !!value && typeof value === 'object' && !Array.isArray(value); +} + +/** + * Follow a `$ref` chain from `node` within `root`, returning the node reached. + * Stops on a dangling, malformed or cyclic ref and returns what it has, so a + * schema reshape degrades to an empty table rather than throwing during the + * Astro build. + * + * Uses the same `jsonpointer` the other schema readers use (ConfigOptions, + * schemaToMarkdown) rather than splitting and decoding by hand: a pointer + * containing a stray `%` makes `decodeURIComponent` raise, which would break + * the never-throws contract this function advertises. + */ +export function resolveRef(root: unknown, node: unknown): unknown { + let current = node; + for (let hop = 0; hop < MAX_REF_HOPS; hop++) { + if (!isObject(current)) { + return current; + } + const ref = (current as SchemaNode).$ref; + if (typeof ref !== 'string' || !ref.startsWith('#/')) { + return current; + } + let target: unknown; + try { + target = jsonpointer.get(root as object, ref.slice(1)); + } catch { + return current; + } + if (target === undefined || target === null) { + return current; + } + current = target; + } + return current; +} + +/** + * The raw values a resolved node accepts directly (no branch recursion). + * + * `const` is stringified the same way `enum` entries are: a single-value + * choice set publishes `{const: 1}` where a multi-value one publishes + * `{enum: [1, 2]}`, and accepting only strings would silently drop the + * former while rendering the latter. + */ +function ownValues(node: SchemaNode): string[] { + if (Array.isArray(node.enum)) { + return node.enum.map(String); + } + const single = node.const; + if (single !== undefined && single !== null && typeof single !== 'object') { + return [String(single)]; + } + return []; +} + +/** + * Per-value metadata published on `node` or on any node its `$ref` chain + * passes through. + * + * Pydantic publishes an annotation inline for an inlined type but as a + * *sibling of `$ref`* for a type hoisted into `$defs`, so both the raw node + * and the resolved target have to be consulted — the same walk + * `ConfigOptions.getDataTypeLink` performs for `x-has-data-type`. Nearest wins: + * a sibling on the referring node overrides the shared component. + */ +function collectMetadata(root: unknown, node: unknown): SchemaNode { + const merged: SchemaNode = {}; + let current = node; + for (let hop = 0; hop < MAX_REF_HOPS && isObject(current); hop++) { + const schema = current as SchemaNode; + if (merged['x-mergify-enum'] === undefined && Array.isArray(schema['x-mergify-enum'])) { + merged['x-mergify-enum'] = schema['x-mergify-enum']; + } + if (merged['x-enum-descriptions'] === undefined && isObject(schema['x-enum-descriptions'])) { + merged['x-enum-descriptions'] = schema['x-enum-descriptions']; + } + const ref = schema.$ref; + if (typeof ref !== 'string' || !ref.startsWith('#/')) { + break; + } + let next: unknown; + try { + next = jsonpointer.get(root as object, ref.slice(1)); + } catch { + break; + } + if (next === undefined || next === null) { + break; + } + current = next; + } + return merged; +} + +/** + * The documented choices of `node` (which may be a `$ref` into `root`). + * Returns [] for anything that is not a choice set. + * + * Branch unions are flattened by concatenation, and each branch supplies the + * metadata for its own values — so a hoisted enum keeps its descriptions + * whether the `$ref` sits at the top of the node or inside one of its + * branches. + */ +export function readEnumChoices(root: unknown, node: unknown): EnumChoice[] { + return read(root, node, {}); +} + +function read(root: unknown, node: unknown, inherited: SchemaNode): EnumChoice[] { + const resolved = resolveRef(root, node); + if (!isObject(resolved)) { + return []; + } + + // Metadata on this node wins over anything inherited from an enclosing + // union. The engine publishes the annotations at the top of an optional + // node while the values sit in its non-null branch, so a branch with no + // metadata of its own must still see the parent's. + const own = collectMetadata(root, node); + const meta: SchemaNode = { + 'x-mergify-enum': own['x-mergify-enum'] ?? inherited['x-mergify-enum'], + 'x-enum-descriptions': own['x-enum-descriptions'] ?? inherited['x-enum-descriptions'], + }; + + const values = ownValues(resolved as SchemaNode); + if (values.length === 0) { + const branches = (resolved as SchemaNode).anyOf ?? (resolved as SchemaNode).oneOf; + if (Array.isArray(branches)) { + return branches.flatMap((branch) => read(root, branch, meta)); + } + return []; + } + + const entries = meta['x-mergify-enum']; + // `x-mergify-enum` is positional, so a length mismatch means every entry + // after the first divergence describes the wrong value. Publishing 40 subtly + // wrong sentences is worse than publishing none, and the misalignment is + // otherwise undetectable — the map shape this replaced could not drift. + const aligned = Array.isArray(entries) && entries.length === values.length ? entries : undefined; + + const legacy = meta['x-enum-descriptions']; + const descriptions = isObject(legacy) ? legacy : {}; + + return values.map((value, index) => { + const entry = aligned?.[index]; + const positional = isObject(entry) ? entry : {}; + const fallback = descriptions[value]; + return { + value, + title: typeof positional.title === 'string' ? positional.title : undefined, + description: + typeof positional.description === 'string' && positional.description !== '' + ? positional.description + : typeof fallback === 'string' + ? fallback + : '', + deprecated: positional.deprecated === true, + }; + }); +}