Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve issue PR status tracking #38

Merged
merged 12 commits into from
Dec 10, 2024
1,314 changes: 252 additions & 1,062 deletions public/cache/bot-activities.json

Large diffs are not rendered by default.

74 changes: 67 additions & 7 deletions scripts/src/github-api.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { GitHubComment, GitHubIssue, GitHubPR, ApiResponse, Activity } from './types';
import type { GitHubComment, GitHubIssue, GitHubPR, GitHubPRResponse, ApiResponse, Activity, IssueStatus, PRStatus } from './types';
import fetch from 'node-fetch';
import { performance } from 'perf_hooks';

Expand All @@ -8,7 +8,7 @@ const REPO_NAME = 'OpenHands';

import fs from 'fs';

async function fetchWithAuth(url: string): Promise<ApiResponse> {
async function fetchWithAuth<T = unknown>(url: string): Promise<ApiResponse<T>> {
// Log the request
fs.appendFileSync('github-api.log', `\n[${new Date().toISOString()}] REQUEST: ${url}\n`);

Expand Down Expand Up @@ -56,9 +56,13 @@ async function fetchAllPages<T>(url: string): Promise<T[]> {
while (currentUrl !== '') {
pageCount++;
console.log(`Fetching page ${pageCount.toString()} from ${currentUrl}`);
const response = await fetchWithAuth(currentUrl);
console.log(`Got ${response.data.length.toString()} items`);
allItems.push(...(response.data as T[]));
const response = await fetchWithAuth<T>(currentUrl);
console.log(`Got ${Array.isArray(response.data) ? String(response.data.length) : '1'} items`);
if (Array.isArray(response.data)) {
allItems.push(...response.data);
} else {
allItems.push(response.data);
}
currentUrl = response.nextUrl ?? '';
}

Expand Down Expand Up @@ -112,6 +116,24 @@ export function isPRModificationFailureComment(comment: GitHubComment): boolean
return isFailureComment(comment);
}

async function checkPRStatus(prUrl: string): Promise<IssueStatus> {
try {
const response = await fetchWithAuth<GitHubPRResponse>(prUrl);
const pr = response.data as GitHubPRResponse;

if (pr?.merged) {
return 'pr_merged';
} else if (pr?.state === 'closed') {
return 'pr_closed';
} else {
return 'pr_open';
}
} catch (error) {
console.error('Error checking PR status:', error);
return 'no_pr';
}
}

async function processIssueComments(issue: GitHubIssue): Promise<Activity[]> {
const activities: Activity[] = [];
const comments = await fetchAllPages<GitHubComment>(issue.comments_url);
Expand All @@ -126,8 +148,45 @@ async function processIssueComments(issue: GitHubIssue): Promise<Activity[]> {
const resultComment = successComment ?? failureComment;

if (resultComment !== undefined) {
const status = successComment !== undefined ? 'success' : 'failure';
const timestamp = new Date(resultComment.created_at).toLocaleString();

// Extract PR URL from success comment if available
let prUrl: string | undefined;
let status: IssueStatus = 'no_pr';

if (successComment) {
// Try different PR reference formats
let prNumber: string | undefined;

// Try full PR URL format
const fullUrlMatch = successComment.body.match(/https:\/\/github\.com\/[^/]+\/[^/]+\/pull\/(\d+)/);
if (fullUrlMatch) {
prNumber = fullUrlMatch[1];
}

// Try pull/123 format
if (!prNumber) {
const pullMatch = successComment.body.match(/pull\/(\d+)/);
if (pullMatch) {
prNumber = pullMatch[1];
}
}

// Try #123 format when it refers to a PR
if (!prNumber) {
const hashMatch = successComment.body.match(/PR #(\d+)/i);
if (hashMatch) {
prNumber = hashMatch[1];
}
}

if (prNumber) {
prUrl = `https://api.github.com/repos/${REPO_OWNER}/${REPO_NAME}/pulls/${prNumber}`;
// Check PR status
status = await checkPRStatus(prUrl);
}
}

activities.push({
id: `issue-${String(issue.number)}-${String(comment.id)}`,
type: 'issue',
Expand All @@ -136,6 +195,7 @@ async function processIssueComments(issue: GitHubIssue): Promise<Activity[]> {
url: resultComment.html_url,
title: `ISSUE ${status} ${timestamp} -- ${issue.title}`,
description: issue.body.slice(0, 500) + (issue.body.length > 500 ? '...' : ''),
prUrl,
});
}
}
Expand All @@ -158,7 +218,7 @@ async function processPRComments(pr: GitHubPR): Promise<Activity[]> {
const resultComment = successComment ?? failureComment;

if (resultComment !== undefined) {
const status = successComment !== undefined ? 'success' : 'failure';
const status: PRStatus = successComment !== undefined ? 'success' : 'failure';
const timestamp = new Date(resultComment.created_at).toLocaleString();
activities.push({
id: `pr-${String(pr.number)}-${String(comment.id)}`,
Expand Down
15 changes: 12 additions & 3 deletions scripts/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,27 @@ export interface GitHubPR {
body?: string;
}

export interface ApiResponse {
data: any[];
export interface GitHubPRResponse {
state: string;
merged: boolean;
}

export interface ApiResponse<T = unknown> {
data: T | T[];
hasNextPage: boolean;
nextUrl: string | null;
}

export type IssueStatus = 'no_pr' | 'pr_open' | 'pr_merged' | 'pr_closed';
export type PRStatus = 'success' | 'failure';

export interface Activity {
id: string;
type: 'issue' | 'pr';
status: 'success' | 'failure';
status: IssueStatus | PRStatus;
timestamp: string;
url: string;
title: string;
description: string;
prUrl?: string;
}
17 changes: 17 additions & 0 deletions src/App.css
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,23 @@ input[type="date"]::-webkit-calendar-picker {
border-left: 4px solid var(--danger);
}

/* Issue-specific status colors */
.activity-item.no_pr {
border-left: 4px solid #ffffff;
}

.activity-item.pr_open {
border-left: 4px solid #4caf50;
}

.activity-item.pr_merged {
border-left: 4px solid #9c27b0;
}

.activity-item.pr_closed {
border-left: 4px solid #f44336;
}

.activity-header {
display: flex;
gap: 1rem;
Expand Down
1 change: 1 addition & 0 deletions src/__integration_tests__/github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ describe('GitHub Service Integration Tests', () => {
url: expect.stringMatching(/^https:\/\/github\.com\//),
title: expect.any(String),
description: expect.any(String),
prUrl: expect.any(String),
};

for (const activity of activities) {
Expand Down
145 changes: 137 additions & 8 deletions src/components/ActivityChart.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,92 @@ import { describe, it, expect, vi } from 'vitest';
import { ActivityChart } from './ActivityChart';
import { BotActivity } from '../types';

// Mock react-vega since it's a complex visualization component
// Mock VegaLite component and capture the spec prop
interface VegaLiteProps {
spec: {
data: { values: any[] };
encoding: {
color: { scale: { domain: string[]; range: string[] } };
x: any;
y: any;
};
title: { text: string; color: string };
};
}

const mockVegaLite = vi.fn();
vi.mock('react-vega', () => ({
VegaLite: vi.fn(() => <div data-testid="vega-lite-chart" />),
VegaLite: (props: VegaLiteProps) => {
mockVegaLite(props);
return <div data-testid="vega-lite-chart" />;
},
}));

function getLastVegaLiteProps(): VegaLiteProps {
expect(mockVegaLite).toHaveBeenCalled();
const lastCall = mockVegaLite.mock.calls[mockVegaLite.mock.calls.length - 1][0];
expect(lastCall).toBeDefined();
return lastCall;
}

describe('ActivityChart', () => {
const mockActivities: BotActivity[] = [
{
id: '1',
type: 'issue',
status: 'success',
status: 'no_pr',
timestamp: '2023-11-28T12:00:00Z',
url: 'https://github.com/example/1',
title: 'ISSUE success 11/28/2023, 12:00:00 PM -- Test Issue 1',
description: 'Successfully resolved issue',
title: 'Test Issue 1',
description: 'Issue without PR',
},
{
id: '2',
type: 'issue',
status: 'failure',
status: 'pr_open',
timestamp: '2023-11-28T13:00:00Z',
url: 'https://github.com/example/2',
title: 'ISSUE failure 11/28/2023, 1:00:00 PM -- Test Issue 2',
description: 'Failed to resolve issue',
title: 'Test Issue 2',
description: 'Issue with open PR',
prUrl: 'https://github.com/example/pr/2',
},
{
id: '3',
type: 'issue',
status: 'pr_merged',
timestamp: '2023-11-28T14:00:00Z',
url: 'https://github.com/example/3',
title: 'Test Issue 3',
description: 'Issue with merged PR',
prUrl: 'https://github.com/example/pr/3',
},
{
id: '4',
type: 'issue',
status: 'pr_closed',
timestamp: '2023-11-28T15:00:00Z',
url: 'https://github.com/example/4',
title: 'Test Issue 4',
description: 'Issue with closed PR',
prUrl: 'https://github.com/example/pr/4',
},
{
id: '5',
type: 'pr',
status: 'success',
timestamp: '2023-11-28T16:00:00Z',
url: 'https://github.com/example/5',
title: 'Test PR 1',
description: 'Successful PR',
},
{
id: '6',
type: 'pr',
status: 'failure',
timestamp: '2023-11-28T17:00:00Z',
url: 'https://github.com/example/6',
title: 'Test PR 2',
description: 'Failed PR',
},
];

Expand All @@ -37,4 +99,71 @@ describe('ActivityChart', () => {

expect(getByTestId('vega-lite-chart')).toBeInTheDocument();
});

it('filters activities by type', () => {
render(<ActivityChart activities={mockActivities} type="issue" />);
const lastCall = getLastVegaLiteProps();
const chartData = lastCall.spec.data.values;

// Should only include issue activities
expect(chartData).toHaveLength(4);
expect(chartData.every((d: { date: string; status: string }) =>
['no_pr', 'pr_open', 'pr_merged', 'pr_closed'].includes(d.status)
)).toBe(true);
});

it('configures issue color scale correctly', () => {
render(<ActivityChart activities={mockActivities} type="issue" />);
const lastCall = getLastVegaLiteProps();
const colorScale = lastCall.spec.encoding.color.scale;

expect(colorScale.domain).toEqual(['no_pr', 'pr_open', 'pr_merged', 'pr_closed']);
expect(colorScale.range).toEqual(['#ffffff', '#4caf50', '#9c27b0', '#f44336']);
});

it('configures PR color scale correctly', () => {
render(<ActivityChart activities={mockActivities} type="pr" />);
const lastCall = getLastVegaLiteProps();
const colorScale = lastCall.spec.encoding.color.scale;

expect(colorScale.domain).toEqual(['success', 'failure']);
expect(colorScale.range).toEqual(['#22c55e', '#ef4444']);
});

it('configures chart axes correctly', () => {
render(<ActivityChart activities={mockActivities} type="issue" />);
const lastCall = getLastVegaLiteProps();
const { x, y } = lastCall.spec.encoding;

expect(x.field).toBe('date');
expect(x.type).toBe('temporal');
expect(x.title).toBe('Date');
expect(x.axis).toEqual({
labelColor: '#C4CBDA',
titleColor: '#C4CBDA',
gridColor: '#3c3c4a',
domainColor: '#3c3c4a'
});

expect(y.aggregate).toBe('count');
expect(y.type).toBe('quantitative');
expect(y.title).toBe('Count');
expect(y.axis).toEqual({
labelColor: '#C4CBDA',
titleColor: '#C4CBDA',
gridColor: '#3c3c4a',
domainColor: '#3c3c4a'
});
});

it('configures chart title correctly', () => {
render(<ActivityChart activities={mockActivities} type="issue" />);
const lastCall = getLastVegaLiteProps();
const { title } = lastCall.spec;

expect(title).toEqual({
text: 'ISSUE Activity Over Time',
color: '#C4CBDA'
});
});
});
5 changes: 4 additions & 1 deletion src/components/ActivityChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ export function ActivityChart({ activities, type }: ActivityChartProps): React.J
field: 'status',
type: 'nominal',
title: 'Status',
scale: {
scale: type === 'issue' ? {
domain: ['no_pr', 'pr_open', 'pr_merged', 'pr_closed'],
range: ['#ffffff', '#4caf50', '#9c27b0', '#f44336'] // White, Green, Purple, Red
} : {
domain: ['success', 'failure'],
range: ['#22c55e', '#ef4444'] // Green for success, Red for failure
},
Expand Down
Loading
Loading