gitea-pr: show the diff a review comment was left against #98

Merged

branderson merged 1 commits from gitea-pr-hunks into main 2026-08-07 14:19:29 -07:00

Conversation 1 Commits 1 Files Changed 2

+43 -2

dejima-agent commented 2026-08-07 14:07:31 -07:00

Collaborator

Copy Link

Copy Source

Summary generated by LLM agent

gitea-pr comments rendered every inline review comment as path:?. Always, not sometimes: the jq asked for .line, and Gitea's review-comment objects have no line field, so // "?" was the only branch reachable.

[branderson @ README.md:?]
Check whether _the project_ wants it first. "They" is confusing when the subject hasn't been introduced.

The effect is not cosmetic. A reviewer writing "this is confusing" or "let's reword this" is pointing at a passage they can see and the reader cannot; all that survived the fetch was the filename. On a review of any size several comments land on the same file, and some say "this" about a phrase that occurs more than once, so acting on the review means guessing which passage was meant.

That happened on #96: a comment about historical content in a file with two candidate passages, one of which was load-bearing design rationale. Resolving it took a round trip.

The objects already carried it

diff_hunk is the exact text the comment was attached to, and position is its offset into the diff. Neither was being read. Now:

[branderson @ README.md @@118]
@@ -105,0 +115,4 @@
+dejima bounds what a session can reach. It has nothing to say about whether
+what that session wrote should exist. That part is yours.
+
+**Check whether they want it first.** A growing number of projects state that
--- comment ---
Check whether _the project_ wants it first. "They" is confusing when the subject hasn't been introduced.

position is an index into the diff rather than a file line, so it is printed as a diff offset rather than as something to jump to. Both fields are null-guarded: an outdated comment whose hunk Gitea no longer has renders (no hunk recorded) instead of the string null.

The hunk makes the output longer, deliberately. Scrolling costs nothing; guessing wrong costs a round trip through a human.

Testing

comments had no coverage at all. The shared curl stub in static.bats answered every read with [], so the reviews list came back empty, the per-review comment fetch never ran, and the rendering under test was never reached. The stub now answers the two review endpoints with shaped data and everything else exactly as before.

One test. Confirmed to fail against the previous gitea-pr, at the assertion for the hunk text rather than incidentally:

not ok 1 an inline review comment arrives with the diff it was left against
#   `[[ "$output" == *"a line the reviewer is looking at"* ]]' failed

static 202 and providers 65, both green. No other suite touches that stub.

Independent of #96 and touches none of its files.

**Summary generated by LLM agent** `gitea-pr comments` rendered every inline review comment as `path:?`. Always, not sometimes: the jq asked for `.line`, and Gitea's review-comment objects have no `line` field, so `// "?"` was the only branch reachable. ``` [branderson @ README.md:?] Check whether _the project_ wants it first. "They" is confusing when the subject hasn't been introduced. ``` The effect is not cosmetic. A reviewer writing "this is confusing" or "let's reword this" is pointing at a passage they can see and the reader cannot; all that survived the fetch was the filename. On a review of any size several comments land on the same file, and some say "this" about a phrase that occurs more than once, so acting on the review means guessing which passage was meant. That happened on #96: a comment about historical content in a file with two candidate passages, one of which was load-bearing design rationale. Resolving it took a round trip. ## The objects already carried it `diff_hunk` is the exact text the comment was attached to, and `position` is its offset into the diff. Neither was being read. Now: ``` [branderson @ README.md @@118] @@ -105,0 +115,4 @@ +dejima bounds what a session can reach. It has nothing to say about whether +what that session wrote should exist. That part is yours. + +**Check whether they want it first.** A growing number of projects state that --- comment --- Check whether _the project_ wants it first. "They" is confusing when the subject hasn't been introduced. ``` `position` is an index into the diff rather than a file line, so it is printed as a diff offset rather than as something to jump to. Both fields are null-guarded: an outdated comment whose hunk Gitea no longer has renders `(no hunk recorded)` instead of the string `null`. The hunk makes the output longer, deliberately. Scrolling costs nothing; guessing wrong costs a round trip through a human. ## Testing `comments` had no coverage at all. The shared curl stub in `static.bats` answered every read with `[]`, so the reviews list came back empty, the per-review comment fetch never ran, and the rendering under test was never reached. The stub now answers the two review endpoints with shaped data and everything else exactly as before. One test. **Confirmed to fail against the previous `gitea-pr`**, at the assertion for the hunk text rather than incidentally: ``` not ok 1 an inline review comment arrives with the diff it was left against # `[[ "$output" == *"a line the reviewer is looking at"* ]]' failed ``` `static` 202 and `providers` 65, both green. No other suite touches that stub. Independent of #96 and touches none of its files.

dejima-agent added 1 commit 2026-08-07 14:07:32 -07:00

gitea-pr: show the diff a review comment was left against ... 2222aef7d1

`gitea-pr comments` rendered every inline review comment as `path:?`. Not
sometimes: always. The jq asked for `.line`, and Gitea's review-comment
objects have no `line` field, so `// "?"` was the only branch that could ever
be taken.

The effect is not cosmetic. A reviewer writing "this is confusing" or "let's
reword this" is pointing at a passage they can see and the reader cannot. All
that survived the fetch was the file. On a review of any size, several
comments land on the same file, and some of them say "this" about a phrase
that occurs more than once, so acting on the review means guessing which
passage was meant and sometimes guessing wrong. That happened on #96, on a
comment about historical content in a file with two candidate passages, and
cost a round trip to resolve.

The objects do carry what is needed: `diff_hunk`, which is the exact text the
comment was attached to, and `position`, the offset into the diff. Both are
printed now, with the comment marked off from the quoted code so the two
cannot be read as one block.

The hunk makes the output longer, and that is the right trade: scrolling
costs nothing, and guessing wrong costs a round trip through a human.

## Testing

The shared curl stub answered every read with `[]`, which meant `comments`
had no coverage at all: the reviews list came back empty, so the per-review
comment fetch never ran and the rendering under test was never reached. The
stub now answers the two review endpoints with shaped data and everything
else as before.

One test, and it was confirmed to fail against the previous `gitea-pr`,
at the assertion for the hunk text rather than incidentally.

static 202 and providers 65, both green.

branderson approved these changes 2026-08-07 14:19:24 -07:00

branderson merged commit 0aef363910 into main 2026-08-07 14:19:29 -07:00

branderson referenced this issue from a commit 2026-08-07 14:19:30 -07:00

Merge pull request 'gitea-pr: show the diff a review comment was left against' (#98) from gitea-pr-hunks into main

Sign in to join this conversation.

Reviewers

No Reviewers

branderson

Labels

Clear labels

Compat/Breaking

Breaking change that won't be backward compatible

Kind/Bug

Something is not working

Kind/Documentation

Documentation changes

Kind/Enhancement

Improve existing functionality

Kind/Feature

New functionality

Kind/Security

This is security issue

Kind/Testing

Issue or pull request related to testing

Priority

Critical

1

The priority is critical

Priority

High

2

The priority is high

Priority

Low

4

The priority is low

Priority

Medium

3

The priority is medium

Reviewed

Confirmed

1

Issue has been confirmed

Reviewed

Duplicate

2

This issue or pull request already exists

Reviewed

Invalid

3

Invalid issue

Reviewed

Won't Fix

3

This issue won't be fixed

Status

Abandoned

3

Somebody has started to work on this but abandoned work

Status

Blocked

1

Something is blocking this issue or pull request

Status

Need More Info

2

Feedback is required to reproduce issue or to continue work

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

branderson dejima-agent

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dejima/dejima#98