Skip to content

Commit 0a21966

Browse files
perf(inventory): cache extracted toolset IDs in sort comparator
Avoid evaluating the extractor closures up to three times per comparison. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent be14b88 commit 0a21966

2 files changed

Lines changed: 17 additions & 19 deletions

File tree

pkg/github/csv_output.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ type csvOutputDocument struct {
4545
// withCSVOutput wraps the handler of every default-toolset list_* tool so that,
4646
// at request time, it checks the csv_output feature flag and converts the JSON
4747
// text response to CSV when enabled. The tool's schema, name, and scope are
48-
// unchanged — only the response payload format differs — so a single tool with
49-
// a runtime check is sufficient (no dual feature-gated variants needed).
48+
// unchanged — only the response payload format differs.
5049
func withCSVOutput(tools []inventory.ServerTool) []inventory.ServerTool {
5150
for i := range tools {
5251
if !isCSVOutputTool(tools[i]) {

pkg/inventory/filters.go

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,25 +44,23 @@ func (r *Inventory) checkFeatureFlag(ctx context.Context, flagName string) bool
4444
// - If FeatureFlagEnable is set, the item is only allowed if the flag is enabled.
4545
// - If FeatureFlagDisable is set, the item is excluded if the flag is enabled.
4646
func featureFlagAllowed(ctx context.Context, checker FeatureFlagChecker, enableFlag, disableFlag string) bool {
47-
if enableFlag != "" {
48-
enabled, err := checker(ctx, enableFlag)
47+
// Error semantics match the previous checkFeatureFlag helper: a checker
48+
// error is logged and treated as "flag not enabled". So an enable-flag
49+
// check on error excludes the tool, but a disable-flag check on error
50+
// keeps it (the disable condition wasn't met).
51+
check := func(flag string) bool {
52+
enabled, err := checker(ctx, flag)
4953
if err != nil {
50-
fmt.Fprintf(os.Stderr, "Feature flag check error for %q: %v\n", enableFlag, err)
51-
return false
52-
}
53-
if !enabled {
54+
fmt.Fprintf(os.Stderr, "Feature flag check error for %q: %v\n", flag, err)
5455
return false
5556
}
57+
return enabled
5658
}
57-
if disableFlag != "" {
58-
enabled, err := checker(ctx, disableFlag)
59-
if err != nil {
60-
fmt.Fprintf(os.Stderr, "Feature flag check error for %q: %v\n", disableFlag, err)
61-
return false
62-
}
63-
if enabled {
64-
return false
65-
}
59+
if enableFlag != "" && !check(enableFlag) {
60+
return false
61+
}
62+
if disableFlag != "" && check(disableFlag) {
63+
return false
6664
}
6765
return true
6866
}
@@ -130,8 +128,9 @@ func (r *Inventory) isToolEnabled(ctx context.Context, tool *ServerTool) bool {
130128
// prompts).
131129
func sortByToolsetThenName[T any](items []T, toolsetID func(T) ToolsetID, name func(T) string) {
132130
sort.Slice(items, func(i, j int) bool {
133-
if toolsetID(items[i]) != toolsetID(items[j]) {
134-
return toolsetID(items[i]) < toolsetID(items[j])
131+
idI, idJ := toolsetID(items[i]), toolsetID(items[j])
132+
if idI != idJ {
133+
return idI < idJ
135134
}
136135
return name(items[i]) < name(items[j])
137136
})

0 commit comments

Comments
 (0)