# GitHub Support for review-bot

## Goal

AI code reviews on GitHub PRs using SAP AI Core as the LLM provider.

## Non-Goals

- Auto-detection of platform (explicit is fine)
- Single binary supporting both (two entry points OK)
- Unifying Gitea and GitHub into one abstraction layer

## Constraints

1. **Same features on both platforms** — anything review-bot does on Gitea should work on GitHub
2. **Testable** — small interfaces, dependency injection, no global state
3. **Verified** — the strat/review-bot github/ code was never tested; we build from scratch with tests

---

## Part 1: Feature Inventory

What does review-bot actually do?

### Core Review Flow

| Feature | Description | Used In |
|---------|-------------|---------|
| Get PR metadata | Title, body, head SHA, base ref | Review context |
| Get PR diff | Unified diff format | LLM input |
| Get PR files | List of changed files with status | Filtering, context |
| Get file content | Raw file at ref | Conventions, patterns |
| List directory | Enumerate files in path | Pattern repos |
| Post review | Body + inline comments + verdict | Output |

### Review Management

| Feature | Description | Used In |
|---------|-------------|---------|
| List reviews | Get existing reviews on PR | Supersede check |
| Delete review | Remove old review before re-posting | Supersede flow |
| Get authenticated user | Who am I? | Filter own reviews |

### Optional/Extended

| Feature | Description | Used In |
|---------|-------------|---------|
| Resolve comment | Mark inline comment resolved | Gitea-specific |
| Edit comment | Update existing comment | Supersede variant |
| Request reviewer | Add reviewer to PR | Workflow integration |
| Get commit statuses | CI status | Gate logic |

---

## Part 2: GitHub API Mapping

### Supported

| Feature | Gitea API | GitHub API | Notes |
|---------|-----------|------------|-------|
| Get PR | `GET /api/v1/repos/{owner}/{repo}/pulls/{n}` | `GET /repos/{owner}/{repo}/pulls/{n}` | Same structure |
| Get diff | `GET .../pulls/{n}.diff` | `GET .../pulls/{n}` + `Accept: application/vnd.github.diff` | Different mechanism |
| Get files | `GET .../pulls/{n}/files` | `GET .../pulls/{n}/files` | Same |
| Get file content | `GET .../raw/{path}?ref=` | `GET .../contents/{path}?ref=` + base64 decode | Different format |
| List directory | `GET .../contents/{path}` | `GET .../contents/{path}` | Same |
| Post review | `POST .../pulls/{n}/reviews` | `POST .../pulls/{n}/reviews` | Minor field differences |
| List reviews | `GET .../pulls/{n}/reviews` | `GET .../pulls/{n}/reviews` | Same |
| Delete review | `DELETE .../pulls/{n}/reviews/{id}` | `DELETE .../pulls/{n}/reviews/{id}` | Same |
| Get user | `GET /api/v1/user` | `GET /user` | Same |

### Gaps

| Feature | Gitea | GitHub | Workaround |
|---------|-------|--------|------------|
| Resolve comment | `POST .../pulls/comments/{id}/resolve` | No equivalent | Skip (document limitation) |
| Timeline API | `GET .../issues/{n}/timeline` | No equivalent | Use events API or skip |

---

## Part 3: Interface Design

Small, role-based interfaces. Test what you use.

```go
// pr.go — PR operations
type PRReader interface {
    GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error)
    GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error)
    GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error)
}

// files.go — File/content operations
type FileReader interface {
    GetFileContent(ctx context.Context, owner, repo, path, ref string) (string, error)
    ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error)
}

// review.go — Review operations
type Reviewer interface {
    PostReview(ctx context.Context, owner, repo string, number int, req ReviewRequest) (*Review, error)
    ListReviews(ctx context.Context, owner, repo string, number int) ([]Review, error)
    DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error
}

// identity.go — Auth/identity
type Identity interface {
    GetAuthenticatedUser(ctx context.Context) (string, error)
}
```

### Composite

```go
// Client combines all capabilities for callers that need everything
type Client interface {
    PRReader
    FileReader
    Reviewer
    Identity
}
```

### Types (shared)

```go
type PullRequest struct {
    Number  int
    Title   string
    Body    string
    HeadSHA string
    HeadRef string
    BaseRef string
}

type ChangedFile struct {
    Filename string
    Status   string // added, modified, removed, renamed
}

type ContentEntry struct {
    Name string
    Path string
    Type string // file, dir
}

type ReviewRequest struct {
    Event    string          // APPROVE, REQUEST_CHANGES, COMMENT
    Body     string
    Comments []ReviewComment
}

type ReviewComment struct {
    Path string
    Line int
    Body string
}

type Review struct {
    ID    int64
    User  string
    State string
    Body  string
}
```

---

## Part 4: Test Plan

### Unit Tests (mock HTTP)

Each interface method gets a test against a mock HTTP server.

```
github/
  pr_test.go        # TestGetPullRequest, TestGetDiff, TestGetFiles
  files_test.go     # TestGetFileContent, TestListContents
  review_test.go    # TestPostReview, TestListReviews, TestDeleteReview
  identity_test.go  # TestGetAuthenticatedUser
```

Test cases per method:
- Happy path
- 404 not found
- 401 unauthorized
- Rate limited (429)
- Malformed response

### Integration Tests (real GitHub)

Against github.com/aweiker/ai-core-review-bot:

```
integration/
  github_test.go    # +build integration
```

Requires `GITHUB_TOKEN` env var. Run manually or in CI with secrets.

| Test | Verifies |
|------|----------|
| Fetch PR #1 | PRReader works |
| Fetch README.md | FileReader works |
| Post + delete draft review | Reviewer works (clean up after) |
| Get authenticated user | Identity works |

### End-to-End Test

Open a real PR on the test repo, run review-bot against it, verify review appears.

---

## Part 5: Implementation Phases

### Phase 1: Types + Interfaces

**Files:**
- `vcs/types.go` — shared types
- `vcs/interfaces.go` — PRReader, FileReader, Reviewer, Identity, Client

**Exit criteria:** Compiles. No implementation yet.

**PR:** #1 on feature branch

---

### Phase 2: GitHub Client — PRReader

**Files:**
- `github/client.go` — struct, constructor, HTTP helpers
- `github/pr.go` — GetPullRequest, GetPullRequestDiff, GetPullRequestFiles
- `github/pr_test.go` — unit tests

**Exit criteria:** `go test ./github/...` passes for PR methods.

**PR:** #2

---

### Phase 3: GitHub Client — FileReader

**Files:**
- `github/files.go` — GetFileContent, ListContents
- `github/files_test.go`

**Exit criteria:** Unit tests pass.

**PR:** #3

---

### Phase 4: GitHub Client — Reviewer + Identity

**Files:**
- `github/review.go` — PostReview, ListReviews, DeleteReview
- `github/identity.go` — GetAuthenticatedUser
- `github/review_test.go`, `github/identity_test.go`

**Exit criteria:** Unit tests pass.

**PR:** #4

---

### Phase 5: Integration Tests

**Files:**
- `integration/github_test.go`

**Exit criteria:** All integration tests pass against real GitHub.

**PR:** #5

---

### Phase 6: Wire into cmd/review-bot

**Files:**
- `cmd/review-bot/main.go` — add `--provider github` flag
- Update to use vcs interfaces

**Exit criteria:**
- `./review-bot --provider github ...` works
- Existing Gitea path unchanged

**PR:** #6

---

### Phase 7: GitHub Actions Workflow

**Files:**
- `.github/workflows/ci.yml`
- `.github/workflows/review.yml`
- `.github/actions/review/action.yml`

**Exit criteria:** CI runs on github.com/aweiker/ai-core-review-bot, review posts successfully.

**PR:** #7

---

## Part 6: Open Questions

1. **Auth token format** — GitHub PAT vs fine-grained token vs GitHub App? Start with PAT.

2. **Rate limiting** — GitHub has stricter limits. Add retry with backoff? (Already have this for Gitea.)

3. **Pagination** — GitHub paginates at 100. Do we need >100 files/reviews? Probably not for MVP.

4. **Comment threading** — GitHub review comments work differently. Test and document.

5. **Draft reviews** — Use for integration tests to avoid noise?

---

## Summary

7 phases, each with clear exit criteria and a PR. Build from types up, test at each layer, integrate last.