Skip to content

Commit c1386ff

Browse files
olaservoalmaleksia
andauthored
Apply suggestions from code review
Co-authored-by: Ksenia Bobrova <[email protected]>
1 parent e5645b9 commit c1386ff

File tree

2 files changed

+27
-31
lines changed

2 files changed

+27
-31
lines changed

pkg/github/instructions.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,15 @@ func GenerateInstructions(enabledToolsets []string) string {
2727
}
2828

2929
// Base instruction with context management
30-
baseInstruction := "The GitHub MCP Server provides GitHub API tools. Tool selection guidance: Use 'list_*' tools for broad, simple retrieval and pagination of all items of a type (e.g., all issues, all PRs, all branches) with basic filtering. Use 'search_*' tools for targeted queries with specific criteria, keywords, or complex filters (e.g., issues with certain text, PRs by author, code containing functions). Context management: 1) GitHub API responses can overflow context windows, 2) Process large datasets in batches of 5-10 items, 3) For summarization tasks, fetch minimal data first, then drill down into specifics."
30+
baseInstruction := `The GitHub MCP Server provides tools to interact with GitHub platform.
31+
32+
Tool selection guidance:
33+
1. Use 'list_*' tools for broad, simple retrieval and pagination of all items of a type (e.g., all issues, all PRs, all branches) with basic filtering.
34+
2. Use 'search_*' tools for targeted queries with specific criteria, keywords, or complex filters (e.g., issues with certain text, PRs by author, code containing functions).
35+
36+
Context management:
37+
1. Use pagination whenever possible with batches of 5-10 items.
38+
2. Use minimal_output parameter set to true if the full information is not needed to accomplish a task.
3139
3240
allInstructions := []string{baseInstruction}
3341
allInstructions = append(allInstructions, instructions...)
@@ -39,13 +47,22 @@ func GenerateInstructions(enabledToolsets []string) string {
3947
func getToolsetInstructions(toolset string) string {
4048
switch toolset {
4149
case "pull_requests":
42-
return "PR review workflow: Always use 'create_pending_pull_request_review' → 'add_comment_to_pending_review' → 'submit_pending_pull_request_review' for complex reviews with line-specific comments."
50+
return `## Pull Requests
51+
52+
PR review workflow: Always use 'create_pending_pull_request_review' → 'add_comment_to_pending_review' → 'submit_pending_pull_request_review' for complex reviews with line-specific comments.
53+
`
4354
case "issues":
44-
return "Issue workflow: Check 'list_issue_types' first for organizations to use proper issue types. Use 'search_issues' before creating new issues to avoid duplicates. Always set 'state_reason' when closing issues."
55+
return `## Issues
56+
57+
Check 'list_issue_types' first for organizations to use proper issue types. Use 'search_issues' before creating new issues to avoid duplicates. Always set 'state_reason' when closing issues.
58+
`
4559
case "notifications":
4660
return "Notifications: Filter by 'participating' for issues/PRs you're involved in. Use 'mark_all_notifications_read' with repository filters to avoid marking unrelated notifications."
4761
case "discussions":
48-
return "Discussions: Use 'list_discussion_categories' to understand available categories before creating discussions. Filter by category for better organization."
62+
return `## Discussions
63+
64+
Use 'list_discussion_categories' to understand available categories before creating discussions. Filter by category for better organization.
65+
`
4966
default:
5067
return ""
5168
}

pkg/github/instructions_test.go

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -34,53 +34,32 @@ func TestGenerateInstructions(t *testing.T) {
3434
{
3535
name: "pull requests toolset",
3636
enabledToolsets: []string{"pull_requests"},
37-
expectedContains: []string{
38-
"create_pending_pull_request_review",
39-
"add_comment_to_pending_review",
40-
"submit_pending_pull_request_review",
41-
},
37+
expectedContains: []string{"## Pull Requests"},
4238
},
4339
{
4440
name: "issues toolset",
4541
enabledToolsets: []string{"issues"},
46-
expectedContains: []string{
47-
"search_issues",
48-
"list_issue_types",
49-
"state_reason",
50-
},
51-
},
52-
{
53-
name: "notifications toolset",
54-
enabledToolsets: []string{"notifications"},
55-
expectedContains: []string{
56-
"participating",
57-
"mark_all_notifications_read",
58-
"repository filters",
59-
},
42+
expectedContains: []string{"## Issues"},
6043
},
6144
{
6245
name: "discussions toolset",
6346
enabledToolsets: []string{"discussions"},
64-
expectedContains: []string{
65-
"list_discussion_categories",
66-
"Filter by category",
67-
},
47+
expectedContains: []string{"## Discussions"},
6848
},
6949
{
7050
name: "multiple toolsets (context + pull_requests)",
7151
enabledToolsets: []string{"context", "pull_requests"},
7252
expectedContains: []string{
7353
"get_me",
74-
"create_pending_pull_request_review",
54+
"## Pull Requests",
7555
},
7656
},
7757
{
7858
name: "multiple toolsets (issues + pull_requests)",
7959
enabledToolsets: []string{"issues", "pull_requests"},
8060
expectedContains: []string{
81-
"search_issues",
82-
"list_issue_types",
83-
"create_pending_pull_request_review",
61+
"## Issues",
62+
"## Pull Requests",
8463
},
8564
},
8665
}

0 commit comments

Comments
 (0)