Use native Slack Table Block for test results sample rendering#2227
Use native Slack Table Block for test results sample rendering#2227quickcoffee wants to merge 4 commits intoelementary-data:masterfrom
Conversation
Replaces the tabulate-based ASCII table in code blocks with Slack's native Table Block (type: "table"), giving column-aligned, scannable tables directly in alert messages. - Bold rich_text header row, raw_text data cells - Falls back to JSON code block for >20 columns (Slack limit) or if a second table would appear in the same message - Removes unused tabulate import and cell-truncation helpers from BlockKitBuilder - Regenerates all block_kit test fixtures to match the new format Closes elementary-data#2225
|
👋 @quickcoffee |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR replaces markdown/code-fence table rendering with native Slack Table Blocks, adds constants to cap columns (20) and rows (100), introduces header/data cell helpers, rewrites table-emission logic with a JSON fallback when limits or prior-table constraints apply, resets table state per build, and updates fixtures plus a unit test to the new structured table format. ChangesSlack Table Block Integration
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
quickcoffee
requested a deployment
to
elementary_test_env
GitHub Actions
Waiting
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55d2ac0fa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/unit/messages/formats/block_kit/fixtures/table_block_200_2.json (1)
93-102: ⚡ Quick win
"is_wrapped": falsehardcoded on all columns may silently clip 200-char cell contentAll fixtures set
"is_wrapped": falseon every column. For the 200-character cells in this fixture that is the entire cell content budget, meaning Slack will not wrap it — the row stays one-line and text is clipped or requires horizontal scroll. Thecolumn_settingsproperty controls text alignment and wrapping behaviour for table columns, so enablingis_wrapped: truefor columns that are expected to receive long free-text content would allow the full value to be visible.If this is intentional (keeping rows compact, relying on upstream truncation), at minimum the fixture name
table_block_200_2implies cells up to 200 characters are expected — that's a scenario where wrapping is needed to avoid loss of information.💡 Suggested `column_settings` for variable-length content columns
"column_settings": [ { "align": "left", - "is_wrapped": false + "is_wrapped": true }, { "align": "left", - "is_wrapped": false + "is_wrapped": true } ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/messages/formats/block_kit/fixtures/table_block_200_2.json` around lines 93 - 102, The fixture table_block_200_2.json sets "is_wrapped": false on every entry of the "column_settings" array which will force 200-character cell content to be clipped or require horizontal scrolling; update the "column_settings" entries for columns expected to hold long/free-form text to use "is_wrapped": true (leave short/static columns as false if desired) so Slack will wrap long cell content and preserve visibility — modify the "column_settings" objects in this fixture accordingly.elementary/messages/formats/block_kit.py (1)
283-285: ⚡ Quick winSilent row truncation gives users no indication that data was dropped.
When
block.rowsexceeds_MAX_SLACK_TABLE_ROWS - 1(99 rows), the slice on line 284 silently discards the remainder. Unlike the column-overflow and second-table cases — which fall back to a visible JSON code block — this case produces a structurally valid table that is simply incomplete, with no truncation notice.Consider appending an informational row (e.g.
"… N more rows") or logging a warning so operators know data was dropped.♻️ Proposed fix
- rows: List[List[dict]] = [[self._make_header_cell(h) for h in block.headers]] - for row in block.rows[: self._MAX_SLACK_TABLE_ROWS - 1]: - rows.append([self._make_data_cell(v) for v in row]) + max_data_rows = self._MAX_SLACK_TABLE_ROWS - 1 # one slot taken by header + data_rows = block.rows[:max_data_rows] + omitted = len(block.rows) - len(data_rows) + + rows: List[List[dict]] = [[self._make_header_cell(h) for h in block.headers]] + for row in data_rows: + rows.append([self._make_data_cell(v) for v in row]) + if omitted: + notice = f"… {omitted} more row(s) not shown" + rows.append([self._make_data_cell(notice)] + [self._make_data_cell("") for _ in block.headers[1:]])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@elementary/messages/formats/block_kit.py` around lines 283 - 285, The slice on block.rows silently drops rows when len(block.rows) > self._MAX_SLACK_TABLE_ROWS - 1; update the rows-building logic (where rows, _make_header_cell, _make_data_cell and block.rows are used) to detect omitted = len(block.rows) - (self._MAX_SLACK_TABLE_ROWS - 1) and, if omitted > 0, append a visible informational row indicating the number omitted (e.g. "… N more rows") so users see truncation; implement the info-row using _make_data_cell for the first column and matching empty cells for the rest (to keep column structure), and also emit a debug/warning log mentioning the truncation for operators.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@elementary/messages/formats/block_kit.py`:
- Around line 270-281: The fallback JSON-dump path in _add_table_block silently
truncates rows because zip(block.headers, row) isn't strict; update the dict
construction in _add_table_block to either use zip(block.headers, row,
strict=True) so a ValueError is raised on length mismatch, or explicitly
validate that len(row) == len(block.headers) before building dicts and handle
mismatches (e.g., log the problem and either pad/truncate deterministically or
raise) so malformed rows cannot produce corrupted dict output; keep references
to TableBlock, _MAX_SLACK_TABLE_COLUMNS, _has_table_block, _add_block, and
_format_markdown_section when locating the change.
---
Nitpick comments:
In `@elementary/messages/formats/block_kit.py`:
- Around line 283-285: The slice on block.rows silently drops rows when
len(block.rows) > self._MAX_SLACK_TABLE_ROWS - 1; update the rows-building logic
(where rows, _make_header_cell, _make_data_cell and block.rows are used) to
detect omitted = len(block.rows) - (self._MAX_SLACK_TABLE_ROWS - 1) and, if
omitted > 0, append a visible informational row indicating the number omitted
(e.g. "… N more rows") so users see truncation; implement the info-row using
_make_data_cell for the first column and matching empty cells for the rest (to
keep column structure), and also emit a debug/warning log mentioning the
truncation for operators.
In `@tests/unit/messages/formats/block_kit/fixtures/table_block_200_2.json`:
- Around line 93-102: The fixture table_block_200_2.json sets "is_wrapped":
false on every entry of the "column_settings" array which will force
200-character cell content to be clipped or require horizontal scrolling; update
the "column_settings" entries for columns expected to hold long/free-form text
to use "is_wrapped": true (leave short/static columns as false if desired) so
Slack will wrap long cell content and preserve visibility — modify the
"column_settings" objects in this fixture accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2e2b5c8-dc8b-430c-b7b4-bbb6cfb2c7f5
📒 Files selected for processing (20)
elementary/messages/formats/block_kit.pytests/unit/messages/formats/block_kit/fixtures/all_blocks_green.jsontests/unit/messages/formats/block_kit/fixtures/all_blocks_no_color.jsontests/unit/messages/formats/block_kit/fixtures/all_blocks_red.jsontests/unit/messages/formats/block_kit/fixtures/all_blocks_yellow.jsontests/unit/messages/formats/block_kit/fixtures/table_block_200_1.jsontests/unit/messages/formats/block_kit/fixtures/table_block_200_2.jsontests/unit/messages/formats/block_kit/fixtures/table_block_200_3.jsontests/unit/messages/formats/block_kit/fixtures/table_block_200_4.jsontests/unit/messages/formats/block_kit/fixtures/table_block_200_5.jsontests/unit/messages/formats/block_kit/fixtures/table_block_30_1.jsontests/unit/messages/formats/block_kit/fixtures/table_block_30_2.jsontests/unit/messages/formats/block_kit/fixtures/table_block_30_3.jsontests/unit/messages/formats/block_kit/fixtures/table_block_30_4.jsontests/unit/messages/formats/block_kit/fixtures/table_block_30_5.jsontests/unit/messages/formats/block_kit/fixtures/table_block_8_1.jsontests/unit/messages/formats/block_kit/fixtures/table_block_8_2.jsontests/unit/messages/formats/block_kit/fixtures/table_block_8_3.jsontests/unit/messages/formats/block_kit/fixtures/table_block_8_4.jsontests/unit/messages/formats/block_kit/fixtures/table_block_8_5.json
|
hey @quickcoffee, looks really awesome 🚀 |
Slack rejects raw_text cells with empty text. Convert None to "NULL" (represents database NULL values) and guard against any other value that stringifies to an empty string with a single-space fallback. Adds a dedicated test to assert no raw_text cell ever has empty text.
quickcoffee
requested a deployment
to
elementary_test_env
GitHub Actions
Waiting
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/messages/formats/block_kit/test_block_kit.py`:
- Around line 42-45: Run Black to reformat the test file containing the
TableBlock.from_dicts call and commit the changes so CI pre-commit passes;
specifically, open the test that constructs TableBlock.from_dicts([{"col_a":
None, "col_b": "value"}, {"col_a": "", "col_b": "other"}]) and apply Black (or
your project's pre-commit hooks) to produce the normalized formatting, then add
and commit the formatted file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 63836521-efa1-415f-bec1-96557216851c
📒 Files selected for processing (2)
elementary/messages/formats/block_kit.pytests/unit/messages/formats/block_kit/test_block_kit.py
|
@quickcoffee some of the tests are not passing, seems like not all of the test fixture were updated |
quickcoffee
requested a deployment
to
elementary_test_env
GitHub Actions
Waiting
2 participants
Summary
tabulate-based ASCII table-in-code-block rendering with Slack's native Table Block ("type": "table") inBlockKitBuilder._add_table_block()rich_textcells; data rows useraw_textcellstabulateimport and cell-truncation helpers fromBlockKitBuilderScreenshot on how this looks like in Slack (manually rendered via a script):
Closes #2225
Test plan
pytest tests/unit/messages/formats/block_kit/passes with regenerated fixturesSummary by CodeRabbit
New Features
Improvements / Bug Fixes
Tests