Repository navigation
#352 — domain: PR-scope DAG node + connector computation (Slice A) - #357
Conversation
Add `src/domain/pr_dag.rs` — a pure-domain function computing the PR-scope lineage mini-DAG: the modified set unioned with the connectors between distinct modified models, plus DELETED ghosts. - Node set = modified ∪ connectors ∪ removed. Connectors = (descendants(M) ∩ ancestors(M)) \ M over the model→model `depends_on` graph, keeping only nodes reached forward from one seed and backward from a different seed (the differing-seed "between" predicate). An isolated modified model renders alone; a convergence sink (downstream-of-both, between-neither) is correctly excluded. - Multi-source BFS (one shared queue + visited set per direction) collapses the naive O(|M| × (N+E)) to O(N+E), mirroring the governance reverse-adjacency precompute-once idiom. - Edges = the induced model→model subgraph (both endpoints in-set), self-edge-guarded, deterministically ordered (BTreeSet), acyclic. - Per-node state reuses the scope taxonomy: New / Modified / Deleted, plus an `is_connector` quiet-tier marker. Consumes the existing modified set (ModelInScopeSet) and the caller-derived removed set — no comparator / state.rs / scope.rs rewrite. - Serialize-only render PODs (PrDagGraph / PrDagNode / PrDagEdge); versioned-id-safe `bare_name` via Node::bare_name (fallback strips a trailing vN for removed nodes absent from the current manifest). Property tests cover every Slice-A invariant: connector-between-two, 1-hop-context excluded, convergence-sink excluded, isolated-alone, exact induced-edge set, DELETED ghosts, model-only projection, NEW/MODIFIED taxonomy, acyclicity, determinism, and a brute-force pairwise-intersection oracle for the multi-source BFS. `lines ±` per node is deliberately out of this slice (needs the diff; later render-side concern). Domain-pure (std + serde derive only). crap4rs strict <15 keyed for the new module (worst fn 11.00). Part of #352 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 40 minutes and 59 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces ChangesPR-scope lineage mini-DAG computation
Sequence DiagramsequenceDiagram
participant Caller
participant compute_pr_dag
participant ModelAdjacency
participant connectors_between
participant reach
participant assemble_nodes
Caller->>compute_pr_dag: manifest, modified set, new subset, removed list
compute_pr_dag->>ModelAdjacency: build forward/backward adjacency
ModelAdjacency-->>compute_pr_dag: model→model edge maps
compute_pr_dag->>connectors_between: adjacency, modified seeds
connectors_between->>reach: forward reachability from seeds
reach-->>connectors_between: reachable nodes per seed
connectors_between->>reach: backward reachability from seeds
reach-->>connectors_between: backward-reachable nodes per seed
connectors_between-->>compute_pr_dag: connector node ids
compute_pr_dag->>assemble_nodes: seeds, connectors, removed ids
assemble_nodes-->>compute_pr_dag: deterministic node collection
compute_pr_dag-->>Caller: PrDagGraph (nodes, edges)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related Issues
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Ready to review this PR? Stage has broken it down into 3 individual chapters for you:
Chapters generated by Stage for commit c762135 on Jun 13, 2026 4:12pm UTC. |
📄 Rendered report previewAll golden examples regenerated cleanly. 🟡 Golden examplesCommitted to
🐶 Live dogfood previewThis PR doesn't touch 🧭 Explore previewThe two-page 🟡 Golden exploreThe committed
🐶 Live exploreThis PR doesn't touch ▶ Open ↗ opens the report or explorer in your browser in one The Pages preview may take ~1 min to update after this comment Alternative: GitHub CLI# gh CLI >= 2.63 extracts into ./report-preview-playground/.
gh run download 27472025225 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.htmlPosted by |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new module src/domain/pr_dag.rs to compute a focused, PR-scope cross-model lineage mini-DAG that highlights modified, connector, and deleted models. Feedback on the implementation focuses on performance optimizations and codebase consistency: avoiding heap allocations by querying the manifest directly instead of building a temporary BTreeSet in ModelAdjacency::build, replacing VecDeque with a Vec and manual index tracking in reach to match established idioms, optimizing has_distinct_pair to O(1) complexity, and utilizing binary search on the sorted nodes slice in induced_edges to eliminate another temporary BTreeSet allocation.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/domain/pr_dag.rs`:
- Around line 553-1018: Add exhaustive table-driven JSON serde round-trip tests
for the new serialized types PrDagState, PrDagNode, PrDagEdge, and PrDagGraph:
create unit tests in src/domain/pr_dag.rs that enumerate combinatorial cases
(all enum variants of PrDagState; nodes with/without is_connector, different
state values, name/id variations; edges with typical from/to and self-edge
guard; graphs with empty and populated nodes/edges, deleted/new/modified mixes)
and for each case serialize with serde_json::to_string and deserialize with
serde_json::from_str, asserting equality (or expected normalized equality where
name-stripping for removed versions applies) to validate round-trip and union
semantics; structure tests as table-driven vectors of cases and include explicit
checks for StateComparator/union-like behavior (e.g., New vs Modified
precedence) and ensure coverage of empty and edge-case payloads.
- Around line 272-293: The current reach function does a full BFS per seed
causing O(|seeds|*(N+E)) work; change it to a single multi-source BFS:
initialize a VecDeque queue seeded with (seed, seed) for each seed, pop
(current, origin_seed) and attempt to insert origin_seed into reached[current],
and if insertion succeeds push (neighbor, origin_seed) for each neighbor from
adjacency; stop propagation for that origin_seed when insertion fails. Update
the queue type and push/pop logic accordingly (instead of looping seeds and
running separate BFSs), keep reached as BTreeMap<&NodeId, BTreeSet<&NodeId>>,
and ensure adjacency access uses get(current).into_iter().flatten() or
unwrap_or(&empty_vec) to iterate neighbors safely. This yields O(N+E +
total_tags) instead of per-seed repetition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5bba79d-39ac-4d95-a94f-d734b0987056
📒 Files selected for processing (3)
crap4rs.tomlsrc/domain/mod.rssrc/domain/pr_dag.rs
Replace the O(|forward| × |backward|) nested-loop scan in has_distinct_pair with the provably-equivalent O(1) form (gemini on PR #357): either side empty ⇒ no pair; both singletons ⇒ distinct iff the lone elements differ (set inequality ⟺ element inequality for singletons); otherwise one side has ≥2 elements so a distinct pair is guaranteed. The predicate runs once per scanned node in the connector walk, so this matters on large PRs. Behaviorally identical — all 21 existing pr_dag invariant tests pass unchanged. Adds a focused unit test for the both-singleton-equal ({a} vs {a} ⇒ false), distinct-singleton ({a} vs {b} ⇒ true), asymmetric-multi ({a,b} vs {a} ⇒ true), and empty cases. Part of #352 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🧾 Merge debrief — #357 (
|
1 participant
Part of #352.
Slice A of the PR-scope lineage mini-DAG: the pure-domain graph core only. No render surface, no CLI wiring —
src/domain/pr_dag.rsplus itsmod.rsregistration and a crap4rs strict-threshold entry.What it computes
compute_pr_dag(manifest, modified, new, removed) -> PrDagGraph:modified ∪ connectors ∪ removed. Connectors =(descendants(M) ∩ ancestors(M)) \ Mover the model→modeldepends_ongraph, keeping only nodes reached forward from one seed and backward from a different seed (the differing-seed "between" predicate). An isolated modified model renders alone; a convergence sink (downstream-of-both, between-neither) is correctly excluded.is_connectorquiet-tier marker.Discipline
ModelInScopeSet) and the caller-derived removed set — no comparator /state.rs/scope.rsrewrite.O(|M| × (N+E))toO(N+E), mirroring the governance reverse-adjacency precompute-once idiom.std+ serde derive only).Serialize-only render PODs. Versioned-id-safebare_name(fallback strips a trailingvNfor removed nodes absent from the current manifest).lines ±per node is deliberately out of this slice (needs the diff; later render-side concern).Tests
21 property-style invariant tests covering every Slice-A invariant: connector-between-two, 1-hop-context excluded, convergence-sink excluded, isolated-alone, exact induced-edge set, DELETED ghosts, model-only projection, NEW/MODIFIED taxonomy, acyclicity (Kahn), determinism (insertion-order-independent), and a brute-force pairwise-intersection oracle for the multi-source BFS.
Gates
fmt, clippy
--all-targets --locked -D warnings, fullcargo nextest(1936 passed),cargo doc -D warnings,cargo deny check— all green. crap4rs strict<15keyed for the new module; worst function 11.00.🤖 Generated with Claude Code
Summary by CodeRabbit