Repository navigation
#199 — design-2 fold steppers: gutter +/− steppers, expand-step setting, split-view folds, per-diff fold toggle, copy-icon buttons - #213
Conversation
|
Ready to review this PR? Stage has broken it down into 7 individual chapters for you: 
Chapters generated by Stage for commit 0865893 on Jun 11, 2026 2:59am UTC. |
|
Warning Review limit reached
More reviews will be available in 40 minutes and 7 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ 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 (2)
📒 Files selected for processing (7)
✨ 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 ▶ Open ↗ opens the report in your browser in one click — 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 27320800683 -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 a new step-expansion model for hunk folds (cute-dbt#199), replacing the old top-of-report controls strip with per-diff fold toggles and gutter steppers (+/−) for both unified and split layouts. It also replaces the text-based Copy button with an inline-SVG copy-icon button in the code headers. The review feedback highlights three key areas for improvement: correcting the copy-to-clipboard button to properly handle and display copy failures using the existing .copy-failed CSS class, refactoring the per-diff fold toggle to dynamically check the fold state instead of relying on a local state variable that can get out of sync, and updating setAllFolds to keep these toggle buttons synchronized with the actual fold states.
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.
…findings) Applies all three Gemini findings — each holds the new controls to the same symmetric-DOM-mirror standard the #136 guards assert: 1. copyIconBtn signals failure truthfully: success flashes 'copied', failure flashes 'copy-failed' (title AND aria-label carry the outcome; both reset after the flash). Write strategy now mirrors copySql — writeText, then the shared execCommand fallbackCopy (which reports its own success) on rejection/absence. The .copy-failed CSS is no longer dead. 2. buildFoldToggleBtn is stateless: the click derives intent from the DOM at activation time (any hidden folded line in this diff => expand-all, else collapse-all) instead of a cached boolean that desyncs under per-hunk stepping or the __cute hooks. 3. setAllFolds keeps the per-diff toggles truthful: every fold mutation funnels through updateFoldControl -> syncFoldToggles, which relabels each toggle (text + aria-pressed) from ITS OWN diff's DOM truth via the root accessor stored on the element. Per-root truth is deliberately stronger than relabeling 'toggles in scope' to one shared state: a partial-scope op never lies about an untouched sibling diff, and per-hunk steppers stay covered too. New/extended guards (headless_toggle): - per_diff_fold_toggle_drives_its_own_diffs_folds: (a) global __cuteExpandAllFolds flips the toggle's label/aria-pressed; a click after the global op acts on DOM truth and collapses; (b) stepping the unified fold fully keeps the toggle truthful while the split twin still holds hidden rows, and the next click expands the remainder. - NEW copy_icon_button_signals_failure_truthfully: both write paths stubbed to fail in-page (writeText rejects, execCommand false) => copy-failed + 'Copy failed' title/aria-label, never 'copied', reset to rest state after the flash. insta render_integration snapshot regenerated (inlined interaction.js). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
…ep setting, split-view folds, per-diff fold toggle, copy-icon buttons Ports the pass-2 directional fold model onto the shipped diff renderers (cute-dbt#199, design package design-return-2): - settings.expandStep in cute-dbt.settings.v1 (default 20, clamp 0–500, 0 = all; NaN-tolerant hydrate) + the static #settings-expand-step row (the #188 static-markup contract). Read live by expandFold/contractFold — no re-render on change. - Unified renderer: data-fold-dir (leading fold expands UP toward the hunk, else DOWN), gutter .fold-steppers (+ by step / − by step, disabled at bounds), per-hunk .fold-collapse-all once anything is revealed, label progression Show N unchanged lines → All N lines shown, and updateFoldControl re-parks the control adjacent to the remaining hidden run (below the run when fully revealed). Reveal stays parent-scoped via closest(code, tbody) — the #132 duplicate-fold-id rule. - Split renderer folds long context runs with the SAME fold model (fold row: stepper gutter + colspan label cell) and gains the ds-c-num/ds-c-code colgroup (3.8em/auto) for a true 50/50; setAllFolds drives both layouts from one control set, so __cuteExpandAllFolds / __cuteCollapseAllFolds keep mirroring every fold control (label + aria-expanded + steppers + collapse-all — the #136 symmetric-mirror invariant on the new anatomy). - Per-diff fold toggle (buildFoldToggleBtn, aria-pressed) in the Model-SQL and Model-YAML diff code headers; the #132 top-of-report .diff-expand-all strip is removed (diffAllExpanded / renderExpandAllToggle / bindDiffViewControls retired with it). - copyIconBtn (inline-SVG icon, aria-label Copy, copied-class flash, execCommand fallback) replaces the absolutely-positioned text Copy in the Model-SQL header and is added to the Model-YAML header; code-header padding per engine/base.css. Functional-SVG-only iconography (README §2.4) — no icon fonts, nothing external, zero-egress untouched. No payload change; no src/domain change. Goldens + insta snapshots regenerated — every changed byte traces to this delta. Closes #199 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n contract Deliberate guard migration (never deleted, each replaced with the new-behavior assertion): - block_diff_folds_long_context_runs_and_reveals_on_activate: activate = expand-by-step (default step 20 >= hidden 4 still reveals all in one activation); the 'Hide N unchanged lines' relabel assertion is superseded by 'All N lines shown' + disabled + stepper + visible collapse-all; re-collapse moved from band-click to the explicit .fold-collapse-all. Kept: parent-scoped reveal, control stays visible, aria-expanded truth, Enter/Space keyboard activation, short-block never folds, new data-fold-dir/steppers markup assertions. - global_expand_collapse_mirrors_every_fold: the #136 bidirectional mirror retained on the new control anatomy (label, stepper disabled-states, collapse-all visibility); now also pins the retired top-of-report .diff-view-controls strip at 0 nodes. - settings_context_lines_refolds_block_diffs_live: contextLines re-render still live; extended with the expandStep no-re-render proof (a block mounted before the setting change steps by the new value). - split_diff_renders_the_same_block_diff_as_unified: parity extends to folds — same directional control anatomy, count, colgroup geometry, band activation + setAllFolds mirror on the split tbody. - model_sql_section_defaults_to_diff_and_toggles_to_raw / yaml_diff_drawer_defaults_to_diff_and_toggles_to_authored: assert the per-diff fold toggle + the inline-SVG copy-icon button (real focusable <button>, aria-label Copy) in both code headers. - NEW expand_step_steppers_reveal_contract_directionally_and_persist: + reveals exactly step lines toward the hunk, − re-hides them mirroring direction, collapse-all restores, up/down direction proofs, control re-parking, and cute-dbt.settings.v1 persistence across reload. - NEW per_diff_fold_toggle_drives_its_own_diffs_folds: the header toggle expands/restores every fold in its own diff (unified + split) with aria-pressed + label tracking. render_block_diff_honors_a_configurable_fold_pad is unchanged — its fold-pad arithmetic and labels survive the new model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…findings) Applies all three Gemini findings — each holds the new controls to the same symmetric-DOM-mirror standard the #136 guards assert: 1. copyIconBtn signals failure truthfully: success flashes 'copied', failure flashes 'copy-failed' (title AND aria-label carry the outcome; both reset after the flash). Write strategy now mirrors copySql — writeText, then the shared execCommand fallbackCopy (which reports its own success) on rejection/absence. The .copy-failed CSS is no longer dead. 2. buildFoldToggleBtn is stateless: the click derives intent from the DOM at activation time (any hidden folded line in this diff => expand-all, else collapse-all) instead of a cached boolean that desyncs under per-hunk stepping or the __cute hooks. 3. setAllFolds keeps the per-diff toggles truthful: every fold mutation funnels through updateFoldControl -> syncFoldToggles, which relabels each toggle (text + aria-pressed) from ITS OWN diff's DOM truth via the root accessor stored on the element. Per-root truth is deliberately stronger than relabeling 'toggles in scope' to one shared state: a partial-scope op never lies about an untouched sibling diff, and per-hunk steppers stay covered too. New/extended guards (headless_toggle): - per_diff_fold_toggle_drives_its_own_diffs_folds: (a) global __cuteExpandAllFolds flips the toggle's label/aria-pressed; a click after the global op acts on DOM truth and collapses; (b) stepping the unified fold fully keeps the toggle truthful while the split twin still holds hidden rows, and the next click expands the remainder. - NEW copy_icon_button_signals_failure_truthfully: both write paths stubbed to fail in-page (writeText rejects, execCommand false) => copy-failed + 'Copy failed' title/aria-label, never 'copied', reset to rest state after the flash. insta render_integration snapshot regenerated (inlined interaction.js). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mini fix delta) All three examples re-rendered per the example-report-check recipe after merging origin/main (#204/#205/#211/#212). Byte audit: every changed line traces to the truthful fold-toggle/copy-outcome fix (a4c95d7) — main's cross-join/source-binding golden content came through the merge intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6dc5fee to
0865893
Compare
…howcase) + golden regen Conflict resolution by REGENERATION, never manual picking: all three examples re-rendered from the merged tree with the ci.yml recipe. Non-payload bytes are byte-identical to origin/main's goldens (the #213 template/CSS delta carries through untouched); the payload delta vs origin/main is exclusively the #200 contract fields — manifest_nodes, ModelPayload.description, the native-scalar overrides, and the three \u003c= escapes inside new description content. The #214 external-fixture given rides through unchanged (no existing payload key altered). Double-render byte-identity verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- tests/steps/world.rs: kept BOTH accumulators — main's ContextPlan (#216) and the explore fields/structs (#100). - tests/steps/report_generation.rs: the NEW #216 context-report invocation (landed on main pre-verb) gains the report verb. - examples: report goldens taken from main and verified byte-identical over the merged tree under the report verb; explore goldens REGENERATED over the merged tree (stale twice: #214 changed the playground manifest, #216 added payload fields tests.html embeds) + double-render byte-identity + leak-grep clean. - ci.yml/lefthook.yml auto-merged: verb-aware gate + tripwire + feature-count 16 intact (no upstream bump — no renumber). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
Design-2 integration PR-2 (epic #197): ports the pass-2 directional fold model onto the shipped diff renderers. No payload change; no
src/domainchange —templates/*+ headless guards + goldens only.Closes #199
What landed
settings.expandStepincute-dbt.settings.v1(default 20, clamp 0–500, 0 = all; NaN-tolerant hydrate) + the static#settings-expand-steprow inreport.html(the #178 — design integration PR-2: engine merge (settings panel, unified/split diffs) + port-forward #188 static-markup contract — markup copied from the pass-2 prototype's settings row). Read live byexpandFold/contractFold— changing it needs no re-render.data-fold-dir(leading fold with the hunk below expands UP, else DOWN), gutter.fold-steppers(+expands by step,−contracts by step, disabled at bounds), far-right.fold-collapse-allonce anything is revealed, label progressionShow N unchanged lines → All N lines shown, andupdateFoldControlre-parks the control adjacent to the remaining hidden run / below the run when fully revealed. Enter/Space on the band still activates (expand-by-step); reveal stays parent-scoped viaclosest("code, tbody")(the polish: bold red/green diff text (drop github-style fill) + type-aware NULL in cell diffs #132 duplicate-fold-id rule).ds-c-num3.8em /ds-c-codeauto<colgroup>for a true 50/50;setAllFoldsdrives both layouts from one control set.buildFoldToggleBtn,aria-pressed) in the Model-SQL and Model-YAML diff code headers; the top-of-report.diff-expand-allstrip is removed (design intent), along with its now-deaddiffAllExpanded/renderExpandAllToggle/bindDiffViewControlsJS and CSS.window.__cuteExpandAllFolds/__cuteCollapseAllFoldsremain and mirror every fold control (label +aria-expanded+ steppers + collapse-all state) — the headless hooks stay the test seam.copyIconBtn(inline-SVG icon,aria-label="Copy", copied-class flash,execCommandfallback) replaces the absolutely-positioned text Copy in the Model-SQL header and is added to the Model-YAML header;code-headerpadding perengine/base.css(the 4.8rem right-reserve for the old absolute button is gone). Functional-SVG-only iconography (README §2.4) — no icon fonts, nothing external.<button>s with aria labels (the #145 — surface incremental-model unit-test semantics #146 rule — no hover-only affordances).Guard supersessions (#132/#136 → #199) — updated deliberately, never deleted
block_diff_folds_long_context_runs_and_reveals_on_activate+disabled + collapse-all visible; re-collapse via.fold-collapse-all. Kept: parent-scoped reveal, control stays visible, aria-expanded truth, Enter/Space activation, short-block-never-folds. Added:data-fold-dir/ steppers / collapse-all markupglobal_expand_collapse_mirrors_every_foldfoldLabel.diff-view-controlsstrip at 0 nodes (was: asserted 1)settings_context_lines_refolds_block_diffs_livesplit_diff_renders_the_same_block_diff_as_unifiedsetAllFoldsmirror on the split<tbody>model_sql_section_defaults_to_diff_and_toggles_to_raw/yaml_diff_drawer_defaults_to_diff_and_toggles_to_authoredaria-pressed) and inline-SVG copy-icon button (real<button>,aria-label="Copy") in both headersrender_block_diff_honors_a_configurable_fold_padNew guards:
expand_step_steppers_reveal_contract_directionally_and_persist—+reveals exactly step lines toward the hunk,−re-hides them mirroring direction, collapse-all restores, up/down direction proofs (c3,c4from the top on a down fold;c5,c6from the bottom on an up fold), control re-parking, 0–500 input range, andcute-dbt.settings.v1persistence across reload.per_diff_fold_toggle_drives_its_own_diffs_folds— the header toggle expands/restores every fold in its own diff (unified and split) witharia-pressed+ label tracking.Goldens / snapshots
All three goldens regenerated per the
example-report-checkrecipe (jaffle-shop,playground,diff-showcase);git diff --text -U0 -- examples/audited — every changed byte traces to the fold/stepper/copy-button/strip/settings delta (inlinedinteraction.js/report.css/report.htmlblocks ×3). Two insta snapshots regenerated (golden_report__rendered_report_skeleton— static Copy button removal + settings row;render_integration__rendered_chrome_jaffle_shop— the same template delta), diff-reviewed the same way. The diff-showcase's existing long-context SQL diff already folds, so the steppers + per-diff toggle + copy icons manifest in the committed showcase with no fixture change.Gates (run directly in the worktree — not via lefthook)
cargo fmt --checkcargo clippy --all-targets --locked -- -D warningscargo nextest runcargo test --test bddcargo test --test headless_zero_egress --locked -- --ignoredcargo test --test headless_toggle --locked -- --ignoredRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --lockedcargo deny checkcargo llvm-cov nextest+crap4rs --config crap4rs.toml --coverage lcov.infoOut of scope (per the collision contract)
Expect-tooltip, panel-toggle/show_all_inputs, findings-panel surfaces (#201/#202); data contracts (#200). The
## Discoveryquestion on the issue (aria-live for the toggle label) is left open — the toggle already exposes state viaaria-pressed.🤖 Generated with Claude Code