refactor: long-file audit full split (awakening package + LLM reply chain) - #247
Merged
Merged
Conversation
- Replace silent except-pass on persona interest_topics with debug log (fail-soft kept) - Add missing second blank line after class definition (E305) - Drop dead user_id params from record_message/check_interest/check_fallback/check_relevance/check_qa; sync orchestrator, adapter and test callers
- Drive resolve_group by fields(ResolvedAwakeningSettings): single merge loop replaces two 10-line per-field enumerations - Converge both from_dict filters into _filter_config_fields helper - Derive routes _OVERRIDE_FIELDS from AwakeningSettingsBody.model_fields; document interest_topics Web-contract decision - Fold over-length lines in routes/awakening.py
- Define structural Protocols for the LLM surface awakening depends on (judge channel, persona topics, group status, rule switch, rate limiter, stats, generate callable); svc/Any annotations converge to narrow types - Replace _judge_target getattr probing with explicit JudgeTarget/JudgeSettings resolved via typed config view - Add LLMService.persona_interest_topics as the narrow persona-extras read owned by the persona domain - Promote _is_group_llm_enabled to public is_group_llm_enabled; scheduler and tests follow - Type BoredomSendPlan.reply_result via ReplyResult TypedDict (new llm/reply_types.py) with delivered_text extension - Add optional config injection param to check_awakening_triggers/iter_boredom_send_plans as the official test seam
- chat/awakening.py -> chat/awakening/ package: config/state/text_signals/judge/triggers/boredom with one-way deps (config <- state <- text_signals <- judge <- triggers <- boredom) and a facade re-exporting only the public contract - Facade attribute-assignment test seams migrated with the split: orchestrator tests use the config injection param; boredom tests patch owning submodule singletons (state._state) and pass config through - random patch point moves to quickquip.chat.awakening.triggers.random.random; group-messages harness patches owning config/state singletons - routes/awakening.py imports CONFIG_AWAKENING_TOML from common.paths directly; facade drops the pass-through re-export - All package files satisfy the <=100 col baseline
- Move collect_mention_profiles / collect_known_participants (plus AT_QQ_PATTERN and MENTION_PROFILE_LIMIT) into the llm identity domain as pure projections over IdentityIndex - LLMService keeps thin delegates resolving the scope identities, preserving the test seam and call sites - service.py sheds ~130 lines of envelope identity knowledge; identity.py grows from a re-export shim into the envelope orchestration owner
- New llm/reply_chain.py: TurnRequestAssembler (explicit assembly object replacing the 74-line _assemble_request closure with 6 nonlocals), normalize_turn_input, finalize_reply_text, reply_result factory, build_raw_turn_text (dedup of the twin raw_turn assembly in _begin_agent_recorder / _persist_turn_and_build_reply), image_caption_blob, LLM_RULE_NAME / MAX_QUOTED_MESSAGE_CHARS constants (re-exported by service for plugins/llm_runtime) - New ChatTurnRequest input struct (llm/reply_types.py) collapses the three-layer 24-param threading through generate_reply / generate_private_reply / _generate_reply_for_scope - _generate_reply_for_scope drops from 524 to ~395 lines of phase-annotated orchestration; per-path error dicts converge on the factory with key sets preserved exactly - Module patch points stay in service.py: moved code receives the resolved sensitive filter and bound callables as explicit parameters (single_shot pattern)
- llm-module.md: service.py entry notes the reply-chain extraction; new reply_chain.py entry; identity.py entry covers envelope orchestration; awakening module path updated to the package layout
- Drop dead TurnRequestAssembler.session_preset field (effect already baked into system_prompt) - Promote cross-submodule contract names in the awakening package to public (judge/text_signals/triggers exports consumed by sibling modules), leaving module-internal helpers private - Normalize judge target diagnostics: empty string instead of str(None) when default_provider is unset - Fix _filter_config_fields docstring to match actual passthrough behavior - Restore private names for identity.py pattern constants (no external consumers) - Remove AwakeningExtendSession from the facade (zero external consumers); docstring states the real export surface
![khpilot[bot]](https://avatars.githubusercontent.com/u/178400596?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bot Review — 💬 Comment
This PR refactors two large modules by splitting awakening.py into a 6-submodule package and extracting identity/reply-chain logic from llm/service.py, with behavior-preserving changes to internal structure only. The verdict is "comment" because the general reviewer's coverage is incomplete (19/20 units, 1 failed batch); an incomplete review cannot validly approve or request changes regardless of the standing findings. Notable unresolved concerns from the capped findings include: _parse_judge_text exception handling gaps that could break the fail-closed contract on malformed LLM JSON output (should-fix), deep-cr-trigger.sh path mapping not updated for the awakening package (should-fix), misaligned svc Protocol annotations in check_awakening_triggers, and delivery_sink typed as Any instead of reusing the existing DeliverySink Protocol.
Findings
🔴 BLOCKING: General reviewer coverage is incomplete (19/20 units, 1 failed batch). Per adjudication rules, an incomplete review cannot produce a valid approve or request_changes verdict.
File: coverage
Await completion of the failed batch and re-adjudicate.
Review provenance
- General reviewer: incomplete — 19/20 units completed, 1 failed batches
- Style reviewer: complete — 20/20 units completed
- Coverage: incomplete — manifest=complete; general=incomplete(1 failed batches); style=complete
- Reviewed head:
49130d9(base18181b4)
Automated review by @KHPilot. Reply with @KHPilot to ask follow-up questions.
- _parse_judge_text: fail-closed on non-numeric score values (string/null/nested) instead of raising through the judge chain; regression tests added - deep-cr-trigger.sh: map the awakening package paths (src/quickquip/chat/awakening*) to message-policy so the Deep-CR gate keeps covering the split modules - check_awakening_triggers: svc typed as AwakeningServiceView (judge channel + persona topics composite protocol) aligning orchestrator and per-rule narrow interfaces - ChatTurnRequest.delivery_sink typed via the existing DeliverySink Protocol instead of Any
Owner
Author
Bot Review 处置（head 5d9caec）评审由宕机前的入队 job 经队列恢复自动完成，感谢覆盖。逐条处置：
其中前两条在本地五透镜 Deep-CR 中未捕获（parse 缺口为拆分前既有、随代码迁移进入 diff；脚本映射为本 PR 直接引入），系 Bot Review 的有效增量发现。 |
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.
概要
按 2026-09-14 长文件审计快照的建议执行全流程拆分（单分支单 PR，Huge PR 流程）。行为保持型重构：命令、配置、持久化格式、交付行为与用户可见文案均不变；唯一可观察差异是 persona 兴趣话题读取失败时新增一条 debug 日志，以及两处日志记录器命名空间随模块迁移。
变更内容
唤醒模块拆包（
chat/awakening.py1188 行单文件 →chat/awakening/包）：user_idresolve_group改fields(ResolvedAwakeningSettings)驱动合并；两个from_dict收敛为_filter_config_fields；Web 路由_OVERRIDE_FIELDS改由AwakeningSettingsBody.model_fields派生（键集与旧硬编码逐项相同）svc: Any宽对象；_is_group_llm_enabled升公共名；BoredomSendPlan.reply_result经 TypedDict 定形；check_awakening_triggers/iter_boredom_send_plans新增可选config注入参数作为正式测试接缝；LLMService.persona_interest_topics承担 persona extras 窄读取LLM 回复主链瘦身（
llm/service.py1907 → 1681 行）：llm/identity.py（collect_mention_profiles/collect_known_participants，service 留薄委托）llm/reply_chain.py：TurnRequestAssembler显式装配对象替代 74 行闭包 + 6 个 nonlocal；normalize_turn_input/finalize_reply_text/reply_result工厂 /build_raw_turn_text（双份 raw_turn 拼装去重，字节级一致经差分验证）llm/reply_types.py：ChatTurnRequest/ReplyResult定形输入输出契约，三层 24 形参穿透收敛为两层；_generate_reply_for_scope524 → ~395 行分阶段编排plugins/llm_runtime.py12 个 re-export 名不变；quickquip.llm.service._get_sensitive_filter/build_provider_client/_reload_sensitive_filter模块 patch 点不变（被移代码以参数接收已解析过滤器与绑定可调用）文档：
docs/dev/llm-module.md同步新结构（reply_chain / identity 条目、awakening 包路径）。验证
.venv/bin/ruff check .全绿；validate_toml_examples.py12 文件通过pytest -n auto2088 全绿（含 re-export 契约测试、预算重试、分段交付、取消、敏感词、epoch/锚点、身份注入等集成路径）normalize_turn_input393k 组合与旧实现零偏差Deep-CR（Tier 2，五透镜）
scripts/check/deep-cr-trigger.sh origin/dev→trigger: true（20 文件 / +4556 行 / 跨 3 域）。五个独立透镜（provider/MCP、工具副作用与敏感词、持久化、触发与配置契约、全局结构）+ 独立交叉复核（≥80 置信度保留）：TurnRequestAssembler.session_preset死字段（效果已含于预计算 system_prompt）→ 删字段；② awakening 包内生产代码跨子模块 import 下划线私有名 → 跨模块消费的名字升公共，模块内部助手保持私有（AST 复核清零）default_provider=None时输出"None"→ 归一空串；_filter_config_fieldsdocstring 与实际透传行为不符 → 改写；llm/identity.py两个无消费常量放宽为公共 → 恢复私有；facade 冗余 re-export（AwakeningExtendSession）与 docstring 失实 → 移除并如实描述TurnRequestAssembler经结构透镜判定为显式装配对象而非新宽 bag（36 字段中 35 个被 assemble 消费、4 个可调用为刻意窄缝）CHANGELOG 草稿（release 时汇总）
变更
llm/identity.py，请求装配与产出 shaping 拆至llm/reply_chain.py/llm/reply_types.py；对外契约与文案不变