[object Object]

← back to Cli Printing Press

fix(cli): preserve novel-command keys in --compact partial-strip path (#1167)

f88a10b3a9fc3738acffc496db4e476a57851c21 · 2026-05-12 01:00:31 -0700 · Trevin Chow

* fix(cli): preserve novel-command keys in --compact partial-strip path

PR #1078 added a per-item fallback that preserves the original row when
no allow-list key matches. The gap: if even one allow-list key matches
(say `id`), the rest of the row's keys get stripped — so a search-like
novel command emitting `{"id":"x","object_name":"users","match_key":"uuid"}`
silently projected to `{"id":"x"}` under `--compact` / `--agent`.

`compactListFields` now extends the static allow-list with a data-driven
keep set: any scalar key present in 80%+ of input rows is also kept.
Verbose fields (description, body, content, …) stay excluded regardless
of frequency, so the compact intent is preserved. The per-item fallback
from #1078 still fires when nothing matches.

Side effects:
- Consolidated the verbose blocklist into a package-level
  `compactVerboseFields` shared by `compactListFields` and
  `compactObjectFields`.
- `printAutoTable`'s inline scalar type-switch now reuses the new
  `isCompactScalar` helper.

Closes #1127

* fix(cli): cap compactListFields threshold so single-row miss doesn't veto

Greptile flagged that ceil(0.8 * n) == n for n in {2,3,4}, so the new
data-driven keep rule silently required 100% presence on small lists
and reintroduced the partial-strip bug for heterogeneous 2-4 row
responses (e.g., a 2-row search where row 1 carries object_name and
row 2 doesn't — row 1 still got stripped).

Cap the threshold at len(items)-1 when len >= 2 so a single missing row
cannot veto a key. len == 1 keeps the 100% requirement (only row must
have the key; per-item fallback still preserves the original row when
nothing matches).

Files touched

Diff

commit f88a10b3a9fc3738acffc496db4e476a57851c21
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Tue May 12 01:00:31 2026 -0700

    fix(cli): preserve novel-command keys in --compact partial-strip path (#1167)
    
    * fix(cli): preserve novel-command keys in --compact partial-strip path
    
    PR #1078 added a per-item fallback that preserves the original row when
    no allow-list key matches. The gap: if even one allow-list key matches
    (say `id`), the rest of the row's keys get stripped — so a search-like
    novel command emitting `{"id":"x","object_name":"users","match_key":"uuid"}`
    silently projected to `{"id":"x"}` under `--compact` / `--agent`.
    
    `compactListFields` now extends the static allow-list with a data-driven
    keep set: any scalar key present in 80%+ of input rows is also kept.
    Verbose fields (description, body, content, …) stay excluded regardless
    of frequency, so the compact intent is preserved. The per-item fallback
    from #1078 still fires when nothing matches.
    
    Side effects:
    - Consolidated the verbose blocklist into a package-level
      `compactVerboseFields` shared by `compactListFields` and
      `compactObjectFields`.
    - `printAutoTable`'s inline scalar type-switch now reuses the new
      `isCompactScalar` helper.
    
    Closes #1127
    
    * fix(cli): cap compactListFields threshold so single-row miss doesn't veto
    
    Greptile flagged that ceil(0.8 * n) == n for n in {2,3,4}, so the new
    data-driven keep rule silently required 100% presence on small lists
    and reintroduced the partial-strip bug for heterogeneous 2-4 row
    responses (e.g., a 2-row search where row 1 carries object_name and
    row 2 doesn't — row 1 still got stripped).
    
    Cap the threshold at len(items)-1 when len >= 2 so a single missing row
    cannot veto a key. len == 1 keeps the 100% requirement (only row must
    have the key; per-item fallback still preserves the original row when
    nothing matches).
---
 internal/generator/generator_test.go               | 31 +++++++--
 internal/generator/templates/helpers.go.tmpl       | 77 ++++++++++++++++++----
 .../internal/cli/helpers.go                        | 77 ++++++++++++++++++----
 .../expected/generate-golden-api/dogfood.json      |  2 +-
 .../printing-press-golden/internal/cli/helpers.go  | 77 ++++++++++++++++++----
 5 files changed, 224 insertions(+), 40 deletions(-)

diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go
index 87b29c08..c7147496 100644
--- a/internal/generator/generator_test.go
+++ b/internal/generator/generator_test.go
@@ -3556,11 +3556,17 @@ func TestGeneratedOutput_MutatingCommandsHaveEnvelope(t *testing.T) {
 	assert.Contains(t, content, `envelope["success"] = false`)
 }
 
-// TestCompactListFieldsPreservesUnknownShapes pins the two contracts that
-// keep `--agent` / `--compact` from silently emitting {} for records that
-// don't match the canonical id/name/title allowlist: a per-item fallback
-// to the original item, and a wider scalar allowlist that covers
-// monetary, metric, locale, and identity-adjacent keys.
+// TestCompactListFieldsPreservesUnknownShapes pins the contracts that keep
+// `--agent` / `--compact` from silently emitting {} or partial-strip rows
+// for records that don't match the canonical id/name/title allowlist:
+//
+//  1. The static allow-list covers canonical scalars (price/fare/locale/code/...).
+//  2. A data-driven extension keeps any scalar key present in 80%+ of rows,
+//     so hand-written novel commands whose output keys aren't on the
+//     allow-list (object_name, match_key, snippet) survive projection.
+//  3. Verbose fields (description, body, ...) are excluded from the
+//     extension regardless of frequency.
+//  4. A per-item fallback preserves the original item when no key matches.
 //
 // Pinned at the template level because the helper renders into every
 // printed CLI's helpers.go and a per-fixture assertion would miss future
@@ -3574,7 +3580,7 @@ func TestCompactListFieldsPreservesUnknownShapes(t *testing.T) {
 	body := string(data)
 
 	assert.Contains(t, body, "if len(compact) == 0 {",
-		"compactListFields must preserve the original item when no allowlist keys match — otherwise records with domain-specific field names get reduced to {}")
+		"compactListFields must preserve the original item when no keep keys match — otherwise records with domain-specific field names get reduced to {}")
 	assert.Contains(t, body, "compact = item",
 		"compactListFields must assign the original item as the per-item fallback")
 
@@ -3587,6 +3593,19 @@ func TestCompactListFieldsPreservesUnknownShapes(t *testing.T) {
 		assert.Contains(t, body, key,
 			"compactListFields allowlist must include %s so canonical-schema records keep high-signal scalars across business/travel/commerce/locale APIs", key)
 	}
+
+	// Data-driven extension: pin the symbols that make the 80%-frequency
+	// rule fire. Without these, partial-strip silently drops novel-command
+	// fields whenever a row happens to also carry one allow-list key
+	// (e.g. {"id":"x","object_name":"users","match_key":"uuid"} -> {"id":"x"}).
+	assert.Contains(t, body, "keyCounts",
+		"compactListFields must count per-key occurrence so frequent novel-command keys survive")
+	assert.Contains(t, body, "isCompactScalar",
+		"compactListFields must filter the data-driven extension by scalar type so nested objects/arrays don't bloat --compact output")
+	assert.Contains(t, body, "compactVerboseFields",
+		"compactListFields must exclude description/body/content from the data-driven extension regardless of frequency")
+	assert.Contains(t, body, "threshold",
+		"compactListFields must compute a frequency threshold for the data-driven extension")
 }
 
 // The cursor param's "0" must survive paginatedGet's zero-value strip:
diff --git a/internal/generator/templates/helpers.go.tmpl b/internal/generator/templates/helpers.go.tmpl
index 5b7ab10c..cf8bec32 100644
--- a/internal/generator/templates/helpers.go.tmpl
+++ b/internal/generator/templates/helpers.go.tmpl
@@ -878,6 +878,14 @@ func extractResponseData(data json.RawMessage) json.RawMessage {
 	}
 }
 
+// compactVerboseFields are the prose-shaped fields stripped by both the
+// list and single-object compact paths. Centralised so the contract stays
+// in lockstep across both helpers.
+var compactVerboseFields = map[string]bool{
+	"description": true, "body": true, "content": true,
+	"comments": true, "attachments": true, "html": true, "markdown": true,
+}
+
 // compactFields keeps only the most important fields for agent consumption.
 // For arrays: allowlist of high-gravity fields (no descriptions).
 // For single objects: blocklist that strips known-verbose fields (descriptions, comments, etc.).
@@ -898,9 +906,23 @@ func compactFields(data json.RawMessage) json.RawMessage {
 }
 
 // compactListFields keeps only high-gravity fields for array responses.
-// When an item carries none of these keys the original is preserved, so
-// `--agent` does not silently emit {} for APIs whose response shapes use
-// domain-specific field names (travel: airline/destination; commerce: vendor).
+//
+// Two-layer keep rule:
+//
+//  1. A static allow-list covers canonical scalars (id/name/price/status/...).
+//  2. A data-driven extension also keeps any scalar key present in at least
+//     80% of input rows. This catches hand-written novel commands whose
+//     output keys (object_name, match_key, snippet) aren't on the canonical
+//     allow-list, without forcing every printed CLI to expand the list.
+//
+// Verbose fields (description, body, content, etc.) are excluded from the
+// data-driven extension regardless of frequency, so the compact intent
+// (short identifying values for agent consumption, not full prose) is
+// preserved.
+//
+// When an item still carries none of the keep keys, the original is
+// preserved so `--agent` does not silently emit {} for shapes whose key
+// names are entirely off-canonical.
 func compactListFields(items []map[string]any) json.RawMessage {
 	keepFields := map[string]bool{
 		// Identity
@@ -924,6 +946,31 @@ func compactListFields(items []map[string]any) json.RawMessage {
 		// Versioning
 		"version": true,
 	}
+	if len(items) > 0 {
+		keyCounts := map[string]int{}
+		for _, item := range items {
+			for k, v := range item {
+				if compactVerboseFields[k] || !isCompactScalar(v) {
+					continue
+				}
+				keyCounts[k]++
+			}
+		}
+		// ceil(len(items) * 0.8) without importing math. Capped at len-1 for
+		// len >= 2 so a single missing row cannot veto a key on small lists
+		// (without the cap, ceil(0.8*n) == n for n in {2,3,4}, which silently
+		// reintroduces the partial-strip bug whenever a heterogeneous 2-4 row
+		// response mixes one allow-list key with novel keys).
+		threshold := (len(items)*4 + 4) / 5
+		if len(items) >= 2 && threshold > len(items)-1 {
+			threshold = len(items) - 1
+		}
+		for k, count := range keyCounts {
+			if count >= threshold {
+				keepFields[k] = true
+			}
+		}
+	}
 
 	filtered := make([]map[string]any, 0, len(items))
 	for _, item := range items {
@@ -942,17 +989,26 @@ func compactListFields(items []map[string]any) json.RawMessage {
 	return result
 }
 
+// isCompactScalar reports whether v is a small primitive (string, number,
+// bool, null) suitable for --compact projection. Objects and arrays are
+// rejected: they almost always represent nested structure that defeats the
+// "short identifying value" intent of --compact, and including them in the
+// data-driven keep set would bloat agent output.
+func isCompactScalar(v any) bool {
+	switch v.(type) {
+	case nil, bool, float64, string:
+		return true
+	default:
+		return false
+	}
+}
+
 // compactObjectFields strips known-verbose fields from single-object responses.
 // Uses a blocklist so it works across all API domains (project management, payments, CRM, etc.).
 func compactObjectFields(obj map[string]any) json.RawMessage {
-	stripFields := map[string]bool{
-		"description": true, "body": true, "content": true,
-		"comments": true, "attachments": true, "html": true, "markdown": true,
-	}
-
 	compact := map[string]any{}
 	for k, v := range obj {
-		if !stripFields[k] {
+		if !compactVerboseFields[k] {
 			compact[k] = v
 		}
 	}
@@ -1124,8 +1180,7 @@ func printAutoTable(w io.Writer, items []map[string]any) error {
 	// Count scalar vs complex fields to decide format
 	scalarCount := 0
 	for _, v := range items[0] {
-		switch v.(type) {
-		case string, float64, bool, nil:
+		if isCompactScalar(v) {
 			scalarCount++
 		}
 	}
diff --git a/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/helpers.go b/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/helpers.go
index 74a5990e..6dbef5a3 100644
--- a/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/helpers.go
+++ b/testdata/golden/expected/generate-golden-api-rich-auth/printing-press-rich-auth/internal/cli/helpers.go
@@ -678,6 +678,14 @@ func extractResponseData(data json.RawMessage) json.RawMessage {
 	}
 }
 
+// compactVerboseFields are the prose-shaped fields stripped by both the
+// list and single-object compact paths. Centralised so the contract stays
+// in lockstep across both helpers.
+var compactVerboseFields = map[string]bool{
+	"description": true, "body": true, "content": true,
+	"comments": true, "attachments": true, "html": true, "markdown": true,
+}
+
 // compactFields keeps only the most important fields for agent consumption.
 // For arrays: allowlist of high-gravity fields (no descriptions).
 // For single objects: blocklist that strips known-verbose fields (descriptions, comments, etc.).
@@ -698,9 +706,23 @@ func compactFields(data json.RawMessage) json.RawMessage {
 }
 
 // compactListFields keeps only high-gravity fields for array responses.
-// When an item carries none of these keys the original is preserved, so
-// `--agent` does not silently emit {} for APIs whose response shapes use
-// domain-specific field names (travel: airline/destination; commerce: vendor).
+//
+// Two-layer keep rule:
+//
+//  1. A static allow-list covers canonical scalars (id/name/price/status/...).
+//  2. A data-driven extension also keeps any scalar key present in at least
+//     80% of input rows. This catches hand-written novel commands whose
+//     output keys (object_name, match_key, snippet) aren't on the canonical
+//     allow-list, without forcing every printed CLI to expand the list.
+//
+// Verbose fields (description, body, content, etc.) are excluded from the
+// data-driven extension regardless of frequency, so the compact intent
+// (short identifying values for agent consumption, not full prose) is
+// preserved.
+//
+// When an item still carries none of the keep keys, the original is
+// preserved so `--agent` does not silently emit {} for shapes whose key
+// names are entirely off-canonical.
 func compactListFields(items []map[string]any) json.RawMessage {
 	keepFields := map[string]bool{
 		// Identity
@@ -724,6 +746,31 @@ func compactListFields(items []map[string]any) json.RawMessage {
 		// Versioning
 		"version": true,
 	}
+	if len(items) > 0 {
+		keyCounts := map[string]int{}
+		for _, item := range items {
+			for k, v := range item {
+				if compactVerboseFields[k] || !isCompactScalar(v) {
+					continue
+				}
+				keyCounts[k]++
+			}
+		}
+		// ceil(len(items) * 0.8) without importing math. Capped at len-1 for
+		// len >= 2 so a single missing row cannot veto a key on small lists
+		// (without the cap, ceil(0.8*n) == n for n in {2,3,4}, which silently
+		// reintroduces the partial-strip bug whenever a heterogeneous 2-4 row
+		// response mixes one allow-list key with novel keys).
+		threshold := (len(items)*4 + 4) / 5
+		if len(items) >= 2 && threshold > len(items)-1 {
+			threshold = len(items) - 1
+		}
+		for k, count := range keyCounts {
+			if count >= threshold {
+				keepFields[k] = true
+			}
+		}
+	}
 
 	filtered := make([]map[string]any, 0, len(items))
 	for _, item := range items {
@@ -742,17 +789,26 @@ func compactListFields(items []map[string]any) json.RawMessage {
 	return result
 }
 
+// isCompactScalar reports whether v is a small primitive (string, number,
+// bool, null) suitable for --compact projection. Objects and arrays are
+// rejected: they almost always represent nested structure that defeats the
+// "short identifying value" intent of --compact, and including them in the
+// data-driven keep set would bloat agent output.
+func isCompactScalar(v any) bool {
+	switch v.(type) {
+	case nil, bool, float64, string:
+		return true
+	default:
+		return false
+	}
+}
+
 // compactObjectFields strips known-verbose fields from single-object responses.
 // Uses a blocklist so it works across all API domains (project management, payments, CRM, etc.).
 func compactObjectFields(obj map[string]any) json.RawMessage {
-	stripFields := map[string]bool{
-		"description": true, "body": true, "content": true,
-		"comments": true, "attachments": true, "html": true, "markdown": true,
-	}
-
 	compact := map[string]any{}
 	for k, v := range obj {
-		if !stripFields[k] {
+		if !compactVerboseFields[k] {
 			compact[k] = v
 		}
 	}
@@ -924,8 +980,7 @@ func printAutoTable(w io.Writer, items []map[string]any) error {
 	// Count scalar vs complex fields to decide format
 	scalarCount := 0
 	for _, v := range items[0] {
-		switch v.(type) {
-		case string, float64, bool, nil:
+		if isCompactScalar(v) {
 			scalarCount++
 		}
 	}
diff --git a/testdata/golden/expected/generate-golden-api/dogfood.json b/testdata/golden/expected/generate-golden-api/dogfood.json
index d36d06aa..a2d47881 100644
--- a/testdata/golden/expected/generate-golden-api/dogfood.json
+++ b/testdata/golden/expected/generate-golden-api/dogfood.json
@@ -16,7 +16,7 @@
   },
   "dead_functions": {
     "dead": 0,
-    "total": 56
+    "total": 57
   },
   "dir": "<ARTIFACT_DIR>/generate-golden-api/printing-press-golden",
   "example_check": {
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go
index 13562857..aaa19131 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/cli/helpers.go
@@ -681,6 +681,14 @@ func extractResponseData(data json.RawMessage) json.RawMessage {
 	}
 }
 
+// compactVerboseFields are the prose-shaped fields stripped by both the
+// list and single-object compact paths. Centralised so the contract stays
+// in lockstep across both helpers.
+var compactVerboseFields = map[string]bool{
+	"description": true, "body": true, "content": true,
+	"comments": true, "attachments": true, "html": true, "markdown": true,
+}
+
 // compactFields keeps only the most important fields for agent consumption.
 // For arrays: allowlist of high-gravity fields (no descriptions).
 // For single objects: blocklist that strips known-verbose fields (descriptions, comments, etc.).
@@ -701,9 +709,23 @@ func compactFields(data json.RawMessage) json.RawMessage {
 }
 
 // compactListFields keeps only high-gravity fields for array responses.
-// When an item carries none of these keys the original is preserved, so
-// `--agent` does not silently emit {} for APIs whose response shapes use
-// domain-specific field names (travel: airline/destination; commerce: vendor).
+//
+// Two-layer keep rule:
+//
+//  1. A static allow-list covers canonical scalars (id/name/price/status/...).
+//  2. A data-driven extension also keeps any scalar key present in at least
+//     80% of input rows. This catches hand-written novel commands whose
+//     output keys (object_name, match_key, snippet) aren't on the canonical
+//     allow-list, without forcing every printed CLI to expand the list.
+//
+// Verbose fields (description, body, content, etc.) are excluded from the
+// data-driven extension regardless of frequency, so the compact intent
+// (short identifying values for agent consumption, not full prose) is
+// preserved.
+//
+// When an item still carries none of the keep keys, the original is
+// preserved so `--agent` does not silently emit {} for shapes whose key
+// names are entirely off-canonical.
 func compactListFields(items []map[string]any) json.RawMessage {
 	keepFields := map[string]bool{
 		// Identity
@@ -727,6 +749,31 @@ func compactListFields(items []map[string]any) json.RawMessage {
 		// Versioning
 		"version": true,
 	}
+	if len(items) > 0 {
+		keyCounts := map[string]int{}
+		for _, item := range items {
+			for k, v := range item {
+				if compactVerboseFields[k] || !isCompactScalar(v) {
+					continue
+				}
+				keyCounts[k]++
+			}
+		}
+		// ceil(len(items) * 0.8) without importing math. Capped at len-1 for
+		// len >= 2 so a single missing row cannot veto a key on small lists
+		// (without the cap, ceil(0.8*n) == n for n in {2,3,4}, which silently
+		// reintroduces the partial-strip bug whenever a heterogeneous 2-4 row
+		// response mixes one allow-list key with novel keys).
+		threshold := (len(items)*4 + 4) / 5
+		if len(items) >= 2 && threshold > len(items)-1 {
+			threshold = len(items) - 1
+		}
+		for k, count := range keyCounts {
+			if count >= threshold {
+				keepFields[k] = true
+			}
+		}
+	}
 
 	filtered := make([]map[string]any, 0, len(items))
 	for _, item := range items {
@@ -745,17 +792,26 @@ func compactListFields(items []map[string]any) json.RawMessage {
 	return result
 }
 
+// isCompactScalar reports whether v is a small primitive (string, number,
+// bool, null) suitable for --compact projection. Objects and arrays are
+// rejected: they almost always represent nested structure that defeats the
+// "short identifying value" intent of --compact, and including them in the
+// data-driven keep set would bloat agent output.
+func isCompactScalar(v any) bool {
+	switch v.(type) {
+	case nil, bool, float64, string:
+		return true
+	default:
+		return false
+	}
+}
+
 // compactObjectFields strips known-verbose fields from single-object responses.
 // Uses a blocklist so it works across all API domains (project management, payments, CRM, etc.).
 func compactObjectFields(obj map[string]any) json.RawMessage {
-	stripFields := map[string]bool{
-		"description": true, "body": true, "content": true,
-		"comments": true, "attachments": true, "html": true, "markdown": true,
-	}
-
 	compact := map[string]any{}
 	for k, v := range obj {
-		if !stripFields[k] {
+		if !compactVerboseFields[k] {
 			compact[k] = v
 		}
 	}
@@ -927,8 +983,7 @@ func printAutoTable(w io.Writer, items []map[string]any) error {
 	// Count scalar vs complex fields to decide format
 	scalarCount := 0
 	for _, v := range items[0] {
-		switch v.(type) {
-		case string, float64, bool, nil:
+		if isCompactScalar(v) {
 			scalarCount++
 		}
 	}

← 36976106 fix(cli): scope HTTP cache to <api>/http so invalidateCache  ·  back to Cli Printing Press  ·  fix(cli): accept backtick raw-string Use: in dogfood walkers 7ba9c20a →