fix(mcp): prevent UnboundLocalError in impact_analysis value_diff step

[iamcxa]

Summary

Move node_id_by_name dict construction out of the schema-diff try block so the subsequent value-diff step can still run when schema diff fails.

Bug: If schema diff raised an exception before node_id_by_name was assigned (L1274), the value-diff loop at L1308 would fail with UnboundLocalError — defeating the tool's error-resilient design where each step captures errors independently.

Fix: Initialize node_id_by_name before the schema-diff try block. The dict only depends on impacted_node_ids and all_nodes, which are computed in Step 1 (always succeeds before schema diff runs).

Found by: Copilot review on PR #1233

Test plan

• Verified node_id_by_name is used by both schema-diff (L1281) and value-diff (L1308) — both now have guaranteed access
• No behavioral change when schema diff succeeds (same code, different scope)

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
recce/mcp_server.py	86.86% <100.00%> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

This PR improves the error-resilience of the impact_analysis MCP tool by ensuring the value-diff step can still execute even if the schema-diff step fails early, aligning with the tool’s “capture errors per step” design.

Changes:

• Initialize node_id_by_name before entering the schema-diff try block to avoid UnboundLocalError in the subsequent value-diff step.
• Keep schema-diff and value-diff steps independently failure-tolerant by removing the cross-step dependency on a try-scoped variable.

[Copilot]

node_id_by_name is built from impacted_node_ids, but select_nodes() can return non-model resources (seeds/snapshots/etc.). Because the map is keyed only by name and iterates an unordered set, a non-model node with the same name as a model can overwrite the model entry and cause schema/value diff to use the wrong node_id. Filter to node_id.startswith("model.") (or build the map from the already-filtered impacted_models) to make the lookup deterministic and model-only.

Suggested change

	for node_id in impacted_node_ids:
	for node_id in impacted_node_ids:
	# Only include models to avoid non-model resources (seeds/snapshots/etc.)
	# overwriting entries when they share the same name.
	if not node_id.startswith("model."):
	continue

[Copilot]

This change fixes a control-flow bug where schema-diff failures could prevent the value-diff step from running. Please add a regression test that forces the schema-diff block to raise before it would have assigned node_id_by_name previously (e.g., craft a lineage_diff where base is a non-dict so lineage_diff.get("base", {}).get(...) raises), and assert that impact_analysis still returns successfully with a schema_diff entry in errors and proceeds through the value-diff loop without crashing.