feat(skilldoc): dedupe response shapes, name page-wrapper pagination fields, fix enum misparse - #120
Merged
Merged
Conversation
Several commands in the same group (create/get/update/enable/disable, ...) commonly return the same resource, so the generated "- response: ..." field list was repeated verbatim once per command inside a card's GENERATED fence -- up to several hundred bytes each, and up to a few KB per card. An agent reading the card paid that token cost on every read. Within one GENERATED group, only the first command to emit a given response shape keeps the full field list; every later command whose shape is byte-identical instead emits a short reference to it (e.g. "same shape as `get <rule_id>` above"). Distinct shapes are left untouched, and the dedup map is scoped per card so cards stay self-contained -- no card ever references another card's command.
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.
The Request-fields enum extractor matched the FIRST "[...]" bracket
anywhere in a flag's tail text and treated its contents as an enum
declaration. cligen's own trailing enum bracket is always the last
thing on the line (before an optional constraint suffix), but a
description can legitimately quote an unrelated bracket earlier in its
own prose -- e.g. a regex character class ("1-40 chars of
'[a-zA-Z0-9_]'"). The old code fabricated a one-value enum from that
character class AND deleted it from the sentence, leaving an empty
quoted pair where the class used to be.
Only recognize a bracket anchored to the end of the tail (once an
optional trailing "(constraint)" is set aside) as a genuine enum --
exactly where cligen's generator appends one. Anything earlier in the
text is left untouched, description and constraint both.
Regenerating all cards with this fix touches only field.md (the sole
card affected): the fabricated enum tag is gone and the character
class renders intact in the description.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem 1: repeated response field lists
The
skilldocgenerator emits, for each command inside a card'sGENERATED:<group>fence, a- response: ...line listing the command's full response field shape. Several commands in the same group commonly return the same resource (create/get/update/enable/disable, ...), so that field list -- often 700-800+ bytes -- was repeated verbatim once per command. Across the reference cards this added up to ~18KB of byte-identical text that an agent reading a card pays for on every read, with zero new information.Fix: within one
GENERATED:<group>fence, only the first command to emit a given response shape keeps the full field list. Every later command in the same group whose response shape is byte-identical instead emits a short reference, e.g.:Dedup is scoped per card -- a card never references another card's command. Only byte-identical shapes are deduplicated; anything even slightly different keeps its own full line. Output stays deterministic.
Problem 2: page-wrapper responses dropped their pagination fields
The response-shape line for a
{items: [...]}page-wrapper response only ever named the row fields nested underitems-- it silently dropped the wrapper's OTHER top-level fields (total,has_next_page,search_after_ctx, and similar pagination metadata). A card would tell an agent to pass--search-after-ctxto paginate, but never say where the returned cursor actually lives in the response.Fix: name those siblings in the wrapper descriptor itself:
items fields:replacesfields: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 composes with the dedup fix: two page-wrapper responses with byte-identical fields AND siblings still collapse to a reference line; two with the same row fields but different siblings (e.g. one paginated, one a batch-get) now correctly stay distinct. This surfaced and fixed two pre-existing false-duplicate cases the dedup would otherwise have produced from the same root bug:
schedule listandrum application-listused to render as exact duplicates of an unrelated batch-get command purely because both had their real pagination fields dropped.Problem 3: a description's bracketed prose was misparsed as an enum
field create --field-name's real description is: "Machine name. Must start with a letter or underscore; 1–40 chars of '[a-zA-Z0-9_]'. Immutable after creation." The Request-fields enum extractor matched the FIRST[...]bracket ANYWHERE in a flag's tail and treated it as an enum declaration -- so this regex character class, quoted mid-sentence, got parsed as a one-value enum and deleted from the description, rendering as:Fix: only recognize a bracket anchored to the END of the tail (once an optional trailing
(constraint)is set aside) as a genuine enum -- exactly where the CLI generator (cligen) appends one. Anything earlier in the text is left untouched. Regenerating with this fix touches onlyfield.md(the sole card affected); the character class now renders intact and the fabricated enum tag is gone. Scanned all 22 regenerated cards for other instances of this pattern -- none found; this was the only occurrence in the current command set.Measured byte impact
Regenerating all cards with
go run ./internal/cmd/skilldoc gen, relative to the pre-fix baseline:(Total repo delta across all 22 cards is −16,050, which also includes +302 bytes from merging in an unrelated, already-merged upstream prose fix to
automation.mdpicked up along the way -- not from this PR's generator changes.)Verified every unique response shape still appears in full exactly once per group (nothing lost, only true duplicates removed), every
same shape asreference resolves to an earlier heading in the same file, and no other card besidesfield.mdwas touched by the enum fix.Testing
Added to
internal/skilldoc:TestGenerateFence_DedupesIdenticalResponseShapesWithinGroup/TestGenerateFence_ResponseShapeDedupIsPerGroupNotGlobalTestResponseShapeLine_ItemsWrappedNamesPaginationSiblings/TestResponseShapeLine_ItemsWrappedNoSiblingsUnchangedTestSplitTrailingEnum_IgnoresBracketedProseInsideDescription/TestSplitTrailingEnum_RecognizesGenuineTrailingEnum/TestSplitTrailingEnum_GenuineEnumWithTrailingConstraint/TestSplitTrailingEnum_SingleValueEnumAtEnd/TestGenerateFence_BracketedProseSurvivesAlongsideGenuineEnumgo build ./...andgo test ./...pass with no pre-existing failures.go run ./internal/cmd/skilldoc checkreports all cards fresh against the regenerated fences.gofmt -lclean.