Skip to content

Commit 5209a4a

Browse files
committed
fix: remove search_merged_prs tool, update appraisal flow
1 parent d32d664 commit 5209a4a

5 files changed

Lines changed: 674 additions & 92 deletions

File tree

mcp_server/tools/github_tools.py

Lines changed: 0 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -393,84 +393,6 @@ def list_branches(
393393
except Exception as e:
394394
raise ToolError(f"Failed to list branches: {str(e)}")
395395

396-
@mcp.tool(tags={"github", "prs"})
397-
def search_merged_prs(
398-
author: Optional[str] = Field(
399-
default=None,
400-
description="GitHub username to filter by. Defaults to authenticated user if not specified.",
401-
),
402-
days: int = Field(
403-
default=180,
404-
description="Number of days to look back (default: 180 for ~6 months)",
405-
),
406-
org: Optional[str] = Field(
407-
default=None,
408-
description="GitHub org to search within. If not specified, searches all accessible repos.",
409-
),
410-
repo: Optional[str] = Field(
411-
default=None,
412-
description="Specific repo in 'owner/repo' format (e.g., 'revolving-org/supabase'). Overrides org if specified.",
413-
),
414-
limit: int = Field(
415-
default=100,
416-
description="Maximum PRs to return (default: 100)",
417-
),
418-
detail_level: str = Field(
419-
default="summary",
420-
description="'summary' for minimal fields (number, title, merged_at, repo, owner, html_url, author), "
421-
"'full' adds body and labels. Use 'summary' for large result sets.",
422-
),
423-
) -> dict:
424-
"""
425-
Search for merged pull requests by author within a time period.
426-
427-
NOTE: For appraisals/performance reviews, use prepare_appraisal_data instead!
428-
It fetches all PRs with full stats in parallel and avoids context overflow.
429-
430-
This tool returns basic PR info without stats (additions, deletions).
431-
Use detail_level='summary' (default) for large result sets.
432-
433-
Requires QuickCall authentication with GitHub connected.
434-
"""
435-
try:
436-
client = _get_client()
437-
438-
# Calculate since_date from days
439-
from datetime import datetime, timedelta, timezone
440-
441-
since_date = (datetime.now(timezone.utc) - timedelta(days=days)).strftime(
442-
"%Y-%m-%d"
443-
)
444-
445-
# Use authenticated user if author not specified
446-
if not author:
447-
creds = get_credential_store().get_api_credentials()
448-
if creds and creds.github_username:
449-
author = creds.github_username
450-
451-
prs = client.search_merged_prs(
452-
author=author,
453-
since_date=since_date,
454-
org=org,
455-
repo=repo,
456-
limit=limit,
457-
detail_level=detail_level,
458-
)
459-
460-
return {
461-
"count": len(prs),
462-
"detail_level": detail_level,
463-
"period": f"Last {days} days",
464-
"author": author,
465-
"org": org,
466-
"repo": repo,
467-
"prs": prs,
468-
}
469-
except ToolError:
470-
raise
471-
except Exception as e:
472-
raise ToolError(f"Failed to search merged PRs: {str(e)}")
473-
474396
@mcp.tool(tags={"github", "prs", "appraisal"})
475397
def prepare_appraisal_data(
476398
author: Optional[str] = Field(

plugins/quickcall/commands/appraisal.md

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,30 +16,39 @@ Parse `$ARGUMENTS` for time period:
1616

1717
## Instructions
1818

19+
**IMPORTANT:** Only use these two MCP tools for appraisals:
20+
1. `prepare_appraisal_data` - fetches and dumps all PR data to a temp file
21+
2. `get_appraisal_pr_details` - reads specific PRs from that file (no API calls)
22+
23+
Do NOT use `get_prs`, `list_prs`, or any other tools - they will overflow context.
24+
25+
---
26+
1927
1. **Gather contribution data:**
2028

21-
**Option A - GitHub API (preferred if connected):**
22-
- Use `search_merged_prs` tool with parsed days
23-
- If user specifies an org, pass the `org` parameter
24-
- If user specifies a repo, pass the `repo` parameter (format: "owner/repo")
25-
- Default author is the authenticated user
29+
**Option A - GitHub API (preferred):**
30+
- Call `prepare_appraisal_data(days=X)` with the parsed time period
31+
- This fetches ALL merged PRs with full stats in PARALLEL
32+
- Dumps everything to a temp file (avoids context overflow)
33+
- Returns: `file_path` + list of `pr_titles` (number, title, repo only)
34+
- Optional: pass `org` or `repo` parameter to filter
2635

27-
**Option B - Local Git (fallback if not connected or for specific repo):**
36+
**Option B - Local Git (fallback):**
2837
- Use `get_local_contributions` tool on the current directory
2938
- This parses local git history for commits by the user
30-
- Extracts PR numbers from merge commit messages where available
3139

32-
2. **Analyze and categorize each PR:**
33-
Examine each PR's title, body, and labels to categorize:
40+
2. **Analyze and categorize PRs from the titles:**
41+
Review the `pr_titles` list returned in step 1 and categorize:
3442
- **Features**: New functionality (feat:, add:, implement, new, create)
3543
- **Enhancements**: Improvements (improve:, update:, perf:, optimize, enhance)
3644
- **Bug fixes**: (fix:, bugfix:, hotfix:, resolve, patch)
3745
- **Chores**: Maintenance work (docs:, test:, ci:, chore:, refactor:, bump)
3846

39-
3. **Identify top accomplishments:**
40-
- Filter out chores for the highlights
41-
- Sort features and enhancements by significance (based on title complexity, labels)
42-
- For top 3-5 PRs, call `get_pr` to fetch detailed metrics (additions, deletions, files)
47+
3. **Get details for top accomplishments:**
48+
- Pick the top 5-10 significant PRs from the categorized list
49+
- Call `get_appraisal_pr_details(file_path, [pr_numbers])`
50+
- This reads from the temp file - NO additional API calls
51+
- Returns full details: additions, deletions, files, body
4352

4453
4. **Calculate summary metrics:**
4554
- Total PRs merged by category

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "quickcall-integrations"
3-
version = "0.3.1"
3+
version = "0.3.2"
44
description = "MCP server with developer integrations for Claude Code and Cursor"
55
readme = "README.md"
66
requires-python = ">=3.10"
Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Integration test for appraisal tools.
4+
5+
Tests the actual MCP tool functions with real GitHub API calls.
6+
7+
Usage:
8+
uv run python tests/test_appraisal_integration.py
9+
"""
10+
11+
import json
12+
import os
13+
import sys
14+
15+
# Add parent dir to path for imports
16+
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
17+
18+
19+
def test_prepare_appraisal_data_tool():
20+
"""Test prepare_appraisal_data MCP tool with real API."""
21+
print("\n=== Integration Test: prepare_appraisal_data ===")
22+
23+
from mcp_server.auth import get_github_pat, get_credential_store
24+
from mcp_server.api_clients.github_client import GitHubClient
25+
26+
# Check auth
27+
pat_token, source = get_github_pat()
28+
store = get_credential_store()
29+
30+
if not pat_token and not store.is_authenticated():
31+
print("⚠️ No GitHub auth available, skipping integration test")
32+
return None
33+
34+
# Get client
35+
if pat_token:
36+
client = GitHubClient(token=pat_token)
37+
author = client.get_authenticated_user()
38+
print(f"✅ Using PAT ({source}), user: {author}")
39+
else:
40+
creds = store.get_api_credentials()
41+
client = GitHubClient(
42+
token=creds.github_token,
43+
default_owner=creds.github_username,
44+
installation_id=creds.github_installation_id,
45+
)
46+
author = creds.github_username
47+
print(f"✅ Using GitHub App, user: {author}")
48+
49+
# Step 1: Search merged PRs
50+
print("\n[Step 1] Searching merged PRs (last 30 days, limit 10)...")
51+
from datetime import datetime, timedelta, timezone
52+
53+
since_date = (datetime.now(timezone.utc) - timedelta(days=30)).strftime("%Y-%m-%d")
54+
55+
pr_list = client.search_merged_prs(
56+
author=author,
57+
since_date=since_date,
58+
limit=10,
59+
detail_level="full",
60+
)
61+
62+
if not pr_list:
63+
print("⚠️ No merged PRs found in last 30 days")
64+
return None
65+
66+
print(f"✅ Found {len(pr_list)} PRs")
67+
for pr in pr_list[:3]:
68+
print(f" - #{pr['number']}: {pr['title'][:50]}")
69+
70+
# Step 2: Fetch full details in parallel
71+
print("\n[Step 2] Fetching full PR details in parallel...")
72+
pr_refs = [
73+
{"owner": pr["owner"], "repo": pr["repo"], "number": pr["number"]}
74+
for pr in pr_list
75+
]
76+
77+
full_prs = client.fetch_prs_parallel(pr_refs, max_workers=5)
78+
print(f"✅ Fetched {len(full_prs)} PRs with full details")
79+
80+
if full_prs:
81+
pr = full_prs[0]
82+
print(
83+
f" Sample: #{pr['number']} - +{pr.get('additions', 0)}/-{pr.get('deletions', 0)}"
84+
)
85+
86+
# Step 3: Dump to file
87+
print("\n[Step 3] Dumping to file...")
88+
import tempfile
89+
90+
dump_data = {
91+
"author": author,
92+
"period": "Last 30 days",
93+
"fetched_at": datetime.now(timezone.utc).isoformat(),
94+
"count": len(full_prs),
95+
"prs": full_prs,
96+
}
97+
98+
fd, file_path = tempfile.mkstemp(suffix=".json", prefix="appraisal_integration_")
99+
with open(file_path, "w") as f:
100+
json.dump(dump_data, f, indent=2, default=str)
101+
102+
file_size = os.path.getsize(file_path)
103+
print(f"✅ Dumped to: {file_path}")
104+
print(f" Size: {file_size / 1024:.1f} KB")
105+
106+
# Step 4: Generate titles (what tool returns)
107+
print("\n[Step 4] Generating PR titles for Claude...")
108+
pr_titles = [
109+
{
110+
"number": pr["number"],
111+
"title": pr["title"],
112+
"repo": f"{pr.get('owner', '')}/{pr.get('repo', '')}",
113+
}
114+
for pr in full_prs
115+
]
116+
117+
print(f"✅ Generated {len(pr_titles)} titles")
118+
for t in pr_titles[:5]:
119+
print(f" - #{t['number']}: {t['title'][:50]}")
120+
121+
# Step 5: Test get_appraisal_pr_details
122+
print("\n[Step 5] Testing get_appraisal_pr_details...")
123+
if len(full_prs) >= 2:
124+
selected_numbers = [full_prs[0]["number"], full_prs[1]["number"]]
125+
126+
with open(file_path) as f:
127+
data = json.load(f)
128+
129+
pr_numbers_set = set(selected_numbers)
130+
selected_prs = [
131+
pr for pr in data.get("prs", []) if pr["number"] in pr_numbers_set
132+
]
133+
134+
print(f"✅ Retrieved {len(selected_prs)} selected PRs from dump")
135+
for pr in selected_prs:
136+
print(
137+
f" - #{pr['number']}: {pr['title'][:40]} (+{pr.get('additions', 0)}/-{pr.get('deletions', 0)})"
138+
)
139+
140+
# Cleanup
141+
os.unlink(file_path)
142+
print("\n✅ Integration test passed!")
143+
return True
144+
145+
146+
def test_response_size():
147+
"""Test that response sizes are reasonable."""
148+
print("\n=== Test: Response sizes ===")
149+
150+
# Simulate 100 PRs
151+
mock_pr_titles = [
152+
{
153+
"number": i,
154+
"title": f"PR title for #{i} - some description here",
155+
"repo": "org/repo",
156+
}
157+
for i in range(1, 101)
158+
]
159+
160+
# Calculate size of titles-only response
161+
titles_response = {
162+
"file_path": "/tmp/appraisal_xxx.json",
163+
"count": 100,
164+
"author": "testuser",
165+
"period": "Last 180 days",
166+
"pr_titles": mock_pr_titles,
167+
"next_step": "Call get_appraisal_pr_details...",
168+
}
169+
170+
titles_size = len(json.dumps(titles_response))
171+
print(f"✅ Titles-only response for 100 PRs: {titles_size / 1024:.1f} KB")
172+
173+
# Compare to full response (old way)
174+
mock_full_prs = [
175+
{
176+
"number": i,
177+
"title": f"PR title for #{i} - some description here",
178+
"body": "This is a longer description " * 10,
179+
"owner": "org",
180+
"repo": "repo",
181+
"additions": 100,
182+
"deletions": 50,
183+
"changed_files": 10,
184+
"labels": ["bug", "feature"],
185+
"merged_at": "2024-01-01T00:00:00Z",
186+
"html_url": f"https://github.com/org/repo/pull/{i}",
187+
}
188+
for i in range(1, 101)
189+
]
190+
191+
full_response = {
192+
"count": 100,
193+
"prs": mock_full_prs,
194+
}
195+
196+
full_size = len(json.dumps(full_response))
197+
print(f"✅ Full response for 100 PRs: {full_size / 1024:.1f} KB")
198+
print(f"✅ Reduction: {(1 - titles_size / full_size) * 100:.0f}%")
199+
200+
assert titles_size < full_size / 2, "Titles response should be <50% of full"
201+
print("✅ Test passed!\n")
202+
return True
203+
204+
205+
if __name__ == "__main__":
206+
print("=" * 60)
207+
print("Appraisal Tools - Integration Tests")
208+
print("=" * 60)
209+
210+
results = []
211+
212+
# Test response sizes first (no auth needed)
213+
try:
214+
results.append(("Response sizes", test_response_size()))
215+
except Exception as e:
216+
print(f"❌ Failed: {e}")
217+
results.append(("Response sizes", False))
218+
219+
# Test actual tool flow
220+
try:
221+
result = test_prepare_appraisal_data_tool()
222+
results.append(("prepare_appraisal_data", result))
223+
except Exception as e:
224+
print(f"❌ Failed: {e}")
225+
import traceback
226+
227+
traceback.print_exc()
228+
results.append(("prepare_appraisal_data", False))
229+
230+
print("\n" + "=" * 60)
231+
print("Results:")
232+
for name, passed in results:
233+
status = "✅ PASS" if passed else ("⚠️ SKIP" if passed is None else "❌ FAIL")
234+
print(f" {name}: {status}")
235+
print("=" * 60)

0 commit comments

Comments
 (0)