Skip to content

fix(import): resolve graph-index.json race condition in batch import - #67

Merged
jeff-r2026 merged 1 commit into
Tencent:mainfrom
m0Nst3r873:fix/graph-merge-race
Jun 30, 2026
Merged

fix(import): resolve graph-index.json race condition in batch import#67
jeff-r2026 merged 1 commit into
Tencent:mainfrom
m0Nst3r873:fix/graph-merge-race

Conversation

@m0Nst3r873

@m0Nst3r873 m0Nst3r873 commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes critical data loss in teamai import --from-org / --from-repo-list / --from-repo when graph-index.json is written concurrently or without full aggregation.

Problem

When import-repo-list dispatches multiple importFromRepo calls concurrently (default concurrency=3), the global graph-index.json read-merge-write cycle has no mutual exclusion. The last writer wins, silently discarding other repos' nodes/edges. Result: only the first repo's data survives in the global graph.

Fix: batch-then-aggregate strategy + shared aggregation

  1. Per-repo isolation: each importFromRepo now only writes its own graph to evidence/<slug>/.indices/
  2. Shared aggregateGlobalGraph() (src/graph-aggregate.ts): serially merges all per-repo graphs into one global graph-index.json with cross-repo edge detection
  3. All import modes covered: single-repo aggregates after write; batch aggregates after all repos complete
  4. Single push: graph + aggregate files committed in one push at the end

Review feedback addressed

Issue Status
P1 🔴 detect/merge 顺序回归 (先 merge 后 detect 导致 self-match 噪声) ✅ 修复为先 detect 再 merge
P2-1 🟠 push 在 regenerateAggregate 之前 ✅ push 移到 step 6 统一一次
P2-2 🟠 skipAutoPush 误伤 deep-enrich ✅ deep-enrich 始终执行,仅 push 受控
P2-3 🟠 aggregateGlobalGraph 无测试 ✅ 4 个集成测试含 P1 回归用例
P3-1 🟡 push 路径来源不一致 ✅ 统一从 autoDetectInit() 获取
P3-2 🟡 _newProject 参数未使用 ✅ 已删除
P3-3 🟡 单仓全量重聚合 O(n) ✅ JSDoc 注明限制,>50 仓后需增量优化
P3-4 🟡 as GraphIndex['edges'] 类型断言 ✅ 返回类型改为字面量 'DEPENDS_ON',断言已移除

Additional fixes (from E2E testing)

  • AI 超时保护: DEFAULT_TIMEOUT_MS 从 1200s → 120s + 超时诊断日志
  • --skip-enrich 选项: CLI + import-repo + codebase-extract,跳过 AI 只做 extract+graph
  • autoPushTeamRepo: log.warn on failure instead of silent catch {}
  • explicitDomain from whitelist now overrides AI-inferred _domains.json
  • detectCrossRepoEdges: compatible with both id/label/file and slug/title/type node formats
  • README: 中英文文档新增 --skip-enrich 选项说明

E2E verification (TGit group 378710, 11 repos)

Metric Before (issue report) After
Global nodes 14 1179
Global edges 1 780
Cross-repo DEPENDS_ON 0 Functional
Repos in global graph 1 11

Test plan

  • npx tsc --noEmit — zero errors
  • npx vitest run — 1587 passed (117 files)
  • aggregateGlobalGraph: 4 integration tests (merge, cross-repo edges, P1 regression, empty dir)
  • detectCrossRepoEdges: 4 unit tests (slug/title, config match, mixed format, no-match)
  • E2E: import --from-org git.woa.com/378710 — 11 repos, 1179 nodes, 780 edges
  • --skip-enrich: import 跳过 AI 调用,仅做 extract+graph

@m0Nst3r873
m0Nst3r873 force-pushed the fix/graph-merge-race branch from 2f8ba1f to 6cb25d8 Compare June 29, 2026 11:23
@jeff-r2026

Copy link
Copy Markdown
Collaborator

Code Review

概览

PR 解决了一个真实且严重的数据丢失问题:批量 import 并发写全局 graph-index.json,last-writer-wins 导致只有第一个仓库存活。修复策略(per-repo 隔离写入 + 批量结束后串行 aggregateGlobalGraph() 聚合)架构上是正确的,E2E 数据有说服力。但聚合逻辑有一个正确性回归,外加几处时序/语义问题需确认。


🔴 P1 — 聚合时「先 merge 后 detect」会把单仓内部依赖误判为跨仓边

src/graph-aggregate.ts

globalGraph = mergeGraphs(globalGraph, overlay);
const crossEdges = detectCrossRepoEdges(overlay, globalGraph, dir.name);  // ⚠️ globalGraph 已含 overlay 节点

被删除的旧代码顺序相反——先 detect 再 merge,existing 不含 overlay 节点:

const crossRepoEdges = detectCrossRepoEdges(overlay, existing ?? {...}, slug);  // existing = merge 前
const merged2 = existing ? mergeGraphs(existing, overlay) : overlay;

detectCrossRepoEdges 第一段会遍历 overlayimports 边,把 edge.to 文件名转 PascalCase 去 existingIndex 找同名组件。现在 existing 已含 overlay 自己的全部节点,于是 overlay 内部的 import 关系会匹配到 overlay 自己的组件,被错误标成跨仓 DEPENDS_ON(第二段遍历 existing.edges 同理)。

后果:全局图被单仓内部依赖污染,边数虚高——宣称的 780 edges 很可能混入大量这种自匹配噪声

修复(保留 merge 前的引用再检测):

if (globalGraph) {
    const crossEdges = detectCrossRepoEdges(overlay, globalGraph, dir.name);  // 先用 merge 前的
    globalGraph = mergeGraphs(globalGraph, overlay);
    if (crossEdges.length > 0) {
        globalGraph.edges.push(...crossEdges as GraphIndex['edges']);
    }
} else {
    globalGraph = overlay;
}

🟠 P2 — 需确认

1. batch 模式:聚合 markdown 在 push 之后才重建,落后一次未推送

import-repo-list.ts 流程:

Promise.all(inFlight)
→ aggregateGlobalGraph() + autoPushTeamRepo(...)   // push 在这里
→ "5. 重建聚合文件" regenerateAggregate()           // 生成 domain-*.md / index.md,之后无 push

本轮重建的 index.md / domain-*.md 不会被这次 push 带上,要等下一次 import 才推送。建议把全局图聚合放到 regenerateAggregate 之后统一一次 push。

2. skipAutoPush 被复用来跳过后台深度生成,语义被放大

import-repo.ts 第 5b 步:

if (!dryRun && !skipAutoPush && teamwikiRoot) {  // 后台深度生成也被 skipAutoPush 关掉了

skipAutoPush 字面只是「跳过 push」,但这里连带让 batch 模式下所有仓库都不再触发后台深度生成。这是未在 PR 描述中说明的行为变化。若有意为之,建议改名 batchMode 或单列 skipBackgroundDeepGen 让意图显式;若误伤需恢复。请确认。

3. 测试只覆盖纯函数,未覆盖聚合本身

cross-repo-edges.test.tsdetectCrossRepoEdges 的格式兼容覆盖得不错 👍。但 aggregateGlobalGraph() 完全没有测试,而 P1 回归恰发生在聚合层。建议补一个集成测试:构造 2~3 个 per-repo graph,断言聚合后节点/边数正确,且单仓内部 import 不应产生跨仓 DEPENDS_ON 边(正好回归测 P1)。


🟡 P3 / 观察

  • push 路径来源不一致:单仓走 path.join(process.cwd(), '.teamai', 'team-repo'),batch 走 lc.repo.localPath。通常同一目录,但来源不同,建议统一从 autoDetectInit() 取。
  • detectCrossRepoEdges 第三参数 _newProject 始终未使用,传进去是空载,可删或确认是否本应参与去重 key。
  • 单仓 import 每次全量重聚合 evidence/code 下所有仓库:正确性 OK,但仓库多时是 O(n) 重复 I/O,留意后续规模增长。
  • ...crossEdges as GraphIndex['edges'] 类型断言绕过了类型检查,结构漂移不会被编译器发现,属技术债。

正向亮点

  • batch-then-aggregate 是解决并发竞态的正确架构。
  • 节点格式兼容(id/label/fileslug/title/type)处理干净,helper 抽取可读性好。
  • autoPushTeamRepo 从静默 catch {} 改为 log.warn,符合「不静默吞错」约定。
  • explicitDomain 覆盖 _domains.json 逻辑合理;E2E 验证数据详实。

结论

REQUEST_CHANGES — 主因是 P1 的 detect/merge 顺序回归,直接影响本 PR 核心产物(全局图边集)的正确性并使指标失真。修掉 P1 + 补一个覆盖聚合路径的测试,并确认 P2 的两处行为后即可合入。

@m0Nst3r873
m0Nst3r873 force-pushed the fix/graph-merge-race branch 2 times, most recently from f452772 to abc6390 Compare June 30, 2026 02:52
When import-repo-list runs with concurrency=3, multiple importFromRepo
calls read-merge-write the global graph-index.json concurrently. The
last writer wins, silently discarding other repos' nodes/edges.

Fix: batch-then-aggregate strategy
- Each repo now only writes its own graph to evidence/<slug>/.indices/
- After all repos complete, import-repo-list serially aggregates all
  per-repo graphs into the global graph-index.json in one pass
- Cross-repo edge detection runs during aggregation with full context
- A single autoPushTeamRepo call at the end replaces per-repo pushes

Additional fixes:
- autoPushTeamRepo: log.warn on failure instead of silent catch
- explicitDomain from whitelist now overrides AI-inferred _domains.json
- Add skipAutoPush option to suppress per-repo push in batch mode

Closes Tencent/teamai-cli#(graph-race)
@m0Nst3r873
m0Nst3r873 force-pushed the fix/graph-merge-race branch from abc6390 to 671218e Compare June 30, 2026 02:59
@jeff-r2026
jeff-r2026 merged commit a4a344e into Tencent:main Jun 30, 2026
7 checks passed
@m0Nst3r873
m0Nst3r873 deleted the fix/graph-merge-race branch June 30, 2026 03:58
@hsuchifeng hsuchifeng mentioned this pull request Jul 3, 2026
3 tasks
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.

2 participants