feat: add background deep topic hooks#1651
Open
wuyehan wants to merge 5 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough新增“低频主题钩子”系统：从慢速全局证据收集、LLM 候选生成、结构化素材构建与在线富化，到后台 TopicHookPool 的调度与静默窗口触发，最后通过 topic_delivery 的 session-manager 注入把回调入队并调用现有 agent callbacks 投递喵。 ChangesTopic Hooks - 主动聊天增强系统
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fea44f46fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
main_logic/core.py (2)
5999-6112:⚠️ Potential issue | 🟠 Major | ⚡ Quick win这里有两处队列操作会把 callback 静默吃掉喵。
Line 6000-6003 先把
callbacks_snapshot从pending_agent_callbacks里摘掉了，但 Line 6020 这个“keeping for later”分支直接return False，没有放回队列；只要当时没有可用的 text session / websocket，这批 callback 就会被悄悄丢掉喵。然后 Line 6108 又在文本投递成功后clear()整个pending_extra_replies，这会把与本次 snapshot 无关、甚至prompt_ephemeral()await 期间新入队的 voice hot-swap extra 一起清掉喵。🐾 可直接改的小补丁喵
self.pending_agent_callbacks = [ cb for cb in self.pending_agent_callbacks if id(cb) not in snapshot_ids ] @@ if isinstance(self.session, OmniOfflineClient): delivered = await self._deliver_agent_callbacks_text(instruction, callbacks_snapshot) logger.debug("[%s] trigger_agent_callbacks: auto text session delivered", self.lanlan_name) else: + self.pending_agent_callbacks.extend(callbacks_snapshot) logger.debug("[%s] trigger_agent_callbacks: no websocket/session, keeping for later", self.lanlan_name) @@ if delivered: - self.pending_extra_replies.clear() return TrueBased on learnings:
pending_agent_callbacks和pending_extra_replies必须保持独立队列，文本模式成功投递时不要消费pending_extra_replies；另外 snapshot-prune 之后失败/延后路径必须把本次 snapshot 放回去喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main_logic/core.py` around lines 5999 - 6112, The current flow in trigger_agent_callbacks and _deliver_agent_callbacks_text removes callbacks_snapshot from pending_agent_callbacks early and also clears pending_extra_replies on text success, causing silent loss of callbacks and unrelated voice extras; revert this by (1) when trigger_agent_callbacks removes the snapshot (the snapshot_ids/prune logic around pending_agent_callbacks and callbacks_snapshot), ensure any early non-delivery path (e.g., the "no websocket/session, keeping for later" branch and any caught exceptions) re-appends callbacks_snapshot back into pending_agent_callbacks before returning/ending, and (2) in _deliver_agent_callbacks_text remove the pending_extra_replies.clear() on successful text delivery (or limit its effect to only items originating from this snapshot) so text-mode prompt_ephemeral success does not wipe unrelated voice-mode pending_extra_replies; touch the functions trigger_agent_callbacks and _deliver_agent_callbacks_text and the variables pending_agent_callbacks, callbacks_snapshot, pending_extra_replies, and prompt_ephemeral to implement these fixes.
5877-5961:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift别把 voice 注入分支的
True当成“已经成功送达”喵。
inject_text_and_request_response()的拒绝是靠on_rejected异步回来的。现在 Line 5961 只要 await 正常返回就会return True，但 reject 完全可能在这之后才到；如果新的 topic-hook 调用方拿这个布尔值去记“成功触发次数 / 日额度”，就会把一次实际失败的投递记成成功，后面即使回调被重新入队也纠不回来了喵。这里要么把 success accounting 挪到更晚的 accepted/commit 信号，要么让 voice 分支不要承诺同步成功语义喵。Based on learnings:
OmniRealtimeClient.inject_text_and_request_response()的 accepted/rejected 合同是异步通过on_rejected回传的，不是 await 返回值喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main_logic/core.py` around lines 5877 - 5961, The code incorrectly treats a normal return from voice_sess.inject_text_and_request_response(...) as a synchronous success; because OmniRealtimeClient uses async on_rejected/_on_voice_inject_rejected to signal actual accept/reject, do not return True or perform success accounting in trigger_agent_callbacks immediately after that await. Instead either (A) move the queue-pruning and delivered_ids accounting into the acceptance/commit callback that runs when the provider confirms delivery, or (B) change the voice branch to return a non-committal value (e.g., False/None) and keep pending_agent_callbacks/pending_extra_replies intact until _on_voice_inject_rejected confirms acceptance; update trigger_agent_callbacks, inject_text_and_request_response usage, and _on_voice_inject_rejected logic accordingly so no delivery is counted as successful before the async accept/commit signal arrives.tests/unit/test_topic_delivery.py (1)
53-159:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win建议用
autousefixture 兜底清理全局 getter，避免失败用例污染后续测试喵。现在每个测试在末尾手动
clear_topic_session_manager_getter()，但若中途断言失败，清理不会执行，后续用例可能出现顺序相关失败（flaky）喵。可直接套用的小改动喵
+@pytest.fixture(autouse=True) +def _topic_session_manager_getter_guard(): + clear_topic_session_manager_getter() + yield + clear_topic_session_manager_getter() + `@pytest.mark.asyncio` async def test_trigger_topic_hook_once_enqueues_existing_manager_callback(monkeypatch): @@ - clear_topic_session_manager_getter() + # cleanup 由 autouse fixture 统一处理🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_topic_delivery.py` around lines 53 - 159, Tests manually call clear_topic_session_manager_getter() at the end of each test but can leak state if a test fails; add a pytest fixture with autouse=True (e.g., in tests/unit/conftest.py or at top of this test module) that calls clear_topic_session_manager_getter() before and after each test to guarantee cleanup, and remove or keep the manual clear calls as desired; reference the existing functions clear_topic_session_manager_getter and register_topic_session_manager_getter and ensure the fixture yields to run the test between pre/post cleanup.
🧹 Nitpick comments (1)
main_logic/topic_delivery.py (1)
46-60: ⚡ Quick win
detail文案建议按lang模板化，避免非中文回合混入中文指令当前指令文本是全中文，
lang只放进 metadata；在 en/ja/ko/es/pt/ru 会话里会把中文约束直接注入回合上下文，降低语言一致性稳定度喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main_logic/topic_delivery.py` around lines 46 - 60, The current construction of detail/detail_parts embeds a Chinese instruction string directly, causing non-Chinese sessions to inherit Chinese constraints; refactor by templating the prompt per language (e.g., create a dict templates keyed by lang with entries for "zh","en","ja","ko","es","pt","ru") and build detail from templates[lang] + the language-specific fragments (interest, hook, opening, deepening, why_now, hint_summary, online_angle) so that the top-level instruction uses the correct localized phrasing; update the code that assembles detail (symbols: detail_parts and detail) to select templates.get(lang, templates["zh"]) and format/concatenate only the chosen language strings, keeping the existing conditional inclusion logic for each fragment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@main_logic/topic_hooks.py`:
- Around line 98-113: The current construction of parts in topic_materials uses
hard-coded Chinese labels and extra sentence fragments (see variables parts,
interest, hook, opening, deepening, hint_summary, online_angle, hint_links,
text), causing mixed-language prompts when lang == 'en'; fix by introducing a
small language mapping (e.g., a dict keyed by lang) for each label and for the
online_angle extra sentence and use those mapped strings when appending to parts
instead of the Chinese literals, ensure the hint_links label and any punctuation
are localized too, and apply the same localized-label replacement for the
analogous block referenced at lines 136-153.
In `@main_logic/topic_materials.py`:
- Line 284: The current assignment to available_fetchers uses a truthy check
that treats an explicit empty dict as "no fetchers" and falls back to await
_default_fetchers(lang); change it to only fall back when fetchers is None by
using a conditional that checks fetchers is not None (e.g., set
available_fetchers from fetchers when fetchers is not None, otherwise await
_default_fetchers(lang)), so the explicit empty {} passed by callers is
preserved and default network fetchers are not unintentionally enabled.
In `@main_logic/topic_pipeline.py`:
- Around line 348-356: In _run_later, wrap the await self.process_now(name) call
in a try/except so any exception from process_now does not skip the cleanup and
reschedule logic; on exception, log the error (or record it) and continue to the
existing checks that pop self._tasks[name] and call self._schedule(name) so
_dirty is not left unprocessed and automatic reordering continues; update
references in this function (_run_later, process_now, _dirty, _tasks, _schedule)
accordingly.
- Around line 88-98: The dict construction is vulnerable: media_intent can be a
string (which will become a list of characters) and created_at can be
non-numeric (raising on float()). Update the normalization before putting values
into the dict: for media_intent, coerce material.get("media_intent") into a list
of strings — if it's None use ["news"], if it's a string wrap it as
[that_string], if it's already a list filter/ensure elements are strings and
then slice [:2]; for created_at, attempt to coerce material.get("created_at") to
float inside a try/except (catch ValueError/TypeError) and fall back to
time.time() on failure. Apply these fixes where the dict is built (references:
media_intent, created_at, material.get).
In `@main_logic/topic_signals.py`:
- Around line 172-176: Clamp max_lines to a non-negative value before using it
for slicing: ensure max_lines = max(0, max_lines) (or early return [] when
max_lines <= 0) so the subsequent logic with all_turns, head_count and
tail_count cannot produce negative slices; then compute head_count = min(12,
max_lines // 2) and tail_count = max_lines - head_count and proceed with the
existing slicing using all_turns, head_count and tail_count.
In `@main_routers/system_router.py`:
- Around line 6271-6292: The code currently can reintroduce open_threads into
phase2_memory_context via followup_topics_prompt even when
restricted_screen_only should prevent non-screen threads; update the logic
around building/appending refreshed_topic_hook_prompt (variables/functions:
restricted_screen_only, open_threads_for_topic_hooks,
refreshed_topic_hook_prompt, followup_topics_prompt, phase2_memory_context,
_allow_reminiscence, _followup_topics, display_snap) so that when
restricted_screen_only is true you either pass open_threads=None into
build_topic_hook_prompt or strip any open-thread content out of
refreshed_topic_hook_prompt before assigning followup_topics_prompt; ensure the
combined phase2_memory_context never includes open_threads in
restricted_screen_only rounds.
---
Outside diff comments:
In `@main_logic/core.py`:
- Around line 5999-6112: The current flow in trigger_agent_callbacks and
_deliver_agent_callbacks_text removes callbacks_snapshot from
pending_agent_callbacks early and also clears pending_extra_replies on text
success, causing silent loss of callbacks and unrelated voice extras; revert
this by (1) when trigger_agent_callbacks removes the snapshot (the
snapshot_ids/prune logic around pending_agent_callbacks and callbacks_snapshot),
ensure any early non-delivery path (e.g., the "no websocket/session, keeping for
later" branch and any caught exceptions) re-appends callbacks_snapshot back into
pending_agent_callbacks before returning/ending, and (2) in
_deliver_agent_callbacks_text remove the pending_extra_replies.clear() on
successful text delivery (or limit its effect to only items originating from
this snapshot) so text-mode prompt_ephemeral success does not wipe unrelated
voice-mode pending_extra_replies; touch the functions trigger_agent_callbacks
and _deliver_agent_callbacks_text and the variables pending_agent_callbacks,
callbacks_snapshot, pending_extra_replies, and prompt_ephemeral to implement
these fixes.
- Around line 5877-5961: The code incorrectly treats a normal return from
voice_sess.inject_text_and_request_response(...) as a synchronous success;
because OmniRealtimeClient uses async on_rejected/_on_voice_inject_rejected to
signal actual accept/reject, do not return True or perform success accounting in
trigger_agent_callbacks immediately after that await. Instead either (A) move
the queue-pruning and delivered_ids accounting into the acceptance/commit
callback that runs when the provider confirms delivery, or (B) change the voice
branch to return a non-committal value (e.g., False/None) and keep
pending_agent_callbacks/pending_extra_replies intact until
_on_voice_inject_rejected confirms acceptance; update trigger_agent_callbacks,
inject_text_and_request_response usage, and _on_voice_inject_rejected logic
accordingly so no delivery is counted as successful before the async
accept/commit signal arrives.
In `@tests/unit/test_topic_delivery.py`:
- Around line 53-159: Tests manually call clear_topic_session_manager_getter()
at the end of each test but can leak state if a test fails; add a pytest fixture
with autouse=True (e.g., in tests/unit/conftest.py or at top of this test
module) that calls clear_topic_session_manager_getter() before and after each
test to guarantee cleanup, and remove or keep the manual clear calls as desired;
reference the existing functions clear_topic_session_manager_getter and
register_topic_session_manager_getter and ensure the fixture yields to run the
test between pre/post cleanup.
---
Nitpick comments:
In `@main_logic/topic_delivery.py`:
- Around line 46-60: The current construction of detail/detail_parts embeds a
Chinese instruction string directly, causing non-Chinese sessions to inherit
Chinese constraints; refactor by templating the prompt per language (e.g.,
create a dict templates keyed by lang with entries for
"zh","en","ja","ko","es","pt","ru") and build detail from templates[lang] + the
language-specific fragments (interest, hook, opening, deepening, why_now,
hint_summary, online_angle) so that the top-level instruction uses the correct
localized phrasing; update the code that assembles detail (symbols: detail_parts
and detail) to select templates.get(lang, templates["zh"]) and
format/concatenate only the chosen language strings, keeping the existing
conditional inclusion logic for each fragment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c4de3fe-276a-47eb-bf37-e61c8497d414
📒 Files selected for processing (20)
app/main_server.pyconfig/prompts/prompts_activity.pymain_logic/activity/llm_enrichment.pymain_logic/activity/tracker.pymain_logic/core.pymain_logic/topic_delivery.pymain_logic/topic_hooks.pymain_logic/topic_materials.pymain_logic/topic_pipeline.pymain_logic/topic_signals.pymain_routers/system_router.pytests/test_activity_tracker_followup.pytests/unit/test_topic_delivery.pytests/unit/test_topic_hooks.pytests/unit/test_topic_llm_enrichment.pytests/unit/test_topic_materials.pytests/unit/test_topic_pipeline.pytests/unit/test_topic_signals.pyutils/meme_fetcher.pyutils/music_crawlers.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a13c31301
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8b45b860d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test Plan
uv run pytest tests/unit/test_topic_signals.py tests/unit/test_topic_pipeline.py tests/unit/test_topic_materials.py tests/unit/test_topic_delivery.py tests/unit/test_topic_hooks.py tests/unit/test_topic_llm_enrichment.py tests/test_activity_tracker_followup.py -q（121 passed）uv run pytest tests/unit/test_proactive_sm_integration.py tests/unit/test_proactive_delivery.py tests/unit/test_proactive_phase1_pass.py tests/unit/test_proactive_sid_guard.py tests/unit/test_callback_instruction_origin.py tests/unit/test_session_state.py tests/unit/test_session_start_guard.py -q（131 passed）uv run pytest tests/unit/test_meme_fetcher.py tests/unit/test_music_crawlers.py tests/integration/test_meme_proxy_security.py -q（15 passed, 2 skipped）uv run python -m py_compile config/prompts/prompts_activity.py main_logic/activity/llm_enrichment.py main_logic/activity/tracker.py main_logic/core.py main_logic/topic_materials.pygit diff --checkNotes
/api/agent/* 502、CSRF heartbeat warning 属于既有运行噪音，本 PR 未改这部分。Summary by CodeRabbit
新功能
改进
测试