Skip to content

fix(collection): identity dedup + chunk IN() + kill tree + help TTL + safe CDP cleanup [修复组⑥] - #30

Merged
2233admin merged 1 commit into
mainfrom
fix/collection-misc
Jul 19, 2026
Merged

fix(collection): identity dedup + chunk IN() + kill tree + help TTL + safe CDP cleanup [修复组⑥]#30
2233admin merged 1 commit into
mainfrom
fix/collection-misc

Conversation

@2233admin

Copy link
Copy Markdown
Owner

修复组⑥ — 杂项 (账本 C7/C15/C16/C17/C20)

Sonnet 实施, Fable 审计 + rebase 修 migration 冲突。已 rebase 到含 ④⑤ 的最新 main。

  • C7 RSS identity() 去重键从未接线 → 补 CollectedRecord.identity_key 列 + (source_id, identity_key) 复合索引; storer 按 identity supplementary 匹配: 命中同 content_hash 跳过, 不同则原地 update (刷新 content/hash, 清 ai_enrichment 重跑) 并入 new_records; identity 为 None 的通道走原 content_hash 路径不变。migration
  • C15 去重 IN() 撞 SQLite 变量上限 → 分块 ≤500/批 union
  • C16 cli 超时只 kill 直接子进程 → 复用 opencli_channel 的进程树 kill (Windows taskkill /T + POSIX 进程组); reap 失败落日志非静默
  • C17 opencli --help 热路径 subprocess + 缓存永不失效 → 事件显示串改 cache-only 不 spawn; 缓存加 TTL
  • C20 CDP tab 快照吞错关用户标签页 → 快照失败返回 None → 跳过 cleanup (无可信基线不关任何 tab)

Fable 收口 (关键)

  • migration revision 冲突修复: ⑥ 原 migration 与 ④ 并行开发撞了同一 revision id t9y0z1a2b3c4 (各自基于旧 head 生成"下一个"), ④ 先合占用 → 会造成 alembic 双 revision/multiple heads。已重链: 新 id u0a1b2c3d4e5, down_revision=t9y0z1a2b3c4 (④ 之后)。alembic heads 现单一 u0a1b2c3d4e5 (chain: s8x→t9 cursor→u0 identity)
  • storer 事务微妙点 (agent 自查): flush 刷全部脏状态 + rollback 撤整事务, insert-collision 回滚可能静默丢已 flush 的 update → 已改为 update+insert 单 flush + collision 分支显式清 updated_records + 日志, 下轮自然重采自愈

Test

  • 主 repo cp313 真跑 (rebase + migration 改名后): 1323 passed, 1 skipped, 3 failed。3 failed 全已知 flake: 2× nodes_install (GBK), 1× test_acquisition_runner (0.01s heartbeat 取消竞态, 隔离重跑通过, ⑥ 未碰 acquisition)
  • migration upgrade/downgrade 双向验证

合并顺序

已 rebase 到最新 main (含 #28/#29)。migration 挂 ④ 之后, 直接可合。

… + help cache TTL + safe CDP cleanup [C7,C15,C16,C17,C20]
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@2233admin, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 29 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9a43dd5a-8164-474b-af2a-e684dbfefd2b

📥 Commits

Reviewing files that changed from the base of the PR and between c5aae2a and 650f35b.

📒 Files selected for processing (17)
  • .claude/CLAUDE.md
  • .claude/launch.json
  • .omc/state/sessions/b4f7d02c-1da1-4b4d-9835-33e44401355e/hud-cache.json
  • .omc/state/sessions/dc752497-c4fd-4221-ae35-e78f40e96767/hud-cache.json
  • .omc/state/sessions/de0b7a77-e9c7-46ac-961a-f0e90ffa1f07/hud-cache.json
  • backend/channels/cli_channel.py
  • backend/channels/opencli_channel.py
  • backend/migrations/versions/u0a1b2c3d4e5_add_identity_key_to_collected_records.py
  • backend/models/record.py
  • backend/pipeline/pipeline.py
  • backend/pipeline/sinks/legacy_db_sink.py
  • backend/pipeline/storer.py
  • tests/unit/channels/test_cli_channel.py
  • tests/unit/channels/test_opencli_channel.py
  • tests/unit/pipeline/test_legacy_db_sink.py
  • tests/unit/pipeline/test_pipeline.py
  • tests/unit/pipeline/test_storer.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@repowise-bot

repowise-bot Bot commented Jul 18, 2026

Copy link
Copy Markdown

✅ Health: 6.8

📋 At a glance
5 hotspots touched · 16 new findings introduced · 1 co-change pair left out.

Files & modules (2)
  • backend (2 files)
    • backend/pipeline/pipeline.py
    • backend/channels/opencli_channel.py
  • tests (3 files)
    • .../channels/test_opencli_channel.py
    • .../channels/test_cli_channel.py
    • .../pipeline/test_pipeline.py

🚨 Change risk: 9.5/10 (high)
This change's risk is driven by:

  • more lines added than baseline
  • more scattered than baseline
🔎 More signals (2)

🔥 Hotspots touched (5)

  • backend/pipeline/pipeline.py — 10 commits/90d, 6 dependents · primary owner: xujinghua (100%)
  • backend/channels/opencli_channel.py — 31 commits/90d, 8 dependents · primary owner: xujinghua (100%)
  • .../channels/test_opencli_channel.py — 3 commits/90d, 0 dependents · primary owner: xujinghua (100%)
2 more
  • .../channels/test_cli_channel.py — 2 commits/90d, 0 dependents · primary owner: xujinghua (100%)
  • .../pipeline/test_pipeline.py — 2 commits/90d, 1 dependents · primary owner: xujinghua (100%)

🔗 Hidden coupling (1 file)

  • backend/channels/opencli_channel.py co-changes with backend/agent_server.py (10× — 🟡 notable) — not in this PR.

👀 Suggested reviewers @xujinghua


📊 Full report · ⭐ Star Repowise · 📥 Install bot · Last updated 2026-07-18 18:45 UTC
Silence on a single PR with [skip repowise] in the title · Per-repo toggle on repowise.dev/settings?tab=bot

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces several robustness improvements to the data collection pipeline, including TTL-based caching for OpenCLI commands to support binary upgrades, process-tree termination on CLI timeouts, and safe CDP tab cleanup that avoids closing user tabs on snapshot failures. It also implements stable identity-based deduplication and in-place updates for collected records, with chunked database queries to prevent exceeding SQLite's variable limits. The review feedback highlights a critical logic bug in the batch deduplication process where duplicate updates can occur for pre-existing identities, and advises against committing local .omc session cache files.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +155 to +179
if identity is not None:
existing = existing_by_identity.get(identity)
if existing is not None:
if existing.content_hash == content_hash:
# Same source-native item, content unchanged: a genuine
# duplicate, same as the content_hash-only path always did.
skipped += 1
else:
# Same source-native item, content changed (e.g. a feed
# fixed a title typo) — update in place instead of
# inserting a duplicate row (C7's fix).
existing.raw_data = raw
existing.normalized_data = normalized
existing.content_hash = content_hash
existing.status = "normalized"
existing.ai_enrichment = None
updated_records.append(existing)
continue
if identity in seen_identities_in_batch:
# Two triples in this same batch share an identity (e.g. a
# feed listed the same entry twice) — keep the first, skip
# the rest rather than fight over which one "wins".
skipped += 1
continue
seen_identities_in_batch.add(identity)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

问题分析
当前逻辑中,seen_identities_in_batch 的去重检查被放在了 existing is not None 的判断之后。
如果当前批次(batch)中包含多个具有相同 identity 的项,且该 identity 在数据库中已存在:

  1. 第一项会触发 existing is not None 分支,更新 existing 并将其加入 updated_records,然后通过 continue 跳过后续代码(因此该 identity 不会被加入 seen_identities_in_batch)。
  2. 第二项(以及后续相同 identity 的项)也会触发 existing is not None 分支,再次更新同一个 existing 实例,并再次将其追加到 updated_records 中。

这会导致:

  • updated_records 中存在对同一个 CollectedRecord 实例的重复引用。
  • 返回的 new_records + updated_records 包含重复元素,导致下游(如 AI 处理、通知发送)对同一条记录进行多次重复处理,且 accepted 计数不准确。

解决方案
应该将 seen_identities_in_batch 的去重检查移到 existing is not None 判断之前。这样,批次中后续重复的 identity 会被直接作为 duplicate 跳过。

        if identity is not None:
            if identity in seen_identities_in_batch:
                # Two triples in this same batch share an identity (e.g. a
                # feed listed the same entry twice) — keep the first, skip
                # the rest rather than fight over which one "wins".
                skipped += 1
                continue
            seen_identities_in_batch.add(identity)

            existing = existing_by_identity.get(identity)
            if existing is not None:
                if existing.content_hash == content_hash:
                    # Same source-native item, content unchanged: a genuine
                    # duplicate, same as the content_hash-only path always did.
                    skipped += 1
                else:
                    # Same source-native item, content changed (e.g. a feed
                    # fixed a title typo) — update in place instead of
                    # inserting a duplicate row (C7's fix).
                    existing.raw_data = raw
                    existing.normalized_data = normalized
                    existing.content_hash = content_hash
                    existing.status = "normalized"
                    existing.ai_enrichment = None
                    updated_records.append(existing)
                continue

@@ -0,0 +1 @@
{"session_id":"b4f7d02c-1da1-4b4d-9835-33e44401355e","context_samples":[{"ts_ms":1783878055117,"tokens":109500},{"ts_ms":1783878057926,"tokens":109939},{"ts_ms":1783878059350,"tokens":109939},{"ts_ms":1783878062169,"tokens":109939},{"ts_ms":1783878065137,"tokens":112312},{"ts_ms":1783878066527,"tokens":112312},{"ts_ms":1783878069643,"tokens":112437},{"ts_ms":1783878070036,"tokens":112437},{"ts_ms":1783878070541,"tokens":112437},{"ts_ms":1783878071081,"tokens":112437},{"ts_ms":1783878077649,"tokens":113096},{"ts_ms":1783878078111,"tokens":113096},{"ts_ms":1783878083683,"tokens":113513},{"ts_ms":1783878084794,"tokens":113513},{"ts_ms":1783878091897,"tokens":114044},{"ts_ms":1783878093626,"tokens":114044},{"ts_ms":1783878095116,"tokens":114044},{"ts_ms":1783878100551,"tokens":114749},{"ts_ms":1783878117149,"tokens":114749},{"ts_ms":1783878475480,"tokens":116909},{"ts_ms":1783878477603,"tokens":116909},{"ts_ms":1783878481720,"tokens":118189},{"ts_ms":1783878483660,"tokens":118189},{"ts_ms":1783878488025,"tokens":118636},{"ts_ms":1783878488449,"tokens":118636},{"ts_ms":1783878495026,"tokens":119031},{"ts_ms":1783878514349,"tokens":119031},{"ts_ms":1783878521236,"tokens":121158},{"ts_ms":1783878586833,"tokens":121464},{"ts_ms":1783878587475,"tokens":121464},{"ts_ms":1783878597857,"tokens":121836},{"ts_ms":1783878604356,"tokens":122080},{"ts_ms":1783878604705,"tokens":122080},{"ts_ms":1783878614717,"tokens":122341},{"ts_ms":1783878621949,"tokens":122341},{"ts_ms":1783878630484,"tokens":123658}],"last_updated_ms":1783878630484,"session_start_tokens":72839} No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

这些 .omc/state/ 目录下的 session 缓存文件是本地运行状态,不应该提交到版本控制中。建议将 .omc/ 目录添加到 .gitignore 中,并从 Git 中移除 these 文件。

@2233admin
2233admin merged commit dd8ef68 into main Jul 19, 2026
5 checks passed
@2233admin
2233admin deleted the fix/collection-misc branch July 19, 2026 03:25
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.

1 participant