feat: improve market review report cards - #1265
Conversation
404508f to
9dbd23a
Compare
🤖 自动审查报告
📁 修改的文件
🧠 AI 代码审查意见结论: Ready to Merge 审查结果
必改项 (无)本次审查未发现阻断合入的必改项。代码质量、功能健壮性、文档完整性均符合要求。 建议项 (2 条)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dbd23a473
ℹ️ 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".
| def _parse_table_row(row: str) -> List[str]: | ||
| return [cell.strip() for cell in row.strip().strip("|").split("|")] |
There was a problem hiding this comment.
Preserve escaped pipes when parsing card tables
When a report table cell contains a literal pipe, the Markdown is escaped as \| by _escape_table_cell (for example in news headlines/snippets), but this parser still splits on that escaped delimiter. In that scenario a valid row such as a headline AI \| chips becomes an extra card column, shifting or dropping subsequent cells in the Feishu card; parse escaped pipes as cell content (and unescape them after splitting) before building the columns.
Useful? React with 👍 / 👎.
| if '首次封板时间' in df.columns: | ||
| df['首次封板时间'] = df['首次封板时间'].astype(str) | ||
| sort_cols = [col for col in ('连板数', '首次封板时间') if col in df.columns] | ||
| if sort_cols: | ||
| ascending = [False if col == '连板数' else True for col in sort_cols] | ||
| df = df.sort_values(sort_cols, ascending=ascending) |
There was a problem hiding this comment.
Normalize first-seal times before sorting
When AkShare returns 首次封板时间 as numeric-looking values without a leading zero for pre-10:00 seals (e.g. 92500 for 09:25), casting to str and sorting lexicographically puts those early seals after afternoon values such as 141354 within the same board count. The new ladder is meant to prioritize earliest seals, so common open-auction limit-ups can be misordered and later displayed as raw 5-digit times; normalize to zero-padded HHMMSS or a sortable timestamp before sorting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR enhances the CN market review (“大盘复盘”) by adding concept/theme breadth, hot-stock popularity, and limit-up ladder structure to better validate tradable leadership, and upgrades Feishu notifications to render reports as structured interactive cards (with a text fallback).
Changes:
- Extend market overview/report injection to include concept rankings, hot stocks, and limit-up ladder blocks (plus updated prompt/template structure).
- Add Feishu interactive-card element renderer to properly display tables/quotes/code fences, with fallback to text formatting.
- Add tests and docs/changelog updates covering the new report sections and Feishu rendering.
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
data_provider/akshare_fetcher.py |
Adds AkShare-backed concept rankings, hot-stock ranking (with source fallback chain), and limit-up pool retrieval. |
data_provider/base.py |
Introduces new provider/manager APIs for concept rankings, hot stocks, and limit-up pool with multi-fetcher fallback behavior. |
src/market_analyzer.py |
Expands MarketOverview, fetches new CN-only sections, injects them into generated reviews, and updates prompt/template structure/headings. |
src/formatters.py |
Improves Feishu markdown normalization (tables -> aligned text) and adds build_feishu_card_elements() to render structured card modules. |
src/notification_sender/feishu_sender.py |
Switches Feishu delivery to structured interactive cards first, with fallback to formatted text messages. |
tests/test_formatters.py |
Adds unit coverage for Feishu markdown normalization and card-element table/quote rendering. |
tests/test_notification_sender.py |
Verifies Feishu sender produces structured card elements for markdown tables. |
tests/test_market_analyzer_generate_text.py |
Updates heading expectations and adds regression coverage for injected hot-stocks + limit-up ladder blocks. |
tests/test_akshare_realtime_logging.py |
Adds a hot-stocks test case (currently not accurately named for fallback behavior). |
docs/full-guide.md |
Updates CN guide to reflect the new “盘面评分/概念/人气股/连板” report structure and hot-stock fallback sources. |
docs/full-guide_EN.md |
Updates EN guide with the new market-review structure and hot-stock fallback explanation. |
docs/CHANGELOG.md |
Adds Unreleased entries for the new report blocks, Feishu card rendering, and new tests. |
| @@ -127,7 +125,7 @@ def send_to_feishu(self, content: str, *, timeout_seconds: Optional[float] = Non | |||
| return False | |||
|
|
|||
| # 检查字节长度,超长则分批发送 | |||
| content_bytes = len(formatted_content.encode('utf-8')) + keyword_overhead | |||
| content_bytes = len(content.encode('utf-8')) + keyword_overhead | |||
| if content_bytes > max_bytes: | |||
| min_chunk_bytes = MIN_MAX_BYTES + PAGE_MARKER_SAFE_BYTES | |||
| if effective_max_bytes < min_chunk_bytes: | |||
| @@ -138,10 +136,10 @@ def send_to_feishu(self, content: str, *, timeout_seconds: Optional[float] = Non | |||
| ) | |||
| return False | |||
| logger.info(f"飞书消息内容超长({content_bytes}字节/{len(content)}字符),将分批发送") | |||
| return self._send_feishu_chunked(formatted_content, effective_max_bytes) | |||
| return self._send_feishu_chunked(content, effective_max_bytes) | |||
|
|
|||
| try: | |||
| return self._send_feishu_message(formatted_content, timeout_seconds=timeout_seconds) | |||
| return self._send_feishu_message(content, timeout_seconds=timeout_seconds) | |||
| except Exception as e: | |||
| for idx in range(max_cols) | ||
| ] | ||
|
|
||
| output.append("```") |
| def test_hot_stocks_falls_back_to_eastmoney(monkeypatch, akshare_fetcher): | ||
| monkeypatch.setattr( | ||
| akshare_fetcher, | ||
| "_get_eastmoney_hot_stocks", | ||
| lambda _ak, n: [ | ||
| { | ||
| "rank": 1, | ||
| "code": "SZ000066", | ||
| "name": "中国长城", | ||
| "price": 21.8, | ||
| "change_pct": 9.99, | ||
| "source": "东方财富人气榜", | ||
| } | ||
| ], | ||
| ) | ||
|
|
||
| result = akshare_fetcher.get_hot_stocks(5) | ||
|
|
||
| assert result[0]["source"] == "东方财富人气榜" | ||
| assert result[0]["name"] == "中国长城" |
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性:通过。本 PR 补齐 A 股大盘复盘的人气股、概念题材、涨停连板结构,并改善飞书推送展示,业务价值明确。
- 是否有对应 issue:无。PR 描述未检测到 Fixes/Closes/Refs 关联语句。
- PR 类型:feat/test。新增报告数据与飞书卡片能力,并补充回归测试。
- description 完整性:基本完整。已包含背景、范围、截图、验证结果、风险与回滚方案;但未说明检测到的 LLM/provider/Base URL 相关改动是否仅为测试/既有语义同步,缺少兼容性说明。
- 是否可直接合入:不可。当前 CI 为 success,代码也可合并,但仍有飞书回退分片正确性风险,以及外部模型/API 兼容性检测项未澄清。
主要问题
- [Correctness blocker]
src/notification_sender/feishu_sender.py:飞书发送的分片/长度检查基于原始content,但卡片发送失败后会回退到format_feishu_markdown(prepared_content)。该 fallback 文本可能因表格转等宽块、代码围栏等显著膨胀,导致原本未分片的内容在回退文本路径超过飞书 20KB 限制并发送失败。风险是卡片失败时本应兜底的文本消息也被拒收。建议按 fallback 文本的最坏情况做长度预算,或在回退发送前再次按字节限制分片。 - [Correctness blocker]
src/formatters.py:表格解析仍直接按|split。当前 Markdown 表格单元格如果包含转义管道符\|,会被错误拆列;这类内容在新闻标题、摘要或概念描述中并不少见。风险是飞书卡片列错位、内容丢失或表格 fallback 展示错误。建议解析表格行时保留转义语义,至少补充覆盖\|的单测。 - [Correctness blocker]
data_provider/akshare_fetcher.py/src/market_analyzer.py:涨停池first_limit_time若来自 AkShare 的数值形态且缺少前导零,例如92500,按字符串排序会把 09:25 误排到 10:00 之后。风险是连板梯队中的“首次封板时间”排序失真,影响复盘对情绪强度的判断。建议入库/排序前统一规范为HHMMSS或HH:MM:SS再比较,并补测试覆盖。 - [Process blocker] 结构化检测提示本 PR 涉及模型名、provider、Base URL、LiteLLM 或 LLM 配置相关改动,但 PR 描述只说明
EXTRACT_PROMPT未变更,没有解释这些检测项是否来自测试桩、既有文档同步,还是运行时兼容语义变化。按仓库规范,涉及外部模型/API 兼容面时需要说明官方来源、当前依赖/运行时兼容验证、旧配置迁移和回退路径;若实际没有运行时变化,也需要在描述中明确收窄说明。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性:通过。补齐 A 股复盘的人气股、概念题材、涨停连板结构,并改善飞书报告卡片展示,业务价值明确。
- 是否有对应 issue:无。PR 描述未检测到 Fixes/Closes/Refs 关联语句。
- PR 类型:feat/test。新增报告数据与飞书结构化卡片能力,并补充对应回归测试。
- description 完整性:不完整。背景、范围、截图、验证、风险和回滚方案较完整,但未说明检测到的 LLM/provider/Base URL/运行时模型配置相关改动是否为既有测试上下文、无运行时语义变更,缺少兼容性与迁移风险说明。
- 是否可直接合入:不可。CI 当前为 success,mergeable_state=blocked 不单独构成阻断;但外部模型/API 与运行时配置迁移风险提示未在 PR 描述和证据中闭环,合入前需要澄清或补充验证依据。
主要问题
- [Process blocker]
tests/test_market_analyzer_generate_text.py及结构化检测结果提示本 PR 涉及模型名、provider、Base URL、LiteLLM 或运行时模型配置相关改动/清理逻辑。按照 AGENTS.md 的稳定性护栏,这类改动需要明确当前依赖/运行时兼容性、旧配置迁移或回退路径,以及是否会静默清空/迁移用户配置。当前 PR 描述只说明报告与飞书卡片改动,未解释该检测命中的范围是否仅为测试/既有上下文,也未给出兼容性证据;在澄清前不能按低风险直接合入。 src/notification_sender/feishu_sender.py/src/formatters.py的飞书消息形态从单个lark_md文本块扩展为结构化交互卡片,属于用户可见行为变化。PR 已补充 docs 和 CHANGELOG,也有回退文本与分片测试;建议在描述中再明确卡片发送失败后的 fallback 分片路径已覆盖,便于维护者确认前面自动评审指出的 20KB 回退文本风险已被当前 head 修复。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性:通过。补齐 A 股复盘的人气股、概念题材、涨停连板结构,并改善飞书卡片展示,业务价值明确。
- 是否有对应 issue:无。PR 描述未检测到 Fixes/Closes/Refs 关联语句。
- PR 类型:feat/test。新增复盘数据与飞书结构化卡片能力,并补充对应回归测试。
- description 完整性:完整。已包含背景、范围、截图、验证结果、兼容性风险与回滚方案;README 未更新的理由符合 AGENTS.md 对首页级文档的边界要求,
docs/CHANGELOG.md也使用了[Unreleased]扁平格式。 - 是否可直接合入:可。当前 CI 为 success,文档与中英文指南已同步;
mergeable_state=blocked仅按分支保护状态看待,未发现冲突或阻断性实现风险。结构化检测命中的 LLM/provider/Base URL 线索,从当前 diff 看已有说明为无运行时语义变更,并补充了不修改运行时配置字段的回归测试,不作为合入阻断。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
7c9ff6a to
d8da56a
Compare
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性:通过。补齐 A 股复盘的人气股、概念题材、涨停连板结构,并改善飞书卡片展示,业务价值明确。
- 是否有对应 issue:无,PR 描述未检测到 Fixes/Closes/Refs 关联语句。
- PR 类型:feat/test + 文档同步;新增报告数据与飞书结构化卡片能力,并补充回归测试和指南/CHANGELOG。
- description 完整性:完整。已覆盖背景、范围、截图、验证结果、兼容性风险与回滚方案;README 未更新的说明符合 AGENTS.md 对首页级文档边界的要求,
docs/CHANGELOG.md也使用了[Unreleased]扁平格式。 - 是否可直接合入:可。当前 CI 为 success,mergeable_state=blocked 仅表示受分支保护/权限限制;未看到代码层阻断点。结构化检测提示的 LLM/provider/Base URL/运行时配置风险,当前改动已在描述、CHANGELOG 和测试中说明/覆盖为“不改变运行时语义、不静默迁移配置”,且改动文件不包含配置入口或模型默认值文件。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
d8da56a to
553f2b2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 553f2b2958
ℹ️ 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".
| return False | ||
| logger.info(f"飞书消息内容超长({content_bytes}字节/{len(content)}字符),将分批发送") | ||
| return self._send_feishu_chunked(formatted_content, effective_max_bytes) | ||
| return self._send_feishu_chunked(content, effective_max_bytes) |
There was a problem hiding this comment.
Preserve table context when chunking card messages
When a report exceeds FEISHU_MAX_BYTES, this now chunks the raw Markdown and sends each chunk through build_feishu_card_elements. If a chunk boundary falls inside one of the report tables, the card renderer treats the first row in that chunk as the table header and excludes it from data_rows, so the first stock/news/metric row of that page is lost or rendered as headers. This can affect long market reports or lower configured byte limits; chunk after rendering to a table-safe fallback, or carry the table header/context across chunks.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性:通过。本 PR 补齐 A 股复盘的人气股、概念题材、涨停连板结构,并改善飞书卡片展示,业务价值明确。
- 是否有对应 issue:无,PR 描述未检测到 Fixes/Closes/Refs 关联语句。
- PR 类型:feat/test + docs,新增报告与推送能力,并补充测试、指南和 CHANGELOG。
- description 完整性:完整。已覆盖背景、范围、截图、验证结果、兼容性风险与回滚方案;README 未更新理由符合 AGENTS.md 对首页级文档边界的要求,
docs/CHANGELOG.md也使用了[Unreleased]扁平格式。 - 是否可直接合入:不可。CI 当前为 success,mergeable_state=blocked 不单独构成阻断;但飞书超长报告分片后的表格上下文仍有真实展示风险,需要先修正或补充验证。
主要问题
- [Correctness blocker]
src/notification_sender/feishu_sender.py/src/formatters.py:飞书卡片路径在内容超过FEISHU_MAX_BYTES时会先按原始 Markdown 分片,再对每个 chunk 调用卡片元素构建。若分片边界落在 Markdown 表格中间,后续 chunk 可能缺少原始表头/分隔行上下文,导致表格行被降级为普通文本、被错误解释,或同一张表在多条卡片中结构不一致。现有测试覆盖了“数据行不被提升为表头”的一类情况,但还不足以证明跨 chunk 的表格语义和列结构被完整保留。风险是大盘复盘里人气股、涨停池、行业/概念表较长时,飞书端展示会丢失表格上下文,正好影响本 PR 的核心用户可见能力。建议在分片前按 Markdown 块级结构切分,或在表格跨片时为续片显式携带表头,并补充覆盖表头与多行数据分属不同 chunk 的回归测试。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性:通过。补齐 A 股大盘复盘的人气股、概念题材、涨停连板结构,并改善飞书卡片展示,业务价值明确。
- 是否有对应 issue:无。PR 描述未检测到 Fixes/Closes/Refs 关联语句。
- PR 类型:feat/test/docs。新增报告与推送能力,补充回归测试,并同步指南与
docs/CHANGELOG.md。 - description 完整性:完整。已覆盖背景、范围、截图、验证结果、兼容性风险与回滚方案;README 未更新理由符合 AGENTS.md 对首页级文档边界的要求,
docs/CHANGELOG.md也使用了[Unreleased]扁平格式。 - 是否可直接合入:可。当前 CI 为 success,完整 diff 中未发现阻断性行为风险;
mergeable_state=blocked更像分支保护/权限状态,不单独构成代码层阻断。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
|
这个 PR 我建议先不要直接合入,先拆分。 当前改动范围太大了:14 个文件、 建议拆成几个独立 PR:
尤其飞书卡片这部分,我倾向于不要混在本 PR 里。先把报告数据增强做小、做稳;飞书展示等方案确认后再单独推进,这样后续如果卡片效果不符合预期,也可以独立回滚,不会影响大盘复盘能力。 |
ZhuLinsen
left a comment
There was a problem hiding this comment.
跟进结论
- 结论:部分接受。最新回复对改动粒度过大、审查和回滚成本偏高的判断成立,但这更像合入策略建议,不等同于当前代码存在明确正确性阻断。
- 是否仍有阻断:无。当前 CI 为 success,已补充文档、CHANGELOG、测试与回滚说明,且现有证据未显示仍有未修复的代码行为风险。
说明
- 改动确实横跨
data_provider/、src/market_analyzer.py、src/formatters.py、src/notification_sender/feishu_sender.py、文档、截图资产和测试,且统计为 14 个文件、+1668/-182;“建议拆分以降低审查与回滚成本”这个维护建议成立。 - 但 PR 描述已明确把数据源扩展、复盘结构、飞书卡片渲染、文档和测试列入 scope,不能简单判定为范围漂移。
docs/CHANGELOG.md已补充说明本 PR 未改变模型名/provider/Base URL/LLM_CHANNELS 运行时语义,tests/test_market_analyzer_generate_text.py也增加了运行时 LLM 配置不被 mutate 的回归测试;此前关于外部模型/API 配置迁移风险的结论可以收缩为已覆盖。- 之前 Codex 自动评论提到飞书卡片分片时表格上下文丢失风险;当前 diff 中
src/formatters.py已围绕表格分片补强,tests/test_formatters.py和tests/test_notification_sender.py也覆盖了分片后复用表头、不把数据行提升为表头的场景。 - 如果维护者希望按数据源、复盘结构、飞书展示拆成多个 PR,我支持这个合入策略;但基于当前代码、讨论和 CI 状态,我不会再把“未拆分”单独列为阻断。
🤖 此回复由 OpenReview Bot 自动生成,仅供参考。如有疑问请 @维护者。
|
已按前面讨论把 #1265 拆成独立 PR:
建议后续不要继续合并这个大 PR。优先推进 #1278 和 #1280;飞书展示等方案确认后再单独重做/推进 #1281 或新 PR。 |
PR Type
Background And Problem
A 股大盘复盘当前更像指数/行业摘要,缺少能校验真实交易主线的人气股、概念题材和涨停连板结构。飞书推送侧也仍依赖单个
lark_md文本块,Markdown 表格、引用和代码块在群消息里容易原样散乱展示。本 PR 恢复并收敛本地 stash 中的报告/推送改动,同时移除最初方案里的同花顺 Cookie 依赖:人气榜默认走东方财富人气榜、东方财富飙升榜、雪球关注榜等免配置渠道。
Scope Of Change
data_provider/src/market_analyzer.pysrc/formatters.py/src/notification_sender/feishu_sender.pydocs/docs/CHANGELOG.md。tests/Screenshots
Market Review Report Preview
Feishu Card Preview
Verification Commands And Results
Key output/conclusion:
git diff --checkpassed.py_compilepassed.128 passed, 3 warnings in 1.40s../scripts/ci_gate.sh: syntax, flake8 critical checks, deterministic scripts, and offline suite passed.1888 passed, 2 deselected, 61 warnings, 166 subtests passed in 840.74s.Compatibility And Risk
docs/CHANGELOG.mdand the detailed full guides were updated.README.mdwas not changed because this is detailed report behavior, not homepage-level project positioning.Rollback Plan
Revert this PR. No configuration, database, or migration rollback is required. If Feishu rendering has unexpected compatibility issues, reverting restores the previous single-block card/text behavior.
EXTRACT_PROMPT Change (if applicable)
Not applicable. This PR does not change
src/services/image_stock_extractor.pyorEXTRACT_PROMPT.Checklist
docs/CHANGELOG.md;README.md仅在首页级信息变化时更新,细节优先写入docs/*.md/ If user-visible changes are included, relevant docs anddocs/CHANGELOG.mdare updated;README.mdis updated only for homepage-level changes, with details kept indocs/*.md