fix(resolve): guard OnFinished against nil loaderHookContext on skipped fetches

[jensneuse]

Summary by CodeRabbit

• Tests

  • Added integration tests validating correct federation behavior when parent data is null, ensuring child fetches are properly skipped.
  • Expanded test coverage for loader hook lifecycle across serial and parallel execution paths.

• Bug Fixes

  • Corrected loader hook finalization to ensure callback hooks are only invoked when appropriate, preventing unexpected behavior in skipped fetch scenarios.

Fixes a panic where LoaderHooks.OnFinished was called with a nil context.Context when a fetch was skipped (e.g. null parent data, auth rejection, rate limiting). The nil guard is now consolidated in a callOnFinished helper that covers all four fetch paths: SingleFetch, EntityFetch, BatchEntityFetch, and the parallel merge loop. Tests were added for each path, the LoaderHooks interface now documents the OnLoad/OnFinished contract, and an integration test verifies the skip behaviour end-to-end.

Checklist

• I have discussed my proposed changes in an issue and have received approval to proceed.
• I have followed the coding standards of the project.
• Tests or benchmarks have been added or updated.

[StarpTech]

LGTM

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

📝 Walkthrough

Walkthrough

This PR adds integration and unit test coverage for skipped fetch behavior when a parent entity is null, refactors loader hook finalization to only call OnFinished when hooks were actually invoked, and updates test helpers accordingly.

Changes

Cohort / File(s)	Summary
Integration Test for Skipped Fetch execution/federationtesting/skipped_fetch_test.go	New integration test validating that downstream subgraphs are not invoked when a parent entity is null, using mock Users and Reviews subgraphs with call count tracking.
Loader Hook Lifecycle Refactoring v2/pkg/engine/resolve/loader.go	Refactors OnFinished hook invocation with a new callOnFinished helper that only triggers hooks when loaderHookContext exists, ensuring consistency with skip conditions and preventing OnFinished from being called on skipped fetches.
Loader Hooks Test Coverage Expansion v2/pkg/engine/resolve/loader_hooks_test.go	Expands test coverage with multiple test cases validating that OnFinished is not called in skipped fetch scenarios (null parent) across serial and parallel code paths; updates NewTestLoaderHooks() signature to return pointer type.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix: group subgraph errors by subgraph name #1351 — Addresses skipped-fetch behavior for loader hooks, ensuring downstream subgraphs/hooks aren't invoked when a parent is null.
• feat: validate presence of optional @requires dependencies #1297 — Modifies loader hook lifecycle logic in resolve/loader.go around skipped fetch behavior, ensuring OnFinished is only called when loaderHookContext exists.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding a nil guard for OnFinished to handle skipped fetches.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jensneuse/nil-hookctx-guard

---

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