From 9d7fa102ea9524be22580b092e9c084c11f52194 Mon Sep 17 00:00:00 2001 From: margaretjgu Date: Fri, 8 May 2026 14:34:58 -0400 Subject: [PATCH] feat(es): group top-level commands by domain in help output (#221) Applies Commander.js helpGroup() to organize elastic stack es --help into named sections (Documents, Search, Analysis, Scripts, Cluster, Advanced, API namespaces, Helpers) instead of a flat alphabetical list. Group priority controls section order so the most-used domains appear first. Applies to both the eager (test/script) and lazy (production) tree builders. Also ports OOM fix from feat/bulk-ingest-csv: use sequential loadAllEsApis and spawn a dedicated child process for the schema-serialization validation test instead of competing with the test-runner heap. --- scripts/validate-es-schemas.mts | 21 ++++++ src/es/apis.ts | 12 +++- src/es/register.ts | 119 +++++++++++++++++++++++++++++--- test/es/register.test.ts | 82 +++++++++++++++++++++- 4 files changed, 217 insertions(+), 17 deletions(-) create mode 100644 scripts/validate-es-schemas.mts diff --git a/scripts/validate-es-schemas.mts b/scripts/validate-es-schemas.mts new file mode 100644 index 00000000..2734d85b --- /dev/null +++ b/scripts/validate-es-schemas.mts @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Validates that every built-in Elasticsearch API schema can be registered + * without throwing (i.e., all Zod types are JSON-Schema-serializable). + * + * Run via the unit tests in a dedicated child process so the ~5 GB of Zod + * schema closures don't compete with the shared test-runner heap. + * + * Usage: + * node --max-old-space-size=6144 --import tsx/esm scripts/validate-es-schemas.mts + */ + +import { loadAllEsApis } from '../src/es/apis.ts' +import { registerEsCommands } from '../src/es/register.ts' + +const allApis = await loadAllEsApis() +registerEsCommands(allApis) diff --git a/src/es/apis.ts b/src/es/apis.ts index 84d2215f..a9b773f8 100644 --- a/src/es/apis.ts +++ b/src/es/apis.ts @@ -77,7 +77,13 @@ export async function loadEsApi (meta: EsApiMeta): Promise { * startup path stays on the manifest + `loadEsApi()`. */ export async function loadAllEsApis (): Promise { - const files = new Set(apiManifest.map((m) => m.namespaceFile)) - const loaded = await Promise.all([...files].map(loadEsApisInFile)) - return loaded.flat() + // Load namespace files sequentially rather than concurrently to keep peak heap + // usage bounded. Concurrent loading via Promise.all compiles all ~40 modules at + // once, which can exceed 4 GB on tight heap environments (e.g. V8 on Apple Silicon). + const files = [...new Set(apiManifest.map((m) => m.namespaceFile))] + const results: EsApiDefinition[][] = [] + for (const file of files) { + results.push(await loadEsApisInFile(file)) + } + return results.flat() } diff --git a/src/es/register.ts b/src/es/register.ts index c3dc4a17..72a35428 100644 --- a/src/es/register.ts +++ b/src/es/register.ts @@ -15,6 +15,86 @@ import type { EsApiMeta } from './api-manifest.ts' import { createEsHandler } from './handler.ts' import { registerHelperCommands } from './helpers/register.ts' +/** + * Maps root-level (no-namespace) command names to the help section they belong to. + * Commands not listed here fall into the catch-all "Other commands" group. + * Order within each group reflects usage frequency — most-common first. + */ +const ROOT_COMMAND_GROUPS: Record = { + // Documents + get: 'Documents', + index: 'Documents', + create: 'Documents', + update: 'Documents', + delete: 'Documents', + bulk: 'Documents', + mget: 'Documents', + exists: 'Documents', + 'exists-source': 'Documents', + 'get-source': 'Documents', + 'delete-by-query': 'Documents', + 'update-by-query': 'Documents', + reindex: 'Documents', + // Search + search: 'Search', + msearch: 'Search', + 'search-template': 'Search', + 'msearch-template': 'Search', + scroll: 'Search', + 'clear-scroll': 'Search', + 'open-point-in-time': 'Search', + 'close-point-in-time': 'Search', + 'search-mvt': 'Search', + 'search-shards': 'Search', + 'render-search-template': 'Search', + // Analysis + count: 'Analysis', + explain: 'Analysis', + 'field-caps': 'Analysis', + termvectors: 'Analysis', + mtermvectors: 'Analysis', + 'rank-eval': 'Analysis', + 'terms-enum': 'Analysis', + // Scripts + 'get-script': 'Scripts', + 'put-script': 'Scripts', + 'delete-script': 'Scripts', + 'scripts-painless-execute': 'Scripts', + 'get-script-context': 'Scripts', + 'get-script-languages': 'Scripts', + // Cluster + ping: 'Cluster', + info: 'Cluster', + 'health-report': 'Cluster', + // Throttle / admin — shown but deprioritised + 'delete-by-query-rethrottle': 'Advanced', + 'update-by-query-rethrottle': 'Advanced', + 'reindex-rethrottle': 'Advanced', +} + +/** Group label applied to every namespace sub-tree (cat, cluster, indices, …). */ +const NAMESPACE_GROUP = 'API namespaces' + +/** + * Controls the display order of root-level sections in help output. + * Lower numbers appear first. Sections not listed here sort after all listed ones. + */ +const GROUP_PRIORITY: Record = { + Documents: 0, + Search: 1, + Analysis: 2, + Scripts: 3, + Cluster: 4, + Advanced: 5, + 'Other commands': 6, +} + +/** Applies a help-section heading to a command handle (no-op if already set). */ +function applyHelpGroup (handle: OpaqueCommandHandle, group: string): OpaqueCommandHandle { + handle.helpGroup(group) + return handle +} + /** Builds a leaf command handle from an eagerly-available definition and its pre-computed schema args. */ function buildLeafHandle ( def: EsApiDefinition, @@ -189,21 +269,30 @@ function buildEagerTree (definitions: EsApiDefinition[]): OpaqueCommandHandle { } const leafHandles = defs.map((def) => buildLeafHandle(def, defSchemaArgs)) - namespaceHandles.push( - defineGroup({ name: namespace, description: `Elasticsearch ${namespace} API commands` }, ...leafHandles) - ) + const nsHandle = defineGroup({ name: namespace, description: `Elasticsearch ${namespace} API commands` }, ...leafHandles) + applyHelpGroup(nsHandle, NAMESPACE_GROUP) + namespaceHandles.push(nsHandle) } + rootDefs.sort((a, b) => { + const pa = GROUP_PRIORITY[ROOT_COMMAND_GROUPS[a.name] ?? 'Other commands'] ?? 99 + const pb = GROUP_PRIORITY[ROOT_COMMAND_GROUPS[b.name] ?? 'Other commands'] ?? 99 + return pa - pb || a.name.localeCompare(b.name) + }) + const rootHandles: OpaqueCommandHandle[] = [] for (const def of rootDefs) { if (topLevelNames.has(def.name)) { throw new Error(`duplicate command name "${def.name}" at the top level of es`) } topLevelNames.add(def.name) - rootHandles.push(buildLeafHandle(def, defSchemaArgs)) + const h = buildLeafHandle(def, defSchemaArgs) + applyHelpGroup(h, ROOT_COMMAND_GROUPS[def.name] ?? 'Other commands') + rootHandles.push(h) } const helpersGroup = registerHelperCommands() + applyHelpGroup(helpersGroup, 'Helpers') return defineGroup({ name: 'es', description: 'Interact with the Elasticsearch API' }, ...namespaceHandles, ...rootHandles, helpersGroup) } @@ -271,9 +360,9 @@ async function buildLazyTree (manifest: readonly EsApiMeta[], argv: readonly str // This keeps Commander object count proportional to the invoked namespace // size (worst case ~70 stubs) rather than the total manifest size (~560). if (namespace !== invokedNamespace) { - namespaceHandles.push( - defineGroup({ name: namespace, description: `Elasticsearch ${namespace} API commands` }) - ) + const stubHandle = defineGroup({ name: namespace, description: `Elasticsearch ${namespace} API commands` }) + applyHelpGroup(stubHandle, NAMESPACE_GROUP) + namespaceHandles.push(stubHandle) continue } @@ -286,25 +375,33 @@ async function buildLazyTree (manifest: readonly EsApiMeta[], argv: readonly str } const leafHandles = metas.map(leafHandleFor) - namespaceHandles.push( - defineGroup({ name: namespace, description: `Elasticsearch ${namespace} API commands` }, ...leafHandles) - ) + const nsHandle = defineGroup({ name: namespace, description: `Elasticsearch ${namespace} API commands` }, ...leafHandles) + applyHelpGroup(nsHandle, NAMESPACE_GROUP) + namespaceHandles.push(nsHandle) } // Root-level commands: only build stubs when the user targets root-level. // When a namespace is targeted, root stubs are skipped entirely. const rootHandles: OpaqueCommandHandle[] = [] if (invokedNamespace == null || !byNamespace.has(invokedNamespace)) { + rootMetas.sort((a, b) => { + const pa = GROUP_PRIORITY[ROOT_COMMAND_GROUPS[a.name] ?? 'Other commands'] ?? 99 + const pb = GROUP_PRIORITY[ROOT_COMMAND_GROUPS[b.name] ?? 'Other commands'] ?? 99 + return pa - pb || a.name.localeCompare(b.name) + }) for (const m of rootMetas) { if (topLevelNames.has(m.name)) { throw new Error(`duplicate command name "${m.name}" at the top level of es`) } topLevelNames.add(m.name) - rootHandles.push(leafHandleFor(m)) + const h = leafHandleFor(m) + applyHelpGroup(h, ROOT_COMMAND_GROUPS[m.name] ?? 'Other commands') + rootHandles.push(h) } } const helpersGroup = registerHelperCommands() + applyHelpGroup(helpersGroup, 'Helpers') return defineGroup({ name: 'es', description: 'Interact with the Elasticsearch API' }, ...namespaceHandles, ...rootHandles, helpersGroup) } diff --git a/test/es/register.test.ts b/test/es/register.test.ts index 95297596..58137844 100644 --- a/test/es/register.test.ts +++ b/test/es/register.test.ts @@ -345,11 +345,87 @@ describe('registerEsCommands - external schema consumption', () => { }) }) +describe('registerEsCommands - help groups', () => { + it('namespace commands belong to the "API namespaces" group', () => { + const defs: EsApiDefinition[] = [makeDef('health', 'cat'), makeDef('create', 'indices')] + const handle = registerEsCommands(defs) + const cat = handle.commands.find((c) => c.name() === 'cat') + assert.ok(cat != null) + assert.equal(cat.helpGroup(), 'API namespaces') + const idx = handle.commands.find((c) => c.name() === 'indices') + assert.ok(idx != null) + assert.equal(idx.helpGroup(), 'API namespaces') + }) + + it('helpers group belongs to the "Helpers" group', () => { + const handle = registerEsCommands([]) + const helpers = handle.commands.find((c) => c.name() === 'helpers') + assert.ok(helpers != null) + assert.equal(helpers.helpGroup(), 'Helpers') + }) + + it('known root-level commands are assigned to their domain group', () => { + const defs: EsApiDefinition[] = [ + makeRootDef('search'), + makeRootDef('get'), + makeRootDef('count'), + makeRootDef('put-script'), + makeRootDef('ping'), + makeRootDef('reindex-rethrottle'), + ] + const handle = registerEsCommands(defs) + const groupOf = (name: string) => handle.commands.find((c) => c.name() === name)?.helpGroup() + assert.equal(groupOf('search'), 'Search') + assert.equal(groupOf('get'), 'Documents') + assert.equal(groupOf('count'), 'Analysis') + assert.equal(groupOf('put-script'), 'Scripts') + assert.equal(groupOf('ping'), 'Cluster') + assert.equal(groupOf('reindex-rethrottle'), 'Advanced') + }) + + it('unknown root-level commands fall back to "Other commands"', () => { + const defs: EsApiDefinition[] = [makeRootDef('some-new-command')] + const handle = registerEsCommands(defs) + const cmd = handle.commands.find((c) => c.name() === 'some-new-command') + assert.ok(cmd != null) + assert.equal(cmd.helpGroup(), 'Other commands') + }) + + it('root-level commands appear in section order: Documents before Search before Analysis', () => { + const defs: EsApiDefinition[] = [ + makeRootDef('search'), + makeRootDef('count'), + makeRootDef('get'), + ] + const handle = registerEsCommands(defs) + const rootCmds = handle.commands.filter((c) => c.name() !== 'helpers') + const names = rootCmds.map((c) => c.name()) + assert.ok(names.indexOf('get') < names.indexOf('search'), 'Documents (get) should precede Search (search)') + assert.ok(names.indexOf('search') < names.indexOf('count'), 'Search (search) should precede Analysis (count)') + }) +}) + describe('registerEsCommands - built-in API surface', () => { it('all built-in API schemas are JSON-Schema-serializable', async () => { - const { loadAllEsApis } = await import('../../src/es/apis.ts') - const allApis = await loadAllEsApis() - assert.doesNotThrow(() => registerEsCommands(allApis)) + // Loading all 560 Zod schema modules simultaneously occupies ~5 GB of heap. + // V8 can dynamically lower the effective heap limit under memory pressure even + // when --max-old-space-size is set, causing OOM in the shared test-runner process. + // Spawning a dedicated child process gives the validation a clean 6 GB V8 isolate + // with no competing allocations from the test runner or other test modules. + const { spawnSync } = await import('node:child_process') + const { fileURLToPath } = await import('node:url') + // fileURLToPath handles Windows paths correctly (URL.pathname gives /C:/... on Windows) + const script = fileURLToPath(new URL('../../scripts/validate-es-schemas.mts', import.meta.url)) + // Bun runs this test suite in CI and understands TypeScript natively — no tsx + // loader or --max-old-space-size flag needed. Under Node, both are required. + // Always use process.execPath (guaranteed correct binary) rather than a bare + // 'bun' or 'node' string that depends on PATH resolution in the child env. + const isBun = 'bun' in process.versions + const args = isBun + ? [script] + : ['--max-old-space-size=6144', '--import', 'tsx/esm', script] + const result = spawnSync(process.execPath, args, { encoding: 'utf-8', timeout: 120_000 }) + assert.strictEqual(result.status, 0, `Schema validation failed:\n${result.stderr}`) }) it('throws at registration time when a schema contains z.date()', () => {