From 8a0eed298a666dcd19c026c17106581f8edf5c68 Mon Sep 17 00:00:00 2001
From: claw <claw@weiker.me>
Date: Tue, 12 May 2026 13:30:26 -0700
Subject: [PATCH 1/4] feat(vcs): Gitea adapter with diff-position translation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Implements the Gitea adapter (gitea.Adapter) that satisfies vcs.Client.

Key components:
- gitea/adapter.go: Adapter struct wrapping *Client with all vcs.Client methods
- gitea/position.go: BuildPositionToLineMap for diff-position → line translation
- gitea/adapter_test.go: Tests for all mapping methods and event translation
- gitea/position_test.go: Tests for position translation edge cases

Translation details:
- ReviewEvent: APPROVE → APPROVED (Gitea-native)
- PostReview: fetches diff, builds position map, translates each comment
- Deletion-targeted positions map to nearest non-deletion line below
- All field-mapping methods tested (GetPullRequest, GetPullRequestFiles,
  ListReviews, GetCommitStatuses, ListContents)

Also:
- Added Base field to gitea.PullRequest struct
- Updated conformance tests to assert Adapter (not raw Client) satisfies vcs.Client
- Removed phase2 build tag from conformance tests

Closes #79
---
 gitea/adapter.go          | 225 ++++++++++++++++++++++++
 gitea/adapter_test.go     | 356 ++++++++++++++++++++++++++++++++++++++
 gitea/client.go           |   3 +
 gitea/conformance_test.go |  21 +--
 gitea/position.go         | 182 +++++++++++++++++++
 gitea/position_test.go    | 301 ++++++++++++++++++++++++++++++++
 vcs/check_test.go         |  24 +--
 7 files changed, 1074 insertions(+), 38 deletions(-)
 create mode 100644 gitea/adapter.go
 create mode 100644 gitea/adapter_test.go
 create mode 100644 gitea/position.go
 create mode 100644 gitea/position_test.go

diff --git a/gitea/adapter.go b/gitea/adapter.go
new file mode 100644
index 0000000..20806b7
--- /dev/null
+++ b/gitea/adapter.go
@@ -0,0 +1,225 @@
+package gitea
+
+import (
+	"context"
+	"fmt"
+
+	"gitea.weiker.me/rodin/review-bot/vcs"
+)
+
+// Adapter wraps a gitea.Client and satisfies the vcs.Client interface.
+// It handles translation between GitHub-canonical diff positions and Gitea
+// line numbers, and between canonical review event strings and Gitea-native values.
+type Adapter struct {
+	client *Client
+}
+
+// Compile-time interface conformance assertion.
+var _ vcs.Client = (*Adapter)(nil)
+
+// NewAdapter creates a new Adapter wrapping the given gitea Client.
+func NewAdapter(client *Client) *Adapter {
+	return &Adapter{client: client}
+}
+
+// Underlying returns the wrapped gitea.Client for Gitea-specific operations
+// that have no vcs.Client equivalent (resolve comment, timeline, supersede flow).
+func (a *Adapter) Underlying() *Client {
+	return a.client
+}
+
+// --- PRReader ---
+
+// GetPullRequest maps gitea.PullRequest to vcs.PullRequest.
+func (a *Adapter) GetPullRequest(ctx context.Context, owner, repo string, number int) (*vcs.PullRequest, error) {
+	pr, err := a.client.GetPullRequest(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	return &vcs.PullRequest{
+		Number: number,
+		Title:  pr.Title,
+		Body:   pr.Body,
+		Head: vcs.HeadRef{
+			SHA: pr.Head.Sha,
+			Ref: pr.Head.Ref,
+		},
+		Base: vcs.BaseRef{
+			Ref: pr.Base.Ref,
+		},
+	}, nil
+}
+
+// GetPullRequestDiff is a pass-through to the underlying client.
+func (a *Adapter) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) {
+	return a.client.GetPullRequestDiff(ctx, owner, repo, number)
+}
+
+// GetPullRequestFiles maps []gitea.ChangedFile to []vcs.ChangedFile.
+// Patch is set to empty string since Gitea's /pulls/{n}/files does not return patch text.
+func (a *Adapter) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]vcs.ChangedFile, error) {
+	files, err := a.client.GetPullRequestFiles(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]vcs.ChangedFile, len(files))
+	for i, f := range files {
+		result[i] = vcs.ChangedFile{
+			Filename: f.Filename,
+			Status:   f.Status,
+		}
+	}
+	return result, nil
+}
+
+// GetFileContentAtRef is a pass-through to the underlying client.
+func (a *Adapter) GetFileContentAtRef(ctx context.Context, owner, repo, path, ref string) (string, error) {
+	return a.client.GetFileContentAtRef(ctx, owner, repo, path, ref)
+}
+
+// GetCommitStatuses maps []gitea.CommitStatus to []vcs.CommitStatus.
+func (a *Adapter) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]vcs.CommitStatus, error) {
+	statuses, err := a.client.GetCommitStatuses(ctx, owner, repo, sha)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]vcs.CommitStatus, len(statuses))
+	for i, s := range statuses {
+		result[i] = vcs.CommitStatus{
+			Status:      s.Status,
+			Context:     s.Context,
+			Description: s.Description,
+			TargetURL:   s.TargetURL,
+		}
+	}
+	return result, nil
+}
+
+// --- FileReader ---
+
+// GetFileContent delegates to the underlying client, routing to the ref-aware
+// variant when ref is non-empty.
+func (a *Adapter) GetFileContent(ctx context.Context, owner, repo, path, ref string) (string, error) {
+	if ref != "" {
+		return a.client.GetFileContentRef(ctx, owner, repo, path, ref)
+	}
+	return a.client.GetFileContent(ctx, owner, repo, path)
+}
+
+// ListContents maps []gitea.ContentEntry to []vcs.ContentEntry.
+func (a *Adapter) ListContents(ctx context.Context, owner, repo, path string) ([]vcs.ContentEntry, error) {
+	entries, err := a.client.ListContents(ctx, owner, repo, path)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]vcs.ContentEntry, len(entries))
+	for i, e := range entries {
+		result[i] = vcs.ContentEntry{
+			Name: e.Name,
+			Path: e.Path,
+			Type: e.Type,
+		}
+	}
+	return result, nil
+}
+
+// --- Reviewer ---
+
+// translateEvent translates a vcs.ReviewEvent (GitHub-canonical) to a Gitea-native event string.
+func translateEvent(event vcs.ReviewEvent) string {
+	switch event {
+	case vcs.ReviewEventApprove:
+		return "APPROVED"
+	case vcs.ReviewEventRequestChanges:
+		return "REQUEST_CHANGES"
+	case vcs.ReviewEventComment:
+		return "COMMENT"
+	default:
+		return string(event)
+	}
+}
+
+// PostReview translates vcs.ReviewRequest to the Gitea-native format.
+// It fetches the PR diff, builds a position-to-line map, and translates each
+// ReviewComment.Position (GitHub diff-position) to a Gitea new_position (line number).
+func (a *Adapter) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*vcs.Review, error) {
+	event := translateEvent(req.Event)
+
+	var giteaComments []ReviewComment
+	if len(req.Comments) > 0 {
+		// Fetch diff to build position → line number map
+		diff, err := a.client.GetPullRequestDiff(ctx, owner, repo, number)
+		if err != nil {
+			return nil, fmt.Errorf("fetch diff for position translation: %w", err)
+		}
+
+		posMap, err := BuildPositionToLineMap(diff)
+		if err != nil {
+			return nil, fmt.Errorf("build position map: %w", err)
+		}
+
+		for _, c := range req.Comments {
+			lineNum, err := posMap.Translate(c.Path, c.Position)
+			if err != nil {
+				return nil, fmt.Errorf("translate position %d in %s: %w", c.Position, c.Path, err)
+			}
+			giteaComments = append(giteaComments, ReviewComment{
+				Path:        c.Path,
+				NewPosition: int64(lineNum),
+				Body:        c.Body,
+			})
+		}
+	}
+
+	review, err := a.client.PostReview(ctx, owner, repo, number, event, req.Body, giteaComments)
+	if err != nil {
+		return nil, err
+	}
+
+	return &vcs.Review{
+		ID:       review.ID,
+		Body:     review.Body,
+		User:     vcs.UserInfo{Login: review.User.Login},
+		State:    review.State,
+		Stale:    review.Stale,
+		CommitID: review.CommitID,
+	}, nil
+}
+
+// ListReviews maps []gitea.Review to []vcs.Review.
+func (a *Adapter) ListReviews(ctx context.Context, owner, repo string, number int) ([]vcs.Review, error) {
+	reviews, err := a.client.ListReviews(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]vcs.Review, len(reviews))
+	for i, r := range reviews {
+		result[i] = vcs.Review{
+			ID:       r.ID,
+			Body:     r.Body,
+			User:     vcs.UserInfo{Login: r.User.Login},
+			State:    r.State,
+			Stale:    r.Stale,
+			CommitID: r.CommitID,
+		}
+	}
+	return result, nil
+}
+
+// DeleteReview is a pass-through to the underlying client.
+func (a *Adapter) DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error {
+	return a.client.DeleteReview(ctx, owner, repo, number, reviewID)
+}
+
+// DismissReview deletes the review. Gitea supports full deletion of any review state.
+// The message parameter is intentionally unused — Gitea deletion has no dismissal message.
+func (a *Adapter) DismissReview(ctx context.Context, owner, repo string, number int, reviewID int64, message string) error {
+	return a.client.DeleteReview(ctx, owner, repo, number, reviewID)
+}
+
+// --- Identity ---
+
+// GetAuthenticatedUser is a pass-through to the underlying client.
+func (a *Adapter) GetAuthenticatedUser(ctx context.Context) (string, error) {
+	return a.client.GetAuthenticatedUser(ctx)
+}
diff --git a/gitea/adapter_test.go b/gitea/adapter_test.go
new file mode 100644
index 0000000..380e643
--- /dev/null
+++ b/gitea/adapter_test.go
@@ -0,0 +1,356 @@
+package gitea_test
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"gitea.weiker.me/rodin/review-bot/gitea"
+	"gitea.weiker.me/rodin/review-bot/vcs"
+)
+
+func TestAdapter_GetPullRequest(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(map[string]any{
+			"title": "Test PR",
+			"body":  "PR body",
+			"head": map[string]any{
+				"sha": "abc123",
+				"ref": "feature-branch",
+			},
+			"base": map[string]any{
+				"ref": "main",
+			},
+		})
+	}))
+	defer server.Close()
+
+	client := gitea.NewClient(server.URL, "token")
+	adapter := gitea.NewAdapter(client)
+
+	pr, err := adapter.GetPullRequest(context.Background(), "owner", "repo", 42)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if pr.Number != 42 {
+		t.Errorf("Number = %d, want 42", pr.Number)
+	}
+	if pr.Title != "Test PR" {
+		t.Errorf("Title = %q, want %q", pr.Title, "Test PR")
+	}
+	if pr.Body != "PR body" {
+		t.Errorf("Body = %q, want %q", pr.Body, "PR body")
+	}
+	if pr.Head.SHA != "abc123" {
+		t.Errorf("Head.SHA = %q, want %q", pr.Head.SHA, "abc123")
+	}
+	if pr.Head.Ref != "feature-branch" {
+		t.Errorf("Head.Ref = %q, want %q", pr.Head.Ref, "feature-branch")
+	}
+	if pr.Base.Ref != "main" {
+		t.Errorf("Base.Ref = %q, want %q", pr.Base.Ref, "main")
+	}
+}
+
+func TestAdapter_GetPullRequestFiles(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode([]map[string]any{
+			{"filename": "main.go", "status": "modified"},
+			{"filename": "new.go", "status": "added"},
+		})
+	}))
+	defer server.Close()
+
+	client := gitea.NewClient(server.URL, "token")
+	adapter := gitea.NewAdapter(client)
+
+	files, err := adapter.GetPullRequestFiles(context.Background(), "owner", "repo", 1)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(files) != 2 {
+		t.Fatalf("got %d files, want 2", len(files))
+	}
+	if files[0].Filename != "main.go" || files[0].Status != "modified" {
+		t.Errorf("files[0] = %+v", files[0])
+	}
+	if files[1].Filename != "new.go" || files[1].Status != "added" {
+		t.Errorf("files[1] = %+v", files[1])
+	}
+}
+
+func TestAdapter_ListReviews(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode([]map[string]any{
+			{
+				"id":        1,
+				"body":      "LGTM",
+				"user":      map[string]any{"login": "reviewer1"},
+				"state":     "APPROVED",
+				"stale":     false,
+				"commit_id": "abc123",
+			},
+			{
+				"id":        2,
+				"body":      "Needs work",
+				"user":      map[string]any{"login": "reviewer2"},
+				"state":     "REQUEST_CHANGES",
+				"stale":     true,
+				"commit_id": "def456",
+			},
+		})
+	}))
+	defer server.Close()
+
+	client := gitea.NewClient(server.URL, "token")
+	adapter := gitea.NewAdapter(client)
+
+	reviews, err := adapter.ListReviews(context.Background(), "owner", "repo", 1)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(reviews) != 2 {
+		t.Fatalf("got %d reviews, want 2", len(reviews))
+	}
+	if reviews[0].ID != 1 || reviews[0].Body != "LGTM" || reviews[0].User.Login != "reviewer1" {
+		t.Errorf("reviews[0] = %+v", reviews[0])
+	}
+	if reviews[0].State != "APPROVED" || reviews[0].Stale || reviews[0].CommitID != "abc123" {
+		t.Errorf("reviews[0] state/stale/commit = %v/%v/%v", reviews[0].State, reviews[0].Stale, reviews[0].CommitID)
+	}
+	if reviews[1].ID != 2 || !reviews[1].Stale || reviews[1].State != "REQUEST_CHANGES" {
+		t.Errorf("reviews[1] = %+v", reviews[1])
+	}
+}
+
+func TestAdapter_GetCommitStatuses(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode([]map[string]any{
+			{
+				"status":      "success",
+				"context":     "ci/test",
+				"description": "All tests pass",
+				"target_url":  "https://ci.example.com/1",
+			},
+		})
+	}))
+	defer server.Close()
+
+	client := gitea.NewClient(server.URL, "token")
+	adapter := gitea.NewAdapter(client)
+
+	statuses, err := adapter.GetCommitStatuses(context.Background(), "owner", "repo", "abc123")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(statuses) != 1 {
+		t.Fatalf("got %d statuses, want 1", len(statuses))
+	}
+	if statuses[0].Status != "success" {
+		t.Errorf("Status = %q, want %q", statuses[0].Status, "success")
+	}
+	if statuses[0].Context != "ci/test" {
+		t.Errorf("Context = %q, want %q", statuses[0].Context, "ci/test")
+	}
+	if statuses[0].Description != "All tests pass" {
+		t.Errorf("Description = %q, want %q", statuses[0].Description, "All tests pass")
+	}
+	if statuses[0].TargetURL != "https://ci.example.com/1" {
+		t.Errorf("TargetURL = %q, want %q", statuses[0].TargetURL, "https://ci.example.com/1")
+	}
+}
+
+func TestAdapter_PostReview_EventTranslation(t *testing.T) {
+	tests := []struct {
+		name      string
+		event     vcs.ReviewEvent
+		wantEvent string
+	}{
+		{"APPROVE becomes APPROVED", vcs.ReviewEventApprove, "APPROVED"},
+		{"REQUEST_CHANGES stays", vcs.ReviewEventRequestChanges, "REQUEST_CHANGES"},
+		{"COMMENT stays", vcs.ReviewEventComment, "COMMENT"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var gotEvent string
+			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				var payload struct {
+					Event string `json:"event"`
+				}
+				json.NewDecoder(r.Body).Decode(&payload)
+				gotEvent = payload.Event
+				json.NewEncoder(w).Encode(map[string]any{
+					"id":   1,
+					"body": "test",
+					"user": map[string]any{"login": "bot"},
+				})
+			}))
+			defer server.Close()
+
+			client := gitea.NewClient(server.URL, "token")
+			adapter := gitea.NewAdapter(client)
+
+			_, err := adapter.PostReview(context.Background(), "owner", "repo", 1, vcs.ReviewRequest{
+				Body:  "test",
+				Event: tt.event,
+				// No comments → no diff fetch needed
+			})
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if gotEvent != tt.wantEvent {
+				t.Errorf("event = %q, want %q", gotEvent, tt.wantEvent)
+			}
+		})
+	}
+}
+
+func TestAdapter_PostReview_WithComments_PositionTranslation(t *testing.T) {
+	diff := `diff --git a/main.go b/main.go
+--- a/main.go
++++ b/main.go
+@@ -1,3 +1,4 @@
+ package main
+ 
++// new comment at line 3
+ func main() {}
+`
+	var gotComments []struct {
+		Path        string `json:"path"`
+		NewPosition int64  `json:"new_position"`
+		Body        string `json:"body"`
+	}
+
+	call := 0
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		call++
+		w.Header().Set("Content-Type", "application/json")
+		if call == 1 {
+			// Diff request
+			w.Write([]byte(diff))
+			return
+		}
+		// Review post
+		var payload struct {
+			Comments []struct {
+				Path        string `json:"path"`
+				NewPosition int64  `json:"new_position"`
+				Body        string `json:"body"`
+			} `json:"comments"`
+		}
+		json.NewDecoder(r.Body).Decode(&payload)
+		gotComments = payload.Comments
+		json.NewEncoder(w).Encode(map[string]any{
+			"id":   1,
+			"body": "review",
+			"user": map[string]any{"login": "bot"},
+		})
+	}))
+	defer server.Close()
+
+	client := gitea.NewClient(server.URL, "token")
+	adapter := gitea.NewAdapter(client)
+
+	// Position 4 in this diff is "+// new comment at line 3" → new line 3
+	_, err := adapter.PostReview(context.Background(), "owner", "repo", 1, vcs.ReviewRequest{
+		Body:  "review",
+		Event: vcs.ReviewEventRequestChanges,
+		Comments: []vcs.ReviewComment{
+			{
+				Path:     "main.go",
+				Position: 4,
+				CommitID: "abc123",
+				Body:     "needs fix",
+			},
+		},
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if len(gotComments) != 1 {
+		t.Fatalf("got %d comments, want 1", len(gotComments))
+	}
+	if gotComments[0].Path != "main.go" {
+		t.Errorf("path = %q, want %q", gotComments[0].Path, "main.go")
+	}
+	if gotComments[0].NewPosition != 3 {
+		t.Errorf("new_position = %d, want 3", gotComments[0].NewPosition)
+	}
+	if gotComments[0].Body != "needs fix" {
+		t.Errorf("body = %q, want %q", gotComments[0].Body, "needs fix")
+	}
+}
+
+func TestAdapter_DismissReview(t *testing.T) {
+	var deleteCalled bool
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == http.MethodDelete {
+			deleteCalled = true
+			w.WriteHeader(204)
+			return
+		}
+		w.WriteHeader(404)
+	}))
+	defer server.Close()
+
+	client := gitea.NewClient(server.URL, "token")
+	adapter := gitea.NewAdapter(client)
+
+	err := adapter.DismissReview(context.Background(), "owner", "repo", 1, 99, "stale review")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !deleteCalled {
+		t.Error("expected delete to be called")
+	}
+}
+
+func TestAdapter_Underlying(t *testing.T) {
+	client := gitea.NewClient("http://example.com", "token")
+	adapter := gitea.NewAdapter(client)
+	if adapter.Underlying() != client {
+		t.Error("Underlying() should return the wrapped client")
+	}
+}
+
+func TestAdapter_ListContents(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode([]map[string]any{
+			{"name": "main.go", "path": "src/main.go", "type": "file"},
+			{"name": "util", "path": "src/util", "type": "dir"},
+		})
+	}))
+	defer server.Close()
+
+	client := gitea.NewClient(server.URL, "token")
+	adapter := gitea.NewAdapter(client)
+
+	entries, err := adapter.ListContents(context.Background(), "owner", "repo", "src")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(entries) != 2 {
+		t.Fatalf("got %d entries, want 2", len(entries))
+	}
+	if entries[0].Name != "main.go" || entries[0].Type != "file" {
+		t.Errorf("entries[0] = %+v", entries[0])
+	}
+	if entries[1].Name != "util" || entries[1].Type != "dir" {
+		t.Errorf("entries[1] = %+v", entries[1])
+	}
+}
+
+func TestAdapter_CompileTimeCheck(t *testing.T) {
+	// This is a compile-time assertion — if it compiles, the adapter satisfies vcs.Client
+	var _ vcs.Client = (*gitea.Adapter)(nil)
+}
diff --git a/gitea/client.go b/gitea/client.go
index 3284314..2fec35d 100644
--- a/gitea/client.go
+++ b/gitea/client.go
@@ -86,6 +86,9 @@ type PullRequest struct {
 		Sha string `json:"sha"`
 		Ref string `json:"ref"`
 	} `json:"head"`
+	Base struct {
+		Ref string `json:"ref"`
+	} `json:"base"`
 }
 
 // CommitStatus represents a single CI status entry.
diff --git a/gitea/conformance_test.go b/gitea/conformance_test.go
index 08b7f97..cb3b9f0 100644
--- a/gitea/conformance_test.go
+++ b/gitea/conformance_test.go
@@ -1,5 +1,3 @@
-//go:build phase2
-
 package gitea_test
 
 import (
@@ -7,19 +5,6 @@ import (
 	"gitea.weiker.me/rodin/review-bot/vcs"
 )
 
-// Compile-time interface conformance assertions.
-// These will verify gitea.Client satisfies vcs interfaces once the Phase 2
-// adapter bridges the method signature gaps:
-//
-//   - PRReader: GetPullRequest returns *gitea.PullRequest (needs *vcs.PullRequest)
-//   - PRReader: GetPullRequestFiles returns []gitea.ChangedFile (needs []vcs.ChangedFile)
-//   - FileReader: GetFileContent lacks ref parameter
-//   - Reviewer: PostReview uses (event, body, comments) instead of vcs.ReviewRequest
-//
-// Remove the phase2 build tag once the adapter is complete.
-var (
-	_ vcs.PRReader   = (*gitea.Client)(nil)
-	_ vcs.FileReader = (*gitea.Client)(nil)
-	_ vcs.Reviewer   = (*gitea.Client)(nil)
-	_ vcs.Identity   = (*gitea.Client)(nil)
-)
+// Compile-time interface conformance assertion.
+// The Adapter (not the raw Client) satisfies the full vcs.Client interface.
+var _ vcs.Client = (*gitea.Adapter)(nil)
diff --git a/gitea/position.go b/gitea/position.go
new file mode 100644
index 0000000..ff84212
--- /dev/null
+++ b/gitea/position.go
@@ -0,0 +1,182 @@
+package gitea
+
+import (
+	"fmt"
+	"strconv"
+	"strings"
+)
+
+// PositionMap holds a per-file mapping of GitHub diff-position to new-file line number.
+// Position is a 1-indexed offset from the @@ hunk header line in the unified diff.
+type PositionMap struct {
+	// files maps filename → (position → new-file line number).
+	// Deletion lines are mapped to -1 (no new-file line).
+	files map[string]map[int]int
+}
+
+// Translate converts a GitHub diff-position to a new-file line number for a given file.
+// Returns an error if the file is not in the diff or the position is out of range.
+// If the position targets a deletion line, it maps to the nearest non-deletion line below;
+// if no such line exists, returns an error.
+func (pm *PositionMap) Translate(file string, position int) (int, error) {
+	if pm == nil || pm.files == nil {
+		return 0, fmt.Errorf("empty position map")
+	}
+
+	fileMap, ok := pm.files[file]
+	if !ok {
+		return 0, fmt.Errorf("file %q not found in diff", file)
+	}
+
+	if position < 1 {
+		return 0, fmt.Errorf("position %d out of range (must be >= 1)", position)
+	}
+
+	lineNum, ok := fileMap[position]
+	if !ok {
+		return 0, fmt.Errorf("position %d out of range for file %q", position, file)
+	}
+
+	// lineNum == -1 means this position is a deletion line.
+	// Map to the nearest non-deletion line below.
+	if lineNum == -1 {
+		maxPos := pm.maxPosition(file)
+		for p := position + 1; p <= maxPos; p++ {
+			if ln, exists := fileMap[p]; exists && ln > 0 {
+				return ln, nil
+			}
+		}
+		return 0, fmt.Errorf("position %d targets a deletion line with no subsequent new-file line in %q", position, file)
+	}
+
+	return lineNum, nil
+}
+
+// maxPosition returns the highest position number for a file.
+func (pm *PositionMap) maxPosition(file string) int {
+	max := 0
+	for pos := range pm.files[file] {
+		if pos > max {
+			max = pos
+		}
+	}
+	return max
+}
+
+// BuildPositionToLineMap parses a unified diff and builds a PositionMap
+// mapping diff-position → new-file line number per file.
+//
+// Diff-position counting rules (GitHub spec):
+//   - The @@ hunk header line is position 1 for the file's first hunk
+//   - Every subsequent line increments position by 1 — context, additions, AND deletions
+//   - A new @@ hunk within the same file continues incrementing (does not reset)
+//   - Position maps to the new file line number for additions and context lines
+//   - Deletion lines have a position but no new-file line number (stored as -1)
+func BuildPositionToLineMap(diff string) (*PositionMap, error) {
+	pm := &PositionMap{files: make(map[string]map[int]int)}
+
+	lines := strings.Split(diff, "\n")
+	var currentFile string
+	var position int
+	var newLine int
+
+	for _, line := range lines {
+		// Detect new file in diff
+		if strings.HasPrefix(line, "+++ b/") {
+			currentFile = strings.TrimPrefix(line, "+++ b/")
+			position = 0
+			newLine = 0
+			if pm.files[currentFile] == nil {
+				pm.files[currentFile] = make(map[int]int)
+			}
+			continue
+		}
+
+		// Deleted file: +++ /dev/null means the file is being deleted
+		if strings.HasPrefix(line, "+++ /dev/null") {
+			currentFile = ""
+			continue
+		}
+
+		// Skip --- lines (old file header)
+		if strings.HasPrefix(line, "--- ") {
+			continue
+		}
+
+		// Skip diff --git lines
+		if strings.HasPrefix(line, "diff --git") {
+			continue
+		}
+
+		// Skip index lines
+		if strings.HasPrefix(line, "index ") {
+			continue
+		}
+
+		// Binary file detection
+		if strings.HasPrefix(line, "Binary files") {
+			currentFile = ""
+			continue
+		}
+
+		// Parse hunk headers
+		if strings.HasPrefix(line, "@@") && currentFile != "" {
+			position++
+			newLine = parseHunkStart(line)
+			continue
+		}
+
+		if currentFile == "" {
+			continue
+		}
+
+		// Skip "\ No newline at end of file" markers
+		if strings.HasPrefix(line, `\`) {
+			continue
+		}
+
+		// Process diff content lines
+		if strings.HasPrefix(line, "+") {
+			// Addition: has a new-file line number
+			position++
+			pm.files[currentFile][position] = newLine
+			newLine++
+		} else if strings.HasPrefix(line, "-") {
+			// Deletion: has a position but no new-file line number
+			position++
+			pm.files[currentFile][position] = -1
+		} else if strings.HasPrefix(line, " ") {
+			// Context line
+			position++
+			pm.files[currentFile][position] = newLine
+			newLine++
+		}
+	}
+
+	return pm, nil
+}
+
+// parseHunkStart extracts the new-file starting line number from a hunk header.
+// Format: @@ -old_start[,old_count] +new_start[,new_count] @@
+func parseHunkStart(hunkLine string) int {
+	plusIdx := strings.Index(hunkLine, "+")
+	if plusIdx < 0 {
+		return 1
+	}
+	rest := hunkLine[plusIdx+1:]
+
+	endIdx := 0
+	for endIdx < len(rest) && rest[endIdx] >= '0' && rest[endIdx] <= '9' {
+		endIdx++
+	}
+
+	if endIdx == 0 {
+		return 1
+	}
+
+	n, err := strconv.Atoi(rest[:endIdx])
+	if err != nil {
+		return 1
+	}
+	return n
+}
diff --git a/gitea/position_test.go b/gitea/position_test.go
new file mode 100644
index 0000000..843a0a8
--- /dev/null
+++ b/gitea/position_test.go
@@ -0,0 +1,301 @@
+package gitea
+
+import (
+	"testing"
+)
+
+func TestBuildPositionToLineMap_SingleHunk(t *testing.T) {
+	// @@ -16,4 +16,5 @@  ← position 1
+	// context           ← position 2, new line 16
+	//-deleted           ← position 3, no new line
+	//+added             ← position 4, new line 17
+	// context           ← position 5, new line 18
+	diff := `diff --git a/file.go b/file.go
+index abc..def 100644
+--- a/file.go
++++ b/file.go
+@@ -16,4 +16,5 @@ func example() {
+ context line
+-deleted line
++added line
+ context after
+`
+	pm, err := BuildPositionToLineMap(diff)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	tests := []struct {
+		pos      int
+		wantLine int
+	}{
+		{2, 16}, // context line -> new line 16
+		{4, 17}, // added line -> new line 17
+		{5, 18}, // context after -> new line 18
+	}
+	for _, tt := range tests {
+		got, err := pm.Translate("file.go", tt.pos)
+		if err != nil {
+			t.Errorf("Translate(file.go, %d): unexpected error: %v", tt.pos, err)
+			continue
+		}
+		if got != tt.wantLine {
+			t.Errorf("Translate(file.go, %d) = %d, want %d", tt.pos, got, tt.wantLine)
+		}
+	}
+}
+
+func TestBuildPositionToLineMap_MultipleHunks(t *testing.T) {
+	diff := `diff --git a/file.go b/file.go
+--- a/file.go
++++ b/file.go
+@@ -1,3 +1,3 @@ package main
+ line1
+-old
++new
+@@ -10,3 +10,4 @@ func foo() {
+ func foo() {
++	// added
+ 	return
+ }
+`
+	pm, err := BuildPositionToLineMap(diff)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	tests := []struct {
+		pos      int
+		wantLine int
+	}{
+		// First hunk: @@ is pos 1
+		{2, 1},  // " line1" -> new line 1
+		{4, 2},  // "+new" -> new line 2
+		// Second hunk: @@ is pos 5 (continues from 4)
+		// Wait: first hunk has pos 1(@@ hdr), 2(" line1"), 3("-old"), 4("+new")
+		// Second hunk @@ is pos 5
+		{6, 10}, // " func foo() {" -> new line 10
+		{7, 11}, // "+\t// added" -> new line 11
+		{8, 12}, // " \treturn" -> new line 12
+		{9, 13}, // " }" -> new line 13
+	}
+	for _, tt := range tests {
+		got, err := pm.Translate("file.go", tt.pos)
+		if err != nil {
+			t.Errorf("Translate(file.go, %d): unexpected error: %v", tt.pos, err)
+			continue
+		}
+		if got != tt.wantLine {
+			t.Errorf("Translate(file.go, %d) = %d, want %d", tt.pos, got, tt.wantLine)
+		}
+	}
+}
+
+func TestBuildPositionToLineMap_DeletionTargeted(t *testing.T) {
+	diff := `diff --git a/file.go b/file.go
+--- a/file.go
++++ b/file.go
+@@ -1,4 +1,3 @@ package main
+ line1
+-deleted
+ line3
+`
+	pm, err := BuildPositionToLineMap(diff)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Position 3 is the deletion line "-deleted" — should map to nearest below
+	// Position 4 is " line3" which is new line 2
+	got, err := pm.Translate("file.go", 3)
+	if err != nil {
+		t.Fatalf("Translate(file.go, 3): unexpected error: %v", err)
+	}
+	if got != 2 {
+		t.Errorf("Translate(file.go, 3) = %d, want 2 (nearest non-deletion below)", got)
+	}
+}
+
+func TestBuildPositionToLineMap_DeletionAtEnd(t *testing.T) {
+	// If a deletion line is at the end with no subsequent non-deletion line, error
+	diff := `diff --git a/file.go b/file.go
+--- a/file.go
++++ b/file.go
+@@ -1,3 +1,2 @@ package main
+ line1
+ line2
+-deleted at end
+`
+	pm, err := BuildPositionToLineMap(diff)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	_, err = pm.Translate("file.go", 4)
+	if err == nil {
+		t.Error("expected error for deletion at end with no subsequent line")
+	}
+}
+
+func TestBuildPositionToLineMap_NewFile(t *testing.T) {
+	diff := `diff --git a/new.go b/new.go
+new file mode 100644
+--- /dev/null
++++ b/new.go
+@@ -0,0 +1,3 @@
++package main
++
++func init() {}
+`
+	pm, err := BuildPositionToLineMap(diff)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	tests := []struct {
+		pos      int
+		wantLine int
+	}{
+		{2, 1}, // "+package main" -> line 1
+		{3, 2}, // "+" (empty line) -> line 2
+		{4, 3}, // "+func init() {}" -> line 3
+	}
+	for _, tt := range tests {
+		got, err := pm.Translate("new.go", tt.pos)
+		if err != nil {
+			t.Errorf("Translate(new.go, %d): unexpected error: %v", tt.pos, err)
+			continue
+		}
+		if got != tt.wantLine {
+			t.Errorf("Translate(new.go, %d) = %d, want %d", tt.pos, got, tt.wantLine)
+		}
+	}
+}
+
+func TestBuildPositionToLineMap_DeletedFile(t *testing.T) {
+	diff := `diff --git a/old.go b/old.go
+deleted file mode 100644
+--- a/old.go
++++ /dev/null
+@@ -1,3 +0,0 @@
+-package main
+-
+-func old() {}
+`
+	pm, err := BuildPositionToLineMap(diff)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Deleted file has no new-file lines; positions should error
+	_, err = pm.Translate("old.go", 2)
+	if err == nil {
+		t.Error("expected error for deleted file position")
+	}
+}
+
+func TestBuildPositionToLineMap_BinaryFile(t *testing.T) {
+	diff := `diff --git a/image.png b/image.png
+Binary files /dev/null and b/image.png differ
+diff --git a/code.go b/code.go
+--- a/code.go
++++ b/code.go
+@@ -1,2 +1,3 @@
+ package main
++// added
+ func main() {}
+`
+	pm, err := BuildPositionToLineMap(diff)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Binary file should not be in the map
+	_, err = pm.Translate("image.png", 1)
+	if err == nil {
+		t.Error("expected error for binary file")
+	}
+
+	// code.go should still work
+	got, err := pm.Translate("code.go", 3)
+	if err != nil {
+		t.Fatalf("Translate(code.go, 3): unexpected error: %v", err)
+	}
+	if got != 2 {
+		t.Errorf("Translate(code.go, 3) = %d, want 2", got)
+	}
+}
+
+func TestBuildPositionToLineMap_OutOfRange(t *testing.T) {
+	diff := `diff --git a/file.go b/file.go
+--- a/file.go
++++ b/file.go
+@@ -1,2 +1,2 @@
+ line1
+-old
++new
+`
+	pm, err := BuildPositionToLineMap(diff)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Position 0 is invalid
+	_, err = pm.Translate("file.go", 0)
+	if err == nil {
+		t.Error("expected error for position 0")
+	}
+
+	// Position 5 is out of range (only positions 1-4 exist)
+	_, err = pm.Translate("file.go", 5)
+	if err == nil {
+		t.Error("expected error for position 5 (out of range)")
+	}
+
+	// Unknown file
+	_, err = pm.Translate("unknown.go", 1)
+	if err == nil {
+		t.Error("expected error for unknown file")
+	}
+}
+
+func TestBuildPositionToLineMap_MultipleFiles(t *testing.T) {
+	diff := `diff --git a/a.go b/a.go
+--- a/a.go
++++ b/a.go
+@@ -1,2 +1,3 @@
+ package a
++// file a
+ func aFunc() {}
+diff --git a/b.go b/b.go
+--- a/b.go
++++ b/b.go
+@@ -1,2 +1,3 @@
+ package b
++// file b
+ func bFunc() {}
+`
+	pm, err := BuildPositionToLineMap(diff)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// a.go: pos 3 is "+// file a" -> new line 2
+	got, err := pm.Translate("a.go", 3)
+	if err != nil {
+		t.Fatalf("Translate(a.go, 3): %v", err)
+	}
+	if got != 2 {
+		t.Errorf("Translate(a.go, 3) = %d, want 2", got)
+	}
+
+	// b.go: pos 3 is "+// file b" -> new line 2
+	// Note: position resets per file
+	got, err = pm.Translate("b.go", 3)
+	if err != nil {
+		t.Fatalf("Translate(b.go, 3): %v", err)
+	}
+	if got != 2 {
+		t.Errorf("Translate(b.go, 3) = %d, want 2", got)
+	}
+}
diff --git a/vcs/check_test.go b/vcs/check_test.go
index c64e954..217aecd 100644
--- a/vcs/check_test.go
+++ b/vcs/check_test.go
@@ -1,5 +1,3 @@
-//go:build phase2
-
 package vcs_test
 
 import (
@@ -7,21 +5,7 @@ import (
 	"gitea.weiker.me/rodin/review-bot/vcs"
 )
 
-// Compile-time assertion: documents the gap between gitea.Client and vcs.Client.
-// Guarded by the "phase2" build tag — enable once the Gitea adapter bridges these gaps:
-//
-//  1. PostReview signature mismatch:
-//     gitea.Client:  PostReview(ctx, owner, repo, number, event, body string, comments []gitea.ReviewComment)
-//     vcs.Reviewer:  PostReview(ctx, owner, repo, number, req vcs.ReviewRequest)
-//
-//  2. GetFileContent signature mismatch:
-//     gitea.Client:  GetFileContent(ctx, owner, repo, filepath string)  [no ref; uses default branch]
-//     vcs.FileReader: GetFileContent(ctx, owner, repo, path, ref string)
-//     (gitea.Client has GetFileContentRef for the ref variant)
-//
-//  3. ReviewComment type mismatch:
-//     gitea.ReviewComment uses NewPosition int64 (Gitea line-number convention)
-//     vcs.ReviewComment uses Position int (GitHub diff-position convention)
-//
-// The Gitea adapter (Phase 2) will wrap gitea.Client to bridge these gaps.
-var _ vcs.Client = (*gitea.Client)(nil)
+// Compile-time assertion: the gitea.Adapter satisfies vcs.Client.
+// (The raw gitea.Client does NOT satisfy vcs.Client due to signature differences;
+// the Adapter bridges them.)
+var _ vcs.Client = (*gitea.Adapter)(nil)

From 0ec5093aeb5a5821004e1d4421d6b1a430633e39 Mon Sep 17 00:00:00 2001
From: claw <claw@weiker.me>
Date: Tue, 12 May 2026 13:49:36 -0700
Subject: [PATCH 2/4] fix: address self-review findings on PR #90

- Remove unused error return from BuildPositionToLineMap (always nil)
- Add comment explaining intentional CommitID drop in PostReview
- Refactor TestAdapter_PostReview_WithComments to route by URL path
- Add TestAdapter_GetFileContent_RefRouting test
- Acknowledge maxPosition O(n) with code comment
- Remove redundant TestAdapter_CompileTimeCheck (compile-time var _ exists)
- Fix GetPullRequestFiles comment (Patch field is omitted, not 'set to empty')
- Acknowledge translateEvent fallback as intentional design
---
 gitea/adapter.go       | 13 +++++---
 gitea/adapter_test.go  | 73 ++++++++++++++++++++++++++++++------------
 gitea/position.go      |  5 +--
 gitea/position_test.go | 53 ++++++++----------------------
 4 files changed, 77 insertions(+), 67 deletions(-)

diff --git a/gitea/adapter.go b/gitea/adapter.go
index 20806b7..b3d8122 100644
--- a/gitea/adapter.go
+++ b/gitea/adapter.go
@@ -56,7 +56,7 @@ func (a *Adapter) GetPullRequestDiff(ctx context.Context, owner, repo string, nu
 }
 
 // GetPullRequestFiles maps []gitea.ChangedFile to []vcs.ChangedFile.
-// Patch is set to empty string since Gitea's /pulls/{n}/files does not return patch text.
+// Patch field is omitted (zero-value) since Gitea's /pulls/{n}/files does not return patch text.
 func (a *Adapter) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]vcs.ChangedFile, error) {
 	files, err := a.client.GetPullRequestFiles(ctx, owner, repo, number)
 	if err != nil {
@@ -135,6 +135,9 @@ func translateEvent(event vcs.ReviewEvent) string {
 	case vcs.ReviewEventComment:
 		return "COMMENT"
 	default:
+		// Unknown events pass through as-is. This is intentional: new event types
+		// added to vcs.ReviewEvent will still be forwarded without a code change here,
+		// and Gitea will reject truly invalid values with a clear API error.
 		return string(event)
 	}
 }
@@ -153,16 +156,16 @@ func (a *Adapter) PostReview(ctx context.Context, owner, repo string, number int
 			return nil, fmt.Errorf("fetch diff for position translation: %w", err)
 		}
 
-		posMap, err := BuildPositionToLineMap(diff)
-		if err != nil {
-			return nil, fmt.Errorf("build position map: %w", err)
-		}
+		posMap := BuildPositionToLineMap(diff)
 
 		for _, c := range req.Comments {
 			lineNum, err := posMap.Translate(c.Path, c.Position)
 			if err != nil {
 				return nil, fmt.Errorf("translate position %d in %s: %w", c.Position, c.Path, err)
 			}
+			// CommitID from vcs.ReviewComment is intentionally not forwarded:
+			// Gitea review comments are pinned to the PR head SHA automatically,
+			// and the CreatePullReview API has no per-comment commit_id field.
 			giteaComments = append(giteaComments, ReviewComment{
 				Path:        c.Path,
 				NewPosition: int64(lineNum),
diff --git a/gitea/adapter_test.go b/gitea/adapter_test.go
index 380e643..f272116 100644
--- a/gitea/adapter_test.go
+++ b/gitea/adapter_test.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"gitea.weiker.me/rodin/review-bot/gitea"
@@ -229,30 +230,33 @@ func TestAdapter_PostReview_WithComments_PositionTranslation(t *testing.T) {
 		Body        string `json:"body"`
 	}
 
-	call := 0
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		call++
 		w.Header().Set("Content-Type", "application/json")
-		if call == 1 {
+		if strings.HasSuffix(r.URL.Path, ".diff") {
 			// Diff request
 			w.Write([]byte(diff))
 			return
 		}
-		// Review post
-		var payload struct {
-			Comments []struct {
-				Path        string `json:"path"`
-				NewPosition int64  `json:"new_position"`
-				Body        string `json:"body"`
-			} `json:"comments"`
+		if strings.HasSuffix(r.URL.Path, "/reviews") {
+			// Review post
+			var payload struct {
+				Comments []struct {
+					Path        string `json:"path"`
+					NewPosition int64  `json:"new_position"`
+					Body        string `json:"body"`
+				} `json:"comments"`
+			}
+			json.NewDecoder(r.Body).Decode(&payload)
+			gotComments = payload.Comments
+			json.NewEncoder(w).Encode(map[string]any{
+				"id":   1,
+				"body": "review",
+				"user": map[string]any{"login": "bot"},
+			})
+			return
 		}
-		json.NewDecoder(r.Body).Decode(&payload)
-		gotComments = payload.Comments
-		json.NewEncoder(w).Encode(map[string]any{
-			"id":   1,
-			"body": "review",
-			"user": map[string]any{"login": "bot"},
-		})
+		t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
+		w.WriteHeader(http.StatusNotFound)
 	}))
 	defer server.Close()
 
@@ -350,7 +354,36 @@ func TestAdapter_ListContents(t *testing.T) {
 	}
 }
 
-func TestAdapter_CompileTimeCheck(t *testing.T) {
-	// This is a compile-time assertion — if it compiles, the adapter satisfies vcs.Client
-	var _ vcs.Client = (*gitea.Adapter)(nil)
+
+func TestAdapter_GetFileContent_RefRouting(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// When ref is provided, the URL should contain ?ref=
+		if r.URL.RawQuery != "" && strings.Contains(r.URL.RawQuery, "ref=") {
+			w.Write([]byte("content-at-ref"))
+		} else {
+			w.Write([]byte("content-default"))
+		}
+	}))
+	defer server.Close()
+
+	client := gitea.NewClient(server.URL, "token")
+	adapter := gitea.NewAdapter(client)
+
+	// Empty ref → routes to GetFileContent (no ?ref= query param)
+	got, err := adapter.GetFileContent(context.Background(), "owner", "repo", "main.go", "")
+	if err != nil {
+		t.Fatalf("GetFileContent(ref=\"\"): %v", err)
+	}
+	if got != "content-default" {
+		t.Errorf("GetFileContent(ref=\"\") = %q, want %q", got, "content-default")
+	}
+
+	// Non-empty ref → routes to GetFileContentRef (with ?ref= query param)
+	got, err = adapter.GetFileContent(context.Background(), "owner", "repo", "main.go", "abc123")
+	if err != nil {
+		t.Fatalf("GetFileContent(ref=\"abc123\"): %v", err)
+	}
+	if got != "content-at-ref" {
+		t.Errorf("GetFileContent(ref=\"abc123\") = %q, want %q", got, "content-at-ref")
+	}
 }
diff --git a/gitea/position.go b/gitea/position.go
index ff84212..4be26b4 100644
--- a/gitea/position.go
+++ b/gitea/position.go
@@ -53,6 +53,7 @@ func (pm *PositionMap) Translate(file string, position int) (int, error) {
 }
 
 // maxPosition returns the highest position number for a file.
+// O(n) per call — acceptable since deletion-line fallback is rare and n is small (typical hunk size).
 func (pm *PositionMap) maxPosition(file string) int {
 	max := 0
 	for pos := range pm.files[file] {
@@ -72,7 +73,7 @@ func (pm *PositionMap) maxPosition(file string) int {
 //   - A new @@ hunk within the same file continues incrementing (does not reset)
 //   - Position maps to the new file line number for additions and context lines
 //   - Deletion lines have a position but no new-file line number (stored as -1)
-func BuildPositionToLineMap(diff string) (*PositionMap, error) {
+func BuildPositionToLineMap(diff string) *PositionMap {
 	pm := &PositionMap{files: make(map[string]map[int]int)}
 
 	lines := strings.Split(diff, "\n")
@@ -153,7 +154,7 @@ func BuildPositionToLineMap(diff string) (*PositionMap, error) {
 		}
 	}
 
-	return pm, nil
+	return pm
 }
 
 // parseHunkStart extracts the new-file starting line number from a hunk header.
diff --git a/gitea/position_test.go b/gitea/position_test.go
index 843a0a8..743a2b0 100644
--- a/gitea/position_test.go
+++ b/gitea/position_test.go
@@ -20,10 +20,7 @@ index abc..def 100644
 +added line
  context after
 `
-	pm, err := BuildPositionToLineMap(diff)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
+	pm := BuildPositionToLineMap(diff)
 
 	tests := []struct {
 		pos      int
@@ -59,10 +56,7 @@ func TestBuildPositionToLineMap_MultipleHunks(t *testing.T) {
  	return
  }
 `
-	pm, err := BuildPositionToLineMap(diff)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
+	pm := BuildPositionToLineMap(diff)
 
 	tests := []struct {
 		pos      int
@@ -100,10 +94,7 @@ func TestBuildPositionToLineMap_DeletionTargeted(t *testing.T) {
 -deleted
  line3
 `
-	pm, err := BuildPositionToLineMap(diff)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
+	pm := BuildPositionToLineMap(diff)
 
 	// Position 3 is the deletion line "-deleted" — should map to nearest below
 	// Position 4 is " line3" which is new line 2
@@ -126,12 +117,9 @@ func TestBuildPositionToLineMap_DeletionAtEnd(t *testing.T) {
  line2
 -deleted at end
 `
-	pm, err := BuildPositionToLineMap(diff)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
+	pm := BuildPositionToLineMap(diff)
 
-	_, err = pm.Translate("file.go", 4)
+	_, err := pm.Translate("file.go", 4)
 	if err == nil {
 		t.Error("expected error for deletion at end with no subsequent line")
 	}
@@ -147,10 +135,7 @@ new file mode 100644
 +
 +func init() {}
 `
-	pm, err := BuildPositionToLineMap(diff)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
+	pm := BuildPositionToLineMap(diff)
 
 	tests := []struct {
 		pos      int
@@ -182,13 +167,10 @@ deleted file mode 100644
 -
 -func old() {}
 `
-	pm, err := BuildPositionToLineMap(diff)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
+	pm := BuildPositionToLineMap(diff)
 
 	// Deleted file has no new-file lines; positions should error
-	_, err = pm.Translate("old.go", 2)
+	_, err := pm.Translate("old.go", 2)
 	if err == nil {
 		t.Error("expected error for deleted file position")
 	}
@@ -205,13 +187,10 @@ diff --git a/code.go b/code.go
 +// added
  func main() {}
 `
-	pm, err := BuildPositionToLineMap(diff)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
+	pm := BuildPositionToLineMap(diff)
 
 	// Binary file should not be in the map
-	_, err = pm.Translate("image.png", 1)
+	_, err := pm.Translate("image.png", 1)
 	if err == nil {
 		t.Error("expected error for binary file")
 	}
@@ -235,13 +214,10 @@ func TestBuildPositionToLineMap_OutOfRange(t *testing.T) {
 -old
 +new
 `
-	pm, err := BuildPositionToLineMap(diff)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
+	pm := BuildPositionToLineMap(diff)
 
 	// Position 0 is invalid
-	_, err = pm.Translate("file.go", 0)
+	_, err := pm.Translate("file.go", 0)
 	if err == nil {
 		t.Error("expected error for position 0")
 	}
@@ -275,10 +251,7 @@ diff --git a/b.go b/b.go
 +// file b
  func bFunc() {}
 `
-	pm, err := BuildPositionToLineMap(diff)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
+	pm := BuildPositionToLineMap(diff)
 
 	// a.go: pos 3 is "+// file a" -> new line 2
 	got, err := pm.Translate("a.go", 3)

From b2eea502d0ae10ee4a21d5f9a66348094829bfef Mon Sep 17 00:00:00 2001
From: claw <claw@weiker.me>
Date: Tue, 12 May 2026 13:57:44 -0700
Subject: [PATCH 3/4] refactor(gitea): address review feedback on PR #90

- position.go: Replace O(n) maxPosition scan with O(1) lookup by
  tracking max position during map construction. Also eliminates
  shadowing of the builtin max identifier (Go 1.21+).
- position.go: Add comment clarifying +++ prefix ordering intent.
- adapter.go: Document diff-fetch tradeoff in PostReview.
- adapter_test.go: Remove extra blank line between test functions.
---
 gitea/adapter.go      |  6 +++++-
 gitea/adapter_test.go |  1 -
 gitea/position.go     | 27 +++++++++++++++++----------
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/gitea/adapter.go b/gitea/adapter.go
index b3d8122..d6134fb 100644
--- a/gitea/adapter.go
+++ b/gitea/adapter.go
@@ -150,7 +150,11 @@ func (a *Adapter) PostReview(ctx context.Context, owner, repo string, number int
 
 	var giteaComments []ReviewComment
 	if len(req.Comments) > 0 {
-		// Fetch diff to build position → line number map
+		// Fetch diff to build position → line number map.
+		// The diff is fetched unconditionally when comments exist. This adds latency
+		// for reviews with inline comments but keeps the implementation simple — caching
+		// the diff across calls would add complexity for minimal gain since PostReview
+		// is called at most once per review cycle.
 		diff, err := a.client.GetPullRequestDiff(ctx, owner, repo, number)
 		if err != nil {
 			return nil, fmt.Errorf("fetch diff for position translation: %w", err)
diff --git a/gitea/adapter_test.go b/gitea/adapter_test.go
index f272116..0071699 100644
--- a/gitea/adapter_test.go
+++ b/gitea/adapter_test.go
@@ -354,7 +354,6 @@ func TestAdapter_ListContents(t *testing.T) {
 	}
 }
 
-
 func TestAdapter_GetFileContent_RefRouting(t *testing.T) {
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// When ref is provided, the URL should contain ?ref=
diff --git a/gitea/position.go b/gitea/position.go
index 4be26b4..e1aa4be 100644
--- a/gitea/position.go
+++ b/gitea/position.go
@@ -12,6 +12,9 @@ type PositionMap struct {
 	// files maps filename → (position → new-file line number).
 	// Deletion lines are mapped to -1 (no new-file line).
 	files map[string]map[int]int
+	// maxPositions caches the highest position number per file,
+	// tracked during construction to avoid O(n) scans at translate time.
+	maxPositions map[string]int
 }
 
 // Translate converts a GitHub diff-position to a new-file line number for a given file.
@@ -53,15 +56,9 @@ func (pm *PositionMap) Translate(file string, position int) (int, error) {
 }
 
 // maxPosition returns the highest position number for a file.
-// O(n) per call — acceptable since deletion-line fallback is rare and n is small (typical hunk size).
+// O(1) — the maximum is tracked during map construction.
 func (pm *PositionMap) maxPosition(file string) int {
-	max := 0
-	for pos := range pm.files[file] {
-		if pos > max {
-			max = pos
-		}
-	}
-	return max
+	return pm.maxPositions[file]
 }
 
 // BuildPositionToLineMap parses a unified diff and builds a PositionMap
@@ -74,7 +71,10 @@ func (pm *PositionMap) maxPosition(file string) int {
 //   - Position maps to the new file line number for additions and context lines
 //   - Deletion lines have a position but no new-file line number (stored as -1)
 func BuildPositionToLineMap(diff string) *PositionMap {
-	pm := &PositionMap{files: make(map[string]map[int]int)}
+	pm := &PositionMap{
+		files:        make(map[string]map[int]int),
+		maxPositions: make(map[string]int),
+	}
 
 	lines := strings.Split(diff, "\n")
 	var currentFile string
@@ -82,7 +82,10 @@ func BuildPositionToLineMap(diff string) *PositionMap {
 	var newLine int
 
 	for _, line := range lines {
-		// Detect new file in diff
+		// Detect new file in diff.
+		// "+++ b/" is checked before "+++ /dev/null" — the two prefixes are
+		// non-overlapping ("+++ /dev/null" does not start with "+++ b/"), so
+		// ordering is independent. Checking the common case first for clarity.
 		if strings.HasPrefix(line, "+++ b/") {
 			currentFile = strings.TrimPrefix(line, "+++ b/")
 			position = 0
@@ -123,6 +126,7 @@ func BuildPositionToLineMap(diff string) *PositionMap {
 		// Parse hunk headers
 		if strings.HasPrefix(line, "@@") && currentFile != "" {
 			position++
+			pm.maxPositions[currentFile] = position
 			newLine = parseHunkStart(line)
 			continue
 		}
@@ -141,15 +145,18 @@ func BuildPositionToLineMap(diff string) *PositionMap {
 			// Addition: has a new-file line number
 			position++
 			pm.files[currentFile][position] = newLine
+			pm.maxPositions[currentFile] = position
 			newLine++
 		} else if strings.HasPrefix(line, "-") {
 			// Deletion: has a position but no new-file line number
 			position++
 			pm.files[currentFile][position] = -1
+			pm.maxPositions[currentFile] = position
 		} else if strings.HasPrefix(line, " ") {
 			// Context line
 			position++
 			pm.files[currentFile][position] = newLine
+			pm.maxPositions[currentFile] = position
 			newLine++
 		}
 	}

From d8270262d618a4feb2624e202c225dea0da13f6d Mon Sep 17 00:00:00 2001
From: claw <claw@weiker.me>
Date: Tue, 12 May 2026 14:56:55 -0700
Subject: [PATCH 4/4] Wrap errors in GetPullRequest and PostReview for
 consistency

Add fmt.Errorf wrapping to the two remaining unwrapped error returns
in the adapter:
- GetPullRequest: 'get pull request: %w'
- PostReview (final client call): 'post review: %w'

This makes all error paths in the adapter consistent with the wrapping
pattern used by the diff-fetch and position-translation errors.

Addresses self-review findings #1 and #2 from b2eea502.
---
 gitea/adapter.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitea/adapter.go b/gitea/adapter.go
index d6134fb..4cef61e 100644
--- a/gitea/adapter.go
+++ b/gitea/adapter.go
@@ -34,7 +34,7 @@ func (a *Adapter) Underlying() *Client {
 func (a *Adapter) GetPullRequest(ctx context.Context, owner, repo string, number int) (*vcs.PullRequest, error) {
 	pr, err := a.client.GetPullRequest(ctx, owner, repo, number)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("get pull request: %w", err)
 	}
 	return &vcs.PullRequest{
 		Number: number,
@@ -180,7 +180,7 @@ func (a *Adapter) PostReview(ctx context.Context, owner, repo string, number int
 
 	review, err := a.client.PostReview(ctx, owner, repo, number, event, req.Body, giteaComments)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("post review: %w", err)
 	}
 
 	return &vcs.Review{