Repository navigation
slack: a reply posted with reply_broadcast is listed in conversations.history as a thread_broadcast carrying its root - #1151
Conversation
….history as a thread_broadcast carrying its root chat.postMessage with thread_ts and a true reply_broadcast stores the reply as subtype thread_broadcast. conversations.history lists it beside top-level messages, and the echo, history and conversations.replies carry its root, with no team or parent_user_id. Measured against slack.com on 2026-10-30. Closes brekkylab#1150.
nuriyoo
left a comment
There was a problem hiding this comment.
Thanks for this, and for measuring it on slack.com before writing it: the real column of the table in the description is what I saw too.
I measured reply_broadcast on slack.com on 2026-10-30, 10:32 to 10:42 UTC, in a private channel made for it: 213 calls, most with a bot token and the edit, reaction and plain history checks with a user token as well. Every message was deleted afterwards and the channel archived. I then sent the same 66 request sequences to Backlot in-process on the bundled corpus and compared order, subtype, keys and root fields: main matches real on 21, this head on 57, and the head with the suggestions on 65. The one left is conversations.replies with ts set to the broadcast's, which real answers with the broadcast alone: a ts that names a reply, which is #637.
What has to change, each in an inline comment:
reply_broadcastcounts beside anythread_tssent. Withthread_tsset to0, left empty, or naming no message, real posts the message top level as athread_broadcast, where this head posts a plain message.rootis on three more surfaces, and an edited broadcast has noedited. Thechat.updateecho,reactions.getandreactions.listcarryrooton real. Their handlers are outside the diff, so their change is a diff in the comment on_with_root.- The tests. The subtype-and-root test can read those surfaces and the fields no test pins yet (
blocks,reply_users,edited), and two test comments point at_with_rootfor measurements its docstring does not hold.
Smaller, also inline: the comment on team in _message, list_slack_top_level's docstring as the home of the history order, _flag's docstring with inclusive read through it, and _history's oldest.
Not this PR's: real's chat.postMessage echo of a plain reply carries parent_user_id, the root poster's id on all 32 I posted, and Backlot's carries none, on main too, though its conversations.replies does. I can file it.
On the description: with 2 in, its first paragraph can name chat.update, reactions.get and reactions.list beside the echo, history and replies. The checked first box can name the tests that fail on main, and the empty second one can say why it is empty: there is no new endpoint, and root is read with the caller's visible ids.
Non-blocking: backlot/routers/slack.py:2161 stops at 68 characters, and the first words of the next line fit on it.
With every suggestion and the diffs applied, on a .[dev] install: the full suite passes (8341 passed, 40 skipped), and 8696 passed, 40 skipped once merged with main at 257bc733; ruff format --check ., ruff check ., ruff check --preview --select PLW1514 . and scripts/prose_check.py exit 0. The two tests the suggestions extend fail on this head's code. I also changed each line the diff adds, one change at a time, 39 changes: the PR's tests catch 27, and with the suggestions 36. Of the three left, one moves the root lookup above the subtype check, which gives the same answer, and two are operands of the elif in _message that main already has.
| # A reply also sent to the channel. Measured against slack.com on 2026-10-30: it is | ||
| # listed in `conversations.history` and carries this subtype and a `root`. | ||
| "subtype": "thread_broadcast" | ||
| if thread_ts and _flag(request, "reply_broadcast") | ||
| else None, |
There was a problem hiding this comment.
Real reads reply_broadcast beside any thread_ts the request sends, not only one that names a thread. With reply_broadcast=true, on 2026-10-30:
thread_ts sent real this head
a root's ts or a reply's in the thread, thread_broadcast, root same
0, empty, or a ts naming no message top level, thread_broadcast, no team top level, plain message
none top level, plain message same
conversations.history lists the three in the middle row the same way. By this line thread_ts is the thread the post landed in, None for those three, so the condition reads the value sent instead. test_a_thread_ts_naming_nothing_posts_top_level_and_an_unreadable_one_is_refused already posts two of the three; it can take 0, a reply_broadcast column and the post with no thread_ts, below the suggestion since it is outside the diff.
| # A reply also sent to the channel. Measured against slack.com on 2026-10-30: it is | |
| # listed in `conversations.history` and carries this subtype and a `root`. | |
| "subtype": "thread_broadcast" | |
| if thread_ts and _flag(request, "reply_broadcast") | |
| else None, | |
| # The flag counts beside any `thread_ts` sent: measured against slack.com on 2026-10-30, | |
| # `0`, an empty one and one naming no message each posted the message top-level with | |
| # this subtype all the same, and a post with no `thread_ts` had no subtype. | |
| "subtype": "thread_broadcast" | |
| if _param(request, "thread_ts") is not None and _flag(request, "reply_broadcast") | |
| else None, |
Outside the diff, in tests/test_slack.py:
--- a/tests/test_slack.py
+++ b/tests/test_slack.py
@@ -5992,17 +5992,30 @@ test_a_thread_ts_naming_nothing_posts_top_level_and_an_unreadable_one_is_refused
wclient, tokens
):
# `chat_post_message` records the measurement: a well-formed `thread_ts` that names no message
- # posts top-level, an empty one does the same, and one real cannot read is refused.
+ # posts top-level, an empty one or `0` does the same, `reply_broadcast` makes each of those a
+ # `thread_broadcast` and a post with no `thread_ts` a plain message, and one real cannot read is
+ # refused.
h = _uh(tokens, "ava@acme.com")
cid = synth.slack_channel_id("incidents")
- for value in ("1700000000.000001", ""):
- j = wclient.post(
- "/slack/api/chat.postMessage",
- headers=h,
- data={"channel": cid, "text": "orphan", "thread_ts": value},
- ).json()
- assert j["ok"] is True, value
- assert "thread_ts" not in j["message"], value
+ for value in ("1700000000.000001", "", "0"):
+ for extra, subtype in (({}, None), ({"reply_broadcast": "true"}, "thread_broadcast")):
+ j = wclient.post(
+ "/slack/api/chat.postMessage",
+ headers=h,
+ data={"channel": cid, "text": "orphan", "thread_ts": value, **extra},
+ ).json()
+ assert j["ok"] is True, value
+ assert "thread_ts" not in j["message"] and "root" not in j["message"], value
+ assert (j["message"].get("subtype"), "team" in j["message"]) == (
+ subtype,
+ subtype is None,
+ ), value
+ alone = wclient.post(
+ "/slack/api/chat.postMessage",
+ headers=h,
+ data={"channel": cid, "text": "orphan", "reply_broadcast": "true"},
+ ).json()
+ assert "subtype" not in alone["message"]
# Refused outside the window `_thread_ts_error` records, and the message carries the value
# as real reads it, in microseconds.
import time as _time| """Add `root` to a `thread_broadcast` message: its thread's root, as history lists it. | ||
|
|
||
| Measured against slack.com on 2026-10-30: the echo, `conversations.history` and | ||
| `conversations.replies` all carry it, with the root's `reply_count` and `latest_reply`. | ||
| """ | ||
| if row["subtype"] != "thread_broadcast": | ||
| return m | ||
| root = store.document_by_key(conn, "slack", (name, row["thread_ts"]), ids) | ||
| if root is None: | ||
| return m | ||
| rc = store.slack_reply_count(conn, name, root["ts"], ids) | ||
| ru = store.slack_reply_authors(conn, name, root["ts"], ids) if rc else [] | ||
| m["root"] = _message( | ||
| root, | ||
| reply_count=rc, | ||
| latest_reply=store.slack_latest_reply_ts(conn, name, root["ts"], ids) if rc else None, | ||
| reply_users=[_uid(e) for e in ru[:5]], | ||
| reply_users_count=len(ru), | ||
| subscribed=_subscribed(caller, root["author_email"], ru), | ||
| ) |
There was a problem hiding this comment.
Real carries root on three surfaces besides the echo, history and replies, and an edited broadcast carries no edited on any of them, where an edited plain message does. On 2026-10-30, a broadcast edited with a user token and with a bot token, and one with a reaction added with a user token:
the broadcast in real this head
chat.update echo root, no edited edited, no root
conversations.history root, no edited root, edited
conversations.replies root, no edited root, edited
reactions.get root no root
reactions.list root no root
chat_update, reactions_get and reactions_list build the message with _message, as the three call sites in the diff do, but they are outside the diff, so their change is below the suggestion. In chat_update the call goes after the line that drops ts and reactions, which stays as it is, and the edited stamp skips a broadcast.
The if rc else guards never take their else: the broadcast is one of its root's replies, so rc is at least 1 wherever this runs, and replacing either guard with its first branch leaves every test passing.
| """Add `root` to a `thread_broadcast` message: its thread's root, as history lists it. | |
| Measured against slack.com on 2026-10-30: the echo, `conversations.history` and | |
| `conversations.replies` all carry it, with the root's `reply_count` and `latest_reply`. | |
| """ | |
| if row["subtype"] != "thread_broadcast": | |
| return m | |
| root = store.document_by_key(conn, "slack", (name, row["thread_ts"]), ids) | |
| if root is None: | |
| return m | |
| rc = store.slack_reply_count(conn, name, root["ts"], ids) | |
| ru = store.slack_reply_authors(conn, name, root["ts"], ids) if rc else [] | |
| m["root"] = _message( | |
| root, | |
| reply_count=rc, | |
| latest_reply=store.slack_latest_reply_ts(conn, name, root["ts"], ids) if rc else None, | |
| reply_users=[_uid(e) for e in ru[:5]], | |
| reply_users_count=len(ru), | |
| subscribed=_subscribed(caller, root["author_email"], ru), | |
| ) | |
| """Add `root` to a `thread_broadcast` in a thread: its thread's root, as history lists it. | |
| Measured against slack.com on 2026-10-30: the `chat.postMessage` and `chat.update` echoes, | |
| `conversations.history`, `conversations.replies`, `reactions.get` and `reactions.list` all | |
| carry it, with the root's `reply_count`, `reply_users` and `latest_reply`, and a broadcast | |
| posted top-level (`chat_post_message`) carries none. The broadcast is one of the root's | |
| replies, so `reply_count` is at least 1 here. | |
| """ | |
| if row["subtype"] != "thread_broadcast": | |
| return m | |
| root = store.document_by_key(conn, "slack", (name, row["thread_ts"]), ids) | |
| if root is None: | |
| return m | |
| rc = store.slack_reply_count(conn, name, root["ts"], ids) | |
| ru = store.slack_reply_authors(conn, name, root["ts"], ids) | |
| m["root"] = _message( | |
| root, | |
| reply_count=rc, | |
| latest_reply=store.slack_latest_reply_ts(conn, name, root["ts"], ids), | |
| reply_users=[_uid(e) for e in ru[:5]], | |
| reply_users_count=len(ru), | |
| subscribed=_subscribed(caller, root["author_email"], ru), | |
| ) |
Outside the diff, in backlot/routers/slack.py:
--- a/backlot/routers/slack.py
+++ b/backlot/routers/slack.py
@@ -2362,26 +2362,30 @@ chat_update
author = _writer(caller)
store.patch_document(conn, "slack", (name, ts), "content", text)
# Real Slack stamps an edited message with `edited: {user, ts}`, which is what a client renders
- # "(edited)" from. `_message` already serves the column, so filling it is the whole of it.
+ # "(edited)" from. `_message` already serves the column, so filling it is the whole of it. A
+ # `thread_broadcast` gets none: measured against slack.com on 2026-10-30, an edited one
+ # carried no `edited` in the echo, in history or in replies, over a user and a bot token.
#
# The ADDRESS is stored, not the id: `_edited` renders one from the other when it serves the
# message, so an id here would be rendered a second time — the same representation a corpus
# states and `reactions.users` keeps.
- store.patch_document(
- conn,
- "slack",
- (name, ts),
- "edited",
- json.dumps(
- {"user": author, "ts": synth.slack_fmt_ts(int(time.time()), f"edit:{name}:{ts}")}
- ),
- )
+ if row["subtype"] != "thread_broadcast":
+ store.patch_document(
+ conn,
+ "slack",
+ (name, ts),
+ "edited",
+ json.dumps(
+ {"user": author, "ts": synth.slack_fmt_ts(int(time.time()), f"edit:{name}:{ts}")}
+ ),
+ )
updated = store.document_by_key(conn, "slack", (name, ts))
# The message an edit echoes is not the message a read answers: measured against slack.com on
# 2026-10-21, `chat.update` carried neither `ts` nor `reactions` for a threaded message with a
# reaction, where `conversations.replies` answered the same message carrying both. `ts` is on
# the answer beside it either way.
echoed = {k: v for k, v in _message(updated).items() if k not in ("ts", "reactions")}
+ _with_root(conn, name, auth.visible_ids(request, caller), caller, updated, echoed)
return {
"ok": True,
"channel": synth.slack_channel_id(name),
@@ -2762,8 +2766,8 @@ reactions_get
Errors quoted from the spec's ``default`` response: `bad_timestamp`, `file_not_found`,
`file_comment_not_found`, `message_not_found`, `no_item_specified`.
- The message is built by `_message`, the same builder conversations.history uses, so a client
- that reads a reaction here and the message there cannot get two shapes for one message.
+ The message is built by `_message` and `_with_root`, as conversations.history builds it, so a
+ client that reads a reaction here and the message there cannot get two shapes for one message.
"""
conn = auth.conn(request)
caller, err = _caller_or_error(request)
@@ -2774,6 +2778,7 @@ reactions_get
)
if err is not None:
return err
+ message = _with_root(conn, name, auth.visible_ids(request, caller), caller, row, _message(row))
return {
"ok": True,
"type": "message",
@@ -2782,7 +2787,7 @@ reactions_get
# that answers one: measured against slack.com on 2026-10-21, `conversations.history` and
# `conversations.replies` carry none. So it goes on at these two call sites rather than
# in `_message`, which all of them share.
- "message": _message(row) | {"permalink": _permalink(name, row)},
+ "message": message | {"permalink": _permalink(name, row)},
}
@@ -2858,7 +2863,8 @@ reactions_list
{
"type": "message",
"channel": synth.slack_channel_id(channel),
- "message": _message(row) | {"permalink": _permalink(channel, row)},
+ "message": _with_root(conn, channel, ids, caller, row, _message(row))
+ | {"permalink": _permalink(channel, row)},
}
)
page = items[offset : offset + limit]| # Measured against slack.com on 2026-10-30: a reply posted with `reply_broadcast` carries | ||
| # no `team` and no `parent_user_id`, in the echo, in history and in replies. |
There was a problem hiding this comment.
With the change in the comment on thread_ts above, a broadcast can be top level as well, and real gives that one no team either (the middle row there). The list of methods can point at _with_root, which names them, rather than repeat it.
| # Measured against slack.com on 2026-10-30: a reply posted with `reply_broadcast` carries | |
| # no `team` and no `parent_user_id`, in the echo, in history and in replies. | |
| # Measured against slack.com on 2026-10-30: a `thread_broadcast` carries no `team`, one | |
| # posted top-level included, and a reply among them no `parent_user_id`, in each method | |
| # `_with_root` names. |
|
|
||
|
|
||
| def _flag(request: Request, key: str) -> bool: | ||
| """A boolean argument: absent, empty, `0` and `false` (any case) are off.""" |
There was a problem hiding this comment.
conversations_history reads inclusive with this same expression (line 1502, outside the diff), and its comment holds the measurement behind it, so one function can read both. reply_broadcast read the same on 2026-10-30, over 23 form values and 12 JSON ones: absent, empty, 0 and false in any case were off, and every other value on, 00, no, null and false among them. Non-blocking.
| """A boolean argument: absent, empty, `0` and `false` (any case) are off.""" | |
| """A boolean argument as `conversations_history` measured `inclusive`: absent, empty, `0` and | |
| `false` in any case are off, and every other value is on. `reply_broadcast` read the same as a | |
| form field on slack.com on 2026-10-30, `00`, `no`, `null` and ` false` among the values on.""" |
Outside the diff, in backlot/routers/slack.py:
--- a/backlot/routers/slack.py
+++ b/backlot/routers/slack.py
@@ -1499,7 +1499,7 @@ conversations_history
# Measured against slack.com/api on 2026-10-18: only absent, empty, "0" and
# case-insensitive "false" exclude the bound. Other strings, including "00", "no"
# and whitespace-padded "false", include it. Repeated query values use the last one.
- inclusive = (_param(request, "inclusive") or "").lower() not in ("", "0", "false")
+ inclusive = _flag(request, "inclusive")
def _ts_to_us(ts_str: str) -> int:
"""Convert a Slack ts string "SSSSSSSSSS.MMMMMM" to total microseconds."""| """Top-level (thread-root/standalone) messages in a channel, and the replies also sent to it | ||
| (`thread_broadcast`, which real Slack lists in history), oldest first unless | ||
| ``newest_first``. ``ts_lo``/``ts_hi`` bound ``created_ts`` for a time-windowed |
There was a problem hiding this comment.
This is the function that lists a broadcast at its own ts, so its docstring is the place for the order measurement, and for what test_deleting_a_broadcast_reply_takes_it_out_of_history pins. The two test comments can then point here, as scripts/prose_check.py asks of a dated measurement.
| """Top-level (thread-root/standalone) messages in a channel, and the replies also sent to it | |
| (`thread_broadcast`, which real Slack lists in history), oldest first unless | |
| ``newest_first``. ``ts_lo``/``ts_hi`` bound ``created_ts`` for a time-windowed | |
| """Top-level (thread-root/standalone) messages in a channel, and the replies also sent to it | |
| (`thread_broadcast`), each at its own ts, oldest first unless ``newest_first``: measured | |
| against slack.com on 2026-10-30, history listed a broadcast between its root and a later | |
| post, on `limit=1` pages too, and a deleted one not at all, with no placeholder and its root | |
| one reply fewer. ``ts_lo``/``ts_hi`` bound ``created_ts`` for a time-windowed |
| def _history(wclient, h, cid, **extra): | ||
| data = {"channel": cid, "oldest": extra.pop("oldest"), "inclusive": "true", **extra} |
There was a problem hiding this comment.
Every call passes oldest, and without it extra.pop raises KeyError, so it reads plainer as a parameter. Non-blocking.
| def _history(wclient, h, cid, **extra): | |
| data = {"channel": cid, "oldest": extra.pop("oldest"), "inclusive": "true", **extra} | |
| def _history(wclient, h, cid, oldest, **extra): | |
| data = {"channel": cid, "oldest": oldest, "inclusive": "true", **extra} |
| return wclient.post("/slack/api/conversations.history", headers=h, data=data).json() | ||
|
|
||
|
|
||
| # The order is real's: `_with_root` records the measurement. |
There was a problem hiding this comment.
_with_root's docstring says nothing about the order. The order is list_slack_top_level's, and the suggestion on its docstring puts the measurement there.
| # The order is real's: `_with_root` records the measurement. | |
| # The order is real's (`store.list_slack_top_level`). |
| hist = _history(wclient, h, cid, oldest=parent)["messages"] | ||
| replies = wclient.post( | ||
| "/slack/api/conversations.replies", headers=h, data={"channel": cid, "ts": parent} | ||
| ).json()["messages"] | ||
| by_name = { | ||
| "chat.postMessage": echo["message"], | ||
| "conversations.history": next(m for m in hist if names[m["ts"]] == "broadcast"), | ||
| "conversations.replies": next(m for m in replies if names[m["ts"]] == "broadcast"), | ||
| } | ||
| for method, m in by_name.items(): | ||
| assert m["subtype"] == "thread_broadcast", method | ||
| assert m["thread_ts"] == parent, method | ||
| assert "team" not in m and "parent_user_id" not in m, method | ||
| root = m["root"] | ||
| assert (root["ts"], root["text"], root["reply_count"]) == (parent, "bc-parent", 2), method | ||
| assert root["latest_reply"] == echo["ts"], method |
There was a problem hiding this comment.
With the change in the comment on _with_root, this test can read the three other surfaces, and three fields no test pins yet: blocks (_message serves it for a subtype of None or "thread_broadcast", and dropping the second leaves every test passing), the root's reply_users (an empty list there passes too) and edited.
| hist = _history(wclient, h, cid, oldest=parent)["messages"] | |
| replies = wclient.post( | |
| "/slack/api/conversations.replies", headers=h, data={"channel": cid, "ts": parent} | |
| ).json()["messages"] | |
| by_name = { | |
| "chat.postMessage": echo["message"], | |
| "conversations.history": next(m for m in hist if names[m["ts"]] == "broadcast"), | |
| "conversations.replies": next(m for m in replies if names[m["ts"]] == "broadcast"), | |
| } | |
| for method, m in by_name.items(): | |
| assert m["subtype"] == "thread_broadcast", method | |
| assert m["thread_ts"] == parent, method | |
| assert "team" not in m and "parent_user_id" not in m, method | |
| root = m["root"] | |
| assert (root["ts"], root["text"], root["reply_count"]) == (parent, "bc-parent", 2), method | |
| assert root["latest_reply"] == echo["ts"], method | |
| def call(method, **data): | |
| return wclient.post(f"/slack/api/{method}", headers=h, data=data).json() | |
| assert call("reactions.add", channel=cid, timestamp=echo["ts"], name="eyes")["ok"] is True | |
| edited = call("chat.update", channel=cid, ts=echo["ts"], text="bc-broadcast, edited") | |
| hist = _history(wclient, h, cid, oldest=parent)["messages"] | |
| replies = call("conversations.replies", channel=cid, ts=parent)["messages"] | |
| listed = call("reactions.list")["items"] | |
| by_name = { | |
| "chat.postMessage": echo["message"], | |
| "chat.update": edited["message"], | |
| "conversations.history": next(m for m in hist if names[m["ts"]] == "broadcast"), | |
| "conversations.replies": next(m for m in replies if names[m["ts"]] == "broadcast"), | |
| "reactions.get": call("reactions.get", channel=cid, timestamp=echo["ts"])["message"], | |
| "reactions.list": next(i["message"] for i in listed if i["message"]["ts"] == echo["ts"]), | |
| } | |
| ava = synth.slack_user_id("ava@acme.com") | |
| for method, m in by_name.items(): | |
| assert m["subtype"] == "thread_broadcast", method | |
| assert m["thread_ts"] == parent, method | |
| assert "blocks" in m, method | |
| assert "team" not in m and "parent_user_id" not in m and "edited" not in m, method | |
| root = m["root"] | |
| assert (root["ts"], root["text"], root["reply_count"]) == (parent, "bc-parent", 2), method | |
| assert (root["reply_users"], root["reply_users_count"]) == ([ava], 1), method | |
| assert root["latest_reply"] == echo["ts"], method |
|
|
||
|
|
||
| def test_deleting_a_broadcast_reply_takes_it_out_of_history(wclient, tokens): | ||
| # Real leaves no placeholder and the root counts one reply fewer (measured with `_with_root`'s). |
There was a problem hiding this comment.
_with_root's docstring does not hold this measurement either. The suggestion on list_slack_top_level's docstring adds it, and this can point there.
| # Real leaves no placeholder and the root counts one reply fewer (measured with `_with_root`'s). | |
| # Real leaves no placeholder and the root counts one reply fewer (`store.list_slack_top_level`). |
…on chat.update, reactions.get and reactions.list
|
tks for the detailed review + measurements! applied all ur suggestions and diffs in 101880c, incl. the wrap at 2161. re-ran on live slack: top-level broadcasts with |
What changed
chat.postMessagereadsreply_broadcastbeside anythread_tssent. A reply sent with it is stored assubtype: thread_broadcastand listed inconversations.history;thread_tsof0, empty or naming no message posts it top level as athread_broadcast. The broadcast carries itsrootin thechat.postMessageandchat.updateechoes,conversations.history,conversations.replies,reactions.getandreactions.list, and noteam,parent_user_idoredited.Closes #1150
Why this is what the real API does
Live calls to slack.com on 2026-10-30 (bot token, public channel), same steps on Backlot:
rootin echo,chat.update, history, replieseditedthread_ts0/ empty / missing +reply_broadcastthread_broadcast, noteamthread_ts+reply_broadcastreply_count2→1reactions.getandreactions.listfollow the review's measurement; the test app has noreactionsscope.Verification
Tests —
pyteston auv sync --all-extrasinstall: 8819 passed, 2 skipped.Optional surfaces that ran rather than skipped — SDK, MCP, llamaindex, fsspec and mirage tests ran. Skipped: HubSpot reader, Atlassian MCP (no Docker).
Lint —
ruff check . && ruff format --check .: clean.PLW1514andscripts/prose_check.pyclean.A test fails without this change:
test_a_broadcast_reply_is_listed_in_history,test_a_broadcast_reply_carries_its_subtype_and_rootandtest_a_thread_ts_naming_nothing_posts_top_level_and_an_unreadable_one_is_refusedfail onmain.A new endpoint serving corpus content is ACL-scoped: no new endpoint, and
rootis read with the caller's visible ids.Comments state what was measured, not the history of the fix