From bdd71c600096273512f2185ee8843721cfb15ece Mon Sep 17 00:00:00 2001 From: alonelish Date: Wed, 15 Jul 2026 14:46:26 +0300 Subject: [PATCH 1/3] ROB-3946 Truncate over-wide summary table cells instead of wrapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Slack "Alerts Summary" digest rendered a corrupted ASCII table: long values in text columns (e.g. Java class names in `label:site`) were passed to tabulate via `maxcolwidths`, which *wraps* cells onto extra physical lines rather than truncating them, mangling every following row. TableBlock.to_table_string now truncates over-wide cells itself (with a single-char "…" ellipsis) so each row stays on one line, and no longer passes maxcolwidths to tabulate. Dotted, space-free qualified names are trimmed from the left to keep the distinctive class-name suffix (e.g. `…settler.AbstractBetSettler`); other text is trimmed from the right. __calc_max_width now water-fills the reduction across the widest text columns (fair distribution, with a minimum width floor) and never shrinks numeric columns, so the Fired/Resolved counters are always shown in full. Adds regression tests proving rows stay single-line and are truncated (not wrapped). Co-Authored-By: Claude Opus 4.8 --- src/robusta/core/reporting/blocks.py | 97 +++++++++++++++++++++++----- tests/test_blocks.py | 82 +++++++++++++++++++++++ 2 files changed, 163 insertions(+), 16 deletions(-) diff --git a/src/robusta/core/reporting/blocks.py b/src/robusta/core/reporting/blocks.py index 9e6c7cf6a..3de9784e5 100644 --- a/src/robusta/core/reporting/blocks.py +++ b/src/robusta/core/reporting/blocks.py @@ -38,6 +38,13 @@ def tabulate(*args, **kwargs): BLOCK_SIZE_LIMIT = 2997 # due to slack block size limit of 3000 +# A single-character ellipsis is used (rather than "...") for table-cell truncation +# so the width accounting stays exact - it consumes exactly one column of the cell. +TABLE_TRUNCATION_ELLIPSIS = "…" +# Never shrink a text column below this many characters, otherwise the value +# becomes unreadable (essentially just the ellipsis). +TABLE_MIN_COLUMN_WIDTH = 4 + class MarkdownBlock(BaseBlock): """ @@ -364,28 +371,78 @@ 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: + # A column is considered numeric when every non-empty cell parses as a + # number (e.g. the "Fired"/"Resolved" counters). These are kept at full + # width so the counts always stay readable. + numeric_indices = set() + for idx in range(num_columns): + values = [str(row[idx]) for row in rendered_rows if idx < len(row)] + non_empty = [v for v in values if v.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] + # We need to make sure the total table width doesn't exceed the max width, + # otherwise the table is printed corrupted. + num_columns = len(headers) + columns_max_widths = [len(str(header)) for header in headers] 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; numeric columns (counters) are left intact so + # the numbers are never the ones that get truncated. + 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: # nothing but numeric columns - fall back to shrinking everything + shrinkable = list(range(num_columns)) + + # Water-fill: repeatedly trim the currently-widest shrinkable column by one + # character. This distributes the reduction fairly (widest first) instead of + # gutting a single column, and never drops a column 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 # can't shrink any further without making columns unreadable + 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 an over-wide cell to a single line with an ellipsis, instead of + # letting tabulate word-wrap it onto extra physical lines (which mangles + # every following row in the rendered table). + 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) + # For dotted, space-free qualified names (e.g. Java class paths like + # "ats.betting...settler.AbstractBetSettler") the distinctive part is the + # suffix (the class name), so trim from the left and keep the tail. For any + # other text a trailing cut reads more naturally. + 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 +487,19 @@ 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 rather than passing maxcolwidths to + # tabulate. tabulate would *wrap* long cells onto extra physical lines, + # which corrupts the whole table; truncating keeps every row on one line. + 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..1ff5f6870 100644 --- a/tests/test_blocks.py +++ b/tests/test_blocks.py @@ -134,3 +134,85 @@ 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: the "Alerts Summary" digest table +# rendered a corrupted ASCII table because long cell values (Java class names in +# the label:site column) were word-wrapped onto extra physical lines instead of +# being truncated. These tests pin down that rows stay single-line and over-wide +# cells are truncated with an ellipsis. +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 line + separator line + exactly one line per row. + # If any cell had wrapped, there would be extra physical lines here. + assert len(lines) == 2 + len(rows) + # Every physical line stays bounded: the content budget (table_max_width) plus + # the fixed per-column separator/padding overhead tabulate adds. Without the + # fix, wrapped cells would have blown past this and split rows across lines. + assert all(len(line) <= 40 + 6 * len(table_block.headers) for line in lines) + + # The over-wide class name is truncated, not present in full, and the "wrap + # spillover" fragments from the bug report never appear on their own line. + 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) From aeac17374161adf97df281f3b5e4d7e4bc93925f Mon Sep 17 00:00:00 2001 From: alonelish Date: Wed, 15 Jul 2026 15:01:07 +0300 Subject: [PATCH 2/3] ROB-3946 Address review: headerless/ragged rows and all-numeric tables - __calc_max_width now sizes columns to the widest row, not just the headers, so headerless or ragged tables no longer raise IndexError. - An all-numeric over-width table is left at full width instead of ellipsizing the numbers. - Shorten code/test comments; add regression tests for both cases. Co-Authored-By: Claude Opus 4.8 --- src/robusta/core/reporting/blocks.py | 52 +++++++++++----------------- tests/test_blocks.py | 42 +++++++++++++++------- 2 files changed, 51 insertions(+), 43 deletions(-) diff --git a/src/robusta/core/reporting/blocks.py b/src/robusta/core/reporting/blocks.py index 3de9784e5..638b9874b 100644 --- a/src/robusta/core/reporting/blocks.py +++ b/src/robusta/core/reporting/blocks.py @@ -38,11 +38,9 @@ def tabulate(*args, **kwargs): BLOCK_SIZE_LIMIT = 2997 # due to slack block size limit of 3000 -# A single-character ellipsis is used (rather than "...") for table-cell truncation -# so the width accounting stays exact - it consumes exactly one column of the cell. +# Single-char ellipsis so truncated-cell width accounting stays exact (one column). TABLE_TRUNCATION_ELLIPSIS = "…" -# Never shrink a text column below this many characters, otherwise the value -# becomes unreadable (essentially just the ellipsis). +# Don't shrink a text column below this, or it becomes just the ellipsis. TABLE_MIN_COLUMN_WIDTH = 4 @@ -381,23 +379,21 @@ def __is_number(cls, value) -> bool: @classmethod def __numeric_column_indices(cls, rendered_rows, num_columns: int) -> set: - # A column is considered numeric when every non-empty cell parses as a - # number (e.g. the "Fired"/"Resolved" counters). These are kept at full - # width so the counts always stay readable. + # Numeric columns (e.g. the Fired/Resolved counters) are kept at full width. numeric_indices = set() for idx in range(num_columns): - values = [str(row[idx]) for row in rendered_rows if idx < len(row)] - non_empty = [v for v in values if v.strip() != ""] + 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. - num_columns = len(headers) - columns_max_widths = [len(str(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]) @@ -405,20 +401,19 @@ def __calc_max_width(cls, headers, rendered_rows, table_max_width: int) -> List[ if sum(columns_max_widths) <= table_max_width: return columns_max_widths - # Only shrink text columns; numeric columns (counters) are left intact so - # the numbers are never the ones that get truncated. + # 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: # nothing but numeric columns - fall back to shrinking everything - shrinkable = list(range(num_columns)) + if not shrinkable: + return columns_max_widths - # Water-fill: repeatedly trim the currently-widest shrinkable column by one - # character. This distributes the reduction fairly (widest first) instead of - # gutting a single column, and never drops a column below TABLE_MIN_COLUMN_WIDTH. + # 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 # can't shrink any further without making columns unreadable + break widest = max(candidates, key=lambda idx: columns_max_widths[idx]) columns_max_widths[widest] -= 1 @@ -426,19 +421,15 @@ def __calc_max_width(cls, headers, rendered_rows, table_max_width: int) -> List[ @classmethod def __truncate_cell(cls, value: str, max_width: int) -> str: - # Truncate an over-wide cell to a single line with an ellipsis, instead of - # letting tabulate word-wrap it onto extra physical lines (which mangles - # every following row in the rendered table). + # 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) - # For dotted, space-free qualified names (e.g. Java class paths like - # "ats.betting...settler.AbstractBetSettler") the distinctive part is the - # suffix (the class name), so trim from the left and keep the tail. For any - # other text a trailing cut reads more naturally. + # 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 @@ -487,9 +478,8 @@ 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 rather than passing maxcolwidths to - # tabulate. tabulate would *wrap* long cells onto extra physical lines, - # which corrupts the whole table; truncating keeps every row on one line. + # 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) ] diff --git a/tests/test_blocks.py b/tests/test_blocks.py index 1ff5f6870..e92af01a1 100644 --- a/tests/test_blocks.py +++ b/tests/test_blocks.py @@ -136,11 +136,8 @@ def test_all_block_types(slack_channel: SlackChannel): print(result) -# Regression tests for FRO-211 / ROB-3946: the "Alerts Summary" digest table -# rendered a corrupted ASCII table because long cell values (Java class names in -# the label:site column) were word-wrapped onto extra physical lines instead of -# being truncated. These tests pin down that rows stay single-line and over-wide -# cells are truncated with an ellipsis. +# 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" @@ -154,16 +151,12 @@ def test_to_table_string_truncates_wide_column_without_wrapping(): output = table_block.to_table_string(table_max_width=40) lines = output.splitlines() - # presto renders: header line + separator line + exactly one line per row. - # If any cell had wrapped, there would be extra physical lines here. + # presto renders header + separator + one line per row; wrapping would add lines. assert len(lines) == 2 + len(rows) - # Every physical line stays bounded: the content budget (table_max_width) plus - # the fixed per-column separator/padding overhead tabulate adds. Without the - # fix, wrapped cells would have blown past this and split rows across lines. + # 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 over-wide class name is truncated, not present in full, and the "wrap - # spillover" fragments from the bug report never appear on their own line. + # 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) @@ -216,3 +209,28 @@ def test_to_markdown_wide_table_stays_single_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 From 82fb6a2b17cd8dba7c3ef6f56f1bbe7ba60dd125 Mon Sep 17 00:00:00 2001 From: alonelish Date: Wed, 15 Jul 2026 15:19:23 +0300 Subject: [PATCH 3/3] ROB-3946 Genericize numeric-column comment in shared blocks module Drop the Slack-digest-specific "Fired/Resolved" example, since blocks.py is core code shared across all sinks. Co-Authored-By: Claude Opus 4.8 --- src/robusta/core/reporting/blocks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/robusta/core/reporting/blocks.py b/src/robusta/core/reporting/blocks.py index 638b9874b..e615f85a1 100644 --- a/src/robusta/core/reporting/blocks.py +++ b/src/robusta/core/reporting/blocks.py @@ -379,7 +379,7 @@ def __is_number(cls, value) -> bool: @classmethod def __numeric_column_indices(cls, rendered_rows, num_columns: int) -> set: - # Numeric columns (e.g. the Fired/Resolved counters) are kept at full width. + # 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()]