Repository navigation
fix(graph-rs): run pagerank iteration counts past u32::MAX like Python - #3441
Conversation
pagerank converted its iteration count with `v.max(0) as u32`, so a count above u32::MAX wrapped: CALL pagerank.get(4294967297) ran one iteration on the Rust path, while the Python reference's range() runs until the ranks converge. Count in u64, keeping max(0) so that a negative count still runs no iteration.
This comment has been minimized.
This comment has been minimized.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the
Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Iteration count handling and validation rust/cgr_graph_rs/src/lib.rs, rust/cgr_graph_rs/tests/algorithms.rs |
The default and parsed nonnegative iteration counts use u64. Tests compare results for (1 << 32) + 1 and 1000 iterations, and check uniform ranks for a negative count. |
Package version alignment rust/cgr_graph_rs/Cargo.toml, rust/cgr_graph_rs/pyproject.toml, pyproject.toml, codebase_rag/tests/test_rust_extension_packaging.py |
The Rust package versions and minimum cgr-graph-rs dependency version change to 0.1.4. Packaging tests expect the six platform wheels and source archive at version 0.1.4. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix
Suggested labels: bug
Suggested reviewers: claude
Merge Risk: 
Minimal · up to f18a2
-
No concrete merge-blocking defect or unresolved active risk is established by the reviewed changes.
Pre-merge checks: 
4
-
Full details: Description check
-
Explanation
The description includes all required sections, explains the PageRank fix, documents the version updates, and lists tests. The unchecked full pre-commit item is non-critical because individual validation checks are listed as clean.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- 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.
Comment @coderabbitai help to get the list of available commands.
[Medium impact] The PR appears safe to merge. SummaryPageRank now preserves iteration counts above
Reviews (2) · Last reviewed commit: "chore(graph-rs): bump cgr-graph-rs to 0...." · Reviewed by Greptile |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVdycfDFvbqaoyHWVWHJ1N
The version bump set cgr-graph-rs to 0.1.1 in rust/cgr_graph_rs/pyproject.toml and in the requirement floor, but left the crate's own Cargo.toml (and its Cargo.lock entry) at 0.1.0, so test_the_crate_has_one_version failed with '0.1.0' == '0.1.1'. Both manifests now carry 0.1.1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVdycfDFvbqaoyHWVWHJ1N
Main reached 0.1.3 with #3495, so the pagerank fix needs a version above it for publish.yml to ship the changed library. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ChfULvr1gXZn6MQXr4xNFS
|
|
@greptile review Generated by Claude Code |



Summary
pagerankincgr_graph_rsconverted its iteration count withv.max(0) as u32, so a count aboveu32::MAXwrapped:CALL pagerank.get(4294967297, 0.85, 1e-12)ran one iteration on the Rust path, while the Python reference (for _ in range(iterations)) runs until the ranks converge. The two paths returned different ranks.u64.max(0)stays, so a negative count still runs no iteration, asrange()of a negative number does.cgr-graph-rsto 0.1.4 (main is at 0.1.3 since perf(indexer): compute interface fingerprints in Rust on the tree bridge #3495), inrust/cgr_graph_rs/pyproject.toml,rust/cgr_graph_rs/Cargo.toml,rust/cgr_graph_rs/Cargo.lock, the requirement inpyproject.tomlanduv.lock, so the fixed library reaches users. The wheel names intest_rust_extension_packaging.pyfollow the new version.Demo
No user-visible change: the Rust and Python procedures now agree on iteration counts above
u32::MAX.Type of Change
Related Issues
None.
Test Plan
rust/cgr_graph_rs/tests/algorithms.rs):pagerank_runs_an_iteration_count_past_u32_max: a count of 2^32 + 1 gives the same ranks as a count of 1000 (both converge at a tolerance of 1e-12). Fails before the fix: the ranks after one iteration (0.25, 0.356, 0.25, 0.144).pagerank_runs_no_iteration_for_a_negative_count(negative test): a count of -5 leaves the ranks uniform. Fails ifmax(0)is removed from the new conversion, because -5 then casts to a huge count.cargo test --release --manifest-path rust/cgr_graph_rs/Cargo.tomlon f18a299 (main merged in): all pass (54 unit, 9 inalgorithms.rs).test_rust_extension_packaging.pyandtest_native_interface.py: 708 passed.uv lock --check,ruff check,ruff format --check,scripts/generate_rust_licenses.py --check, and the "Crate version bumped with its code" comparison (0.1.3 on main, 0.1.4 here): clean.Checklist
!in the title and its release notes inchanges/breaking/<PR number>.mdmake pre-commit)# type: ignore,cast(),Any, orobjecttype hintsSummary by CodeRabbit