From f219ba22a61e68fa1bcda3dfeb921955aba36aa4 Mon Sep 17 00:00:00 2001 From: Luca Martial <48870843+luca-martial@users.noreply.github.com> Date: Wed, 13 May 2026 17:28:48 -0400 Subject: [PATCH 1/5] feat(cli): redesign Notion page picker UI and add skip-empty flow (#78) Rework the inline picker to use a cleaner visual style (filled/empty square glyphs, bordered layout, clack-style header) and streamlined keybindings (Enter to confirm, Escape to quit, Right Arrow to expand). Replace the transient "select at least one" hint with a skip-empty confirmation prompt that exits cleanly via quit-without-save. Co-authored-by: Claude Opus 4.6 (1M context) --- .../cli/src/notion-page-picker-tree.test.ts | 25 ++--- packages/cli/src/notion-page-picker-tree.ts | 15 +-- .../cli/src/notion-page-picker-tui.test.tsx | 70 ++++++------ packages/cli/src/notion-page-picker-tui.tsx | 102 ++++++++++++------ 4 files changed, 126 insertions(+), 86 deletions(-) diff --git a/packages/cli/src/notion-page-picker-tree.test.ts b/packages/cli/src/notion-page-picker-tree.test.ts index 94b46b57..58e8c7ca 100644 --- a/packages/cli/src/notion-page-picker-tree.test.ts +++ b/packages/cli/src/notion-page-picker-tree.test.ts @@ -11,7 +11,6 @@ import { selectAllVisible, selectNone, toggleChecked, - TRANSIENT_HINT_DURATION_MS, visibleNodeIds, type NotionPickerPageInput, } from './notion-page-picker-tree.js'; @@ -223,22 +222,24 @@ describe('bulk actions and reducer effects', () => { }); }); - it('blocks empty saves, updates search state, and quits without saving', () => { + it('prompts skip-empty confirmation on empty save, updates search state, and quits without saving', () => { const state = buildInitialState({ tree: buildPickerTree(pages()), existingRootPageIds: [], currentCrawlMode: 'selected_roots', }); - const blockedSave = reducer(state, 'save-request', 9000); - expect(blockedSave).toEqual({ - next: { - ...state, - transientHint: { - text: 'Select at least one page or press q to quit', - expiresAt: 9000 + TRANSIENT_HINT_DURATION_MS, - }, - }, + const emptySave = reducer(state, 'save-request'); + expect(emptySave).toEqual({ + next: { ...state, pendingConfirm: 'skip-empty' }, + effect: null, + }); + expect(reducer(emptySave.next, 'save-confirm')).toEqual({ + next: { ...state, pendingConfirm: null }, + effect: 'quit-without-save', + }); + expect(reducer(emptySave.next, 'save-cancel')).toEqual({ + next: { ...state, pendingConfirm: null }, effect: null, }); expect( @@ -262,7 +263,7 @@ describe('bulk actions and reducer effects', () => { const withHint = { ...state, transientHint: { - text: 'Select at least one page or press q to quit', + text: 'Select at least one page or press esc to cancel', expiresAt: 11500, }, }; diff --git a/packages/cli/src/notion-page-picker-tree.ts b/packages/cli/src/notion-page-picker-tree.ts index 379ac938..738ab723 100644 --- a/packages/cli/src/notion-page-picker-tree.ts +++ b/packages/cli/src/notion-page-picker-tree.ts @@ -22,7 +22,7 @@ export interface PickerState { checked: Set; cursorId: string; search: { editing: boolean; query: string }; - pendingConfirm: 'mode-switch' | null; + pendingConfirm: 'mode-switch' | 'skip-empty' | null; preLoadWarnings: string[]; transientHint: { text: string; expiresAt: number } | null; currentCrawlMode: 'all_accessible' | 'selected_roots'; @@ -61,7 +61,7 @@ interface MutableNode { childIds: string[]; } -export const TRANSIENT_HINT_DURATION_MS = 2500; +const TRANSIENT_HINT_DURATION_MS = 2500; const collator = new Intl.Collator('en', { sensitivity: 'base', numeric: true }); @@ -444,7 +444,8 @@ export function buildInitialState(args: { export function reducer(state: PickerState, cmd: PickerCommand, now = Date.now()): { next: PickerState; effect: PickerEffect } { if (state.pendingConfirm) { if (cmd === 'save-confirm') { - return { next: cloneState(state, { pendingConfirm: null }), effect: 'save' }; + const effect: PickerEffect = state.pendingConfirm === 'skip-empty' ? 'quit-without-save' : 'save'; + return { next: cloneState(state, { pendingConfirm: null }), effect }; } if (cmd === 'save-cancel') { return { next: cloneState(state, { pendingConfirm: null }), effect: null }; @@ -498,19 +499,13 @@ export function reducer(state: PickerState, cmd: PickerCommand, now = Date.now() }; case 'save-request': if (state.checked.size === 0) { - return { - next: cloneState(state, { - transientHint: transientHint('Select at least one page or press q to quit', now), - }), - effect: null, - }; + return { next: cloneState(state, { pendingConfirm: 'skip-empty' }), effect: null }; } if (state.currentCrawlMode === 'all_accessible') { return { next: cloneState(state, { pendingConfirm: 'mode-switch' }), effect: null }; } return { next: state, effect: 'save' }; case 'save-confirm': - return { next: state, effect: 'save' }; case 'save-cancel': return { next: state, effect: null }; case 'quit': diff --git a/packages/cli/src/notion-page-picker-tui.test.tsx b/packages/cli/src/notion-page-picker-tui.test.tsx index 2d4dffc3..16ad93db 100644 --- a/packages/cli/src/notion-page-picker-tui.test.tsx +++ b/packages/cli/src/notion-page-picker-tui.test.tsx @@ -1,7 +1,7 @@ /* @jsxImportSource react */ import { render as renderInkTest } from 'ink-testing-library'; -import { act, type ReactNode } from 'react'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { type ReactNode } from 'react'; +import { describe, expect, it, vi } from 'vitest'; import { buildInitialState, buildPickerTree, type NotionPickerPageInput } from './notion-page-picker-tree.js'; import { NotionPickerApp, @@ -70,13 +70,9 @@ function fakeInkInstance(): NotionPickerInkInstance { } function normalizeFrameWrap(frame: string | undefined): string { - return frame?.replace(/\n/g, ' ') ?? ''; + return frame?.replace(/\n/g, ' ').replace(/│ /g, '').replace(/ +/g, ' ') ?? ''; } -afterEach(() => { - vi.useRealTimers(); -}); - describe('notionPickerCommandForInkInput', () => { it('maps browse, search, and confirm input to reducer commands', () => { expect(notionPickerCommandForInkInput('', { downArrow: true }, state().search, null)).toBe('cursor-down'); @@ -87,9 +83,11 @@ describe('notionPickerCommandForInkInput', () => { expect(notionPickerCommandForInkInput('/', {}, state().search, null)).toBe('search-start'); expect(notionPickerCommandForInkInput('a', {}, state().search, null)).toBe('select-all-visible'); expect(notionPickerCommandForInkInput('n', {}, state().search, null)).toBe('select-none'); - expect(notionPickerCommandForInkInput('s', {}, state().search, null)).toBe('save-request'); - expect(notionPickerCommandForInkInput('q', {}, state().search, null)).toBe('quit'); + expect(notionPickerCommandForInkInput('', { return: true }, state().search, null)).toBe('save-request'); + expect(notionPickerCommandForInkInput('', { escape: true }, state().search, null)).toBe('quit'); expect(notionPickerCommandForInkInput('c', { ctrl: true }, state().search, null)).toBe('quit'); + expect(notionPickerCommandForInkInput('s', {}, state().search, null)).toBeNull(); + expect(notionPickerCommandForInkInput('q', {}, state().search, null)).toBeNull(); expect(notionPickerCommandForInkInput('x', {}, { editing: true, query: '' }, null)).toEqual({ type: 'search-input', @@ -145,13 +143,16 @@ describe('NotionPickerApp', () => { ); const frame = lastFrame() ?? ''; - expect(frame).toContain('Notion pages visible to integration "Design Workspace"'); + expect(frame).toContain('Select Notion pages to ingest'); + expect(frame).toContain('Workspace: Design Workspace'); expect(frame).toContain('5000-page cap reached - some pages not shown'); expect(frame).toContain('1 stored root_page_ids no longer visible - they will be removed if you save'); - expect(frame).toContain('▸ [ ] Engineering Docs ▸ (1)'); - expect(frame).toContain(' [ ] Marketing'); + expect(frame).toContain('◻ Engineering Docs ▸ (1)'); + expect(frame).toContain('◻ Marketing'); expect(frame).not.toContain('Search ready: -'); - expect(frame).toContain('space toggle · enter expand · / search · a all · n none · s save & exit · q quit'); + expect(normalizeFrameWrap(frame)).toContain( + 'Right Arrow to expand, Up/Down to move, Space to select or unselect, Slash to filter, Enter to confirm, Escape to go back, or Ctrl+C to exit.', + ); }); it('renders partial discovery warnings without stale-root save suffix', () => { @@ -199,8 +200,8 @@ describe('NotionPickerApp', () => { ); const frame = lastFrame() ?? ''; - expect(frame).toContain('▸ [×] Engineering Docs ▾'); - expect(frame).toContain(' [~] Architecture'); + expect(frame).toContain('◼ Engineering Docs ▾'); + expect(frame).toContain(' ◼ Architecture'); }); it('supports keyboard selection, all_accessible confirmation, and save callback', async () => { @@ -220,12 +221,12 @@ describe('NotionPickerApp', () => { stdin.write(' '); await waitForInkInput(); - expect(lastFrame()).toContain('[×] Engineering Docs'); + expect(lastFrame()).toContain('◼ Engineering Docs'); - stdin.write('s'); + stdin.write('\r'); await waitForInkInput(); expect(normalizeFrameWrap(lastFrame())).toContain( - 'Save will switch crawl_mode all_accessible -> selected_roots and limit ingest to 1 selected page. [y] confirm [esc] back', + 'Switch crawl_mode from all_accessible to selected_roots? Will limit ingest to 1 selected page. Press Enter to confirm or Escape to go back.', ); stdin.write('y'); @@ -233,8 +234,7 @@ describe('NotionPickerApp', () => { expect(onExit).toHaveBeenCalledWith({ kind: 'save', rootPageIds: [IDS.engineering] }); }); - it('removes transient hints after their expiry time', async () => { - vi.useFakeTimers(); + it('prompts skip-empty confirmation on empty submit and dismisses on cancel', async () => { const onExit = vi.fn(); const { stdin, lastFrame } = renderInkTest( { />, ); - await act(async () => { - stdin.write('s'); - await vi.advanceTimersByTimeAsync(10); - }); - expect(lastFrame()).toContain('Select at least one page or press q to quit'); - - await act(async () => { - await vi.advanceTimersByTimeAsync(2500); - }); - expect(lastFrame()).not.toContain('Select at least one page or press q to quit'); + stdin.write('\r'); + await waitForInkInput(); + expect(normalizeFrameWrap(lastFrame())).toContain( + 'Nothing selected. Skip this step? Press Enter to skip or Escape to go back.', + ); expect(onExit).not.toHaveBeenCalled(); + + stdin.write('n'); + await waitForInkInput(); + expect(lastFrame()).not.toContain('Nothing selected. Skip this step?'); + expect(onExit).not.toHaveBeenCalled(); + + stdin.write('\r'); + await waitForInkInput(); + expect(lastFrame()).toContain('Nothing selected. Skip this step?'); + + stdin.write('\r'); + await waitForInkInput(); + expect(onExit).toHaveBeenCalledWith({ kind: 'quit' }); }); it('renders row-window overflow indicators when the visible list is clipped', async () => { @@ -312,7 +320,7 @@ describe('NotionPickerApp', () => { />, ); - stdin.write('q'); + stdin.write('\u0003'); await waitForInkInput(); expect(onExit).toHaveBeenCalledWith({ kind: 'quit' }); }); diff --git a/packages/cli/src/notion-page-picker-tui.tsx b/packages/cli/src/notion-page-picker-tui.tsx index 30af7522..d627d200 100644 --- a/packages/cli/src/notion-page-picker-tui.tsx +++ b/packages/cli/src/notion-page-picker-tui.tsx @@ -16,6 +16,7 @@ const COLOR_THEME = { text: 'white', muted: 'gray', active: 'cyan', + selected: 'green', warning: 'yellow', } as const; @@ -23,6 +24,7 @@ const NO_COLOR_THEME = { text: 'white', muted: 'white', active: 'white', + selected: 'white', warning: 'white', } as const; @@ -158,13 +160,12 @@ export function notionPickerCommandForInkInput( if (key.downArrow) return 'cursor-down'; if (key.leftArrow) return 'cursor-left'; if (key.rightArrow) return 'cursor-right'; - if (key.return) return 'expand'; + if (key.return) return 'save-request'; if (input === ' ') return 'toggle-check'; if (input === '/') return 'search-start'; if (input === 'a') return 'select-all-visible'; if (input === 'n') return 'select-none'; - if (input === 's') return 'save-request'; - if (input === 'q' || key.escape) return 'quit'; + if (key.escape) return 'quit'; return null; } @@ -174,18 +175,27 @@ function PickerRow(props: { state: PickerState; nodeId: string; width: number; t const focused = props.state.cursorId === node.id; const locked = isAncestorChecked(node.id, props.state.checked, props.state.byId); const checked = props.state.checked.has(node.id); - const glyph = locked ? '[~]' : checked ? '[×]' : '[ ]'; - const children = + const isSelected = checked || locked; + const glyph = isSelected ? '◼' : '◻'; + const glyphColor = locked ? props.theme.muted : checked ? props.theme.selected : props.theme.muted; + const childAffordance = node.childIds.length > 0 ? (props.state.expanded.has(node.id) ? ' ▾' : ` ▸ (${node.childIds.length})`) : ''; - const prefix = `${focused ? '▸' : ' '} ${glyph} ${' '.repeat(node.depth * 2)}`; - const color = focused ? props.theme.active : locked || node.archived ? props.theme.muted : props.theme.text; - const title = truncateText(`${node.title}${children}`, Math.max(10, props.width - prefix.length)); + const indent = ' '.repeat(node.depth * 2); + const titleColor = focused ? props.theme.text : props.theme.muted; const inverse = rowMatchesSearch(props.state, node.id); + const prefixWidth = indent.length + 2; + const title = truncateText(`${node.title}${childAffordance}`, Math.max(10, props.width - prefixWidth)); return ( - - {prefix} - {title} + + + {indent} + {glyph} + + + {' '} + {title} + ); } @@ -198,7 +208,7 @@ export function NotionPickerApp(props: NotionPickerAppProps): ReactNode { const visibleIds = visibleNodeIds(state); const selectedIndex = Math.max(0, visibleIds.indexOf(state.cursorId)); const reservedRows = state.pendingConfirm === 'mode-switch' ? 9 : 8; - const visibleRows = Math.max(5, Math.min(20, (props.terminalRows ?? 24) - reservedRows)); + const visibleRows = Math.max(5, Math.min(12, (props.terminalRows ?? 24) - reservedRows)); const rows = windowItems(visibleIds, selectedIndex, visibleRows); const hiddenAbove = rows.offset; const hiddenBelow = Math.max(0, visibleIds.length - rows.offset - rows.items.length); @@ -254,34 +264,60 @@ export function NotionPickerApp(props: NotionPickerAppProps): ReactNode { return ( - Notion pages visible to integration "{props.workspaceLabel}" - {props.cappedAtCount ? {props.cappedAtCount}-page cap reached - some pages not shown : null} - {state.preLoadWarnings.map((warning) => ( - - {staleWarningText(warning)} - - ))} - {showSearch ? ( + + ◆ + Select Notion pages to ingest + + - / {state.search.query} - {state.search.editing ? '█' : ''} ({searchMatchCount} matches) + Right Arrow to expand, Up/Down to move, Space to select or unselect, Slash to filter, Enter to confirm, Escape + to go back, or Ctrl+C to exit. - ) : null} - + + Workspace: {props.workspaceLabel} + {props.cappedAtCount ? ( + {props.cappedAtCount}-page cap reached - some pages not shown + ) : null} + {state.preLoadWarnings.map((warning) => ( + + {staleWarningText(warning)} + + ))} + {showSearch ? ( + + / + + {state.search.query} + {state.search.editing ? '█' : ''} + + ({searchMatchCount} matches) + + ) : null} {hiddenAbove > 0 ? ↑ {hiddenAbove} more : null} {rows.items.map((nodeId) => ( ))} {hiddenBelow > 0 ? ↓ {hiddenBelow} more : null} + {state.pendingConfirm === 'mode-switch' ? ( + + Switch crawl_mode from all_accessible to selected_roots? Will limit ingest to{' '} + {selectedPageCountText(selectedCount)}. Press Enter to confirm or Escape to go back. + + ) : null} + {state.pendingConfirm === 'skip-empty' ? ( + Nothing selected. Skip this step? Press Enter to skip or Escape to go back. + ) : null} + {state.transientHint ? {state.transientHint.text} : null} - {state.pendingConfirm === 'mode-switch' ? ( - - Save will switch crawl_mode all_accessible -> selected_roots and limit ingest to{' '} - {selectedPageCountText(selectedCount)}. [y] confirm [esc] back - - ) : null} - {state.transientHint ? {state.transientHint.text} : null} - space toggle · enter expand · / search · a all · n none · s save & exit · q quit + └ ); } @@ -323,7 +359,7 @@ export async function renderNotionPickerTui( exitOnCtrlC: false, patchConsole: false, maxFps: 30, - alternateScreen: true, + alternateScreen: false, }, ); await instance.waitUntilExit(); From c2750dd7970be18a18f4cf3fb595258c88be323f Mon Sep 17 00:00:00 2001 From: Luca Martial <48870843+luca-martial@users.noreply.github.com> Date: Wed, 13 May 2026 17:55:25 -0400 Subject: [PATCH 2/5] refactor(cli): hide internal setup options and remove dead flags (#79) Hide advanced/internal `ktx setup` options from --help output using .hideHelp() so the command surface is approachable for new users. Remove the --project, --agent-scope, and --skip-initial-source-ingest flags that are no longer needed. Update docs and tests to match. Co-authored-by: Claude Opus 4.6 (1M context) --- .../content/docs/cli-reference/ktx-setup.mdx | 94 +------- .../docs/getting-started/quickstart.mdx | 2 +- packages/cli/src/commands/setup-commands.ts | 213 ++++++++++-------- packages/cli/src/index.test.ts | 72 +++++- packages/cli/src/setup-agents.ts | 2 +- 5 files changed, 185 insertions(+), 198 deletions(-) diff --git a/docs-site/content/docs/cli-reference/ktx-setup.mdx b/docs-site/content/docs/cli-reference/ktx-setup.mdx index f490988a..59fbe666 100644 --- a/docs-site/content/docs/cli-reference/ktx-setup.mdx +++ b/docs-site/content/docs/cli-reference/ktx-setup.mdx @@ -18,8 +18,6 @@ ktx setup [options] | Flag | Description | Default | |------|-------------|---------| | `--project-dir ` | KTX project directory | `KTX_PROJECT_DIR`, nearest `ktx.yaml`, or cwd | -| `--new` | Create a new KTX project before setup | `false` | -| `--existing` | Use an existing KTX project | `false` | | `--yes` | Accept safe defaults in non-interactive setup | `false` | | `--no-input` | Disable interactive terminal input | — | @@ -29,76 +27,11 @@ ktx setup [options] |------|-------------|---------| | `--agents` | Install agent integration only | `false` | | `--target ` | Agent target (`claude-code`, `codex`, `cursor`, `opencode`, `universal`) | — | -| `--agent-scope ` | Agent install scope (`project` or `global`) | `project` | -| `--project` | Install agent integration into the project scope | `false` | | `--global` | Install agent integration into the global target scope (Claude Code and Codex only) | `false` | -| `--skip-agents` | Leave agent integration incomplete for now | `false` | -### LLM Configuration - -| Flag | Description | Default | -|------|-------------|---------| -| `--anthropic-api-key-env ` | Environment variable containing the Anthropic API key | — | -| `--anthropic-api-key-file ` | File containing the Anthropic API key | — | -| `--anthropic-model ` | Anthropic model ID to validate and save | — | -| `--skip-llm` | Leave LLM setup incomplete for now | `false` | - -### Embedding Configuration - -| Flag | Description | Default | -|------|-------------|---------| -| `--embedding-backend ` | Embedding backend (`openai` or `sentence-transformers`) | — | -| `--embedding-api-key-env ` | Environment variable containing the embedding provider API key | — | -| `--embedding-api-key-file ` | File containing the embedding provider API key | — | -| `--skip-embeddings` | Leave embedding setup incomplete for now | `false` | - -### Database Configuration - -| Flag | Description | Default | -|------|-------------|---------| -| `--database ` | Database driver to configure; repeatable (`sqlite`, `postgres`, `mysql`, `clickhouse`, `sqlserver`, `bigquery`, `snowflake`) | — | -| `--database-connection-id ` | Existing or new connection id; repeatable | — | -| `--new-database-connection-id ` | Connection id for one new database connection | — | -| `--database-url ` | URL, `env:NAME`, or `file:/path` for one new URL-style database connection | — | -| `--database-schema ` | Database schema to include; repeatable | — | -| `--skip-databases` | Leave database setup incomplete | `false` | - -### Historic SQL - -| Flag | Description | Default | -|------|-------------|---------| -| `--enable-historic-sql` | Enable Historic SQL when the selected database supports it | `false` | -| `--disable-historic-sql` | Disable Historic SQL for the selected database | `false` | -| `--historic-sql-window-days ` | Historic SQL query-history window in days | — | -| `--historic-sql-min-executions ` | Minimum executions for a Historic SQL template | — | -| `--historic-sql-min-calls ` | Alias for `--historic-sql-min-executions` for one release | — | -| `--historic-sql-service-account-pattern ` | Historic SQL service-account regex; repeatable | — | -| `--historic-sql-redaction-pattern ` | Historic SQL SQL-literal redaction regex; repeatable | — | - -### Context Source Configuration - -| Flag | Description | Default | -|------|-------------|---------| -| `--source ` | Source connector type (`dbt`, `metricflow`, `metabase`, `looker`, `lookml`, `notion`) | — | -| `--source-connection-id ` | Connection id for source setup | — | -| `--source-path ` | Local source path for dbt, MetricFlow, or LookML | — | -| `--source-git-url ` | Git URL for dbt, MetricFlow, or LookML | — | -| `--source-branch ` | Git branch for source setup | — | -| `--source-subpath ` | Repo subpath for source setup | — | -| `--source-auth-token-ref ` | `env:` or `file:` credential ref for source repo auth | — | -| `--source-url ` | Source service URL for Metabase or Looker | — | -| `--source-api-key-ref ` | `env:` or `file:` API key ref for Metabase or Notion | — | -| `--source-client-id ` | Looker client id | — | -| `--source-client-secret-ref ` | `env:` or `file:` Looker client secret ref | — | -| `--source-warehouse-connection-id ` | Mapped warehouse connection id | — | -| `--source-project-name ` | dbt project name override | — | -| `--source-profiles-path ` | dbt profiles path | — | -| `--source-target ` | dbt target or source-specific mapping target | — | -| `--metabase-database-id ` | Metabase database id to map | — | -| `--notion-crawl-mode ` | Notion crawl mode (`all_accessible` or `selected_roots`) | — | -| `--notion-root-page-id ` | Notion root page id; repeatable | — | -| `--skip-initial-source-ingest` | Validate source setup without building source context during setup | `false` | -| `--skip-sources` | Mark optional source setup complete with no sources | `false` | +The setup wizard is the public configuration interface. It prompts for LLM +credentials, embeddings, database connections, context sources, Historic SQL, +and agent integration when those values are needed. ## Examples @@ -106,17 +39,8 @@ ktx setup [options] # Run the interactive setup wizard ktx setup -# Create a new project and run setup -ktx setup --new - -# Resume setup in an existing project -ktx setup --existing - -# Non-interactive setup with Anthropic key from environment -ktx setup --yes --anthropic-api-key-env ANTHROPIC_API_KEY - -# Set up a Postgres connection -ktx setup --database postgres --database-url "env:DATABASE_URL" +# Run setup for a specific project directory +ktx setup --project-dir ./analytics # Install agent integration for Claude Code only ktx setup --agents --target claude-code @@ -124,12 +48,6 @@ ktx setup --agents --target claude-code # Install agent integration globally for Codex ktx setup --agents --target codex --global -# Add a dbt source from a local path -ktx setup --source dbt --source-path ./my-dbt-project - -# Skip optional steps for a minimal setup -ktx setup --skip-sources --skip-agents - # Check setup readiness ktx status ``` @@ -156,5 +74,5 @@ Agent integration ready: yes (codex:project) |-------|-------|----------| | Setup resumes an unexpected project | `KTX_PROJECT_DIR` or nearest `ktx.yaml` points to another directory | Pass `--project-dir ` explicitly | | Health check for model fails | Provider key or model id is invalid | Set the correct environment variable or secret file and rerun setup | -| Setup cannot run in CI | Interactive prompts need a TTY | Use `--yes --no-input` with explicit flags for required values | +| Setup cannot run in CI | Interactive prompts need a TTY | Run setup interactively before CI, or provide a fixture `ktx.yaml` for automated tests | | Agent integration missing | Setup skipped the agents step | Run `ktx setup --agents --target ` | diff --git a/docs-site/content/docs/getting-started/quickstart.mdx b/docs-site/content/docs/getting-started/quickstart.mdx index 7aba00fd..635c666b 100644 --- a/docs-site/content/docs/getting-started/quickstart.mdx +++ b/docs-site/content/docs/getting-started/quickstart.mdx @@ -242,7 +242,7 @@ Agent integration ready: yes (claude-code:project) | Local embeddings hang or fail | The managed Python runtime cannot start or the local model runtime is unavailable | Install `uv`, run `ktx dev runtime status`, then run `ktx dev runtime install --feature local-embeddings --yes` and rerun setup | | Database connection test fails | Credentials, network access, warehouse, database, or schema value is wrong | Test the same URL with the database's native client, then rerun `ktx setup` and reconfigure the connection | | `KTX context built: no` in `ktx status` | Setup saved configuration but did not build context | Run `ktx setup` and choose to build context now | -| Agent integration is incomplete | Setup skipped the agents step or the target was not installed | Run `ktx setup --agents --target codex --project` using the target you need | +| Agent integration is incomplete | Setup skipped the agents step or the target was not installed | Run `ktx setup --agents --target codex` using the target you need | ## Next steps diff --git a/packages/cli/src/commands/setup-commands.ts b/packages/cli/src/commands/setup-commands.ts index 1688724d..3da8d094 100644 --- a/packages/cli/src/commands/setup-commands.ts +++ b/packages/cli/src/commands/setup-commands.ts @@ -64,13 +64,6 @@ function sourceType(value: string): KtxSetupSourceType { throw new InvalidArgumentError(`invalid choice '${value}'`); } -function agentScope(value: string): 'project' | 'global' { - if (value === 'project' || value === 'global') { - return value; - } - throw new InvalidArgumentError(`invalid choice '${value}'`); -} - function positiveNumber(value: string): number { const parsed = Number.parseInt(value, 10); if (!Number.isInteger(parsed) || parsed <= 0) { @@ -97,7 +90,6 @@ function shouldShowSetupEntryMenu( agents?: boolean; target?: string; global?: boolean; - project?: boolean; skipAgents?: boolean; yes?: boolean; input?: boolean; @@ -142,7 +134,6 @@ function shouldShowSetupEntryMenu( metabaseDatabaseId?: number; notionCrawlMode?: string; notionRootPageId?: string[]; - skipInitialSourceIngest?: boolean; skipSources?: boolean; }, command: Command, @@ -172,7 +163,6 @@ function shouldShowSetupEntryMenu( 'agents', 'target', 'global', - 'project', 'skipAgents', 'yes', 'input', @@ -211,7 +201,6 @@ function shouldShowSetupEntryMenu( 'sourceTarget', 'metabaseDatabaseId', 'notionCrawlMode', - 'skipInitialSourceIngest', 'skipSources', ].some((optionName) => optionWasSpecified(command, optionName)); } @@ -220,9 +209,9 @@ export function registerSetupCommands(program: Command, context: KtxCliCommandCo const setup = program .command('setup') .description('Set up or resume a local KTX project') - .option('--project-dir ', 'KTX project directory') - .option('--new', 'Create a new KTX project before setup', false) - .option('--existing', 'Use an existing KTX project', false) + .addOption(new Option('--project-dir ', 'KTX project directory').hideHelp()) + .addOption(new Option('--new', 'Create a new KTX project before setup').hideHelp().default(false)) + .addOption(new Option('--existing', 'Use an existing KTX project').hideHelp().default(false)) .option('--agents', 'Install agent integration only', false) .addOption( new Option('--target ', 'Agent target').choices([ @@ -233,94 +222,124 @@ export function registerSetupCommands(program: Command, context: KtxCliCommandCo 'universal', ]), ) - .addOption(new Option('--agent-scope ', 'Agent install scope').argParser(agentScope).default('project')) - .option('--project', 'Install agent integration into the project scope', false) .option('--global', 'Install agent integration into the global target scope', false) - .option('--skip-agents', 'Leave agent integration incomplete for now', false) + .addOption(new Option('--skip-agents', 'Leave agent integration incomplete for now').hideHelp().default(false)) .option('--yes', 'Accept safe defaults in non-interactive setup', false) .option('--no-input', 'Disable interactive terminal input') - .addOption(new Option('--llm-backend ', 'LLM backend').argParser(llmBackend)) - .option('--anthropic-api-key-env ', 'Environment variable containing the Anthropic API key') - .option('--anthropic-api-key-file ', 'File containing the Anthropic API key') - .option('--anthropic-model ', 'Anthropic model ID to validate and save') - .option('--vertex-project ', 'Google Vertex AI project ID, env:NAME, or file:/path') - .option('--vertex-location ', 'Google Vertex AI location, env:NAME, or file:/path') - .addOption(new Option('--skip-llm', 'Leave LLM setup incomplete for now').hideHelp().default(false)) - .addOption(new Option('--embedding-backend ', 'Embedding backend').argParser(embeddingBackend)) - .option('--embedding-api-key-env ', 'Environment variable containing the embedding provider API key') - .option('--embedding-api-key-file ', 'File containing the embedding provider API key') - .addOption(new Option('--skip-embeddings', 'Leave embedding setup incomplete for now').hideHelp().default(false)) - .option( - '--database ', - 'Database driver to configure; repeatable', - (value, previous: KtxSetupDatabaseDriver[]) => { - return [...previous, databaseDriver(value)]; - }, - [] as KtxSetupDatabaseDriver[], - ) - .option( - '--database-connection-id ', - 'Existing selected connection id or new connection id', - (value, previous: string[]) => [...previous, value], - [], - ) - .option('--new-database-connection-id ', 'Connection id for one new database connection', (value) => { - if (!/^[a-zA-Z0-9][a-zA-Z0-9_-]*$/.test(value)) { - throw new InvalidArgumentError(`Unsafe connection id: ${value}`); - } - return value; - }) - .option('--database-url ', 'URL, env:NAME, or file:/path for one new URL-style database connection') - .option( - '--database-schema ', - 'Database schema to include; repeatable', - (value, previous: string[]) => [...previous, value], - [], - ) - .option('--enable-historic-sql', 'Enable Historic SQL when the selected database supports it', false) - .option('--disable-historic-sql', 'Disable Historic SQL for the selected database', false) - .option('--historic-sql-window-days ', 'Historic SQL query-history window', positiveInteger) - .option('--historic-sql-min-executions ', 'Minimum Historic SQL executions for a template', positiveInteger) - .option( - '--historic-sql-service-account-pattern ', - 'Historic SQL service-account regex; repeatable', - (value, previous: string[]) => [...previous, value], - [], - ) - .option( - '--historic-sql-redaction-pattern ', - 'Historic SQL SQL-literal redaction regex; repeatable', - (value, previous: string[]) => [...previous, value], - [], - ) - .option('--skip-databases', 'Leave database setup incomplete; KTX cannot work until a primary source is added', false) - .addOption(new Option('--source ', 'Source connector type').argParser(sourceType)) - .option('--source-connection-id ', 'Connection id for source setup') - .option('--source-path ', 'Local source path for dbt, MetricFlow, or LookML') - .option('--source-git-url ', 'Git URL for dbt, MetricFlow, or LookML') - .option('--source-branch ', 'Git branch for source setup') - .option('--source-subpath ', 'Repo subpath for source setup') - .option('--source-auth-token-ref ', 'env: or file: credential ref for source repo auth') - .option('--source-url ', 'Source service URL for Metabase or Looker') - .option('--source-api-key-ref ', 'env: or file: API key ref for Metabase or Notion') - .option('--source-client-id ', 'Looker client id') - .option('--source-client-secret-ref ', 'env: or file: Looker client secret ref') - .option('--source-warehouse-connection-id ', 'Mapped warehouse connection id') - .option('--source-project-name ', 'dbt project name override') - .option('--source-profiles-path ', 'dbt profiles path') - .option('--source-target ', 'dbt target or source-specific mapping target') - .option('--metabase-database-id ', 'Metabase database id to map', positiveNumber) + .addOption(new Option('--llm-backend ', 'LLM backend').argParser(llmBackend).hideHelp()) .addOption( - new Option('--notion-crawl-mode ', 'Notion crawl mode').choices(['all_accessible', 'selected_roots']), + new Option('--anthropic-api-key-env ', 'Environment variable containing the Anthropic API key').hideHelp(), ) - .option( - '--notion-root-page-id ', - 'Notion root page id; repeatable', - (value, previous: string[]) => [...previous, value], - [], + .addOption( + new Option('--anthropic-api-key-file ', 'File containing the Anthropic API key').hideHelp(), ) - .option('--skip-initial-source-ingest', 'Validate source setup without building source context during setup', false) - .option('--skip-sources', 'Mark optional source setup complete with no sources', false) + .addOption(new Option('--anthropic-model ', 'Anthropic model ID to validate and save').hideHelp()) + .addOption(new Option('--vertex-project ', 'Google Vertex AI project ID, env:NAME, or file:/path').hideHelp()) + .addOption(new Option('--vertex-location ', 'Google Vertex AI location, env:NAME, or file:/path').hideHelp()) + .addOption(new Option('--skip-llm', 'Leave LLM setup incomplete for now').hideHelp().default(false)) + .addOption(new Option('--embedding-backend ', 'Embedding backend').argParser(embeddingBackend).hideHelp()) + .addOption( + new Option( + '--embedding-api-key-env ', + 'Environment variable containing the embedding provider API key', + ).hideHelp(), + ) + .addOption( + new Option('--embedding-api-key-file ', 'File containing the embedding provider API key').hideHelp(), + ) + .addOption(new Option('--skip-embeddings', 'Leave embedding setup incomplete for now').hideHelp().default(false)) + .addOption( + new Option('--database ', 'Database driver to configure; repeatable') + .argParser((value, previous: KtxSetupDatabaseDriver[]) => { + return [...previous, databaseDriver(value)]; + }) + .default([] as KtxSetupDatabaseDriver[]) + .hideHelp(), + ) + .addOption( + new Option('--database-connection-id ', 'Existing selected connection id or new connection id') + .argParser((value, previous: string[]) => [...previous, value]) + .default([] as string[]) + .hideHelp(), + ) + .addOption( + new Option('--new-database-connection-id ', 'Connection id for one new database connection') + .argParser((value) => { + if (!/^[a-zA-Z0-9][a-zA-Z0-9_-]*$/.test(value)) { + throw new InvalidArgumentError(`Unsafe connection id: ${value}`); + } + return value; + }) + .hideHelp(), + ) + .addOption( + new Option('--database-url ', 'URL, env:NAME, or file:/path for one new URL-style database connection').hideHelp(), + ) + .addOption( + new Option('--database-schema ', 'Database schema to include; repeatable') + .argParser((value, previous: string[]) => [...previous, value]) + .default([] as string[]) + .hideHelp(), + ) + .addOption( + new Option('--enable-historic-sql', 'Enable Historic SQL when the selected database supports it') + .hideHelp() + .default(false), + ) + .addOption( + new Option('--disable-historic-sql', 'Disable Historic SQL for the selected database').hideHelp().default(false), + ) + .addOption(new Option('--historic-sql-window-days ', 'Historic SQL query-history window').argParser(positiveInteger).hideHelp()) + .addOption( + new Option('--historic-sql-min-executions ', 'Minimum Historic SQL executions for a template') + .argParser(positiveInteger) + .hideHelp(), + ) + .addOption( + new Option('--historic-sql-service-account-pattern ', 'Historic SQL service-account regex; repeatable') + .argParser((value, previous: string[]) => [...previous, value]) + .default([] as string[]) + .hideHelp(), + ) + .addOption( + new Option('--historic-sql-redaction-pattern ', 'Historic SQL SQL-literal redaction regex; repeatable') + .argParser((value, previous: string[]) => [...previous, value]) + .default([] as string[]) + .hideHelp(), + ) + .addOption( + new Option('--skip-databases', 'Leave database setup incomplete; KTX cannot work until a primary source is added') + .hideHelp() + .default(false), + ) + .addOption(new Option('--source ', 'Source connector type').argParser(sourceType).hideHelp()) + .addOption(new Option('--source-connection-id ', 'Connection id for source setup').hideHelp()) + .addOption(new Option('--source-path ', 'Local source path for dbt, MetricFlow, or LookML').hideHelp()) + .addOption(new Option('--source-git-url ', 'Git URL for dbt, MetricFlow, or LookML').hideHelp()) + .addOption(new Option('--source-branch ', 'Git branch for source setup').hideHelp()) + .addOption(new Option('--source-subpath ', 'Repo subpath for source setup').hideHelp()) + .addOption(new Option('--source-auth-token-ref ', 'env: or file: credential ref for source repo auth').hideHelp()) + .addOption(new Option('--source-url ', 'Source service URL for Metabase or Looker').hideHelp()) + .addOption(new Option('--source-api-key-ref ', 'env: or file: API key ref for Metabase or Notion').hideHelp()) + .addOption(new Option('--source-client-id ', 'Looker client id').hideHelp()) + .addOption(new Option('--source-client-secret-ref ', 'env: or file: Looker client secret ref').hideHelp()) + .addOption(new Option('--source-warehouse-connection-id ', 'Mapped warehouse connection id').hideHelp()) + .addOption(new Option('--source-project-name ', 'dbt project name override').hideHelp()) + .addOption(new Option('--source-profiles-path ', 'dbt profiles path').hideHelp()) + .addOption(new Option('--source-target ', 'dbt target or source-specific mapping target').hideHelp()) + .addOption(new Option('--metabase-database-id ', 'Metabase database id to map').argParser(positiveNumber).hideHelp()) + .addOption( + new Option('--notion-crawl-mode ', 'Notion crawl mode') + .choices(['all_accessible', 'selected_roots']) + .hideHelp(), + ) + .addOption( + new Option('--notion-root-page-id ', 'Notion root page id; repeatable') + .argParser((value, previous: string[]) => [...previous, value]) + .default([] as string[]) + .hideHelp(), + ) + .addOption(new Option('--skip-sources', 'Mark optional source setup complete with no sources').hideHelp().default(false)) .showHelpAfterError(); setup.hook('preAction', (_thisCommand, actionCommand) => { @@ -371,7 +390,7 @@ export function registerSetupCommands(program: Command, context: KtxCliCommandCo } const mode = options.new ? 'new' : options.existing ? 'existing' : 'auto'; - const resolvedAgentScope = options.global ? 'global' : options.agentScope; + const resolvedAgentScope = options.global ? 'global' : 'project'; await runSetupArgs(context, { command: 'run', projectDir: resolveCommandProjectDir(command), diff --git a/packages/cli/src/index.test.ts b/packages/cli/src/index.test.ts index cd635d78..305cf30e 100644 --- a/packages/cli/src/index.test.ts +++ b/packages/cli/src/index.test.ts @@ -444,20 +444,54 @@ describe('runKtxCli', () => { expect(io.stderr()).toContain('Choose only one runtime install mode: --yes or --no-input'); }); - it('documents setup as a bare command without subcommands', async () => { + it('documents setup with only the common interactive options visible', async () => { const testIo = makeIo(); await expect(runKtxCli(['setup', '--help'], testIo.io)).resolves.toBe(0); - expect(testIo.stdout()).toContain('Usage: ktx setup [options]'); - expect(testIo.stdout()).not.toContain('Commands:'); - expect(testIo.stdout()).not.toContain('setup demo'); - expect(testIo.stdout()).not.toContain('setup context'); - expect(testIo.stdout()).not.toContain('--skip-llm'); - expect(testIo.stdout()).not.toContain('--skip-embeddings'); - expect(testIo.stdout()).not.toContain('--embedding-model'); - expect(testIo.stdout()).not.toContain('--embedding-dimensions'); - expect(testIo.stdout()).not.toContain('--embedding-base-url'); + const stdout = testIo.stdout(); + expect(stdout).toContain('Usage: ktx setup [options]'); + expect(stdout).toContain('--agents'); + expect(stdout).toContain('--target '); + expect(stdout).toContain('--global'); + expect(stdout).toContain('--yes'); + expect(stdout).toContain('--no-input'); + expect(stdout).toContain('Global Options:'); + expect(stdout.match(/--project-dir /g)).toHaveLength(1); + expect(stdout).not.toContain('Commands:'); + expect(stdout).not.toContain('setup demo'); + expect(stdout).not.toContain('setup context'); + + for (const hiddenFlag of [ + '--new', + '--existing', + '--agent-scope', + '--skip-agents', + '--llm-backend', + '--anthropic-api-key-env', + '--vertex-project', + '--embedding-backend', + '--database ', + '--database-connection-id', + '--new-database-connection-id', + '--enable-historic-sql', + '--historic-sql-min-executions', + '--skip-databases', + '--source ', + '--source-connection-id', + '--metabase-database-id', + '--notion-root-page-id', + '--skip-initial-source-ingest', + '--skip-sources', + '--skip-llm', + '--skip-embeddings', + '--embedding-model', + '--embedding-dimensions', + '--embedding-base-url', + ]) { + expect(stdout).not.toContain(hiddenFlag); + } + expect(stdout).not.toMatch(/^ --project\s/m); expect(testIo.stderr()).toBe(''); }); @@ -725,6 +759,23 @@ describe('runKtxCli', () => { expect(setup).not.toHaveBeenCalled(); }); + it('rejects removed setup options', async () => { + const setup = vi.fn(async () => 0); + const cases = [ + ['setup', '--project'], + ['setup', '--agent-scope', 'global'], + ['setup', '--skip-initial-source-ingest'], + ]; + + for (const args of cases) { + const testIo = makeIo(); + await expect(runKtxCli(['--project-dir', tempDir, ...args], testIo.io, { setup })).resolves.toBe(1); + expect(testIo.stderr()).toMatch(/unknown option|error:/i); + } + + expect(setup).not.toHaveBeenCalled(); + }); + it('prints ingest help without invoking ingest execution', async () => { const testIo = makeIo(); const ingest = vi.fn(); @@ -1250,7 +1301,6 @@ describe('runKtxCli', () => { '--agents', '--target', 'codex', - '--project', '--no-input', '--yes', ], diff --git a/packages/cli/src/setup-agents.ts b/packages/cli/src/setup-agents.ts index 9505307d..7a18a969 100644 --- a/packages/cli/src/setup-agents.ts +++ b/packages/cli/src/setup-agents.ts @@ -82,7 +82,7 @@ export function plannedKtxAgentFiles(input: { { kind: 'file', path: join(codexHome, 'instructions/ktx.md'), role: 'rule' as const }, ]; } - throw new Error(`Global ${input.target} installation is not supported; use --project.`); + throw new Error(`Global ${input.target} installation is not supported; omit --global.`); } const root = resolve(input.projectDir); From dabd640cade793288812e6fe90b0ab68a3093ae9 Mon Sep 17 00:00:00 2001 From: Luca Martial <48870843+luca-martial@users.noreply.github.com> Date: Wed, 13 May 2026 18:41:44 -0400 Subject: [PATCH 3/5] feat(cli): tree-picker UI for database scope selection (#81) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(cli): extract generic tree picker from Notion-specific modules Rename notion-page-picker-tree → tree-picker-state and notion-page-picker-tui → tree-picker-tui, removing Notion-specific naming so the tree picker can be reused for database scope selection. Update notion-page-picker to consume the new generic interfaces. Co-Authored-By: Claude Opus 4.6 (1M context) * feat(cli): add database tree picker for schema and table scope selection Replace inline multiselect prompts in setup-databases with a new database-tree-picker that uses the generic tree picker TUI. This gives database scope selection the same grouped tree UI as the Notion page picker, combining schema and table selection into a single step. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- packages/cli/src/database-tree-picker.test.ts | 188 +++++++++ packages/cli/src/database-tree-picker.ts | 210 ++++++++++ .../cli/src/notion-page-picker-tui.test.tsx | 392 ------------------ packages/cli/src/notion-page-picker.test.ts | 50 ++- packages/cli/src/notion-page-picker.ts | 103 +++-- packages/cli/src/setup-databases.test.ts | 160 +++++-- packages/cli/src/setup-databases.ts | 332 ++++++--------- ...tree.test.ts => tree-picker-state.test.ts} | 89 ++-- ...ge-picker-tree.ts => tree-picker-state.ts} | 91 ++-- packages/cli/src/tree-picker-tui.test.tsx | 361 ++++++++++++++++ ...age-picker-tui.tsx => tree-picker-tui.tsx} | 157 +++---- 11 files changed, 1299 insertions(+), 834 deletions(-) create mode 100644 packages/cli/src/database-tree-picker.test.ts create mode 100644 packages/cli/src/database-tree-picker.ts delete mode 100644 packages/cli/src/notion-page-picker-tui.test.tsx rename packages/cli/src/{notion-page-picker-tree.test.ts => tree-picker-state.test.ts} (78%) rename packages/cli/src/{notion-page-picker-tree.ts => tree-picker-state.ts} (86%) create mode 100644 packages/cli/src/tree-picker-tui.test.tsx rename packages/cli/src/{notion-page-picker-tui.tsx => tree-picker-tui.tsx} (67%) diff --git a/packages/cli/src/database-tree-picker.test.ts b/packages/cli/src/database-tree-picker.test.ts new file mode 100644 index 00000000..5559ee42 --- /dev/null +++ b/packages/cli/src/database-tree-picker.test.ts @@ -0,0 +1,188 @@ +import { describe, expect, it, vi } from 'vitest'; +import { + pickDatabaseScope, + type DatabaseTreePickerRenderer, + type PickDatabaseScopeArgs, +} from './database-tree-picker.js'; +import type { TreePickerChrome, TreePickerResult } from './tree-picker-tui.js'; +import type { PickerState } from './tree-picker-state.js'; + +function makeIo() { + let stdout = ''; + let stderr = ''; + return { + io: { + stdout: { isTTY: true, write: (chunk: string) => { stdout += chunk; } }, + stderr: { write: (chunk: string) => { stderr += chunk; } }, + }, + stdout: () => stdout, + stderr: () => stderr, + }; +} + +function captureRenderer(): { + renderer: DatabaseTreePickerRenderer; + capture: { chrome?: TreePickerChrome; state?: PickerState }; + setResult: (result: TreePickerResult) => void; +} { + const capture: { chrome?: TreePickerChrome; state?: PickerState } = {}; + let nextResult: TreePickerResult = { kind: 'quit' }; + const renderer: DatabaseTreePickerRenderer = vi.fn(async (chrome, state) => { + capture.chrome = chrome; + capture.state = state; + return nextResult; + }); + return { + renderer, + capture, + setResult: (result) => { + nextResult = result; + }, + }; +} + +const discovered = [ + { schema: 'analytics', name: 'customers', kind: 'table' as const }, + { schema: 'analytics', name: 'orders', kind: 'table' as const }, + { schema: 'public', name: 'events', kind: 'view' as const }, + { schema: 'public', name: 'sessions', kind: 'table' as const }, +]; + +function baseArgs(overrides: Partial = {}): PickDatabaseScopeArgs { + return { + connectionId: 'warehouse', + schemaNoun: 'schema', + schemaNounPlural: 'schemas', + discovered, + existing: { enabledTables: [] }, + defaultSchemas: ['analytics'], + supportsSchemaScope: true, + ...overrides, + }; +} + +describe('pickDatabaseScope', () => { + it('builds a 2-level tree (schemas as parents, tables as children) and uses save-empty action', async () => { + const { renderer, capture, setResult } = captureRenderer(); + setResult({ kind: 'quit' }); + + await pickDatabaseScope(baseArgs(), makeIo().io, renderer); + + expect(capture.state?.skipEmptyAction).toBe('save-empty'); + const schemaIds = capture.state?.tree.filter((n) => n.parentId === null).map((n) => n.id); + const tableIds = capture.state?.tree.filter((n) => n.parentId !== null).map((n) => n.id); + expect((schemaIds ?? []).sort()).toEqual(['analytics', 'public']); + expect((tableIds ?? []).sort()).toEqual([ + 'analytics.customers', + 'analytics.orders', + 'public.events', + 'public.sessions', + ]); + expect(capture.state?.byId.get('public.events')?.title).toBe('events (view)'); + }); + + it('pre-checks default schemas at the parent level when no existing selection', async () => { + const { renderer, capture, setResult } = captureRenderer(); + setResult({ kind: 'quit' }); + + await pickDatabaseScope(baseArgs({ defaultSchemas: ['analytics'] }), makeIo().io, renderer); + + expect([...(capture.state?.checked ?? [])]).toEqual(['analytics']); + }); + + it('collapses an existing full-schema selection back into the parent check', async () => { + const { renderer, capture, setResult } = captureRenderer(); + setResult({ kind: 'quit' }); + + await pickDatabaseScope( + baseArgs({ existing: { enabledTables: ['analytics.customers', 'analytics.orders'] } }), + makeIo().io, + renderer, + ); + + expect([...(capture.state?.checked ?? [])]).toEqual(['analytics']); + }); + + it('keeps a partial existing selection at the leaf level', async () => { + const { renderer, capture, setResult } = captureRenderer(); + setResult({ kind: 'quit' }); + + await pickDatabaseScope( + baseArgs({ existing: { enabledTables: ['analytics.customers'] } }), + makeIo().io, + renderer, + ); + + expect([...(capture.state?.checked ?? [])]).toEqual(['analytics.customers']); + }); + + it('expands a selected schema parent into all its tables and derives activeSchemas', async () => { + const { renderer, setResult } = captureRenderer(); + setResult({ kind: 'save', selectedIds: ['analytics'] }); + + const result = await pickDatabaseScope(baseArgs(), makeIo().io, renderer); + + expect(result).toEqual({ + kind: 'selected', + activeSchemas: ['analytics'], + enabledTables: ['analytics.customers', 'analytics.orders'], + }); + }); + + it('combines parent and individual leaf selections without duplicate tables', async () => { + const { renderer, setResult } = captureRenderer(); + setResult({ kind: 'save', selectedIds: ['analytics', 'public.events'] }); + + const result = await pickDatabaseScope(baseArgs(), makeIo().io, renderer); + + expect(result).toEqual({ + kind: 'selected', + activeSchemas: ['analytics', 'public'], + enabledTables: ['analytics.customers', 'analytics.orders', 'public.events'], + }); + }); + + it('treats empty save as enable-all', async () => { + const { renderer, setResult } = captureRenderer(); + setResult({ kind: 'save', selectedIds: [] }); + + const result = await pickDatabaseScope(baseArgs(), makeIo().io, renderer); + + expect(result).toEqual({ + kind: 'selected', + activeSchemas: ['analytics', 'public'], + enabledTables: [ + 'analytics.customers', + 'analytics.orders', + 'public.events', + 'public.sessions', + ], + }); + }); + + it('omits activeSchemas when the driver does not support a schema scope', async () => { + const { renderer, setResult } = captureRenderer(); + setResult({ kind: 'save', selectedIds: ['analytics'] }); + + const result = await pickDatabaseScope( + baseArgs({ supportsSchemaScope: false }), + makeIo().io, + renderer, + ); + + expect(result).toEqual({ + kind: 'selected', + activeSchemas: [], + enabledTables: ['analytics.customers', 'analytics.orders'], + }); + }); + + it('returns back when the picker quits', async () => { + const { renderer, setResult } = captureRenderer(); + setResult({ kind: 'quit' }); + + const result = await pickDatabaseScope(baseArgs(), makeIo().io, renderer); + + expect(result).toEqual({ kind: 'back' }); + }); +}); diff --git a/packages/cli/src/database-tree-picker.ts b/packages/cli/src/database-tree-picker.ts new file mode 100644 index 00000000..d494003d --- /dev/null +++ b/packages/cli/src/database-tree-picker.ts @@ -0,0 +1,210 @@ +import type { KtxTableListEntry } from '@ktx/context/scan'; +import type { KtxCliIo } from './cli-runtime.js'; +import { profileMark } from './startup-profile.js'; +import { + buildInitialState, + buildPickerTree, + type PickerState, + type TreePickerNode, + type TreePickerNodeInput, +} from './tree-picker-state.js'; +import { + renderTreePickerTui, + type TreePickerChrome, + type TreePickerResult, + type TreePickerTuiIo, +} from './tree-picker-tui.js'; + +profileMark('module:database-tree-picker'); + +const DATABASE_SCRIPTED_MODE_HINT = + 'Database picker requires a TTY. Use --no-input and the relevant flags for scripted mode.'; + +export type DatabaseTreePickerRenderer = ( + chrome: TreePickerChrome, + initialState: PickerState, + io: TreePickerTuiIo, +) => Promise; + +function defaultRenderer( + chrome: TreePickerChrome, + initialState: PickerState, + io: TreePickerTuiIo, +): Promise { + return renderTreePickerTui({ chrome, initialState }, io, { scriptedModeHint: DATABASE_SCRIPTED_MODE_HINT }); +} + +export type DatabaseScopePickResult = + | { kind: 'selected'; activeSchemas: string[]; enabledTables: string[] } + | { kind: 'back' }; + +export interface PickDatabaseScopeArgs { + connectionId: string; + schemaNoun: string; + schemaNounPlural: string; + discovered: readonly KtxTableListEntry[]; + existing: { enabledTables: readonly string[] }; + defaultSchemas: readonly string[]; + supportsSchemaScope: boolean; +} + +function qualifiedTableId(entry: KtxTableListEntry): string { + return `${entry.schema}.${entry.name}`; +} + +function tableTitle(entry: KtxTableListEntry): string { + return entry.kind === 'view' ? `${entry.name} (view)` : entry.name; +} + +function buildTreeInputs(discovered: readonly KtxTableListEntry[]): { + inputs: TreePickerNodeInput[]; + schemaIds: string[]; + allTables: string[]; +} { + const schemaSeen = new Set(); + const schemaIds: string[] = []; + for (const entry of discovered) { + if (!schemaSeen.has(entry.schema)) { + schemaSeen.add(entry.schema); + schemaIds.push(entry.schema); + } + } + const inputs: TreePickerNodeInput[] = []; + for (const schema of schemaIds) { + inputs.push({ id: schema, title: schema, archived: false, parentId: null }); + } + for (const entry of discovered) { + inputs.push({ + id: qualifiedTableId(entry), + title: tableTitle(entry), + archived: false, + parentId: entry.schema, + }); + } + return { inputs, schemaIds, allTables: discovered.map(qualifiedTableId) }; +} + +function initialSelectionForExisting( + existing: readonly string[], + byId: Map, +): string[] { + const tableIds = new Set( + [...byId.values()].filter((node) => node.parentId !== null).map((node) => node.id), + ); + const existingTables = new Set(existing.filter((id) => tableIds.has(id))); + const schemaChildren = new Map(); + for (const node of byId.values()) { + if (node.parentId === null && node.childIds.length > 0) { + schemaChildren.set(node.id, [...node.childIds]); + } + } + const result: string[] = []; + for (const [schema, children] of schemaChildren) { + const allChecked = children.length > 0 && children.every((childId) => existingTables.has(childId)); + if (allChecked) { + result.push(schema); + for (const childId of children) { + existingTables.delete(childId); + } + } + } + for (const id of existingTables) { + result.push(id); + } + return result; +} + +function initialSelectionFromDefaults( + defaultSchemas: readonly string[], + schemaIds: readonly string[], +): string[] { + const valid = new Set(schemaIds); + const filtered = defaultSchemas.filter((s) => valid.has(s)); + return filtered.length > 0 ? filtered : [...schemaIds]; +} + +function expandSelectedToTables( + selectedIds: readonly string[], + byId: Map, +): string[] { + const expanded: string[] = []; + const seen = new Set(); + for (const id of selectedIds) { + const node = byId.get(id); + if (!node) continue; + if (node.childIds.length === 0) { + if (node.parentId !== null && !seen.has(id)) { + seen.add(id); + expanded.push(id); + } + continue; + } + for (const childId of node.childIds) { + if (!seen.has(childId)) { + seen.add(childId); + expanded.push(childId); + } + } + } + return expanded; +} + +function schemasFromEnabledTables(enabledTables: readonly string[]): string[] { + const seen = new Set(); + const result: string[] = []; + for (const qualified of enabledTables) { + const schema = qualified.split('.')[0] ?? ''; + if (schema.length === 0 || seen.has(schema)) continue; + seen.add(schema); + result.push(schema); + } + return result; +} + +export async function pickDatabaseScope( + args: PickDatabaseScopeArgs, + io: KtxCliIo, + render: DatabaseTreePickerRenderer = defaultRenderer, +): Promise { + const { inputs, schemaIds, allTables } = buildTreeInputs(args.discovered); + const tree = buildPickerTree(inputs); + const byId = new Map(tree.map((node) => [node.id, node])); + const tableCount = allTables.length; + const schemaCount = schemaIds.length; + + const initialSelection = + args.existing.enabledTables.length > 0 + ? initialSelectionForExisting(args.existing.enabledTables, byId) + : initialSelectionFromDefaults(args.defaultSchemas, schemaIds); + + const initialState = buildInitialState({ + tree, + existingSelectedIds: initialSelection, + skipEmptyAction: 'save-empty', + }); + + const schemaWordPlural = schemaCount === 1 ? args.schemaNoun : args.schemaNounPlural; + const subtitleLines = [ + `Connection: ${args.connectionId}`, + `Found ${tableCount} ${tableCount === 1 ? 'table' : 'tables'} across ${schemaCount} ${schemaWordPlural}.`, + `Toggle a ${args.schemaNoun} to enable all of its tables, or expand to pick individual tables.`, + ]; + + const chrome: TreePickerChrome = { + title: `Choose tables to enable for ${args.connectionId}`, + subtitleLines, + skipEmptyMessage: + 'Nothing selected. Enable all tables? Press Enter to enable all or Escape to go back.', + }; + + const result = await render(chrome, initialState, io as TreePickerTuiIo); + if (result.kind === 'quit') { + return { kind: 'back' }; + } + + const enabledTables = + result.selectedIds.length === 0 ? allTables : expandSelectedToTables(result.selectedIds, byId); + const activeSchemas = args.supportsSchemaScope ? schemasFromEnabledTables(enabledTables) : []; + + return { kind: 'selected', activeSchemas, enabledTables }; +} diff --git a/packages/cli/src/notion-page-picker-tui.test.tsx b/packages/cli/src/notion-page-picker-tui.test.tsx deleted file mode 100644 index 16ad93db..00000000 --- a/packages/cli/src/notion-page-picker-tui.test.tsx +++ /dev/null @@ -1,392 +0,0 @@ -/* @jsxImportSource react */ -import { render as renderInkTest } from 'ink-testing-library'; -import { type ReactNode } from 'react'; -import { describe, expect, it, vi } from 'vitest'; -import { buildInitialState, buildPickerTree, type NotionPickerPageInput } from './notion-page-picker-tree.js'; -import { - NotionPickerApp, - notionPickerCommandForInkInput, - renderNotionPickerTui, - resolveNotionPickerWidth, - sanitizeNotionPickerTuiError, - windowItems, - windowOffset, - type NotionPickerInkInstance, - type NotionPickerInkRenderOptions, -} from './notion-page-picker-tui.js'; - -const IDS = { - engineering: '11111111-1111-1111-1111-111111111111', - architecture: '22222222-2222-2222-2222-222222222222', - marketing: '33333333-3333-3333-3333-333333333333', - finance: '44444444-4444-4444-4444-444444444444', - ops: '55555555-5555-5555-5555-555555555555', - sales: '66666666-6666-6666-6666-666666666666', - support: '77777777-7777-7777-7777-777777777777', - product: '88888888-8888-8888-8888-888888888888', - design: '99999999-9999-9999-9999-999999999999', -}; - -function pages(): NotionPickerPageInput[] { - return [ - { id: IDS.engineering, title: 'Engineering Docs', archived: false, parentId: null }, - { id: IDS.architecture, title: 'Architecture', archived: false, parentId: IDS.engineering }, - { id: IDS.marketing, title: 'Marketing', archived: false, parentId: null }, - ]; -} - -function manyPages(): NotionPickerPageInput[] { - return [ - { id: IDS.engineering, title: 'Engineering Docs', archived: false, parentId: null }, - { id: IDS.architecture, title: 'Architecture', archived: false, parentId: IDS.engineering }, - { id: IDS.marketing, title: 'Marketing', archived: false, parentId: null }, - { id: IDS.finance, title: 'Finance', archived: false, parentId: null }, - { id: IDS.ops, title: 'Operations', archived: false, parentId: null }, - { id: IDS.sales, title: 'Sales', archived: false, parentId: null }, - { id: IDS.support, title: 'Support', archived: false, parentId: null }, - { id: IDS.product, title: 'Product', archived: false, parentId: null }, - { id: IDS.design, title: 'Design', archived: false, parentId: null }, - ]; -} - -function state(mode: 'all_accessible' | 'selected_roots' = 'selected_roots') { - return buildInitialState({ - tree: buildPickerTree(pages()), - existingRootPageIds: [], - currentCrawlMode: mode, - }); -} - -async function waitForInkInput(): Promise { - await new Promise((resolve) => setTimeout(resolve, 10)); -} - -function fakeInkInstance(): NotionPickerInkInstance { - return { - rerender: vi.fn(), - unmount: vi.fn(), - waitUntilExit: vi.fn(async () => undefined), - }; -} - -function normalizeFrameWrap(frame: string | undefined): string { - return frame?.replace(/\n/g, ' ').replace(/│ /g, '').replace(/ +/g, ' ') ?? ''; -} - -describe('notionPickerCommandForInkInput', () => { - it('maps browse, search, and confirm input to reducer commands', () => { - expect(notionPickerCommandForInkInput('', { downArrow: true }, state().search, null)).toBe('cursor-down'); - expect(notionPickerCommandForInkInput('', { upArrow: true }, state().search, null)).toBe('cursor-up'); - expect(notionPickerCommandForInkInput('', { rightArrow: true }, state().search, null)).toBe('cursor-right'); - expect(notionPickerCommandForInkInput('', { leftArrow: true }, state().search, null)).toBe('cursor-left'); - expect(notionPickerCommandForInkInput(' ', {}, state().search, null)).toBe('toggle-check'); - expect(notionPickerCommandForInkInput('/', {}, state().search, null)).toBe('search-start'); - expect(notionPickerCommandForInkInput('a', {}, state().search, null)).toBe('select-all-visible'); - expect(notionPickerCommandForInkInput('n', {}, state().search, null)).toBe('select-none'); - expect(notionPickerCommandForInkInput('', { return: true }, state().search, null)).toBe('save-request'); - expect(notionPickerCommandForInkInput('', { escape: true }, state().search, null)).toBe('quit'); - expect(notionPickerCommandForInkInput('c', { ctrl: true }, state().search, null)).toBe('quit'); - expect(notionPickerCommandForInkInput('s', {}, state().search, null)).toBeNull(); - expect(notionPickerCommandForInkInput('q', {}, state().search, null)).toBeNull(); - - expect(notionPickerCommandForInkInput('x', {}, { editing: true, query: '' }, null)).toEqual({ - type: 'search-input', - value: 'x', - }); - expect(notionPickerCommandForInkInput('', { backspace: true }, { editing: true, query: 'x' }, null)).toBe( - 'search-backspace', - ); - expect(notionPickerCommandForInkInput('', { return: true }, { editing: true, query: 'x' }, null)).toBe( - 'search-submit', - ); - expect(notionPickerCommandForInkInput('', { escape: true }, { editing: true, query: 'x' }, null)).toBe( - 'search-cancel', - ); - - expect(notionPickerCommandForInkInput('y', {}, state().search, 'mode-switch')).toBe('save-confirm'); - expect(notionPickerCommandForInkInput('', { return: true }, state().search, 'mode-switch')).toBe('save-confirm'); - expect(notionPickerCommandForInkInput('n', {}, state().search, 'mode-switch')).toBe('save-cancel'); - }); -}); - -describe('window helpers', () => { - it('centers the selected row and returns the visible slice', () => { - expect(windowOffset(20, 10, 5)).toBe(8); - expect(windowItems(['a', 'b', 'c', 'd', 'e'], 3, 3)).toEqual({ items: ['c', 'd', 'e'], offset: 2 }); - }); - - it('clamps picker width to the design rule', () => { - expect(resolveNotionPickerWidth(200)).toBe(120); - expect(resolveNotionPickerWidth(100)).toBe(96); - expect(resolveNotionPickerWidth(50)).toBe(60); - expect(resolveNotionPickerWidth(undefined)).toBe(96); - }); -}); - -describe('NotionPickerApp', () => { - it('renders spec banners, row glyphs, search visibility, and hint text', () => { - const initialState = { - ...state('all_accessible'), - preLoadWarnings: ['1 stored root_page_ids no longer visible'], - }; - const { lastFrame } = renderInkTest( - , - ); - - const frame = lastFrame() ?? ''; - expect(frame).toContain('Select Notion pages to ingest'); - expect(frame).toContain('Workspace: Design Workspace'); - expect(frame).toContain('5000-page cap reached - some pages not shown'); - expect(frame).toContain('1 stored root_page_ids no longer visible - they will be removed if you save'); - expect(frame).toContain('◻ Engineering Docs ▸ (1)'); - expect(frame).toContain('◻ Marketing'); - expect(frame).not.toContain('Search ready: -'); - expect(normalizeFrameWrap(frame)).toContain( - 'Right Arrow to expand, Up/Down to move, Space to select or unselect, Slash to filter, Enter to confirm, Escape to go back, or Ctrl+C to exit.', - ); - }); - - it('renders partial discovery warnings without stale-root save suffix', () => { - const initialState = { - ...state(), - preLoadWarnings: ['Notion search stopped early: rate limit after first page'], - }; - const { lastFrame } = renderInkTest( - , - ); - - const frame = lastFrame() ?? ''; - expect(frame).toContain('Notion search stopped early: rate limit after first page'); - expect(frame).not.toContain( - 'Notion search stopped early: rate limit after first page - they will be removed if you save', - ); - }); - - it('renders checked parents and locked descendants with the locked design glyphs', () => { - const initialState = { - ...state(), - checked: new Set([IDS.engineering]), - expanded: new Set([IDS.engineering]), - }; - const { lastFrame } = renderInkTest( - , - ); - - const frame = lastFrame() ?? ''; - expect(frame).toContain('◼ Engineering Docs ▾'); - expect(frame).toContain(' ◼ Architecture'); - }); - - it('supports keyboard selection, all_accessible confirmation, and save callback', async () => { - const onExit = vi.fn(); - const { stdin, lastFrame } = renderInkTest( - , - ); - - stdin.write(' '); - await waitForInkInput(); - expect(lastFrame()).toContain('◼ Engineering Docs'); - - stdin.write('\r'); - await waitForInkInput(); - expect(normalizeFrameWrap(lastFrame())).toContain( - 'Switch crawl_mode from all_accessible to selected_roots? Will limit ingest to 1 selected page. Press Enter to confirm or Escape to go back.', - ); - - stdin.write('y'); - await waitForInkInput(); - expect(onExit).toHaveBeenCalledWith({ kind: 'save', rootPageIds: [IDS.engineering] }); - }); - - it('prompts skip-empty confirmation on empty submit and dismisses on cancel', async () => { - const onExit = vi.fn(); - const { stdin, lastFrame } = renderInkTest( - , - ); - - stdin.write('\r'); - await waitForInkInput(); - expect(normalizeFrameWrap(lastFrame())).toContain( - 'Nothing selected. Skip this step? Press Enter to skip or Escape to go back.', - ); - expect(onExit).not.toHaveBeenCalled(); - - stdin.write('n'); - await waitForInkInput(); - expect(lastFrame()).not.toContain('Nothing selected. Skip this step?'); - expect(onExit).not.toHaveBeenCalled(); - - stdin.write('\r'); - await waitForInkInput(); - expect(lastFrame()).toContain('Nothing selected. Skip this step?'); - - stdin.write('\r'); - await waitForInkInput(); - expect(onExit).toHaveBeenCalledWith({ kind: 'quit' }); - }); - - it('renders row-window overflow indicators when the visible list is clipped', async () => { - const onExit = vi.fn(); - const initialState = buildInitialState({ - tree: buildPickerTree(manyPages()), - existingRootPageIds: [], - currentCrawlMode: 'selected_roots', - }); - initialState.expanded = new Set([IDS.engineering]); - const { stdin, lastFrame } = renderInkTest( - , - ); - - expect(lastFrame()).toContain('↓ 4 more'); - - stdin.write('\u001B[B'); - stdin.write('\u001B[B'); - stdin.write('\u001B[B'); - stdin.write('\u001B[B'); - await waitForInkInput(); - - const frame = lastFrame() ?? ''; - expect(frame).toContain('↑ '); - expect(frame).toContain('↓ '); - expect(onExit).not.toHaveBeenCalled(); - }); - - it('returns quit without saving', async () => { - const onExit = vi.fn(); - const { stdin } = renderInkTest( - , - ); - - stdin.write('\u0003'); - await waitForInkInput(); - expect(onExit).toHaveBeenCalledWith({ kind: 'quit' }); - }); -}); - -describe('renderNotionPickerTui', () => { - it('returns the app result from the Ink runtime', async () => { - const io = { - stdin: { isTTY: true, setRawMode: vi.fn() }, - stdout: { isTTY: true, columns: 100, rows: 24, write: vi.fn() }, - stderr: { write: vi.fn() }, - }; - const renderInk = vi.fn((_tree: ReactNode, _options: NotionPickerInkRenderOptions) => fakeInkInstance()); - - await expect( - renderNotionPickerTui( - { - initialState: state(), - connectionId: 'notion-main', - workspaceLabel: 'Design Workspace', - cappedAtCount: null, - currentCrawlMode: 'selected_roots', - }, - io, - { renderInk }, - ), - ).resolves.toEqual({ kind: 'quit' }); - expect(renderInk).toHaveBeenCalledOnce(); - }); - - it('sanitizes render errors and tells the user to use no-input mode', async () => { - expect(sanitizeNotionPickerTuiError(new Error('token=secret https://api.notion.com/v1/search'))).toBe( - '[redacted] [redacted-url]', - ); - }); - - it('falls back to quit with a scripted-mode hint when Ink cannot initialize', async () => { - let stderr = ''; - const io = { - stdin: { isTTY: false, setRawMode: vi.fn() }, - stdout: { isTTY: false, columns: 100, rows: 24, write: vi.fn() }, - stderr: { - write(chunk: string) { - stderr += chunk; - }, - }, - }; - - await expect( - renderNotionPickerTui( - { - initialState: state(), - connectionId: 'notion-main', - workspaceLabel: 'Design Workspace', - cappedAtCount: null, - currentCrawlMode: 'selected_roots', - }, - io, - { - renderInk: vi.fn(() => { - throw new Error('token=secret'); - }), - }, - ), - ).resolves.toEqual({ kind: 'quit' }); - expect(stderr).toContain('Use --no-input --notion-root-page-id for scripted mode'); - expect(stderr).not.toContain('secret'); - }); -}); diff --git a/packages/cli/src/notion-page-picker.test.ts b/packages/cli/src/notion-page-picker.test.ts index 77710716..29f5a352 100644 --- a/packages/cli/src/notion-page-picker.test.ts +++ b/packages/cli/src/notion-page-picker.test.ts @@ -1,4 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; +import type { PickerState } from './tree-picker-state.js'; +import type { TreePickerChrome, TreePickerResult, TreePickerTuiIo } from './tree-picker-tui.js'; import { discoverNotionPickerPages, notionPickerPageFromSearchResult, @@ -6,8 +8,6 @@ import { pickNotionRootPages, resolveNotionWorkspaceLabel, type NotionPickerApi, - type PickerRenderInput, - type PickerRenderResult, } from './notion-page-picker.js'; function makeIo() { @@ -162,20 +162,27 @@ describe('Notion page picker helpers', () => { }); }); +type RenderPickerArgs = [TreePickerChrome, PickerState, TreePickerTuiIo]; + describe('pickNotionRootPages', () => { it('discovers visible pages, warns about stale roots, renders the TUI, and returns selected roots', async () => { const api = fakeNotionApi([ notionPage(PAGE_IDS.engineering, 'Engineering'), notionPage(PAGE_IDS.architecture, 'Architecture', PAGE_IDS.engineering), ]); - const renderPicker = vi.fn(async (input: PickerRenderInput): Promise => { - expect(input.connectionId).toBe('notion-main'); - expect(input.workspaceLabel).toBe('Design Workspace'); - expect(input.currentCrawlMode).toBe('all_accessible'); - expect(input.cappedAtCount).toBeNull(); - expect(input.initialState.preLoadWarnings).toEqual(['1 stored root_page_ids no longer visible']); - return { kind: 'save', rootPageIds: [PAGE_IDS.engineering] }; - }); + const renderPicker = vi.fn( + async (chrome: TreePickerChrome, state: PickerState): Promise => { + expect(chrome.title).toBe('Select Notion pages to ingest'); + expect(chrome.subtitleLines).toEqual(['Workspace: Design Workspace']); + expect(chrome.warningLines ?? []).toEqual([]); + expect(chrome.confirmSaveMessage).toBeTypeOf('function'); + expect(state.requireConfirmOnSave).toBe(true); + expect(state.preLoadWarnings).toEqual([ + '1 stored root_page_ids no longer visible - they will be removed if you save', + ]); + return { kind: 'save', selectedIds: [PAGE_IDS.engineering] }; + }, + ); const io = makeIo(); await expect( @@ -223,7 +230,7 @@ describe('pickNotionRootPages', () => { makeIo().io, { createNotionApi, - renderPicker: vi.fn(async (): Promise => ({ kind: 'quit' })), + renderPicker: vi.fn(async (): Promise => ({ kind: 'quit' })), }, ), ).resolves.toEqual({ kind: 'back' }); @@ -243,11 +250,13 @@ describe('pickNotionRootPages', () => { .mockRejectedValueOnce(new Error('rate limit after first page')), retrieveBotUser: vi.fn(async () => ({ name: 'Notion bot', bot: { workspace_name: 'Design Workspace' } })), }; - let renderInput: PickerRenderInput | undefined; - const renderPicker = vi.fn(async (input: PickerRenderInput): Promise => { - renderInput = input; - return { kind: 'quit' }; - }); + let captured: RenderPickerArgs | undefined; + const renderPicker = vi.fn( + async (chrome: TreePickerChrome, state: PickerState, io: TreePickerTuiIo): Promise => { + captured = [chrome, state, io]; + return { kind: 'quit' }; + }, + ); const io = makeIo(); await expect( @@ -271,11 +280,12 @@ describe('pickNotionRootPages', () => { ).resolves.toEqual({ kind: 'back' }); expect(renderPicker).toHaveBeenCalledOnce(); - if (!renderInput) { + if (!captured) { throw new Error('renderPicker was not called'); } - expect(renderInput.initialState.preLoadWarnings).toEqual(['Notion search stopped early: rate limit after first page']); - expect(renderInput.initialState.tree.map((node) => node.title)).toEqual(['Engineering']); + const [, state] = captured; + expect(state.preLoadWarnings).toEqual(['Notion search stopped early: rate limit after first page']); + expect(state.tree.map((node) => node.title)).toEqual(['Engineering']); expect(io.stderr()).toContain('Notion search stopped early: rate limit after first page'); }); @@ -300,7 +310,7 @@ describe('pickNotionRootPages', () => { }), retrieveBotUser: vi.fn(async () => ({ name: 'Notion bot' })), })), - renderPicker: vi.fn(async (): Promise => ({ kind: 'quit' })), + renderPicker: vi.fn(async (): Promise => ({ kind: 'quit' })), }, ), ).resolves.toEqual({ kind: 'unavailable', message: 'Notion API unavailable' }); diff --git a/packages/cli/src/notion-page-picker.ts b/packages/cli/src/notion-page-picker.ts index 807c0fc0..26e561f5 100644 --- a/packages/cli/src/notion-page-picker.ts +++ b/packages/cli/src/notion-page-picker.ts @@ -3,13 +3,19 @@ import { type NotionApi, type NotionBotInfo, NotionClient } from '@ktx/context/i import type { KtxProjectConnectionConfig } from '@ktx/context/project'; import type { KtxCliIo } from './cli-runtime.js'; import { profileMark } from './startup-profile.js'; -import { buildInitialState, buildPickerTree, type NotionPickerPageInput } from './notion-page-picker-tree.js'; import { - type NotionPickerTuiIo, - type PickerRenderInput, - type PickerRenderResult, - renderNotionPickerTui, -} from './notion-page-picker-tui.js'; + buildInitialState, + buildPickerTree, + flattenSelection, + type PickerState, + type TreePickerNodeInput, +} from './tree-picker-state.js'; +import { + renderTreePickerTui, + type TreePickerChrome, + type TreePickerResult, + type TreePickerTuiIo, +} from './tree-picker-tui.js'; profileMark('module:notion-page-picker'); @@ -19,8 +25,6 @@ export interface PickNotionRootPagesArgs { } export type NotionPickerApi = Pick; -export type { PickerRenderInput, PickerRenderResult }; - export type NotionRootPagePickResult = | { kind: 'selected'; rootPageIds: string[] } | { kind: 'back' } @@ -29,10 +33,16 @@ export type NotionRootPagePickResult = export interface NotionRootPagePickerDeps { env?: Record; createNotionApi?: (authToken: string) => NotionPickerApi; - renderPicker?: (input: PickerRenderInput, io: NotionPickerTuiIo) => Promise; + renderPicker?: ( + chrome: TreePickerChrome, + initialState: PickerState, + io: TreePickerTuiIo, + ) => Promise; } const NOTION_PICKER_PAGE_CAP = 5000; +const NOTION_SCRIPTED_MODE_HINT = + 'Notion picker requires a TTY. Use --no-input --notion-root-page-id for scripted mode.'; function assertSafeNotionPickerConnectionId(connectionId: string): void { if (!/^[a-zA-Z0-9][a-zA-Z0-9_-]*$/.test(connectionId)) { @@ -50,6 +60,14 @@ export function normalizeNotionPageId(value: string): string { return `${lower.slice(0, 8)}-${lower.slice(8, 12)}-${lower.slice(12, 16)}-${lower.slice(16, 20)}-${lower.slice(20)}`; } +function tryNormalizeNotionPageId(value: string): string | null { + try { + return normalizeNotionPageId(value); + } catch { + return null; + } +} + function recordValue(value: unknown): Record | null { return typeof value === 'object' && value !== null && !Array.isArray(value) ? (value as Record) @@ -88,7 +106,7 @@ function extractParentPageId(page: Record): string | null { return normalizeNotionPageId(parent.page_id); } -export function notionPickerPageFromSearchResult(result: Record): NotionPickerPageInput { +export function notionPickerPageFromSearchResult(result: Record): TreePickerNodeInput { const id = typeof result.id === 'string' ? normalizeNotionPageId(result.id) : ''; if (!id) { throw new Error('Notion page search result is missing id'); @@ -104,9 +122,9 @@ export function notionPickerPageFromSearchResult(result: Record export async function discoverNotionPickerPages( api: NotionPickerApi, options: { cap?: number } = {}, -): Promise<{ pages: NotionPickerPageInput[]; cappedAtCount: number | null; warnings: string[] }> { +): Promise<{ pages: TreePickerNodeInput[]; cappedAtCount: number | null; warnings: string[] }> { const cap = options.cap ?? NOTION_PICKER_PAGE_CAP; - const pages: NotionPickerPageInput[] = []; + const pages: TreePickerNodeInput[] = []; const warnings: string[] = []; let cursor: string | null | undefined = null; @@ -171,6 +189,33 @@ function notionCrawlMode(connection: KtxProjectConnectionConfig): 'all_accessibl return connection.crawl_mode === 'all_accessible' ? 'all_accessible' : 'selected_roots'; } +function selectedPageCountText(count: number): string { + return `${count} selected ${count === 1 ? 'page' : 'pages'}`; +} + +function notionChrome(args: { + workspaceLabel: string; + cappedAtCount: number | null; + currentCrawlMode: 'all_accessible' | 'selected_roots'; +}): TreePickerChrome { + const warningLines: string[] = []; + if (args.cappedAtCount) { + warningLines.push(`${args.cappedAtCount}-page cap reached - some pages not shown`); + } + return { + title: 'Select Notion pages to ingest', + subtitleLines: [`Workspace: ${args.workspaceLabel}`], + warningLines, + confirmSaveMessage: + args.currentCrawlMode === 'all_accessible' + ? (state) => + `Switch crawl_mode from all_accessible to selected_roots? Will limit ingest to ${selectedPageCountText( + flattenSelection(state.checked, state.byId).length, + )}. Press Enter to confirm or Escape to go back.` + : undefined, + }; +} + export async function pickNotionRootPages( args: PickNotionRootPagesArgs, io: KtxCliIo = process, @@ -190,10 +235,14 @@ export async function pickNotionRootPages( const api = deps.createNotionApi ? deps.createNotionApi(authToken) : new NotionClient(authToken); const discovery = await discoverNotionPickerPages(api); const tree = buildPickerTree(discovery.pages); + const normalizedExistingIds = stringArray(args.connection.root_page_ids) + .map((raw) => tryNormalizeNotionPageId(raw)) + .filter((id): id is string => id !== null); const initialState = buildInitialState({ tree, - existingRootPageIds: stringArray(args.connection.root_page_ids), - currentCrawlMode: crawlMode, + existingSelectedIds: normalizedExistingIds, + requireConfirmOnSave: crawlMode === 'all_accessible', + staleWarning: (count) => `${count} stored root_page_ids no longer visible - they will be removed if you save`, }); const preLoadWarnings = [...discovery.warnings, ...initialState.preLoadWarnings]; const renderState = @@ -207,23 +256,25 @@ export async function pickNotionRootPages( io.stderr.write(`${warning}\n`); } const workspaceLabel = await resolveNotionWorkspaceLabel(api, args.connectionId); - const result = await (deps.renderPicker ?? renderNotionPickerTui)( - { - initialState: renderState, - connectionId: args.connectionId, - workspaceLabel, - cappedAtCount: discovery.cappedAtCount, - currentCrawlMode: crawlMode, - }, - io as NotionPickerTuiIo, - ); + const chrome = notionChrome({ + workspaceLabel, + cappedAtCount: discovery.cappedAtCount, + currentCrawlMode: crawlMode, + }); + const renderPicker = + deps.renderPicker ?? + ((chromeArg, state, ioArg) => + renderTreePickerTui({ chrome: chromeArg, initialState: state }, ioArg, { + scriptedModeHint: NOTION_SCRIPTED_MODE_HINT, + })); + const result = await renderPicker(chrome, renderState, io as TreePickerTuiIo); if (result.kind === 'quit') { return { kind: 'back' }; } - if (result.rootPageIds.length === 0) { + if (result.selectedIds.length === 0) { return { kind: 'unavailable', message: 'Notion picker did not return any selected pages.' }; } - return { kind: 'selected', rootPageIds: result.rootPageIds }; + return { kind: 'selected', rootPageIds: result.selectedIds }; } catch (error) { return { kind: 'unavailable', message: error instanceof Error ? error.message : String(error) }; } diff --git a/packages/cli/src/setup-databases.test.ts b/packages/cli/src/setup-databases.test.ts index fa4ca3f2..d3a55fba 100644 --- a/packages/cli/src/setup-databases.test.ts +++ b/packages/cli/src/setup-databases.test.ts @@ -5,10 +5,15 @@ import { initKtxProject, parseKtxProjectConfig, readKtxSetupState, writeKtxSetup import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { type KtxSetupDatabaseDriver, + type KtxSetupDatabasesDeps, type KtxSetupDatabasesPromptAdapter, runKtxSetupDatabasesStep, } from './setup-databases.js'; import type { KtxCliIo } from './cli-runtime.js'; +import type { + DatabaseScopePickResult, + PickDatabaseScopeArgs, +} from './database-tree-picker.js'; function makeIo() { let stdout = ''; @@ -32,6 +37,43 @@ function makeIo() { }; } +type ScopePick = + | 'back' + | 'enable-all' + | { schemas: string[]; tables: string[] }; + +interface PickerStubs { + pickDatabaseScope: KtxSetupDatabasesDeps['pickDatabaseScope']; + scopeCalls: PickDatabaseScopeArgs[]; +} + +function makePickerStubs(options: { scopes?: ScopePick[] } = {}): PickerStubs { + const queue: ScopePick[] = [...(options.scopes ?? [])]; + const scopeCalls: PickDatabaseScopeArgs[] = []; + return { + scopeCalls, + pickDatabaseScope: vi.fn(async (args: PickDatabaseScopeArgs): Promise => { + scopeCalls.push(args); + const next = queue.shift(); + if (next === undefined || next === 'enable-all') { + const enabledTables = args.discovered.map((t) => `${t.schema}.${t.name}`); + const activeSchemas = args.supportsSchemaScope + ? Array.from(new Set(args.discovered.map((t) => t.schema))) + : []; + return { kind: 'selected', activeSchemas, enabledTables }; + } + if (next === 'back') { + return { kind: 'back' }; + } + return { + kind: 'selected', + activeSchemas: args.supportsSchemaScope ? next.schemas : [], + enabledTables: next.tables, + }; + }), + }; +} + function makePromptAdapter(options: { multiselectValues?: string[][]; selectValues?: string[]; @@ -819,7 +861,6 @@ describe('setup databases step', () => { await writeKtxSetupState(tempDir, { completed_steps: ['databases'] }); const prompts = makePromptAdapter({ textValues: ['env:DATABASE_URL'], - multiselectValues: [['analytics']], }); let primaryMenuCount = 0; vi.mocked(prompts.select).mockImplementation(async (options) => { @@ -835,11 +876,21 @@ describe('setup databases step', () => { const scanConnection = vi.fn(async () => 0); const listSchemas = vi.fn(async () => ['analytics', 'public']); const listTables = vi.fn(async () => [{ schema: 'analytics', name: 'customers', kind: 'table' as const }]); + const pickers = makePickerStubs({ + scopes: [{ schemas: ['analytics'], tables: ['analytics.customers'] }], + }); const result = await runKtxSetupDatabasesStep( { projectDir: tempDir, inputMode: 'auto', skipDatabases: false, databaseSchemas: [] }, makeIo().io, - { prompts, testConnection, scanConnection, listSchemas, listTables }, + { + prompts, + testConnection, + scanConnection, + listSchemas, + listTables, + pickDatabaseScope: pickers.pickDatabaseScope, + }, ); expect(result).toEqual({ status: 'ready', projectDir: tempDir, connectionIds: ['warehouse'] }); @@ -848,7 +899,7 @@ describe('setup databases step', () => { placeholder: 'env:DATABASE_URL', initialValue: 'env:DATABASE_URL', }); - expect(listTables).toHaveBeenCalledWith(tempDir, 'warehouse'); + expect(listTables).toHaveBeenCalledWith(tempDir, 'warehouse', ['analytics', 'public']); expect(testConnection).toHaveBeenCalledWith(tempDir, 'warehouse', expect.anything()); expect(scanConnection).toHaveBeenCalledWith(tempDir, 'warehouse', expect.anything()); const config = parseKtxProjectConfig(await readFile(join(tempDir, 'ktx.yaml'), 'utf-8')); @@ -882,7 +933,6 @@ describe('setup databases step', () => { await writeKtxSetupState(tempDir, { completed_steps: ['databases'] }); const prompts = makePromptAdapter({ textValues: ['env:DATABASE_URL'], - multiselectValues: [['public'], ['public.customers', 'public.orders']], }); let primaryMenuCount = 0; vi.mocked(prompts.select).mockImplementation(async (options) => { @@ -892,7 +942,6 @@ describe('setup databases step', () => { } if (options.message === 'Primary source to edit') return 'warehouse'; if (options.message === 'How do you want to connect to PostgreSQL?') return 'url'; - if (options.message.startsWith('Tables found in selected schemas')) return 'customize'; return 'back'; }); const listSchemas = vi.fn(async () => ['orbit_analytics', 'orbit_raw', 'public']); @@ -901,6 +950,9 @@ describe('setup databases step', () => { { schema: 'public', name: 'orders', kind: 'table' as const }, { schema: 'public', name: 'products', kind: 'table' as const }, ]); + const pickers = makePickerStubs({ + scopes: [{ schemas: ['public'], tables: ['public.customers', 'public.orders'] }], + }); const result = await runKtxSetupDatabasesStep( { projectDir: tempDir, inputMode: 'auto', skipDatabases: false, databaseSchemas: [] }, @@ -911,29 +963,17 @@ describe('setup databases step', () => { scanConnection: vi.fn(async () => 0), listSchemas, listTables, + pickDatabaseScope: pickers.pickDatabaseScope, }, ); expect(result).toEqual({ status: 'ready', projectDir: tempDir, connectionIds: ['warehouse'] }); - expect(prompts.multiselect).toHaveBeenNthCalledWith(1, { - message: expect.stringContaining('PostgreSQL schemas to scan'), - options: [ - { value: 'orbit_analytics', label: 'orbit_analytics' }, - { value: 'orbit_raw', label: 'orbit_raw' }, - { value: 'public', label: 'public' }, - ], - initialValues: ['public'], - required: true, - }); - expect(prompts.multiselect).toHaveBeenNthCalledWith(2, { - message: expect.stringContaining('Tables to enable for warehouse'), - options: [ - { value: 'public.customers', label: 'public.customers' }, - { value: 'public.orders', label: 'public.orders' }, - { value: 'public.products', label: 'public.products' }, - ], - initialValues: ['public.customers', 'public.orders'], - required: true, + expect(pickers.scopeCalls).toHaveLength(1); + expect(pickers.scopeCalls[0]).toMatchObject({ + connectionId: 'warehouse', + schemaNoun: 'schema', + supportsSchemaScope: true, + existing: { enabledTables: ['public.customers', 'public.orders'] }, }); const config = parseKtxProjectConfig(await readFile(join(tempDir, 'ktx.yaml'), 'utf-8')); expect(config.connections.warehouse).toMatchObject({ @@ -965,7 +1005,6 @@ describe('setup databases step', () => { await writeKtxSetupState(tempDir, { completed_steps: ['databases'] }); const prompts = makePromptAdapter({ textValues: ['env:DATABASE_URL'], - multiselectValues: [['back']], }); let primaryMenuCount = 0; vi.mocked(prompts.select).mockImplementation(async (options) => { @@ -980,19 +1019,29 @@ describe('setup databases step', () => { const testConnection = vi.fn(async () => 0); const scanConnection = vi.fn(async () => 0); const listSchemas = vi.fn(async () => ['analytics', 'public']); - const listTables = vi.fn(async () => [{ schema: 'analytics', name: 'customers', kind: 'table' as const }]); + const listTables = vi.fn(async () => [ + { schema: 'analytics', name: 'customers', kind: 'table' as const }, + { schema: 'public', name: 'orders', kind: 'table' as const }, + ]); + const pickers = makePickerStubs({ scopes: ['back'] }); const result = await runKtxSetupDatabasesStep( { projectDir: tempDir, inputMode: 'auto', skipDatabases: false, databaseSchemas: [] }, makeIo().io, - { prompts, testConnection, scanConnection, listSchemas, listTables }, + { + prompts, + testConnection, + scanConnection, + listSchemas, + listTables, + pickDatabaseScope: pickers.pickDatabaseScope, + }, ); expect(result).toEqual({ status: 'ready', projectDir: tempDir, connectionIds: ['warehouse'] }); expect(primaryMenuCount).toBe(2); expect(testConnection).toHaveBeenCalledWith(tempDir, 'warehouse', expect.anything()); expect(scanConnection).not.toHaveBeenCalled(); - expect(listTables).not.toHaveBeenCalled(); const config = parseKtxProjectConfig(await readFile(join(tempDir, 'ktx.yaml'), 'utf-8')); expect(config.connections.warehouse).toMatchObject({ url: 'env:DATABASE_URL', @@ -1031,7 +1080,6 @@ describe('setup databases step', () => { } if (options.message === 'Primary source to edit') return 'warehouse'; if (options.message === 'How do you want to connect to PostgreSQL?') return 'url'; - if (options.message.startsWith('Tables found in selected schemas')) return 'back'; return 'back'; }); const testConnection = vi.fn(async () => 0); @@ -1041,16 +1089,24 @@ describe('setup databases step', () => { { schema: 'public', name: 'customers', kind: 'table' as const }, { schema: 'public', name: 'orders', kind: 'table' as const }, ]); + const pickers = makePickerStubs({ scopes: ['back'] }); const result = await runKtxSetupDatabasesStep( { projectDir: tempDir, inputMode: 'auto', skipDatabases: false, databaseSchemas: [] }, makeIo().io, - { prompts, testConnection, scanConnection, listSchemas, listTables }, + { + prompts, + testConnection, + scanConnection, + listSchemas, + listTables, + pickDatabaseScope: pickers.pickDatabaseScope, + }, ); expect(result).toEqual({ status: 'ready', projectDir: tempDir, connectionIds: ['warehouse'] }); expect(primaryMenuCount).toBe(2); - expect(listTables).toHaveBeenCalledWith(tempDir, 'warehouse'); + expect(listTables).toHaveBeenCalledWith(tempDir, 'warehouse', ['public']); expect(scanConnection).not.toHaveBeenCalled(); const config = parseKtxProjectConfig(await readFile(join(tempDir, 'ktx.yaml'), 'utf-8')); expect(config.connections.warehouse).toMatchObject({ @@ -1083,19 +1139,18 @@ describe('setup databases step', () => { await writeKtxSetupState(tempDir, { completed_steps: ['databases'] }); const prompts = makePromptAdapter({ textValues: ['env:DATABASE_URL'], - multiselectValues: [['public']], }); vi.mocked(prompts.select).mockImplementation(async (options) => { if (options.message === 'Primary sources already configured: warehouse\nWhat would you like to do?') return 'edit'; if (options.message === 'Primary source to edit') return 'warehouse'; if (options.message === 'How do you want to connect to PostgreSQL?') return 'url'; - if (options.message.startsWith('Tables found in selected schemas')) return 'all'; return 'back'; }); const listTables = vi.fn(async () => [ { schema: 'public', name: 'customers', kind: 'table' as const }, { schema: 'public', name: 'orders', kind: 'table' as const }, ]); + const pickers = makePickerStubs({ scopes: ['enable-all'] }); const result = await runKtxSetupDatabasesStep( { projectDir: tempDir, inputMode: 'auto', skipDatabases: false, databaseSchemas: [] }, @@ -1105,6 +1160,7 @@ describe('setup databases step', () => { testConnection: vi.fn(async () => 0), scanConnection: vi.fn(async () => 1), listTables, + pickDatabaseScope: pickers.pickDatabaseScope, }, ); @@ -1390,7 +1446,6 @@ describe('setup databases step', () => { const prompts = makePromptAdapter({ selectValues: ['url'], textValues: ['', 'env:DATABASE_URL'], - multiselectValues: [['orbit_analytics', 'orbit_raw']], }); const testConnection = vi.fn(async () => 0); const scanConnection = vi.fn(async asyncScanProjectDir => { @@ -1401,6 +1456,19 @@ describe('setup databases step', () => { return 0; }); const listSchemas = vi.fn(async () => ['orbit_analytics', 'orbit_raw', 'public']); + const listTables = vi.fn(async () => [ + { schema: 'orbit_analytics', name: 'events', kind: 'table' as const }, + { schema: 'orbit_raw', name: 'inputs', kind: 'table' as const }, + { schema: 'public', name: 'misc', kind: 'table' as const }, + ]); + const pickers = makePickerStubs({ + scopes: [ + { + schemas: ['orbit_analytics', 'orbit_raw'], + tables: ['orbit_analytics.events', 'orbit_raw.inputs'], + }, + ], + }); const result = await runKtxSetupDatabasesStep( { @@ -1411,20 +1479,24 @@ describe('setup databases step', () => { skipDatabases: false, }, io.io, - { prompts, testConnection, scanConnection, listSchemas }, + { + prompts, + testConnection, + scanConnection, + listSchemas, + listTables, + pickDatabaseScope: pickers.pickDatabaseScope, + }, ); expect(result.status).toBe('ready'); expect(listSchemas).toHaveBeenCalledWith(tempDir, 'postgres-warehouse'); - expect(prompts.multiselect).toHaveBeenCalledWith({ - message: expect.stringContaining('PostgreSQL schemas to scan'), - options: [ - { value: 'orbit_analytics', label: 'orbit_analytics' }, - { value: 'orbit_raw', label: 'orbit_raw' }, - { value: 'public', label: 'public' }, - ], - initialValues: ['orbit_analytics', 'orbit_raw'], - required: true, + expect(pickers.scopeCalls).toHaveLength(1); + expect(pickers.scopeCalls[0]).toMatchObject({ + connectionId: 'postgres-warehouse', + schemaNoun: 'schema', + schemaNounPlural: 'schemas', + defaultSchemas: ['orbit_analytics', 'orbit_raw'], }); const config = parseKtxProjectConfig(await readFile(join(tempDir, 'ktx.yaml'), 'utf-8')); expect(config.connections['postgres-warehouse']).toMatchObject({ diff --git a/packages/cli/src/setup-databases.ts b/packages/cli/src/setup-databases.ts index 9db80689..c21ab6d1 100644 --- a/packages/cli/src/setup-databases.ts +++ b/packages/cli/src/setup-databases.ts @@ -14,6 +14,11 @@ import { import type { KtxTableListEntry } from '@ktx/context/scan'; import type { KtxCliIo } from './cli-runtime.js'; import { runKtxConnection } from './connection.js'; +import { + pickDatabaseScope as defaultPickDatabaseScope, + type DatabaseScopePickResult, + type PickDatabaseScopeArgs, +} from './database-tree-picker.js'; import { withMultiselectNavigation, withTextInputNavigation } from './prompt-navigation.js'; import { runKtxScan } from './scan.js'; import { writeProjectLocalSecretReference } from './setup-secrets.js'; @@ -90,7 +95,8 @@ export interface KtxSetupDatabasesDeps { scanConnection?: (projectDir: string, connectionId: string, io: KtxCliIo) => Promise; rebuildNativeSqlite?: (io: KtxCliIo) => Promise; listSchemas?: (projectDir: string, connectionId: string) => Promise; - listTables?: (projectDir: string, connectionId: string) => Promise; + listTables?: (projectDir: string, connectionId: string, schemas?: string[]) => Promise; + pickDatabaseScope?: (args: PickDatabaseScopeArgs, io: KtxCliIo) => Promise; historicSqlProbe?: KtxSetupHistoricSqlProbe; } @@ -363,11 +369,15 @@ function configuredSchemas(connection: KtxProjectConnectionConfig | undefined, d return values.length > 0 ? values : undefined; } -async function defaultListTables(projectDir: string, connectionId: string): Promise { +async function defaultListTables( + projectDir: string, + connectionId: string, + schemasOverride?: string[], +): Promise { const project = await loadKtxProject({ projectDir }); const connection = project.config.connections[connectionId]; const driver = normalizeDriver(connection?.driver); - const schemas = driver ? configuredSchemas(connection, driver) : undefined; + const schemas = schemasOverride ?? (driver ? configuredSchemas(connection, driver) : undefined); if (driver === 'postgres') { const { KtxPostgresScanConnector, isKtxPostgresConnectionConfig } = await import('@ktx/connector-postgres'); @@ -1271,145 +1281,98 @@ async function writeScopeConfig(input: { }); } -async function clearScopeConfig(projectDir: string, connectionId: string): Promise { - const project = await loadKtxProject({ projectDir }); - const connection = project.config.connections[connectionId]; - if (!connection) return; - const driver = normalizeDriver(connection.driver); - if (!driver) return; - const spec = SCOPE_DISCOVERY_SPECS[driver]; - if (!spec) return; - const cleaned = Object.fromEntries( - Object.entries(connection).filter( - ([key]) => key !== spec.configArrayField && key !== spec.configSingleField && key !== 'enabled_tables', - ), - ) as KtxProjectConnectionConfig; - await writeConnectionConfig({ projectDir, connectionId, connection: cleaned }); -} - -async function maybeConfigureSchemaScope(input: { +async function maybeConfigureDatabaseScope(input: { projectDir: string; connectionId: string; args: KtxSetupDatabasesArgs; - prompts: KtxSetupDatabasesPromptAdapter; deps: KtxSetupDatabasesDeps; io: KtxCliIo; forcePrompt?: boolean; -}): Promise { - const project = await loadKtxProject({ projectDir: input.projectDir }); - const connection = project.config.connections[input.connectionId]; - const driver = normalizeDriver(connection?.driver); - if (!driver) return 'ready'; - - const spec = SCOPE_DISCOVERY_SPECS[driver]; - if (!spec) return 'ready'; - - const arrayVal = connection?.[spec.configArrayField]; - if (Array.isArray(arrayVal) && arrayVal.length > 0 && input.forcePrompt !== true) { - return 'ready'; - } - - if (input.args.databaseSchemas.length > 0) { - await writeScopeConfig({ - projectDir: input.projectDir, - connectionId: input.connectionId, - values: input.args.databaseSchemas, - spec, - }); - return 'ready'; - } - - writeSetupSection(input.io, `Discovering ${spec.promptLabel.toLowerCase()}`, [ - `Connecting to ${input.connectionId}…`, - ]); - - let discovered: string[]; - try { - discovered = unique( - await (input.deps.listSchemas ?? defaultListSchemas)(input.projectDir, input.connectionId), - ); - } catch (error) { - const detail = error instanceof Error ? error.message : String(error); - input.io.stderr.write( - input.forcePrompt === true - ? `Could not discover ${spec.promptLabel.toLowerCase()} for ${input.connectionId}; edit was not saved. ` + - `Pass --database-schema to set it explicitly. ${detail}\n` - : `Could not discover ${spec.promptLabel.toLowerCase()} for ${input.connectionId}; continuing with existing ${spec.noun} scope. ` + - `Pass --database-schema to set it explicitly. ${detail}\n`, - ); - return input.forcePrompt === true ? 'failed' : 'ready'; - } - if (discovered.length === 0) { - return 'ready'; - } - - let selected: string[]; - if (input.args.inputMode === 'disabled' || discovered.length === 1) { - const preconfigured = configuredScopeValues(connection, spec).filter((v) => discovered.includes(v)); - selected = preconfigured.length > 0 ? preconfigured : discovered; - } else { - const preconfigured = configuredScopeValues(connection, spec).filter((v) => discovered.includes(v)); - const initialValues = preconfigured.length > 0 ? preconfigured : spec.defaultSelection(discovered); - const choices = await input.prompts.multiselect({ - message: withMultiselectNavigation( - `${spec.promptLabel} to scan\n` + - `KTX found multiple ${spec.nounPlural}. Select every ${spec.noun} agents should use.`, - ), - options: discovered.map((v) => ({ value: v, label: v })), - initialValues, - required: true, - }); - if (choices.includes('back')) { - return 'back'; - } - selected = choices.length > 0 ? choices : initialValues; - } - - await writeScopeConfig({ - projectDir: input.projectDir, - connectionId: input.connectionId, - values: selected, - spec, - }); - const capitalNounPlural = spec.nounPlural[0]!.toUpperCase() + spec.nounPlural.slice(1); - writeSetupSection(input.io, `${capitalNounPlural} saved for ${input.connectionId}`, [ - `✓ ${selected.join(', ')}`, - ]); - return 'ready'; -} - -async function maybeConfigureTableScope(input: { - projectDir: string; - connectionId: string; - args: KtxSetupDatabasesArgs; - prompts: KtxSetupDatabasesPromptAdapter; - io: KtxCliIo; - deps: KtxSetupDatabasesDeps; - forcePrompt?: boolean; }): Promise { const project = await loadKtxProject({ projectDir: input.projectDir }); const connection = project.config.connections[input.connectionId]; const driver = normalizeDriver(connection?.driver); if (!driver || driver === 'sqlite') return 'ready'; + const spec = SCOPE_DISCOVERY_SPECS[driver]; const existingTables = connection?.enabled_tables; - if (Array.isArray(existingTables) && existingTables.length > 0 && input.forcePrompt !== true) { + const hasExistingTables = Array.isArray(existingTables) && existingTables.length > 0; + const existingScope = spec ? configuredScopeValues(connection, spec) : []; + const hasExistingScope = !spec || existingScope.length > 0; + + if (hasExistingTables && hasExistingScope && input.forcePrompt !== true) { return 'ready'; } + const cliSchemas = input.args.databaseSchemas; + if (input.args.inputMode === 'disabled') { + if (spec) { + let scopeToWrite: string[] = cliSchemas; + if (scopeToWrite.length === 0) { + try { + scopeToWrite = unique( + await (input.deps.listSchemas ?? defaultListSchemas)(input.projectDir, input.connectionId), + ); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + input.io.stderr.write( + `Could not discover ${spec.promptLabel.toLowerCase()} for ${input.connectionId}; ${detail}\n`, + ); + return 'ready'; + } + } + if (scopeToWrite.length > 0) { + await writeScopeConfig({ + projectDir: input.projectDir, + connectionId: input.connectionId, + values: scopeToWrite, + spec, + }); + const capitalNounPlural = spec.nounPlural[0]!.toUpperCase() + spec.nounPlural.slice(1); + writeSetupSection(input.io, `${capitalNounPlural} saved for ${input.connectionId}`, [ + `✓ ${scopeToWrite.join(', ')}`, + ]); + } + } return 'ready'; } + if (spec && cliSchemas.length > 0) { + await writeScopeConfig({ + projectDir: input.projectDir, + connectionId: input.connectionId, + values: cliSchemas, + spec, + }); + } + writeSetupSection(input.io, 'Discovering tables', [ `Connecting to ${input.connectionId}…`, ]); + const schemasFilter = await (async (): Promise => { + if (cliSchemas.length > 0) return cliSchemas; + if (!spec) return []; + try { + return unique( + await (input.deps.listSchemas ?? defaultListSchemas)(input.projectDir, input.connectionId), + ); + } catch (error) { + const detail = error instanceof Error ? error.message : String(error); + input.io.stderr.write( + `Could not discover ${spec.promptLabel.toLowerCase()} for ${input.connectionId}; ${detail}\n`, + ); + return []; + } + })(); + let discovered: KtxTableListEntry[]; try { discovered = await (input.deps.listTables ?? defaultListTables)( input.projectDir, input.connectionId, + schemasFilter.length > 0 ? schemasFilter : undefined, ); } catch (error) { const detail = error instanceof Error ? error.message : String(error); @@ -1429,84 +1392,72 @@ async function maybeConfigureTableScope(input: { } const allQualified = discovered.map((t) => `${t.schema}.${t.name}`); + const schemasInDiscovery = unique(discovered.map((t) => t.schema)); + + const defaultSchemas = (() => { + if (cliSchemas.length > 0) return cliSchemas; + if (!spec) return schemasInDiscovery; + return spec.defaultSelection(schemasInDiscovery); + })(); + + const existingEnabled = + hasExistingTables && input.forcePrompt === true + ? (existingTables ?? []).filter( + (table): table is string => typeof table === 'string' && allQualified.includes(table), + ) + : []; + + let activeSchemas: string[]; + let enabledTables: string[]; if (discovered.length === 1) { - await writeConnectionConfig({ - projectDir: input.projectDir, - connectionId: input.connectionId, - connection: { ...connection!, enabled_tables: allQualified }, - }); - writeSetupSection(input.io, `Tables enabled for ${input.connectionId}`, [ - `✓ ${allQualified[0]}`, - ]); - return 'ready'; - } - - const bySchema = new Map(); - for (const entry of discovered) { - const existing = bySchema.get(entry.schema) ?? []; - existing.push(entry); - bySchema.set(entry.schema, existing); - } - const schemaList = [...bySchema.keys()].sort(); - const schemaSummary = schemaList.map((s) => `${s} (${bySchema.get(s)!.length})`).join(', '); - - let selected: string[] | null = null; - - while (selected === null) { - const action = await input.prompts.select({ - message: `Tables found in selected schemas\n` + - `${discovered.length} tables across ${schemaList.length} ${schemaList.length === 1 ? 'schema' : 'schemas'}: ${schemaSummary}`, - options: [ - { value: 'all', label: 'Enable all tables' }, - { value: 'customize', label: 'Customize which tables to enable' }, - { value: 'back', label: 'Back' }, - ], - }); - - if (action === 'back') { + enabledTables = allQualified; + activeSchemas = spec ? schemasInDiscovery : []; + } else { + const pickResult = await (input.deps.pickDatabaseScope ?? defaultPickDatabaseScope)( + { + connectionId: input.connectionId, + schemaNoun: spec?.noun ?? 'schema', + schemaNounPlural: spec?.nounPlural ?? 'schemas', + discovered, + existing: { enabledTables: existingEnabled }, + defaultSchemas, + supportsSchemaScope: spec !== undefined, + }, + input.io, + ); + if (pickResult.kind === 'back') { return 'back'; } - - if (action === 'all') { - selected = allQualified; - } else { - const choices = await input.prompts.multiselect({ - message: withMultiselectNavigation( - `Tables to enable for ${input.connectionId}\n` + - `Deselect any tables agents should not use.`, - ), - options: discovered.map((t) => { - const qualified = `${t.schema}.${t.name}`; - const suffix = t.kind === 'view' ? ' (view)' : ''; - return { value: qualified, label: `${qualified}${suffix}` }; - }), - initialValues: - Array.isArray(existingTables) && input.forcePrompt === true - ? existingTables.filter((table): table is string => typeof table === 'string' && allQualified.includes(table)) - : allQualified, - required: true, - }); - - if (choices.includes('back')) { - continue; - } - if (choices.length === 0) { - input.io.stdout.write('│ KTX needs at least one table enabled. Select a table or press Escape to go back.\n'); - continue; - } - selected = choices; - } + enabledTables = pickResult.enabledTables; + activeSchemas = pickResult.activeSchemas; } + if (spec) { + await writeScopeConfig({ + projectDir: input.projectDir, + connectionId: input.connectionId, + values: activeSchemas, + spec, + }); + } + const refreshedProject = await loadKtxProject({ projectDir: input.projectDir }); + const currentConnection = refreshedProject.config.connections[input.connectionId]; + if (!currentConnection) return 'ready'; await writeConnectionConfig({ projectDir: input.projectDir, connectionId: input.connectionId, - connection: { ...connection!, enabled_tables: selected }, + connection: { ...currentConnection, enabled_tables: enabledTables }, }); + if (spec && activeSchemas.length > 0) { + const capitalNounPlural = spec.nounPlural[0]!.toUpperCase() + spec.nounPlural.slice(1); + writeSetupSection(input.io, `${capitalNounPlural} saved for ${input.connectionId}`, [ + `✓ ${activeSchemas.join(', ')}`, + ]); + } writeSetupSection(input.io, `Tables enabled for ${input.connectionId}`, [ - `✓ ${selected.length}/${discovered.length} tables enabled`, + `✓ ${enabledTables.length}/${discovered.length} tables enabled`, ]); return 'ready'; } @@ -1638,26 +1589,9 @@ async function validateAndScanConnection(input: { const testLines = ['✓ Connection test passed', `Driver: ${driverDisplay}`]; writeSetupSection(input.io, `Testing ${input.connectionId}`, testLines); - while (true) { - const schemaStatus = await maybeConfigureSchemaScope({ ...input, forcePrompt: input.forceScopeAndTables }); - if (schemaStatus !== 'ready') { - return schemaStatus; - } - - const tableStatus = await maybeConfigureTableScope({ ...input, forcePrompt: input.forceScopeAndTables }); - if (tableStatus === 'ready') { - break; - } - - if (input.forceScopeAndTables) { - return tableStatus; - } - - if (tableStatus === 'failed') { - return 'failed'; - } - - await clearScopeConfig(input.projectDir, input.connectionId); + const scopeStatus = await maybeConfigureDatabaseScope({ ...input, forcePrompt: input.forceScopeAndTables }); + if (scopeStatus !== 'ready') { + return scopeStatus; } await maybeRunHistoricSqlSetupProbe({ diff --git a/packages/cli/src/notion-page-picker-tree.test.ts b/packages/cli/src/tree-picker-state.test.ts similarity index 78% rename from packages/cli/src/notion-page-picker-tree.test.ts rename to packages/cli/src/tree-picker-state.test.ts index 58e8c7ca..52e63f3d 100644 --- a/packages/cli/src/notion-page-picker-tree.test.ts +++ b/packages/cli/src/tree-picker-state.test.ts @@ -12,8 +12,8 @@ import { selectNone, toggleChecked, visibleNodeIds, - type NotionPickerPageInput, -} from './notion-page-picker-tree.js'; + type TreePickerNodeInput, +} from './tree-picker-state.js'; const IDS = { engineering: '11111111-1111-1111-1111-111111111111', @@ -27,7 +27,7 @@ const IDS = { cycleB: '99999999-9999-9999-9999-999999999999', }; -function pages(): NotionPickerPageInput[] { +function pages(): TreePickerNodeInput[] { return [ { id: IDS.marketing, title: 'Marketing', archived: false, parentId: null }, { id: IDS.onboarding, title: 'Onboarding', archived: false, parentId: IDS.engineering }, @@ -43,7 +43,7 @@ function pages(): NotionPickerPageInput[] { } describe('buildPickerTree', () => { - it('deduplicates pages, sorts siblings, preserves archived flags, roots orphans, and breaks cycles', () => { + it('deduplicates nodes, sorts siblings, preserves archived flags, roots orphans, and breaks cycles', () => { const tree = buildPickerTree(pages()); const byId = new Map(tree.map((node) => [node.id, node])); @@ -89,8 +89,7 @@ describe('selection invariants', () => { it('checking a parent locks descendants and keeps checked ids minimal', () => { const state = buildInitialState({ tree: buildPickerTree(pages()), - existingRootPageIds: [], - currentCrawlMode: 'selected_roots', + existingSelectedIds: [], }); const checkedParent = toggleChecked(state, IDS.engineering, 1000); @@ -112,15 +111,11 @@ describe('selection invariants', () => { expect(canToggle(IDS.architecture, uncheckedParent)).toEqual({ ok: true }); }); - it('normalizes stored roots, reports stale roots, expands checked ancestors, and flattens descendants', () => { + it('reports stale stored ids via the caller-supplied warning, expands checked ancestors, and flattens descendants', () => { const state = buildInitialState({ tree: buildPickerTree(pages()), - existingRootPageIds: [ - IDS.engineering.replaceAll('-', ''), - IDS.architecture, - 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', - ], - currentCrawlMode: 'selected_roots', + existingSelectedIds: [IDS.engineering, IDS.architecture, 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'], + staleWarning: (staleCount) => `${staleCount} stored root_page_ids no longer visible`, }); expect([...state.checked]).toEqual([IDS.engineering]); @@ -129,14 +124,21 @@ describe('selection invariants', () => { expect(state.preLoadWarnings).toEqual(['1 stored root_page_ids no longer visible']); expect(flattenSelection(new Set([IDS.engineering, IDS.architecture]), state.byId)).toEqual([IDS.engineering]); }); + + it('falls back to a generic stale warning when no warning factory is supplied', () => { + const state = buildInitialState({ + tree: buildPickerTree(pages()), + existingSelectedIds: ['aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'], + }); + expect(state.preLoadWarnings).toEqual(['1 stored selections no longer visible']); + }); }); describe('search and cursor movement', () => { it('filters by title and path while deriving auto-expanded ancestors', () => { const state = buildInitialState({ tree: buildPickerTree(pages()), - existingRootPageIds: [], - currentCrawlMode: 'selected_roots', + existingSelectedIds: [], }); const searching = { ...state, @@ -153,8 +155,7 @@ describe('search and cursor movement', () => { it('moves the cursor through visible nodes and implements left/right tree semantics', () => { const state = buildInitialState({ tree: buildPickerTree(pages()), - existingRootPageIds: [], - currentCrawlMode: 'selected_roots', + existingSelectedIds: [], }); const atEngineering = { @@ -175,8 +176,7 @@ describe('bulk actions and reducer effects', () => { it('selects only matching visible roots under search and clears selection', () => { const state = buildInitialState({ tree: buildPickerTree(pages()), - existingRootPageIds: [IDS.marketing], - currentCrawlMode: 'selected_roots', + existingSelectedIds: [IDS.marketing], }); const searching = { ...state, @@ -188,36 +188,35 @@ describe('bulk actions and reducer effects', () => { expect([...selectNone(selected).checked]).toEqual([]); }); - it('returns save immediately for selected_roots and requires confirmation for all_accessible', () => { - const selectedRoots = toggleChecked( + it('saves immediately when confirm is not required and prompts confirmation when requireConfirmOnSave is true', () => { + const noConfirm = toggleChecked( buildInitialState({ tree: buildPickerTree(pages()), - existingRootPageIds: [], - currentCrawlMode: 'selected_roots', + existingSelectedIds: [], }), IDS.marketing, 1000, ); - expect(reducer(selectedRoots, 'save-request')).toEqual({ - next: selectedRoots, + expect(reducer(noConfirm, 'save-request')).toEqual({ + next: noConfirm, effect: 'save', }); - const allAccessible = { - ...selectedRoots, - currentCrawlMode: 'all_accessible' as const, + const confirmRequired = { + ...noConfirm, + requireConfirmOnSave: true, }; - const confirm = reducer(allAccessible, 'save-request'); + const confirm = reducer(confirmRequired, 'save-request'); expect(confirm).toEqual({ - next: { ...allAccessible, pendingConfirm: 'mode-switch' }, + next: { ...confirmRequired, pendingConfirm: 'save-confirm' }, effect: null, }); expect(reducer(confirm.next, 'save-cancel')).toEqual({ - next: { ...allAccessible, pendingConfirm: null }, + next: { ...confirmRequired, pendingConfirm: null }, effect: null, }); expect(reducer(confirm.next, 'save-confirm')).toEqual({ - next: { ...allAccessible, pendingConfirm: null }, + next: { ...confirmRequired, pendingConfirm: null }, effect: 'save', }); }); @@ -225,8 +224,7 @@ describe('bulk actions and reducer effects', () => { it('prompts skip-empty confirmation on empty save, updates search state, and quits without saving', () => { const state = buildInitialState({ tree: buildPickerTree(pages()), - existingRootPageIds: [], - currentCrawlMode: 'selected_roots', + existingSelectedIds: [], }); const emptySave = reducer(state, 'save-request'); @@ -254,16 +252,33 @@ describe('bulk actions and reducer effects', () => { }); }); + it('treats skip-empty confirmation as a save with empty selection when skipEmptyAction is save-empty', () => { + const state = buildInitialState({ + tree: buildPickerTree(pages()), + existingSelectedIds: [], + skipEmptyAction: 'save-empty', + }); + + const emptySave = reducer(state, 'save-request'); + expect(emptySave).toEqual({ + next: { ...state, pendingConfirm: 'skip-empty' }, + effect: null, + }); + expect(reducer(emptySave.next, 'save-confirm')).toEqual({ + next: { ...state, pendingConfirm: null }, + effect: 'save', + }); + }); + it('clears transient hints only when their expiry time has passed', () => { const state = buildInitialState({ tree: buildPickerTree(pages()), - existingRootPageIds: [], - currentCrawlMode: 'selected_roots', + existingSelectedIds: [], }); const withHint = { ...state, transientHint: { - text: 'Select at least one page or press esc to cancel', + text: 'Select at least one item or press esc to cancel', expiresAt: 11500, }, }; diff --git a/packages/cli/src/notion-page-picker-tree.ts b/packages/cli/src/tree-picker-state.ts similarity index 86% rename from packages/cli/src/notion-page-picker-tree.ts rename to packages/cli/src/tree-picker-state.ts index 738ab723..9d9b3c68 100644 --- a/packages/cli/src/notion-page-picker-tree.ts +++ b/packages/cli/src/tree-picker-state.ts @@ -1,11 +1,11 @@ -export interface NotionPickerPageInput { +export interface TreePickerNodeInput { id: string; title?: string | null; archived?: boolean; parentId?: string | null; } -interface NotionPickerNode { +export interface TreePickerNode { id: string; title: string; archived: boolean; @@ -15,17 +15,22 @@ interface NotionPickerNode { path: string; } +type PendingConfirmKind = 'save-confirm' | 'skip-empty'; + +export type SkipEmptyAction = 'quit' | 'save-empty'; + export interface PickerState { - tree: NotionPickerNode[]; - byId: Map; + tree: TreePickerNode[]; + byId: Map; expanded: Set; checked: Set; cursorId: string; search: { editing: boolean; query: string }; - pendingConfirm: 'mode-switch' | 'skip-empty' | null; + pendingConfirm: PendingConfirmKind | null; preLoadWarnings: string[]; transientHint: { text: string; expiresAt: number } | null; - currentCrawlMode: 'all_accessible' | 'selected_roots'; + requireConfirmOnSave: boolean; + skipEmptyAction: SkipEmptyAction; } export type PickerCommand = @@ -65,25 +70,12 @@ const TRANSIENT_HINT_DURATION_MS = 2500; const collator = new Intl.Collator('en', { sensitivity: 'base', numeric: true }); -function normalizePageId(value: string): string { - const trimmed = value.trim(); - const compact = trimmed.replace(/-/g, ''); - if (/^[0-9a-fA-F]{32}$/.test(compact)) { - const lower = compact.toLowerCase(); - return `${lower.slice(0, 8)}-${lower.slice(8, 12)}-${lower.slice(12, 16)}-${lower.slice( - 16, - 20, - )}-${lower.slice(20)}`; - } - return trimmed; -} - function titleValue(value: string | null | undefined): string { const trimmed = value?.trim() ?? ''; return trimmed.length > 0 ? trimmed : 'Untitled'; } -function sortedNodeIds(ids: string[], nodes: Map): string[] { +function sortedNodeIds(ids: string[], nodes: Map): string[] { return [...ids].sort((leftId, rightId) => { const left = nodes.get(leftId); const right = nodes.get(rightId); @@ -107,7 +99,7 @@ export function clearExpiredTransientHint(state: PickerState, now = Date.now()): return cloneState(state, { transientHint: null }); } -function ancestorsOf(nodeId: string, byId: Map): string[] { +function ancestorsOf(nodeId: string, byId: Map): string[] { const ancestors: string[] = []; let parentId = byId.get(nodeId)?.parentId ?? null; const seen = new Set(); @@ -119,7 +111,7 @@ function ancestorsOf(nodeId: string, byId: Map): strin return ancestors; } -function descendantsOf(nodeId: string, byId: Map): string[] { +function descendantsOf(nodeId: string, byId: Map): string[] { const result: string[] = []; const stack = [...(byId.get(nodeId)?.childIds ?? [])].reverse(); while (stack.length > 0) { @@ -152,18 +144,18 @@ function matchingIds(state: PickerState): Set { ); } -export function buildPickerTree(searchResults: NotionPickerPageInput[]): NotionPickerNode[] { +export function buildPickerTree(inputs: TreePickerNodeInput[]): TreePickerNode[] { const nodes = new Map(); - for (const result of searchResults) { - const id = normalizePageId(result.id); - if (nodes.has(id)) { + for (const result of inputs) { + const id = result.id.trim(); + if (id.length === 0 || nodes.has(id)) { continue; } nodes.set(id, { id, title: titleValue(result.title), archived: result.archived === true, - parentId: result.parentId ? normalizePageId(result.parentId) : null, + parentId: result.parentId ? result.parentId.trim() : null, childIds: [], }); } @@ -202,7 +194,7 @@ export function buildPickerTree(searchResults: NotionPickerPageInput[]): NotionP [...nodes.values()].filter((node) => node.parentId === null).map((node) => node.id), nodes, ); - const tree: NotionPickerNode[] = []; + const tree: TreePickerNode[] = []; function visit(nodeId: string, depth: number, pathPrefix: string[]): void { const raw = nodes.get(nodeId); @@ -210,7 +202,7 @@ export function buildPickerTree(searchResults: NotionPickerPageInput[]): NotionP return; } const path = [...pathPrefix, raw.title].join(' / '); - const node: NotionPickerNode = { + const node: TreePickerNode = { id: raw.id, title: raw.title, archived: raw.archived, @@ -232,11 +224,11 @@ export function buildPickerTree(searchResults: NotionPickerPageInput[]): NotionP return tree; } -export function isAncestorChecked(nodeId: string, checked: Set, byId: Map): boolean { +export function isAncestorChecked(nodeId: string, checked: Set, byId: Map): boolean { return ancestorsOf(nodeId, byId).some((ancestorId) => checked.has(ancestorId)); } -function checkedAncestor(nodeId: string, state: PickerState): NotionPickerNode | null { +function checkedAncestor(nodeId: string, state: PickerState): TreePickerNode | null { for (const ancestorId of ancestorsOf(nodeId, state.byId)) { if (state.checked.has(ancestorId)) { return state.byId.get(ancestorId) ?? null; @@ -247,7 +239,7 @@ function checkedAncestor(nodeId: string, state: PickerState): NotionPickerNode | export function canToggle(nodeId: string, state: PickerState): { ok: true } | { ok: false; reason: string } { if (!state.byId.has(nodeId)) { - return { ok: false, reason: 'Page not found' }; + return { ok: false, reason: 'Node not found' }; } const ancestor = checkedAncestor(nodeId, state); if (ancestor) { @@ -276,7 +268,7 @@ export function toggleChecked(state: PickerState, nodeId: string, now = Date.now return cloneState(state, { checked, transientHint: null }); } -export function flattenSelection(checked: Set, byId: Map): string[] { +export function flattenSelection(checked: Set, byId: Map): string[] { const result: string[] = []; for (const node of byId.values()) { if (checked.has(node.id) && !isAncestorChecked(node.id, checked, byId)) { @@ -402,16 +394,21 @@ export function moveCursor(state: PickerState, dir: 'up' | 'down' | 'left' | 'ri } export function buildInitialState(args: { - tree: NotionPickerNode[]; - existingRootPageIds: string[]; - currentCrawlMode?: 'all_accessible' | 'selected_roots'; + tree: TreePickerNode[]; + existingSelectedIds: string[]; + requireConfirmOnSave?: boolean; + skipEmptyAction?: SkipEmptyAction; + staleWarning?: (staleCount: number) => string; }): PickerState { const byId = new Map(args.tree.map((node) => [node.id, node])); const checked = new Set(); let staleCount = 0; - for (const rawId of args.existingRootPageIds) { - const id = normalizePageId(rawId); + for (const rawId of args.existingSelectedIds) { + const id = rawId.trim(); + if (id.length === 0) { + continue; + } if (byId.has(id)) { checked.add(id); } else { @@ -427,6 +424,12 @@ export function buildInitialState(args: { } } + const preLoadWarnings: string[] = []; + if (staleCount > 0) { + const warning = args.staleWarning ? args.staleWarning(staleCount) : `${staleCount} stored selections no longer visible`; + preLoadWarnings.push(warning); + } + return { tree: args.tree, byId, @@ -435,16 +438,18 @@ export function buildInitialState(args: { cursorId: args.tree[0]?.id ?? '', search: { editing: false, query: '' }, pendingConfirm: null, - preLoadWarnings: staleCount > 0 ? [`${staleCount} stored root_page_ids no longer visible`] : [], + preLoadWarnings, transientHint: null, - currentCrawlMode: args.currentCrawlMode ?? 'selected_roots', + requireConfirmOnSave: args.requireConfirmOnSave ?? false, + skipEmptyAction: args.skipEmptyAction ?? 'quit', }; } export function reducer(state: PickerState, cmd: PickerCommand, now = Date.now()): { next: PickerState; effect: PickerEffect } { if (state.pendingConfirm) { if (cmd === 'save-confirm') { - const effect: PickerEffect = state.pendingConfirm === 'skip-empty' ? 'quit-without-save' : 'save'; + const effect: PickerEffect = + state.pendingConfirm === 'skip-empty' ? (state.skipEmptyAction === 'save-empty' ? 'save' : 'quit-without-save') : 'save'; return { next: cloneState(state, { pendingConfirm: null }), effect }; } if (cmd === 'save-cancel') { @@ -501,8 +506,8 @@ export function reducer(state: PickerState, cmd: PickerCommand, now = Date.now() if (state.checked.size === 0) { return { next: cloneState(state, { pendingConfirm: 'skip-empty' }), effect: null }; } - if (state.currentCrawlMode === 'all_accessible') { - return { next: cloneState(state, { pendingConfirm: 'mode-switch' }), effect: null }; + if (state.requireConfirmOnSave) { + return { next: cloneState(state, { pendingConfirm: 'save-confirm' }), effect: null }; } return { next: state, effect: 'save' }; case 'save-confirm': diff --git a/packages/cli/src/tree-picker-tui.test.tsx b/packages/cli/src/tree-picker-tui.test.tsx new file mode 100644 index 00000000..8c4f8d1e --- /dev/null +++ b/packages/cli/src/tree-picker-tui.test.tsx @@ -0,0 +1,361 @@ +/* @jsxImportSource react */ +import { render as renderInkTest } from 'ink-testing-library'; +import { type ReactNode } from 'react'; +import { describe, expect, it, vi } from 'vitest'; +import { buildInitialState, buildPickerTree, type TreePickerNodeInput } from './tree-picker-state.js'; +import { + TreePickerApp, + renderTreePickerTui, + resolveTreePickerWidth, + sanitizeTreePickerTuiError, + treePickerCommandForInkInput, + windowItems, + windowOffset, + type TreePickerChrome, + type TreePickerInkInstance, + type TreePickerInkRenderOptions, +} from './tree-picker-tui.js'; + +const IDS = { + engineering: '11111111-1111-1111-1111-111111111111', + architecture: '22222222-2222-2222-2222-222222222222', + marketing: '33333333-3333-3333-3333-333333333333', + finance: '44444444-4444-4444-4444-444444444444', + ops: '55555555-5555-5555-5555-555555555555', + sales: '66666666-6666-6666-6666-666666666666', + support: '77777777-7777-7777-7777-777777777777', + product: '88888888-8888-8888-8888-888888888888', + design: '99999999-9999-9999-9999-999999999999', +}; + +function pages(): TreePickerNodeInput[] { + return [ + { id: IDS.engineering, title: 'Engineering Docs', archived: false, parentId: null }, + { id: IDS.architecture, title: 'Architecture', archived: false, parentId: IDS.engineering }, + { id: IDS.marketing, title: 'Marketing', archived: false, parentId: null }, + ]; +} + +function manyPages(): TreePickerNodeInput[] { + return [ + { id: IDS.engineering, title: 'Engineering Docs', archived: false, parentId: null }, + { id: IDS.architecture, title: 'Architecture', archived: false, parentId: IDS.engineering }, + { id: IDS.marketing, title: 'Marketing', archived: false, parentId: null }, + { id: IDS.finance, title: 'Finance', archived: false, parentId: null }, + { id: IDS.ops, title: 'Operations', archived: false, parentId: null }, + { id: IDS.sales, title: 'Sales', archived: false, parentId: null }, + { id: IDS.support, title: 'Support', archived: false, parentId: null }, + { id: IDS.product, title: 'Product', archived: false, parentId: null }, + { id: IDS.design, title: 'Design', archived: false, parentId: null }, + ]; +} + +function state(options: { requireConfirmOnSave?: boolean } = {}) { + return buildInitialState({ + tree: buildPickerTree(pages()), + existingSelectedIds: [], + requireConfirmOnSave: options.requireConfirmOnSave ?? false, + }); +} + +function chrome(overrides: Partial = {}): TreePickerChrome { + return { + title: 'Select items', + subtitleLines: ['Source: Test'], + ...overrides, + }; +} + +async function waitForInkInput(): Promise { + await new Promise((resolve) => setTimeout(resolve, 10)); +} + +function fakeInkInstance(): TreePickerInkInstance { + return { + rerender: vi.fn(), + unmount: vi.fn(), + waitUntilExit: vi.fn(async () => undefined), + }; +} + +function normalizeFrameWrap(frame: string | undefined): string { + return frame?.replace(/\n/g, ' ').replace(/│ /g, '').replace(/ +/g, ' ') ?? ''; +} + +describe('treePickerCommandForInkInput', () => { + it('maps browse, search, and confirm input to reducer commands', () => { + expect(treePickerCommandForInkInput('', { downArrow: true }, state().search, null)).toBe('cursor-down'); + expect(treePickerCommandForInkInput('', { upArrow: true }, state().search, null)).toBe('cursor-up'); + expect(treePickerCommandForInkInput('', { rightArrow: true }, state().search, null)).toBe('cursor-right'); + expect(treePickerCommandForInkInput('', { leftArrow: true }, state().search, null)).toBe('cursor-left'); + expect(treePickerCommandForInkInput(' ', {}, state().search, null)).toBe('toggle-check'); + expect(treePickerCommandForInkInput('/', {}, state().search, null)).toBe('search-start'); + expect(treePickerCommandForInkInput('a', {}, state().search, null)).toBe('select-all-visible'); + expect(treePickerCommandForInkInput('n', {}, state().search, null)).toBe('select-none'); + expect(treePickerCommandForInkInput('', { return: true }, state().search, null)).toBe('save-request'); + expect(treePickerCommandForInkInput('', { escape: true }, state().search, null)).toBe('quit'); + expect(treePickerCommandForInkInput('c', { ctrl: true }, state().search, null)).toBe('quit'); + expect(treePickerCommandForInkInput('s', {}, state().search, null)).toBeNull(); + expect(treePickerCommandForInkInput('q', {}, state().search, null)).toBeNull(); + + expect(treePickerCommandForInkInput('x', {}, { editing: true, query: '' }, null)).toEqual({ + type: 'search-input', + value: 'x', + }); + expect(treePickerCommandForInkInput('', { backspace: true }, { editing: true, query: 'x' }, null)).toBe( + 'search-backspace', + ); + expect(treePickerCommandForInkInput('', { return: true }, { editing: true, query: 'x' }, null)).toBe( + 'search-submit', + ); + expect(treePickerCommandForInkInput('', { escape: true }, { editing: true, query: 'x' }, null)).toBe( + 'search-cancel', + ); + + expect(treePickerCommandForInkInput('y', {}, state().search, 'save-confirm')).toBe('save-confirm'); + expect(treePickerCommandForInkInput('', { return: true }, state().search, 'save-confirm')).toBe('save-confirm'); + expect(treePickerCommandForInkInput('n', {}, state().search, 'save-confirm')).toBe('save-cancel'); + }); +}); + +describe('window helpers', () => { + it('centers the selected row and returns the visible slice', () => { + expect(windowOffset(20, 10, 5)).toBe(8); + expect(windowItems(['a', 'b', 'c', 'd', 'e'], 3, 3)).toEqual({ items: ['c', 'd', 'e'], offset: 2 }); + }); + + it('clamps picker width to the design rule', () => { + expect(resolveTreePickerWidth(200)).toBe(120); + expect(resolveTreePickerWidth(100)).toBe(96); + expect(resolveTreePickerWidth(50)).toBe(60); + expect(resolveTreePickerWidth(undefined)).toBe(96); + }); +}); + +describe('TreePickerApp', () => { + it('renders chrome title, subtitle, warnings, help, and row glyphs', () => { + const initialState = { + ...state(), + preLoadWarnings: ['1 stale stored selections - they will be removed if you save'], + }; + const { lastFrame } = renderInkTest( + , + ); + + const frame = lastFrame() ?? ''; + expect(frame).toContain('Select fancy widgets'); + expect(frame).toContain('Workspace: Design Workspace'); + expect(frame).toContain('5000-page cap reached - some pages not shown'); + expect(frame).toContain('1 stale stored selections - they will be removed if you save'); + expect(frame).toContain('◻ Engineering Docs ▸ (1)'); + expect(frame).toContain('◻ Marketing'); + expect(normalizeFrameWrap(frame)).toContain( + 'Right Arrow to expand, Up/Down to move, Space to select or unselect, Slash to filter, Enter to confirm, Escape to go back, or Ctrl+C to exit.', + ); + }); + + it('renders custom help text when supplied', () => { + const { lastFrame } = renderInkTest( + , + ); + expect(lastFrame() ?? '').toContain('Bespoke instructions here.'); + }); + + it('renders checked parents and locked descendants with locked glyphs', () => { + const initialState = { + ...state(), + checked: new Set([IDS.engineering]), + expanded: new Set([IDS.engineering]), + }; + const { lastFrame } = renderInkTest( + , + ); + + const frame = lastFrame() ?? ''; + expect(frame).toContain('◼ Engineering Docs ▾'); + expect(frame).toContain(' ◼ Architecture'); + }); + + it('supports keyboard selection, confirm-on-save, and save callback', async () => { + const onExit = vi.fn(); + const { stdin, lastFrame } = renderInkTest( + + `Confirm: ${current.checked.size} item${current.checked.size === 1 ? '' : 's'}? Press Enter or Escape.`, + })} + terminalRows={24} + terminalWidth={100} + onExit={onExit} + />, + ); + + stdin.write(' '); + await waitForInkInput(); + expect(lastFrame()).toContain('◼ Engineering Docs'); + + stdin.write('\r'); + await waitForInkInput(); + expect(normalizeFrameWrap(lastFrame())).toContain('Confirm: 1 item? Press Enter or Escape.'); + + stdin.write('y'); + await waitForInkInput(); + expect(onExit).toHaveBeenCalledWith({ kind: 'save', selectedIds: [IDS.engineering] }); + }); + + it('uses the chrome-supplied skip-empty message and quits on confirm', async () => { + const onExit = vi.fn(); + const { stdin, lastFrame } = renderInkTest( + , + ); + + stdin.write('\r'); + await waitForInkInput(); + expect(normalizeFrameWrap(lastFrame())).toContain('No selections. Skip or back?'); + expect(onExit).not.toHaveBeenCalled(); + + stdin.write('n'); + await waitForInkInput(); + expect(lastFrame()).not.toContain('No selections. Skip or back?'); + expect(onExit).not.toHaveBeenCalled(); + + stdin.write('\r'); + await waitForInkInput(); + expect(lastFrame()).toContain('No selections. Skip or back?'); + + stdin.write('\r'); + await waitForInkInput(); + expect(onExit).toHaveBeenCalledWith({ kind: 'quit' }); + }); + + it('renders row-window overflow indicators when the visible list is clipped', async () => { + const onExit = vi.fn(); + const initialState = buildInitialState({ + tree: buildPickerTree(manyPages()), + existingSelectedIds: [], + }); + initialState.expanded = new Set([IDS.engineering]); + const { stdin, lastFrame } = renderInkTest( + , + ); + + expect(lastFrame()).toContain('↓ 4 more'); + + stdin.write(''); + stdin.write(''); + stdin.write(''); + stdin.write(''); + await waitForInkInput(); + + const frame = lastFrame() ?? ''; + expect(frame).toContain('↑ '); + expect(frame).toContain('↓ '); + expect(onExit).not.toHaveBeenCalled(); + }); + + it('quits without saving on Ctrl+C', async () => { + const onExit = vi.fn(); + const { stdin } = renderInkTest( + , + ); + + stdin.write(''); + await waitForInkInput(); + expect(onExit).toHaveBeenCalledWith({ kind: 'quit' }); + }); +}); + +describe('renderTreePickerTui', () => { + it('returns the app result from the Ink runtime', async () => { + const io = { + stdin: { isTTY: true, setRawMode: vi.fn() }, + stdout: { isTTY: true, columns: 100, rows: 24, write: vi.fn() }, + stderr: { write: vi.fn() }, + }; + const renderInk = vi.fn((_tree: ReactNode, _options: TreePickerInkRenderOptions) => fakeInkInstance()); + + await expect( + renderTreePickerTui( + { initialState: state(), chrome: chrome() }, + io, + { renderInk }, + ), + ).resolves.toEqual({ kind: 'quit' }); + expect(renderInk).toHaveBeenCalledOnce(); + }); + + it('sanitizes render errors and uses the supplied scripted-mode hint', async () => { + expect(sanitizeTreePickerTuiError(new Error('token=secret https://api.example.com/v1/search'))).toBe( + '[redacted] [redacted-url]', + ); + }); + + it('falls back to quit with the scripted-mode hint when Ink cannot initialize', async () => { + let stderr = ''; + const io = { + stdin: { isTTY: false, setRawMode: vi.fn() }, + stdout: { isTTY: false, columns: 100, rows: 24, write: vi.fn() }, + stderr: { + write(chunk: string) { + stderr += chunk; + }, + }, + }; + + await expect( + renderTreePickerTui( + { initialState: state(), chrome: chrome() }, + io, + { + renderInk: vi.fn(() => { + throw new Error('token=secret'); + }), + scriptedModeHint: 'Use --no-input --foo bar for scripted mode.', + }, + ), + ).resolves.toEqual({ kind: 'quit' }); + expect(stderr).toContain('Use --no-input --foo bar for scripted mode.'); + expect(stderr).not.toContain('secret'); + }); +}); diff --git a/packages/cli/src/notion-page-picker-tui.tsx b/packages/cli/src/tree-picker-tui.tsx similarity index 67% rename from packages/cli/src/notion-page-picker-tui.tsx rename to packages/cli/src/tree-picker-tui.tsx index d627d200..9cdbef8d 100644 --- a/packages/cli/src/notion-page-picker-tui.tsx +++ b/packages/cli/src/tree-picker-tui.tsx @@ -9,7 +9,7 @@ import { visibleNodeIds, type PickerCommand, type PickerState, -} from './notion-page-picker-tree.js'; +} from './tree-picker-state.js'; import type { KtxCliIo } from './cli-runtime.js'; const COLOR_THEME = { @@ -28,9 +28,15 @@ const NO_COLOR_THEME = { warning: 'white', } as const; -type NotionPickerTheme = Record; +type TreePickerTheme = Record; -export interface NotionPickerTuiIo extends KtxCliIo { +const DEFAULT_TREE_PICKER_HELP_TEXT = + 'Right Arrow to expand, Up/Down to move, Space to select or unselect, Slash to filter, Enter to confirm, Escape to go back, or Ctrl+C to exit.'; + +const DEFAULT_SKIP_EMPTY_MESSAGE = + 'Nothing selected. Skip this step? Press Enter to skip or Escape to go back.'; + +export interface TreePickerTuiIo extends KtxCliIo { stdin?: { isTTY?: boolean; setRawMode?(value: boolean): void }; stdout: KtxCliIo['stdout'] & { isTTY?: boolean; columns?: number; rows?: number }; } @@ -47,58 +53,54 @@ interface InkKey { delete?: boolean; } -export type PickerRenderResult = { kind: 'save'; rootPageIds: string[] } | { kind: 'quit' }; +export type TreePickerResult = { kind: 'save'; selectedIds: string[] } | { kind: 'quit' }; -export interface PickerRenderInput { - initialState: PickerState; - connectionId: string; - workspaceLabel: string; - cappedAtCount: number | null; - currentCrawlMode: 'all_accessible' | 'selected_roots'; +export interface TreePickerChrome { + title: string; + helpText?: string; + subtitleLines?: readonly string[]; + warningLines?: readonly string[]; + confirmSaveMessage?: (state: PickerState) => string; + skipEmptyMessage?: string; } -interface NotionPickerAppProps extends PickerRenderInput { +export interface TreePickerRenderInput { + initialState: PickerState; + chrome: TreePickerChrome; +} + +interface TreePickerAppProps extends TreePickerRenderInput { terminalRows?: number; terminalWidth?: number; env?: NodeJS.ProcessEnv; - onExit(result: PickerRenderResult): void; + onExit(result: TreePickerResult): void; } -export interface NotionPickerInkInstance { +export interface TreePickerInkInstance { rerender(tree: ReactNode): void; unmount(): void; waitUntilExit(): Promise; } -export interface NotionPickerInkRenderOptions { - stdin?: NotionPickerTuiIo['stdin']; - stdout: NotionPickerTuiIo['stdout']; - stderr: NotionPickerTuiIo['stderr']; +export interface TreePickerInkRenderOptions { + stdin?: TreePickerTuiIo['stdin']; + stdout: TreePickerTuiIo['stdout']; + stderr: TreePickerTuiIo['stderr']; exitOnCtrlC: boolean; patchConsole: boolean; maxFps: number; alternateScreen: boolean; } -function resolveTheme(env: NodeJS.ProcessEnv = process.env): NotionPickerTheme { +function resolveTheme(env: NodeJS.ProcessEnv = process.env): TreePickerTheme { return env.NO_COLOR || env.TERM === 'dumb' ? NO_COLOR_THEME : COLOR_THEME; } -export function resolveNotionPickerWidth(columns: number | undefined): number { +export function resolveTreePickerWidth(columns: number | undefined): number { const resolvedColumns = columns ?? 100; return Math.max(60, Math.min(120, resolvedColumns - 4)); } -function staleWarningText(warning: string): string { - return warning.includes('stored root_page_ids no longer visible') - ? `${warning} - they will be removed if you save` - : warning; -} - -function selectedPageCountText(count: number): string { - return `${count} selected ${count === 1 ? 'page' : 'pages'}`; -} - function rowMatchesSearch(state: PickerState, nodeId: string): boolean { const query = state.search.query.trim().toLocaleLowerCase(); if (!query) { @@ -111,7 +113,7 @@ function rowMatchesSearch(state: PickerState, nodeId: string): boolean { return node.title.toLocaleLowerCase().includes(query) || node.path.toLocaleLowerCase().includes(query); } -export function sanitizeNotionPickerTuiError(error: unknown): string { +export function sanitizeTreePickerTuiError(error: unknown): string { const message = error instanceof Error ? error.message : String(error); return message .replace(/[a-z][a-z0-9+.-]*:\/\/[^\s]+/gi, '[redacted-url]') @@ -134,7 +136,7 @@ function truncateText(value: string, width: number): string { return `${value.slice(0, width - 3)}...`; } -export function notionPickerCommandForInkInput( +export function treePickerCommandForInkInput( input: string, key: InkKey, search: PickerState['search'], @@ -152,7 +154,7 @@ export function notionPickerCommandForInkInput( if (key.backspace || key.delete) return 'search-backspace'; if (key.downArrow) return 'cursor-down'; if (key.upArrow) return 'cursor-up'; - if (input.length === 1 && input >= ' ' && input !== '\u007f') return { type: 'search-input', value: input }; + if (input.length === 1 && input >= ' ' && input !== '') return { type: 'search-input', value: input }; return null; } if (key.ctrl === true && input === 'c') return 'quit'; @@ -169,7 +171,7 @@ export function notionPickerCommandForInkInput( return null; } -function PickerRow(props: { state: PickerState; nodeId: string; width: number; theme: NotionPickerTheme }): ReactNode { +function PickerRow(props: { state: PickerState; nodeId: string; width: number; theme: TreePickerTheme }): ReactNode { const node = props.state.byId.get(props.nodeId); if (!node) return null; const focused = props.state.cursorId === node.id; @@ -177,14 +179,14 @@ function PickerRow(props: { state: PickerState; nodeId: string; width: number; t const checked = props.state.checked.has(node.id); const isSelected = checked || locked; const glyph = isSelected ? '◼' : '◻'; - const glyphColor = locked ? props.theme.muted : checked ? props.theme.selected : props.theme.muted; + const glyphColor = checked || locked ? props.theme.selected : props.theme.muted; const childAffordance = node.childIds.length > 0 ? (props.state.expanded.has(node.id) ? ' ▾' : ` ▸ (${node.childIds.length})`) : ''; const indent = ' '.repeat(node.depth * 2); - const titleColor = focused ? props.theme.text : props.theme.muted; + const titleColor = focused ? props.theme.active : props.theme.text; const inverse = rowMatchesSearch(props.state, node.id); - const prefixWidth = indent.length + 2; - const title = truncateText(`${node.title}${childAffordance}`, Math.max(10, props.width - prefixWidth)); + const prefixWidth = indent.length + 2 + childAffordance.length; + const title = truncateText(node.title, Math.max(10, props.width - prefixWidth)); return ( @@ -192,30 +194,32 @@ function PickerRow(props: { state: PickerState; nodeId: string; width: number; t {indent} {glyph} - + {' '} {title} + {childAffordance.length > 0 ? {childAffordance} : null} ); } -export function NotionPickerApp(props: NotionPickerAppProps): ReactNode { +export function TreePickerApp(props: TreePickerAppProps): ReactNode { const app = useApp(); const [state, setState] = useState(props.initialState); const stateRef = useRef(state); const theme = useMemo(() => resolveTheme(props.env), [props.env]); const visibleIds = visibleNodeIds(state); const selectedIndex = Math.max(0, visibleIds.indexOf(state.cursorId)); - const reservedRows = state.pendingConfirm === 'mode-switch' ? 9 : 8; + const reservedRows = state.pendingConfirm === 'save-confirm' ? 10 : 9; const visibleRows = Math.max(5, Math.min(12, (props.terminalRows ?? 24) - reservedRows)); const rows = windowItems(visibleIds, selectedIndex, visibleRows); const hiddenAbove = rows.offset; const hiddenBelow = Math.max(0, visibleIds.length - rows.offset - rows.items.length); const searchMatchCount = filterTree(state).visibleIds.size; - const width = resolveNotionPickerWidth(props.terminalWidth); + const width = resolveTreePickerWidth(props.terminalWidth); const showSearch = state.search.editing || state.search.query.trim().length > 0; - const selectedCount = flattenSelection(state.checked, state.byId).length; + const helpText = props.chrome.helpText ?? DEFAULT_TREE_PICKER_HELP_TEXT; + const skipEmptyMessage = props.chrome.skipEmptyMessage ?? DEFAULT_SKIP_EMPTY_MESSAGE; stateRef.current = state; @@ -244,7 +248,7 @@ export function NotionPickerApp(props: NotionPickerAppProps): ReactNode { }, [state.transientHint?.expiresAt]); useInput((input, key) => { - const command = notionPickerCommandForInkInput(input, key, stateRef.current.search, stateRef.current.pendingConfirm); + const command = treePickerCommandForInkInput(input, key, stateRef.current.search, stateRef.current.pendingConfirm); if (!command) { return; } @@ -252,7 +256,7 @@ export function NotionPickerApp(props: NotionPickerAppProps): ReactNode { stateRef.current = next; setState(next); if (effect === 'save') { - props.onExit({ kind: 'save', rootPageIds: flattenSelection(next.checked, next.byId) }); + props.onExit({ kind: 'save', selectedIds: flattenSelection(next.checked, next.byId) }); app.exit(); return; } @@ -266,7 +270,7 @@ export function NotionPickerApp(props: NotionPickerAppProps): ReactNode { ◆ - Select Notion pages to ingest + {props.chrome.title} - - Right Arrow to expand, Up/Down to move, Space to select or unselect, Slash to filter, Enter to confirm, Escape - to go back, or Ctrl+C to exit. - + {helpText} - Workspace: {props.workspaceLabel} - {props.cappedAtCount ? ( - {props.cappedAtCount}-page cap reached - some pages not shown - ) : null} + {(props.chrome.subtitleLines ?? []).map((line, idx) => ( + + {line} + + ))} + {(props.chrome.warningLines ?? []).map((line, idx) => ( + + {line} + + ))} {state.preLoadWarnings.map((warning) => ( - {staleWarningText(warning)} + {warning} ))} {showSearch ? ( @@ -301,20 +308,20 @@ export function NotionPickerApp(props: NotionPickerAppProps): ReactNode { ({searchMatchCount} matches) ) : null} + {hiddenAbove > 0 ? ↑ {hiddenAbove} more : null} {rows.items.map((nodeId) => ( ))} {hiddenBelow > 0 ? ↓ {hiddenBelow} more : null} - {state.pendingConfirm === 'mode-switch' ? ( + {state.pendingConfirm === 'save-confirm' ? ( - Switch crawl_mode from all_accessible to selected_roots? Will limit ingest to{' '} - {selectedPageCountText(selectedCount)}. Press Enter to confirm or Escape to go back. + {props.chrome.confirmSaveMessage + ? props.chrome.confirmSaveMessage(state) + : 'Confirm save? Press Enter to confirm or Escape to go back.'} ) : null} - {state.pendingConfirm === 'skip-empty' ? ( - Nothing selected. Skip this step? Press Enter to skip or Escape to go back. - ) : null} + {state.pendingConfirm === 'skip-empty' ? {skipEmptyMessage} : null} {state.transientHint ? {state.transientHint.text} : null} └ @@ -322,7 +329,7 @@ export function NotionPickerApp(props: NotionPickerAppProps): ReactNode { ); } -function renderInk(tree: ReactNode, options: NotionPickerInkRenderOptions): NotionPickerInkInstance { +function renderInk(tree: ReactNode, options: TreePickerInkRenderOptions): TreePickerInkInstance { return renderInkRuntime(tree, { stdin: options.stdin as NodeJS.ReadStream | undefined, stdout: options.stdout as NodeJS.WriteStream, @@ -331,19 +338,24 @@ function renderInk(tree: ReactNode, options: NotionPickerInkRenderOptions): Noti patchConsole: options.patchConsole, maxFps: options.maxFps, alternateScreen: options.alternateScreen, - }) as NotionPickerInkInstance; + }) as TreePickerInkInstance; } -export async function renderNotionPickerTui( - input: PickerRenderInput, - io: NotionPickerTuiIo, - options: { renderInk?: (tree: ReactNode, options: NotionPickerInkRenderOptions) => NotionPickerInkInstance } = {}, -): Promise { - let result: PickerRenderResult = { kind: 'quit' }; - let instance: NotionPickerInkInstance | null = null; +export interface RenderTreePickerOptions { + renderInk?: (tree: ReactNode, options: TreePickerInkRenderOptions) => TreePickerInkInstance; + scriptedModeHint?: string; +} + +export async function renderTreePickerTui( + input: TreePickerRenderInput, + io: TreePickerTuiIo, + options: RenderTreePickerOptions = {}, +): Promise { + let result: TreePickerResult = { kind: 'quit' }; + let instance: TreePickerInkInstance | null = null; try { instance = (options.renderInk ?? renderInk)( - for scripted mode. ${sanitizeNotionPickerTuiError(error)}\n`, - ); + const hint = options.scriptedModeHint ?? 'Picker requires a TTY.'; + io.stderr.write(`${hint} ${sanitizeTreePickerTuiError(error)}\n`); return { kind: 'quit' }; } } From ed690ef60c54e4a9778034894f212c8911d94899 Mon Sep 17 00:00:00 2001 From: Luca Martial <48870843+luca-martial@users.noreply.github.com> Date: Wed, 13 May 2026 18:47:58 -0400 Subject: [PATCH 4/5] feat(cli): redesign ktx status output UX (#80) * feat(cli): redesign ktx status output with grouped checks and color Replace flat PASS/FAIL/WARN text output with a grouped, symbol-based layout (Environment, Project, Semantic search, Query history). Passing groups collapse to a single summary line; failing groups expand to show individual checks with fix hints. Adds --verbose flag to show all checks including passing ones, color support for TTY terminals, a dedicated setup-mode report that guides users toward `ktx setup`, and timing info. Co-Authored-By: Claude Opus 4.6 (1M context) * refactor(cli): extract project checks and historic SQL doctor into status-project Move project-level doctor checks, semantic search embedding checks, and historic SQL doctor logic from doctor.ts into a dedicated status-project.ts module. Removes historic-sql-doctor.ts and its test file, consolidating everything into the new module with its own tests. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- packages/cli/src/commands/status-commands.ts | 5 +- packages/cli/src/doctor.test.ts | 365 +++++------ packages/cli/src/doctor.ts | 433 ++++++++----- packages/cli/src/historic-sql-doctor.test.ts | 202 ------ packages/cli/src/historic-sql-doctor.ts | 167 ----- packages/cli/src/index.test.ts | 4 +- packages/cli/src/status-project.ts | 614 +++++++++++++++++++ 7 files changed, 1040 insertions(+), 750 deletions(-) delete mode 100644 packages/cli/src/historic-sql-doctor.test.ts delete mode 100644 packages/cli/src/historic-sql-doctor.ts create mode 100644 packages/cli/src/status-project.ts diff --git a/packages/cli/src/commands/status-commands.ts b/packages/cli/src/commands/status-commands.ts index d834e15b..52032e59 100644 --- a/packages/cli/src/commands/status-commands.ts +++ b/packages/cli/src/commands/status-commands.ts @@ -16,8 +16,9 @@ export function registerStatusCommands(program: Command, context: KtxCliCommandC .command('status') .description('Check current KTX setup and project readiness') .option('--json', 'Print JSON output', false) + .option('-v, --verbose', 'Show every check, including passing ones', false) .option('--no-input', 'Disable interactive terminal input') - .action(async (options: { json?: boolean; input?: boolean }, command) => { + .action(async (options: { json?: boolean; verbose?: boolean; input?: boolean }, command) => { const runner = context.deps.doctor ?? (await import('../doctor.js')).runKtxDoctor; const explicitOrEnvProjectDir = resolveCommandProjectDirOverride(command); const nearestProjectDir = explicitOrEnvProjectDir ? undefined : findNearestKtxProjectDir(process.cwd()); @@ -27,6 +28,7 @@ export function registerStatusCommands(program: Command, context: KtxCliCommandC { command: 'setup', outputMode: outputMode(options), + verbose: options.verbose === true, ...inputMode(options), }, context.io, @@ -40,6 +42,7 @@ export function registerStatusCommands(program: Command, context: KtxCliCommandC command: 'project', projectDir: resolveCommandProjectDir(command), outputMode: outputMode(options), + verbose: options.verbose === true, ...inputMode(options), }, context.io, diff --git a/packages/cli/src/doctor.test.ts b/packages/cli/src/doctor.test.ts index cd96e6d2..d5db8ab6 100644 --- a/packages/cli/src/doctor.test.ts +++ b/packages/cli/src/doctor.test.ts @@ -1,8 +1,7 @@ import { mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { KtxEmbeddingConfig, KtxEmbeddingHealthCheckOptions, KtxEmbeddingHealthCheckResult } from '@ktx/llm'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { formatDoctorReport, runKtxDoctor, @@ -31,53 +30,65 @@ function makeIo() { }; } -type EmbeddingHealthCheck = ( - config: KtxEmbeddingConfig, - options?: KtxEmbeddingHealthCheckOptions, -) => Promise; - -async function writeProjectConfig(projectDir: string, embeddingLines: string[]): Promise { - await writeFile( - join(projectDir, 'ktx.yaml'), - [ - 'project: warehouse', - 'connections:', - ' warehouse:', - ' driver: sqlite', - ' path: ./warehouse.db', - 'ingest:', - ' adapters:', - ' - live-database', - ' embeddings:', - ...embeddingLines.map((line) => ` ${line}`), - '', - ].join('\n'), - 'utf-8', - ); -} - describe('formatDoctorReport', () => { - it('prints exact fixes for failing setup checks', () => { + it('shows the failing check and its fix in plain output', () => { const checks: DoctorCheck[] = [ - { id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0 ABI 127' }, + { id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0 ABI 127', group: 'toolchain' }, { id: 'native-sqlite', label: 'Native SQLite', status: 'fail', detail: 'Cannot load better-sqlite3', fix: 'Run: pnpm run native:rebuild', + group: 'toolchain', }, ]; - expect(formatDoctorReport({ title: 'KTX setup doctor', checks })).toBe( - [ - 'KTX setup doctor', - 'PASS Node 22+: v22.16.0 ABI 127', - 'FAIL Native SQLite: Cannot load better-sqlite3', - ' Fix: Run: pnpm run native:rebuild', - '', - ].join('\n'), - ); + const output = formatDoctorReport({ title: 'KTX status', checks }); + expect(output).toContain('KTX status'); + expect(output).toContain('✗ Environment'); + expect(output).toContain('1 of 2 need attention'); + expect(output).toContain('✗ Native SQLite: Cannot load better-sqlite3'); + expect(output).toContain('→ Run: pnpm run native:rebuild'); + expect(output).toContain('1 issue to fix.'); + }); + + it('lists what was checked when a group has all passing checks', () => { + const checks: DoctorCheck[] = [ + { id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0', group: 'toolchain' }, + { id: 'pnpm', label: 'pnpm 10.20+', status: 'pass', detail: '10.28.0', group: 'toolchain' }, + ]; + + const output = formatDoctorReport({ title: 'KTX status', checks }); + expect(output).toContain('✓ Environment'); + expect(output).toContain('Node 22+ · pnpm 10.20+'); + expect(output).not.toContain('v22.16.0'); + expect(output).toContain('Everything ready.'); + }); + + it('shows the underlying detail for a single-check group on the group line', () => { + const checks: DoctorCheck[] = [ + { + id: 'semantic-search-embeddings', + label: 'Semantic search embeddings', + status: 'pass', + detail: 'openai/text-embedding-3-small (1536d) probe succeeded', + group: 'search', + }, + ]; + + const output = formatDoctorReport({ title: 'KTX status', checks }); + expect(output).toContain('✓ Semantic search'); + expect(output).toContain('openai/text-embedding-3-small (1536d) probe succeeded'); + }); + + it('lists every check in verbose mode', () => { + const checks: DoctorCheck[] = [ + { id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0', group: 'toolchain' }, + ]; + + const output = formatDoctorReport({ title: 'KTX status', checks }, { verbose: true }); + expect(output).toContain('✓ Node 22+: v22.16.0'); }); }); @@ -127,6 +138,7 @@ describe('runSetupDoctorChecks', () => { status: 'fail', detail: 'pnpm not found', fix: 'Run: corepack enable && corepack prepare pnpm@10.28.0 --activate', + group: 'toolchain', }); expect(checks).toContainEqual({ id: 'package-build', @@ -134,6 +146,7 @@ describe('runSetupDoctorChecks', () => { status: 'fail', detail: 'Missing packages/cli/dist/bin.js', fix: 'Run: pnpm run build', + group: 'toolchain', }); }); @@ -154,9 +167,11 @@ describe('runSetupDoctorChecks', () => { const testIo = makeIo(); await expect( - runKtxDoctor({ command: 'setup', outputMode: 'plain', inputMode: 'disabled' }, testIo.io, { - runSetupChecks: async () => checks, - }), + runKtxDoctor( + { command: 'setup', outputMode: 'plain', inputMode: 'disabled', verbose: true }, + testIo.io, + { runSetupChecks: async () => checks }, + ), ).resolves.toBe(0); expect(checks).toContainEqual({ @@ -165,8 +180,9 @@ describe('runSetupDoctorChecks', () => { status: 'warn', detail: 'spawn corepack ENOENT', fix: 'Run: corepack enable', + group: 'toolchain', }); - expect(testIo.stdout()).toContain('WARN Corepack: spawn corepack ENOENT'); + expect(testIo.stdout()).toContain('⚠ Corepack: spawn corepack ENOENT'); expect(testIo.stderr()).toBe(''); }); }); @@ -204,12 +220,45 @@ describe('runKtxDoctor', () => { ), ).resolves.toBe(1); - expect(testIo.stdout()).toContain('KTX setup doctor'); - expect(testIo.stdout()).toContain('FAIL TypeScript package build: Missing packages/cli/dist/bin.js'); - expect(testIo.stdout()).toContain('Fix: Run: pnpm run build'); + expect(testIo.stdout()).toContain('KTX status'); + expect(testIo.stdout()).toContain('No project here yet.'); + expect(testIo.stdout()).toContain('Before you can run'); + expect(testIo.stdout()).toContain('✗ TypeScript package build: Missing packages/cli/dist/bin.js'); + expect(testIo.stdout()).toContain('→ Run: pnpm run build'); expect(testIo.stderr()).toBe(''); }); + it('leads with `ktx setup` and hides toolchain warnings when no project exists', async () => { + const testIo = makeIo(); + + await expect( + runKtxDoctor( + { command: 'setup', outputMode: 'plain', inputMode: 'disabled' }, + testIo.io, + { + runSetupChecks: async () => [ + { id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0', group: 'toolchain' }, + { + id: 'corepack', + label: 'Corepack', + status: 'warn', + detail: 'spawn corepack ENOENT', + fix: 'Run: corepack enable', + group: 'toolchain', + }, + ], + }, + ), + ).resolves.toBe(0); + + const out = testIo.stdout(); + expect(out).toContain('No project here yet.'); + expect(out).toContain('Run'); + expect(out).toContain('ktx setup'); + expect(out).not.toContain('Corepack'); + expect(out).not.toContain('Node 22+'); + }); + it('prints JSON setup report', async () => { const testIo = makeIo(); @@ -226,12 +275,13 @@ describe('runKtxDoctor', () => { ).resolves.toBe(0); expect(JSON.parse(testIo.stdout())).toEqual({ - title: 'KTX setup doctor', + title: 'KTX status', checks: [{ id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0 ABI 127' }], }); }); it('runs project checks against a valid ktx.yaml', async () => { + process.env.ANTHROPIC_API_KEY = 'test-key'; await writeFile( join(tempDir, 'ktx.yaml'), [ @@ -240,220 +290,109 @@ describe('runKtxDoctor', () => { ' warehouse:', ' driver: sqlite', ' path: ./warehouse.db', + 'llm:', + ' provider:', + ' backend: anthropic', + ' models:', + ' default: claude-sonnet-4-5', 'ingest:', ' adapters:', ' - live-database', + ' embeddings:', + ' backend: openai', + ' model: text-embedding-3-small', + ' dimensions: 1536', '', ].join('\n'), 'utf-8', ); + process.env.OPENAI_API_KEY = 'test-key'; const testIo = makeIo(); await expect( runKtxDoctor( { command: 'project', projectDir: tempDir, outputMode: 'plain', inputMode: 'disabled' }, testIo.io, - { - runSetupChecks: async () => [ - { id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0 ABI 127' }, - ], - }, + {}, ), ).resolves.toBe(0); - expect(testIo.stdout()).toContain('KTX project doctor'); - expect(testIo.stdout()).toContain('PASS Project config: warehouse'); - expect(testIo.stdout()).toContain('PASS Connections: 1 configured'); + const out = testIo.stdout(); + expect(out).toContain('KTX status'); + expect(out).toContain('· warehouse'); + expect(out).toContain('Connections (1)'); + expect(out).toContain('LLM'); + expect(out).toContain('anthropic'); + expect(out).toContain('Embeddings'); + expect(out).toContain('Ready.'); + delete process.env.ANTHROPIC_API_KEY; + delete process.env.OPENAI_API_KEY; }); - it('includes Postgres historic-SQL readiness in project doctor output', async () => { + it('returns blocked verdict when LLM is not configured', async () => { await writeFile( join(tempDir, 'ktx.yaml'), [ 'project: warehouse', 'connections:', ' warehouse:', - ' driver: postgres', - ' url: env:WAREHOUSE_DATABASE_URL', - ' historicSql:', - ' enabled: true', - ' dialect: postgres', - 'ingest:', - ' adapters:', - ' - live-database', - ' - historic-sql', + ' driver: sqlite', + ' path: ./warehouse.db', '', ].join('\n'), 'utf-8', ); const testIo = makeIo(); - const runHistoricSqlDoctorChecks = vi.fn(async () => [ - { - id: 'historic-sql-postgres-warehouse', - label: 'Postgres Historic SQL (warehouse)', - status: 'pass' as const, - detail: - 'pg_stat_statements ready (PostgreSQL 16.4); info: pg_stat_statements.max is 1000; set it to at least 5000 to reduce query-template eviction churn', - }, - ]); await expect( runKtxDoctor( { command: 'project', projectDir: tempDir, outputMode: 'plain', inputMode: 'disabled' }, testIo.io, - { - runSetupChecks: async () => [ - { id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0 ABI 127' }, - ], - runHistoricSqlDoctorChecks, - }, + {}, ), - ).resolves.toBe(0); + ).resolves.toBe(1); - expect(runHistoricSqlDoctorChecks).toHaveBeenCalledTimes(1); - expect(testIo.stdout()).toContain('PASS Postgres Historic SQL (warehouse): pg_stat_statements ready'); - expect(testIo.stdout()).toContain('info: pg_stat_statements.max is 1000'); - expect(testIo.stdout()).not.toContain('Fix: Update the Postgres parameter group or config'); + expect(testIo.stdout()).toContain('no LLM configured'); + expect(testIo.stdout()).toContain('ktx setup'); }); it('warns when semantic-search embeddings are not configured', async () => { - await writeProjectConfig(tempDir, ['backend: deterministic', 'model: deterministic', 'dimensions: 8']); + process.env.ANTHROPIC_API_KEY = 'test-key'; + await writeFile( + join(tempDir, 'ktx.yaml'), + [ + 'project: warehouse', + 'connections:', + ' warehouse:', + ' driver: sqlite', + ' path: ./warehouse.db', + 'llm:', + ' provider:', + ' backend: anthropic', + 'ingest:', + ' adapters:', + ' - live-database', + ' embeddings:', + ' backend: deterministic', + ' model: deterministic', + ' dimensions: 8', + '', + ].join('\n'), + 'utf-8', + ); const testIo = makeIo(); await expect( runKtxDoctor( { command: 'project', projectDir: tempDir, outputMode: 'plain', inputMode: 'disabled' }, testIo.io, - { - runSetupChecks: async () => [ - { id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0 ABI 127' }, - ], - }, + {}, ), ).resolves.toBe(0); - expect(testIo.stdout()).toContain('WARN Semantic search embeddings: ingest.embeddings.backend is deterministic.'); - expect(testIo.stdout()).toContain( - 'Semantic lane will be skipped; lexical, dictionary, and token lanes remain available.', - ); - expect(testIo.stdout()).toContain( - `Fix: Run: ktx setup --project-dir ${tempDir} --no-input`, - ); - }); - - it('probes configured semantic-search embeddings for project doctor', async () => { - await writeProjectConfig(tempDir, [ - 'backend: sentence-transformers', - 'model: all-MiniLM-L6-v2', - 'dimensions: 384', - 'sentenceTransformers:', - ' base_url: http://127.0.0.1:8765', - " pathPrefix: ''", - ]); - const healthCheck = vi.fn(async () => ({ ok: true })); - const testIo = makeIo(); - - await expect( - runKtxDoctor( - { command: 'project', projectDir: tempDir, outputMode: 'plain', inputMode: 'disabled' }, - testIo.io, - { - runSetupChecks: async () => [ - { id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0 ABI 127' }, - ], - embeddingHealthCheck: healthCheck, - embeddingProbeTimeoutMs: 1234, - }, - ), - ).resolves.toBe(0); - - expect(healthCheck).toHaveBeenCalledWith( - { - backend: 'sentence-transformers', - model: 'all-MiniLM-L6-v2', - dimensions: 384, - sentenceTransformers: { baseURL: 'http://127.0.0.1:8765', pathPrefix: '' }, - }, - { text: 'KTX semantic search doctor probe', timeoutMs: 1234 }, - ); - expect(testIo.stdout()).toContain( - 'PASS Semantic search embeddings: sentence-transformers/all-MiniLM-L6-v2 (384d) probe succeeded', - ); - }); - - it('allows local sentence-transformers semantic-search probes enough time for cold start', async () => { - await writeProjectConfig(tempDir, [ - 'backend: sentence-transformers', - 'model: all-MiniLM-L6-v2', - 'dimensions: 384', - 'sentenceTransformers:', - ' base_url: http://127.0.0.1:8765', - " pathPrefix: ''", - ]); - const healthCheck = vi.fn(async () => ({ ok: true })); - const testIo = makeIo(); - - await expect( - runKtxDoctor( - { command: 'project', projectDir: tempDir, outputMode: 'plain', inputMode: 'disabled' }, - testIo.io, - { - runSetupChecks: async () => [ - { id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0 ABI 127' }, - ], - embeddingHealthCheck: healthCheck, - }, - ), - ).resolves.toBe(0); - - expect(healthCheck).toHaveBeenCalledWith( - expect.objectContaining({ - backend: 'sentence-transformers', - model: 'all-MiniLM-L6-v2', - dimensions: 384, - }), - { text: 'KTX semantic search doctor probe', timeoutMs: 120_000 }, - ); - }); - - it('reports unhealthy semantic-search embeddings as a warning in JSON output', async () => { - await writeProjectConfig(tempDir, [ - 'backend: sentence-transformers', - 'model: all-MiniLM-L6-v2', - 'dimensions: 384', - 'sentenceTransformers:', - ' base_url: http://127.0.0.1:8765', - " pathPrefix: ''", - ]); - const healthCheck = vi.fn(async () => ({ - ok: false, - message: 'connect ECONNREFUSED 127.0.0.1:8765', - })); - const testIo = makeIo(); - - await expect( - runKtxDoctor( - { command: 'project', projectDir: tempDir, outputMode: 'json', inputMode: 'disabled' }, - testIo.io, - { - runSetupChecks: async () => [ - { id: 'node', label: 'Node 22+', status: 'pass', detail: 'v22.16.0 ABI 127' }, - ], - embeddingHealthCheck: healthCheck, - }, - ), - ).resolves.toBe(0); - - const report = JSON.parse(testIo.stdout()) as { - checks: Array<{ id: string; label: string; status: string; detail: string; fix?: string }>; - }; - expect(report.checks).toContainEqual({ - id: 'semantic-search-embeddings', - label: 'Semantic search embeddings', - status: 'warn', - detail: - 'sentence-transformers/all-MiniLM-L6-v2 (384d) probe failed: connect ECONNREFUSED 127.0.0.1:8765. Semantic lane will be skipped; lexical, dictionary, and token lanes remain available.', - fix: `Run: ktx setup --project-dir ${tempDir} --no-input`, - }); + expect(testIo.stdout()).toContain('Embeddings'); + expect(testIo.stdout()).toContain('deterministic'); + expect(testIo.stdout()).toContain('semantic search degraded'); + delete process.env.ANTHROPIC_API_KEY; }); }); diff --git a/packages/cli/src/doctor.ts b/packages/cli/src/doctor.ts index 4203369c..9342f24e 100644 --- a/packages/cli/src/doctor.ts +++ b/packages/cli/src/doctor.ts @@ -4,15 +4,13 @@ import { access } from 'node:fs/promises'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { promisify } from 'node:util'; -import type { KtxLocalProject, KtxProjectEmbeddingConfig } from '@ktx/context/project'; -import type { KtxEmbeddingConfig, KtxEmbeddingHealthCheckOptions, KtxEmbeddingHealthCheckResult } from '@ktx/llm'; -import type { HistoricSqlDoctorDeps } from './historic-sql-doctor.js'; const execFileAsync = promisify(execFile); type DoctorStatus = 'pass' | 'warn' | 'fail'; type KtxDoctorOutputMode = 'plain' | 'json'; type KtxDoctorInputMode = 'auto' | 'disabled'; +type DoctorGroup = 'toolchain' | 'project' | 'search' | 'history'; export interface DoctorCheck { id: string; @@ -20,6 +18,7 @@ export interface DoctorCheck { status: DoctorStatus; detail: string; fix?: string; + group?: DoctorGroup; } interface DoctorReport { @@ -28,11 +27,22 @@ interface DoctorReport { } export type KtxDoctorArgs = - | { command: 'setup'; outputMode: KtxDoctorOutputMode; inputMode?: KtxDoctorInputMode } - | { command: 'project'; projectDir: string; outputMode: KtxDoctorOutputMode; inputMode?: KtxDoctorInputMode }; + | { + command: 'setup'; + outputMode: KtxDoctorOutputMode; + inputMode?: KtxDoctorInputMode; + verbose?: boolean; + } + | { + command: 'project'; + projectDir: string; + outputMode: KtxDoctorOutputMode; + inputMode?: KtxDoctorInputMode; + verbose?: boolean; + }; interface KtxDoctorIo { - stdout: { write(chunk: string): void }; + stdout: { isTTY?: boolean; write(chunk: string): void }; stderr: { write(chunk: string): void }; } @@ -44,20 +54,8 @@ interface SetupDoctorDeps { importBetterSqlite3?: () => Promise; } -type EmbeddingHealthCheck = ( - config: KtxEmbeddingConfig, - options?: KtxEmbeddingHealthCheckOptions, -) => Promise; - -interface SemanticSearchDoctorDeps { - env?: NodeJS.ProcessEnv; - embeddingHealthCheck?: EmbeddingHealthCheck; - embeddingProbeTimeoutMs?: number; -} - -interface KtxDoctorDeps extends SemanticSearchDoctorDeps, HistoricSqlDoctorDeps { +interface KtxDoctorDeps { runSetupChecks?: () => Promise; - runHistoricSqlDoctorChecks?: (project: KtxLocalProject, deps: HistoricSqlDoctorDeps) => Promise; } function workspaceRootDir(): string { @@ -118,99 +116,6 @@ function check(status: DoctorStatus, id: string, label: string, detail: string, return fix ? { id, label, status, detail, fix } : { id, label, status, detail }; } -const SEMANTIC_SEARCH_HEALTH_TEXT = 'KTX semantic search doctor probe'; -const SEMANTIC_SEARCH_HEALTH_TIMEOUT_MS = 5_000; -const SEMANTIC_SEARCH_LOCAL_HEALTH_TIMEOUT_MS = 120_000; - -function semanticEmbeddingSetupFix(projectDir: string, backend: KtxProjectEmbeddingConfig['backend']): string { - if (backend === 'openai') { - return `Set OPENAI_API_KEY or rerun: ktx setup --project-dir ${projectDir} --embedding-backend openai --no-input`; - } - return `Run: ktx setup --project-dir ${projectDir} --no-input`; -} - -function embeddingConfigLabel(config: KtxProjectEmbeddingConfig | KtxEmbeddingConfig): string { - const model = config.model?.trim() || 'model not configured'; - return `${config.backend}/${model} (${config.dimensions}d)`; -} - -function semanticLaneFallbackDetail(reason: string): string { - return `${reason}. Semantic lane will be skipped; lexical, dictionary, and token lanes remain available.`; -} - -async function defaultEmbeddingHealthCheck( - config: KtxEmbeddingConfig, - options?: KtxEmbeddingHealthCheckOptions, -): Promise { - const { runKtxEmbeddingHealthCheck } = await import('@ktx/llm'); - return runKtxEmbeddingHealthCheck(config, options); -} - -async function runSemanticSearchEmbeddingCheck( - config: KtxProjectEmbeddingConfig, - projectDir: string, - deps: SemanticSearchDoctorDeps = {}, -): Promise { - if (config.backend === 'none' || config.backend === 'deterministic') { - return check( - 'warn', - 'semantic-search-embeddings', - 'Semantic search embeddings', - semanticLaneFallbackDetail(`ingest.embeddings.backend is ${config.backend}`), - semanticEmbeddingSetupFix(projectDir, config.backend), - ); - } - - try { - const { resolveLocalKtxEmbeddingConfig } = await import('@ktx/context'); - const resolved = resolveLocalKtxEmbeddingConfig(config, deps.env ?? process.env); - if (!resolved) { - return check( - 'warn', - 'semantic-search-embeddings', - 'Semantic search embeddings', - semanticLaneFallbackDetail(`No runtime embedding config resolved for ${embeddingConfigLabel(config)}`), - semanticEmbeddingSetupFix(projectDir, config.backend), - ); - } - - const healthCheck = deps.embeddingHealthCheck ?? defaultEmbeddingHealthCheck; - const timeoutMs = - deps.embeddingProbeTimeoutMs ?? - (resolved.backend === 'sentence-transformers' - ? SEMANTIC_SEARCH_LOCAL_HEALTH_TIMEOUT_MS - : SEMANTIC_SEARCH_HEALTH_TIMEOUT_MS); - const health = await healthCheck(resolved, { - text: SEMANTIC_SEARCH_HEALTH_TEXT, - timeoutMs, - }); - if (health.ok) { - return check( - 'pass', - 'semantic-search-embeddings', - 'Semantic search embeddings', - `${embeddingConfigLabel(resolved)} probe succeeded`, - ); - } - - return check( - 'warn', - 'semantic-search-embeddings', - 'Semantic search embeddings', - semanticLaneFallbackDetail(`${embeddingConfigLabel(resolved)} probe failed: ${health.message}`), - semanticEmbeddingSetupFix(projectDir, config.backend), - ); - } catch (error) { - return check( - 'warn', - 'semantic-search-embeddings', - 'Semantic search embeddings', - semanticLaneFallbackDetail(`${embeddingConfigLabel(config)} probe failed: ${failureMessage(error)}`), - semanticEmbeddingSetupFix(projectDir, config.backend), - ); - } -} - export async function runSetupDoctorChecks(deps: SetupDoctorDeps = {}): Promise { const env = deps.env ?? process.env; const root = deps.workspaceRoot ?? workspaceRootDir(); @@ -304,56 +209,231 @@ export async function runSetupDoctorChecks(deps: SetupDoctorDeps = {}): Promise< ); } - return checks; + return checks.map((entry) => ({ ...entry, group: 'toolchain' })); } -async function runProjectChecks(projectDir: string, deps: KtxDoctorDeps = {}): Promise { - const { loadKtxProject } = await import('@ktx/context/project'); - const checks: DoctorCheck[] = []; - try { - const project = await loadKtxProject({ projectDir }); - checks.push(check('pass', 'project-config', 'Project config', project.config.project)); - const connectionCount = Object.keys(project.config.connections).length; - checks.push( - connectionCount > 0 - ? check('pass', 'connections', 'Connections', `${connectionCount} configured`) - : check( - 'warn', - 'connections', - 'Connections', - '0 configured', - 'Add a connection to ktx.yaml or run `ktx setup`', - ), - ); - checks.push(check('pass', 'storage', 'Storage', `${project.config.storage.state}/${project.config.storage.search}`)); - checks.push(check('pass', 'llm-provider', 'LLM provider', project.config.llm.provider.backend)); - checks.push(await runSemanticSearchEmbeddingCheck(project.config.ingest.embeddings, projectDir, deps)); - const runHistoricSqlDoctorChecks = - deps.runHistoricSqlDoctorChecks ?? (await import('./historic-sql-doctor.js')).runPostgresHistoricSqlDoctorChecks; - checks.push(...(await runHistoricSqlDoctorChecks(project, deps))); - } catch (error) { - checks.push( - check( - 'fail', - 'project-config', - 'Project config', - failureMessage(error), - `Run: ktx init ${projectDir} --name `, - ), - ); +const STATUS_SYMBOL: Record = { pass: '✓', warn: '⚠', fail: '✗' }; + +const GROUP_ORDER: DoctorGroup[] = ['toolchain', 'project', 'search', 'history']; + +const GROUP_LABEL: Record = { + toolchain: 'Environment', + project: 'Project', + search: 'Semantic search', + history: 'Query history', +}; + +function shouldUseColor(io: KtxDoctorIo): boolean { + if (io.stdout.isTTY !== true) return false; + const env = process.env; + return !env.NO_COLOR && env.TERM !== 'dumb' && !env.CI; +} + +function styleStatus(useColor: boolean, status: DoctorStatus, text: string): string { + if (!useColor) return text; + const code = status === 'pass' ? 32 : status === 'warn' ? 33 : 31; + return `\u001b[${code}m${text}\u001b[39m`; +} + +function styleDim(useColor: boolean, text: string): string { + return useColor ? `\u001b[2m${text}\u001b[22m` : text; +} + +function styleBold(useColor: boolean, text: string): string { + return useColor ? `\u001b[1m${text}\u001b[22m` : text; +} + +function groupOf(entry: DoctorCheck): DoctorGroup { + return entry.group ?? 'project'; +} + +function aggregateStatus(checks: DoctorCheck[]): DoctorStatus { + if (checks.some((c) => c.status === 'fail')) return 'fail'; + if (checks.some((c) => c.status === 'warn')) return 'warn'; + return 'pass'; +} + +function abbreviateHome(filePath: string | undefined): string | undefined { + if (!filePath) return filePath; + const home = process.env.HOME; + if (home && (filePath === home || filePath.startsWith(`${home}/`))) { + return filePath === home ? '~' : `~${filePath.slice(home.length)}`; } - return checks; + return filePath; } -export function formatDoctorReport(report: DoctorReport): string { - const lines = [report.title]; - for (const item of report.checks) { - lines.push(`${item.status.toUpperCase()} ${item.label}: ${item.detail}`); - if (item.fix) { - lines.push(` Fix: ${item.fix}`); +function groupSummaryWhenAllPass(entries: DoctorCheck[]): string { + if (entries.length === 1) { + const only = entries[0]!; + return only.detail || only.label; + } + return entries.map((c) => c.label).join(' · '); +} + +interface RenderOptions { + verbose: boolean; + useColor: boolean; + durationMs?: number; + projectName?: string; + projectDir?: string; + command?: 'setup' | 'project'; +} + +const NEXT_STEPS_PROJECT = ['ktx scan', 'ktx wiki', 'ktx sl ask "…"']; + +export function formatDoctorReport(report: DoctorReport, options: Partial = {}): string { + const opts: RenderOptions = { + verbose: options.verbose ?? false, + useColor: options.useColor ?? false, + durationMs: options.durationMs, + projectName: options.projectName, + projectDir: options.projectDir, + command: options.command, + }; + return renderPlainReport(report, opts); +} + +function renderSetupReport(report: DoctorReport, options: RenderOptions): string { + const { verbose, useColor } = options; + const dim = (text: string) => styleDim(useColor, text); + const bold = (text: string) => styleBold(useColor, text); + const status = (s: DoctorStatus, text: string) => styleStatus(useColor, s, text); + const symbol = (s: DoctorStatus) => status(s, STATUS_SYMBOL[s]); + + const fails = report.checks.filter((c) => c.status === 'fail'); + const lines: string[] = []; + lines.push(bold(report.title)); + lines.push(''); + lines.push(` No project here yet.`); + lines.push(''); + + if (fails.length > 0) { + lines.push(` Before you can run ${bold('ktx setup')}, fix this:`); + for (const entry of fails) { + lines.push(` ${symbol('fail')} ${entry.label}: ${entry.detail}`); + if (entry.fix) { + lines.push(` ${dim(`→ ${entry.fix}`)}`); + } + } + lines.push(''); + } else { + lines.push(` Run ${bold('ktx setup')} to get started.`); + lines.push(''); + } + + if (verbose) { + lines.push(dim(' Toolchain:')); + for (const entry of report.checks) { + lines.push(` ${symbol(entry.status)} ${entry.label}: ${entry.detail}`); + if (entry.fix && entry.status !== 'pass') { + lines.push(` ${dim(`→ ${entry.fix}`)}`); + } + } + lines.push(''); + } + + return lines.join('\n'); +} + +function renderPlainReport(report: DoctorReport, options: RenderOptions): string { + if (options.command === 'setup') return renderSetupReport(report, options); + const { verbose, useColor, durationMs, projectName, projectDir } = options; + const dim = (text: string) => styleDim(useColor, text); + const bold = (text: string) => styleBold(useColor, text); + const status = (s: DoctorStatus, text: string) => styleStatus(useColor, s, text); + const symbol = (s: DoctorStatus) => status(s, STATUS_SYMBOL[s]); + + const lines: string[] = []; + const titleParts: string[] = [bold(report.title)]; + if (projectName) titleParts.push(projectName); + const abbreviatedDir = abbreviateHome(projectDir); + const titleLine = titleParts.join(` ${dim('·')} `); + const dirSuffix = abbreviatedDir ? ` ${dim(`(${abbreviatedDir})`)}` : ''; + lines.push(`${titleLine}${dirSuffix}`); + lines.push(''); + + const groups = new Map(); + for (const entry of report.checks) { + const group = groupOf(entry); + const bucket = groups.get(group) ?? []; + bucket.push(entry); + groups.set(group, bucket); + } + + const orderedGroups: DoctorGroup[] = []; + for (const g of GROUP_ORDER) { + if (groups.has(g)) orderedGroups.push(g); + } + for (const g of groups.keys()) { + if (!orderedGroups.includes(g)) orderedGroups.push(g); + } + + const labelWidth = orderedGroups.reduce( + (max, g) => Math.max(max, (GROUP_LABEL[g] ?? g).length), + 0, + ); + + for (const group of orderedGroups) { + const entries = groups.get(group) ?? []; + const head = aggregateStatus(entries); + const nonPass = entries.filter((c) => c.status !== 'pass'); + const label = (GROUP_LABEL[group] ?? group).padEnd(labelWidth); + + if (nonPass.length === 0) { + lines.push(` ${symbol(head)} ${label} ${dim(groupSummaryWhenAllPass(entries))}`); + if (verbose) { + for (const entry of entries) { + lines.push(` ${symbol(entry.status)} ${entry.label}: ${entry.detail}`); + } + } + continue; + } + + if (entries.length === 1) { + const only = entries[0]!; + lines.push(` ${symbol(only.status)} ${label} ${only.detail}`); + if (only.fix) { + lines.push(` ${' '.repeat(2 + labelWidth + 4)}${dim(`→ ${only.fix}`)}`); + } + continue; + } + + lines.push(` ${symbol(head)} ${label} ${dim(`${nonPass.length} of ${entries.length} need attention`)}`); + for (const entry of entries) { + if (entry.status === 'pass' && !verbose) continue; + lines.push(` ${symbol(entry.status)} ${entry.label}: ${entry.detail}`); + if (entry.fix) { + lines.push(` ${dim(`→ ${entry.fix}`)}`); + } } } + lines.push(''); + + const totalFail = report.checks.filter((c) => c.status === 'fail').length; + const totalWarn = report.checks.filter((c) => c.status === 'warn').length; + const durationText = durationMs !== undefined ? ` ${dim(`(${(durationMs / 1000).toFixed(2)}s)`)}` : ''; + + if (totalFail === 0 && totalWarn === 0) { + const hint = ` ${dim('Try:')} ${NEXT_STEPS_PROJECT.join(dim(' · '))}`; + lines.push(`${status('pass', 'Everything ready.')}${hint}${durationText}`); + } else if (totalFail === 0) { + const word = totalWarn === 1 ? 'warning' : 'warnings'; + lines.push( + `${status('warn', `${totalWarn} ${word}.`)} ${dim('Run')} ktx status --verbose ${dim('for full details.')}${durationText}`, + ); + } else { + const fWord = totalFail === 1 ? 'issue' : 'issues'; + const warnSuffix = + totalWarn > 0 + ? ` ${dim('·')} ${status('warn', `${totalWarn} ${totalWarn === 1 ? 'warning' : 'warnings'}`)}` + : ''; + lines.push( + `${status('fail', `${totalFail} ${fWord} to fix.`)}${warnSuffix}${durationText}`, + ); + } + lines.push(''); + return lines.join('\n'); } @@ -361,12 +441,12 @@ function hasFailures(report: DoctorReport): boolean { return report.checks.some((item) => item.status === 'fail'); } -function writeReport(report: DoctorReport, outputMode: KtxDoctorOutputMode, io: KtxDoctorIo): void { +function writeReport(report: DoctorReport, outputMode: KtxDoctorOutputMode, io: KtxDoctorIo, options: RenderOptions): void { if (outputMode === 'json') { io.stdout.write(`${JSON.stringify(report, null, 2)}\n`); return; } - io.stdout.write(formatDoctorReport(report)); + io.stdout.write(renderPlainReport(report, options)); } export async function runKtxDoctor( @@ -374,18 +454,41 @@ export async function runKtxDoctor( io: KtxDoctorIo = process, deps: KtxDoctorDeps = {}, ): Promise { + const startedAt = Date.now(); try { const runSetupChecks = deps.runSetupChecks ?? (() => runSetupDoctorChecks()); - const setupChecks = await runSetupChecks(); - const report: DoctorReport = - args.command === 'setup' - ? { title: 'KTX setup doctor', checks: setupChecks } - : { - title: 'KTX project doctor', - checks: [...setupChecks, ...(await runProjectChecks(args.projectDir, deps))], - }; - writeReport(report, args.outputMode, io); + if (args.command === 'project') { + const { loadKtxProject } = await import('@ktx/context/project'); + const { buildProjectStatus, renderProjectStatus } = await import('./status-project.js'); + const project = await loadKtxProject({ projectDir: args.projectDir }); + const projectStatus = buildProjectStatus(project); + const verbose = args.verbose ?? false; + const toolchainChecks = verbose ? await runSetupChecks() : undefined; + if (args.outputMode === 'json') { + io.stdout.write(`${JSON.stringify(projectStatus, null, 2)}\n`); + } else { + io.stdout.write( + renderProjectStatus(projectStatus, { + verbose, + useColor: shouldUseColor(io), + durationMs: Date.now() - startedAt, + toolchainChecks, + }), + ); + } + return projectStatus.verdict === 'blocked' ? 1 : 0; + } + + const setupChecks = await runSetupChecks(); + const report: DoctorReport = { title: 'KTX status', checks: setupChecks }; + const renderOptions: RenderOptions = { + verbose: args.verbose ?? false, + useColor: shouldUseColor(io), + durationMs: Date.now() - startedAt, + command: args.command, + }; + writeReport(report, args.outputMode, io, renderOptions); return hasFailures(report) ? 1 : 0; } catch (error) { io.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); diff --git a/packages/cli/src/historic-sql-doctor.test.ts b/packages/cli/src/historic-sql-doctor.test.ts deleted file mode 100644 index f3bc347e..00000000 --- a/packages/cli/src/historic-sql-doctor.test.ts +++ /dev/null @@ -1,202 +0,0 @@ -import { buildDefaultKtxProjectConfig, type KtxProjectConnectionConfig } from '@ktx/context/project'; -import { HistoricSqlExtensionMissingError } from '@ktx/context/ingest'; -import { describe, expect, it, vi } from 'vitest'; -import { - runPostgresHistoricSqlDoctorChecks, - type HistoricSqlDoctorProject, - type PostgresHistoricSqlDoctorProbe, -} from './historic-sql-doctor.js'; - -function projectWithConnections(connections: Record): HistoricSqlDoctorProject { - return { - projectDir: '/tmp/ktx-project', - config: { - ...buildDefaultKtxProjectConfig('warehouse'), - connections, - ingest: { - ...buildDefaultKtxProjectConfig('warehouse').ingest, - adapters: ['live-database', 'historic-sql'], - }, - }, - }; -} - -describe('runPostgresHistoricSqlDoctorChecks', () => { - it('passes when no Postgres historic-SQL connections are enabled', async () => { - const checks = await runPostgresHistoricSqlDoctorChecks( - projectWithConnections({ - warehouse: { driver: 'sqlite', path: './warehouse.db' }, - }), - { - postgresHistoricSqlProbe: vi.fn(), - }, - ); - - expect(checks).toEqual([ - { - id: 'historic-sql-postgres', - label: 'Postgres Historic SQL', - status: 'pass', - detail: 'No enabled Postgres historic-SQL connections', - }, - ]); - }); - - it('passes when the PGSS probe succeeds without warnings', async () => { - const probe = vi.fn(async () => ({ - pgServerVersion: 'PostgreSQL 16.4', - warnings: [], - })); - - const checks = await runPostgresHistoricSqlDoctorChecks( - projectWithConnections({ - warehouse: { - driver: 'postgres', - url: 'env:WAREHOUSE_DATABASE_URL', - historicSql: { enabled: true, dialect: 'postgres' }, - }, - }), - { postgresHistoricSqlProbe: probe }, - ); - - expect(probe).toHaveBeenCalledWith({ - projectDir: '/tmp/ktx-project', - connectionId: 'warehouse', - connection: { - driver: 'postgres', - url: 'env:WAREHOUSE_DATABASE_URL', - historicSql: { enabled: true, dialect: 'postgres' }, - }, - env: process.env, - }); - expect(checks).toEqual([ - { - id: 'historic-sql-postgres-warehouse', - label: 'Postgres Historic SQL (warehouse)', - status: 'pass', - detail: 'pg_stat_statements ready (PostgreSQL 16.4)', - }, - ]); - }); - - it('passes with an informational note when only pg_stat_statements.max is below the recommended floor', async () => { - const checks = await runPostgresHistoricSqlDoctorChecks( - projectWithConnections({ - warehouse: { - driver: 'postgres', - url: 'env:WAREHOUSE_DATABASE_URL', - historicSql: { enabled: true, dialect: 'postgres' }, - }, - }), - { - postgresHistoricSqlProbe: async () => ({ - pgServerVersion: 'PostgreSQL 16.4', - warnings: [], - info: [ - 'pg_stat_statements.max is 1000; set it to at least 5000 to reduce query-template eviction churn', - ], - }), - }, - ); - - expect(checks).toEqual([ - { - id: 'historic-sql-postgres-warehouse', - label: 'Postgres Historic SQL (warehouse)', - status: 'pass', - detail: - 'pg_stat_statements ready (PostgreSQL 16.4); info: pg_stat_statements.max is 1000; set it to at least 5000 to reduce query-template eviction churn', - }, - ]); - }); - - it('warns when pg_stat_statements tracking is disabled', async () => { - const checks = await runPostgresHistoricSqlDoctorChecks( - projectWithConnections({ - warehouse: { - driver: 'postgres', - url: 'env:WAREHOUSE_DATABASE_URL', - historicSql: { enabled: true, dialect: 'postgres' }, - }, - }), - { - postgresHistoricSqlProbe: async () => ({ - pgServerVersion: 'PostgreSQL 16.4', - warnings: [ - 'pg_stat_statements.track is none; set it to top or all in the Postgres parameter group or config', - ], - info: [ - 'pg_stat_statements.max is 1000; set it to at least 5000 to reduce query-template eviction churn', - ], - }), - }, - ); - - expect(checks).toEqual([ - { - id: 'historic-sql-postgres-warehouse', - label: 'Postgres Historic SQL (warehouse)', - status: 'warn', - detail: - 'pg_stat_statements ready (PostgreSQL 16.4) with warnings: pg_stat_statements.track is none; set it to top or all in the Postgres parameter group or config; info: pg_stat_statements.max is 1000; set it to at least 5000 to reduce query-template eviction churn', - fix: 'Update the Postgres parameter group or config, then rerun `ktx status --project-dir /tmp/ktx-project`', - }, - ]); - }); - - it('fails when a connection has postgres historic SQL but is not a Postgres driver', async () => { - const checks = await runPostgresHistoricSqlDoctorChecks( - projectWithConnections({ - warehouse: { - driver: 'mysql', - url: 'env:WAREHOUSE_DATABASE_URL', - historicSql: { enabled: true, dialect: 'postgres' }, - }, - }), - { - postgresHistoricSqlProbe: vi.fn(), - }, - ); - - expect(checks).toEqual([ - { - id: 'historic-sql-postgres-warehouse', - label: 'Postgres Historic SQL (warehouse)', - status: 'fail', - detail: 'connections.warehouse.historicSql.dialect is postgres but driver is mysql', - fix: 'Set connections.warehouse.driver to postgres or disable historicSql for this connection', - }, - ]); - }); - - it('maps PGSS capability errors to actionable failures', async () => { - const checks = await runPostgresHistoricSqlDoctorChecks( - projectWithConnections({ - warehouse: { - driver: 'postgres', - url: 'env:WAREHOUSE_DATABASE_URL', - historicSql: { enabled: true, dialect: 'postgres' }, - }, - }), - { - postgresHistoricSqlProbe: async () => { - throw new HistoricSqlExtensionMissingError({ - dialect: 'postgres', - message: 'pg_stat_statements extension is not installed in the connection database.', - remediation: 'Run CREATE EXTENSION pg_stat_statements; against the connection database.', - }); - }, - }, - ); - - expect(checks).toEqual([ - { - id: 'historic-sql-postgres-warehouse', - label: 'Postgres Historic SQL (warehouse)', - status: 'fail', - detail: 'pg_stat_statements extension is not installed in the connection database.', - fix: 'Run CREATE EXTENSION pg_stat_statements; against the connection database.', - }, - ]); - }); -}); diff --git a/packages/cli/src/historic-sql-doctor.ts b/packages/cli/src/historic-sql-doctor.ts deleted file mode 100644 index bb9a681c..00000000 --- a/packages/cli/src/historic-sql-doctor.ts +++ /dev/null @@ -1,167 +0,0 @@ -import type { KtxProjectConfig, KtxProjectConnectionConfig } from '@ktx/context/project'; -import type { DoctorCheck } from './doctor.js'; - -export interface HistoricSqlDoctorProject { - projectDir: string; - config: Pick; -} - -export interface PostgresHistoricSqlDoctorProbeInput { - projectDir: string; - connectionId: string; - connection: KtxProjectConnectionConfig; - env: NodeJS.ProcessEnv; -} - -export interface PostgresHistoricSqlDoctorProbeResult { - pgServerVersion: string; - warnings: string[]; - info?: string[]; -} - -export type PostgresHistoricSqlDoctorProbe = ( - input: PostgresHistoricSqlDoctorProbeInput, -) => Promise; - -export interface HistoricSqlDoctorDeps { - env?: NodeJS.ProcessEnv; - postgresHistoricSqlProbe?: PostgresHistoricSqlDoctorProbe; -} - -function check(status: DoctorCheck['status'], id: string, label: string, detail: string, fix?: string): DoctorCheck { - return fix ? { id, label, status, detail, fix } : { id, label, status, detail }; -} - -function historicSqlRecord(connection: KtxProjectConnectionConfig): Record | null { - const historicSql = connection.historicSql; - return historicSql && typeof historicSql === 'object' && !Array.isArray(historicSql) - ? (historicSql as Record) - : null; -} - -function isEnabledPostgresHistoricSql(connection: KtxProjectConnectionConfig): boolean { - const historicSql = historicSqlRecord(connection); - return historicSql?.enabled === true && historicSql.dialect === 'postgres'; -} - -function isPostgresDriver(connection: KtxProjectConnectionConfig): boolean { - const driver = String(connection.driver ?? '').toLowerCase(); - return driver === 'postgres' || driver === 'postgresql'; -} - -function checkId(connectionId: string): string { - return `historic-sql-postgres-${connectionId.replace(/[^a-z0-9_-]+/gi, '-')}`; -} - -function capabilityFailureFix(error: unknown, connectionId: string, projectDir: string): string { - if (error instanceof Error && error.name === 'HistoricSqlExtensionMissingError' && 'remediation' in error) { - return String(error.remediation); - } - if (error instanceof Error && error.name === 'HistoricSqlGrantsMissingError' && 'remediation' in error) { - return String(error.remediation); - } - if (error instanceof Error && error.name === 'HistoricSqlVersionUnsupportedError') { - return 'Use PostgreSQL 14 or newer, or disable historicSql for this connection'; - } - return `Fix connections.${connectionId} Postgres settings, then rerun \`ktx status --project-dir ${projectDir}\``; -} - -function failureDetail(error: unknown): string { - if (error instanceof Error && error.message.trim().length > 0) { - return error.message.trim().split('\n')[0] ?? error.message.trim(); - } - return String(error); -} - -function readinessDetail(result: PostgresHistoricSqlDoctorProbeResult): string { - const warningText = result.warnings.length > 0 ? ` with warnings: ${result.warnings.join('; ')}` : ''; - const info = result.info ?? []; - const infoText = info.length > 0 ? `; info: ${info.join('; ')}` : ''; - return `pg_stat_statements ready (${result.pgServerVersion})${warningText}${infoText}`; -} - -async function defaultPostgresHistoricSqlProbe( - input: PostgresHistoricSqlDoctorProbeInput, -): Promise { - const [{ PostgresPgssReader }, { KtxPostgresHistoricSqlQueryClient, isKtxPostgresConnectionConfig }] = - await Promise.all([import('@ktx/context/ingest'), import('@ktx/connector-postgres')]); - - const inputDriver = input.connection.driver ?? 'unknown'; - if (!isKtxPostgresConnectionConfig(input.connection)) { - throw new Error(`Native PostgreSQL connector cannot run driver "${inputDriver}"`); - } - - const client = new KtxPostgresHistoricSqlQueryClient({ - connectionId: input.connectionId, - connection: input.connection, - env: input.env, - }); - try { - return await new PostgresPgssReader().probe(client); - } finally { - await client.cleanup(); - } -} - -export async function runPostgresHistoricSqlDoctorChecks( - project: HistoricSqlDoctorProject, - deps: HistoricSqlDoctorDeps = {}, -): Promise { - const targets = Object.entries(project.config.connections) - .filter(([, connection]) => isEnabledPostgresHistoricSql(connection)) - .sort(([left], [right]) => left.localeCompare(right)); - - if (targets.length === 0) { - return [ - check('pass', 'historic-sql-postgres', 'Postgres Historic SQL', 'No enabled Postgres historic-SQL connections'), - ]; - } - - const probe = deps.postgresHistoricSqlProbe ?? defaultPostgresHistoricSqlProbe; - const env = deps.env ?? process.env; - const checks: DoctorCheck[] = []; - for (const [connectionId, connection] of targets) { - const label = `Postgres Historic SQL (${connectionId})`; - if (!isPostgresDriver(connection)) { - checks.push( - check( - 'fail', - checkId(connectionId), - label, - `connections.${connectionId}.historicSql.dialect is postgres but driver is ${String(connection.driver)}`, - `Set connections.${connectionId}.driver to postgres or disable historicSql for this connection`, - ), - ); - continue; - } - - try { - const result = await probe({ projectDir: project.projectDir, connectionId, connection, env }); - if (result.warnings.length > 0) { - checks.push( - check( - 'warn', - checkId(connectionId), - label, - readinessDetail(result), - `Update the Postgres parameter group or config, then rerun \`ktx status --project-dir ${project.projectDir}\``, - ), - ); - } else { - checks.push(check('pass', checkId(connectionId), label, readinessDetail(result))); - } - } catch (error) { - checks.push( - check( - 'fail', - checkId(connectionId), - label, - failureDetail(error), - capabilityFailureFix(error, connectionId, project.projectDir), - ), - ); - } - } - - return checks; -} diff --git a/packages/cli/src/index.test.ts b/packages/cli/src/index.test.ts index 305cf30e..d1c2587e 100644 --- a/packages/cli/src/index.test.ts +++ b/packages/cli/src/index.test.ts @@ -1012,7 +1012,7 @@ describe('runKtxCli', () => { expect(setup).not.toHaveBeenCalled(); expect(doctor).toHaveBeenCalledWith( - { command: 'project', projectDir: tempDir, outputMode: 'json', inputMode: 'disabled' }, + { command: 'project', projectDir: tempDir, outputMode: 'json', inputMode: 'disabled', verbose: false }, statusIo.io, ); expect(statusIo.stderr()).toBe(''); @@ -1035,7 +1035,7 @@ describe('runKtxCli', () => { await expect(runKtxCli(['status', '--json', '--no-input'], statusIo.io, { doctor })).resolves.toBe(0); expect(doctor).toHaveBeenCalledWith( - { command: 'setup', outputMode: 'json', inputMode: 'disabled' }, + { command: 'setup', outputMode: 'json', inputMode: 'disabled', verbose: false }, statusIo.io, ); expect(statusIo.stderr()).toBe(''); diff --git a/packages/cli/src/status-project.ts b/packages/cli/src/status-project.ts new file mode 100644 index 00000000..47b9a49f --- /dev/null +++ b/packages/cli/src/status-project.ts @@ -0,0 +1,614 @@ +import type { + KtxLocalProject, + KtxProjectConfig, + KtxProjectConnectionConfig, + KtxProjectEmbeddingConfig, + KtxProjectLlmConfig, +} from '@ktx/context/project'; +import type { DoctorCheck } from './doctor.js'; + +type ProjectStatusLevel = 'ok' | 'warn' | 'fail'; +type ProjectVerdict = 'ready' | 'partial' | 'blocked'; + +interface ProjectStatusLine { + status: ProjectStatusLevel; + detail: string; + fix?: string; +} + +interface LlmStatus extends ProjectStatusLine { + backend: string; + model?: string; +} + +interface EmbeddingsStatus extends ProjectStatusLine { + backend: string; + model?: string; + dimensions?: number; +} + +interface ConnectionStatus extends ProjectStatusLine { + name: string; + driver: string; +} + +interface PipelineStatus { + adapters: string[]; + enrichmentMode: string; + relationshipsEnabled: boolean; + relationshipsLlmProposals: boolean; + relationshipsValidationRequired: boolean; + agentEnabled: boolean; + agentTools: string[]; + agentMaxIterations: number; +} + +interface StorageStatus { + state: string; + search: string; + gitAutoCommit: boolean; + gitAuthor: string; +} + +interface WarningItem { + message: string; + fix?: string; +} + +export interface ProjectStatus { + projectName: string; + projectDir: string; + llm: LlmStatus; + embeddings: EmbeddingsStatus; + storage: StorageStatus; + connections: ConnectionStatus[]; + pipeline: PipelineStatus; + warnings: WarningItem[]; + verdict: ProjectVerdict; + verdictReason: string; + nextActions: string[]; + promptCaching?: { enabled: boolean; systemTtl?: string; toolsTtl?: string; historyTtl?: string }; + workUnits?: { stepBudget: number; maxConcurrency: number; failureMode: string }; + memoryAutoCommit: boolean; + relationshipsDetail?: { + acceptThreshold: number; + reviewThreshold: number; + maxLlmTablesPerBatch: number; + validationConcurrency: number; + }; +} + +function resolveRef(value: unknown, env: NodeJS.ProcessEnv): { resolved: string; via: 'literal' | 'env' | 'file' | 'missing' } { + if (typeof value !== 'string') return { resolved: '', via: 'missing' }; + const trimmed = value.trim(); + if (trimmed.length === 0) return { resolved: '', via: 'missing' }; + if (trimmed.startsWith('env:')) { + const name = trimmed.slice(4).trim(); + const v = env[name]; + return v && v.trim().length > 0 ? { resolved: v, via: 'env' } : { resolved: '', via: 'missing' }; + } + if (trimmed.startsWith('file:')) { + return { resolved: trimmed.slice(5), via: 'file' }; + } + return { resolved: trimmed, via: 'literal' }; +} + +function envHint(value: unknown): string | undefined { + if (typeof value === 'string' && value.trim().startsWith('env:')) { + return value.trim().slice(4).trim(); + } + return undefined; +} + +function buildLlmStatus(config: KtxProjectLlmConfig, env: NodeJS.ProcessEnv): LlmStatus { + const backend = config.provider.backend; + const model = config.models?.default; + if (backend === 'none') { + return { + backend, + model, + status: 'fail', + detail: 'no LLM configured — ktx ask will not work', + fix: 'Run: ktx setup (choose an LLM provider)', + }; + } + if (backend === 'anthropic') { + const ref = config.provider.anthropic?.api_key; + const resolved = resolveRef(ref, env); + if (resolved.resolved.length > 0) { + return { backend, model, status: 'ok', detail: `key set${resolved.via === 'env' ? ` (env)` : ''}` }; + } + if (env.ANTHROPIC_API_KEY && env.ANTHROPIC_API_KEY.trim().length > 0) { + return { backend, model, status: 'ok', detail: 'key set (env: ANTHROPIC_API_KEY)' }; + } + const hint = envHint(ref); + return { + backend, + model, + status: 'warn', + detail: hint ? `key missing (env: ${hint})` : 'key missing', + fix: hint ? `Set ${hint}` : 'Set ANTHROPIC_API_KEY or rerun `ktx setup`', + }; + } + if (backend === 'vertex') { + const project = config.provider.vertex?.project; + if (project && project.length > 0) { + return { backend, model, status: 'ok', detail: `project=${project}` }; + } + return { backend, model, status: 'warn', detail: 'vertex project not configured', fix: 'Rerun `ktx setup`' }; + } + if (backend === 'gateway') { + const ref = config.provider.gateway?.api_key; + const resolved = resolveRef(ref, env); + if (resolved.resolved.length > 0) { + return { backend, model, status: 'ok', detail: 'key set' }; + } + const hint = envHint(ref); + return { + backend, + model, + status: 'warn', + detail: hint ? `key missing (env: ${hint})` : 'key missing', + fix: hint ? `Set ${hint}` : 'Set the gateway api_key or rerun `ktx setup`', + }; + } + return { backend, model, status: 'warn', detail: 'unknown LLM backend' }; +} + +function buildEmbeddingsStatus(config: KtxProjectEmbeddingConfig, env: NodeJS.ProcessEnv): EmbeddingsStatus { + const backend = config.backend; + const model = config.model; + const dimensions = config.dimensions; + if (backend === 'none') { + return { + backend, + model, + dimensions, + status: 'warn', + detail: 'disabled — semantic search will be skipped', + }; + } + if (backend === 'deterministic') { + return { + backend, + model, + dimensions, + status: 'warn', + detail: 'deterministic — semantic search degraded (lexical/dictionary lanes still work)', + }; + } + if (backend === 'openai') { + const ref = config.openai?.api_key; + const resolved = resolveRef(ref, env); + if (resolved.resolved.length > 0 || (env.OPENAI_API_KEY && env.OPENAI_API_KEY.trim().length > 0)) { + return { backend, model, dimensions, status: 'ok', detail: 'key set' }; + } + const hint = envHint(ref); + return { + backend, + model, + dimensions, + status: 'warn', + detail: hint ? `key missing (env: ${hint})` : 'key missing', + fix: hint ? `Set ${hint}` : 'Set OPENAI_API_KEY or rerun `ktx setup`', + }; + } + if (backend === 'sentence-transformers') { + const url = config.sentenceTransformers?.base_url; + if (typeof url === 'string' && url.length > 0) { + return { backend, model, dimensions, status: 'ok', detail: `service: ${url}` }; + } + return { + backend, + model, + dimensions, + status: 'warn', + detail: 'no base_url configured', + fix: 'Rerun `ktx setup`', + }; + } + return { backend, model, dimensions, status: 'warn', detail: 'unknown embedding backend' }; +} + +function buildConnectionStatus( + name: string, + conn: KtxProjectConnectionConfig, + env: NodeJS.ProcessEnv, +): ConnectionStatus { + const driver = (conn.driver ?? 'unknown').toLowerCase(); + const ok = (detail: string): ConnectionStatus => ({ name, driver, status: 'ok', detail }); + const warn = (detail: string, fix?: string): ConnectionStatus => ({ name, driver, status: 'warn', detail, fix }); + + switch (driver) { + case 'postgres': + case 'postgresql': + case 'mysql': + case 'clickhouse': + case 'sqlserver': { + const urlRef = resolveRef(conn.url, env); + if (urlRef.resolved.length > 0) return ok(`url configured`); + if (typeof (conn as Record).host === 'string') return ok('host configured'); + const hint = envHint(conn.url); + return warn(hint ? `url missing (env: ${hint})` : 'url not set', hint ? `Set ${hint}` : 'Rerun `ktx setup`'); + } + case 'snowflake': { + const account = (conn as Record).account; + if (typeof account === 'string' && account.length > 0) return ok(`account: ${account}`); + return warn('account not set', 'Rerun `ktx setup`'); + } + case 'bigquery': { + const cred = resolveRef((conn as Record).credentials_json, env); + if (cred.resolved.length > 0) return ok('credentials configured'); + const hint = envHint((conn as Record).credentials_json); + return warn(hint ? `credentials missing (env: ${hint})` : 'credentials not set', hint ? `Set ${hint}` : 'Rerun `ktx setup`'); + } + case 'sqlite': { + const path = (conn as Record).path; + if (typeof path === 'string' && path.length > 0) return ok(`path: ${path}`); + return warn('path not set', 'Rerun `ktx setup`'); + } + case 'notion': { + const tokenRef = + (conn as Record).auth_token_ref ?? + (conn as Record).auth_token; + const resolved = resolveRef(tokenRef, env); + if (resolved.resolved.length > 0) return ok('auth token configured'); + const hint = envHint(tokenRef); + return warn(hint ? `auth token missing (env: ${hint})` : 'auth token not set', hint ? `Set ${hint}` : 'Rerun `ktx setup`'); + } + case 'dbt': + case 'dbt-core': + case 'dbt-cloud': { + const repoUrl = + (conn as Record).repoUrl ?? + (conn as Record).repo_url; + if (typeof repoUrl === 'string' && repoUrl.length > 0) return ok(`repo: ${repoUrl}`); + return warn('repoUrl not set', 'Rerun `ktx setup`'); + } + case 'metabase': { + const url = (conn as Record).url ?? (conn as Record).base_url; + if (typeof url === 'string' && url.length > 0) return ok(`url: ${url}`); + return warn('url not set', 'Rerun `ktx setup`'); + } + case 'looker': + case 'lookml': { + const url = (conn as Record).base_url ?? (conn as Record).url; + if (typeof url === 'string' && url.length > 0) return ok(`url: ${url}`); + return warn('base_url not set', 'Rerun `ktx setup`'); + } + case 'metricflow': { + const repoUrl = (conn as Record).repoUrl ?? (conn as Record).repo_url; + if (typeof repoUrl === 'string' && repoUrl.length > 0) return ok(`repo: ${repoUrl}`); + return warn('repoUrl not set', 'Rerun `ktx setup`'); + } + default: + return { name, driver, status: 'ok', detail: 'configured' }; + } +} + +const ADAPTER_DRIVER_REQUIREMENT: Record = { + 'live-database': ['postgres', 'postgresql', 'mysql', 'snowflake', 'bigquery', 'clickhouse', 'sqlite', 'sqlserver'], + dbt: ['dbt', 'dbt-core', 'dbt-cloud'], + notion: ['notion'], + metabase: ['metabase'], + looker: ['looker', 'lookml'], + lookml: ['looker', 'lookml'], + metricflow: ['metricflow'], +}; + +function buildPipelineStatus(config: KtxProjectConfig): PipelineStatus { + return { + adapters: config.ingest.adapters, + enrichmentMode: config.scan.enrichment.mode, + relationshipsEnabled: config.scan.relationships.enabled, + relationshipsLlmProposals: config.scan.relationships.llmProposals, + relationshipsValidationRequired: config.scan.relationships.validationRequiredForManifest, + agentEnabled: config.agent.run_research.enabled, + agentTools: config.agent.run_research.default_toolset, + agentMaxIterations: config.agent.run_research.max_iterations, + }; +} + +function buildStorageStatus(config: KtxProjectConfig): StorageStatus { + return { + state: config.storage.state, + search: config.storage.search, + gitAutoCommit: config.storage.git.auto_commit, + gitAuthor: config.storage.git.author, + }; +} + +function buildWarnings( + config: KtxProjectConfig, + connections: ConnectionStatus[], + llm: LlmStatus, + embeddings: EmbeddingsStatus, +): WarningItem[] { + const warnings: WarningItem[] = []; + + for (const adapter of config.ingest.adapters) { + const requiredDrivers = ADAPTER_DRIVER_REQUIREMENT[adapter]; + if (!requiredDrivers) continue; + const hasMatching = connections.some((c) => requiredDrivers.includes(c.driver)); + if (!hasMatching) { + warnings.push({ + message: `Adapter "${adapter}" is enabled but no connection of type ${requiredDrivers.slice(0, 2).join('/')} is configured.`, + fix: 'Rerun `ktx setup` to add a connection, or remove the adapter from ingest.adapters.', + }); + } + } + + if (config.agent.run_research.enabled && llm.backend === 'none') { + warnings.push({ + message: 'Research agent is enabled but LLM is not configured.', + fix: 'Set up an LLM provider via `ktx setup` or disable agent.run_research.enabled.', + }); + } + + if (embeddings.backend === 'none' && config.ingest.adapters.includes('live-database')) { + warnings.push({ + message: 'Semantic search is off (embeddings backend = none). Lexical/dictionary lanes still work.', + }); + } + + return warnings; +} + +function buildVerdict( + llm: LlmStatus, + embeddings: EmbeddingsStatus, + connections: ConnectionStatus[], + warnings: WarningItem[], +): { verdict: ProjectVerdict; reason: string; nextActions: string[] } { + if (llm.status === 'fail') { + return { + verdict: 'blocked', + reason: 'LLM not configured — `ktx ask` will not work.', + nextActions: ['ktx setup'], + }; + } + + const reasons: string[] = []; + if (llm.status === 'warn') reasons.push('LLM credentials missing'); + if (embeddings.status === 'warn') { + if (embeddings.backend === 'deterministic' || embeddings.backend === 'none') { + reasons.push('semantic search disabled'); + } else { + reasons.push('embedding credentials missing'); + } + } + const missing = connections.filter((c) => c.status !== 'ok').length; + if (missing > 0) reasons.push(`${missing} connection${missing === 1 ? '' : 's'} need configuration`); + if (warnings.length > 0) reasons.push(`${warnings.length} config warning${warnings.length === 1 ? '' : 's'}`); + + if (reasons.length === 0) { + return { + verdict: 'ready', + reason: 'Ready.', + nextActions: ['ktx scan', 'ktx wiki', 'ktx sl ask "…"'], + }; + } + + return { + verdict: 'partial', + reason: `Partially ready — ${reasons.join('; ')}.`, + nextActions: ['ktx setup'], + }; +} + +export interface BuildProjectStatusOptions { + env?: NodeJS.ProcessEnv; +} + +export function buildProjectStatus(project: KtxLocalProject, options: BuildProjectStatusOptions = {}): ProjectStatus { + const env = options.env ?? process.env; + const config = project.config; + + const llm = buildLlmStatus(config.llm, env); + const embeddings = buildEmbeddingsStatus(config.ingest.embeddings, env); + const storage = buildStorageStatus(config); + const connections = Object.entries(config.connections).map(([name, conn]) => + buildConnectionStatus(name, conn, env), + ); + const pipeline = buildPipelineStatus(config); + const warnings = buildWarnings(config, connections, llm, embeddings); + const { verdict, reason, nextActions } = buildVerdict(llm, embeddings, connections, warnings); + + return { + projectName: config.project, + projectDir: project.projectDir, + llm, + embeddings, + storage, + connections, + pipeline, + warnings, + verdict, + verdictReason: reason, + nextActions, + promptCaching: config.llm.promptCaching + ? { + enabled: config.llm.promptCaching.enabled ?? false, + systemTtl: config.llm.promptCaching.systemTtl, + toolsTtl: config.llm.promptCaching.toolsTtl, + historyTtl: config.llm.promptCaching.historyTtl, + } + : undefined, + workUnits: { + stepBudget: config.ingest.workUnits.stepBudget, + maxConcurrency: config.ingest.workUnits.maxConcurrency, + failureMode: config.ingest.workUnits.failureMode, + }, + memoryAutoCommit: config.memory.auto_commit, + relationshipsDetail: { + acceptThreshold: config.scan.relationships.acceptThreshold, + reviewThreshold: config.scan.relationships.reviewThreshold, + maxLlmTablesPerBatch: config.scan.relationships.maxLlmTablesPerBatch, + validationConcurrency: config.scan.relationships.validationConcurrency, + }, + }; +} + +// ─── Rendering ────────────────────────────────────────────────────────────── + +const SYMBOL: Record = { ok: '✓', warn: '⚠', fail: '✗' }; + +function ansi(useColor: boolean, code: string, text: string, closer = '39'): string { + return useColor ? `\u001b[${code}m${text}\u001b[${closer}m` : text; +} + +function colorFor(level: ProjectStatusLevel): string { + return level === 'ok' ? '32' : level === 'warn' ? '33' : '31'; +} + +function abbreviateHome(filePath: string, env: NodeJS.ProcessEnv): string { + const home = env.HOME; + if (home && (filePath === home || filePath.startsWith(`${home}/`))) { + return filePath === home ? '~' : `~${filePath.slice(home.length)}`; + } + return filePath; +} + +export interface RenderProjectStatusOptions { + verbose?: boolean; + useColor?: boolean; + durationMs?: number; + toolchainChecks?: DoctorCheck[]; + env?: NodeJS.ProcessEnv; +} + +export function renderProjectStatus(status: ProjectStatus, options: RenderProjectStatusOptions = {}): string { + const verbose = options.verbose ?? false; + const useColor = options.useColor ?? false; + const env = options.env ?? process.env; + const dim = (s: string) => ansi(useColor, '2', s, '22'); + const bold = (s: string) => ansi(useColor, '1', s, '22'); + const color = (level: ProjectStatusLevel, s: string) => ansi(useColor, colorFor(level), s); + const sym = (level: ProjectStatusLevel) => color(level, SYMBOL[level]); + + const lines: string[] = []; + const dirStr = abbreviateHome(status.projectDir, env); + lines.push(`${bold('KTX status')} ${dim('·')} ${status.projectName} ${dim(`(${dirStr})`)}`); + lines.push(''); + + const labelPad = 'Connections'.length; + const label = (text: string) => text.padEnd(labelPad); + + // Core readiness rows + const llmDetail = [status.llm.backend, status.llm.model].filter(Boolean).join(` ${dim('·')} `); + lines.push(` ${label('LLM')} ${llmDetail} ${sym(status.llm.status)} ${dim(status.llm.detail)}`); + + const embedParts = [status.embeddings.backend]; + if (status.embeddings.model) embedParts.push(status.embeddings.model); + const embedDim = status.embeddings.dimensions ? `(${status.embeddings.dimensions}d)` : ''; + const embedDetail = `${embedParts.join(` ${dim('·')} `)}${embedDim ? ` ${embedDim}` : ''}`; + lines.push(` ${label('Embeddings')} ${embedDetail} ${sym(status.embeddings.status)} ${dim(status.embeddings.detail)}`); + + lines.push(` ${label('Storage')} ${dim(`${status.storage.state} (state) · ${status.storage.search} (search)`)}`); + lines.push(''); + + // Connections + if (status.connections.length === 0) { + lines.push(` ${bold('Connections')} ${dim('(none)')}`); + lines.push(` ${dim('No connections configured. Run `ktx setup` to add one.')}`); + } else { + lines.push(` ${bold('Connections')} ${dim(`(${status.connections.length})`)}`); + const nameWidth = Math.max(...status.connections.map((c) => c.name.length)); + const driverWidth = Math.max(...status.connections.map((c) => c.driver.length)); + for (const conn of status.connections) { + lines.push( + ` ${sym(conn.status)} ${conn.name.padEnd(nameWidth)} ${dim(conn.driver.padEnd(driverWidth))} ${conn.detail}`, + ); + if (conn.fix && conn.status !== 'ok') { + const indent = 6 + nameWidth + 3 + driverWidth + 3; + lines.push(`${' '.repeat(indent)}${dim(`→ ${conn.fix}`)}`); + } + } + } + lines.push(''); + + // Pipeline + lines.push(` ${bold('Pipeline')}`); + const pipelineLabelWidth = Math.max('Adapters'.length, 'Enrichment'.length, 'Research agent'.length); + const pLabel = (text: string) => text.padEnd(pipelineLabelWidth); + lines.push(` ${pLabel('Adapters')} ${status.pipeline.adapters.length > 0 ? status.pipeline.adapters.join(', ') : dim('(none)')}`); + const enrichmentDetail = [`${status.pipeline.enrichmentMode} mode`]; + if (status.pipeline.relationshipsEnabled) { + const bits = ['relationships on']; + if (status.pipeline.relationshipsLlmProposals) bits.push('LLM proposals'); + if (status.pipeline.relationshipsValidationRequired) bits.push('validation required'); + enrichmentDetail.push(bits.join(', ')); + } else { + enrichmentDetail.push('relationships off'); + } + lines.push(` ${pLabel('Enrichment')} ${enrichmentDetail.join(` ${dim('·')} `)}`); + const agentDetail = status.pipeline.agentEnabled + ? `enabled ${dim(`(${status.pipeline.agentTools.length} tool${status.pipeline.agentTools.length === 1 ? '' : 's'})`)}` + : dim('disabled'); + lines.push(` ${pLabel('Research agent')} ${agentDetail}`); + lines.push(''); + + // Warnings + if (status.warnings.length > 0) { + lines.push(` ${bold('Warnings')}`); + for (const w of status.warnings) { + lines.push(` ${color('warn', SYMBOL.warn)} ${w.message}`); + if (w.fix) lines.push(` ${dim(`→ ${w.fix}`)}`); + } + lines.push(''); + } + + // Verbose extras + if (verbose) { + if (options.toolchainChecks && options.toolchainChecks.length > 0) { + lines.push(` ${bold('Toolchain')}`); + for (const check of options.toolchainChecks) { + const lv: ProjectStatusLevel = check.status === 'pass' ? 'ok' : check.status === 'warn' ? 'warn' : 'fail'; + lines.push(` ${sym(lv)} ${check.label}: ${check.detail}`); + if (check.fix && lv !== 'ok') lines.push(` ${dim(`→ ${check.fix}`)}`); + } + lines.push(''); + } + if (status.promptCaching) { + const pc = status.promptCaching; + const bits = [`enabled=${pc.enabled}`]; + if (pc.systemTtl) bits.push(`system=${pc.systemTtl}`); + if (pc.toolsTtl) bits.push(`tools=${pc.toolsTtl}`); + if (pc.historyTtl) bits.push(`history=${pc.historyTtl}`); + lines.push(` ${bold('Prompt caching')} ${dim(bits.join(', '))}`); + } + if (status.workUnits) { + const wu = status.workUnits; + lines.push(` ${bold('Work units')} ${dim(`stepBudget=${wu.stepBudget}, maxConcurrency=${wu.maxConcurrency}, failureMode=${wu.failureMode}`)}`); + } + if (status.relationshipsDetail) { + const r = status.relationshipsDetail; + lines.push( + ` ${bold('Relationships')} ${dim(`accept=${r.acceptThreshold}, review=${r.reviewThreshold}, maxLlmTables=${r.maxLlmTablesPerBatch}, concurrency=${r.validationConcurrency}`)}`, + ); + } + lines.push( + ` ${bold('Agent')} ${dim(`max_iterations=${status.pipeline.agentMaxIterations}, tools=${status.pipeline.agentTools.join(', ') || '(none)'}`)}`, + ); + lines.push(` ${bold('Memory')} ${dim(`auto_commit=${status.memoryAutoCommit}`)}`); + lines.push( + ` ${bold('Git')} ${dim(`auto_commit=${status.storage.gitAutoCommit}, author=${status.storage.gitAuthor}`)}`, + ); + lines.push(''); + } + + // Verdict + next steps + const verdictLevel: ProjectStatusLevel = + status.verdict === 'ready' ? 'ok' : status.verdict === 'partial' ? 'warn' : 'fail'; + const duration = options.durationMs !== undefined ? ` ${dim(`(${(options.durationMs / 1000).toFixed(2)}s)`)}` : ''; + if (status.verdict === 'ready') { + const hint = ` ${dim('Try:')} ${status.nextActions.join(dim(' · '))}`; + lines.push(`${color(verdictLevel, status.verdictReason)}${hint}${duration}`); + } else { + const hint = status.nextActions.length > 0 ? ` ${dim('Next:')} ${status.nextActions.join(dim(' · '))}` : ''; + lines.push(`${color(verdictLevel, status.verdictReason)}${hint}${duration}`); + } + lines.push(''); + + return lines.join('\n'); +} From 28b5e2a83e543ac2b215f2e07f44fbdcf30c5139 Mon Sep 17 00:00:00 2001 From: Andrey Avtomonov Date: Thu, 14 May 2026 00:57:51 +0200 Subject: [PATCH 5/5] fix: align KTX agent tools and repair handling (#73) --- .../memory_agent_bundle_ingest_reconcile.md | 2 +- .../context/skills/ingest_triage/SKILL.md | 4 +-- .../context/skills/lookml_ingest/SKILL.md | 2 +- .../context/skills/notion_synthesize/SKILL.md | 4 +-- packages/context/skills/sl/SKILL.md | 27 +++++++------- packages/context/skills/sl_capture/SKILL.md | 36 ++++++++++++------- packages/context/skills/wiki_capture/SKILL.md | 4 +-- .../src/agent/agent-runner.service.test.ts | 4 +++ .../context/src/agent/agent-runner.service.ts | 3 ++ .../src/ingest/ingest-bundle.runner.test.ts | 10 ++++-- .../src/ingest/stages/stage-index.types.ts | 4 ++- .../emit-reconciliation-records.tool.test.ts | 24 +++++++++++++ .../tools/emit-unmapped-fallback.tool.ts | 6 ++++ .../ingest/tools/eviction-list.tool.test.ts | 2 +- .../src/ingest/tools/eviction-list.tool.ts | 2 +- .../ingest/tools/verification-ledger.tool.ts | 2 +- packages/context/src/llm/generation.ts | 18 ++++++++++ .../context/src/sl/tools/sl-discover.tool.ts | 2 +- .../src/sl/tools/sl-read-source.tool.ts | 2 +- 19 files changed, 113 insertions(+), 45 deletions(-) diff --git a/packages/context/prompts/memory_agent_bundle_ingest_reconcile.md b/packages/context/prompts/memory_agent_bundle_ingest_reconcile.md index 515fecd3..3813afcb 100644 --- a/packages/context/prompts/memory_agent_bundle_ingest_reconcile.md +++ b/packages/context/prompts/memory_agent_bundle_ingest_reconcile.md @@ -12,7 +12,7 @@ Parsimonious. Stage 3 WUs already loaded `ingest_triage` and handled conflicts t 3. If the system prompt includes ``, apply those pins before flagging a same-name or near-duplicate conflict. A pinned `canonicalArtifactKey` keeps the contested name when it is present in the Stage Index; competing variants keep or receive disambiguated names. 4. Sweep both exact-key conflicts and near-duplicate writes. Compare WUs that wrote overlapping SL source names, overlapping wiki keys, the same `tables:` or `sl_refs:` action details, or obviously equivalent topic titles under different wiki keys. Call `stage_diff` to see the actual difference, and use `wiki_read`/`sl_read_source` when two different keys appear to describe the same table, metric, or source-of-truth mapping. If they're the same content, leave one canonical artifact and record the duplicate as subsumed. If they differ per `ingest_triage` rules, apply the correct resolution (rename + capture; election of canonical; silent replace for expression-only re-ingest change; or pinned canonical), then call `emit_conflict_resolution` with the artifact key and decision. 5. For any `wiki_write`, `wiki_remove`, `sl_write_source`, or `sl_edit_source` call you make during reconciliation, include `rawPaths` with only the raw paths that directly caused that reconciliation action. -6. Call `eviction_list()` for deleted raw paths. For each listed artifact, remove it (`sl_delete`, `wiki_remove`) and include the evicted raw path in `rawPaths`. Then call `emit_eviction_decision` with `action: "removed"` for every removed artifact. +6. Call `eviction_list()` for deleted raw paths. For each listed artifact, remove it (`sl_write_source`/`sl_edit_source` with `delete: true` for SL sources, `wiki_remove` for wiki pages) and include the evicted raw path in `rawPaths`. Then call `emit_eviction_decision` with `action: "removed"` for every removed artifact. 7. If the Stage 4 sweep discovers a raw file whose only honest outcome is standalone SQL, wiki-only capture, or a human flag, call `emit_unmapped_fallback` with the raw path, reason, and fallback kind. 8. Use `read_raw_span` to zoom into specific raw files when you need to resolve what two contested measures or wiki pages actually describe. 9. Exit when you've processed every item. diff --git a/packages/context/skills/ingest_triage/SKILL.md b/packages/context/skills/ingest_triage/SKILL.md index df13ed83..c7cee225 100644 --- a/packages/context/skills/ingest_triage/SKILL.md +++ b/packages/context/skills/ingest_triage/SKILL.md @@ -7,7 +7,7 @@ callers: [memory_agent] # Ingest Triage — conflict classification and resolution This skill is loaded in two contexts: -- By a Stage 3 WorkUnit agent when `sl_discover` or an `sl_discover` reveals that a prior WU (or a prior sync) already wrote something that overlaps with what the current WU is about to write. +- By a Stage 3 WorkUnit agent when `sl_discover` reveals that a prior WU (or a prior sync) already wrote something that overlaps with what the current WU is about to write. - By the Stage 4 reconciliation agent for cross-WU sweeps and for eviction decisions. Apply the rules below before every write that could collide with an existing artifact. @@ -32,7 +32,7 @@ Apply the rules below before every write that could collide with an existing art | Definitional contradiction | Same name, substantively different formulas (different aggregation, different filters, different columns) | **Rename + capture**: disambiguate ALL variants with suffix derived from the domain (`churn_risk_engagement_based`, `churn_risk_billing_based`) and write a unified wiki page listing every variant with provenance. The contested name does NOT land in the SL. **Always flag.** | 5. **Eviction (Stage 4 only)**: for each entry in `eviction_list()`: - - Remove the artifact (`sl_delete` for SL sources, `wiki_remove` for wiki pages). + - Remove the artifact (`sl_write_source` or `sl_edit_source` with `delete: true` for SL sources, `wiki_remove` for wiki pages). - Record the removal with `emit_eviction_decision` and `action: "removed"`. ## Why same-ingest vs re-ingest differs diff --git a/packages/context/skills/lookml_ingest/SKILL.md b/packages/context/skills/lookml_ingest/SKILL.md index 5a9c79a3..6a01a355 100644 --- a/packages/context/skills/lookml_ingest/SKILL.md +++ b/packages/context/skills/lookml_ingest/SKILL.md @@ -84,7 +84,7 @@ SL source, `tables:` frontmatter, `sl_refs`, or `emit_unmapped_fallback`: **Required flow before writing any overlay or standalone**: -1. Call `sl_discover()` for each base table you're about to touch. That returns the real columns. +1. Call `sl_discover({ query: "" })` for each base table you're about to touch. That returns the real columns. 2. If the table isn't in the manifest, use the warehouse `connectionName` returned by `discover_data` or the target connection chosen from `sl_discover`, then call a dialect-appropriate SQL probe with that diff --git a/packages/context/skills/notion_synthesize/SKILL.md b/packages/context/skills/notion_synthesize/SKILL.md index 524c6832..1b5417e3 100644 --- a/packages/context/skills/notion_synthesize/SKILL.md +++ b/packages/context/skills/notion_synthesize/SKILL.md @@ -20,7 +20,7 @@ Each WorkUnit is either a single Notion page/span or a topical cluster of relate 4. Use `context_evidence_search`, `context_evidence_read`, and `context_evidence_neighbors` to pull supporting chunks when indexed evidence is relevant. Pass `chunkId` and `documentId` values verbatim as returned by the evidence tools. 5. Write durable business knowledge with `wiki_write`. Aim for a small number of high-quality pages per WorkUnit or cluster. Include `rawPaths` with the exact Notion raw files that support each page. 6. When the Notion content defines a reusable dataset, metric, segment, join rule, source-of-truth mapping, or table with explicit columns, load `sl_capture`, discover existing sources first with `sl_discover` or `sl_read_source`, then use `sl_write_source` or `sl_edit_source` only for a confirmed mapped non-Notion target source. Include `rawPaths` with the exact Notion raw files that support the SL action. If no mapped target exists, call `emit_unmapped_fallback` and keep the content wiki-only. -7. For every deleted raw path in the Eviction Set, call `eviction_list`, decide retention, then `context_eviction_decision_write`. Do this even when no wiki write is needed. +7. For every deleted raw path in the Eviction Set, call `eviction_list`, decide retention, then `emit_eviction_decision`. Do this even when no wiki write is needed. ## What To Capture @@ -99,6 +99,6 @@ SL source, `tables:` frontmatter, `sl_refs`, or `emit_unmapped_fallback`: ## Tools -Allowed: `read_raw_file`, `read_raw_span`, `wiki_search`, `wiki_read`, `wiki_write`, `discover_data`, `entity_details`, `sql_execution`, `sl_discover`, `sl_read_source`, `sl_write_source`, `sl_edit_source`, `sl_validate`, `context_evidence_search`, `context_evidence_read`, `context_evidence_neighbors`, `emit_unmapped_fallback`, `eviction_list`, `context_eviction_decision_write`. +Allowed: `read_raw_file`, `read_raw_span`, `wiki_search`, `wiki_read`, `wiki_write`, `discover_data`, `entity_details`, `sql_execution`, `sl_discover`, `sl_read_source`, `sl_write_source`, `sl_edit_source`, `sl_validate`, `context_evidence_search`, `context_evidence_read`, `context_evidence_neighbors`, `emit_unmapped_fallback`, `eviction_list`, `emit_eviction_decision`. Not allowed: `context_candidate_write`, `context_candidate_mark`. diff --git a/packages/context/skills/sl/SKILL.md b/packages/context/skills/sl/SKILL.md index f7077c33..7103a276 100644 --- a/packages/context/skills/sl/SKILL.md +++ b/packages/context/skills/sl/SKILL.md @@ -1,6 +1,6 @@ --- name: sl -description: KTX's semantic layer — a structured catalog of sources (tables/views), measures, joins, and segments expressed as YAML. Covers the schema and how to query it via `semantic_query`. Use when the task involves querying pre-defined metrics (ARR, churn, retention, LTV, MAU) or reading SL source YAML to understand the catalog. Capture is handled by the `sl_capture` skill (memory-agent only). +description: KTX's semantic layer — a structured catalog of sources (tables/views), measures, joins, and segments expressed as YAML. Covers the schema and how to query it via `sl_query`. Use when the task involves querying pre-defined metrics (ARR, churn, retention, LTV, MAU) or reading SL source YAML to understand the catalog. Capture is handled by the `sl_capture` skill (memory-agent only). --- # Semantic Layer @@ -9,7 +9,7 @@ KTX's semantic layer (SL) is a structured catalog. Each **source** represents a This skill covers two parts: - **Part 1** — Schema reference (what an SL source looks like). -- **Part 2** — Querying via `semantic_query`. +- **Part 2** — Querying via `sl_query`. Capture (when and how to add new patterns to the SL) is a separate concern handled by the memory-agent — see the `sl_capture` skill if you are running in capture mode. The research agent **reads** and **queries** the SL via the tools described here; it does not write to it. @@ -162,7 +162,7 @@ segments: description: Orders that were paid and not refunded ``` -Named, reusable boolean predicates scoped to one source. Reference by bare name in a measure's `segments: []`, or by dotted form `source.segment_name` in a `semantic_query`. Segments are predicates only — they are NOT selectable as dimensions. If you need to group by the predicate, add a `columns[]` entry instead. +Named, reusable boolean predicates scoped to one source. Reference by bare name in a measure's `segments: []`, or by dotted form `source.segment_name` in an `sl_query`. Segments are predicates only — they are NOT selectable as dimensions. If you need to group by the predicate, add a `columns[]` entry instead. ### Cross-references with the wiki @@ -170,11 +170,11 @@ The reverse edge (wiki pages that cite this source) is derived automatically fro --- -## Part 2 — Querying via `semantic_query` +## Part 2 — Querying via `sl_query` -The `semantic_query` tool generates correct SQL from a structured query. It handles joins, fan-out prevention, aggregation correctness, and filter classification automatically. Prefer it over writing raw SQL whenever the SL has the relevant sources. +The `sl_query` tool generates correct SQL from a structured query. It handles joins, fan-out prevention, aggregation correctness, and filter classification automatically. Prefer it over writing raw SQL whenever the SL has the relevant sources. -### When to prefer semantic_query over raw SQL +### When to prefer sl_query over raw SQL - A pre-defined measure already exists (`source.measure_name` appears in the catalog). - The question combines fields from multiple sources — the engine resolves the join path automatically. @@ -189,15 +189,12 @@ Use raw SQL (`sql_execution`) only when: ```json { "connectionId": "uuid-of-the-connection", - "reasoning": "Brief note on what this query analyzes", - "query": { - "measures": ["orders.total_revenue", "sum(orders.amount)"], - "dimensions": ["customers.segment", { "field": "orders.created_at", "granularity": "month" }], - "filters": ["orders.status != 'cancelled'", "orders.total_revenue > 10000"], - "segments": ["orders.paid_non_refunded"], - "order_by": [{ "field": "orders.created_at", "direction": "desc" }], - "limit": 1000 - } + "measures": ["orders.total_revenue", "sum(orders.amount)"], + "dimensions": ["customers.segment", { "field": "orders.created_at", "granularity": "month" }], + "filters": ["orders.status != 'cancelled'", "orders.total_revenue > 10000"], + "segments": ["orders.paid_non_refunded"], + "order_by": [{ "field": "orders.created_at", "direction": "desc" }], + "limit": 1000 } ``` diff --git a/packages/context/skills/sl_capture/SKILL.md b/packages/context/skills/sl_capture/SKILL.md index a40111ea..4ec21545 100644 --- a/packages/context/skills/sl_capture/SKILL.md +++ b/packages/context/skills/sl_capture/SKILL.md @@ -63,7 +63,7 @@ Preferred: - name: total_revenue expr: sum(amount) ``` -Callers filter `region = 'US'` at `semantic_query` time. +Callers filter `region = 'US'` at query time. **Bake constants in only when the filter has named business meaning that won't change** (`enterprise_arr` for a contractually defined tier), cannot be expressed via the source's dimensions, or comes from a regulated/fixed list. @@ -100,7 +100,7 @@ measures: **Extract repeated filter bundles into named segments.** If the same predicate appears on multiple measures of the same source, lift it to a `segments[]` entry and have each measure reference it. One edit updates every measure that depends on it. -**Never write a standalone file on a manifest-backed name.** If `sl_discover({ tableName })` finds an existing schema for that name, you MUST write an overlay (`name:` + `measures:`/`segments:`/`descriptions:` only — no `sql:`, `table:`, `grain:`, `columns:`, `joins:`). A standalone with `sql:` or `table:` on a manifest-backed name clobbers the inherited columns and joins; `sl_write_source` and `sl_validate` both reject this shape with a clear fix hint. Always run `sl_discover` before your first write on any existing name. +**Never write a standalone file on a manifest-backed name.** If `sl_discover({ query: "" })` finds an existing schema for that name, you MUST write an overlay (`name:` + `measures:`/`segments:`/`descriptions:` only — no `sql:`, `table:`, `grain:`, `columns:`, `joins:`). A standalone with `sql:` or `table:` on a manifest-backed name clobbers the inherited columns and joins; `sl_write_source` and `sl_validate` both reject this shape with a clear fix hint. Always run `sl_discover` before your first write on any existing name. **Prefer overlay decomposition over standalone SQL sources.** Before reaching for `source_type: sql`, check whether the metric decomposes into measures on existing overlays (including cross-source derived measures). Use `source_type: sql` only when: - The metric requires per-user/per-entity derivation that cannot be expressed as a single `expr` (e.g., `EXISTS` over a time-windowed subset), OR @@ -209,10 +209,10 @@ SL source, `tables:` frontmatter, `sl_refs`, or `emit_unmapped_fallback`: ## Tool sequence 1. `sl_discover` — see what source files exist. -2. `sl_discover({ tableName })` — **REQUIRED before the first write on any name**. Shows columns/joins/grain from the manifest. If the call returns a schema, you MUST write an overlay, not a standalone. Skipping this is the #1 cause of accidentally shadowing the manifest. -3. `sl_read_source({ sourceName })` — read the raw YAML before editing. -4. For modifications: `sl_edit_source({ sourceName, old_string, new_string })` with exact-string replacements. `old_string` must match exactly and be unique in the file. -5. For new sources or full rewrites: `sl_write_source({ sourceName, content })` with the full YAML content. +2. `sl_discover({ query: "" })` — **REQUIRED before the first write on any name**. Shows columns/joins/grain from the manifest. If the call returns a schema, you MUST write an overlay, not a standalone. Skipping this is the #1 cause of accidentally shadowing the manifest. +3. `sl_read_source({ connectionId, sourceName })` — read the raw YAML before editing. +4. For modifications: `sl_edit_source({ connectionId, sourceName, yaml_edits: [{ oldText, newText, reason }] })` with exact-string replacements. `oldText` must match exactly and be unique in the file. +5. For new sources or full rewrites: `sl_write_source({ connectionId, sourceName, source })` with the full structured source definition. 6. For join discovery: use `sql_execution({connectionName: "warehouse", sql: "SELECT count(*) FROM public.orders o JOIN public.customers c ON c.id = o.customer_id LIMIT 20"})` with the target warehouse connection name and dialect-correct table names to verify the join key exists in both tables and assess cardinality before declaring the join. 7. Cross-reference knowledge: author the edge once on the **wiki** side via `sl_refs: [source_name]` in the page's front-matter. The reverse edge (wiki pages that cite an SL source) is derived automatically by the reconciler — do not add a `knowledge_refs:` field to SL YAMLs. 8. `sl_validate` — run after writing or editing to surface schema issues, duplicate measure names, and cross-source validation errors. Read-only; the writes are already committed (the squash-at-end flow will collapse them into one commit). @@ -235,13 +235,21 @@ Existing index: `orders [measures=0, joins=0] — candidate for enrichment`. ``` sl_discover() → orders.yaml does not exist yet -sl_discover({ tableName: "orders" }) +sl_discover({ query: "orders" }) → see grain, columns, no current overlay sl_write_source({ + connectionId: "warehouse", sourceName: "orders", - content: "name: orders\nmeasures:\n - name: avg_order_value\n expr: avg(amount)\n description: Mean order transaction amount — filter by product_category at query time\n" + source: { + name: "orders", + measures: [{ + name: "avg_order_value", + expr: "avg(amount)", + description: "Mean order transaction amount - filter by product_category at query time" + }] + } }) -sl_validate() +sl_validate({ connectionId: "warehouse" }) → clean ``` @@ -258,16 +266,17 @@ Current user: "Wait, by 'active' I mean users who have placed an order in the la The existing `users.active_count` measure is wrong by the new definition. ``` -sl_read_source({ sourceName: "users" }) +sl_read_source({ connectionId: "warehouse", sourceName: "users" }) → see the wrong measure sl_edit_source({ + connectionId: "warehouse", sourceName: "users", yaml_edits: [{ oldText: " - name: active_count\n expr: \"count(*)\"\n filter: \"last_login_at > now() - interval '30 days'\"\n description: Users who logged in within the last 30 days", newText: " - name: active_count\n expr: \"count(distinct case when last_order_at > now() - interval '30 days' then user_id end)\"\n description: Users with at least one order in the last 30 days" }] }) -sl_validate() +sl_validate({ connectionId: "warehouse" }) ``` If you only added a new measure, the old incorrect `active_count` would stay and future queries would keep answering the wrong question. @@ -277,7 +286,7 @@ If you only added a new measure, the old incorrect `active_count` would stay and Prior turn: user asked to correlate LTV with protocol count; assistant joined `fct_orders` with `fct_mau_multiprotocol` on `admin_user_id` in raw SQL. ``` -sl_read_source({ sourceName: "fct_orders" }) +sl_read_source({ connectionId: "warehouse", sourceName: "fct_orders" }) → no joins section yet sql_execution({ connectionName: "warehouse", @@ -285,13 +294,14 @@ sql_execution({ }) → confirms cardinality (many orders per MAU row = many_to_one) sl_edit_source({ + connectionId: "warehouse", sourceName: "fct_orders", yaml_edits: [{ oldText: "measures:", newText: "joins:\n - to: fct_mau_multiprotocol\n on: admin_user_id = fct_mau_multiprotocol.admin_user_id\n relationship: many_to_one\nmeasures:" }] }) -sl_validate() +sl_validate({ connectionId: "warehouse" }) ``` Always verify joins with `sql_execution` before adding them. diff --git a/packages/context/skills/wiki_capture/SKILL.md b/packages/context/skills/wiki_capture/SKILL.md index 30188be6..d57a39ad 100644 --- a/packages/context/skills/wiki_capture/SKILL.md +++ b/packages/context/skills/wiki_capture/SKILL.md @@ -31,7 +31,7 @@ Do NOT capture: - Temporary instructions scoped to the current chat. - Ad-hoc formatting preferences. - Information already present in the semantic layer (column names, join paths, measure formulas — those belong in SL). -- **Query results, snapshots, or time-bounded benchmark tables.** Numbers go stale; pasting "Oct 2025: 25%, Nov 2025: 19.9%, …" creates misinformation as soon as new data lands. Reference the SL source by name (`sl_refs`) and let future queries pull live data — the wiki captures the *rule* (definition, exclusion, segmentation), the SL source captures the *measure*, and `semantic_query` captures the *current values*. +- **Query results, snapshots, or time-bounded benchmark tables.** Numbers go stale; pasting "Oct 2025: 25%, Nov 2025: 19.9%, …" creates misinformation as soon as new data lands. Reference the SL source by name (`sl_refs`) and let future query tools pull live data — the wiki captures the *rule* (definition, exclusion, segmentation), the SL source captures the *measure*, and query execution captures the *current values*. - **Interpretive narrative tied to a specific snapshot** ("M1 retention degraded sharply from Dec 2025"). The observation is anchored to data that will move; the actionable convention (e.g., "always exclude in-progress cohorts") may be worth capturing on its own, but the snapshot-specific commentary is not. If nothing is worth capturing, respond without calling any tool. @@ -136,7 +136,7 @@ wiki_search({ query: "refund revenue paid orders" }) → returns `revenue-definition` (related — defines paid-orders filter) sl_discover({ query: "refund rate" }) → returns fct_orders (score 0.08), fct_gaap_revenue (0.06) -sl_read_source({ sourceName: "fct_orders" }) +sl_read_source({ connectionId: "warehouse", sourceName: "fct_orders" }) → confirms amount_refunded_dollars and transaction_amount_dollars exist wiki_write({ key: "refund-rate-definition", diff --git a/packages/context/src/agent/agent-runner.service.test.ts b/packages/context/src/agent/agent-runner.service.test.ts index 70b4e0da..8f405841 100644 --- a/packages/context/src/agent/agent-runner.service.test.ts +++ b/packages/context/src/agent/agent-runner.service.test.ts @@ -40,6 +40,8 @@ describe('AgentRunnerService.runLoop', () => { it('passes systemPrompt, userPrompt, tools, and step budget through to generateText', async () => { (generateText as any).mockResolvedValue({ text: 'ok', toolCalls: [], steps: [] }); + const repairHandler = vi.fn(); + llmProvider.repairToolCallHandler.mockReturnValueOnce(repairHandler); const tools = { noop: { description: 'noop', inputSchema: {}, execute: vi.fn() } }; await runner.runLoop({ modelRole: 'candidateExtraction', @@ -59,7 +61,9 @@ describe('AgentRunnerService.runLoop', () => { expect(call.tools).toEqual(tools); expect(call.stopWhen).toBe(17); expect(call.temperature).toBe(0); + expect(call.experimental_repairToolCall).toBe(repairHandler); expect(llmProvider.getModel).toHaveBeenCalledWith('candidateExtraction'); + expect(llmProvider.repairToolCallHandler).toHaveBeenCalledWith({ source: 'ktx-agent-runner' }); }); it('returns stopReason=natural when the loop completes without error', async () => { diff --git a/packages/context/src/agent/agent-runner.service.ts b/packages/context/src/agent/agent-runner.service.ts index c394fd75..92daad8f 100644 --- a/packages/context/src/agent/agent-runner.service.ts +++ b/packages/context/src/agent/agent-runner.service.ts @@ -73,6 +73,9 @@ export class AgentRunnerService { temperature: 0, stopWhen: stepCountIs(params.stepBudget), experimental_telemetry: this.deps.telemetry?.createTelemetry(params.telemetryTags), + experimental_repairToolCall: this.deps.llmProvider.repairToolCallHandler({ + source: params.telemetryTags.operationName ?? 'ktx-agent-runner', + }), messages: built.messages, tools: built.tools as Record, onStepFinish: async () => { diff --git a/packages/context/src/ingest/ingest-bundle.runner.test.ts b/packages/context/src/ingest/ingest-bundle.runner.test.ts index b9831c0f..bc25308f 100644 --- a/packages/context/src/ingest/ingest-bundle.runner.test.ts +++ b/packages/context/src/ingest/ingest-bundle.runner.test.ts @@ -695,7 +695,8 @@ describe('IngestBundleRunner — Stages 1 → 7', () => { await params.toolSet.emit_unmapped_fallback.execute( { rawPath: 'a.yml', - reason: 'semantic_not_representable', + reason: 'parse_error', + clarification: 'semantic_not_representable', fallback: 'flagged', }, { toolCallId: 'fallback-1', messages: [] }, @@ -954,6 +955,7 @@ describe('IngestBundleRunner — Stages 1 → 7', () => { { rawPath: 'a.yml', reason: 'conversion_metric_unsupported', + detail: expect.stringContaining('conversion metric'), fallback: 'flagged', }, ], @@ -1006,7 +1008,8 @@ describe('IngestBundleRunner — Stages 1 → 7', () => { await params.toolSet.emit_unmapped_fallback.execute( { rawPath: 'cards/untranslated.json', - reason: 'metabase_sql_untranslated', + reason: 'parse_error', + clarification: 'metabase_sql_untranslated', fallback: 'flagged', }, { toolCallId: 'fallback-1', messages: [] }, @@ -1053,7 +1056,8 @@ describe('IngestBundleRunner — Stages 1 → 7', () => { unmappedFallbacks: [ { rawPath: 'cards/untranslated.json', - reason: 'metabase_sql_untranslated', + reason: 'parse_error', + detail: expect.stringContaining('metabase_sql_untranslated'), fallback: 'flagged', }, ], diff --git a/packages/context/src/ingest/stages/stage-index.types.ts b/packages/context/src/ingest/stages/stage-index.types.ts index 7de26bc8..fa1a652e 100644 --- a/packages/context/src/ingest/stages/stage-index.types.ts +++ b/packages/context/src/ingest/stages/stage-index.types.ts @@ -37,7 +37,9 @@ export type UnmappedFallbackReason = | 'multiple_table_references' | 'unsupported_dialect' | 'parse_error' - | 'missing_target_table'; + | 'missing_target_table' + | 'cumulative_metric_unsupported' + | 'conversion_metric_unsupported'; export interface UnmappedFallbackRecord { rawPath: string; diff --git a/packages/context/src/ingest/tools/emit-reconciliation-records.tool.test.ts b/packages/context/src/ingest/tools/emit-reconciliation-records.tool.test.ts index 9178c989..1cd77514 100644 --- a/packages/context/src/ingest/tools/emit-reconciliation-records.tool.test.ts +++ b/packages/context/src/ingest/tools/emit-reconciliation-records.tool.test.ts @@ -182,6 +182,30 @@ describe('reconciliation emit tools', () => { ]); }); + it('records MetricFlow-specific unsupported fallback reasons', async () => { + const stageIndex = makeStageIndex(); + const tool = createEmitUnmappedFallbackTool({ + stageIndex, + allowedPaths: new Set(['metrics/conversion.yml']), + }); + + const output = await executeTool(tool, { + rawPath: 'metrics/conversion.yml', + reason: 'conversion_metric_unsupported', + fallback: 'flagged', + }); + + expect(output).toContain('conversion metric'); + expect(stageIndex.unmappedFallbacks).toEqual([ + { + rawPath: 'metrics/conversion.yml', + reason: 'conversion_metric_unsupported', + detail: expect.stringContaining('conversion metric'), + fallback: 'flagged', + }, + ]); + }); + it('rejects unmapped fallback decisions for raw paths outside the allowed set', async () => { const stageIndex = makeStageIndex(); const tool = createEmitUnmappedFallbackTool({ diff --git a/packages/context/src/ingest/tools/emit-unmapped-fallback.tool.ts b/packages/context/src/ingest/tools/emit-unmapped-fallback.tool.ts index 33a8610e..cdb1a483 100644 --- a/packages/context/src/ingest/tools/emit-unmapped-fallback.tool.ts +++ b/packages/context/src/ingest/tools/emit-unmapped-fallback.tool.ts @@ -17,6 +17,8 @@ const unmappedFallbackReasonSchema = z.enum([ 'unsupported_dialect', 'parse_error', 'missing_target_table', + 'cumulative_metric_unsupported', + 'conversion_metric_unsupported', ]); function sameUnmappedFallback(left: UnmappedFallbackRecord, right: UnmappedFallbackRecord): boolean { @@ -47,6 +49,10 @@ function canonicalDetail(reason: UnmappedFallbackReason, tableRef: string | unde return `${tableClause} uses a SQL dialect that is not yet supported.`; case 'parse_error': return `${tableClause} could not be parsed.`; + case 'cumulative_metric_unsupported': + return `${tableClause} is a cumulative metric, which is not yet supported as a first-class semantic-layer primitive.`; + case 'conversion_metric_unsupported': + return `${tableClause} is a conversion metric, which is not yet supported as a first-class semantic-layer primitive.`; } } diff --git a/packages/context/src/ingest/tools/eviction-list.tool.test.ts b/packages/context/src/ingest/tools/eviction-list.tool.test.ts index 1bd1d82a..96fb7f65 100644 --- a/packages/context/src/ingest/tools/eviction-list.tool.test.ts +++ b/packages/context/src/ingest/tools/eviction-list.tool.test.ts @@ -51,6 +51,6 @@ describe('eviction_list tool', () => { deletedRawPaths: [], }); - expect(tool.description).toContain('context_eviction_decision_write'); + expect(tool.description).toContain('emit_eviction_decision'); }); }); diff --git a/packages/context/src/ingest/tools/eviction-list.tool.ts b/packages/context/src/ingest/tools/eviction-list.tool.ts index 4ed08d63..6a34d48f 100644 --- a/packages/context/src/ingest/tools/eviction-list.tool.ts +++ b/packages/context/src/ingest/tools/eviction-list.tool.ts @@ -12,7 +12,7 @@ export interface EvictionListDeps { export function createEvictionListTool(deps: EvictionListDeps) { return tool({ description: - 'List every artifact that the most recent completed sync produced from a now-deleted raw file. Remove each listed artifact and record the decision with context_eviction_decision_write so the ingest report lists every deleted-source decision.', + 'List every artifact that the most recent completed sync produced from a now-deleted raw file. Remove each listed artifact and record the decision with emit_eviction_decision so the ingest report lists every deleted-source decision.', inputSchema: z.object({}), execute: async () => { if (deps.deletedRawPaths.length === 0) { diff --git a/packages/context/src/ingest/tools/verification-ledger.tool.ts b/packages/context/src/ingest/tools/verification-ledger.tool.ts index ac880607..af27b58d 100644 --- a/packages/context/src/ingest/tools/verification-ledger.tool.ts +++ b/packages/context/src/ingest/tools/verification-ledger.tool.ts @@ -28,7 +28,7 @@ const WRITE_TOOL_NAMES = new Set([ ]); export const VERIFICATION_LEDGER_PROMPT = ` -Before any write-capable tool call (wiki_write, wiki_remove, sl_write_source, sl_edit_source, emit_unmapped_fallback), call record_verification_ledger. +Before any durable wiki, semantic-layer, or unmapped-fallback write (wiki_write, wiki_remove, sl_write_source, sl_edit_source, emit_unmapped_fallback), call record_verification_ledger. The ledger is a model-authored checkpoint, not a deterministic parser gate. Summarize the verification protocol from the loaded skill, list identifiers verified with discover_data/entity_details/sql_execution, and list anything intentionally left unverified. If the write contains no warehouse identifiers, say that explicitly. If a write tool returns verification_ledger_required, complete the ledger and retry the write. `; diff --git a/packages/context/src/llm/generation.ts b/packages/context/src/llm/generation.ts index a7afd5b9..1bbdbcab 100644 --- a/packages/context/src/llm/generation.ts +++ b/packages/context/src/llm/generation.ts @@ -4,6 +4,10 @@ import { generateText, Output, type FlexibleSchema, type ToolSet } from 'ai'; type GenerateTextInput = Parameters[0]; type GenerateTextFn = (input: GenerateTextInput) => Promise<{ text?: string; output?: unknown }>; +function hasTools(tools: ToolSet): boolean { + return Object.keys(tools).length > 0; +} + interface GenerateKtxTextInput { llmProvider: KtxLlmProvider; role: KtxModelRole; @@ -30,6 +34,13 @@ export async function generateKtxText(input: GenerateKtxTextInput): Promise( temperature: input.temperature ?? 0, messages: built.messages, tools: built.tools as ToolSet, + ...(hasTools(built.tools as ToolSet) + ? { + experimental_repairToolCall: input.llmProvider.repairToolCallHandler({ + source: `ktx-${input.role}`, + }), + } + : {}), output: Output.object({ schema: input.schema as FlexibleSchema, }), diff --git a/packages/context/src/sl/tools/sl-discover.tool.ts b/packages/context/src/sl/tools/sl-discover.tool.ts index 97426b40..74570c2e 100644 --- a/packages/context/src/sl/tools/sl-discover.tool.ts +++ b/packages/context/src/sl/tools/sl-discover.tool.ts @@ -53,7 +53,7 @@ export class SlDiscoverTool extends BaseSemanticLayerTool Discover available semantic layer sources, columns, measures, and joins. When called without a connectionId, discovers sources across ALL data sources — grouped by data source name and ID. -Use this to understand what data is available before writing a semantic_query. +Use this to understand what data is available before querying through the semantic layer. diff --git a/packages/context/src/sl/tools/sl-read-source.tool.ts b/packages/context/src/sl/tools/sl-read-source.tool.ts index fb5de830..e4602f31 100644 --- a/packages/context/src/sl/tools/sl-read-source.tool.ts +++ b/packages/context/src/sl/tools/sl-read-source.tool.ts @@ -36,7 +36,7 @@ Use this when you need to understand how a source is built — e.g., before edit - To discover what sources/measures/dimensions are available for querying — use sl_discover instead -- To query data — use semantic_query or create_widget with slQuery +- To query data — use the semantic-layer query surface (\`sl_query\` in MCP) `; }