[onton-tui-ux-polish] Patch P6: Rich styling and consistent information architecture#100
Merged
subsetpark merged 7 commits intomainfrom Mar 18, 2026
Merged
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
subsetpark
force-pushed
the
onton-tui-ux-polish/patch-P6
branch
from
458c6eb to
2f74700
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/tui.ml`:
- Around line 487-492: The header width calculation in render_header currently
uses String.length for project_name which miscomputes visual width for UTF-8;
update the suffix_pad computation to use Term.visible_length project_name (i.e.
replace String.length project_name with Term.visible_length project_name) so the
visible-length-aware subtraction (width - Term.visible_length prefix -
Term.visible_length project_name - 1) is used when computing suffix_pad.
- Around line 507-531: The selection reverse-video is being lost because
render_patch_row applies Term.styled [Term.Sgr.reverse] to a row that already
contains many Term.styled fragments (styled_status, Term.styled for
pr_tag/branch/op_suffix), and those inner styled fragments emit ANSI resets that
cancel reverse; fix render_patch_row (and other similar renderers around the
same area) by avoiding nested Term.styled calls: build the full unstyled string
(or build fragments without emitting resets) and apply the selection style once
at the outermost level, or change helper styled_status/Term.styled usage to
return plain text + attributes so you can compose attributes and call
Term.styled [Term.Sgr.reverse; ...other attrs...] only once on the final row
(ensure functions like styled_status, short_op_name, and the pr_tag/branch
formatting are refactored to not emit independent resets).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: efdccc8c-86a1-4e1a-b631-70ebdf211dcc
📒 Files selected for processing (3)
lib/term.mllib/tui.mllib/tui.mli
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/tui.ml (1)
325-331: 🧹 Nitpick | 🔵 TrivialUse palette constants in status-message rendering for full theme consistency.
This block still uses ad-hoc SGR values (
fg_yellow,fg_red,dim) while the file now has standardized palette constants.♻️ Proposed refactor
- | Info -> Term.styled [ Term.Sgr.dim ] msg.text + | Info -> Term.styled [ c_muted ] msg.text | Warning -> - Term.styled [ Term.Sgr.fg_yellow ] (Printf.sprintf "⚠ %s" msg.text) + Term.styled [ c_warn ] (Printf.sprintf "⚠ %s" msg.text) | Error -> Term.styled - [ Term.Sgr.fg_red; Term.Sgr.bold ] + [ c_alert; Term.Sgr.bold ] (Printf.sprintf "✗ %s" msg.text)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tui.ml` around lines 325 - 331, The Info/Warning/Error arms in the status-message rendering use raw Term.Sgr values (Term.Sgr.dim, Term.Sgr.fg_yellow, Term.Sgr.fg_red) instead of the standardized palette constants; update those three branches to use the project's palette/theme constants (the palette SGR lists defined earlier) when calling Term.styled so the styling is consistent—replace Term.Sgr.dim with the palette's dim constant and Term.Sgr.fg_yellow/Term.Sgr.fg_red with the palette warning/error SGR values in the Term.styled calls that render msg.text (the pattern-match block producing Term.styled for Info, Warning, Error).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/tui.ml`:
- Around line 711-723: The banner width logic currently forces inner_w to be the
larger of banner_text length and available space (width - 6), causing overflow
on narrow terminals; change the calculation in the inner_w binding to cap the
banner to the available space by using Int.min instead of Int.max and guard
against negative widths (e.g. wrap Int.min(...) with Int.max 0 ...), so inner_w
uses banner_text, width and Term.repeat consistently (refer to symbols
banner_text, inner_w, width, Term.repeat).
---
Outside diff comments:
In `@lib/tui.ml`:
- Around line 325-331: The Info/Warning/Error arms in the status-message
rendering use raw Term.Sgr values (Term.Sgr.dim, Term.Sgr.fg_yellow,
Term.Sgr.fg_red) instead of the standardized palette constants; update those
three branches to use the project's palette/theme constants (the palette SGR
lists defined earlier) when calling Term.styled so the styling is
consistent—replace Term.Sgr.dim with the palette's dim constant and
Term.Sgr.fg_yellow/Term.Sgr.fg_red with the palette warning/error SGR values in
the Term.styled calls that render msg.text (the pattern-match block producing
Term.styled for Info, Warning, Error).
0e6be77 to
dcf152f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/tui.ml (1)
389-395: 🧹 Nitpick | 🔵 TrivialUse palette constants in
render_status_msgfor full theme centralization.This block still hardcodes ANSI colors while the module now defines
c_muted,c_warn, andc_alert.♻️ Proposed refactor
let raw = match msg.level with - | Info -> Term.styled [ Term.Sgr.dim ] safe_text + | Info -> Term.styled [ c_muted ] safe_text | Warning -> - Term.styled [ Term.Sgr.fg_yellow ] (Printf.sprintf "⚠ %s" safe_text) + Term.styled [ c_warn ] (Printf.sprintf "⚠ %s" safe_text) | Error -> Term.styled - [ Term.Sgr.fg_red; Term.Sgr.bold ] + [ c_alert; Term.Sgr.bold ] (Printf.sprintf "✗ %s" safe_text)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tui.ml` around lines 389 - 395, The status rendering still uses hardcoded SGR values; update render_status_msg to use the module palette constants instead: replace Term.Sgr.dim with c_muted for the Info branch, replace Term.Sgr.fg_yellow with c_warn for Warning, and replace Term.Sgr.fg_red (and keep Term.Sgr.bold) with c_alert (combined with Term.Sgr.bold) for Error so all three branches use c_muted, c_warn, and c_alert respectively.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/tui.ml`:
- Around line 557-568: The header can overflow when project_name is too long;
update render_header to ensure the composed title_line never exceeds width by
measuring visible lengths (using Term.visible_length) and truncating or eliding
project_name (or shortening rule_suffix) as needed before creating rule_suffix
and title_line; specifically, compute available_space = width -
Term.visible_length prefix - 1, clamp project_name to at most available_space
(or replace tail with "…" when truncated), then build rule_suffix from remaining
space so the final Term.styled title_line fits within width.
---
Outside diff comments:
In `@lib/tui.ml`:
- Around line 389-395: The status rendering still uses hardcoded SGR values;
update render_status_msg to use the module palette constants instead: replace
Term.Sgr.dim with c_muted for the Info branch, replace Term.Sgr.fg_yellow with
c_warn for Warning, and replace Term.Sgr.fg_red (and keep Term.Sgr.bold) with
c_alert (combined with Term.Sgr.bold) for Error so all three branches use
c_muted, c_warn, and c_alert respectively.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f56a5ff4-1bbe-44ae-84b5-f86d678a77d0
📒 Files selected for processing (3)
lib/term.mllib/tui.mllib/tui.mli
Color palette constants (c_accent, c_ok, c_warn, c_alert, c_running, c_muted, c_pending) defined at top of tui.ml, used consistently across all views replacing ad-hoc ANSI codes. List view: project header with c_accent rule, per-token colored summary line, patch rows with indicator/PR#/branch/status, reverse-video selection. Detail view: two-column info grid (16-char label column), section rules for Info/CI Checks/Transcript, bold red box banner for needs-intervention patches, pending comment count, styled CI failure count. Timeline view: placeholder timestamps, muted from-labels, styled transitions with → arrows, reverse-video selection. Footer: bracketed key hints per view mode. Status message bar rendered between content and footer with auto-clear TTL support. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix visible_length to count UTF-8 codepoints instead of bytes so multi-byte characters (e.g. box-drawing ─) measure as 1 column - Use Term.visible_length in section_rule and render_header so horizontal rules fill the correct width - Remove duplicate pending-comments section (already shown in info grid) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove no-op `Term.strip_ansi badge |> fun _ -> badge` in status grid - Use Term.visible_length for project_name to handle non-ASCII correctly Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add apply_reverse_selection that replaces inner SGR resets with reset+reverse so the highlight persists across embedded styled fragments in patch rows and timeline entries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cap inner_w to available space (width - 6) instead of expanding to banner text length, and use Term.fit_width to pad/truncate the text so narrow terminals don't overflow. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use Term.fit_width to truncate the composed title line so it never exceeds the terminal width on narrow viewports. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
subsetpark
force-pushed
the
onton-tui-ux-polish/patch-P6
branch
from
8080d96 to
a5af0aa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
c_accent,c_ok,c_warn,c_alert,c_running,c_muted,c_pending) at top oftui.mland use them consistently across all views, replacing ad-hoc ANSI color codesc_accentrule, per-token colored summary line (merged/running/awaiting/needs-help), patch rows with[indicator] PR# branch status (op), reverse-video selection── Info ──,── CI Checks ──,── Transcript ──), bold red box banner for needs-intervention patches, styled CI failure counts and conflict indicatorsc_muted, transitions with→arrows, reverse-video selection highlightTest plan
dune buildcleandune runtestall tests pass🤖 Generated with Claude Code