feat(api): expose provider PR review summaries on the session PR API by codebanditssss · Pull Request #3011 · AgentWrapper/agent-orchestrator

codebanditssss · 2026-07-23T09:50:12Z

Summary

Step 3 of the reviews-tab revamp (#2433): expose each reviewer's submitted review summary (verdict + body) through the session PR API, so the frontend can render a verdict with its summary text.

sessionsvc.PRReviewSummary gains Reviews []PRReviewEntry. summarizeReview populates it from the latest decisive (approved / changes_requested) review per reviewer, sorted by reviewer for deterministic output.
Controller DTO gains SessionPRReviewEntry (reviewerId, verdict, body, reviewUrl, submittedAt, isBot) and a reviews field on SessionPRReviewSummary.
openapi.yaml + schema.ts regenerated.

Combines the plan's "add API shape" and "populate summarizeReview" steps, since a plumbed-but-unpopulated field isn't a reviewable unit on its own.

Privacy

Inline review comment bodies stay private — they remain folded into unresolvedBy counts and links, never surfaced as raw text. Only the top-level review summary body is exposed. The renamed test TestListPRSummariesExposesReviewSummariesButKeepsRawLogsAndCommentBodiesPrivate asserts both: the review summary is surfaced, and comment bodies + CI log tails never leak into the summary.

`reviews` is optional

Added with omitempty, so reviews? is optional in the generated schema. This keeps the field backward-compatible for existing clients and avoids forcing unrelated fixture churn onto this PR; the step-5 UI will consume it with a ?? [] default.

Stacked on #3010 → #3009

⚠️ Based on feat/scm-fetch-pr-review-body (#3010), which is based on feat/persist-pr-review-body (#3009). Merge #3009 → #3010 → this, in order. Review only the top commit.

Tests

cd backend && go build ./... && go vet ./internal/service/session/ ./internal/httpd/controllers/ clean
go test ./internal/service/session/ ./internal/httpd/controllers/ ./internal/httpd/apispec/... pass
npm run api:spec + openapi-typescript regen — schema.ts change is purely additive
cd frontend && npx tsc --noEmit clean (frontend diff is generated schema.ts only)
gofmt clean

Surface each reviewer's latest decisive review (verdict + submitted body) through the session PR summary so the reviews UI can show a verdict with its summary text. - PRReviewSummary gains Reviews []PRReviewEntry; summarizeReview populates it from the latest approved/changes_requested review per reviewer, sorted by reviewer for determinism - controller DTO gains SessionPRReviewEntry and a reviews field (omitempty, so it stays optional for existing clients) - openapi.yaml + schema.ts regenerated Inline review comment bodies stay private, folded into UnresolvedBy counts and links as before; only the top-level review summary body is exposed. Part of the reviews-tab revamp (#2433); the frontend two-section layout that consumes this is the next PR.

Pulkit7070 · 2026-07-23T12:56:38Z

 		}
 		reviewURLByAuthor[reviewer] = review.URL
 		isBot[reviewer] = review.IsBot
 	}


Reviews here is built from latestChangesRequestedReviews, which only returns reviewers whose latest review is changes_requested (approved reviews are computed and then dropped at the final filter). So verdict is always changes_requested in practice, and an approved review's summary body is never surfaced, even though the doc comment says "latest decisive submitted review per reviewer" and the enum advertises approved/review_required.

Is dropping approved summaries intended? If not, populate these entries from the full latest-per-reviewer set rather than the changes-requested-only subset, so an approved review with a summary body is exposed too.

good catch, you are right. latestChangesRequestedReviews computes the latest decisive review per reviewer and then drops anyone whose latest is approved, so an approved review body never got surfaced even though the field advertises the approved verdict.

split it into latestDecisiveReviews (latest approved-or-changes_requested per reviewer, no drop) and Reviews now builds from that. the unresolved-comment grouping keeps using the changes-requested-only subset so that behaviour is unchanged. added a service test asserting an approved review body is surfaced. fixed in 5218046.

Pulkit7070 · 2026-07-23T12:56:39Z

 	if pr.Mergeability.State != domain.MergeConflicting || len(pr.Mergeability.ConflictFiles) != 0 || !containsString(pr.Mergeability.Reasons, "conflicts") {
 		t.Fatalf("mergeability = %+v", pr.Mergeability)
 	}
+	if len(pr.Review.Reviews) != 1 {


Consider adding a short assertion over newSessionPRReviewSummary output so the domain-to-DTO field mapping (Reviewer -> reviewerId, URL -> reviewUrl, etc.) is directly covered rather than only via compile plus the api-drift check.

done. added a controller test over NewSessionPRSummary that covers the domain to DTO mapping directly (Reviewer to reviewerId, URL to reviewUrl, plus verdict/body/submittedAt/isBot) rather than relying on compile + api-drift. 5218046.

summarizeReview built the Reviews list from latestChangesRequestedReviews, which drops every reviewer whose latest decisive verdict is approved, so an approved review's summary body was never exposed despite the field advertising the approved verdict. Split out latestDecisiveReviews (latest approved-or-changes_requested per reviewer) and build Reviews from it, while the unresolved-comment grouping keeps using the changes-requested subset. Add a service test asserting an approved review's body is surfaced and a controller test covering the domain-to-DTO field mapping (Reviewer -> reviewerId, URL -> reviewUrl, etc.).

.

illegalcall

Standards

[P2] Register the new API component in schemaNames. The generated schema exposes ControllersSessionPRReviewEntry at backend/internal/httpd/apispec/openapi.yaml:2151, leaking the internal package prefix into the public API and generated TypeScript. AGENTS.md requires every new named contract type to be registered. Add:
```
"ControllersSessionPRReviewEntry": "SessionPRReviewEntry",
```
beside the other session PR mappings in specgen/build.go, then run npm run api.

No additional standards or code-smell findings.

Spec

No findings. Review selection, deterministic ordering, DTO mapping, optional reviews, privacy boundaries, and unresolved-comment grouping match issue #2433 and the PR description.

Verification

Focused service/controller/API tests and go vet passed; all GitHub checks are green. The full local backend suite encountered unrelated timing and macOS /private/var canonicalization failures. Frontend typecheck could not run independently because the isolated worktree lacks dependencies; GitHub frontend checks pass.

Summary: Standards — 1 finding, worst P2; Spec — 0 findings.

illegalcall

added review

The reflected DTO surfaced as ControllersSessionPRReviewEntry, leaking the internal package prefix into the public OpenAPI spec and generated TypeScript. Register it in schemaNames so it emits as SessionPRReviewEntry, matching the other session PR contract types, and regenerate the spec + client.

codebanditssss · 2026-07-23T17:18:48Z

@illegalcall good catch, fixed. registered ControllersSessionPRReviewEntry -> SessionPRReviewEntry in schemaNames (specgen/build.go) next to the other session PR contract types, and regenerated the spec + client with npm run api. the openapi component and generated TS now emit as SessionPRReviewEntry, no package prefix. 4d8867f.

… (#3046) * feat(api): expose provider PR review summaries on the session PR API Surface each reviewer's latest decisive review (verdict + submitted body) through the session PR summary so the reviews UI can show a verdict with its summary text. - PRReviewSummary gains Reviews []PRReviewEntry; summarizeReview populates it from the latest approved/changes_requested review per reviewer, sorted by reviewer for determinism - controller DTO gains SessionPRReviewEntry and a reviews field (omitempty, so it stays optional for existing clients) - openapi.yaml + schema.ts regenerated Inline review comment bodies stay private, folded into UnresolvedBy counts and links as before; only the top-level review summary body is exposed. Part of the reviews-tab revamp (#2433); the frontend two-section layout that consumes this is the next PR. * fix(api): surface approved review summaries, not just changes-requested summarizeReview built the Reviews list from latestChangesRequestedReviews, which drops every reviewer whose latest decisive verdict is approved, so an approved review's summary body was never exposed despite the field advertising the approved verdict. Split out latestDecisiveReviews (latest approved-or-changes_requested per reviewer) and build Reviews from it, while the unresolved-comment grouping keeps using the changes-requested subset. Add a service test asserting an approved review's body is surfaced and a controller test covering the domain-to-DTO field mapping (Reviewer -> reviewerId, URL -> reviewUrl, etc.). * fix(api): register SessionPRReviewEntry schema name The reflected DTO surfaced as ControllersSessionPRReviewEntry, leaking the internal package prefix into the public OpenAPI spec and generated TypeScript. Register it in schemaNames so it emits as SessionPRReviewEntry, matching the other session PR contract types, and regenerate the spec + client.

codebanditssss requested a review from Vaibhaav-Tiwari July 23, 2026 10:01

Pulkit7070 reviewed Jul 23, 2026

View reviewed changes

illegalcall previously approved these changes Jul 23, 2026

View reviewed changes

illegalcall reviewed Jul 23, 2026

View reviewed changes

illegalcall requested changes Jul 23, 2026

View reviewed changes

codebanditssss merged commit dcbe5b4 into feat/scm-fetch-pr-review-body Jul 23, 2026
11 checks passed

codebanditssss mentioned this pull request Jul 23, 2026

fix(api): re-land review-summaries API on main (recover stranded #3011) #3046

Merged

codebanditssss mentioned this pull request Jul 23, 2026

fix(scm): re-land review-body fetch/mapping on main (recover stranded #3010) #3049

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(api): expose provider PR review summaries on the session PR API#3011

feat(api): expose provider PR review summaries on the session PR API#3011
codebanditssss merged 3 commits into
feat/scm-fetch-pr-review-bodyfrom
feat/expose-pr-review-body-api

codebanditssss commented Jul 23, 2026

Uh oh!

Pulkit7070 Jul 23, 2026

Uh oh!

codebanditssss Jul 23, 2026

Uh oh!

Pulkit7070 Jul 23, 2026

Uh oh!

codebanditssss Jul 23, 2026

Uh oh!

illegalcall left a comment

Uh oh!

illegalcall left a comment

Uh oh!

codebanditssss commented Jul 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

codebanditssss commented Jul 23, 2026

Summary

Privacy

reviews is optional

Stacked on #3010 → #3009

Tests

Uh oh!

Pulkit7070 Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

codebanditssss Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Pulkit7070 Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

codebanditssss Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

illegalcall left a comment

Choose a reason for hiding this comment

Standards

Spec

Verification

Uh oh!

illegalcall left a comment

Choose a reason for hiding this comment

Uh oh!

codebanditssss commented Jul 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`reviews` is optional