[imbajin]

背景

cycle_detection 在 Hubble 上容易长时间运行甚至超时。除“枚举全部简单环本身可能指数增长”这一算法边界外，旧实现还存在确定性的正确性与实现开销：

• 递归 DFS 每深入一层都重建 path map，产生大量短命对象与 GC；
• 深层搜索持续扩容/复制 stack；
• min_length / max_length 按旧 stack 形态存在一位偏差；
• boolean 实际写回整数，却声明为 TEXT；
• canonical 判断在搜索热路径反复读取、比较外部字符串 ID；
• 过滤后的邻接未排序去重，第二轮仍发送无意义占位值。

Before → After

动图展示递归 DFS 到有界迭代搜索的变化：降低重复分配、消除递归栈风险、将字符串热路径改为整数 rank，并让 boolean 模式跳过全图排序；图片不进入 Git 历史。

Before
每个 root
  └─ recursive DFS
      ├─ 每层重建 visited map
      ├─ 每层 append/copy path
      └─ 热路径比较 external string ID

After
Step 1：过滤、排序去重并同步 incoming adjacency
                         ↓
all / limit：一次性建立 lexical rank
boolean：完全跳过 rank 与排序
                         ↓
Step 2：每个 compute partition 复用一套
        visited + path + iterative frames
                         ↓
all / limit：仅最小 canonical root 输出
boolean：发现首个合法环立即短路

核心设计

正确性

• cycle length 定义为环中实际顶点数，修复 off-by-one；
• 排除 self-loop，因此历史默认 min_length=0 归一化为 2；
• 校验 mode、长度边界、limit > 0 与 compute.max_step >= 2；
• all / limit 仍由外部字符串 ID 最小的顶点负责输出，保持既有去重与输出形状；
• boolean 对环上每个顶点独立判断，并在首个合法环处停止；
• cycle 统计升级为 Int64 aggregate；
• boolean 输出类型修正为 HugeGraph INT，列表模式保持 TEXT；
• 越界远端 ID 被忽略，重复顶点 walk 不会误报为 simple cycle。

性能与内存

• 使用迭代式有界 DFS，避免递归栈；
• 每个 compute.parallel 分区只创建一套可复用 visited/path/frames；
• 邻接排序去重，减少重复搜索分支；
• 第二轮 scatter 使用 zero-byte sentinel；
• all / limit 搜索热路径只比较 uint32 rank，不再反复比较字符串；
• rank 初始化直接读取 GraphData 中已有 external ID，不再复制 []string 头数组：排序阶段仅保留 4 B/vertex 临时顺序和 4 B/vertex rank；
• boolean 完全跳过 rank 数组与 O(V log V) 排序。

与 #31 的取舍

已逐项比较 #31 的实现与测试。#33 覆盖其实际环长、参数校验、canonical root、boolean 输出类型和 Int64 计数语义，并额外提供迭代 DFS、分区级状态复用、boolean 跳过全图排序、邻接去重与零字节 scatter。

#31 独有的防御性场景已纳入当前分支：

• 负数 cycle.min_length；
• 重复顶点 walk 拒绝；
• 越界远端 ID 不影响有效环。

#31 已在完成对比、测试迁移和当前 head 全绿后关闭；#33 为唯一保留方案。

复杂度边界

all 模式必须物化全部符合条件的简单环，结果数量在稠密图上可能指数增长，任何精确实现都不能保证固定运行时间。本 PR 消除实现层面的递归、重复分配、字符串热路径与无效通信，但不把全量枚举描述为有界任务。

生产/Hubble 场景应优先使用：

• boolean：只判断顶点是否位于任意环中；
• limit：限制每个 canonical root 的最大输出环数；
• 同时设置较小且业务合理的 cycle.max_length。

兼容性边界

1. boolean 输出 schema 从 HugeGraph TEXT 修正为 INT。若目标图已有同名 TEXT PropertyKey，需要新建 INT PropertyKey 或完成 schema 迁移。
2. cycle.max_length 现在严格表示环中的实际顶点数；依赖旧错误边界的任务结果会发生预期变化。

回归覆盖

• 参数归一化、负数与非法边界；
• external ID lexical rank；
• 邻接排序去重；
• 2-cycle、3-cycle 的精确长度与 canonical root；
• repeated-vertex walk 与越界 ID 防御；
• boolean 覆盖环上每个顶点、首环短路、无需 rank；
• limit=1 及 early-return 后 search state 复用；
• 完整 worker 两步生命周期、Int64 count、输出类型和 zero-byte scatter。

变更规模

• 5 files；
• 761 additions / 144 deletions；
• 当前基线为最新 master；
• 不改变算法名称和现有参数名；
• 低于 2,000 行 review 拆分阈值。

验证

当前 head：0c13afb

ORG exact-head checks 全部通过：

• Vermeer CI run 89：success；
• CodeQL run 108：success；
• license checker run 135：success。

ORG review threads 已全部处理并 resolve。

ASF apache#380 使用同一 head；其 Vermeer CI 71 / CodeQL 862 / license checker 525 当前仍在 ASF 队列中，不把 ORG 绿灯替代为 ASF 结论。

ASF upstream: apache#380

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 25 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 280bafe4-f54e-4699-9cf3-8e1235f5f772

Commits

Reviewing files that changed from the base of the PR and between 37d189e and 0c13afb.

Files selected for processing: 5

• vermeer/algorithms/cycle_boolean_rank_test.go
• vermeer/algorithms/cycle_detection.go
• vermeer/algorithms/cycle_detection_test.go
• vermeer/algorithms/cycle_search.go
• vermeer/algorithms/cycle_search_regression_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-05T17:29:09.665610Z	0c13afb	New commits

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[bitflicker64]

R1 inline item. The summary follows in the conversation.

[bitflicker64]

Review summary, R1 (FULL)

Superseded head: this round reviewed 72334f2e. The PR has since been rebased and is now at 0c13afb9, with two added perf commits on rank IDs. Nothing below covers the new head, and it needs a new review round.

Verdict: four reviewers found no confirmed defect introduced by this PR. One minor item is still uncertain. The PR is not ready to merge alongside #31 as-is (see below).
Reviewed: head 72334f2e, base d06a23b0. This was a full review of all 4 files.
Score: 9.9 / 10 (mean of 4 of 4 reports, final for 72334f2e).

Reviewer	Raw	Corrections	Adjusted
Reviewer A (model not reported)	10.0	none	10.0
Reviewer B (Gemini 3.8 Flash)	5.0	3 deductions removed, +4.5 (see below)	9.5
Reviewer C (claude-opus-5-5)	10.0	none	10.0
Reviewer D (gemini-4-argon)	7.0	3 deductions removed, +3.0 (see below)	10.0

Uncertain:

• Minor: up-front O(V) rank allocation in all/limit mode

Removed deductions (Reviewer B):

• maxLen clamping after the minLen > maxLen check. totalVertices is the global vertex count, so no cycle with at least min_length vertices can exist and an empty result is correct.
• Boolean output type switching from TEXT to INT. This is an intentional fix: the base wrote SUint8 but declared TEXT, and the PR description documents the migration.
• Zero-byte Unmarshal. Each scatter send carries one offset header, so the RecvScatter loop ends at len(data) and has no reachable trigger.

Docs note (unscored): the compatibility section mentions only the max_length shift. min_length also moves by one (with min_length=3, 3-vertex cycles are now included), so one more line there would help.

Removed deductions (Reviewer D):

• Non-numeric integer options fall back to defaults. This is not new: options.GetInt behaves the same way, and the base read these keys through it too.
• maxLen clamping order. This is the same root cause as Reviewer B's item, and the result is correct for the same reason.
• ranks bounds check. It cannot trigger, because every caller passes cycleRanks and inEdges that are both sized by TotalVertexCount().

Overlap with #31: both PRs share the same core semantics: cycle length counts actual vertices, the vertex with the smallest external ID owns each cycle, and boolean mode checks each vertex. They differ in neighbor ordering, which can change which cycles are kept when the limit truncates, and in validation and output type. A trial merge conflicts in cycle_detection.go and cycle_detection_test.go, so maintainers should choose one PR and fold the other's changes in by hand.

Checks: targeted go test ./algorithms (including -race) passes under Go 1.23.7. A 3000-graph randomized comparison against brute-force cycle enumeration matched exactly. Not run: a multi-worker cluster or an end-to-end HugeGraph schema migration test.

[imbajin]

最终复审结论（head 0c13afb）

评分：9.0 / 10｜审阅优先级：2 / 3｜ORG 阻塞项：无

当前方案以迭代式有界 DFS 替代递归搜索，复用每个 compute partition 的状态；boolean 跳过全图 lexical rank，all/limit 使用紧凑 uint32 rank 热路径。实际环长、canonical root、Int64 计数、boolean INT 输出、越界 ID 与 repeated-vertex 防御均有回归覆盖。

验证证据：

• Vermeer CI run 89 success，其中包含 go test -race ./algorithms；
• CodeQL run 108 success；
• license checker run 135 success；
• 当前 review threads 全部 resolved；
• [fix] Vermeer: make bounded cycle detection correct and allocation-safe #31 独有有效测试已迁入，本地替代 PR 已关闭。

非阻塞边界：all 模式的简单环结果数本身可能指数增长；boolean 输出由 TEXT 修正为 INT、环长语义修正均需要按描述处理 schema/结果兼容。

结论：ORG 侧已达到可提交上游终审状态；最终合入仍以 ASF apache#380 同 head checks 完成且无新增 review 为门槛。