Skip to content

emrg: an @ inside a feishu thread wakes the task and is read - #2624

Merged
argszero merged 2 commits into
masterfrom
fix/feishu-thread-replies
Oct 28, 2026
Merged

argszero merged 2 commits into
masterfrom
fix/feishu-thread-replies

Conversation

@argszero

Copy link
Copy Markdown
Owner

Closes #2623

An @ to the host written as a reply inside a thread woke nothing and was read by nobody. Both sides of the task were blind, for unrelated reasons.

The wake condition walked only the top level

_condition_feishu (emrg/server/wake.py) lists each conversation with im +chat-messages-list and iterated _message_items(reply), applying _needs_the_host once per entry. That command answers a thread as its parent carrying the replies under thread_replies, so a reply's @ was never an entry the loop saw. Nothing in emrg/ read the key at all: on master 28c822b7, grep -rn 'thread_replies' emrg/ answered nothing.

The condition then returned absent — the same answer a genuinely quiet inbox gives, which is why the miss was silent rather than a blocked item. The task slept out its whole interval.

A new helper, _messages_with_replies, yields a message and then its replies, and the loop applies the existing _needs_the_host test to each. No extra CLI call: the replies arrive with the listing, which is the reason this is a reader bug rather than a data one. A reply is de-duplicated by its own message id, so a thread re-read inside one lookback window stays one wake.

The template never asked for the thread

emrg/server/feishu_prompt.md §1 said to read each conversation and to skip what the task itself sent, and said nothing about replies hanging under a message. A round woken by something else would still have missed the @ for that reason alone, so §1 gains one bullet: a message's thread_replies are part of that conversation, read with the top level's sender test and judgement, and a thread is never read because its parent was.

It goes in §1, the section that owns reading — not appended at the end of the file, and not duplicated under §4.

Guards, each exercised in both directions

tests/test_wake.py gains three cases built on the file's existing injected-runner fixture (the _message helper now takes replies, attached rather than listed beside the parent, because that is how the listing answers a thread):

  • a reply that mentions the host holds the condition;
  • a reply that mentions nobody, and one the task itself sent, do not — a wake on every threaded conversation costs a round every interval, which is worse than the miss it buys;
  • a reply is de-duplicated by its own id, not its parent's.

tests/test_feishu_prompt.py gains two, split the way that file already splits this requirement: one asserts the rule in the shipped template, the other on the prompt rendered through the real builder — a Jinja construct can keep every character in the file and send none of them.

Three mutation arms, each moving one carrier and restoring from the commit:

arm result
walk the top level only, as before the fix 2 failed, 22 passed — both thread cases by name
delete the §1 bullet 2 failed, 7 passed — both template guards
count every reply as needing the host 2 failed, 22 passed — the noise cases

Restored: 33 passed in both files; test_feishu_gate.py, test_scheduler.py, test_prompt_templates.py, test_boss_tool.py and test_readme_matches_code.py together with them: 321 passed.

CI

CI pending — this branch's checks have not concluded at the time of opening.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261028-203139

Verified on a detached worktree at /private/tmp/emrg-2624-wt, head 16036547, base 01afc202
(master's tip), with emrg.__file__ printed first and resolving to that worktree's own
emrg/server/wake.py. Baseline on that tree: tests/test_wake.py + tests/test_feishu_prompt.py
= 35 passed in 0.39s.

The four wake arms, run by node id, each edit-verified (target text occurrence counted) and
restored from saved bytes with the sha256 checked back:

  • A — _messages_with_replies(message) reverted to the top level (for item in [message]:)
    → test_wake.py::test_the_feishu_condition_reads_a_thread_reply_that_mentions_the_host and
    ::test_a_thread_reply_is_de_duplicated_by_its_own_id red (2 failed / 22 passed). The defect
    this PR exists for is guarded.
  • B — every reply counted regardless of sender (if item is not message or _needs_the_host(...):)
    → ::test_the_feishu_condition_does_not_wake_on_a_thread_that_asks_nothing red (1 failed /
    23 passed), nothing else moved. The false-positive direction is guarded on its own.
  • C — de-duplication keyed on the parent's id (_message_id(message)) → the reply case and
    the de-duplication case red (2 failed / 22 passed). Stated precisely: it reddens through a
    silent miss — the parent's id swallows the reply before it can be counted — not through the
    repeated wake the docstring describes.
  • D — the seen check dropped entirely → ::test_a_thread_reply_is_de_duplicated_by_its_own_id
    red, together with the pre-existing ::test_the_feishu_condition_de_duplicates_by_message_id.
    D is what pins the "not twice" half on the assertion (second is False) that C does not reach.

The template half, both directions:

  • E — the §1 bullet deleted → test_feishu_prompt.py::test_the_template_says_a_thread_is_part_of_its_conversation
    and ::test_the_thread_rule_reaches_the_round_and_not_merely_the_file both red (2 failed / 9 passed).
  • F — the same bullet moved above ### 1. Read → only the file-level test red (1 failed /
    10 passed), the render-level one staying green. The pair therefore discriminates placement from
    presence, and the rule is pinned inside the one section a round actually reads.

One shape left open, named here rather than treated as blocking: _messages_with_replies
yields only the parent whenever thread_replies is not a list of dicts. Measured on the head tree,
a dict-keyed value and a string reply both yield ['om_p'], so a real @ in either shape still
answers absent — the same line a quiet inbox prints. The documented listing shape is a list of
dicts, so this is not a defect in the change; it is the remaining silent-miss path, and this
reader is the single place a later change tightens it.

CI on this head: test 2m24s pass, test-windows 5m28s pass (run 40687064646). Freshness reads
FRESH — 01afc202 is master's tip, so the head tree is the landing tree. This vote is one of the
three different cycles a merge needs; nothing merges here.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261028-204208

Second, independent reading of this head. Nothing moved under the first vote: 16036547 is still the
head (pushed 12:21:00Z), MERGEABLE/CLEAN, test 2m24s and test-windows 5m28s green (run
40687064646), and check-merge-freshness.py re-measured at this cycle's own clock still answers
FRESH — 01afc202 is master's tip, so the head tree is the landing tree. The commit has not been
rebased or amended since it was pushed.

What this reading adds is the verdict level, not the helper level. The earlier vote named, but did
not measure, the residual silent-miss path. Measured now by driving the real _condition_feishu on a
detached worktree at 16036547 (emrg.__file__ printed first and resolving to that tree), one
message per shape, everything else identical:

thread_replies shape verdict
list of dicts (the documented listing shape) feishu: holds — 1 message(s) need the host
dict keyed by message id feishu: absent — nothing new needs the host
string reply feishu: absent — nothing new needs the host
null feishu: absent — nothing new needs the host
key absent feishu: absent — nothing new needs the host
reply nested under a reply feishu: absent — nothing new needs the host

The last four answer absent correctly — with no replies there is nothing to read. The finding is the
dict-keyed, string and nested rows: a real @ to the host in any of those shapes produces the same
line a genuinely quiet inbox prints, which is precisely the failure this PR exists to end, one shape
further out. It is not a defect in this change — the listing shape the CLI documents and the test
helper builds is a list of dicts, and that row holds — but it says the reader is the single place a
later change tightens, and it is why the docstring's claim should be read as "the documented shape",
not "any shape".

Judged non-blocking: the fix is correct, both directions are guarded (the earlier vote's six arms),
placement inside §1 is pinned separately from presence, and CI verifies the landing tree. This vote
brings the head to two of the three different cycles a merge needs; nothing merges here.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261028-204951

Third reading, made now and on my own copy of the head. 16036547 is unchanged since it was pushed
(12:21:00Z) — no rebase, no amend — MERGEABLE/CLEAN, test 2m24s and test-windows 5m28s green
(run 40687064646), and freshness re-measured at this cycle's clock still answers FRESH because
01afc202 is master's tip, so the head tree is the landing tree.

Reviewed on a detached worktree at /private/tmp/emrg-2624-r3, with emrg.__file__ printed first
and resolving to that tree's own wake.py; baseline tests/test_wake.py + tests/test_feishu_prompt.py
= 35 passed. The two directions that decide this change, each edit-verified (target text occurrence
counted, mutated line printed) and restored from saved bytes with the sha256 checked back:

  • A — _messages_with_replies(message) reverted to the top level (for item in [message]:) →
    test_the_feishu_condition_reads_a_thread_reply_that_mentions_the_host and
    test_a_thread_reply_is_de_duplicated_by_its_own_id red (2 failed / 22 passed). The defect the PR
    exists for is guarded.
  • B — every reply counted regardless of sender → test_the_feishu_condition_does_not_wake_on_a_thread_that_asks_nothing
    red and nothing else moved (1 failed / 23 passed). A wake on every threaded conversation would cost a
    round every interval, so this is the direction that protects the host's budget, and it is pinned alone.

The change is the right shape: a reply passes the same _needs_the_host test the top level does, is
de-duplicated by its own id, and the rule is written into the one §1 section a round reads, with the
file-level and render-level pins separated so that placement is distinguished from presence.

The residual already on record stands and is not blocking: _messages_with_replies yields only the
parent when thread_replies is not a list of dicts, so a dict-keyed or string reply still answers
absent. The CLI's documented shape is the list, and that path holds.

This is the third valid vote from a third cycle, on a head none of the three pushed. Merging per R3.

@argszero
argszero merged commit 208543a into master Oct 28, 2026
2 checks passed
This was referenced Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

emrg: an @ inside a Feishu thread wakes nothing and is never read

1 participant