brooktrails/pr-reviewer
2
0
Fork 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity Actions

feat: per-comparison-group A/B comparison view at /admin/comparisons/{id} #56

Merged
rcsheets merged 1 commit from feat/admin-compare-page into main 2026-05-01 12:45:29 +00:00
Conversation 1 Commits 1 Files changed 8 +220 -18
rcsheets commented 2026-05-01 12:42:40 +00:00
Owner
Copy link

Adds a dedicated admin page that lays out every config's review of a
single PR side-by-side, addressed by the comparison_group_id that
already glues primary + shadow rows together. The pills on
/admin/reviews now wrap a link to this page, so clicking any config
on any row goes straight to the side-by-side view instead of opening
the primary review and scrolling to the existing collapsed
"A/B comparison" section.

Layout uses CSS grid with auto-fit minmax(420px, 1fr), so two configs
sit side-by-side on desktop and stack on narrower viewports without
JS or breakpoints. Each column shows the config name, primary/shadow
pill, verdict pill, slug, complexity, token usage, duration, and the
full review body in a scrollable region. Failed entries surface the
error message in red instead of the body, so a flaky shadow doesn't
get silently rendered as "(No review body returned)".

ConfigsByIDs added to the tracker so the handler can resolve all
configs for the group in one query rather than per-row.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

Adds a dedicated admin page that lays out every config's review of a single PR side-by-side, addressed by the comparison_group_id that already glues primary + shadow rows together. The pills on /admin/reviews now wrap a link to this page, so clicking any config on any row goes straight to the side-by-side view instead of opening the primary review and scrolling to the existing collapsed "A/B comparison" section. Layout uses CSS grid with auto-fit minmax(420px, 1fr), so two configs sit side-by-side on desktop and stack on narrower viewports without JS or breakpoints. Each column shows the config name, primary/shadow pill, verdict pill, slug, complexity, token usage, duration, and the full review body in a scrollable region. Failed entries surface the error message in red instead of the body, so a flaky shadow doesn't get silently rendered as "(No review body returned)". ConfigsByIDs added to the tracker so the handler can resolve all configs for the group in one query rather than per-row. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feat: per-comparison-group A/B comparison view at /admin/comparisons/{id}
All checks were successful
ci / check (pull_request) Successful in 42s
Details
38b9e370fe 
Adds a dedicated admin page that lays out every config's review of a
single PR side-by-side, addressed by the comparison_group_id that
already glues primary + shadow rows together. The pills on
/admin/reviews now wrap a link to this page, so clicking any config
on any row goes straight to the side-by-side view instead of opening
the primary review and scrolling to the existing collapsed
"A/B comparison" section.

Layout uses CSS grid with auto-fit minmax(420px, 1fr), so two configs
sit side-by-side on desktop and stack on narrower viewports without
JS or breakpoints. Each column shows the config name, primary/shadow
pill, verdict pill, slug, complexity, token usage, duration, and the
full review body in a scrollable region. Failed entries surface the
error message in red instead of the body, so a flaky shadow doesn't
get silently rendered as "(No review body returned)".

ConfigsByIDs added to the tracker so the handler can resolve all
configs for the group in one query rather than per-row.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pr-reviewer-bot commented 2026-05-01 12:42:40 +00:00
Collaborator
Copy link

Automated review by pr-reviewer v0.24.0 | Full Review | Claude Sonnet 4 | tracking id r-f49fc0-d8ae46
This is an AI-generated review and may contain mistakes.

Status: ✅ Completed

✅ Verdict: Approve — Well-structured implementation of an A/B comparison view with good database access patterns and clean CSS grid layout.
Complexity: moderate

Review: A/B Comparison View Implementation

This PR adds a dedicated admin page for side-by-side review comparisons. The implementation looks solid with good separation of concerns and efficient database access patterns.

Strengths

Clean Architecture: The handler follows the established pattern with proper error handling, permission checks via RequireAdmin, and structured data projection through compareEntry.

Efficient Database Access: Using ConfigsByIDs() for bulk config lookups and ListComparisonGroup() for review events avoids N+1 queries. The fallback to e.Backend when config lookup fails is a good defensive programming practice.

Responsive CSS Grid: The auto-fit grid layout with minmax(420px, 1fr) provides responsive behavior without JavaScript - configs stack on mobile and sit side-by-side on desktop.

Template Improvements:

  • The toSeconds template function is a nice utility addition
  • The pill-link styling with hover effects improves UX
  • Error handling displays meaningful error messages instead of empty bodies

Data Consistency: Adding ComparisonGroupID to reviewRow enables proper linking from the admin reviews list to the comparison view.

Technical Notes

Route Structure: /admin/comparisons/{id} is well-chosen - it's under admin protection and clearly indicates its purpose.

Error Handling: Good progressive degradation - if config lookup fails, the page still renders with backend names. 404 handling for missing comparison groups is appropriate.

Template Organization: Embedding styles in the template is acceptable for this specialized view, and the CSS variables maintain consistency with the existing theme.

Performance: The bulk queries and single-shot config resolution show good attention to database performance.

Minor Observations

The primary := events[0] assumption relies on ListComparisonGroup ordering, which is documented in the method and appears correct.

The comparison view could potentially show a large number of configs in one group, but the CSS grid handles this gracefully by creating additional columns.

Conclusion

This is a well-implemented feature that follows established patterns and provides clear value for A/B testing workflows. The code is clean, the database access is efficient, and the UX improvements (clickable pills, side-by-side layout) are thoughtful additions.

📚 Context used in this review

Project guidance files:

  • CLAUDE.md

Repository fragments:

  • internal/tracker/dashboard.go (internal/tracker/dashboard.go:96-166) — extracted in answer to: "The definition of ConfigOutcome structure and ConfigOutcomesForGroups method, plus ListComparisonGroup method to understand the data structures being used in the new admin comparison view"
*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.24.0 | Full Review | Claude Sonnet 4 | tracking id `r-f49fc0-d8ae46`* *This is an AI-generated review and may contain mistakes.* **Status:** ✅ Completed --- **✅ Verdict: Approve** — Well-structured implementation of an A/B comparison view with good database access patterns and clean CSS grid layout. **Complexity:** moderate # Review: A/B Comparison View Implementation This PR adds a dedicated admin page for side-by-side review comparisons. The implementation looks solid with good separation of concerns and efficient database access patterns. ## Strengths **Clean Architecture**: The handler follows the established pattern with proper error handling, permission checks via `RequireAdmin`, and structured data projection through `compareEntry`. **Efficient Database Access**: Using `ConfigsByIDs()` for bulk config lookups and `ListComparisonGroup()` for review events avoids N+1 queries. The fallback to `e.Backend` when config lookup fails is a good defensive programming practice. **Responsive CSS Grid**: The auto-fit grid layout with `minmax(420px, 1fr)` provides responsive behavior without JavaScript - configs stack on mobile and sit side-by-side on desktop. **Template Improvements**: - The `toSeconds` template function is a nice utility addition - The pill-link styling with hover effects improves UX - Error handling displays meaningful error messages instead of empty bodies **Data Consistency**: Adding `ComparisonGroupID` to `reviewRow` enables proper linking from the admin reviews list to the comparison view. ## Technical Notes **Route Structure**: `/admin/comparisons/{id}` is well-chosen - it's under admin protection and clearly indicates its purpose. **Error Handling**: Good progressive degradation - if config lookup fails, the page still renders with backend names. 404 handling for missing comparison groups is appropriate. **Template Organization**: Embedding styles in the template is acceptable for this specialized view, and the CSS variables maintain consistency with the existing theme. **Performance**: The bulk queries and single-shot config resolution show good attention to database performance. ## Minor Observations The `primary := events[0]` assumption relies on `ListComparisonGroup` ordering, which is documented in the method and appears correct. The comparison view could potentially show a large number of configs in one group, but the CSS grid handles this gracefully by creating additional columns. ## Conclusion This is a well-implemented feature that follows established patterns and provides clear value for A/B testing workflows. The code is clean, the database access is efficient, and the UX improvements (clickable pills, side-by-side layout) are thoughtful additions. <details> <summary>📚 Context used in this review</summary> **Project guidance files:** - `CLAUDE.md` **Repository fragments:** - `internal/tracker/dashboard.go` (internal/tracker/dashboard.go:96-166) — extracted in answer to: "The definition of ConfigOutcome structure and ConfigOutcomesForGroups method, plus ListComparisonGroup method to understand the data structures being used in the new admin comparison view" </details>
rcsheets deleted branch feat/admin-compare-page 2026-05-01 12:45:29 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
pr-reviewer-bot
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
brooktrails/pr-reviewer!56
No description provided.