fix: surface state:modified nodes in summary lineage graph

[dtaniwaki]

Thanks for maintaining this project! I'd appreciate a review of this bug fix when you have a chance.

PR checklist

• Ensure you have added or ran the appropriate tests for your PR.
• DCO signed

What type of PR is this?

fix

What this PR does / why we need it:

When a dbt macro or project config changes, state:modified correctly returns the affected downstream nodes — even if their SQL body (and thus file checksum) is unchanged. However, generate_markdown_summary was building the lineage graph from checksums alone, never passing the lineage_diff.diff that the adapter had already computed. As a result, nodes changed only by macros or configs were silently invisible in the PR summary comment.

The fix is a one-line change in generate_markdown_summary: pass lineage_diff.diff to _build_lineage_graph. A small supporting mechanism (apply_diff / _forced_change_status) lets the graph override the checksum-based status for nodes that state:modified flagged but whose SQL didn't change.

Which issue(s) this PR fixes:

N/A

Special notes for your reviewer:

The diff dict was already being populated correctly by the adapter (it calls state:modified and maps each node to a NodeDiff). The bug was purely in the summary layer not consuming it.

Does this PR introduce a user-facing change?:

Nodes modified by macro or config changes (with no SQL body change) now correctly appear as changed in the Recce PR summary comment lineage graph.

[codecov]

Codecov Report

❌ Patch coverage is 98.73418% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
recce/summary.py	91.66%	1 Missing ⚠️

Files with missing lines	Coverage Δ
tests/test_summary.py	100.00% <100.00%> (ø)
recce/summary.py	67.09% <91.66%> (+1.03%)	⬆️

... 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

Fixes the PR summary lineage graph so it can surface state:modified nodes even when their SQL checksum hasn’t changed (e.g., macro/config-only changes), by plumbing the adapter-computed lineage_diff.diff into the summary graph builder and allowing that diff to override checksum-derived status.

Changes:

• Pass lineage_diff.diff into _build_lineage_graph from generate_markdown_summary.
• Add a Node.apply_diff() override mechanism (_forced_change_status) so externally computed diffs can drive change_status.
• Add unit tests covering apply_diff behavior and _build_lineage_graph(..., diff=...).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
recce/summary.py	Allows lineage graph node status to be overridden by adapter diff and wires diff into markdown summary generation.
tests/test_summary.py	Adds tests ensuring diff-driven “modified” nodes appear in the graph and in “What’s Changed”.

[gcko]

Code Review — PR 1192

Summary

Fixes generate_markdown_summary to pass the adapter's pre-computed lineage_diff.diff into _build_lineage_graph, so nodes flagged by state:modified (e.g., macro/config-only changes) are correctly surfaced in the PR summary lineage graph. Clean, minimal fix with good test coverage.

Findings

[Warning] "Code" label is misleading for macro/config-only changes

File: recce/summary.py:148-149
Issue: When a node is force-marked as modified via apply_diff (because a macro or project config changed, not the SQL body), _what_changed() labels it as "Code" changed. This is technically inaccurate — the node's raw SQL didn't change; a macro or config dependency did. Users may look at the node's SQL diff and see no difference, which could be confusing.
Suggestion: Consider adding a distinct label like "Macro" or "Config" (would require extending NodeDiff to carry the reason). Not blocking since the current behavior is a clear improvement over nodes being completely invisible, and this can be addressed as a follow-up.

[Warning] Pre-existing shared mutable class-level defaults in LineageGraph

File: recce/summary.py:260-262
Issue: LineageGraph defines nodes and edges as class-level mutable dicts (nodes: Dict[str, Node] = {}). Without an __init__ that creates instance-level copies, all LineageGraph instances share the same dict. This is pre-existing (not introduced by this PR), but the new tests in TestBuildLineageGraphWithDiff.test_diff_marks_state_modified_nodes create two graphs sequentially — the second graph silently mutates state from the first. The assertions happen to pass because they run in sequence, but this is fragile and could cause test pollution if test ordering changes or tests run in parallel.
Suggestion: Not blocking for this PR since it's pre-existing. Worth a follow-up to add __init__ to LineageGraph that initializes instance-level dicts.

Verification

• Tests: 26/26 passed (tests/test_summary.py)
• Lint: flake8 clean
• New test coverage: TestNodeApplyDiff (2 tests), TestWhatChanged (1 test), TestBuildLineageGraphWithDiff (3 tests) — all exercise the new code paths including the backward-compatibility check with real manifests.

Verdict

No critical issues found. The core fix (passing lineage_diff.diff to _build_lineage_graph) is correct and well-tested. The warnings above are non-blocking suggestions for follow-up.

[gcko]

@dtaniwaki thanks for the PR! Can you please resolve the conflict in tests/test_summary.py and look at addressing the copilot and findings above?

If you feel the issues are not valid, please say as such - and address them as necessary.

[gcko]

see my above comment #1192 (comment)

[Copilot]

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

LineageGraph.nodes and LineageGraph.edges are mutable class attributes, so _build_lineage_graph() mutates shared state across calls. With the new apply_diff() / _forced_change_status override, a node marked modified in one summary can remain modified in later summaries even when the next call’s diff doesn’t include it (and graphs can also retain stale nodes/edges from previous builds). Make nodes/edges instance attributes (e.g., add __init__ that sets self.nodes = {} and self.edges = {}), or otherwise ensure each _build_lineage_graph() call starts from a fresh graph state.