Skip to content

feat(skilldoc): generate response shape and field names into every command card - #109

Open
ysyneu wants to merge 6 commits into
fix/incident-comment-shell-safefrom
fix/card-response-shapes
Open

feat(skilldoc): generate response shape and field names into every command card#109
ysyneu wants to merge 6 commits into
fix/incident-comment-shell-safefrom
fix/card-response-shapes

Conversation

@ysyneu

@ysyneu ysyneu commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Stacked on #102 (fix/incident-comment-shell-safe). The card fences are a pure
function of the CLI tree, and both branches change that tree — so the correct content
is one regeneration of the merged tree, not two regenerations text-merged. GitHub
retargets this automatically once #102 merges.

A caller writing a jq filter against a fduty command has no way to learn the
response envelope or the field names from the command cards. The truth exists —
internal/cli/zz_generated_response_help.go documents Schedules.List as a
"TOP-LEVEL array … pipe jq '.[]', NOT .items[]", and escalate-rule-list rows as
status ∈ {enabled, disabled} — but schedule.md, alert.md, incident.md,
insight.md and rum.md carry zero envelope information. Only five cards had it,
hand-written, and hand-written facts drift from the binary.

The practical consequence is silent: a wrong key in a jq filter returns null
rather than an error, so guessing is_enabled when the field is status produces a
plausible, wrong answer instead of a failure.

What changed

internal/skilldoc now derives a one-line response summary per verb from the same
generated help the CLI prints, and injects it into every card's generated fence. Each
line states the envelope — top-level array vs {items: [...]} page wrapper vs single
unwrapped object — with the jq path that actually works, followed by the field
names. Because it is generated, it cannot drift from the CLI: skilldoc check fails
when a fence goes stale, so the cards and the binary move together or CI stops them.

Hand-written prose that the generated lines now cover is deleted rather than left to
disagree with them.

A shape can also be un-derivable, and that case is handled explicitly. When a
response block's sole top-level field is a recognized wrapper name (items, docs,
list — the same three cligen's listEnvelope recognizes) carrying an array, the
generator emits nothing rather than asserting "single object". Claiming a definite
wrong shape is worse than staying silent: silence costs one --help call, a confident
lie costs a wrong jq path and a silent null.

Card corrections in the same pass

schedule.md — the "all N schedules have no on-call" trap. schedule list without
--start/--end omits computed shifts, so cur_oncall is absent by construction.
The card documented the omission under Gotchas but never connected it to cur_oncall,
while the generated help lists cur_oncall as always present — so an empty value reads
as "nobody is on call" rather than "not computed". The card now states the mechanism
and requires per-schedule verification before any coverage claim.

schedule.md — stop recommending the account-wide dump. oncall who was endorsed
as "fine for a quick global snapshot"; on a real account it returns 77-82 KB / ~2.8-3.0k
lines, always spills to a side file, and then costs several more calls to mine three
fields — often including a failed jq, because the spill is TOON. The endorsement is
replaced with the scoped path, plus the line that saves the failed attempt:
--output-format toon is not jq-parseable, use --json when you will filter.

insight.mdtop-alerts and alert-topk-by-label return the same top-10
breakdown.
The card now says to pick one, and to save raw --json to a file once and
run every jq against the file rather than re-invoking the command — each re-invocation
is a real backend query against the OLAP store, and a heavy window can time out.

Verification

go build ./..., go vet ./..., gofmt -l, golangci-lint run (0 issues),
go test ./... -count=1 (0 failures), go run ./internal/cmd/skilldoc check
("cards OK") — all green on the stacked tree.

The wrapper-guard behaviour is covered by a test proven load-bearing: removing the
guard makes it red.

ysyneu added 6 commits July 27, 2026 04:51
Every generated card fence now carries a one-line response-shape summary
per verb (top-level object vs. bare array vs. {items:[...]} page wrapper,
plus the field names at that level), extracted straight from the
"Response fields (...):" block cligen already writes into each command's
Long. That block was previously only reachable via `--help`; an agent
that reads a card alone had no way to learn the real shape or field
names and would guess one, getting a silent null back on a miss instead
of an error (e.g. guessing `is_enabled` on automation rules, which
returned null for every rule/channel because the real field is
`enabled`).

responseShapeLine in internal/skilldoc/generate.go recognizes cligen's
three canonical header phrasings and reads field names from the correct
indent depth for each (row fields nested one level under the sole
"items" key for the page-wrapper shape; top-level fields directly
otherwise). It does not invent a fourth shape or re-derive anything
cligen didn't already say.

Regenerated every card via `skilldoc gen` except incident.md, which
another branch is actively editing (--comment -> --comment-file) and
must not be touched here.
Mechanical `skilldoc gen` regeneration only — picks up the response-shape
line every verb's fence now carries. No hand-written content changed.
incident.md is deliberately excluded (owned by another in-flight branch).
schedule.md:
- Stop recommending `oncall who` as a convenient global on-call snapshot.
  It returns every schedule in one call (77-82KB / ~2800 lines), always
  spills to a file, and still takes 3-10 follow-up reads to dig out 3-4
  fields. Replace the hot-flow and gotcha with the scoped path:
  `schedule list --query|--team-ids` to find IDs, then per-schedule
  `info --start now --end +1h`.
- Note `--output-format toon` is not jq-parseable; use `--json` to filter.

insight.md:
- Note `top-alerts` and `alert-topk-by-label` return the same top-K
  breakdown (alert-topk-by-label is the superset) — pick one.
- Note `responder` has no `--limit`/`--page`; account-wide load analysis
  should take the one full response, not cap-and-refetch.
- Note re-running a whole `insight` command to tweak a jq filter is a
  real repeated backend query; save `--json` once and iterate jq locally.

Also regenerates both fences via `skilldoc gen` to pick up the
response-shape summaries from the prior commit.
…eader

responseShapeLine only recognized the items[]-wrapper shape by the literal
substring "nested under items[]" in cligen's header. If that header wording
ever drifts, the classification silently falls through to "single object"
even though the sole top-level field is exactly one of cligen's own
list-envelope wire names (items/docs/list, array type, per listEnvelope in
internal/cmd/cligen/main.go) — asserting the one shape claim this feature
exists to get right, wrong, with the generator's own authority behind it.

Guard against that: when the fallback classification's sole field matches a
known wrapper wire name, emit nothing instead of asserting "single object".
A missing line is recoverable via --help; a confidently wrong one is not.
Verified this guard is a no-op against every real command today (cligen's
own header already says "wrapped" for every such case) — it only protects
against future drift.

Also generalizes the ground-truth cross-check from one hand-picked command
(schedule list) to every real command that documents a Response fields
block (~200), each rendered as an isolated single-command fence to avoid
same-named-verb collisions across subgroups (e.g. "incident get" vs
"incident war-room get" both heading "### get" in the full group fence).
Classification drift anywhere now fails the build, not just for the one
verb someone happened to write a test against.
…ines

Five spots restated a verb's response envelope shape by hand, now fully
covered by that verb's own generated "- response: ..." line:

- monit.md: targets response-shape restatement removed; kept the
  updated_at="last seen, not online now" fact, which is real knowledge.
- role.md: two bullets restating permission-list (items[]) and
  permission-factor-list (top-level array) shapes, both fully redundant.
- team.md: infos and list gotchas trimmed to drop the shape restatement;
  kept the positional-args fact for infos.
- channel.md: one bullet summarizing list + rule-list shapes collectively,
  now subsumed by the per-verb generated lines.
- monit-query.md: rows bullet trimmed to drop the shape assertion; kept the
  values/fields semantics and the __value__ canonical key fact.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant