fix(p2p): raise DAG-walk iteration ceiling above 20

[iverc]

Follow-up to #1085 (Linux P2P sync stalls).

poll_fetch_dag's selective block-fetch walk was capped at 20 iterations. Each iteration only reveals one more layer of missing blocks, so a document whose missing history is deeper than 20 commits got abandoned unmerged — even though the walk was still fetching a block every pass. It only recovers on the next update to that doc or a node restart, which under Linux CI load reads as a reconciliation stall.

The !made_progress break is the actual stop condition (it exits the moment a pass fetches nothing). The 0..20 was just an arbitrary depth limit layered on top, so I replaced it with a high defensive ceiling.

Deterministic regression test added: a 25-deep chain that reconciles via the selective path (fails on the old cap, passes now). Validated on Linux and macOS; full p2p lib suite green.

Left out of scope: the 10s/30s fetch timeouts and the lack of an in-process retry for a quiescent stranded doc — tracking those separately rather than tuning constants here.

[coderabbitai]

📝 Walkthrough

Walkthrough

Replaces the hardcoded 0..20 iteration cap in the selective DAG-walk loop of poll_fetch_dag with a new MAX_DAG_WALK_ITERATIONS constant, allowing deeper DAG reconciliation while progress continues. Adds a regression test covering a depth-25 DAG scenario.

Changes

DAG Walk Iteration Cap

Layer / File(s)	Summary
Constant and loop update crates/p2p/src/sync/coordinator/dag_fetcher.rs	Adds MAX_DAG_WALK_ITERATIONS constant and updates the selective DAG-walk loop to iterate up to it instead of the fixed 0..20 range, keeping existing termination conditions.
Regression test crates/p2p/src/sync/coordinator/dag_fetcher.rs	Adds poll_fetch_dag_completes_dag_deeper_than_legacy_iteration_cap test verifying full reconciliation and DagReady emission for a depth-25 linear DAG.

Estimated code review effort: 1 (Trivial) | ~5 minutes

---

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

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

[coderabbitai]

🧹 Nitpick comments (1)

crates/p2p/src/sync/coordinator/dag_fetcher.rs (1)

311-872: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Test module pushes this file well past the 400-line guideline threshold.

The file (test module alone spans ~560 lines) exceeds the "consider splitting if over 400 lines" guidance. Consider extracting the tests module into a separate dag_fetcher/tests.rs (or similar) file to keep dag_fetcher.rs focused on the fetch logic.

As per coding guidelines, "Organize code into one concept per file, preferring small files over large files. Keep files under 200 lines; check consistency if 200-400 lines; consider splitting if over 400 lines."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/p2p/src/sync/coordinator/dag_fetcher.rs` around lines 311 - 872, The
inline tests in tests::mod make dag_fetcher.rs exceed the file-size guideline,
so move the entire test module into a dedicated test file and keep the fetcher
focused on production logic. Extract the existing tests around poll_fetch_dag
and the TestTransport helper into dag_fetcher/tests.rs (or an equivalent sibling
test module), preserving all imports and helpers so the test coverage and
symbols like poll_fetch_dag and TestTransport remain unchanged.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/p2p/src/sync/coordinator/dag_fetcher.rs`:
- Around line 311-872: The inline tests in tests::mod make dag_fetcher.rs exceed
the file-size guideline, so move the entire test module into a dedicated test
file and keep the fetcher focused on production logic. Extract the existing
tests around poll_fetch_dag and the TestTransport helper into
dag_fetcher/tests.rs (or an equivalent sibling test module), preserving all
imports and helpers so the test coverage and symbols like poll_fetch_dag and
TestTransport remain unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cdbf5f09-7b76-4753-91c3-c792c613ec0d

📥 Commits

Reviewing files that changed from the base of the PR and between 08c14cb and d1cdc8d.

📒 Files selected for processing (1)

• crates/p2p/src/sync/coordinator/dag_fetcher.rs

📜 Review details

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use self-documenting code with minimal comments - only comment non-obvious WHY, safety invariants, and public API docs with ///. Do not include what the code does, TODO/FIXME comments, commented-out code, or change history
Name files and modules using snake_case (e.g., lww.rs)
Name types using PascalCase (e.g., LwwDelta)
Name functions using snake_case (e.g., encode_priority())
Name constants using SCREAMING_SNAKE_CASE (e.g., MAX_PRIORITY)
Organize code into one concept per file, preferring small files over large files. Keep files under 200 lines; check consistency if 200-400 lines; consider splitting if over 400 lines
Do not include commented-out code in the codebase
Use issues instead of TODO or FIXME comments - do not include TODO/FIXME comments in code

Files:

• crates/p2p/src/sync/coordinator/dag_fetcher.rs

🔇 Additional comments (2)

crates/p2p/src/sync/coordinator/dag_fetcher.rs (2)

21-32: LGTM!

Also applies to: 111-114

---

801-871: Solid regression test; logic and assertions correctly validate the fix.

Depth-25 chain, DEPTH-1 expected selective-fetch batches, full blockstore reconciliation, and DagReady emission are all consistent with the loop semantics in poll_fetch_dag.