rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 6 Wiki Activity

feat(vcs): Gitea adapter with diff-position translation (Phase 2) #79

New Issue
Closed
opened 2026-05-12 16:59:55 +00:00 by rodin · 2 comments
rodin commented 2026-05-12 16:59:55 +00:00
Owner
Copy Link
Copy Source

Architecture Reference

This issue is self-contained. For any design questions, the canonical reference is:
docs/DESIGN-vcs-abstraction.md
(Parts 2, 3, and 4 are most relevant to this issue)

Base branch: feature/github-support — open all PRs against this branch.

Goal

Wrap gitea.Client in an adapter (gitea.Adapter) that satisfies vcs.Client. The critical work is: (1) translating GitHub diff-position in ReviewComment to Gitea line numbers, and (2) translating ReviewRequest.Event canonical values to Gitea-native event strings.

Background

review-bot uses GitHub diff-position as the canonical ReviewComment format (path + position + commit_id, where position is a 1-indexed offset from the @@ hunk line). Gitea takes line numbers instead. The adapter is responsible for this translation.

ReviewRequest.Event uses GitHub canonical values ("APPROVE", "REQUEST_CHANGES", "COMMENT"). Gitea uses different strings. The adapter must translate.

The vcs.Client interface and all types live in vcs/ (created in issue #78).

Work

gitea/adapter.go

type Adapter struct {
    client *Client
}

func NewAdapter(client *Client) *Adapter

// Underlying returns the wrapped gitea.Client for Gitea-specific operations
// that have no vcs.Client equivalent (resolve comment, timeline, supersede flow).
// Callers in main.go type-assert to *Adapter and call Underlying() when needed.
func (a *Adapter) Underlying() *Client

Alias strategy

Before implementing any method, check whether the gitea type can be aliased to the vcs type (see issue #78 type alias strategy). If gitea.ContentEntry = vcs.ContentEntry and gitea.CommitStatus = vcs.CommitStatus are in place, ListContents and GetCommitStatuses become true pass-throughs. Only types with structural differences (e.g. gitea.PullRequest vs vcs.PullRequest, gitea.Review vs vcs.Review) require field mapping.

Methods requiring mapping (not simple pass-through)

These methods require explicit type translation, not just delegation:

GetPullRequest — maps gitea.PullRequest → vcs.PullRequest:

func (a *Adapter) GetPullRequest(ctx, 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,  // gitea.PullRequest has no Number field; use the method parameter
        Title:  pr.Title,
        Body:   pr.Body,
        Head:   vcs.PullRequest.Head{Ref: pr.Head.Ref, Sha: pr.Head.Sha},
        Base:   vcs.PullRequest.Base{Ref: pr.Base.Ref},
    }, nil
}

(Pseudo-code; use the correct anonymous struct literal syntax for Go.)

GetPullRequestFiles — maps []gitea.ChangedFile → []vcs.ChangedFile:

// gitea.ChangedFile has {Filename, Status} — no Patch field in the existing struct.
// vcs.ChangedFile has {Filename, Status, Patch}.
// Set Patch to "" for all entries (Gitea /pulls/{n}/files does not return patch text).
// Patch is populated by GetPullRequestDiff in main.go where needed.

ListReviews — maps []gitea.Review → []vcs.Review:

// Map each gitea.Review field to vcs.Review.
// Gitea State values happen to match the vcs canonical values already:
// "APPROVED", "REQUEST_CHANGES", "COMMENT" — pass through as-is.
// If Gitea ever returns other values, map to vcs.ReviewState* constants.

GetCommitStatuses — maps []gitea.CommitStatus → []vcs.CommitStatus:

// Map Context, Status, Description, TargetURL from gitea.CommitStatus to vcs.CommitStatus.

Pass-through methods

These delegate directly with no type translation (signature and return type already match):

  • GetPullRequestDiff
  • GetFileContent
  • GetAuthenticatedUser
  • DeleteReview

ListContents

Per the alias strategy (see issue #78): gitea.ContentEntry should be aliased to vcs.ContentEntry (type ContentEntry = vcs.ContentEntry). If that alias is in place, gitea.Client.ListContents already satisfies the interface with no adapter wrapper needed — it becomes a true pass-through.

If for any reason the alias cannot be applied (e.g. a conflicting field), fall back to a field-by-field map (Name, Path, Type — all strings). Add a unit test either way.

PostReview translation

Two translations required:

1. Event string — translate ReviewRequest.Event from GitHub canonical to Gitea-native:

ReviewRequest.Event (canonical) Gitea API event string
"APPROVE" "APPROVED"
"REQUEST_CHANGES" "REQUEST_CHANGES"
"COMMENT" "COMMENT"

Note: "APPROVE" (no D) → "APPROVED" (with D) is the critical translation. gitea.Client.PostReview passes the event string verbatim to the Gitea API, which requires "APPROVED".

Unit test: verify "APPROVE" produces a Gitea API call with event "APPROVED".

2. Diff-position → line number — for each ReviewComment.Position:

  1. Fetch the PR diff using GetPullRequestDiff
  2. Parse unified diff hunks to build a position → new-file line number map per file
  3. Translate each ReviewComment.Position to the corresponding file line number
  4. Set CommitID from ReviewComment.CommitID (pass through to Gitea)

Diff-position counting rules (GitHub spec)

Position is a per-file counter. For each file in the diff:

  • The @@ hunk header line is position 1
  • Every subsequent line increments position by 1 — context, additions, AND deletions all count
  • 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 — map to the nearest non-deletion line below

Example:

@@ -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

DismissReview

func (a *Adapter) DismissReview(ctx context.Context, owner, repo string, number int, reviewID int64, message string) error {
    // Gitea supports full deletion of any review state.
    // The message parameter is intentionally unused — Gitea deletion has no dismissal message.
    return a.client.DeleteReview(ctx, owner, repo, number, reviewID)  // a.client is *Client (within package gitea)
}

GetFileContentAtRef

Pass-through to gitea.Client.GetFileContentRef:

func (a *Adapter) GetFileContentAtRef(ctx context.Context, owner, repo, path, ref string) (string, error) {
    return a.client.GetFileContentRef(ctx, owner, repo, path, ref)
}

GetCommitStatuses

Pass-through to gitea.Client.GetCommitStatuses after type mapping (see above).

Compile-time check

var _ vcs.Client = (*Adapter)(nil)

Edge cases for diff-position parser

Case Behavior
Binary file No diff lines; skip (cannot comment on binary diff position)
New file (no context) First @@ is still position 1; count from there
Deleted file Has diff lines but no new-file line numbers; treat like deletion lines
Position out of range Return error
Deletion line targeted Map to nearest non-deletion line below; if none, return error

Exit criteria

  • var _ vcs.Client = (*Adapter)(nil) compiles
  • All existing Gitea integration tests pass unchanged
  • Unit tests for diff-position → line-number translation:
    • Single hunk, multiple hunks
    • Context, addition, deletion lines (deletions DO count toward position)
    • New file, deleted file, binary file
    • Deletion line targeted → maps to line below
    • Out-of-range position → error
  • Unit test for Event string translation: "APPROVE" → Gitea API receives "APPROVED" (not "APPROVE")
  • Unit tests for all field-mapping methods: GetPullRequest, GetPullRequestFiles, ListReviews, GetCommitStatuses

Out of scope

  • GitHub client (issues #80, #81)
  • CLI flag changes (issue #82)
## Architecture Reference > This issue is self-contained. For any design questions, the canonical reference is: > [`docs/DESIGN-vcs-abstraction.md`](https://gitea.weiker.me/rodin/review-bot/src/branch/feature/github-support/docs/DESIGN-vcs-abstraction.md) > (Parts 2, 3, and 4 are most relevant to this issue) **Base branch:** `feature/github-support` — open all PRs against this branch. --- ## Goal Wrap `gitea.Client` in an adapter (`gitea.Adapter`) that satisfies `vcs.Client`. The critical work is: (1) translating GitHub diff-position in `ReviewComment` to Gitea line numbers, and (2) translating `ReviewRequest.Event` canonical values to Gitea-native event strings. ## Background review-bot uses GitHub diff-position as the canonical `ReviewComment` format (`path` + `position` + `commit_id`, where `position` is a 1-indexed offset from the `@@` hunk line). Gitea takes line numbers instead. The adapter is responsible for this translation. `ReviewRequest.Event` uses GitHub canonical values (`"APPROVE"`, `"REQUEST_CHANGES"`, `"COMMENT"`). Gitea uses different strings. The adapter must translate. The `vcs.Client` interface and all types live in `vcs/` (created in issue #78). ## Work ### `gitea/adapter.go` ```go type Adapter struct { client *Client } func NewAdapter(client *Client) *Adapter // Underlying returns the wrapped gitea.Client for Gitea-specific operations // that have no vcs.Client equivalent (resolve comment, timeline, supersede flow). // Callers in main.go type-assert to *Adapter and call Underlying() when needed. func (a *Adapter) Underlying() *Client ``` ### Alias strategy Before implementing any method, check whether the gitea type can be aliased to the vcs type (see issue #78 type alias strategy). If `gitea.ContentEntry = vcs.ContentEntry` and `gitea.CommitStatus = vcs.CommitStatus` are in place, `ListContents` and `GetCommitStatuses` become true pass-throughs. Only types with structural differences (e.g. `gitea.PullRequest` vs `vcs.PullRequest`, `gitea.Review` vs `vcs.Review`) require field mapping. ### Methods requiring mapping (not simple pass-through) These methods require explicit type translation, not just delegation: **`GetPullRequest`** — maps `gitea.PullRequest` → `vcs.PullRequest`: ```go func (a *Adapter) GetPullRequest(ctx, 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, // gitea.PullRequest has no Number field; use the method parameter Title: pr.Title, Body: pr.Body, Head: vcs.PullRequest.Head{Ref: pr.Head.Ref, Sha: pr.Head.Sha}, Base: vcs.PullRequest.Base{Ref: pr.Base.Ref}, }, nil } ``` (Pseudo-code; use the correct anonymous struct literal syntax for Go.) **`GetPullRequestFiles`** — maps `[]gitea.ChangedFile` → `[]vcs.ChangedFile`: ```go // gitea.ChangedFile has {Filename, Status} — no Patch field in the existing struct. // vcs.ChangedFile has {Filename, Status, Patch}. // Set Patch to "" for all entries (Gitea /pulls/{n}/files does not return patch text). // Patch is populated by GetPullRequestDiff in main.go where needed. ``` **`ListReviews`** — maps `[]gitea.Review` → `[]vcs.Review`: ```go // Map each gitea.Review field to vcs.Review. // Gitea State values happen to match the vcs canonical values already: // "APPROVED", "REQUEST_CHANGES", "COMMENT" — pass through as-is. // If Gitea ever returns other values, map to vcs.ReviewState* constants. ``` **`GetCommitStatuses`** — maps `[]gitea.CommitStatus` → `[]vcs.CommitStatus`: ```go // Map Context, Status, Description, TargetURL from gitea.CommitStatus to vcs.CommitStatus. ``` ### Pass-through methods These delegate directly with no type translation (signature and return type already match): - `GetPullRequestDiff` - `GetFileContent` - `GetAuthenticatedUser` - `DeleteReview` ### `ListContents` Per the alias strategy (see issue #78): `gitea.ContentEntry` should be aliased to `vcs.ContentEntry` (`type ContentEntry = vcs.ContentEntry`). If that alias is in place, `gitea.Client.ListContents` already satisfies the interface with no adapter wrapper needed — it becomes a true pass-through. If for any reason the alias cannot be applied (e.g. a conflicting field), fall back to a field-by-field map (Name, Path, Type — all strings). Add a unit test either way. ### `PostReview` translation Two translations required: **1. Event string** — translate `ReviewRequest.Event` from GitHub canonical to Gitea-native: | `ReviewRequest.Event` (canonical) | Gitea API event string | |-----------------------------------|-----------------------| | `"APPROVE"` | `"APPROVED"` | | `"REQUEST_CHANGES"` | `"REQUEST_CHANGES"` | | `"COMMENT"` | `"COMMENT"` | Note: `"APPROVE"` (no D) → `"APPROVED"` (with D) is the critical translation. `gitea.Client.PostReview` passes the event string verbatim to the Gitea API, which requires `"APPROVED"`. Unit test: verify `"APPROVE"` produces a Gitea API call with event `"APPROVED"`. **2. Diff-position → line number** — for each `ReviewComment.Position`: 1. Fetch the PR diff using `GetPullRequestDiff` 2. Parse unified diff hunks to build a `position → new-file line number` map per file 3. Translate each `ReviewComment.Position` to the corresponding file line number 4. Set `CommitID` from `ReviewComment.CommitID` (pass through to Gitea) ### Diff-position counting rules (GitHub spec) Position is a **per-file** counter. For each file in the diff: - The `@@` hunk header line is position **1** - **Every** subsequent line increments position by 1 — context, additions, AND deletions all count - 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 — map to the nearest non-deletion line below **Example:** ``` @@ -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 ``` ### `DismissReview` ```go func (a *Adapter) DismissReview(ctx context.Context, owner, repo string, number int, reviewID int64, message string) error { // Gitea supports full deletion of any review state. // The message parameter is intentionally unused — Gitea deletion has no dismissal message. return a.client.DeleteReview(ctx, owner, repo, number, reviewID) // a.client is *Client (within package gitea) } ``` ### `GetFileContentAtRef` Pass-through to `gitea.Client.GetFileContentRef`: ```go func (a *Adapter) GetFileContentAtRef(ctx context.Context, owner, repo, path, ref string) (string, error) { return a.client.GetFileContentRef(ctx, owner, repo, path, ref) } ``` ### `GetCommitStatuses` Pass-through to `gitea.Client.GetCommitStatuses` after type mapping (see above). ### Compile-time check ```go var _ vcs.Client = (*Adapter)(nil) ``` ## Edge cases for diff-position parser | Case | Behavior | |------|----------| | Binary file | No diff lines; skip (cannot comment on binary diff position) | | New file (no context) | First `@@` is still position 1; count from there | | Deleted file | Has diff lines but no new-file line numbers; treat like deletion lines | | Position out of range | Return error | | Deletion line targeted | Map to nearest non-deletion line below; if none, return error | ## Exit criteria - `var _ vcs.Client = (*Adapter)(nil)` compiles - All existing Gitea integration tests pass unchanged - Unit tests for diff-position → line-number translation: - Single hunk, multiple hunks - Context, addition, deletion lines (deletions DO count toward position) - New file, deleted file, binary file - Deletion line targeted → maps to line below - Out-of-range position → error - Unit test for Event string translation: `"APPROVE"` → Gitea API receives `"APPROVED"` (not `"APPROVE"`) - Unit tests for all field-mapping methods: `GetPullRequest`, `GetPullRequestFiles`, `ListReviews`, `GetCommitStatuses` ## Out of scope - GitHub client (issues #80, #81) - CLI flag changes (issue #82)
rodin commented 2026-05-12 17:24:49 +00:00
Author
Owner
Copy Link
Copy Source

Update: The Reviewer interface now includes DismissReview (see #81). For the Gitea adapter, implement DismissReview as a call to the underlying gitea.Client.DeleteReview — Gitea supports deletion of any review state, so dismissal and deletion are equivalent outcomes.

**Update:** The `Reviewer` interface now includes `DismissReview` (see #81). For the Gitea adapter, implement `DismissReview` as a call to the underlying `gitea.Client.DeleteReview` — Gitea supports deletion of any review state, so dismissal and deletion are equivalent outcomes.
rodin commented 2026-05-12 17:28:06 +00:00
Author
Owner
Copy Link
Copy Source

Update from codebase audit (issue #82): The Gitea adapter needs two additional pass-through methods beyond PostReview:

  1. GetFileContentAtRef → pass through to gitea.Client.GetFileContentRef
  2. GetCommitStatuses → pass through to gitea.Client.GetCommitStatuses

See #78 update comment for the full interface signatures.

**Update from codebase audit (issue #82):** The Gitea adapter needs two additional pass-through methods beyond PostReview: 1. `GetFileContentAtRef` → pass through to `gitea.Client.GetFileContentRef` 2. `GetCommitStatuses` → pass through to `gitea.Client.GetCommitStatuses` See #78 update comment for the full interface signatures.
rodin self-assigned this 2026-05-12 20:24:16 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ai-review
bug
needs-review
ready
Ready for human review
 self-reviewed
Self-review passed since last push
 wip
Work in progress - do not start new work on this PR
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
security-review-bot rodin (Rodin)
No Assignees rodin
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rodin/review-bot#79
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?