Skip to content

fix(detectorkit): PropagateScopes stands down when the root is absent from the graph - #40

Merged
bomly-guy merged 1 commit into
mainfrom
fix/propagate-scopes-absent-root
Sep 5, 2026
Merged

fix(detectorkit): PropagateScopes stands down when the root is absent from the graph#40
bomly-guy merged 1 commit into
mainfrom
fix/propagate-scopes-absent-root

Conversation

@bomly-guy

@bomly-guy bomly-guy commented Sep 4, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Closes #38.

What was wrong

PropagateScopes discarded the error from g.DirectDependencies(rootID). When the root is not in the graph, the seeding loop is empty and the breadth-first walk never runs, but the closing pass still ran and stamped every unscoped dependency in the graph runtime. No scope had propagated from any root, and the caller got no signal. The result is wrong in the unsafe direction: development-only packages present as shipped.

Reproduced before the change with the snippet from the issue (dev.PrimaryScope() came back runtime).

The fix

An absent root is now a no-op for the whole pass: nothing is seeded, so nothing is defaulted. This is the minimal, non-breaking option from the issue. The function keeps its signature, since every current caller ignores scope-propagation failure and adding an error return is a deliberate API decision rather than a bug fix.

A root that is present with zero direct dependencies is deliberately unchanged. That lookup succeeds with an empty list, so the runtime default still applies, and TestPropagateScopesDefaultsOrphansToRuntime continues to pin that side. The new test pins both sides of the distinction in one place.

Verification

go test ./...
go vet ./...
gofmt -l .

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Scope propagation now safely does nothing when the specified root is absent from the graph, preventing unrelated dependencies from being incorrectly marked as runtime.
    • Existing behavior is preserved for roots present in the graph, including applying the runtime default when they have no direct dependencies.

  • Tests

    • Added coverage for absent roots and present roots without direct dependencies.

@bomly-guy @claude
fix(detectorkit): PropagateScopes stands down when the root is absent
616a48f 
The seeding lookup's error was discarded, so a root that is not in the
graph (most plausibly an ID held from before PromoteToModule replaced it)
seeded nothing, walked nothing, and still ran the closing runtime default
over every unscoped dependency. Development-only packages presented as
shipped, silently.

An absent root is now a no-op for the whole pass. A root that is present
with zero direct dependencies is unchanged: that is not an error, and the
default still applies, which the existing orphan test pins.

Closes #38

Co-Authored-By: Claude Fable 5.1 <[email protected]>
@coderabbitai

coderabbitai Bot commented Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: b304b8d3-6a3b-459e-ac70-c08df87c8f93

📥 Commits

Reviewing files that changed from the base of the PR and between 1624880 and 616a48f.

📒 Files selected for processing (2)
  • detectorkit/nodes.go
  • detectorkit/nodes_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

PropagateScopes now returns without applying runtime defaults when the root is absent. Tests verify this behavior and preserve runtime defaulting for a present root with no direct dependencies.

Changes

Scope propagation behavior

Layer / File(s) Summary
Guard absent roots
detectorkit/nodes.go		 PropagateScopes checks the direct-dependency lookup error and returns early when the root is absent. Its comment documents the distinction from a present root with no dependencies.
Validate root handling
detectorkit/nodes_test.go		 The test confirms that an absent root leaves an unscoped dependency at ScopeUnknown, while a present root with no direct dependencies applies ScopeRuntime.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 616a4

Absent graph roots now leave dependency scopes unchanged instead of defaulting them to runtime, while valid empty roots retain existing runtime default behavior. The intended behavior is covered with no active merge risk identified.

Poem

A rabbit checks the missing root
No runtime stamp is sent astray
Unknown scopes remain untouched
An empty root still finds its way
Tests guard both paths today

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: PropagateScopes now does nothing when the root is absent from the graph.
Linked Issues check ✅ Passed The changes satisfy issue #38. PropagateScopes now returns early when the root lookup fails, which prevents the runtime default from applying to unrelated dependencies. The added test also preserves t…
Out of Scope Changes check ✅ Passed The changes are limited to the required PropagateScopes fix, its documentation, and focused regression coverage. No unrelated code changes are present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/propagate-scopes-absent-root

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

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

@bomly-guy
bomly-guy merged commit 671fcb7 into main Sep 5, 2026
3 checks passed
@bomly-guy
bomly-guy deleted the fix/propagate-scopes-absent-root branch September 5, 2026 06:20
@bomly-guy bomly-guy mentioned this pull request Sep 5, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PropagateScopes stamps every dependency runtime when the root ID is absent from the graph

1 participant

@bomly-guy