Skip to content

Commit bb2a905

Browse files
montfortclaude
andauthored
fix(cli): prevent UTF-8 panic in explore table wrapping (#52)
`devtrail explore` panicked with `byte index X is not a char boundary` when rendering Markdown tables whose cells contained multi-byte UTF-8 characters such as em-dash (U+2014, 3 bytes), CJK ideograms, accented characters or emoji. The cause was `wrap_cell_text()` slicing `&str` by a byte offset derived from terminal columns. Fix: - Rewrite `wrap_cell_text()` to iterate with `char_indices()` so every slice boundary is a valid char boundary. - Measure text width with `unicode-width` (added as a direct dep under the existing `tui` feature; was already a transitive dep of `ratatui`, so no new compile cost) so wrapping and natural column widths match the visual columns ratatui reserves at render time. - Manually pad cells by visual width in `render_table_row()` instead of `format!("{:<width$}", ...)`, which counts chars, not columns — this also aligns borders for CJK / double-wide content (relevant for the zh-CN docs the project ships). - Add 16 unit tests, including a regression for the exact panic (`em_dash_no_panic`) and an end-to-end pipeline test using the literal row that crashed the reported document. Bump CLI to 3.2.3. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ee176a6 commit bb2a905

10 files changed

Lines changed: 263 additions & 40 deletions

File tree

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@ and this project uses [independent versioning](README.md#versioning) for Framewo
77

88
---
99

10+
## CLI 3.2.3 — UTF-8 Crash Fix in `explore` Tables
11+
12+
### Fixed (CLI)
13+
- Fix panic in `devtrail explore` when rendering Markdown tables whose cells contain multi-byte UTF-8 characters (em-dash ``, CJK ideograms, accented characters, emoji). Cell wrapping now uses `char_indices()` for safe slicing and measures text in visual columns via `unicode-width`, so table borders also stay aligned with Chinese and double-wide content.
14+
15+
---
16+
1017
## CLI 3.2.2 — crates.io README Broken Links Fix
1118

1219
### Fixed (CLI)

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ DevTrail uses independent version tags for each component:
150150
| Component | Tag prefix | Example | Includes |
151151
|-----------|-----------|---------|----------|
152152
| Framework | `fw-` | `fw-4.2.0` | Templates (12 types), governance, directives |
153-
| CLI | `cli-` | `cli-3.2.2` | The `devtrail` binary |
153+
| CLI | `cli-` | `cli-3.2.3` | The `devtrail` binary |
154154

155155
Check installed versions with `devtrail status` or `devtrail about`.
156156

cli/Cargo.lock

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cli/Cargo.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "devtrail-cli"
3-
version = "3.2.2"
3+
version = "3.2.3"
44
edition = "2021"
55
description = "CLI tool for DevTrail - Documentation Governance for AI-Assisted Development"
66
license = "MIT"
@@ -36,11 +36,12 @@ tar = "0.4"
3636
ratatui = { version = "0.29", optional = true, default-features = false, features = ["crossterm"] }
3737
crossterm = { version = "0.28", optional = true }
3838
pulldown-cmark = { version = "0.12", optional = true }
39+
unicode-width = { version = "0.2", optional = true }
3940
arborist-metrics = { version = "0.1", optional = true, features = ["all"] }
4041

4142
[features]
4243
default = ["tui", "analyze"]
43-
tui = ["ratatui", "crossterm", "pulldown-cmark"]
44+
tui = ["ratatui", "crossterm", "pulldown-cmark", "unicode-width"]
4445
analyze = ["arborist-metrics"]
4546

4647
[dev-dependencies]

cli/src/tui/markdown.rs

Lines changed: 230 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use pulldown_cmark::{Event, HeadingLevel, Options, Parser, Tag, TagEnd};
22
use ratatui::style::{Color, Modifier, Style};
33
use ratatui::text::{Line, Span};
4+
use unicode_width::{UnicodeWidthChar, UnicodeWidthStr};
45

56
use crate::tui::theme;
67

@@ -339,15 +340,15 @@ fn compute_column_widths(
339340
return Vec::new();
340341
}
341342

342-
// Calculate natural (max content) width per column
343+
// Calculate natural (max content) width per column, measured in visual columns
343344
let mut natural = vec![0usize; num_cols];
344345
for (i, cell) in header.iter().enumerate() {
345-
natural[i] = natural[i].max(cell.len());
346+
natural[i] = natural[i].max(UnicodeWidthStr::width(cell.as_str()));
346347
}
347348
for row in body {
348349
for (i, cell) in row.iter().enumerate() {
349350
if i < num_cols {
350-
natural[i] = natural[i].max(cell.len());
351+
natural[i] = natural[i].max(UnicodeWidthStr::width(cell.as_str()));
351352
}
352353
}
353354
}
@@ -396,29 +397,68 @@ fn compute_column_widths(
396397
widths
397398
}
398399

399-
/// Wrap text into lines of at most `width` characters, breaking at word boundaries
400+
/// Wrap text into lines whose visual width is at most `width` columns,
401+
/// breaking at word boundaries when possible. Safe for any UTF-8 input:
402+
/// slice offsets are always taken at `char_indices()` boundaries, and
403+
/// widths are measured with `unicode-width` so CJK and other double-wide
404+
/// characters account for two visual columns.
400405
fn wrap_cell_text(text: &str, width: usize) -> Vec<String> {
401406
if width == 0 {
402407
return vec![text.to_string()];
403408
}
404-
if text.len() <= width {
409+
if UnicodeWidthStr::width(text) <= width {
405410
return vec![text.to_string()];
406411
}
407412

408413
let mut result = Vec::new();
409414
let mut remaining = text;
410415

411416
while !remaining.is_empty() {
412-
if remaining.len() <= width {
417+
if UnicodeWidthStr::width(remaining) <= width {
413418
result.push(remaining.to_string());
414419
break;
415420
}
416-
// Find last space within width
417-
let chunk = &remaining[..width];
418-
let break_at = chunk.rfind(' ').unwrap_or(width);
419-
let break_at = if break_at == 0 { width } else { break_at };
420-
result.push(remaining[..break_at].to_string());
421-
remaining = remaining[break_at..].trim_start();
421+
422+
let mut used = 0usize;
423+
let mut last_space_byte: Option<usize> = None;
424+
let mut break_byte: Option<usize> = None;
425+
let mut first_char_end: Option<usize> = None;
426+
427+
for (byte_idx, ch) in remaining.char_indices() {
428+
let char_w = UnicodeWidthChar::width(ch).unwrap_or(0);
429+
if first_char_end.is_none() {
430+
first_char_end = Some(byte_idx + ch.len_utf8());
431+
}
432+
if used + char_w > width {
433+
break_byte = Some(byte_idx);
434+
break;
435+
}
436+
if ch == ' ' {
437+
last_space_byte = Some(byte_idx);
438+
}
439+
used += char_w;
440+
}
441+
442+
let (chunk_end, resume_start) = match break_byte {
443+
Some(bb) => match last_space_byte {
444+
Some(sb) if sb > 0 => (sb, sb + 1),
445+
_ => {
446+
// No usable space: break mid-word at the char boundary.
447+
// If nothing fit (e.g. width=1 and a double-wide char),
448+
// force-consume the first char to guarantee progress.
449+
if bb == 0 {
450+
let end = first_char_end.unwrap_or(remaining.len());
451+
(end, end)
452+
} else {
453+
(bb, bb)
454+
}
455+
}
456+
},
457+
None => (remaining.len(), remaining.len()),
458+
};
459+
460+
result.push(remaining[..chunk_end].to_string());
461+
remaining = remaining[resume_start..].trim_start();
422462
}
423463

424464
if result.is_empty() {
@@ -471,10 +511,17 @@ fn render_table_row(
471511
.and_then(|w| w.get(line_idx))
472512
.map(|s| s.as_str())
473513
.unwrap_or("");
474-
spans.push(Span::styled(
475-
format!("{:<width$}", text, width = width),
476-
style,
477-
));
514+
// Pad by visual columns, not by chars: Rust's `{:<width$}` counts
515+
// chars, which misaligns borders when cells contain double-wide
516+
// characters (CJK, emoji).
517+
let visual = UnicodeWidthStr::width(text);
518+
let pad = width.saturating_sub(visual);
519+
let mut cell = String::with_capacity(text.len() + pad);
520+
cell.push_str(text);
521+
if pad > 0 {
522+
cell.push_str(&" ".repeat(pad));
523+
}
524+
spans.push(Span::styled(cell, style));
478525
}
479526
spans.push(Span::styled(" │", border));
480527
lines.push(Line::from(spans));
@@ -501,3 +548,170 @@ fn render_table_separator(
501548
s.push_str("─┤");
502549
lines.push(Line::from(Span::styled(s, sep_style)));
503550
}
551+
552+
#[cfg(test)]
553+
mod tests {
554+
use super::*;
555+
556+
fn visual_width(s: &str) -> usize {
557+
UnicodeWidthStr::width(s)
558+
}
559+
560+
#[test]
561+
fn ascii_short_returns_as_is() {
562+
assert_eq!(wrap_cell_text("hello", 10), vec!["hello".to_string()]);
563+
}
564+
565+
#[test]
566+
fn ascii_wrap_at_space() {
567+
let out = wrap_cell_text("the quick brown fox jumps", 10);
568+
for line in &out {
569+
assert!(visual_width(line) <= 10, "line {line:?} exceeds width");
570+
}
571+
assert!(out.len() >= 2);
572+
assert!(out.iter().all(|l| !l.starts_with(' ') && !l.ends_with(' ')));
573+
}
574+
575+
#[test]
576+
fn ascii_no_space_forced_break() {
577+
let out = wrap_cell_text("abcdefghij", 5);
578+
assert_eq!(out, vec!["abcde".to_string(), "fghij".to_string()]);
579+
}
580+
581+
#[test]
582+
fn empty_input() {
583+
assert_eq!(wrap_cell_text("", 10), vec!["".to_string()]);
584+
}
585+
586+
#[test]
587+
fn zero_width_returns_input() {
588+
assert_eq!(wrap_cell_text("hola", 0), vec!["hola".to_string()]);
589+
}
590+
591+
/// Regression test for the crash reported against `devtrail explore`:
592+
/// width offset landing inside a 3-byte em-dash used to panic with
593+
/// "byte index is not a char boundary".
594+
#[test]
595+
fn em_dash_no_panic() {
596+
let prefix = "middleware adds tenant isolation at DB layer. Partially m"; // 57 bytes
597+
let text = format!("{prefix}itigated — RLS is not active until middleware is connected.");
598+
// Width smaller than the text in visual columns, near the em-dash.
599+
for w in [30usize, 50, 60, 67, 80] {
600+
let out = wrap_cell_text(&text, w);
601+
assert!(!out.is_empty());
602+
for line in &out {
603+
assert!(std::str::from_utf8(line.as_bytes()).is_ok());
604+
assert!(visual_width(line) <= w, "{line:?} exceeds width {w}");
605+
}
606+
}
607+
}
608+
609+
#[test]
610+
fn accents_counted_as_one_column() {
611+
// "áéíóú" is 5 code points, each width 1.
612+
assert_eq!(wrap_cell_text("áéíóú", 5), vec!["áéíóú".to_string()]);
613+
}
614+
615+
#[test]
616+
fn cjk_double_width() {
617+
// Each ideogram has visual width 2, so width=6 fits 3 chars per line.
618+
let out = wrap_cell_text("数据表格示例", 6);
619+
assert_eq!(out.len(), 2);
620+
for line in &out {
621+
assert!(visual_width(line) <= 6);
622+
}
623+
assert_eq!(out[0].chars().count(), 3);
624+
assert_eq!(out[1].chars().count(), 3);
625+
}
626+
627+
#[test]
628+
fn emoji_no_panic() {
629+
let out = wrap_cell_text("hola 🚀 mundo feliz", 6);
630+
assert!(!out.is_empty());
631+
for line in &out {
632+
assert!(std::str::from_utf8(line.as_bytes()).is_ok());
633+
}
634+
}
635+
636+
#[test]
637+
fn word_longer_than_width_breaks_mid_word() {
638+
let out = wrap_cell_text("supercalifragilistic", 5);
639+
assert!(out.len() >= 4);
640+
for line in &out {
641+
assert!(visual_width(line) <= 5);
642+
}
643+
let joined: String = out.concat();
644+
assert_eq!(joined, "supercalifragilistic");
645+
}
646+
647+
#[test]
648+
fn leading_trailing_spaces_trimmed_between_chunks() {
649+
let out = wrap_cell_text("alpha beta gamma delta", 6);
650+
for line in &out {
651+
assert!(!line.starts_with(' '));
652+
assert!(!line.ends_with(' '));
653+
}
654+
}
655+
656+
#[test]
657+
fn width_one_with_cjk_terminates() {
658+
// A width-2 ideogram into width=1: guarantees forward progress by
659+
// force-consuming one char per iteration. Must not loop forever.
660+
let out = wrap_cell_text("数据", 1);
661+
assert_eq!(out.len(), 2);
662+
}
663+
664+
#[test]
665+
fn natural_widths_measure_visual() {
666+
let header: Vec<String> = vec!["数据".to_string()];
667+
let body: Vec<Vec<String>> = vec![];
668+
// Large available width so we return natural widths directly.
669+
let widths = compute_column_widths(&header, &body, 100);
670+
assert_eq!(widths.len(), 1);
671+
// "数据" has visual width 4; minimum clamp is 3, so result is 4.
672+
assert_eq!(widths[0], 4);
673+
}
674+
675+
#[test]
676+
fn cjk_fits_without_scaling() {
677+
let header: Vec<String> = vec!["列1".to_string(), "列2".to_string()];
678+
let body: Vec<Vec<String>> = vec![vec!["数据".to_string(), "テスト".to_string()]];
679+
let widths = compute_column_widths(&header, &body, 100);
680+
assert_eq!(widths.len(), 2);
681+
// Col0: max of "列1" (3) and "数据" (4) = 4.
682+
assert_eq!(widths[0], 4);
683+
// Col1: max of "列2" (3) and "テスト" (6) = 6.
684+
assert_eq!(widths[1], 6);
685+
}
686+
687+
/// End-to-end regression: the exact table row that crashed
688+
/// `devtrail explore` must render through the full pipeline
689+
/// (parser + renderer + cell wrapping) without panicking.
690+
#[test]
691+
fn full_pipeline_em_dash_table_no_panic() {
692+
let md = "\
693+
| Risk | Prob | Impact | Score | Mitigation |
694+
|------|------|--------|-------|------------|
695+
| E-003 | 2 | 3 | 6 | Admin/SuperAdmin role required. RLS middleware adds tenant isolation at DB layer. **Partially mitigated** — RLS is not active until Auth middleware is connected (Etapa 4). |
696+
";
697+
// Widths near the one that triggered the original panic.
698+
for w in [60usize, 80, 100, 120, 160] {
699+
let lines = markdown_to_lines(md, w);
700+
assert!(!lines.is_empty());
701+
}
702+
}
703+
704+
#[test]
705+
fn proportional_distribution_respects_budget() {
706+
let header: Vec<String> = vec!["A".to_string(), "B".to_string()];
707+
let body: Vec<Vec<String>> = vec![vec![
708+
"数据数据数据数据".to_string(),
709+
"テストテストテスト".to_string(),
710+
]];
711+
let available = 30;
712+
let widths = compute_column_widths(&header, &body, available);
713+
let border_overhead = 2 + (widths.len() - 1) * 3 + 2;
714+
let content: usize = widths.iter().sum();
715+
assert!(content + border_overhead <= available);
716+
}
717+
}

0 commit comments

Comments
 (0)