Skip to content

Commit a7f3373

Browse files
committed
address feedback: add nil guard, add protected to preserved fields, add depth drop test, review error handling
1 parent f21b014 commit a7f3373

File tree

4 files changed

+28
-3
lines changed

4 files changed

+28
-3
lines changed

pkg/github/issues.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1613,7 +1613,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
16131613

16141614
optimizedIssues, err := response.OptimizeList(minimalIssues)
16151615
if err != nil {
1616-
return nil, nil, fmt.Errorf("failed to optimize issues: %w", err)
1616+
return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
16171617
}
16181618

16191619
// Wrap optimized issues with pagination metadata

pkg/github/repositories.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,9 @@ func ListBranches(t translations.TranslationHelperFunc) inventory.ServerTool {
307307
minimalBranches = append(minimalBranches, convertToMinimalBranch(branch))
308308
}
309309

310-
r, err := response.OptimizeList(minimalBranches)
310+
r, err := response.OptimizeList(minimalBranches,
311+
response.WithPreservedFields("protected"),
312+
)
311313
if err != nil {
312314
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
313315
}

pkg/response/optimize.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ func WithPreservedFields(fields ...string) OptimizeListOption {
4545
// OptimizeList optimizes a list of items by applying flattening, URL removal, zero-value removal,
4646
// whitespace normalization, and fill-rate filtering.
4747
func OptimizeList[T any](items []T, opts ...OptimizeListOption) ([]byte, error) {
48+
if items == nil {
49+
return []byte("[]"), nil
50+
}
51+
4852
cfg := OptimizeListConfig{maxDepth: defaultMaxDepth}
4953
for _, opt := range opts {
5054
opt(&cfg)
@@ -83,6 +87,7 @@ func flattenTo(item map[string]any, maxDepth int) map[string]any {
8387
}
8488

8589
// flattenInto is the recursive worker for flattenTo.
90+
// Nested maps at maxDepth are intentionally dropped, as deeply nested data is not useful in list responses.
8691
func flattenInto(item map[string]any, prefix string, result map[string]any, depth int, maxDepth int) {
8792
for key, value := range item {
8893
fullKey := prefix + key

pkg/response/optimize_test.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,24 @@ func TestFlatten(t *testing.T) {
6969
"commit.author.date": "2026-01-01",
7070
}, result)
7171
})
72+
73+
t.Run("drops nested maps at maxDepth boundary", func(t *testing.T) {
74+
input := map[string]any{
75+
"title": "issue",
76+
"user": map[string]any{
77+
"login": "alice",
78+
"org": map[string]any{
79+
"name": "acme",
80+
},
81+
},
82+
}
83+
result := flattenTo(input, defaultMaxDepth)
84+
85+
assert.Equal(t, "issue", result["title"])
86+
assert.Equal(t, "alice", result["user.login"])
87+
assert.Nil(t, result["user.org"], "nested map at maxDepth is dropped")
88+
assert.Nil(t, result["user.org.name"], "nested map contents at maxDepth are dropped")
89+
})
7290
}
7391

7492
func TestFilterByFillRate(t *testing.T) {
@@ -153,7 +171,7 @@ func TestOptimizeList_AllStrategies(t *testing.T) {
153171
func TestOptimizeList_NilInput(t *testing.T) {
154172
raw, err := OptimizeList[map[string]any](nil)
155173
require.NoError(t, err)
156-
assert.Equal(t, "null", string(raw))
174+
assert.Equal(t, "[]", string(raw))
157175
}
158176

159177
func TestOptimizeList_SkipsFillRateBelowMinRows(t *testing.T) {

0 commit comments

Comments
 (0)