Repository navigation
#419 — render: PR review comments in the report (inline threads + counts + nav + dogfood) - #439
Conversation
…nts + navigation Consolidates #419 + #420 + #421 + #422 (epic #353): the ingested GitHub PR review threads (#395) are anchored onto the rendered diff (the shipped #418 anchoring — re-used verbatim, never re-anchored) and grouped per model, then rendered three ways — gated behind the new `pr-comments` experiment + the `--pr-diff` arm. - #419 inline diff-line comment threads: each anchored thread renders at its Model-SQL diff line (author · body · replies · resolved/open state, GitHub-style). Resolved threads collapse; outdated threads are labeled and surfaced in a per-model tail (never mis-anchored, never dropped). - #420 top-of-report comment-count button: shows the report-wide total and navigates (selects the model via __cuteSelectNode's selectModelByName + scrolls to the anchored diff line). In-page JS only — zero fetch. - #421 per-model count tooltip: the focusable-bubble pattern (.expect-tooltip + CSS bubble on :hover AND :focus, aria-label for AT) — never a native title. - #422 fixture + BDD + dogfood: synthetic --pr-comments review-threads fixture drives the committed comments-showcase golden (visible inline threads + counts); 5 BDD scenarios; headless test asserting inline render, navigation, and tooltip-on-focus; zero-egress gate covers the example. Domain: new pure `pr_comment_render` module (CommentsView / ModelCommentBucket / group_comment_threads) — std+serde only, re-uses anchor_comment_thread. Adapter: additive Option<CommentsView> on ReportPayload (JSON-rendered, the pr_dag precedent). CLI: --pr-comments @file value-parser (the gh-fetch live path resolves owner/repo/number from --pr-url/--pr-number) + gather_pr_comments. Gating: byte-identity goldens when the experiment is OFF / no PR context / no comments (the always-present static containers stay empty). All report goldens regenerated for the JS/CSS bundle shift; explore goldens untouched. Closes #419 Closes #420 Closes #421 Closes #422 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 30 minutes and 38 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
📝 WalkthroughWalkthroughImplements the PR review comments epic ( ChangesPR Review Comments Feature
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as cute-dbt report
participant gather as gather_pr_comments
participant domain as group_comment_threads
participant render as render_report_with_externals
participant HTML as report HTML + interaction.js
User->>CLI: --pr-comments `@fixture.json` --pr-diff patch (experiment=pr-comments)
CLI->>gather: args, scope, experiments
gather->>domain: PrCommentThread[], Manifest, NormalizedDiffIndex
domain-->>gather: CommentsView (by_model buckets + unanchored + total)
gather-->>CLI: Some(CommentsView)
CLI->>render: pr_comments=Some(&CommentsView)
render-->>HTML: ReportPayload JSON with pr_comments embedded
HTML->>HTML: renderPrCommentsCountButton() on boot (unhide button)
HTML->>HTML: renderModelCommentCount(m) on model select
HTML->>HTML: renderModelSql(m) → diff lines with data-oldline/data-newline
HTML->>HTML: anchorModelCommentThreads(m) → inject thread cards
User->>HTML: click pr-comments-count button
HTML->>HTML: navigateToFirstComment() → select model → scrollToThread
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📄 Rendered report previewAll golden examples regenerated cleanly. 🟡 Golden examplesCommitted to
🐶 Live dogfood previewThis PR doesn't touch 🧭 Explore previewThe two-page 🟡 Golden exploreThe committed
🐶 Live exploreThis PR doesn't touch ▶ Open ↗ opens the report or explorer in your browser in one The Pages preview may take ~1 min to update after this comment Alternative: GitHub CLI# gh CLI >= 2.63 extracts into ./report-preview-playground/.
gh run download 27512995203 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.htmlPosted by |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the experimental pr-comments feature, enabling the ingestion and inline rendering of GitHub PR review comments anchored to the diff in the HTML report. The implementation includes CLI argument parsing, domain grouping logic, frontend template updates, and comprehensive headless and integration tests. The feedback suggests a minor optimization in src/cli/mod.rs to avoid an unnecessary clone of args.project_root when calling group_comment_threads.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
src/cli/mod.rs (1)
2074-2075: 💤 Low valueAvoid unnecessary clone of
project_root.
group_comment_threadsacceptsOption<&Path>, so you can pass the reference directly without cloning thePathBuf.🔧 Suggested fix
- let project_root = args.project_root.clone(); - let view = group_comment_threads(&comments.threads, current, index, project_root.as_deref()); + let view = group_comment_threads(&comments.threads, current, index, args.project_root.as_deref());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/mod.rs` around lines 2074 - 2075, The code unnecessarily clones the PathBuf from args.project_root before dereferencing it to pass to group_comment_threads. Since group_comment_threads accepts Option<&Path>, remove the clone() call entirely and pass args.project_root.as_deref() directly to avoid the unnecessary allocation and copy of the PathBuf.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/diff-showcase-report.html`:
- Around line 7427-7438: In the navigateToFirstComment function, the current
implementation unconditionally selects the first thread from bucket.threads
using bucket.threads[0], which can result in selecting an outdated or unplaced
thread. Instead of taking the first thread directly, iterate through the
bucket.threads array to find the first thread that is anchored or valid (not
outdated/unplaced), and pass that thread to the scrollToThread function. This
ensures the jump-to-first-comment button lands on the first live diff anchor as
intended for the `#420` behavior.
In `@examples/macro-heavy-report.html`:
- Around line 7200-7205: The selector that finds the anchor `.diff-line` element
in the $diff container (line 7202) matches hidden/folded-away nodes, causing
comment threads to be incorrectly placed after invisible anchors. Modify the
`.find('.diff-line[' + attr + '="' + t.line + '"]')` selector to exclude hidden
elements by adding a `:not([hidden])` pseudo-class to the selector, or
alternatively, treat folded-away lines the same way the code already handles
missing lines by routing them through the unplaced mechanism instead of
attempting to inject them after an invisible anchor.
- Around line 7204-7205: The current code uses .after($line) for each thread,
which reverses the visual order when multiple threads anchor to the same diff
line. Instead of always anchoring to the original $line, track the previously
injected thread element and insert subsequent threads after that element. After
calling $line.after(buildCommentThread(t)) for the first thread in
bucket.threads, capture the returned injected element and use it as the anchor
for the next insertion, repeating this process so each thread is inserted after
the one before it, preserving their original order.
In `@examples/playground-report.html`:
- Around line 6813-6820: The code currently assigns the first thread from
bucket.threads to the thread variable without checking if it has a live anchor,
which can cause scrollToThread() to jump to an outdated or unplaced thread
instead of the anchored diff line. Instead of directly using bucket.threads[0],
implement logic to scan through bucket.threads and select the first thread that
has a live anchor (you will need to determine what constitutes a "live anchor"
in the thread object), then fall back to bucket.threads[0] only if no thread
with a live anchor is found. Pass the selected thread (or the fallback) to the
scrollToThread() call.
In `@examples/prdiff-minidag-report.html`:
- Around line 6881-6889: The scrollToThread function directly interpolates
thread.__key into a CSS selector string on Line 6884, which will break if
thread.__key contains special characters like quotes, backslashes, or brackets.
Wrap thread.__key with CSS.escape() before inserting it into the querySelector
call to properly escape any special characters and ensure the selector remains
valid regardless of the thread key's content.
In `@examples/seed-showcase-report.html`:
- Around line 6864-6875: In the navigateToFirstComment function, replace the
unconditional selection of bucket.threads[0] with logic that iterates through
bucket.threads to find the first thread that has a live line property and is not
outdated (i.e., thread.line exists and thread.outdated is false). Pass this
valid thread to scrollToThread, or skip to the next bucket iteration if no valid
thread is found in the current bucket. This ensures the top "PR review comments"
button navigates to an anchored diff line rather than to outdated or unplaced
threads.
In `@templates/interaction.js`:
- Around line 877-880: The condition checking whether to insert the comment
thread inline after the anchor line only verifies the element exists ($line &&
$line.length) but does not verify the element is actually visible. When the
anchor line is hidden, inserting after it will effectively hide the thread from
view. Add a visibility check in the condition before calling
$line.after(buildCommentThread(t)) to ensure the line is visible; if the line is
hidden, push the thread to the unplaced array instead to allow it to surface in
the tail fallback.
- Around line 783-794: The loop iterates through pr_comments by_model buckets
and may return prematurely after selecting a non-selectable model or invalid
thread, preventing navigation to a live diff anchor. Modify the
selectModelByName function to return a boolean indicating whether the model
selection was successful (false if not selectable, true on success). Then in the
loop, check the return value of selectModelByName before proceeding to call
scrollToThread and return; if the selection failed, continue to the next
iteration to try the next bucket instead of returning immediately.
In `@templates/report.css`:
- Line 1619: The CSS keyword `currentColor` in the border property violates the
Stylelint `value-keyword-case` rule which requires keywords to be lowercase.
Change `currentColor` to `currentcolor` in the border declaration at line 1619
to normalize the keyword casing and satisfy the linting rule.
---
Nitpick comments:
In `@src/cli/mod.rs`:
- Around line 2074-2075: The code unnecessarily clones the PathBuf from
args.project_root before dereferencing it to pass to group_comment_threads.
Since group_comment_threads accepts Option<&Path>, remove the clone() call
entirely and pass args.project_root.as_deref() directly to avoid the unnecessary
allocation and copy of the PathBuf.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1999ef5d-930e-486f-b657-6656c16bca54
⛔ Files ignored due to path filters (2)
tests/snapshots/golden_report__rendered_report_skeleton.snapis excluded by!**/*.snaptests/snapshots/render_integration__rendered_chrome_jaffle_shop.snapis excluded by!**/*.snap
📒 Files selected for processing (28)
.github/workflows/ci.ymlexamples/comments-showcase-report.htmlexamples/diff-showcase-report.htmlexamples/jaffle-shop-report.htmlexamples/macro-heavy-report.htmlexamples/playground-report.htmlexamples/prdiff-minidag-report.htmlexamples/seed-showcase-report.htmlfeatures/pr_comments.featurelefthook.ymlsrc/adapters/render.rssrc/cli/args.rssrc/cli/mod.rssrc/cli/pr_comments.rssrc/cli/review.rssrc/domain/experimental.rssrc/domain/mod.rssrc/domain/pr_comment_render.rstemplates/interaction.jstemplates/report.csstemplates/report.htmltests/common/mod.rstests/fixture_parse.rstests/fixtures/MANIFEST.tomltests/fixtures/comments-showcase-pr-comments.jsontests/headless_toggle.rstests/steps/mod.rstests/steps/pr_comments.rs
…4rs coverage Addresses the PR #439 review gate failure + CodeRabbit MAJOR correctness bugs: - crap4rs (BLOCKING): cover the two new 0%-coverage cli fns. `owner_repo_from_url` CRAP 30.00 → 5.00 (7 exhaustive branch tests, 100% cov); `resolve_pr_comments` 20.00 → 5.57 (file-payload + fail-soft-no-PR tests). Gate now PASS (worst 20.0). - Top-count button + nav must target a LIVE anchor (CR major): `navigateToFirstComment` picks the first thread with a live `line` + `!outdated` (was `bucket.threads[0]`, which lands on the outdated tail when it sorts first), scanning past non-selectable / anchorless buckets with a graceful fallback. - Folded/hidden diff-line anchors → tail, not inline (CR major): the anchor selector adds `:not([hidden])` so a thread on a folded-away line is routed to the per-model "not shown" tail instead of injected after an invisible line. - Selector injection (CR major): `scrollToThread` locates the thread by a non-selector `getAttribute("data-thread-key")` scan, so a model path with `"`, `\`, or `]` can't break the query. - Thread order (CR minor): multiple threads on one diff line now insert after the previously-injected thread (per-anchor last-inserted tracking), preserving `bucket.threads` order instead of reversing via repeated `.after()`. - project_root borrow (gemini): pass `args.project_root.as_deref()` (no clone). - Declined: the `currentColor` → `currentcolor` stylelint nit — stylelint is not a repo gate and `currentColor` is the established codebase convention (5 existing uses); normalizing one line would make it inconsistent. TDD: two new headless tests, proven RED before the fix — the count button navigating to the LIVE line-4 thread when the bucket's first thread is outdated, and a thread on a folded line landing in the tail (control: a visible-line thread still inlines). All report goldens regenerated for the JS shift; chrome snapshot re-accepted. Refs #419 #420 #421 #422 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merge debrief — #439 (epic #353: PR review comments · #419+#420+#421+#422)Shipped the report's inline PR review-comments surface as one consolidated PR (founder-authorized bundle): inline diff-line threads (author/body/replies/resolved/outdated, #419), top comment-count button→navigate (#420), per-model count focusable-bubble tooltip (#421), and the synthetic fixture + BDD + comments-showcase golden (#422). Domain was shipped (#395 ingestion, #418 anchoring); this was the greenfield render layer. Orchestrator probe (above the bots) — clean on the architecture surfaces: Gate discipline paid off — this PR did NOT merge on the builder's first "all green" claim. The orchestrator's independent CI + bot pass caught real problems the builder's local run missed:
Bot disposition: gemini Gates (fix push, raw exit 0): fmt, clippy Closes #419 / #420 / #421 / #422 → epic #353 complete. Pre-design engineering lane COMPLETE. |
1 participant
DO NOT MERGE — builder PR; the orchestrator gates + merges.
Consolidates #419 + #420 + #421 + #422 (epic #353 — the report's inline PR review-comments surface) as one vertical slice. The comment-domain primitives were already shipped (#395 ingestion, #418 anchoring); this PR is the render layer, which was greenfield.
What shipped
anchor_comment_thread, domain: comment→diff-line anchoring — align PrCommentThread to the rendered diff line (+ outdated handling) #418) and rendered at its line: author · body · replies · resolved/open state, GitHub-style. Resolved threads collapse (a<details>); outdated threads are labeled and surfaced in a per-model tail (honest placement — never mis-anchored, never dropped). Gen-time inlined; view-time zero-egress.selectModelByName→__cuteSelectNode's sibling) and scrolls to the anchored diff line. In-page JS only — no fetch. Absent when no PR context / no comments..expect-tooltip+ CSS bubble on:hoverAND:focus,aria-labelfor AT) — never a nativetitle.--pr-commentsreview-threads fixture drives the committed comments-showcase golden (visible inline threads + counts in the downloadable artifact); 5 BDD scenarios (features/pr_comments.feature); a real-Chrome headless test asserting inline render, navigation, and tooltip-reveal-on-focus; the zero-egress gate covers the new example.Architecture
src/domain/pr_comment_render.rs): new pure module —CommentsView/ModelCommentBucket/RenderedThreadPODs +group_comment_threads(std+serde only; re-usesanchor_comment_thread, maps the resolved path → model node, buckets per model). 11 unit tests.render.rs): additiveOption<CommentsView>onReportPayload(JSON-rendered client-side, thepr_dag/seed_cardsprecedent — no server-rendered template fields).--pr-comments @filevalue-parser (deterministic golden/test seam) + a livegh api graphqlfetch path that resolves owner/repo/number from--pr-url/--pr-number;gather_pr_commentsis the single gating source.Experiment::PrComments; surface is--pr-diff-arm only.Bot-review probe targets
group_comment_threadscalls the shippedanchor_comment_thread(domain: comment→diff-line anchoring — align PrCommentThread to the rendered diff line (+ outdated handling) #418) verbatim and only adds the manifest join (resolvedpath→ model node-id viaoriginal_file_path). Confirm it never re-implements line-anchoring logic — the diff join stays inpr_comment_anchor, the manifest join is the new pass. The JS↔Rust join key is the modelpath(the model payload carriespath, not the node-id), surfaced asModelCommentBucket.model_path.pr-commentsOFF / no PR context / no comments,DATA.pr_commentsis omitted and the always-present static containers (.pr-comments-count,.model-comments-row) stay empty + hidden — every default golden stays byte-identical. The JS/CSS bundle change shifted ALL report goldens (all 7 regenerated; explore goldens untouched). Probe: does any OFF-state path emit comment bytes?<button>+ CSS bubble on:hoverAND:focus(aria-labelfor AT, bubblearia-hidden), never a nativetitle. The headless test asserts the bubble is hidden before focus and revealed on.focus(). Probe: is any load-bearing comment affordance reachable only by hover?Gates (all green, raw exit 0)
cargo fmt --check·clippy --all-targets --locked -D warnings·nextest(2358 pass) ·cargo test --test bdd(31 features / 242 scenarios) ·headless_toggle+headless_zero_egress(real Chrome, 122 + 12 pass) ·cargo doc -D warnings·cargo deny· resource-ref lint · synthetic/asset provenance · domain_clean_arch · non-mirror-guard.Closes #419
Closes #420
Closes #421
Closes #422
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--pr-commentsCLI argument to supply PR review comment data for report rendering.Tests