Skip to content

[fix] 修复聚合报告通知失败隔离 ( P5 ) - #1269

Merged
ZhuLinsen merged 1 commit into
ZhuLinsen:mainfrom
massif-01:fix/1200-p5-notification-failure-isolation
May 12, 2026
Merged

[fix] 修复聚合报告通知失败隔离 ( P5 )#1269
ZhuLinsen merged 1 commit into
ZhuLinsen:mainfrom
massif-01:fix/1200-p5-notification-failure-isolation

Conversation

@massif-01

@massif-01 massif-01 commented May 11, 2026

Copy link
Copy Markdown
Collaborator

PR Type

  • fix
  • feat
  • refactor
  • docs
  • chore
  • test

Background And Problem

Refs #1200

P0-P4 已合并后,P5 需要补齐通知失败隔离边界:聚合报告路径中,路由后的单一静态通知渠道如果抛异常,不应跳过后续渠道,也不应影响分析主流程。同时 Web 通知测试需要更明确展示 custom webhook 部分成功和 timeout/异常诊断。

Scope Of Change

  • src/core/pipeline.py:在聚合报告 _send_notifications() 路径内增加局部 safe-send helper,对现有静态通知渠道逐个隔离;邮件按 receiver group 单独隔离。
  • src/services/system_config_service.py:仅调整 custom webhook 测试顶层 message,明确成功数 / 总数。
  • tests/:补充渠道异常后续仍发送、全部静态渠道失败释放 noise、邮件分组隔离、custom webhook partial/all-failed、普通 sender timeout 分类与脱敏测试。
  • apps/dsa-web/:只补 NotificationTestPanel 组件测试,不改生产 UI。
  • docs/notifications.mddocs/CHANGELOG.md:同步 P5 行为说明和 changelog。

Issue Link

Refs #1200

Verification Commands And Results

python -m py_compile src/core/pipeline.py src/services/system_config_service.py
python -m pytest tests/test_pipeline_notification_image_routing.py tests/test_system_config_service.py tests/test_system_config_api.py
cd apps/dsa-web && npm run test -- NotificationTestPanel
cd apps/dsa-web && npm run lint
cd apps/dsa-web && npm run build
./scripts/ci_gate.sh
git diff --check

关键输出/结论:

  • python -m py_compile ...:通过。
  • targeted pytest:145 passed, 35 warnings, 43 subtests passed
  • npm run test -- NotificationTestPanel3 passed
  • npm run lint:通过。
  • npm run build:通过,仅有既有 chunk size warning。
  • ./scripts/ci_gate.sh1881 passed, 2 deselected, 47 warnings, 166 subtests passed
  • git diff --check:通过。

未运行 python main.py --check-notify:当前未注入真实临时通知配置;无配置环境下该命令可能返回配置诊断,不作为本 PR 阻断项。

Compatibility And Risk

  • 不新增配置项,不修改 .env.example、config registry、workflow env 或 API Schema。
  • 第三方模型/API 兼容性:不适用。本 PR 不修改 LLM/provider/Base URL/LiteLLM、请求参数、路由前缀、provider fallback 或模型兼容语义。
  • 运行时配置迁移:不适用。本 PR 不修改配置保存、清理、迁移或回填逻辑,也不会改写用户既有配置。
  • src/services/system_config_service.py 的生产改动仅限 custom webhook 通知测试顶层 message,新增成功数 / 总数展示;不触及 LLM 配置校验、模型发现或运行时 provider 解析。
  • 不改普通 sender 返回契约,不新增 retry loop;普通渠道返回 False 仍按 send_failed 处理。
  • 聚合报告路径中单一渠道异常现在会被记录并视为该渠道失败,后续渠道继续发送;任一静态渠道成功时仍 record noise,全部静态渠道失败时 release noise。
  • Web 生产组件未改,仅补测试覆盖。

Diff-Level Compatibility Evidence

结构化 review 检测到的 LLM/provider/Base URL/LiteLLM/运行时配置迁移风险是误报。实际 diff 级证据如下:

git diff --name-only upstream/main...HEAD

输出只包含本 PR 的通知、测试和文档文件:

apps/dsa-web/src/components/settings/__tests__/NotificationTestPanel.test.tsx
docs/CHANGELOG.md
docs/notifications.md
src/core/pipeline.py
src/services/system_config_service.py
tests/test_pipeline_notification_image_routing.py
tests/test_system_config_service.py

src/services/system_config_service.py 的生产 diff 只有 custom webhook 通知测试 message 聚合:

-            success = any(bool(attempt.get("success")) for attempt in attempts)
+            success_count = sum(1 for attempt in attempts if bool(attempt.get("success")))
+            total_count = len(attempts)
+            success = success_count > 0
+            if success_count == total_count and total_count > 0:
+                message = f"自定义 Webhook 通知测试成功({success_count}/{total_count})"
+            elif success_count > 0:
+                message = f"自定义 Webhook 通知测试部分成功({success_count}/{total_count})"
+            else:
+                message = f"自定义 Webhook 通知测试失败({success_count}/{total_count})"
...
-                message="自定义 Webhook 通知测试成功" if success else "自定义 Webhook 通知测试失败",
+                message=message,

针对新增/删除行做兼容风险关键词过滤:

git diff -U0 upstream/main...HEAD -- src/services/system_config_service.py tests/test_system_config_service.py docs/notifications.md apps/dsa-web/src/components/settings/__tests__/NotificationTestPanel.test.tsx | rg -n "^[+-].*(LLM|LiteLLM|provider|model|Base URL|base_url|api_key|OPENAI|ANTHROPIC|GEMINI|废弃|deprecated)"

该命令无输出,rg 返回 1 表示没有匹配到任何新增/删除行。由此确认本 PR 没有触及 LLM/provider/Base URL/LiteLLM、运行时模型选择、provider fallback、配置保存/清理/迁移/回填逻辑。

Rollback Plan

Revert this PR 即可回到旧行为;无需额外回滚配置、环境变量或数据迁移。

EXTRACT_PROMPT Change (if applicable)

不适用。

Checklist

  • 本 PR 有明确动机和业务价值 / This PR has a clear motivation and value
  • 已提供可复现的验证命令与结果 / Reproducible verification commands and results are included
  • 已评估兼容性与风险 / Compatibility and risk have been assessed
  • 已提供回滚方案 / A rollback plan is provided
  • 若涉及用户可见变更,已同步更新相关文档与 docs/CHANGELOG.mdREADME.md 仅在首页级信息变化时更新,细节优先写入 docs/*.md / If user-visible changes are included, relevant docs and docs/CHANGELOG.md are updated; README.md is updated only for homepage-level changes, with details kept in docs/*.md

@github-actions github-actions Bot added configuration documentation Improvements or additions to documentation notification size/L testing labels May 11, 2026
@github-actions

Copy link
Copy Markdown

🤖 自动审查报告

项目 结果
📊 变更文件 7 个
➕ 新增行数 361 行
➖ 删除行数 80 行
🔍 静态检查 ✅ 通过
🧠 AI 审查 ✅ 已完成

📁 修改的文件

  • 📝 apps/dsa-web/src/components/settings/__tests__/NotificationTestPanel.test.tsx (+91/-0)
  • 📝 docs/CHANGELOG.md (+1/-0)
  • 📝 docs/notifications.md (+10/-2)
  • 📝 src/core/pipeline.py (+138/-76)
  • 📝 src/services/system_config_service.py (+10/-2)
  • 📝 tests/test_pipeline_notification_image_routing.py (+43/-0)
  • 📝 tests/test_system_config_service.py (+68/-0)

🧠 AI 代码审查意见

结论

Ready to Merge

结构化审查结果

  • 必要性通过

    • 理由:本 PR 明确解决了聚合报告通知中单个渠道失败导致后续渠道无法发送的问题(P5 隔离边界),并增强了自定义 Webhook 测试的诊断信息,具有明确的业务价值和问题修复动机。
  • 关联性通过

  • 类型:建议类型为 fix, docs, test。与 PR 描述中勾选的类型 fix, docs, test 完全一致。

  • 描述完整性完整

    • 背景与问题:清晰阐述 P5 需求,指出隔离边界和 Web 通知测试诊断改进。
    • 变更范围:详细列出涉及的文件及其修改内容,与代码 diff 和文档更新吻合。
    • 验证命令与结果:提供了详细的验证命令和输出结果,包括 Python 编译、定向 pytest、前端测试、Lint 和 npm run build,以及 ci_gate.sh 的执行和通过,充分证明了变更的正确性。CI 检查状态也与此一致。
    • 兼容性与风险:评估了兼容性,指出不新增配置、不修改 API 契约,并说明了新的失败隔离行为,风险等级为低。
    • 回滚方案:提供了明确的回滚方案。
    • 用户可见变更:更新了 docs/CHANGELOG.mddocs/notifications.md,符合要求。
  • 风险级别

    • 关键风险:主要修改在聚合报告的通知发送逻辑中增加了异常隔离,并通过单元测试和集成测试进行了验证。Web UI 仅增加了测试用例,未改动生产组件。对现有系统行为影响可控,风险较低。

必改项(无)


建议项

  1. src/services/system_config_service.py 文件 diff 缺失:PR 描述中提到 src/services/system_config_service.py 调整了 custom webhook 测试的顶层 message。虽然前端测试 NotificationTestPanel.test.tsx 涵盖了这部分变更的验证,并且测试已通过,但在提供的代码 diff 片段中未看到该文件的具体修改。建议在未来的 PR 中,即使改动较小,也尽可能提供所有涉及文件的 diff 内容,以供更全面的代码审查。但这不影响本次 PR 的合入。

详细审查注释

  1. src/core/pipeline.py (核心逻辑)

    • 引入 _send_channel_safely 辅助函数,完美地实现了单个通知渠道的异常隔离,确保异常不会中断后续渠道的发送。日志输出也清晰明了,便于诊断。
    • WECHAT, FEISHU, TELEGRAM, EMAIL, CUSTOM 等所有静态渠道都应用了 _send_channel_safely,覆盖范围全面。
    • 邮件通知的发送逻辑,特别是在 stock_email_groups 存在时,也确保了按 receiver group 进行独立的异常隔离,符合 PR 描述。
    • non_wechat_success = ... or non_wechat_success 的逻辑正确地聚合了各渠道的成功状态,这将正确地支持 docs/notifications.md 中描述的降噪机制(任一渠道成功则记录噪音,全部失败则释放噪音)。
  2. apps/dsa-web/src/components/settings/__tests__/NotificationTestPanel.test.tsx (前端测试)

    • 新增了两个测试用例,清晰验证了自定义 Webhook 部分失败和普通渠道超时异常的 UI 显示逻辑。
    • 测试断言涵盖了消息文本 (部分成功, timeout)、HTTP 状态码 (HTTP 500, HTTP 200) 以及错误码样式 (text-warning),验证充分。
  3. docs/CHANGELOG.mddocs/notifications.md (文档)

    • CHANGELOG.md 已同步更新了本次修复内容。
    • notifications.md 更新了 P5 基线描述,并新增了 聚合报告失败隔离 的详细章节,完整地描述了新的行为,包括渠道隔离、邮件分组隔离、降噪机制和 send_to_context() 的独立性。文档更新及时且详细。

本次 PR 的实现与描述高度一致,代码质量和测试覆盖都很好,文档也同步更新,满足所有审查标准。


💡 提示: 请确保代码已通过本地测试,并遵循项目代码规范。

@massif-01 massif-01 changed the title [P5] 修复聚合报告通知失败隔离 [fix] 修复聚合报告通知失败隔离 ( P5 ) May 11, 2026
@massif-01
massif-01 marked this pull request as ready for review May 11, 2026 17:34
@massif-01
massif-01 requested a review from ZhuLinsen as a code owner May 11, 2026 17:34
ZhuLinsen

This comment was marked as resolved.

@massif-01

Copy link
Copy Markdown
Collaborator Author

评审结论

  • 必要性:通过,聚合报告通知失败隔离符合 AGENTS.md 中“单一通知渠道失败不应拖垮主流程”的稳定性护栏。
  • 是否有对应 issue:有,Refs [Feature] DSA 后续开发规划:[通知渠道网关] 渠道基线、测试、Body 模板、路由、降噪与长尾渠道扩展 #1200
  • PR 类型:fix/test/docs,修复聚合报告通知隔离,并补充后端、Web 测试与通知文档、CHANGELOG。
  • description 完整性:完整,已说明背景、范围、验证命令与结果、兼容性风险、回滚方案,且符合本仓库对用户可见通知行为变更同步文档与 docs/CHANGELOG.md 的要求。
  • 是否可直接合入:不可,需先澄清结构化检测到的外部模型/API 兼容风险是否为误报;当前证据包未提供对应官方来源、依赖/运行时兼容验证、旧配置迁移或回退路径说明。

主要问题

  1. [Process blocker] src/services/system_config_service.py:结构化事实提示本 PR 检测到模型名、provider、Base URL、废弃日期、LiteLLM 或 LLM 配置相关改动。PR 描述称“不新增配置项、不修改 API Schema”,但未明确说明该检测是否为误报,也未提供 AGENTS.md 要求的外部模型/API 兼容性证据,包括官方来源、当前锁定依赖/运行时兼容验证、旧配置迁移与回退路径。若本次实际没有改动 LLM/provider/Base URL 语义,请在 PR 描述中补充说明并给出 diff 范围依据;若确有改动,则需要补齐兼容性与迁移验证后再合入。

🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。

评审结论

  • 必要性:通过,聚合报告通知失败隔离符合 AGENTS.md 中“单一通知渠道失败不应拖垮主流程”的稳定性护栏。
  • 是否有对应 issue:有,Refs [Feature] DSA 后续开发规划:[通知渠道网关] 渠道基线、测试、Body 模板、路由、降噪与长尾渠道扩展 #1200
  • PR 类型:fix/test/docs,修复聚合报告通知隔离,并补充后端、Web 测试与通知文档、CHANGELOG。
  • description 完整性:完整,已说明背景、范围、验证命令与结果、兼容性风险、回滚方案,且符合本仓库对用户可见通知行为变更同步文档与 docs/CHANGELOG.md 的要求。
  • 是否可直接合入:不可,需先澄清结构化检测到的外部模型/API 兼容风险是否为误报;当前证据包未提供对应官方来源、依赖/运行时兼容验证、旧配置迁移或回退路径说明。

主要问题

  1. [Process blocker] src/services/system_config_service.py:结构化事实提示本 PR 检测到模型名、provider、Base URL、废弃日期、LiteLLM 或 LLM 配置相关改动。PR 描述称“不新增配置项、不修改 API Schema”,但未明确说明该检测是否为误报,也未提供 AGENTS.md 要求的外部模型/API 兼容性证据,包括官方来源、当前锁定依赖/运行时兼容验证、旧配置迁移与回退路径。若本次实际没有改动 LLM/provider/Base URL 语义,请在 PR 描述中补充说明并给出 diff 范围依据;若确有改动,则需要补齐兼容性与迁移验证后再合入。

🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。

已复核,这里的结构化检测应为误报。本 PR 没有修改模型名、provider、Base URL、LiteLLM 或 LLM 配置语义。

依据:

  • PR 真实 diff 仅涉及 7 个文件,范围为聚合报告通知失败隔离、通知测试诊断、Web 测试和通知文档。
  • src/services/system_config_service.py 的生产 diff 只在 custom webhook 通知测试分支新增 success_count / total_count,并调整顶层 message 为成功数/总数。
  • 使用 upstream/main...HEAD 复核后,新增/删除行中过滤 LLM|LiteLLM|provider|model|Base URL|base_url|api_key|OPENAI|deprecated 无命中。
  • 未修改 .env.example、config registry、workflow env、API Schema、依赖或运行时配置。

因此本 PR 不需要补充外部模型/API 官方兼容资料、旧配置迁移说明或额外回退路径;回滚方式仍为 revert 当前 PR。

ZhuLinsen

This comment was marked as resolved.

@massif-01

Copy link
Copy Markdown
Collaborator Author

已按最新 review 补充 PR 描述中的 Diff-Level Compatibility Evidence 小节,明确结构化检测到的 LLM/provider/Base URL/LiteLLM/运行时配置迁移风险为误报,并给出 diff 级证据。

补充内容包括:

  • git diff --name-only upstream/main...HEAD 的实际文件范围。
  • src/services/system_config_service.py 的生产 diff 片段,显示仅修改 custom webhook 通知测试顶层 message。
  • 对新增/删除行执行 LLM/provider/Base URL/LiteLLM/配置迁移相关关键词过滤的命令与无匹配结果。

结论:本 PR 未修改 LLM 配置校验、模型发现、运行时 provider 解析、provider fallback、配置保存/清理/迁移/回填逻辑;不需要补充外部模型/API 官方兼容资料或旧配置迁移路径。

@ZhuLinsen ZhuLinsen left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

评审结论

  • 必要性:通过,聚合报告通知失败隔离符合 AGENTS.md 中“单一通知渠道失败不应拖垮整个分析主流程”的稳定性护栏。
  • 是否有对应 issue:有,Refs #1200
  • PR 类型:fix/test/docs,修复通知隔离行为,并补充后端、Web 测试与通知文档、CHANGELOG。
  • description 完整性:完整,已覆盖背景、范围、验证命令与结果、兼容性风险、回滚方案;后续补充的 Diff-Level Compatibility Evidence 已说明结构化检测到的 LLM/provider/Base URL/运行时配置迁移风险为误报,并给出文件范围与新增/删除行过滤证据。
  • 是否可直接合入:可,当前 CI 为 success,改动范围与描述一致,未发现阻断性正确性、兼容性或流程问题;mergeable_state=blocked 在本仓库分支保护规则下不单独构成不可合入理由。

🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。

@ZhuLinsen
ZhuLinsen merged commit f701bfd into ZhuLinsen:main May 12, 2026
10 checks passed
davidzhengdai pushed a commit to davidzhengdai/daily_stock_analysis that referenced this pull request May 12, 2026
Anyone878 pushed a commit to Anyone878/daily_stock_analysis that referenced this pull request May 17, 2026
EchoingFootsteps pushed a commit to EchoingFootsteps/daily_stock_analysis that referenced this pull request Jul 4, 2026
bmwu pushed a commit to bmwu/daily_stock_analysis that referenced this pull request Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants