Skip to content

Commit 9cda77c

Browse files
committed
refactor(cli)!: reorganize command surface
Collapse the top level from ~19 groups to 8 (auth, query, workspaces, databases, jobs, ingest, search, manage) with a single canonical name for every verb. - Rename verbs: set→use, delete→remove, cancel→pause, runs→logs, validate→test, create→add (sources), show→get (results). - Fold `datasource` under `ingest sources`; absorb query/queries/results/ context/tables under `databases`; group usage/completions/upgrade/skills under `manage`. - Reshape search: `indexes` + the flag-based search + `embedding-providers` become `search create|list|show|remove` + `search embeddings`, and the action is `search "<text>" --index <name>` (index addressed by name via a new `indexes::locate_by_name` resolver). - Keep a top-level `query` shortcut into `databases query`. - Remove `databases run`; delete the now-dead search-inference and index-argument code and their tests. BREAKING CHANGE: old command spellings are removed (no back-compat aliases). Unit and integration tests updated for the new surface.
1 parent 72828b5 commit 9cda77c

26 files changed

Lines changed: 1106 additions & 1249 deletions

‎REORG.md‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
# CLI reorganization — mapping existing features onto the proposed tree
2+
3+
## Implemented so far (branch `cli-reorg`, non-breaking, no backend changes)
4+
- **Rename aliases** (old spellings still work): `workspaces use`←set; `databases use`←set, `remove`←delete; `databases tables remove`←delete; `ingest pause`←cancel, `logs`←runs, `remove`←delete; `datasource test`←validate, `add`←create, `remove`←delete; `results get`←show; `embedding-providers show`←get, `add`←create, `remove`←delete.
5+
- **`ingest sources`** — `datasource` command tree surfaced under `hotdata ingest sources …` (reuses the datasource dispatcher; top-level `datasource` kept).
6+
- **`manage`** — `usage` / `completions` / `upgrade` grouped under `hotdata manage …` (top-level kept; update-gate exemptions extended).
7+
- **`databases context`** — top-level `context` surfaced under `hotdata databases context …` (reuses the context handlers; top-level kept).
8+
- **`databases` absorption** — `query`, `queries`, `results` surfaced under `hotdata databases …`, and `tables show` added under `databases tables` (all reuse existing handlers/sub-enums; top-level kept). The `results get` alias flows through the nested path too.
9+
- **`search` reshape** — new `search` namespace: `create --type vector|text`, `list`, `show`, `query`, `remove`, and `providers` (embedding-providers). Indexes are addressed by name via a new `indexes::locate_by_name` resolver (scoped to the active database, else workspace-wide). `search` is dual-mode: the legacy `search "text" --table …` flag form still works (`args_conflicts_with_subcommands`), and top-level `indexes` / `embedding-providers` are untouched. 407 unit tests pass.
10+
11+
- **`ingest create --stream`** — sugar for `--type continuous` (conflicts with `--type`).
12+
13+
**All CLI-only reorg is now on the branch.** Remaining items need backend or are net-new features (out of scope for this reorg): `ingest run` (trigger), `workspaces show`, `databases tag/untag`, `databases tables create`, `manage config/doctor`, and the `models` namespace.
14+
15+
## Backward compat dropped — collapsed to the clean surface
16+
The migration kept both old and new spellings; that's now removed. The surface is 8 top-level groups (`auth`, `workspaces`, `databases`, `skills`, `jobs`, `ingest`, `search`, `manage`):
17+
- **Renames are canonical** (old names gone, not aliased): `use` (was set), `remove` (was delete), `pause` (was cancel), `logs` (was runs), `test` (was validate), `add` (was create, datasource/embeddings), `get` (was show, results).
18+
- **Top-level duplicates removed**: `query`/`queries`/`results`/`context`/`tables` now live only under `databases`; `datasource` only under `ingest sources`; `indexes`/`embedding-providers` only under `search`; `usage`/`completions`/`upgrade` only under `manage`.
19+
- **`search` action reshaped** — the search action is `search "text" --index <name>` (query positional + `--index`, alias `--in`); index management stays as subcommands `create`/`list`/`show`/`remove`/`embeddings`. The redundant `search query` subcommand is gone (it read the verb twice and collided with SQL `query`), as is the old `--table`/`--column` inference.
20+
- **Dead code removed** — the legacy-path orphans are gone: `indexes::infer_for_search`/`resolve_search_params`/`matches_search_column` (+ their tests), `tables::list`/`TableRow`/`full_name`, and the unused `IndexesCommands`/`TablesCommands` enums. Warning-free build; 388 tests pass.
21+
- **`query` kept top-level** as a shortcut (`hotdata query "…"`) alongside `databases query` — the one deliberate ergonomic exception.
22+
- **`skills` moved under `manage`** — `hotdata manage skills install|status|list` (top-level `skills` removed).
23+
- **`databases run` removed** — the scoped-token exec command and its helpers (`run`, `mint_database_token`, `create_and_return_id`, `DatabaseTokenResponse`, + tests) are gone. The `HOTDATA_DATABASE_TOKEN` env-auth path in the client is kept (still usable by an externally-supplied database token); `database_session::save` is now `#[allow(dead_code)]` alongside the existing reserved `load`/`clear`.
24+
25+
## End-to-end validation against production (api.hotdata.dev)
26+
Ran every reachable command against prod in a throwaway database (created + deleted, no existing resources touched). All read/nested/alias/reshape paths passed. One real bug found and fixed:
27+
28+
- **Fixed:** `search query`/`show`/`remove` by name built SQL against the index listing's internal `__db_*` connection label instead of the database's SQL catalog alias, so `bm25_search`/`vector_distance` failed with "catalog '' is not attached". `indexes::locate_by_name` now prefers the active database's `default_catalog`. Verified: bm25 and vector (auto-embed via the system OpenAI provider) searches both return ranked results.
29+
- **Not bugs (auth/business rules the CLI surfaced correctly):** `datasource create` requires a workspace API key (`hd_…`), not a login session — so the datasource→ingest *write* cycle is auth-gated (read/validate paths all pass); an embedding-backed vector index can't coexist with other indexes on a table; `query` is read-only (no DDL/DML).
30+
31+
---
32+
33+
34+
Scope: **relocate/rename features that already ship.** Greenfield (`models`,
35+
`ingest stream`, `workspaces show`, `databases tables create`, `search … show`,
36+
`manage config|doctor`, `ingest … run`) is out of scope here — this is only the
37+
move.
38+
39+
Legend: **rename** = same behavior, new verb · **reparent** = same command, new
40+
home · **reshape** = command model actually changes.
41+
42+
---
43+
44+
## auth (no change)
45+
| today | proposed | kind |
46+
|---|---|---|
47+
| `auth login/logout/status` | same | — |
48+
| `auth register` | *(no slot in proposal — keep)* | — |
49+
50+
## workspaces
51+
| today | proposed | kind |
52+
|---|---|---|
53+
| `workspaces list` | `workspaces list` | — |
54+
| `workspaces set` | `workspaces use` | rename |
55+
56+
## ingest (biggest structural change: flat → `batch` / `stream`)
57+
Today `datasource` and `ingest` are two **top-level** commands. Proposed: one
58+
`ingest` parent, a `batch` child, and `datasource` folded in under
59+
`ingest batch sources`.
60+
61+
| today | proposed | kind |
62+
|---|---|---|
63+
| `datasource create` | `ingest batch sources add` | rename + reparent |
64+
| `datasource list` | `ingest batch sources list` | reparent |
65+
| `datasource show` | `ingest batch sources show` | reparent |
66+
| `datasource validate` | `ingest batch sources test` | rename + reparent |
67+
| `datasource delete` | `ingest batch sources remove` | rename + reparent |
68+
| `datasource update-config` | `ingest batch sources update` *(no proposal slot — keep)* | reparent |
69+
| `datasource types` / `fields` | `ingest batch sources types` / `fields` *(keep)* | reparent |
70+
| `ingest create` | `ingest batch create` | reparent |
71+
| `ingest list` | `ingest batch list` | reparent |
72+
| `ingest show` | `ingest batch show` | reparent |
73+
| `ingest cancel` | `ingest batch pause` | rename + reparent |
74+
| `ingest resume` | `ingest batch resume` | reparent |
75+
| `ingest schedule` | `ingest batch schedule` | reparent |
76+
| `ingest runs` / `ingest run <run>` | `ingest batch logs` | reshape + reparent |
77+
| `ingest delete` | `ingest batch remove` | rename + reparent |
78+
79+
Note: proposed `ingest batch run <ingest>` (trigger-now) has **no equivalent** —
80+
today the scheduler owns dispatch (`schedule --next now` is the workaround). Not
81+
part of the move; it's a new capability.
82+
83+
## databases (absorbs four top-level commands)
84+
| today | proposed | kind |
85+
|---|---|---|
86+
| `databases list/show/create/fork/attach/detach` | same | — |
87+
| `databases set` | `databases use` | rename |
88+
| `databases unset` | `databases use --clear` *(or keep)* | rename |
89+
| `databases delete` | `databases remove` | rename |
90+
| `databases count` | *(no slot — keep)* | — |
91+
| `databases run` | *(no slot — keep; scoped-token power feature)* | — |
92+
| `databases load` | `databases tables load` | reshape (dedupe w/ below) |
93+
| `databases tables list` | `databases tables list` | — |
94+
| `databases tables load` | `databases tables load` | — |
95+
| `databases tables delete` | `databases tables remove` | rename |
96+
| **`tables show`** (top-level) | `databases tables show` | reparent |
97+
| **`tables list`** (top-level) | `databases tables list` | reparent (merge) |
98+
| **`query "<sql>"`** (top-level) | `databases query "<sql>"` | reparent |
99+
| **`queries list` / `queries <id>`** (top-level) | `databases queries list\|show` | reparent |
100+
| **`results list` / `results show`** (top-level) | `databases results list\|get` | rename + reparent |
101+
| **`context list\|show\|pull\|push`** (top-level) | `databases context …` | reparent |
102+
103+
## search (the only non-mechanical reshape: merges 3 commands into an object model)
104+
Today search is three separate things: `indexes` (create/list/delete, typed
105+
vector|bm25), the flag-driven `search` query command, and `embedding-providers`.
106+
Proposed: one `search` parent split into `vector` / `text`, each treating an
107+
index as a named object.
108+
109+
| today | proposed | kind |
110+
|---|---|---|
111+
| `indexes create --type vector …` | `search vector create <name> --from --column` | reshape |
112+
| `indexes create --type bm25 …` | `search text create <name> --from --column` | reshape |
113+
| `indexes list` | `search vector list` + `search text list` | reshape (split by type) |
114+
| `indexes delete` | `search vector remove` / `search text remove` | rename + reshape |
115+
| `search "q" --type vector …` | `search vector query <name> "q"` | reshape |
116+
| `search "q" --type bm25 …` | `search text query <name> "q"` | reshape |
117+
| `embedding-providers list` | `search vector providers list` | reparent |
118+
| `embedding-providers create` | `search vector providers add` | rename + reparent |
119+
| `embedding-providers delete` | `search vector providers remove` | rename + reparent |
120+
| `embedding-providers get` / `update` | `search vector providers show` / *(update kept)* | rename + reparent |
121+
122+
This is the one bucket that's real code, not enum-shuffling: it introduces
123+
"index as a named, listable, show-able object" that doesn't exist today.
124+
125+
## manage (regroup three top-level utilities)
126+
| today | proposed | kind |
127+
|---|---|---|
128+
| `usage` | `manage usage` | reparent |
129+
| `completions` | `manage completions` | reparent |
130+
| `upgrade` | `manage upgrade` | reparent |
131+
132+
## unhomed today — need a decision
133+
Not in the proposal at all; pick a parent or leave top-level:
134+
- `jobs` — cross-cutting (ingest runs, index builds). Candidate: `manage jobs`.
135+
- `skills` — agent-skill install. Candidate: `manage skills`.
136+
- `query` (top-level) — see keystroke note below.
137+
138+
---
139+
140+
## What it looks like in clap (illustrative)
141+
142+
```rust
143+
pub enum Commands {
144+
Auth { command: AuthCommands }, // unchanged
145+
Workspaces { command: WorkspaceCommands }, // list, use (was set)
146+
Ingest { command: IngestCommands }, // batch { sources { … }, … }
147+
Databases { command: DatabasesCommands }, // + query/queries/results/context/tables absorbed
148+
Search { command: SearchCommands }, // vector | text (merges indexes + search + embedding-providers)
149+
Manage { command: ManageCommands }, // usage, completions, upgrade (+ jobs/skills?)
150+
// kept without a proposal home: Query (alias), Jobs, Skills …
151+
}
152+
153+
pub enum IngestCommands { Batch(BatchCommands) /*, Stream(…) later */ }
154+
pub enum BatchCommands {
155+
Sources(SourceCommands), // was top-level `datasource`
156+
Create, List, Show, Schedule, Resume,
157+
Pause, // was Cancel
158+
Logs, // was Runs / Run
159+
Remove, // was Delete
160+
}
161+
pub enum SourceCommands { Add, List, Show, Test, Remove /*, Update, Types, Fields kept */ }
162+
163+
pub enum SearchCommands { Vector(VectorCommands), Text(TextCommands) }
164+
pub enum VectorCommands { Create, List, Query, Remove, Providers(ProviderCommands) /*, Show* */ }
165+
```
166+
167+
The mechanical buckets (workspaces/databases/ingest/manage moves) are mostly
168+
moving enum variants between modules and adding a nesting layer. `search` is the
169+
one that needs new command logic.
170+
171+
---
172+
173+
## Two costs worth deciding before committing
174+
175+
1. **`query` gets demoted.** `hotdata query "…"` → `hotdata databases query "…"`.
176+
Query is the hottest path; nesting it taxes every ad-hoc SQL call. Strong
177+
case for keeping a **top-level `query` visible alias** even after the move.
178+
179+
2. **Depth.** `hotdata ingest batch sources add …` is four levels; `search
180+
vector create` is three. More discoverable via `--help`, more to type. Fine
181+
if aliases cover the common cases.
182+
183+
## Suggested phasing (non-breaking → breaking)
184+
- **Phase 1 — additive.** Stand up the new nested paths as canonical; wire the
185+
old spellings as hidden `alias`es. Zero breakage, scripts keep working.
186+
- **Phase 2 — search reshape.** The only real feature work: index-as-object +
187+
`vector`/`text` split. Do this on its own so it's reviewable in isolation.
188+
- **Phase 3 — deprecate.** Warn on old spellings, then drop in a major. (Repo
189+
already shipped a breaking ingest split in #256, so precedent exists.)

0 commit comments

Comments
 (0)