Skip to content

#240 — review-2 render defects: tooltip overflow, given-header tooltips, covered-by contrast, overrides-badge clip, baseline banner - #244

Merged
cmbays merged 7 commits into
mainfrom
adapters-240-review2-render-defects
Jun 11, 2026
Merged

cmbays merged 7 commits into
mainfrom
adapters-240-review2-render-defects

Conversation

@cmbays

@cmbays cmbays commented Jun 11, 2026 •

Copy link
Copy Markdown
Contributor

Closes #240

Five render defects from the founder's review of the PR #239 sticky-comment reports. Phase 0 reproduced every defect on the exact artifacts the founder reviewed (recovered byte-identical from gh-pages history commit e97ff1a, since the pr-239 Pages preview was cleaned up post-merge) plus a faithful local dbt-fusion-compiled dbt-project twin (dbt compile + --pr-diff on a synthetic remove-one-line patch; manifest ephemeral, never committed). All validation below is real Playwright pointer interaction (mouse.move/hover/real mouseover events) on rebuilt artifacts.

Note on the published pr-239 dbt-project row: it was rendered from PR #239's own diff (a README-only touch) and is 0-unit-tests-in-scope — the synthea surfaces named in the issue live in the playground/diff-showcase fixtures, where every defect reproduced exactly.

A — model-tip content overflows the bubble

Root cause .ct-key (mono, bold) had no overflow-wrap; the bubble caps at max-width: 32rem (320px @ the 10px Sakura root). The 383px-wide unbreakable token dbt_expectations.expect_table_row_count_to_be_between painted to x=444 against a bubble right edge of x=389 — 55px past the painted background (measured on the exact playground artifact, light theme, mart_dq_summary's stg_synthea__medications given pill — the issue's exact surface).

Fix overflow-wrap: anywhere on the .col-tooltip shell + .ct-val chips wrap (break only on overflow — short args stay one-line chips).

Validation (rebuilt golden, same hover) bubble 336px, scrollWidth 336 == clientWidth 336, worst descendant overflow 0px (was 55px); the long name wraps onto two lines inside the dark background (before/after screenshots captured).

B — given column-header tooltips still absent (THIRD report)

Root cause (hypothesis 3, in its systemic form) — the issue's hypotheses were tested in order on the real artifacts:

  1. Decorations wiped by re-render/toggle/sort — not reproduced: real hovers on decorated given headers pass in the Diff view, after Diff→File toggle, after sort, and after the round-trip (diff-showcase stg_synthea__patients; playground dim_payers).
  2. Inner-element trigger — not reproduced: the whole-th trigger works wherever decorated.
  3. Metadata genuinely absent — confirmed, and bigger than one node: the feature: column-header tooltips on unit-test tables — authored descriptions + column-level data tests #165/feature: design integration PR-2 — engine merge (settings panel, unified/split diffs) + port-forward of post-fork behaviors #178 design mapped "no manifest metadata for this column" → no affordance (silent dead header). Real projects under-declare staging columns while target models are richly declared, so given headers are mostly hover-dead while every expected header answers. On the founder's exact surface (playground mart_dq_summary) the given fixtures use DQ-flag columns (is_dq_valid, …) the stg models never declare (the int_dq_quarantine__* twins declare them) — all 10 given headers dead, all 5 expected headers alive. On the dbt-project twin: stg_orders givens decorated 2-of-4 (customer_id/order_date dead), expected 9-of-9.

Why #166 and #236 passed while reality stayed broken (the meta-root-cause) Both prior fixes repaired subsets where metadata existed but wasn't delivered (#166 shipped column_meta; #236 resolved seed/source given nodes), and their guards asserted reveal on columns known to carry metadata — existential assertions, true before and after each fix. The founder hovers arbitrary given columns — usually undeclared ones — which were hover-dead by design and outside every assertion universe. The validations didn't lie about what they measured; they measured the wrong quantifier.

Fix (deliberate contract revision) Every fixture column header (given + expected, Current + Diff views) is now a tooltip trigger. Columns with manifest metadata keep today's rich bubble; metadata-less columns reveal a truthful fallback — customer_id — No description or data tests declared on stg_orders in the project YAML. — naming the owning node (input model / seed / source.table / target model for this and expect). The #146/#161 contract holds (focusable trigger, aria-label, aria-hidden bubble, hover AND focus reveal); the #178 "never an empty bubble" honesty invariant is preserved and strengthened: never empty, and never dead either. Three #235-era guards that pinned "no metadata ⇒ no trigger" are rewritten to pin the new contract.

Guard-methodology change fixture_header_tooltips_universal_after_view_toggle_and_sort quantifies universally: every visible given/expected header, real mouseover, non-empty bubble required — at initial render, after a real Diff→File toggle, after a real column sort, and after the round-trip. The dead-header state no longer exists, so this regression class cannot recur silently.

Validation — every given table in all 4 rebuilt reports (real hover per header; count = visible given+expected headers swept; after-toggle sweeps where a Diff/File toggle exists; after-sort everywhere; zero failures):

report model / test headers toggle sort
playground dim_payers 24 ✓ — ✓
playground fct_encounters_incremental 15 ✓ — ✓
playground mart_date_state_grid 11 ✓ — ✓
playground mart_dq_summary ×2 (the founder's surface) 15 ✓ / 14 ✓ — ✓
playground stg_synthea__patients 14 ✓ — ✓
diff-showcase mart_date_state_grid 11 ✓ ✓ (1) ✓
diff-showcase mart_dq_summary ×2 15 ✓ / 14 ✓ ✓ (1) ✓
jaffle-shop stg_customers 6 ✓ — ✓
dbt-project twin all 7 unit tests 12/8/12/17/17/17/11 ✓ — ✓

Also re-swept at 1680px and 375px viewports (17/17, zero failures). Expected-table parity is now structural: wherever the manifest carries metadata the given bubble shows it (verified, e.g. order_id → "Unique identifier for an order (staging grain)" + data tests); where it doesn't, the bubble says so truthfully instead of staying silent.

C — "Covered by" test names illegible on dark themes

Root cause Sakura 1.5.0's base code { background-color: #f1f1f1 } was never themed for .finding-covered-by code; dark themes pair the near-white --text with that light constant — measured 1.24:1 on the exact artifact (rgb(215,218,225) on rgb(241,241,241)) — the founder's "near-white pills".

Fix themed re-skin background: var(--bg-alt); color: var(--text) (the .td-defined-in idiom). Suppressed-row and chip-token overrides from #227/#228/#239 untouched; #238's ov-tooltip palette untouched.

Validation — all 8 themes, effective-backdrop methodology (computed styles, backdrop walk to the chip's own opaque fill): light 11.19 · solarized 5.92 · latte 6.57 · rosepine 6.82 · dark 11.92 · tokyo 9.63 · gruvbox 9.57 · dracula 14.81 — all ≥4.5 AA. Guard covered_by_test_ids_meet_aa_contrast_on_every_theme exercises a real Verdict::Covered union finding (both arms fed by givens) and was RED at 1.2 pre-fix.

D — overrides badge tooltip clips at the viewport edge

Reproduction record (honest) A hard viewport clip did not reproduce on the recovered artifacts: real hovers on the overrides · 1/2 badges at 375/848/1024/1280/1680 showed the singleton clamp holding the bubble box inside the viewport every time. What the artifacts do carry is the latent mechanism matching the founder's symptom exactly: .ov-tooltip rows were white-space: nowrap inside the 32rem cap, so any override value longer than the cap paints past the bubble's painted background rightward and past the screen edge at a right-edge trigger (the same overflow family as defect A) — confirmed RED by the synthetic long-value guard. Additionally positionTipNear clamped against window.innerWidth, which exceeds the visible width by a classic scrollbar's gutter.

Fix (the same edge-aware mechanism family, geometry-JS + CSS) (1) ov rows flex-wrap + overflow-wrap: anywhere (colors verbatim — #238 owns that palette); (2) all singleton bubbles cap at min(24|32rem, calc(100vw - 16px)) (the #232/#234 cap idiom) so the measured box always fits the clamp range; (3) positionTipNear clamps against documentElement.clientWidth/Height (scrollbar-safe). The badge keeps its #146 contract (tabindex, role=button, hover+focus reveal).

Validation Guard ov_tip_long_value_contained_in_bubble_and_viewport_at_right_edge: a long synthetic env_var value + the trigger parked at a true right-edge geometry — content containment (scrollWidth ≤ clientWidth+1, RED pre-fix) AND bubble box fully inside clientWidth. On the rebuilt artifacts: real hover at 1680px (badge near right edge: bubble right 1324 ≤ vw 1332) and 375px (right 280 ≤ vw 288), box-in-viewport ✓, content contained ✓, 0px descendant overflow.

E — baseline-manifest banner illegible in dark mode

Root cause identical to C: code.diff-scope-baseline (the baseline path in the banner) on Sakura's #f1f1f1 — measured 1.23:1 on dark on the exact playground artifact.

Fix same themed re-skin (.diff-scope-line code).

Validation same 8-theme sweep numbers as C (5.92–14.81, all ≥4.5); guard baseline_banner_path_meets_aa_contrast_on_every_theme was RED on all 4 dark themes pre-fix; dark-theme screenshot shows the path legible on its dark chip.

RED→GREEN discipline

All five new guards were run against the unfixed templates (fixes stashed): 5/5 failed, each for its defect's exact mechanism (covered-by 1.24:1, baseline path sub-AA on dark, dead headers in the universal sweep, both containment pins). Unstashed: 5/5 pass. Full headless suite 61/61; zero-egress 10/10.

Goldens audit

git diff --text -U0 -- examples/ — every hunk in the three regenerated reports (jaffle-shop, playground, diff-showcase) is the embedded template change (the interaction.js owner-threading/fallback/clamp + the report.css wrap/cap/code-theming rules, ×3 reports). No other content changed; zero root_path/absolute-path strings (grep-verified). examples/explore/ untouched (different templates). The one insta snapshot embedding template text regenerated likewise.

Gates (run directly — lefthook skips in fresh worktrees)

cargo fmt --check ✓ · cargo clippy --all-targets --locked -- -D warnings exit 0 ✓ · cargo nextest run 1228/1228 ✓ · cargo test --test bdd 160 scenarios / 1032 steps ✓ · headless pair --ignored 61 + 10 ✓ · RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --locked ✓ · cargo deny check ✓

Live preview

The README touch in dbt-project/ renders the live dbt-project row on this PR's sticky comment — the founder can validate these exact defects there (given headers on the stg_orders/stg_payments fixtures now all answer; the overrides · 2 badge on order_metrics; dark-theme banner/covered-by).

🤖 Generated with Claude Code


Open in Stage

Summary by CodeRabbit

  • New Features

    • Column headers now display ownership information, showing which model or source a column originates from.
    • Metadata-less columns display meaningful fallback messages instead of empty tooltips.
  • Bug Fixes

    • Improved tooltip styling and wrapping to prevent text clipping at viewport edges.
    • Enhanced tooltip positioning logic for better viewport containment.
    • Improved code contrast within tooltips on dark themes.
  • Documentation

    • Updated README to clarify dogfood project validation for docs-only changes.
  • Tests

    • Expanded test coverage for tooltip behavior and metadata-less columns.
    • Added end-to-end defect coverage for sticky-comment reports.

github-actions Bot and others added 3 commits June 11, 2026 16:54
…n-header tooltips, covered-by contrast, overrides-badge clip, baseline banner

Five founder-reported defects from the PR #239 review (issue #240):

A — long mono tokens (dbt_expectations.expect_table_row_count_to_be_between)
    painted 55px past the model tip's painted background: overflow-wrap
    on the .col-tooltip shell + wrapping .ct-val chips.
B — given column headers with no manifest metadata were silently
    hover-dead (third report): EVERY fixture header is a trigger now;
    metadata-less ones reveal a truthful fallback naming the owning node
    (never an empty bubble — the honesty invariant strengthens).
C — .finding-covered-by code rode Sakura's light #f1f1f1 code bg at
    ~1.2:1 on dark themes: themed var(--bg-alt)/var(--text) re-skin
    (5.9–14.8:1 measured across all 8 themes).
D — .ov-tooltip nowrap rows painted long override values past the bubble
    and the viewport edge: rows flex-wrap + overflow-wrap, singleton
    width capped at calc(100vw - 16px), positionTipNear clamps against
    documentElement.clientWidth (scrollbar-safe).
E — code.diff-scope-baseline same root cause as C, same fix.

Goldens regenerated (jaffle-shop, playground, diff-showcase) — every
hunk is the embedded template change; snapshot updated likewise.

Closes #240

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e guard-methodology fix)

Why #166/#236 validations passed while the founder's experience stayed
broken: they asserted tooltip reveal on columns KNOWN to carry manifest
metadata (existential), while the founder hovered undeclared columns —
hover-dead by the old design, outside every assertion universe. The new
guards quantify universally and pin geometry:

- fixture_header_tooltips_universal_after_view_toggle_and_sort: EVERY
  visible given/expected header must reveal a non-empty bubble on a real
  mouseover — initial render, after a real Diff→File toggle, after a
  real column sort, and after the round-trip (the #145/#146 wipe class).
- model_tip_long_test_name_wraps_inside_bubble: content containment
  (scrollWidth ≤ clientWidth + 1; no descendant past the bubble edge).
- ov_tip_long_value_contained_in_bubble_and_viewport_at_right_edge:
  long-override containment + viewport containment at a true right-edge
  trigger geometry.
- covered_by_test_ids_meet_aa_contrast_on_every_theme +
  baseline_banner_path_meets_aa_contrast_on_every_theme: the #206/#227/
  #231/#233 effective-backdrop AA family extended to defects C/E,
  all 8 themes (RED at 1.2:1 pre-fix).

Three #235-era pins are deliberately rewritten to the new contract
(metadata-less headers are fallback triggers, never dead): the no-EMPTY-
bubble honesty invariant is preserved and strengthened.

All five new guards captured RED against the unfixed templates (stash-
verified) and GREEN after the fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…row for #240 validation

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ghost

ghost commented Jun 11, 2026 •

Copy link
Copy Markdown

Ready to review this PR? Stage has broken it down into 6 individual chapters for you:

Title
1 Fix contrast for code and labels
2 Prevent tooltip overflow and clipping
3 Implement truthful fallback for empty tooltips
4 Wire owner metadata through fixture views
5 Update documentation and headless tests
6 Other changes
Open in Stage

Chapters generated by Stage for commit 0ffce4f on Jun 11, 2026 10:12pm UTC.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@cmbays, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 3 minutes. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6bfb7638-d7b0-4bfc-90e3-d6bd8b32b5de

📥 Commits

Reviewing files that changed from the base of the PR and between 3007bf5 and 0ffce4f.

⛔ Files ignored due to path filters (1)
  • tests/snapshots/render_integration__rendered_chrome_jaffle_shop.snap is excluded by !**/*.snap
📒 Files selected for processing (6)
  • examples/diff-showcase-report.html
  • examples/jaffle-shop-report.html
  • examples/playground-report.html
  • templates/interaction.js
  • templates/report.css
  • tests/headless_toggle.rs
📝 Walkthrough

Walkthrough

This PR addresses cute-dbt#240 by fixing column-header tooltip rendering for metadata-less fixture columns. Changes update CSS for viewport clamping and theme-aware styling, add owner-label derivation, thread ownership context through fixture rendering, ensure all column headers display non-empty fallback tooltips with owner names, and include comprehensive tests validating tooltip behavior and accessibility across theme variants.

Changes

Tooltip and Metadata-less Column Handling

Layer / File(s) Summary
CSS theme tokens and viewport-aware tooltip sizing
templates/report.css, examples/diff-showcase-report.html, examples/jaffle-shop-report.html, examples/playground-report.html
Theme-aware code styling in scope lines and tooltips using --bg-alt/--text tokens; viewport-relative width capping for tooltips (min(24rem, calc(100vw - 16px))) and override bubbles to prevent edge clipping; enabled mid-token wrapping with overflow-wrap:anywhere throughout.
Owner-label derivation from given inputs
examples/diff-showcase-report.html, examples/jaffle-shop-report.html, examples/playground-report.html, templates/interaction.js
New givenOwnerLabel(given, targetModel) helper parses given.input for ref(...)/source(...) patterns (supporting multiple quote styles) and derives human-readable owner labels for fallback tooltip naming.
Owner parameter threading through fixture rendering functions
examples/diff-showcase-report.html, examples/jaffle-shop-report.html, examples/playground-report.html, templates/interaction.js
Updated renderGivenSection, buildTable, buildFixtureView, and buildDiffTable to accept and propagate owner/target_model parameters; ensured given and expected fixture sections pass owner labels to enable metadata-less column fallbacks.
Column-header decoration with mandatory tooltip triggers and fallback messaging
examples/diff-showcase-report.html, examples/jaffle-shop-report.html, examples/playground-report.html, templates/interaction.js
Reworked decorateColHeader to always configure elements as tooltip triggers even when metadata is missing; added data-col-owner and aria-label attributes with fallback text, ensuring no metadata-less header is left without accessible fallback messaging.
Tooltip generation with owner-aware fallback content
examples/diff-showcase-report.html, examples/jaffle-shop-report.html, examples/playground-report.html, templates/interaction.js
Updated tooltip rendering to read data-col-owner from trigger elements and conditionally generate non-empty fallback bubbles ("No description or data tests declared …") when columns lack both description and tests, incorporating owner labels in the message.
Tooltip positioning with viewport-aware clamping
examples/diff-showcase-report.html, examples/jaffle-shop-report.html, examples/playground-report.html, templates/interaction.js
Adjusted positionTipNear clamping to use document.documentElement.clientWidth/clientHeight (with fallbacks) for viewport bounds; updated left/top edge flip and clamp calculations to keep bubbles fully visible near viewport edges.
Test assertion updates for column-header tooltip behavior
tests/headless_toggle.rs
Updated expected/given header tooltip expectations: all headers (including metadata-less ones) are now tooltip triggers with non-empty fallback content; assertions verify native title removal, fallback bubble owner context, and no hover-dead tooltips.
New regression and accessibility tests for tooltip geometry and contrast
tests/headless_toggle.rs
Added universal hover sweep over fixture headers after view toggles and column sorts, model-tip wrapping regression test, overrides-tip right-edge geometry test, AA contrast sweeps for code chips and scope banners across themes, and owner-label parsing validation for double-quoted ref/source inputs.
Dogfood project documentation for validation
dbt-project/README.md
Clarified that live dogfood reports include "docs-only" changes touching files under dbt-project/ to validate tooltip/contrast fixes on the rendered dbt-project row in PR preview reports.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

The PR modifies tooltip rendering and fixture view logic across three report examples plus shared templates, requiring verification of consistent behavior across all variants. While the underlying pattern repeats (CSS + JS owner-label plumbing + test updates), the changes are heterogeneous across viewport clamping math, HTML generation, ARIA attributes, and extensive test assertions covering UI geometry, AA contrast, and parsing edge cases.

Possibly related issues

  • breezy-bays-labs/cute-dbt#240: This PR directly addresses the defects described in issue #240 by fixing column-header tooltip rendering for metadata-less columns, implementing owner-label fallbacks, and preventing viewport-edge clipping.

Possibly related PRs

  • breezy-bays-labs/cute-dbt#234: Both PRs modify tooltip positioning/clamping logic (viewport edge handling in positionTipNear and bubble geometry), directly related on the tooltip rendering path.
  • breezy-bays-labs/cute-dbt#140: The main PR extends the fixture rendering pipeline seams introduced in #140 by threading owner/target_model arguments through buildFixtureView and window.__cuteBuildFixtureView.
  • breezy-bays-labs/cute-dbt#236: Both PRs refactor the same column-header tooltip anatomy and given-header tooltip behavior in templates/interaction.js, templates/report.css, and tests/headless_toggle.rs, extending ct-* chip structure and fixture tooltip rendering.

Poem

🐰 Metadata whispers fade,
Tooltips now with owner names displayed,
Viewport edges no longer betray,
Wrapping tokens find their way—
Fallback bubbles bright and brave! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: fixing five specific rendering defects (#240) with clear technical references to tooltip overflow, given-header tooltips, contrast, clipping, and baseline issues.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch adapters-240-review2-render-defects

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 11, 2026 •

Copy link
Copy Markdown
Contributor

📄 Rendered report preview

All golden examples regenerated cleanly.

🟡 Golden examples

Committed to examples/ and byte-identity gated — the canonical reports contributors and consumers browse. Stable across PRs.

Report View Download
diff-showcase-report.html ▶ Open ↗ ⬇ Download
jaffle-shop-report.html ▶ Open ↗ ⬇ Download
playground-report.html ▶ Open ↗ ⬇ Download

🐶 Live dogfood preview

This PR's own dbt-project/ diff, freshly compiled by fusion into an ephemeral manifest (never committed) and rendered with --pr-diff. Regenerated every PR — the live self-dogfood, not a committed example.

Report View Download
dbt-project-report.html ▶ Open ↗ ⬇ Download

▶ Open ↗ opens the report in your browser in one click —
published to this repo's GitHub Pages under /pr-244/.
⬇ Download fetches the same self-contained HTML as a workflow
artifact (auth-gated; works fully offline). Either way the report
makes zero external resource requests.

The Pages preview may take ~1 min to update after this comment
posts. On PRs from forks the Open link is unavailable (read-only
token) — use Download.

Alternative: GitHub CLI
# gh CLI >= 2.63 extracts into ./report-preview-playground/.
gh run download 27380675843 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.html

Posted by report-preview.yml for 0ffce4f1916152d66ad3e1cc92aac1a1c2271c05. Affordance only — never blocks merge.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses several tooltip rendering, containment, and contrast defects (cute-dbt#240). It ensures all column headers act as tooltip triggers by introducing a fallback message for metadata-less columns, wraps long unbreakable tokens inside tooltips, clamps tooltips safely within the viewport, and fixes contrast issues for code elements on dark themes. The review feedback correctly identifies that the regular expressions used to parse ref and source inputs only match single quotes, whereas double quotes are also valid in dbt, and provides a code suggestion to support both.

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.

Comment thread templates/interaction.js Outdated
…rce() inputs (PR #244 review)

dbt accepts both quote styles in a given's input:, and dbt-fusion ships
the authored string VERBATIM — a ref("stg_payments") given compiles
onto the manifest wire double-quoted, unnormalized (verified against a
real fusion 2.0.0-preview.177 compile of the dogfood project). The
owner-label regexes behind the metadata-less fallback bubble were
single-quote-only, degrading double-quoted inputs to the raw-string
fallback. Quote-class + backreference now accepts both styles (mixed
quotes stay unparsed -> truthful raw-input fallback).

New headless guard given_owner_label_accepts_double_quoted_ref_and_source
(RED without the regex fix, stash-verified). Goldens + chrome snapshot
regenerated; every hunk is the embedded regex/comment change.

The repo's Rust-side parsers (parse_ref_name / parse_source_ref) share
the single-quote-only gap pre-dating this PR; tracked separately.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/jaffle-shop-report.html`:
- Around line 8447-8465: Normalize and trim the column description once and use
that trimmed value everywhere: compute something like const desc =
(m.description || "").trim(); then use desc instead of m.description when
determining hasMeta, when pushing to parts, when setting "aria-label" and the
data attributes ("data-col-desc"), while leaving tests logic (tests.length and
colTestSummary) unchanged; update references in this block (variables m,
hasMeta, parts, colTestSummary, and the $th.attr calls) so whitespace-only
descriptions no longer count as metadata and tooltips/data attributes use the
trimmed description.

In `@templates/interaction.js`:
- Around line 3008-3009: The flip logic sets top = r.top - 6 - th but doesn't
clamp it, so very tall tooltips can still overflow the viewport; after the
vertical-flip assignment (the line that sets top when flipping above using
variables top, th, vh, r) clamp top to the visible range (e.g., ensure top is at
least a small margin like 8px and at most vh - th - 8) before applying
el.style.top, keeping the existing el.style.left assignment as-is.

In `@tests/headless_toggle.rs`:
- Around line 9427-9478: The theme-sweep script (COVERED_BY_CONTRAST_SWEEP_JS)
flips data-theme and reads computed styles immediately, which can sample
interpolated values due to transitions; before the measurement loop (after
obtaining root/documentElement and before iterating THEMES), inject a temporary
rule to disable CSS transitions (e.g., add a <style> with "transition: none
!important" or set a non-invasive inline override) and remove it after the loop,
ensuring you restore any changed classes/attributes; do the same change for the
other sweep snippet referenced (the one around lines 9577-9626) so both contrast
sweeps use the same transition-disable setup.
- Around line 9686-9786: The test
given_owner_label_accepts_double_quoted_ref_and_source currently constructs
UnitTestGiven inputs inline (e.g. UnitTestGiven::new("ref(\"bare_seed\")" ...)
and UnitTestGiven::new("source(\"raw\", \"patients\")"...)) which makes the
assertion depend on synthetic strings; instead load a committed dbt-fusion
compiled fixture (manifest/compiled output) and use
render_with_sources_to_file/UnitTest/UnitTestGiven only as wrappers that
reference that fixture, and pin the fixture's git commit SHA in the test context
(add a constant or test helper with the SHA and a comment), replacing the inline
UnitTestGiven strings with references to the fixture data so the test asserts
against real fusion wire-shape output rather than synthetic JSON.
🪄 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: 53260e3f-a2fd-41e8-96a8-c835175c5f9c

📥 Commits

Reviewing files that changed from the base of the PR and between ebd8d47 and 3007bf5.

⛔ Files ignored due to path filters (1)
  • tests/snapshots/render_integration__rendered_chrome_jaffle_shop.snap is excluded by !**/*.snap
📒 Files selected for processing (7)
  • dbt-project/README.md
  • examples/diff-showcase-report.html
  • examples/jaffle-shop-report.html
  • examples/playground-report.html
  • templates/interaction.js
  • templates/report.css
  • tests/headless_toggle.rs

Comment thread examples/jaffle-shop-report.html
Comment thread templates/interaction.js
Comment thread tests/headless_toggle.rs
Comment thread tests/headless_toggle.rs
github-actions Bot and others added 2 commits June 11, 2026 17:59
…eep transition hazard (PR #244 verification residual)

The covered-by CODE pills pass AA on all 8 themes, but the 'Covered by '
prefix label (.finding-covered-by .f-label, 11.88px weight-400 so the
4.5 floor applies) rode the verbatim latte --text-muted: #6c6f85 on the
finding row's #eff1f5 fill = 4.37:1. The established #227/#231 deepened
stand-in extends to this label via the narrowest token override
(:root[data-theme=latte] .finding-covered-by .f-label
{ --text-muted: #5c5f77 }) — re-measured empirically on the actual
finding-row backdrop: 5.53:1. rosepine (4.56) stays verbatim; the latte
theme block stays pinned.

Guard changes (the existential-vs-universal lesson applied to AA sweeps):
- the covered-by sweep now measures EVERY text run in the quoted line
  (code pills AND the f-label prefix), tags entries by element kind, and
  asserts the label run is present — stash-verified RED at latte 4.37
  without the override, GREEN at 5.53 with it;
- both #240 sweeps (covered-by + baseline path) inject
  '* { transition: none !important }' for the sweep's duration: body
  transitions background/color over 120ms, so an instant setAttribute +
  getComputedStyle read can land mid-transition (verifier-observed false
  readings).

Goldens + chrome snapshot regenerated; every hunk is the embedded CSS
override/comment.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-flip top clamp (PR #244 CodeRabbit threads)

Thread 1 (applied): a whitespace-only authored description ('   ')
passed the hasMeta gate untrimmed, suppressing the cute-dbt#240
truthful fallback and opening an effectively-empty bubble on a
description-only column — the exact state the never-empty-bubble
contract forbids. decorateColHeader (the single writer of
data-col-desc) now trims once; aria-label, the fallback arm, and the
bubble all read the trimmed value. New headless guard
whitespace_only_description_degrades_to_truthful_fallback
(stash-verified RED untrimmed, GREEN trimmed).

Thread 2 (applied): positionTipNear gains the post-flip top clamp
(top < 8 -> 8) — at pathological viewport heights the flipped position
(trigger top - bubble height) goes negative, pushing the bubble above
the screen. Mirrors the horizontal clamp; resolves the clamp half of
cute-dbt#246 (the fmt-tooltip overflow half stays there). The existing
right-edge containment guard now also pins top >= 0.

Goldens + chrome snapshot regenerated; every hunk is the embedded
trim/clamp change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cmbays

cmbays commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

CodeRabbit non-thread comments — disposition ledger

Audited every CodeRabbit surface on this PR beyond the 5 review threads (all 5 resolved with disposition replies):

Review body (21:24 review, the only one with content): its collapsed sections are (1) the "AI agents" prompt — a verbatim restatement of the SAME 4 findings that became threads, (2) autofix checkboxes, (3) run-configuration / commits / path-filter metadata. No nitpicks, no additional-comments, no outside-diff findings. The 4 restated findings map to:

Walkthrough issue comment (40KB): pure summary — changes table, effort estimate, related issues/PRs, poem, pre-merge checks (5/5 passed), finishing-touches/rate-limit boilerplate. Zero embedded findings.

The other 3 review entries: empty bodies (thread-reply containers).

Net: no actionable suggestion exists outside the 5 dispositioned threads.

@cmbays
cmbays merged commit 291b639 into main Jun 11, 2026
33 checks passed
@cmbays
cmbays deleted the adapters-240-review2-render-defects branch June 11, 2026 22:27
github-actions Bot added a commit that referenced this pull request Jun 11, 2026
cmbays added a commit that referenced this pull request Jun 12, 2026
…elision (#286)

* fix(render): #246 — bound fmt-tooltip content + viewport-relative height clamp

The #fmt-tooltip is pointer-events:none and hides on mouseleave/blur, so
content laid below its fixed 22rem max-height fold (overflow:auto) could
never be scrolled to or read — silently unreachable for every input
modality (observed on the playground dim_payers panel, ~221px below the
fold).

The fix bounds the content instead of folding it:

- interaction.js: showFmtTip trims the reconstruction to a
  viewport-derived line budget (20px/line + 96px chrome
  over-approximations, 20-line ceiling, 1-line floor) and closes with a
  truthful elision row counting exactly what was trimmed — the tip is a
  glance surface; the grid below the badge carries the full data.
- report.css: the 22rem cap becomes viewport-relative
  (calc(100vh - 16px), the #240 defect-D width-cap pattern) so
  positionTipNear's post-flip top clamp (landed in PR #244) always fully
  contains the box on the height axis at any viewport height;
  overflow:hidden because a scrollbar nothing can operate is a lie.
- headless guards (RED pre-fix): no-fold + truthful-elision +
  viewport-containment pins at the default viewport, the full
  pathological-viewport-height containment pin promised by the PR #244
  guard comment (the platform floors tiny window requests, so the pins
  measure against the live clientHeight), and the existing ov-tip
  right-edge containment guard extended to the height axis.

Goldens regenerated per the ci.yml example-report-check recipes (the
three report examples inline report.css + interaction.js; the explore
pages don't carry the fmt tip and are unchanged).

Closes #246

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(render): #246 — cross-platform premise for the pathological-viewport guard

The Linux CI runner HONORS tiny window requests (the ~111px request
yielded a 24px viewport) while macOS floors the outer window at ~375px
(232px viewport) — so the macOS-derived 50..=352 premise band excluded
the Linux reality and the premise assert panicked in CI.

Request a pathological-but-sane ~300px window instead, landing both
platforms in a comparable sub-352px band (macOS ≈232 via its floor;
Linux ≈213-300 depending on chrome height), and widen the premise to
100..=352. Re-verified RED against the pre-fix templates at the new
band (the no-fold pin fires; the old fixed 222px box folds 1178px of
content). Test-only change — no golden impact.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(render): #246 — font-scale-safe tip budget

PR #286 review (Gemini, HIGH): the 20px/line + 96px chrome constants in
fmtTipLineBudget were layout estimates — correct for today's px-sized
.sql-block lines, but wrong under a forced minimum font size (Chrome
a11y), browser-zoom rounding, wrapped long lines (.code-line is
white-space:normal), or any future rem migration — and overflow:hidden
turned any mismatch into silent data loss.

Both recommended directions, composed:

- (a) measurement-driven trim: showFmtTip renders the ceiling-capped
  block (20 lines), then removes rendered lines until the LIVE laid-out
  content fits the viewport-capped box (scrollHeight <= clientHeight+1,
  the repo's 1px convention), measuring after a left reset (the #232
  D18 lesson). The elision row participates in the measurement and its
  count stays truthful however the trim happened.
- (b) report.css restores overflow:auto as the safety net: an exotic
  residual mismatch degrades to a visible scrollbar — a signal — never
  a silent clip.

Headless: fmt_tip_long_fixture_is_bounded_not_folded gains a font-scale
phase (inject .sql-block{font-size:24px}, re-show, re-assert no-fold +
truthful elision + strictly fewer lines shown than at normal size — the
non-vacuity proof). Both #246 tests pass; full toggle suite 81/81.

Goldens regenerated per the ci.yml recipes; intended-only audit clean
(all changed lines are this rewrite ×3 report goldens; explore pages
unchanged). Chrome snapshot refreshed and diff-audited the same way.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant