Skip to content

Commit 237e0ab

Browse files
committed
feat(skilldoc): name a page-wrapper response's pagination siblings
The response-shape line for a `{items: [...]}` page-wrapper response only ever named the row fields nested under "items" -- it silently dropped the wrapper's OTHER top-level fields (total, has_next_page, search_after_ctx, and similar pagination metadata cligen's listEnvelope documents alongside "items"). A card would show a --search-after-ctx request flag for pagination but never say where the returned cursor actually lives in the response. Name those siblings in the wrapper descriptor itself, e.g.: - response: `{items: [...], has_next_page, search_after_ctx, total}` page wrapper -- ... -- items fields: ... ("items fields:" replaces "fields:" for this shape only, so the row list stays visually distinct from the siblings just named). Wrapper responses with no such siblings render unchanged. This also corrects two page-wrapper response instances (schedule `list`, rum `application-list`) that used to render as duplicates of an unrelated command's response purely because both had their real siblings dropped -- each now renders its own accurate shape.
1 parent 527bc79 commit 237e0ab

19 files changed

Lines changed: 155 additions & 56 deletions

internal/skilldoc/generate.go

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -373,13 +373,21 @@ type respField struct{ name, typ string }
373373
// - top-level object: the block's own fields are the response.
374374
// - top-level array: `--json` is a bare array of these row objects — pipe
375375
// `jq '.[]'`, never `.items[]`.
376-
// - `{items: [...]}` page wrapper: the block's sole top-level field is
377-
// `items`; the row fields are nested one level (2 spaces) deeper under it.
376+
// - `{items: [...]}` page wrapper: the block's top-level array field is
377+
// `items` (row fields nested one level deeper under it), alongside
378+
// scalar pagination siblings — `total`, `has_next_page`,
379+
// `search_after_ctx`, … (cligen's listEnvelope requires every
380+
// non-`items` top-level field to be a scalar, so these are always
381+
// pagination metadata, never more row data).
378382
//
379383
// Field names (with their documented type) are read from whichever indent
380384
// depth holds the actual row/object fields for the detected shape, so the
381385
// summary always names the fields an agent would pipe `jq` at — not the
382-
// wrapper key.
386+
// wrapper key. For the wrapper shape, the pagination siblings are named too
387+
// (by name only — their type is implied by cligen's scalar-sibling
388+
// invariant), since an agent told to paginate via `--search-after-ctx` still
389+
// needs to know the returned cursor lives at `.search_after_ctx` next to
390+
// `.items`, not buried in a field list it never sees.
383391
func responseShapeLine(long string) string {
384392
lines := strings.Split(long, "\n")
385393
headerIdx, header := -1, ""
@@ -396,19 +404,31 @@ func responseShapeLine(long string) string {
396404
wrapped := strings.Contains(header, "nested under items[]")
397405
fieldIndent := " "
398406
if wrapped {
399-
fieldIndent = " " // one level under the sole top-level "items" row
407+
fieldIndent = " " // one level under the top-level "items" row
400408
}
401409

402-
var fields []respField
410+
// fields holds the row-level fields the summary's "fields:" list names
411+
// (top-level for object/array shapes, one level under "items" for the
412+
// wrapper shape). siblings holds the wrapper shape's OTHER top-level
413+
// fields — pagination metadata alongside "items" (total, has_next_page,
414+
// search_after_ctx, …) — collected only when wrapped, since the
415+
// unwrapped shapes have no such siblings to speak of.
416+
var fields, siblings []respField
403417
for _, line := range lines[headerIdx+1:] {
404418
if strings.TrimSpace(line) == "" {
405419
break
406420
}
407421
m := responseFieldRe.FindStringSubmatch(line)
408-
if m == nil || m[1] != fieldIndent {
422+
if m == nil {
409423
continue
410424
}
411-
fields = append(fields, respField{m[2], m[3]})
425+
indent, name, typ := m[1], m[2], m[3]
426+
switch {
427+
case indent == fieldIndent:
428+
fields = append(fields, respField{name, typ})
429+
case wrapped && indent == " " && !wrapperWireNames[name]:
430+
siblings = append(siblings, respField{name, typ})
431+
}
412432
}
413433
if len(fields) == 0 {
414434
return ""
@@ -430,18 +450,37 @@ func responseShapeLine(long string) string {
430450
return ""
431451
}
432452

433-
var shape string
453+
var shape, fieldsLabel string
434454
switch {
435455
case wrapped:
436-
shape = "`{items: [...]}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`)"
456+
shape = fmt.Sprintf("`{items: [...]%s}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`)", siblingSuffix(siblings))
457+
fieldsLabel = "items fields" // disambiguates row fields from the wrapper's own pagination siblings named above
437458
case strings.Contains(header, "TOP-LEVEL array"):
438459
shape = "TOP-LEVEL array — pipe `--json | jq '.[]'` (NOT `.items[]`)"
460+
fieldsLabel = "fields"
439461
default:
440462
shape = "single object (`data` unwrapped to the top level)"
463+
fieldsLabel = "fields"
441464
}
442465
names := make([]string, len(fields))
443466
for i, f := range fields {
444467
names[i] = f.name + " (" + f.typ + ")"
445468
}
446-
return fmt.Sprintf("- response: %s — fields: %s\n", shape, strings.Join(names, "; "))
469+
return fmt.Sprintf("- response: %s — %s: %s\n", shape, fieldsLabel, strings.Join(names, "; "))
470+
}
471+
472+
// siblingSuffix renders a wrapped response's top-level pagination-metadata
473+
// siblings — e.g. ", total, has_next_page, search_after_ctx" — appended
474+
// inside the `{items: [...]}` wrapper descriptor, in the order cligen
475+
// documented them. Empty when the envelope carries no siblings beyond the
476+
// row array itself.
477+
func siblingSuffix(siblings []respField) string {
478+
if len(siblings) == 0 {
479+
return ""
480+
}
481+
names := make([]string, len(siblings))
482+
for i, s := range siblings {
483+
names[i] = s.name
484+
}
485+
return ", " + strings.Join(names, ", ")
447486
}

internal/skilldoc/response_shape_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,24 @@ Request fields:
4848
--schedule-id int (required) — Schedule ID.
4949
`
5050

51+
// itemsWrappedWithPaginationSiblingsShapeLong reproduces the real shape of
52+
// `fduty insight incident-list` (internal/cli/zz_generated_analytics.go): a
53+
// page-wrapper response whose top level carries "items" ALONGSIDE scalar
54+
// pagination metadata — has_next_page, search_after_ctx, total — not just
55+
// "items" alone. cligen's listEnvelope requires every non-"items" top-level
56+
// field here to be a scalar, so these three are always pagination metadata,
57+
// never more row data.
58+
const itemsWrappedWithPaginationSiblingsShapeLong = `List insight incidents.
59+
60+
Response fields ('data' envelope is unwrapped — rows are nested under items[]; pipe 'jq '.items[]'', NOT '.data.items[]'):
61+
- has_next_page (boolean)
62+
- items (array<object>)
63+
- incident_id (string)
64+
- title (string)
65+
- search_after_ctx (string) — Cursor token to fetch the next page. Pass it back in the next request's 'search_after_ctx'.
66+
- total (integer) — Total matching incidents.
67+
`
68+
5169
// driftedWrapperHeaderLong simulates a future cligen wording change to the
5270
// items[]-wrapper header that no longer contains the literal substring
5371
// "nested under items[]" this parser keys on (e.g. cligen's header text was
@@ -145,6 +163,48 @@ func TestResponseShapeLine_ItemsWrapped(t *testing.T) {
145163
}
146164
}
147165

166+
// TestResponseShapeLine_ItemsWrappedNamesPaginationSiblings covers the real
167+
// `insight incident-list` shape: the wrapper's top level carries "items"
168+
// ALONGSIDE scalar pagination metadata (has_next_page, search_after_ctx,
169+
// total). Those siblings must be named in the wrapper descriptor itself —
170+
// an agent told to paginate via `--search-after-ctx` needs to know the
171+
// returned cursor lives at `.search_after_ctx` next to `.items`, not silently
172+
// dropped because the parser only ever looked one level under "items".
173+
func TestResponseShapeLine_ItemsWrappedNamesPaginationSiblings(t *testing.T) {
174+
got := responseShapeLine(itemsWrappedWithPaginationSiblingsShapeLong)
175+
if !strings.Contains(got, "items: [...]") || !strings.Contains(got, "page wrapper") {
176+
t.Errorf("items[] wrapper shape not detected:\n%s", got)
177+
}
178+
// The siblings are named inside the wrapper descriptor, in the order
179+
// cligen documented them (source order, already alphabetical here).
180+
if !strings.Contains(got, "{items: [...], has_next_page, search_after_ctx, total}") {
181+
t.Errorf("wrapper descriptor must name the pagination siblings:\n%s", got)
182+
}
183+
// The row fields (nested under items) are still named, under a label that
184+
// disambiguates them from the siblings just named above.
185+
if !strings.Contains(got, "items fields: incident_id (string); title (string)") {
186+
t.Errorf("row fields must still be listed under an unambiguous label:\n%s", got)
187+
}
188+
// The siblings must not ALSO appear duplicated in the row-field list.
189+
fieldsPart := strings.SplitN(got, "items fields:", 2)[1]
190+
for _, sib := range []string{"has_next_page", "search_after_ctx", "total"} {
191+
if strings.Contains(fieldsPart, sib) {
192+
t.Errorf("pagination sibling %q must not be duplicated in the row-field list:\n%s", sib, got)
193+
}
194+
}
195+
}
196+
197+
// TestResponseShapeLine_ItemsWrappedNoSiblingsUnchanged proves the sibling
198+
// feature is additive: a wrapper with no pagination siblings beyond "items"
199+
// (itemsWrappedShapeLong) renders the exact same wrapper descriptor as
200+
// before — no trailing ", " artifact from an empty sibling list.
201+
func TestResponseShapeLine_ItemsWrappedNoSiblingsUnchanged(t *testing.T) {
202+
got := responseShapeLine(itemsWrappedShapeLong)
203+
if !strings.Contains(got, "`{items: [...]}` page wrapper") {
204+
t.Errorf("wrapper with no siblings must render the bare descriptor, got:\n%s", got)
205+
}
206+
}
207+
148208
func TestResponseShapeLine_NoBlockIsEmpty(t *testing.T) {
149209
if got := responseShapeLine(noResponseBlockLong); got != "" {
150210
t.Errorf("command with no Response fields block must yield no response line, got:\n%s", got)

skills/flashduty/reference/alert.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ List events for an alert
6060
- `--limit` int64 — Page size. Defaults to 20 and cannot exceed 100. (0-100)
6161
- `--page` int64 — Page number starting at 1. Used when 'search_after_ctx' is omitted. (min 0)
6262
- `--search-after-ctx` string — Cursor returned by the previous page. When supplied, cursor pagination is used instead of page-number pagination.
63-
- response: `{items: [...]}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — fields: account_id (integer); alert_id (string); alert_key (string); channel_id (integer); created_at (integer); data_source_id (integer); deleted_at (integer); description (string); event_id (string); event_severity (string); event_status (string); event_time (integer); images (array<object>); integration_id (integer); integration_type (string); labels (object); title (string); title_rule (string); updated_at (integer)
63+
- response: `{items: [...], has_next_page, search_after_ctx, total}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — items fields: account_id (integer); alert_id (string); alert_key (string); channel_id (integer); created_at (integer); data_source_id (integer); deleted_at (integer); description (string); event_id (string); event_severity (string); event_status (string); event_time (integer); images (array<object>); integration_id (integer); integration_type (string); labels (object); title (string); title_rule (string); updated_at (integer)
6464

6565
### events <alert_id>
6666
List alert events
@@ -74,7 +74,7 @@ List alert activity feed
7474
- `--page` int64 — Page number, starting at 1.
7575
- `--search-after-ctx` string
7676
- `--types` stringSlice — Filter by feed types.
77-
- response: `{items: [...]}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — fields: account_id (integer); created_at (integer); creator_id (integer); detail (object); ref_id (string); type (string); updated_at (integer)
77+
- response: `{items: [...], has_next_page}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — items fields: account_id (integer); created_at (integer); creator_id (integer); detail (object); ref_id (string); type (string); updated_at (integer)
7878

7979
### get <alert_id>
8080
Get alert detail
@@ -102,7 +102,7 @@ List alerts
102102
### list-by-ids <alert-id> [<id2>...]
103103
List alerts by IDs
104104
- `<alert-ids>` (positional, required) stringSlice — List of alert IDs (ObjectID hex strings).
105-
- response: `{items: [...]}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — fields: account_id (integer); alert_id (string); alert_key (string); alert_severity (string); alert_status (string); channel_id (integer); channel_name (string); channel_status (string); created_at (integer); data_source_id (integer); data_source_name (string); data_source_ref_id (string); data_source_type (string); description (string); end_time (integer); event_cnt (integer); events (array<object>); ever_muted (boolean); images (array<object>); incident (object); integration_id (integer); integration_name (string); integration_ref_id (string); integration_type (string); labels (object); last_time (integer); responder_email (string); responder_name (string); start_time (integer); title (string); title_rule (string); updated_at (integer)
105+
- response: `{items: [...], has_next_page, search_after_ctx, total}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — items fields: account_id (integer); alert_id (string); alert_key (string); alert_severity (string); alert_status (string); channel_id (integer); channel_name (string); channel_status (string); created_at (integer); data_source_id (integer); data_source_name (string); data_source_ref_id (string); data_source_type (string); description (string); end_time (integer); event_cnt (integer); events (array<object>); ever_muted (boolean); images (array<object>); incident (object); integration_id (integer); integration_name (string); integration_ref_id (string); integration_type (string); labels (object); last_time (integer); responder_email (string); responder_name (string); start_time (integer); title (string); title_rule (string); updated_at (integer)
106106

107107
### merge <alert-id> [<id2>...]
108108
Merge alerts into an incident
@@ -120,7 +120,7 @@ Get alert pipeline
120120
### pipeline-list <integration-id> [<id2>...]
121121
List alert pipelines
122122
- `<integration-ids>` (positional, required) intSlice — Integration IDs.
123-
- response: `{items: [...]}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — fields: created_at (integer); creator_id (integer); integration_id (integer); rules (array<object>); status (string); updated_at (integer); updated_by (integer)
123+
- response: `{items: [...]}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — items fields: created_at (integer); creator_id (integer); integration_id (integer); rules (array<object>); status (string); updated_at (integer); updated_by (integer)
124124

125125
### pipeline-upsert <integration-id>
126126
Create or update alert pipeline

skills/flashduty/reference/calendar.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ List calendar events
8383
- `--day` int64 — Day (1-31). 0 means no day filter. (0-31)
8484
- `--month` int64 — Month (1-12). 0 means no month filter. (0-12)
8585
- `--year` int64 — Year. Defaults to the current year when omitted. (min 2023)
86-
- response: `{items: [...]}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — fields: account_id (integer); cal_id (string); created_at (integer); creator_id (integer); description (string); end_at (string); event_id (string); is_off (boolean); start_at (string); summary (string); updated_at (integer)
86+
- response: `{items: [...], total}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — items fields: account_id (integer); cal_id (string); created_at (integer); creator_id (integer); description (string); end_at (string); event_id (string); is_off (boolean); start_at (string); summary (string); updated_at (integer)
8787

8888
### event-upsert <cal-id>
8989
Upsert calendar event
@@ -105,7 +105,7 @@ Get calendar info
105105
List calendars
106106
- `--kind` string — Calendar kind filter. Defaults to personal when empty. · enum: region.official.holiday | personal
107107
- `--no-locale` bool — Disable locale filtering when listing public-holiday calendars.
108-
- response: `{items: [...]}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — fields: account_id (integer); cal_id (string); cal_name (string); created_at (integer); creator_id (integer); description (string); extra_cal_ids (array<string>); kind (string); status (string); team_id (integer); timezone (string); updated_at (integer); updated_by (integer); workdays (array<integer>)
108+
- response: `{items: [...], total}` page wrapper — pipe `--json | jq '.items[]'` (NOT top-level `.[]`) — items fields: account_id (integer); cal_id (string); cal_name (string); created_at (integer); creator_id (integer); description (string); extra_cal_ids (array<string>); kind (string); status (string); team_id (integer); timezone (string); updated_at (integer); updated_by (integer); workdays (array<integer>)
109109

110110
### update <cal-id>
111111
Update calendar

0 commit comments

Comments
 (0)