Skip to content

Address PR #6 review comments#8

Open
Aaron Wang (tabletenniser) wants to merge 2 commits into
feature/compact-responsesfrom
aaron_address_pr6_comments
Open

Address PR #6 review comments#8
Aaron Wang (tabletenniser) wants to merge 2 commits into
feature/compact-responsesfrom
aaron_address_pr6_comments

Conversation

@tabletenniser
Copy link
Copy Markdown
Contributor

@tabletenniser Aaron Wang (tabletenniser) commented May 27, 2026

Follow-up to #6 that resolves the unresolved review comments.

Summary

  • P1 (discussion_r2996914192)_extract_block_text is now recursive, so nested rich-text containers (rich_text_list, rich_text_quote, etc.) no longer silently drop content when the top-level text is empty.
  • nit (discussion_r3038237696)_compact_attachment collapsed to a single dict comprehension.
  • nit (discussion_r3038241296)_compact_message optional-field copying collapsed to a tuple-driven loop.
  • nit (discussion_r3038236233)_compact_user profile-field copying collapsed to a tuple-driven loop with the walrus operator.
  • ask (review 4060642575) — added tests/test_compact_responses.py covering all five compact helpers plus the recursive Block Kit walker (36 new tests; existing 27 still pass).

Test plan

  • uv run pytest tests/ — 63 passed

Note

Low Risk
Localized changes to response-shaping helpers plus new tests; the main functional change improves text fallback for nested Block Kit, with low blast radius outside compact MCP tool output.

Overview
Fixes Block Kit text extraction when messages have empty top-level text: _extract_block_text now walks nested rich-text containers (rich_text_list, rich_text_quote, etc.) via a recursive _walk_rich_text helper, so list/quote content is no longer dropped in compact channel/search responses.

Several compact helpers are refactored without intended behavior changes—_compact_attachment uses a dict comprehension; _compact_message copies optional keys in a loop; _compact_user loops profile fields and treats "profile": null safely with user.get("profile") or {}.

Adds tests/test_compact_responses.py with broad coverage of the compact helpers and the recursive Block Kit walker (per PR #6 review follow-up).

Reviewed by Cursor Bugbot for commit dc245e3. Bugbot is set up for automated code reviews on this repo. Configure here.

- Make _extract_block_text recursive to handle nested rich_text containers
  (rich_text_list, rich_text_quote, etc.) so compact mode no longer drops
  message content for those Block Kit payloads
- Simplify _compact_attachment, _compact_message, and _compact_user using
  tuple-driven loops / comprehensions per reviewer nits
- Add unit test coverage for all compact helpers and the recursive
  rich_text extraction

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is ON, but it could not run because the service account is not on the team's cloud agent allowlist. To enable Bugbot Autofix, have a team admin update the allowlist in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2dc8fa9. Configure here.

Comment thread slack_tools.py
user.get("profile", {}) returns None when the key is present with a null
value (the default only applies when missing). The earlier refactor lost
the implicit falsy guard, which would raise AttributeError on those
payloads. Use `or {}` instead and pin the behavior with a regression test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants