docs(skills): fix CLI drift + consolidate hotdata-search into the core skill - #243
docs(skills): fix CLI drift + consolidate hotdata-search into the core skill#243eddietejeda wants to merge 1 commit into
Conversation
…e skill
Two changes to the bundled agent skills:
1. Drift fixes (audited against `hotdata … --help` on v0.21.0):
- hotdata-geospatial: `tables list --connection-id <id>` (3 occurrences) used a
flag that does not exist and errored on the first discovery step — replaced
with real `tables list` / `tables show` / `databases tables list`.
- hotdata-analytics: dropped the phantom `connections` command from prerequisites.
- hotdata (core): added `managed_load` to `jobs --job-type`; added `--result-id`
to the `databases load` / `tables load` synopses; documented `query status`
exit codes (0/1/2/3).
2. Consolidated `hotdata-search` into the core `hotdata` skill:
- Merged the search / indexes / embedding-providers content into a new
"Search & retrieval indexes" section in `skills/hotdata/SKILL.md`; moved
`references/INDEXES.md` under the core skill.
- Removed `hotdata-search` from `SKILL_NAMES` (src/commands/skill.rs) and the
cargo-release version-bump list (Cargo.toml); updated the core frontmatter,
sub-skill table, decision tree, and all cross-references in the analytics /
geospatial skills and reference docs; deleted `skills/hotdata-search/`.
Tradeoff: the always-loaded core skill grows ~80 lines; search is now discoverable
without loading a separate skill. `cargo build` clean; `hotdata skills list` now
lists 3 skills.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| "hotdata-analytics", | ||
| "hotdata-geospatial", | ||
| ]; | ||
| const SKILL_NAMES: &[&str] = &["hotdata", "hotdata-analytics", "hotdata-geospatial"]; |
There was a problem hiding this comment.
Dropping hotdata-search from SKILL_NAMES stops it from being installed, but nothing ever removes an existing install. download_and_extract_from_url only unpacks what's in the tarball (it never prunes ~/.hotdata/skills/), and ensure_symlinks / install_project / status all iterate SKILL_NAMES, so for every user who installed skills on ≤0.21.0 the upgrade path leaves behind:
~/.hotdata/skills/hotdata-search/(frozen at the old version)~/.agents/skills/hotdata-search→ that store~/.claude/skills/hotdata-search,~/.pi/skills/hotdata-search(and the project-local.agents/skills/hotdata-searchcopies fromskills install --project)
The agent keeps loading a stale skill whose description still says "use this skill for BM25/vector search", now competing with the new core-skill section — exactly the split this PR is trying to remove. hotdata skills list will also report 3 skills while 4 are live on disk.
Suggest an explicit removal list that install/auto-update prunes, e.g.:
/// Skills that shipped in earlier releases and must be removed on upgrade.
const REMOVED_SKILL_NAMES: &[&str] = &["hotdata-search"];…iterated in ensure_symlinks() (and the project path) to remove_file/remove_dir_all the store, ~/.agents/skills/<name>, and each AGENT_ROOTS link, ignoring NotFound.
| **Skill:** **`hotdata-search`** (schema via **`hotdata`**) | ||
| **Skill:** core **`hotdata`** (Search & retrieval indexes) | ||
|
|
||
| 1. [ ] `hotdata tables list --connection-id <id>` — pick text column (BM25) or embedding/text column (vector) |
There was a problem hiding this comment.
Same agent-breaking flag this PR fixes in hotdata-geospatial: hotdata tables list has no --connection-id (see TablesCommands::List in src/commands/tables.rs — only --workspace-id, --schema, --table, --limit, --cursor), so step 1 of the retrieval flow clap-errors. It survives here even though this hunk was edited, plus at skills/hotdata/references/WORKFLOWS.md:42 and :173, skills/hotdata/references/MODEL_BUILD.md:28 and :95, and README.md:125.
| 1. [ ] `hotdata tables list --connection-id <id>` — pick text column (BM25) or embedding/text column (vector) | |
| 1. [ ] `hotdata tables list` (narrow with `--schema`/`--table`), then `hotdata tables show <catalog.schema.table>` — pick text column (BM25) or embedding/text column (vector) |
There was a problem hiding this comment.
Review
Blocking Issues
-
src/commands/skill.rs:24— removinghotdata-searchfromSKILL_NAMESleaves it installed forever. Nothing prunes skills that leave the list:download_and_extract_from_urlonly unpacks tarball entries, andensure_symlinks/install_project/statusall iterateSKILL_NAMES. Every existing user keeps~/.hotdata/skills/hotdata-search/plus the~/.agents,~/.claude,~/.pilinks (and project-local copies), so agents keep loading a stale skill that still claims BM25/vector search — the exact duplication this PR removes.hotdata skills listwill say 3 while 4 are live. Needs an explicit removal list pruned on install/auto-update (details inline). -
skills/hotdata/references/INDEXES.md— the file this PR moves into the core skill still uses the nonexistent--connection-idflag. It is now the reference the new Search & retrieval indexes section links to:- L17
hotdata tables list --connection-id <connection_id>— no such flag (TablesCommands::Listinsrc/commands/tables.rs). - L25 / L28
hotdata indexes list [--connection-id <id>]— also gone;src/commands/indexes.rs:1369assertslist --connection-idfails to parse.
Step 1 of the index workflow errors for any agent that follows it — same agent-breaking class the PR fixes in
hotdata-geospatial. - L17
-
skills/hotdata/references/WORKFLOWS.md:64— same flag in the Retrieval epic flow, inside a hunk this PR edited. Also present atWORKFLOWS.md:42and:173, andskills/hotdata/references/MODEL_BUILD.md:28and:95— all in the always-loaded core skill.
Non-blocking: README.md:125 has the same hotdata tables list --connection-id <id> line.
Everything else checked out against the binary's surface — managed_load (src/commands/jobs.rs:11), --result-id on databases load / databases tables load (src/commands/databases.rs:160-176, :249-265), and the query status exit codes 0/1/2/3 (EXIT_INCOMPLETE_RESULT = 3 at src/commands/query.rs:52, exit(2) at :588, fail_run → exit(1)).
Action Required
- Add pruning for removed skills (e.g.
const REMOVED_SKILL_NAMES: &[&str] = &["hotdata-search"];) and delete the store dir + all agent-root links on install / auto-update / project install. - Replace the remaining
--connection-idoccurrences inINDEXES.md,WORKFLOWS.md, andMODEL_BUILD.mdwithhotdata tables list(+--schema/--table) andhotdata tables show <catalog.schema.table>; drop--connection-idfrom theindexes listsynopsis and the paragraph explaining it.
|
Closing — wrong approach. This merged search into the base skill; the actual goal is to keep search/analytics/geospatial as full sub-skills but stop them registering as separate autocomplete entries (only |
Two related changes to the bundled agent skills, from a full audit of the skill docs against the live
hotdata … --helpsurface (v0.21.0).1. Drift fixes
hotdata tables list --connection-id <id>— a flag that does not exist and errors on the very first discovery step. Replaced with the real surface (tables list,tables show <catalog.schema.table>,databases tables list).connectionscommand from the prerequisites (no such command; connections are reached viadatabases attach).managed_loadtojobs --job-type; added--result-idto thedatabases load/tables loadsynopses; documentedquery statusexit codes (0/1/2/3).The
hotdata,hotdata-search, andhotdata-analyticsdocs were otherwise faithful — command paths, flags,--outputvalue sets, defaults, and exit-code semantics all checked out, including the recently addedingest raw-sql.2. Consolidate
hotdata-searchinto the core skill (per request)skills/hotdata/SKILL.md; movedreferences/INDEXES.mdunder the core skill.hotdata-searchfromSKILL_NAMES(src/commands/skill.rs) and the cargo-release version-bump list (Cargo.toml); updated the core frontmatter (search triggers), the sub-skill table, decision tree, and all cross-references in the analytics / geospatial skills and reference docs; deletedskills/hotdata-search/.hotdata skills listnow lists 3 skills (hotdata,hotdata-analytics,hotdata-geospatial).Tradeoff: the always-loaded core skill grows ~80 lines (search content that most core interactions don't need), in exchange for search being discoverable without loading a separate skill. Easy to revert if you'd rather keep the split.
cargo buildclean; audited with the binary's recursive--help.