wire minimal types into list tools, remove collection handling from pipeline

Author: kerobbi

pkg/github/issues.go

@@ -1599,9 +1599,14 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
 				totalCount = fragment.TotalCount
 			}
 
-			optimizedIssues, err := response.OptimizeList(issues,
-				response.WithCollectionExtractors(map[string][]string{"labels": {"name"}}),
-			)
+			minimalIssues := make([]MinimalIssue, 0, len(issues))
+			for _, issue := range issues {
+				if issue != nil {
+					minimalIssues = append(minimalIssues, convertToMinimalIssue(issue))
+				}
+			}
+
+			optimizedIssues, err := response.OptimizeList(minimalIssues)
 			if err != nil {
 				return nil, nil, fmt.Errorf("failed to optimize issues: %w", err)
 			}

pkg/github/minimal_types.go

@@ -122,6 +122,12 @@ type MinimalBranch struct {
 	Protected bool   `json:"protected"`
 }
 
+// MinimalTag is the trimmed output type for tag objects.
+type MinimalTag struct {
+	Name string `json:"name"`
+	SHA  string `json:"sha"`
+}
+
 // MinimalResponse represents a minimal response for all CRUD operations.
 // Success is implicit in the HTTP response status, and all other information
 // can be derived from the URL or fetched separately if needed.
@@ -672,6 +678,32 @@ func convertToMinimalBranch(branch *github.Branch) MinimalBranch {
 	}
 }
 
+func convertToMinimalRelease(release *github.RepositoryRelease) MinimalRelease {
+	m := MinimalRelease{
+		ID:         release.GetID(),
+		TagName:    release.GetTagName(),
+		Name:       release.GetName(),
+		Body:       release.GetBody(),
+		HTMLURL:    release.GetHTMLURL(),
+		Prerelease: release.GetPrerelease(),
+		Draft:      release.GetDraft(),
+		Author:     convertToMinimalUser(release.GetAuthor()),
+	}
+
+	if release.PublishedAt != nil {
+		m.PublishedAt = release.PublishedAt.Format(time.RFC3339)
+	}
+
+	return m
+}
+
+func convertToMinimalTag(tag *github.RepositoryTag) MinimalTag {
+	return MinimalTag{
+		Name: tag.GetName(),
+		SHA:  tag.GetCommit().GetSHA(),
+	}
+}
+
 func convertToMinimalReviewThreadsResponse(query reviewThreadsQuery) MinimalReviewThreadsResponse {
 	threads := query.Repository.PullRequest.ReviewThreads
 

pkg/github/pullrequests.go

@@ -1149,13 +1149,15 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool
 				}
 			}
 
-			r, err := response.OptimizeList(prs,
+			minimalPRs := make([]MinimalPullRequest, 0, len(prs))
+			for _, pr := range prs {
+				if pr != nil {
+					minimalPRs = append(minimalPRs, convertToMinimalPullRequest(pr))
+				}
+			}
+
+			r, err := response.OptimizeList(minimalPRs,
 				response.WithPreservedFields("html_url", "draft"),
-				response.WithCollectionExtractors(map[string][]string{
-					"labels":              {"name"},
-					"requested_reviewers": {"login"},
-					"requested_teams":     {"name"},
-				}),
 			)
 			if err != nil {
 				return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil

pkg/github/repositories.go

@@ -1501,7 +1501,14 @@ func ListTags(t translations.TranslationHelperFunc) inventory.ServerTool {
 				return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list tags", resp, body), nil, nil
 			}
 
-			r, err := response.OptimizeList(tags)
+			minimalTags := make([]MinimalTag, 0, len(tags))
+			for _, tag := range tags {
+				if tag != nil {
+					minimalTags = append(minimalTags, convertToMinimalTag(tag))
+				}
+			}
+
+			r, err := response.OptimizeList(minimalTags)
 			if err != nil {
 				return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
 			}
@@ -1674,7 +1681,14 @@ func ListReleases(t translations.TranslationHelperFunc) inventory.ServerTool {
 				return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list releases", resp, body), nil, nil
 			}
 
-			r, err := response.OptimizeList(releases,
+			minimalReleases := make([]MinimalRelease, 0, len(releases))
+			for _, release := range releases {
+				if release != nil {
+					minimalReleases = append(minimalReleases, convertToMinimalRelease(release))
+				}
+			}
+
+			r, err := response.OptimizeList(minimalReleases,
 				response.WithPreservedFields("html_url", "prerelease"),
 			)
 			if err != nil {

pkg/github/repositories_test.go

@@ -2806,7 +2806,7 @@ func Test_ListTags(t *testing.T) {
 			require.Equal(t, len(tc.expectedTags), len(returnedTags))
 			for i, expectedTag := range tc.expectedTags {
 				assert.Equal(t, *expectedTag.Name, returnedTags[i]["name"])
-				assert.Equal(t, *expectedTag.Commit.SHA, returnedTags[i]["commit.sha"])
+				assert.Equal(t, *expectedTag.Commit.SHA, returnedTags[i]["sha"])
 			}
 		})
 	}

pkg/response/optimize.go

@@ -14,9 +14,8 @@ const (
 
 // OptimizeListConfig controls the optimization pipeline behavior.
 type OptimizeListConfig struct {
-	maxDepth             int
-	preservedFields      map[string]bool
-	collectionExtractors map[string][]string
+	maxDepth        int
+	preservedFields map[string]bool
 }
 
 type OptimizeListOption func(*OptimizeListConfig)
@@ -41,20 +40,8 @@ func WithPreservedFields(fields ...string) OptimizeListOption {
 	}
 }
 
-// WithCollectionExtractors controls how array fields are handled instead of being summarized as "[N items]".
-//   - 1 sub-field: comma-joined into a flat string ("bug, enhancement").
-//   - Multiple sub-fields: keep the array, but trim each element to only those fields.
-//
-// These are explicitly exempt from fill-rate filtering; if we asked for the extraction, it's likely important
-// to preserve the data even if only one item has it.
-func WithCollectionExtractors(extractors map[string][]string) OptimizeListOption {
-	return func(c *OptimizeListConfig) {
-		c.collectionExtractors = extractors
-	}
-}
-
 // OptimizeList optimizes a list of items by applying flattening, URL removal, zero-value removal,
-// whitespace normalization, collection summarization, and fill-rate filtering.
+// whitespace normalization, and fill-rate filtering.
 func OptimizeList[T any](items []T, opts ...OptimizeListOption) ([]byte, error) {
 	cfg := OptimizeListConfig{maxDepth: defaultMaxDepth}
 	for _, opt := range opts {
@@ -106,7 +93,7 @@ func flattenInto(item map[string]any, prefix string, result map[string]any, dept
 }
 
 // filterByFillRate drops keys that appear on less than the threshold proportion of items.
-// Preserved fields and extractor keys always survive.
+// Preserved fields always survive.
 func filterByFillRate(items []map[string]any, threshold float64, cfg OptimizeListConfig) []map[string]any {
 	keyCounts := make(map[string]int)
 	for _, item := range items {
@@ -118,8 +105,7 @@ func filterByFillRate(items []map[string]any, threshold float64, cfg OptimizeLis
 	minCount := int(threshold * float64(len(items)))
 	keepKeys := make(map[string]bool, len(keyCounts))
 	for key, count := range keyCounts {
-		_, hasExtractor := cfg.collectionExtractors[key]
-		if count > minCount || cfg.preservedFields[key] || hasExtractor {
+		if count > minCount || cfg.preservedFields[key] {
 			keepKeys[key] = true
 		}
 	}
@@ -138,7 +124,7 @@ func filterByFillRate(items []map[string]any, threshold float64, cfg OptimizeLis
 }
 
 // optimizeItem applies per-item strategies in a single pass: remove URLs,
-// remove zero-values, normalize whitespace, summarize collections.
+// remove zero-values, normalize whitespace.
 // Preserved fields skip everything except whitespace normalization.
 func optimizeItem(item map[string]any, cfg OptimizeListConfig) map[string]any {
 	result := make(map[string]any, len(item))
@@ -151,86 +137,16 @@ func optimizeItem(item map[string]any, cfg OptimizeListConfig) map[string]any {
 			continue
 		}
 
-		switch v := value.(type) {
-		case string:
-			result[key] = strings.Join(strings.Fields(v), " ")
-		case []any:
-			if len(v) == 0 {
-				continue
-			}
-
-			if preserved {
-				result[key] = value
-			} else if fields, ok := cfg.collectionExtractors[key]; ok {
-				if len(fields) == 1 {
-					result[key] = extractSubField(v, fields[0])
-				} else {
-					result[key] = trimArrayFields(v, fields)
-				}
-			} else {
-				result[key] = fmt.Sprintf("[%d items]", len(v))
-			}
-		default:
+		if s, ok := value.(string); ok {
+			result[key] = strings.Join(strings.Fields(s), " ")
+		} else {
 			result[key] = value
 		}
 	}
 
 	return result
 }
 
-// extractSubField pulls a named sub-field from each slice element and joins
-// them with ", ". Elements missing the field are silently skipped.
-func extractSubField(items []any, field string) string {
-	var vals []string
-	for _, item := range items {
-		m, ok := item.(map[string]any)
-		if !ok {
-			continue
-		}
-
-		v, ok := m[field]
-		if !ok || v == nil {
-			continue
-		}
-
-		switch s := v.(type) {
-		case string:
-			if s != "" {
-				vals = append(vals, s)
-			}
-		default:
-			vals = append(vals, fmt.Sprintf("%v", v))
-		}
-	}
-
-	return strings.Join(vals, ", ")
-}
-
-// trimArrayFields keeps only the specified fields from each object in a slice.
-// The trimmed objects are returned as is, no further strategies are applied.
-func trimArrayFields(items []any, fields []string) []any {
-	result := make([]any, 0, len(items))
-	for _, item := range items {
-		m, ok := item.(map[string]any)
-		if !ok {
-			continue
-		}
-
-		trimmed := make(map[string]any, len(fields))
-		for _, f := range fields {
-			if v, exists := m[f]; exists {
-				trimmed[f] = v
-			}
-		}
-
-		if len(trimmed) > 0 {
-			result = append(result, trimmed)
-		}
-	}
-
-	return result
-}
-
 // isURLKey matches "url", "*_url", and their dot-prefixed variants.
 func isURLKey(key string) bool {
 	if idx := strings.LastIndex(key, "."); idx >= 0 {

pkg/response/optimize_test.go

@@ -71,29 +71,6 @@ func TestFlatten(t *testing.T) {
 	})
 }
 
-func TestTrimArrayFields(t *testing.T) {
-	cfg := OptimizeListConfig{
-		collectionExtractors: map[string][]string{
-			"reviewers": {"login", "state"},
-		},
-	}
-
-	result := optimizeItem(map[string]any{
-		"reviewers": []any{
-			map[string]any{"login": "alice", "state": "approved", "id": float64(1)},
-			map[string]any{"login": "bob", "state": "changes_requested", "id": float64(2)},
-		},
-		"title": "Fix bug",
-	}, cfg)
-
-	expected := []any{
-		map[string]any{"login": "alice", "state": "approved"},
-		map[string]any{"login": "bob", "state": "changes_requested"},
-	}
-	assert.Equal(t, expected, result["reviewers"])
-	assert.Equal(t, "Fix bug", result["title"])
-}
-
 func TestFilterByFillRate(t *testing.T) {
 	cfg := OptimizeListConfig{}
 
@@ -144,7 +121,7 @@ func TestOptimizeList_AllStrategies(t *testing.T) {
 			"avatar_url": "https://avatars.githubusercontent.com/1",
 			"draft":      false,
 			"merged_at":  nil,
-			"labels":     []any{map[string]any{"name": "bug"}},
+			"labels":     []any{"bug", "fix"},
 			"user": map[string]any{
 				"login":      "user",
 				"avatar_url": "https://avatars.githubusercontent.com/1",
@@ -154,7 +131,6 @@ func TestOptimizeList_AllStrategies(t *testing.T) {
 
 	raw, err := OptimizeList(items,
 		WithPreservedFields("html_url", "draft"),
-		WithCollectionExtractors(map[string][]string{"labels": {"name"}}),
 	)
 	require.NoError(t, err)
 
@@ -167,7 +143,7 @@ func TestOptimizeList_AllStrategies(t *testing.T) {
 	assert.Equal(t, "line1 line2", result[0]["body"])
 	assert.Equal(t, "https://github.com/repo/1", result[0]["html_url"])
 	assert.Equal(t, false, result[0]["draft"])
-	assert.Equal(t, "bug", result[0]["labels"])
+	assert.Equal(t, []any{"bug", "fix"}, result[0]["labels"])
 	assert.Equal(t, "user", result[0]["user.login"])
 	assert.Nil(t, result[0]["url"])
 	assert.Nil(t, result[0]["avatar_url"])
@@ -233,40 +209,4 @@ func TestPreservedFields(t *testing.T) {
 		assert.Contains(t, result, "draft")
 		assert.NotContains(t, result, "body")
 	})
-
-	t.Run("protects from collection summarization", func(t *testing.T) {
-		cfg := OptimizeListConfig{
-			preservedFields: map[string]bool{"assignees": true},
-		}
-
-		assignees := []any{
-			map[string]any{"login": "alice", "id": float64(1)},
-			map[string]any{"login": "bob", "id": float64(2)},
-		}
-
-		result := optimizeItem(map[string]any{
-			"assignees": assignees,
-			"comments":  []any{map[string]any{"id": "1"}, map[string]any{"id": "2"}},
-		}, cfg)
-
-		assert.Equal(t, assignees, result["assignees"])
-		assert.Equal(t, "[2 items]", result["comments"])
-	})
-}
-
-func TestCollectionFieldExtractors_SurviveFillRate(t *testing.T) {
-	cfg := OptimizeListConfig{
-		collectionExtractors: map[string][]string{"labels": {"name"}},
-	}
-
-	items := []map[string]any{
-		{"title": "PR 1", "labels": "bug"},
-		{"title": "PR 2"},
-		{"title": "PR 3"},
-		{"title": "PR 4"},
-	}
-
-	result := filterByFillRate(items, defaultFillRateThreshold, cfg)
-
-	assert.Contains(t, result[0], "labels")
 }