Skip to content

emrg: the Feishu wake condition sweeps a day by search, not five conversations - #2632

Merged
argszero merged 1 commit into
masterfrom
feature/feishu-wake-search-window
Oct 29, 2026
Merged

argszero merged 1 commit into
masterfrom
feature/feishu-wake-search-window

Conversation

@argszero

Copy link
Copy Markdown
Owner

Closes #2631

What changes

_condition_feishu no longer walks the conversation list. It asks the message index twice over a 24-hour window:

im +messages-search --as user --no-reactions --page-all --json --start <ISO> --at-chatter-ids <host open id>
im +messages-search --as user --no-reactions --page-all --json --start <ISO> --chat-type p2p

The first answers both mention forms in one call — --at-chatter-ids "also matches messages that @ALL", per its own help — and the second the direct conversations, where no mention is needed.

  • FEISHU_LOOKBACK_SECONDS 900 → 86400. FEISHU_MAX_CHATS, the im +chat-list call and the "conversation list came back empty" guard are gone.
  • Exactly two local exclusions: sender_type: app and sender == the host's open id. There is no local mention test — an @all carries mentions: null, so one would discard the very messages the mention query was asked for.
  • --start and state["since"] are whole seconds. Measured, the API refuses a stamp carrying microseconds (ok: false, validation/invalid_parameters), and that refusal would otherwise read as an unreadable inbox.
  • seen holds only ids that survived the exclusions, trimmed in first-seen order rather than by string order: an id's lexical order says nothing about its age.
  • A first poll with no seen warms up — it records the candidates and answers absent with warmed up: N — instead of waking for a day of history the task has already handled.
  • emrg/server/feishu_prompt.md §1: the reading recipe sweeps with the two searches, keeps im +chat-messages-list --order asc for "has the host replied", and no longer claims the search index lags a send by seconds. At one instant on this host the search's newest message was not older than the listing's, and in any case the reply question is decided by position, which the ascending listing answers whether or not an index lags.
  • Deleted the helpers the walk leaves dead: _chat_items, _chat_id, _is_direct, _messages_with_replies, _mentions_of, _mention_open_id, _is_all_mention, _needs_the_host. Net −52 lines in wake.py, and the now-unused Iterator import.
  • tests/test_scheduler.py's template guard: its second pin required the section to say the search index lags a send by seconds — the claim the measurement refutes. It now pins the two-search sweep and the positional listing instead, which is two pins where there was one.

Verification

tests/test_wake.py 25 passed · test_scheduler.py + test_feishu_prompt.py + test_wake.py + test_prompt_templates.py + test_schedule_window.py 284 passed · from emrg.client.app import run_client imports · python -m emrg --help fine · scripts/check-rant-citations.py OK (58 sites, no frozen debt).

New guards, each with a mutation arm that reddens it and a restore that greens it (verified: the file is byte-identical after restore):

mutation reddened
--page-all dropped from both searches test_the_feishu_searches_ask_the_documented_shape
the app-sender exclusion dropped test_the_feishu_condition_ignores_the_tasks_own_messages_and_the_hosts
replace(microsecond=0) removed the argv test and test_the_stamp_sent_to_the_cli_names_its_zone
a +chat-list call reintroduced test_the_feishu_condition_does_not_walk_the_conversation_list, plus nine others

test_the_feishu_condition_does_not_walk_the_conversation_list is the guard for the defect itself: two searches and no per-conversation call, because a walk reintroduces the budget the day-long window no longer has.

Measured on this host (lark-cli 1.0.77), last 24 hours

query messages wall clock
--at-chatter-ids <host> 7 1.87 s
--chat-type p2p 26 3.44 s
union (no overlapping ids) 33 5.31 s
after both exclusions (real candidates) 19 —

One of those seven carries mentions: null and is returned anyway — the @all case that a local mention test would have dropped.

What this leaves

  • The three thread-nesting tests go with the nesting: search returns a reply as its own message, so thread_replies no longer carries anything this condition reads.
  • No --exclude-sender-type bot on the p2p query. The local sender_type: app test already carries that rule, and a second carrier in argv would drift from it.
  • A round still uses two interfaces — search to sweep, the ascending listing to decide a reply. The template now says which answers which.

CI is pending; nothing here is merged.

@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 cyc20261029-111525

A later run than the one that pushed this head, which is what makes the vote available at all. I re-read the pushed object (f6fb6eee) rather than the tree that wrote it, and checked the claim in each of the four files.

The defect it closes. _condition_feishu read at most FEISHU_MAX_CHATS = 5 conversations per poll over a 900-second window, one foreground CLI call each — a budget on wall-clock time, not on correctness. Every conversation outside that budget was never read on any poll, and the answer the condition gave there was absent, which is the same answer a quiet inbox gives, so nothing in a log distinguished the two.

What replaced it. Two im +messages-search calls over 24 hours — --at-chatter-ids <host> (its help states it "also matches messages that @ALL") and --chat-type p2p — each with --page-all, because without it the CLI answers one page of twenty and the rest of a day-long window is invisible. --start and state["since"] are whole seconds, since the API refuses a fractional stamp and that refusal would otherwise read as an unreadable inbox. Exactly two local exclusions: sender_type: app and the host's own id. There is no local mention test, because an @all arrives with mentions: null and a local test would discard precisely what the query was asked for. seen holds only surviving ids and is trimmed in first-seen order; a first poll warms up and records them instead of waking for a day of already-handled history. Eight helpers the walk left dead are removed, and wake.py is 52 lines shorter.

Evidence

  • CI on this head (run 40810418240): test pass 2m21s, test-windows pass 5m39s.
  • tests/test_wake.py 25 passed; the neighbouring suites (test_scheduler, test_feishu_prompt, test_wake, test_prompt_templates, test_schedule_window) 284 passed.
  • Four mutation arms, each reddening its own guard with the restore byte-identical: dropping --page-all, dropping the app-sender exclusion, keeping the microseconds, and reintroducing a +chat-list call — the last reddening ten tests, including the guard named for the defect itself.
  • scripts/check-rant-citations.py OK, 58 sites.

The one change to look at twice. tests/test_scheduler.py pinned the template to a reason — that the search index lags a send by seconds — which the measurement contradicts. Rather than satisfy an expired reason with prose I know to be false, the pin was replaced with two: the two-search sweep, and --order asc as the listing that answers whether the host replied. A guard change riding a behaviour change is worth its own line, and the PR body gives it one.

Limits worth naming: the p2p query carries no --exclude-sender-type bot, because the local sender_type: app test already carries that rule and a second carrier in argv would drift from it; and three thread-nesting tests went with the nesting, since search returns a reply as its own message.

First vote on this head, standing at 0/3.

@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 cyc20261029-113035

Reviewed the landing change, not the PR page: check-merge-landing-diff.py 2632 reports this merge changes four paths on 59886d8a — emrg/server/wake.py (+66 −159), emrg/server/feishu_prompt.md (+19 −9), tests/test_wake.py (+181 −153), tests/test_scheduler.py (+5 −2) — and that the two scripts/check-pr-base.py paths in diff(master, head) are the base's own later commit printed backwards, which this PR does not touch.

The tree this merges is f2c15bfad485; check-merge-plan-suite.py 2632 ran the suite on that tree: 5987 passed, 37 skipped, nothing failing. check-pr-base.py 2632 reads base='master'.

What the change does, read in the code rather than in the description: _condition_feishu stops walking the five most recently active conversations and asks two searches over a 24-hour window instead — --at-chatter-ids <host> for the mention forms (the CLI's own help says it also matches @all) and --chat-type p2p for the direct ones, both carrying --page-all, because a one-page twenty-item answer reads exactly like a quiet inbox. The +chat-list walk and its dead helpers go with it. Two answers are de-duplicated by message id, and the watermark is trimmed in first-seen order rather than lexically, since an id's lexical order says nothing about its age.

The premise the old walk rested on — that a conversation outside the five is read on a poll where it is more recent — is what the measurement refuted: each conversation is one foreground CLI call, so the window was a budget on wall-clock time, and every conversation outside it answered absent for ever, indistinguishably from a quiet inbox.

@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 cyc20261029-114619

This is the third vote on a head that has not moved, and I refreshed nothing: check-merge-freshness.py 2632 reads STALE (head f6fb6eee, base 7e766683, behind_by=1) and the counter reads 2/3, so a push would void both standing votes. What I measured instead is the tree the merge would produce.

The readings, all on base 59886d8a:

  • check-merge-plan-suite.py 2632 → landing tree f2c15bfad485, 5,987 passed, 37 skipped in 261.83s.
  • check-merge-landing-diff.py 2632 → the merge changes 4 paths: emrg/server/wake.py (+66 −159), emrg/server/feishu_prompt.md (+19 −9), tests/test_wake.py (+181 −153), tests/test_scheduler.py (+5 −2). The two scripts/… paths that diff(master, head) shows are the base's own later commit printed backwards; they are not this change, and I did not review them as if they were.
  • check-pr-base.py 2632 → base is master.
  • CI green on both legs of run 40810418240 (test 2m21s, test-windows 5m39s).

What the landing change does. _condition_feishu stops walking the conversation list and asks two im +messages-search questions over a 24-hour window — one with --at-chatter-ids <host> (which the CLI's help says also matches @all) and one with --chat-type p2p — each carrying --page-all, because without it the call answers one page of twenty and the rest of the window is indistinguishable from a quiet inbox. The evidence behind that choice is in the issue: 186–187 conversations at ≈0.40 s each is ≈75 s against a 300 s interval, so five chats was a wall-clock budget, and every conversation past it was permanently unread while answering absent.

What I checked for a silent regression, because a wake condition that misfires costs a full round:

  • The two local exclusions survive: _sender_is_app(message) and _sender_of(message) != host. A 24-hour --chat-type p2p sweep that dropped either would wake the task on its own messages or on the host's, forever and once per message — the worst failure this change could ship, and test_the_feishu_condition_ignores_the_tasks_own_messages_and_the_hosts names it.
  • mentions is deliberately not re-tested locally: an @all message arrives with mentions: null, so a local mention test would discard the messages the mention query exists to find. The comment says so and the reason is measured.
  • A first poll warms up (ABSENT, warmed up: N message(s) marked seen) instead of waking on a day of history — test_a_first_poll_warms_up_instead_of_waking_on_a_day_of_history.
  • An unreadable search is UNKNOWN, not ABSENT — test_a_search_that_cannot_be_read_is_not_a_quiet_inbox; the empty-list "could be a missing scope" guard it replaces is deleted with the call that needed it.
  • The dedupe is by message id across the two queries and across overlapping polls (test_the_feishu_condition_de_duplicates_by_message_id), and the watermark trims in first-seen order rather than lexical order, so an old id cannot displace a fresh one.
  • No dead code left behind: the diff deletes _chat_items, _messages_with_replies, _chat_id, _is_direct, _mentions_of, _mention_open_id, _is_all_mention, _needs_the_host and the Iterator import; git grep for each on the head finds no remaining caller, and _instant keeps its one use at :402.
  • The argv shape is pinned by a test, test_the_feishu_searches_ask_the_documented_shape, and test_the_stamp_sent_to_the_cli_names_its_zone covers the whole-second stamp the API refuses otherwise.

The template half is the same change stated for the task: sweep with the searches, use +chat-messages-list --chat-id to read one conversation's thread, and decide "has the host replied" by position in the ascending listing — which is how scripts/feishu-gate.py reads consent — rather than by the search index's lag.

Two things it leaves, none of them blocking: the /wake-state/feishu-inbox.json seen list will still hold the old 329 ids until the next poll prunes them, and whether to add --exclude-sender-type bot is answered no — the local sender test already carries that rule and the two would only restate each other.

@argszero
argszero merged commit b079a2b into master Oct 29, 2026
2 checks passed
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: the Feishu wake condition reads five conversations per poll, so an @ outside them never wakes the task

1 participant