diff --git a/src/robusta/core/reporting/blocks.py b/src/robusta/core/reporting/blocks.py index 9e6c7cf6a..e615f85a1 100644 --- a/src/robusta/core/reporting/blocks.py +++ b/src/robusta/core/reporting/blocks.py @@ -38,6 +38,11 @@ def tabulate(*args, **kwargs): BLOCK_SIZE_LIMIT = 2997 # due to slack block size limit of 3000 +# Single-char ellipsis so truncated-cell width accounting stays exact (one column). +TABLE_TRUNCATION_ELLIPSIS = "…" +# Don't shrink a text column below this, or it becomes just the ellipsis. +TABLE_MIN_COLUMN_WIDTH = 4 + class MarkdownBlock(BaseBlock): """ @@ -364,28 +369,71 @@ def __init__( if table_format: self.metadata["format"] = TableBlockFormat.vertical.value + @classmethod + def __is_number(cls, value) -> bool: + try: + float(str(value)) + return True + except (TypeError, ValueError): + return False + + @classmethod + def __numeric_column_indices(cls, rendered_rows, num_columns: int) -> set: + # Numeric columns (e.g. counters) are kept at full width. + numeric_indices = set() + for idx in range(num_columns): + non_empty = [str(row[idx]) for row in rendered_rows if idx < len(row) and str(row[idx]).strip()] + if non_empty and all(cls.__is_number(v) for v in non_empty): + numeric_indices.add(idx) + return numeric_indices + @classmethod def __calc_max_width(cls, headers, rendered_rows, table_max_width: int) -> List[int]: - # We need to make sure the total table width, doesn't exceed the max width, - # otherwise, the table is printed corrupted - columns_max_widths = [len(header) for header in headers] + # Keep the total table width within the max, otherwise it renders corrupted. + num_columns = max(len(headers), max((len(row) for row in rendered_rows), default=0)) + columns_max_widths = [0] * num_columns + for idx, header in enumerate(headers): + columns_max_widths[idx] = len(str(header)) for row in rendered_rows: for idx, val in enumerate(row): columns_max_widths[idx] = max(len(str(val)), columns_max_widths[idx]) - if sum(columns_max_widths) > table_max_width: # We want to limit the widest column - largest_width = max(columns_max_widths) - widest_column_idx = columns_max_widths.index(largest_width) - diff = sum(columns_max_widths) - table_max_width - columns_max_widths[widest_column_idx] = largest_width - diff - if columns_max_widths[widest_column_idx] < 0: # in case the diff is bigger than the largest column - # just divide equally - columns_max_widths = [ - int(table_max_width / len(columns_max_widths)) for i in range(0, len(columns_max_widths)) - ] + if sum(columns_max_widths) <= table_max_width: + return columns_max_widths + + # Only shrink text columns, so numeric columns are never truncated. If every + # column is numeric, leave the widths as-is rather than ellipsizing numbers. + numeric_indices = cls.__numeric_column_indices(rendered_rows, num_columns) + shrinkable = [idx for idx in range(num_columns) if idx not in numeric_indices] + if not shrinkable: + return columns_max_widths + + # Water-fill: trim the widest shrinkable column by one char at a time (fair + # distribution), never below TABLE_MIN_COLUMN_WIDTH. + while sum(columns_max_widths) > table_max_width: + candidates = [idx for idx in shrinkable if columns_max_widths[idx] > TABLE_MIN_COLUMN_WIDTH] + if not candidates: + break + widest = max(candidates, key=lambda idx: columns_max_widths[idx]) + columns_max_widths[widest] -= 1 return columns_max_widths + @classmethod + def __truncate_cell(cls, value: str, max_width: int) -> str: + # Truncate over-wide cells to one line, instead of letting tabulate wrap them. + if max_width <= 0 or len(value) <= max_width: + return value + ellipsis = TABLE_TRUNCATION_ELLIPSIS + if max_width <= len(ellipsis): + return value[:max_width] + keep = max_width - len(ellipsis) + # Dotted, space-free names (e.g. Java class paths) keep their distinctive + # suffix via a left trim; other text is trimmed on the right. + if "." in value and " " not in value: + return ellipsis + value[-keep:] + return value[:keep] + ellipsis + @classmethod def __trim_rows(cls, contents: str, max_chars: int): # We need to make sure that the total character count doesn't exceed max_chars, @@ -430,11 +478,18 @@ def to_markdown(self, max_chars=None, add_table_header: bool = True) -> Markdown def to_table_string(self, table_max_width: int = PRINTED_TABLE_MAX_WIDTH, table_fmt: str = "presto") -> str: rendered_rows = self.__to_strings_rows(self.render_rows()) col_max_width = self.__calc_max_width(self.headers, rendered_rows, table_max_width) + # Truncate over-wide cells ourselves; tabulate's maxcolwidths would wrap them + # onto extra lines and corrupt the table. + truncated_headers = [ + self.__truncate_cell(str(header), col_max_width[idx]) for idx, header in enumerate(self.headers) + ] + truncated_rows = [ + [self.__truncate_cell(val, col_max_width[idx]) for idx, val in enumerate(row)] for row in rendered_rows + ] return tabulate( - rendered_rows, - headers=self.headers, + truncated_rows, + headers=truncated_headers, tablefmt=table_fmt, - maxcolwidths=col_max_width, ) def render_rows(self) -> List[List]: diff --git a/tests/test_blocks.py b/tests/test_blocks.py index 86fdc4517..e92af01a1 100644 --- a/tests/test_blocks.py +++ b/tests/test_blocks.py @@ -134,3 +134,103 @@ def test_all_block_types(slack_channel: SlackChannel): result = slack_sender.send_finding_to_slack(finding, slack_params, False) # result = slack_sender.send_finding_to_slack(finding, slack_params, True) print(result) + + +# Regression tests for FRO-211 / ROB-3946: over-wide cells must be truncated to a +# single line, not word-wrapped onto extra lines (which corrupted the digest table). +LONG_CLASS_NAME = "ats.betting.betcatcher.settlement.settler.AbstractBetSettler" + + +def test_to_table_string_truncates_wide_column_without_wrapping(): + rows = [ + [LONG_CLASS_NAME, "103", "0"], + ["orders.checkout.impl.OrderServiceImpl", "16", "4"], + ] + table_block = TableBlock(rows=rows, headers=["label:site", "Fired", "Resolved"]) + + output = table_block.to_table_string(table_max_width=40) + lines = output.splitlines() + + # presto renders header + separator + one line per row; wrapping would add lines. + assert len(lines) == 2 + len(rows) + # Bounded width: content budget + tabulate's per-column separator/padding overhead. + assert all(len(line) <= 40 + 6 * len(table_block.headers) for line in lines) + + # The class name is truncated, not present in full, and no wrap-spillover fragment. + assert LONG_CLASS_NAME not in output + assert "…" in output + assert not any(line.strip() in ("ServiceImpl", "erviceImpl") for line in lines) + + +def test_to_table_string_keeps_distinctive_suffix_of_dotted_names(): + table_block = TableBlock(rows=[[LONG_CLASS_NAME, "103", "0"]], headers=["label:site", "Fired", "Resolved"]) + + output = table_block.to_table_string(table_max_width=40) + + # Dotted, space-free qualified names are trimmed from the left so the class + # name (the distinctive suffix) survives. + assert "AbstractBetSettler" in output + assert "ats.betting.betcatcher" not in output + + +def test_to_table_string_never_truncates_numeric_columns(): + table_block = TableBlock( + rows=[[LONG_CLASS_NAME, "103", "9999"]], + headers=["label:site", "Fired", "Resolved"], + ) + + output = table_block.to_table_string(table_max_width=40) + + # The numeric counters are always shown in full - only the wide text column shrinks. + assert "103" in output + assert "9999" in output + assert "…" not in output.split("103")[1] # nothing after the counters got ellipsized + + +def test_to_table_string_trailing_truncation_for_plain_text(): + long_sentence = "this is a fairly long free text value that should be cut at the end" + table_block = TableBlock(rows=[[long_sentence, "1", "0"]], headers=["message", "Fired", "Resolved"]) + + output = table_block.to_table_string(table_max_width=30) + + # Plain text (contains spaces) is truncated from the right, ending in an ellipsis. + assert "this is a fairly" in output + assert "…" in output + assert long_sentence not in output + + +def test_to_markdown_wide_table_stays_single_line_per_row(): + rows = [[LONG_CLASS_NAME, "103", "0"], ["orders.checkout.impl.OrderServiceImpl", "16", "4"]] + table_block = TableBlock(rows=rows, headers=["label:site", "Fired", "Resolved"]) + + markdown = table_block.to_markdown().text + + # Strip the ``` code fences, then assert the table body is header + separator + one line per row. + inner = markdown.strip().strip("`").strip("\n") + body_lines = [line for line in inner.splitlines() if line.strip()] + assert len(body_lines) == 2 + len(rows) + + +def test_to_table_string_headerless_and_ragged_rows(): + # No headers, and rows wider than the (empty) header list must not IndexError. + table_block = TableBlock(rows=[[LONG_CLASS_NAME, "extra", "cols"]], headers=[]) + + output = table_block.to_table_string(table_max_width=30) + + assert output # rendered without raising + assert LONG_CLASS_NAME not in output # still truncated + assert "…" in output + + +def test_to_table_string_all_numeric_table_is_not_truncated(): + # Every column numeric and over budget: leave widths intact rather than ellipsize numbers. + table_block = TableBlock( + rows=[["123456789012345", "678901234567890", "112233445566778"]], + headers=["a", "b", "c"], + ) + + output = table_block.to_table_string(table_max_width=10) + + assert "…" not in output + for value in ("123456789012345", "678901234567890", "112233445566778"): + assert value in output