docs(skills): sync with streamed results; gate releases on skill freshness - #311
Conversation
The core skill still told agents that hotdata query prints the complete result in every format. Since #300, csv and json stream the persisted result while -o table fetches a 10,000-row window, prints an INCOMPLETE PREVIEW footer, and exits 3. Both skills now say so, and the analytics skill lists exit code 3 alongside 0/1/2. Also: wide decimals print at full precision and list/struct cells print on one line in -o json (#299); databases queries and results accept -d/--database and -w/--workspace-id, which both skills denied; two duplicated phrases in the geospatial skill's discovery notes.
scripts/check-skills.sh runs at the start of release.sh prepare and again before finish pushes the tag. It fails when src/ changed since the last v* tag but skills/ did not (SKIP_SKILL_DRIFT=1 to override a verified skill-neutral release), when the built binary exposes a subcommand no skill names, and on finish when a SKILL.md version disagrees with Cargo.toml. The coverage check found two commands documented only under an alias: databases show and databases query status now appear in the core skill.
eddietejeda
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| # --- 2. coverage ------------------------------------------------------------ | ||
| if [ ! -x "$BIN" ]; then | ||
| echo "→ skills: building $BIN for the command inventory..." | ||
| cargo build -q |
There was a problem hiding this comment.
nit: the script aborts with no message when the build does not produce $BIN (not blocking).
Failure scenario: a maintainer has CARGO_TARGET_DIR set in the environment. cargo build -q then writes the binary elsewhere, so target/debug/hotdata stays absent. The first "$BIN" --help fails with 127, 2>/dev/null hides the "No such file or directory" message, and pipefail plus set -e exit the script at the assignment in walk. The maintainer sees the "building ..." line and nothing else, and release.sh stops.
Re-check $BIN after the build and report the path explicitly:
| cargo build -q | |
| cargo build -q | |
| fi | |
| if [ ! -x "$BIN" ]; then | |
| echo "error: $BIN not found after cargo build; set HOTDATA_BIN to the binary path." >&2 | |
| exit 1 |
| printf ' hotdata %s\n' "${missing[@]}" >&2 | ||
| fail=1 | ||
| else | ||
| echo "→ skills: coverage ok (${#leaves[@]} commands documented)" |
There was a problem hiding this comment.
nit: an empty command inventory reports success (not blocking).
When leaves is empty the loop above never runs, missing stays empty, and line 91 prints → skills: coverage ok (0 commands documented). The gate then passes while checking nothing. This happens whenever the root --help parse yields no names, for example after a clap upgrade renames the Commands: header. Treat an empty inventory as a failure:
| echo "→ skills: coverage ok (${#leaves[@]} commands documented)" | |
| if [ ${#leaves[@]} -eq 0 ]; then | |
| echo "error: no subcommands parsed from $BIN --help; the coverage check verified nothing." >&2 | |
| fail=1 | |
| else | |
| echo "→ skills: coverage ok (${#leaves[@]} commands documented)" | |
| fi |
| walk() { | ||
| local path=("$@") | ||
| local subs | ||
| subs="$(list_subcommands "${path[@]}")" |
There was a problem hiding this comment.
nit: this expansion requires bash 4.4 or newer (not blocking).
The first walk call passes no arguments, so path is an empty array. Bash 3.2 treats "${path[@]}" as unbound under set -u and exits with path[@]: unbound variable. macOS ships bash 3.2 as /bin/bash, and release.sh otherwise stays 3.2-compatible. Fix this if a maintainer releases from macOS system bash:
| subs="$(list_subcommands "${path[@]}")" | |
| subs="$(list_subcommands ${path[@]+"${path[@]}"})" |
The command-coverage check let a flag ship undocumented; it now also requires every long flag in a leaf command's --help to be named in a skill. That found eight: the Kafka/Delta/continuous shorthands on ingest create (--topic, --table-path, --stream, --dest-table-prefix), --include-deleted on both ingest listings, --ingest-id on ingest logs, and --message on support report. All are documented now. CI runs the gate on pull requests with the drift check skipped, so a new command or flag cannot merge without its skill text; the drift check stays in release.sh where it judges a whole release.
| # Walk the clap tree: "<group> <sub> [<sub>]", leaf commands only. | ||
| list_subcommands() { | ||
| "$BIN" "$@" --help 2>/dev/null \ | ||
| | awk '/^Commands:/{f=1;next} /^$/{f=0} f && $1!="help" {print $1}' |
There was a problem hiding this comment.
nit: the awk parse reads a wrapped description line as a subcommand name (not blocking).
Clap wraps the Commands: descriptions at 100 columns when stdout is not a TTY. The continuation line is indented to the description column, so f && $1!="help" prints its first word as a subcommand. walk then recurses into a name the binary does not have, --help fails, and the word becomes a leaf. The coverage check demands hotdata <word> in the skills, which no maintainer can satisfy.
The margin is already thin. In the root help the description column starts at 14, and the search description is 83 characters, for 97 of the 100 available. One added word to that line trips the parse, and the CI job now blocks every PR on it.
Anchor the match to exactly two leading spaces, which a continuation line never has:
| | awk '/^Commands:/{f=1;next} /^$/{f=0} f && $1!="help" {print $1}' | |
| | awk '/^Commands:/{f=1;next} /^$/{f=0} f && /^ [^ ]/ && $1!="help" {print $1}' |
| | grep -oE -- '--[a-z][a-z0-9-]+' | sort -u | grep -vE "$GLOBAL_FLAGS" || true)" | ||
| for f in $flags; do | ||
| flag_count=$((flag_count + 1)) | ||
| grep -qF -- "$f" <<<"$skill_text" || missing_flags+=("hotdata $leaf $f") |
There was a problem hiding this comment.
nit: grep -F matches a prefix, so a flag passes while undocumented (not blocking).
-F is a fixed-substring match with no word boundary. Any flag that is a prefix of a longer documented flag is treated as covered. This PR adds --table-path at skills/hotdata/SKILL.md:375, which now silences --table, --table-p, and every other prefix of it across all commands.
Require a boundary after the flag name:
| grep -qF -- "$f" <<<"$skill_text" || missing_flags+=("hotdata $leaf $f") | |
| grep -qE -- "$f([^a-z0-9-]|\$)" <<<"$skill_text" || missing_flags+=("hotdata $leaf $f") |
1 participant
Skills
hotdata queryalways prints the complete result. Since fix(query)!: stream results instead of buffering them #300,csv/jsonstream the persisted result while-o tablefetches a 10,000-row window, prints anINCOMPLETE PREVIEWfooter, and exits 3. Both the core and analytics skills now say so.query statusexit code 3 alongside 0/1/2.-o json(fix(query): print wide decimals at full precision #299).databases queriesanddatabases resultsaccept-d/--databaseand-w/--workspace-id; both skills denied this.databases showanddatabases query statuswere documented only under their alias forms.Release gate
scripts/check-skills.shruns at the start ofrelease.sh prepareand again beforefinishpushes the tag:src//README.mdchanged since the lastv*tag butskills/did not → fail, listing the commits.SKIP_SKILL_DRIFT=1overrides a verified skill-neutral release.hotdata <group> <sub>inskills/**/*.md.finishonly) — everySKILL.mdversion:must equal Cargo.toml.Documented in
docs/RELEASING.md.