Repository navigation
emrg: an @ inside a feishu thread wakes the task and is read - #2624
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ 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_hostand
::test_a_thread_reply_is_de_duplicated_by_its_own_idred (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_nothingred (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
seencheck 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_fileboth 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
left a comment
There was a problem hiding this comment.
✅ 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
left a comment
There was a problem hiding this comment.
✅ 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_hostand
test_a_thread_reply_is_de_duplicated_by_its_own_idred (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.
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 withim +chat-messages-listand iterated_message_items(reply), applying_needs_the_hostonce per entry. That command answers a thread as its parent carrying the replies underthread_replies, so a reply's @ was never an entry the loop saw. Nothing inemrg/read the key at all: on master28c822b7,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 ablockeditem. 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_hosttest 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'sthread_repliesare 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.pygains three cases built on the file's existing injected-runner fixture (the_messagehelper now takesreplies, attached rather than listed beside the parent, because that is how the listing answers a thread):tests/test_feishu_prompt.pygains 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:
Restored: 33 passed in both files;
test_feishu_gate.py,test_scheduler.py,test_prompt_templates.py,test_boss_tool.pyandtest_readme_matches_code.pytogether with them: 321 passed.CI
CI pending — this branch's checks have not concluded at the time of opening.