Repository navigation
#428 — feat(domain): 1-hop context halo for disconnected modified models in the PR-scope mini-DAG - #434
Conversation
…the PR-scope mini-DAG A modified model with no connector path to any OTHER modified model is "disconnected" and now pulls its immediate depends_on parents + direct children into the mini-DAG as quiet, is_halo-flagged context nodes — so an isolated change is shown with its neighbors rather than alone. A connected cluster gets no halo; a disconnected model with no neighbors still renders alone. Domain (src/domain/pr_dag.rs): - New is_halo: bool on PrDagNode (#[serde(default, skip_serializing_if = "std::ops::Not::not")] → halo-free goldens stay byte-identical). - halo_for_disconnected + is_connected_to_another_seed + collect_halo_neighbors: a seed is connected iff another seed is reachable forward/backward over the model graph; disconnected seeds earn a halo. Dedup: a neighbor that is itself a seed or connector keeps its stronger role. - Halo nodes induce edges ONLY to/from their anchor (merge_halo_edges); induced_edges excludes halo nodes from the generic pass so no spurious halo↔halo links. Graph stays acyclic + BTreeSet-deterministic. - Exhaustive enumeration tests: connected cluster (no halo), single isolated with neighbors (halo), isolated with no neighbors (alone), two independent halos, halo-node-is-modified/connector dedup, shared halo, no halo↔halo bleed, determinism + acyclicity. Render (honest descriptor only — #429 owns the engine-aware halo render): - PrDagPayload.halo_count counts halo in its own dimmed-context tier so the descriptor reads "1 modified · 3 context" not a misleading "4 modified". - report.html: data-halo + "N context models" wording + prdag_halo dimmed classDef. Goldens: diff-showcase shifts (playground's fct_provider_metrics is disconnected → 3 parent halo nodes); prdiff-minidag + seed-showcase shift cosmetically (new classDef + descriptor wording, is_halo omitted for halo-free nodes). All regenerated + byte-verified. crap4rs: compute_pr_dag CRAP 1.0, halo_for_disconnected 5.0 — strict ≤15 held. Closes #428 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 39 minutes and 41 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 (6)
📝 Walkthrough
WalkthroughAdds a "halo" node role to the PR-scope lineage mini-DAG ( ChangesHalo Nodes for Disconnected Modified Models in PR-scope Mini-DAG
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
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 6 individual chapters for you: Chapters generated by Stage for commit 1538369 on Jun 14, 2026 5:02pm UTC. |
There was a problem hiding this comment.
Code Review
This pull request introduces a 1-hop context halo feature for disconnected modified models in the PR DAG, allowing isolated changes to be rendered with their immediate neighbors. It updates the payload structures, DAG computation logic, HTML reporting templates, and adds comprehensive unit tests. The review feedback suggests simplifying the induced_edges function by removing the redundant halo parameter and directly filtering nodes using the is_halo flag, as well as adopting BTreeSet<String> for more idiomatic string deduplication in Rust.
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.
📄 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 27505903992 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.htmlPosted by |
# Conflicts: # examples/diff-showcase-report.html # examples/prdiff-minidag-report.html # examples/seed-showcase-report.html
… (derive from is_halo) The halo parameter passed to induced_edges was redundant: every PrDagNode already carries is_halo, and the node set is fully assembled (all is_halo set) before induced_edges runs. Derive the non-halo in-set directly from the node flags instead of threading a separate halo set. Keeps the in-set keys borrowed &str (BTreeSet<&str> from n.id.as_str()) — no per-id String allocation, per the no-alloc house style. Emitted edge set is unchanged; induced_edges_are_exactly_the_model_edges_among_the_node_set and all pr_dag tests pass unchanged. Folds the accepted gemini review simplification on PR #434. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Merge debrief (#434 → #428) — squash Shipped: 1-hop context halo for disconnected modified models in the PR-scope mini-DAG. A changed model with no connector path to any other changed model now pulls its immediate parents+children as quiet Justified scope deviation: the brief said domain-only, but a domain change shifts the diff-showcase golden — committing it as-is would bake a false '4 modified' descriptor (3 are halo). The builder added the minimal-honest render companion ( Bot disposition: 2 gemini medium on Merge mechanics: render.rs + report.html auto-merged with #423 (disjoint regions — chips + halo coexist, verified); 3 report goldens regenerated; a 2nd clean update pulled in #432. All gates green (2026 lib + headless 114 + zero-egress 12 over regenerated goldens + crap4rs strict ≤15 on pr_dag.rs). Follow-up: #429/#430 (Models-tab DAG relocation + engine-aware + filter-reactive) build on this halo. |
Summary
Adds the 1-hop context halo for disconnected modified models to
compute_pr_dag(src/domain/pr_dag.rs). A modified model with no connector path to any other modified model is now disconnected, and its immediatedepends_onparents + direct children join the mini-DAG as quiet,is_halo-flagged context nodes — so an isolated change is shown with its neighbors rather than alone (the module doc previously excluded 1-hop, rendering an isolated model alone).This is epic #427 slice A (the domain graph-computation change). The engine-aware, Models-tab, filter-reactive halo render is slice B (#429), which is blocked-by this.
What changed
Domain (
src/domain/pr_dag.rs):is_halo: boolonPrDagNode,#[serde(default, skip_serializing_if = "std::ops::Not::not")]so halo-free goldens stay byte-identical (the established golden-stability serde pattern).halo_for_disconnected+is_connected_to_another_seed+collect_halo_neighbors: a seed is connected iff some other seed is reachable forward/backward over the model→model lineage; a disconnected seed earns a halo. Dedup: a neighbor that is itself a seed or connector keeps its stronger role.merge_halo_edges); the genericinduced_edgespass excludes halo nodes, so no spurious halo↔halo links between two independent isolated anchors. Graph stays acyclic +BTreeSet-deterministic.Render — honest descriptor only (the minimal companion; #429 owns the styling):
PrDagPayload.halo_countcounts halo nodes in their own dimmed-context tier so the descriptor reads "1 modified · 3 context" instead of a misleading "4 modified".templates/report.html:data-haloattr + "N context models" wording + a dimmedprdag_haloMermaid classDef.Role-encoding choice
New
is_haloflag, not reusedis_connector. A connector lies between two modified models; a halo node is context for a single isolated one — a distinct role. Reusingis_connectorwould polluteconnector_countand conflate the two for #429. The flag is serde-skipped when false, keeping it byte-neutral where no halo exists.Goldens
fct_provider_metricsis a disconnected modified model, so it gains 3 parent halo nodes (dim_date,fct_encounters,stg_synthea__procedures). This dogfoods the halo in the canonical showcase. Descriptor now reads "1 modified model · 3 context models".prdag_haloclassDef + descriptor wording +halo_countJSON field;is_halois omitted for their halo-free nodes.All goldens regenerated and byte-verified; headless zero-egress (incl. Cytoscape engine) passes against the regenerated examples.
Gates
cargo fmt --check,clippy --all-targets --locked -D warnings,nextest(2255 pass),cargo doc -D warnings,cargo deny,domain_clean_arch,resource_ref_lint, BDD (226 scenarios), headless zero-egress + mini-DAG toggle — all green. crap4rs (strict ≤15 onpr_dag.rs):compute_pr_dagCRAP 1.0,halo_for_disconnected5.0, max-in-file 11.Closes #428
🤖 Generated with Claude Code
Summary by CodeRabbit