Skip to content

feat(lineage): add Body Changes view mode to filter config-only changes#1268

Open
even-wei wants to merge 12 commits into
mainfrom
feature/drc-3047-body-changes-mode
Open

feat(lineage): add Body Changes view mode to filter config-only changes#1268
even-wei wants to merge 12 commits into
mainfrom
feature/drc-3047-body-changes-mode

Conversation

@even-wei
Copy link
Copy Markdown
Contributor

@even-wei even-wei commented Apr 1, 2026

PR checklist

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

What type of PR is this?

Feature

What this PR does / why we need it:

Adds a new "Body Changes" mode to the lineage view Mode dropdown. This mode uses state:modified.body, state:modified.macros, and state:modified.contract selectors to filter out config-only YAML changes (tags, descriptions, deprecation dates, warehouse, materialization, etc.) while keeping data-impacting changes visible.

This reduces noise for users whose PRs touch YAML config without changing SQL logic — a pain point raised by customers (see Slack thread in #customer-super).

Which issue(s) this PR fixes:

Resolves DRC-3047

Special notes for your reviewer:

  • The existing "Changed Models" (1+state:modified+) and "All" modes remain unchanged
  • No defaults are changed — changed_models is still the default for UI and MCP
  • The recce summary command is not changed in this PR; it already excludes config-only changes via checksum comparison. A follow-up issue can add full selector parity if needed
  • The MCP impact_analysis tool already uses this same selector pattern (state:modified.body+ state:modified.macros+ state:modified.contract+)
  • Pre-existing selector test failures (DuckDB config issue) are unrelated

Does this PR introduce a user-facing change?:

Yes — new "Body Changes" option in the lineage view Mode dropdown that filters to only SQL body, macro, and contract changes.

Verification

Reproduction

Fixture: integration_tests/dbt with a 1-line body change in customers.sql (+ where customer_id > 0) plus a description and meta change on the orders model in schema.yml. Run:

recce server --target-path target --target-base-path target-base

Both customers and orders are flagged modified (change_status=modified). The body_changes selector must exclude orders.

Screenshot evidence

All committed under docs/screenshots/drc-3047/.

ID What it shows Validates
SS-1 Mode dropdown open — Changed Models / Body Changes / All UI option exists
SS-2 Mode=All — full DAG with all upstream context Baseline N
SS-3 Mode=Changed Models (default) — 5 nodes incl. orders (config-only) and customers (body) Default behavior unchanged
SS-4 Mode=Body Changes — 4 nodes; orders excluded New mode works
SS-5 SS-3 vs SS-4 side-by-side; orders circled red on left, dashed green on right marking its absence Effect is unambiguous
SS-6 Saved Lineage Diff check titled "Lineage diff of 4 nodes" — replay renders the 4-node subset, NOT the 5-node modifiedSet Self-review NO-GO #1 fix
SS-7 Branch with only config-only YAML edits, Body Changes mode → "No change detected" placeholder with "Show all nodes" escape hatch Self-review NO-GO #2 fix

Quantitative annotations (verified via /api/select)

Mode Node count Selector Proven by
All 9 (5 models + 4 seeds) (no selector) SS-2
Changed Models 5 1+state:modified+ SS-3
Body Changes 4 (orders excluded) 1+state:modified.body+ state:modified.macros+ state:modified.contract+ SS-4 + SS-5
Body Changes (config-only branch) 0 → empty-state (as above) SS-7

Tests

  • uv run pytest tests/adapter/dbt_adapter/test_selector.py tests/tasks/test_lineage.py tests/tasks/test_row_count.py tests/tasks/test_schema.py25 passed, including non-degenerate body_changes fixture (customers_6 config-only excluded; body_ids < changed_ids asserted) — self-review NO-GO Add diffview in web server #3 fix
  • cd js && pnpm vitest run3724 passed | 5 skipped, including 4 new regression tests in LineageView.test.tsx covering saved-check replay (explicit node_ids honored), no-fallback semantics for body_changes without node_ids, body_changes empty-state, and the changed_models regression guard
  • pnpm lint → clean; pnpm type:check → clean

Self-review NO-GO items — all resolved

Item Fix Validated by
LineageView.tsx saved-check filter would short-circuit body_changes to modifiedSet LineageViewOss.addLineageDiffCheck persists resolved filteredNodeIds on save; LineageView drops body_changes from the modifiedSet fallback SS-6 + new vitest tests
Empty-state guard missing for body_changes zero result Added body_changes branch alongside the changed_models empty-state path in LineageViewOss SS-7 + new vitest test
Degenerate selector test — both modes returned the same set test_selector.py adds customers_6 config-only model (same raw_code, differing unrendered_config.meta); asserts body_ids < changed_ids backend pytest

Rollback / risk

  • Default-off: changed_models remains the default for UI and MCP. New mode is opt-in.
  • If a customer's dbt version doesn't support state:modified.body, fallback is view_mode: "all" — user sees full graph instead of error (existing try/except at LineageViewOss.tsx:514-538).
  • Rollback = remove the dropdown option; static selector strings, no migration.

🤖 Generated with Claude Code

@even-wei even-wei self-assigned this Apr 1, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
recce/adapter/base.py 45.58% <ø> (ø)
recce/adapter/dbt_adapter/__init__.py 79.47% <100.00%> (+0.03%) ⬆️
recce/mcp_server.py 91.19% <ø> (ø)
recce/server.py 70.77% <100.00%> (ø)
recce/tasks/core.py 79.72% <ø> (ø)
recce/tasks/lineage.py 100.00% <100.00%> (ø)
recce/tasks/rowcount.py 90.04% <100.00%> (ø)
recce/tasks/schema.py 46.51% <100.00%> (ø)
tests/adapter/dbt_adapter/test_selector.py 100.00% <100.00%> (ø)
tests/tasks/test_lineage.py 100.00% <ø> (ø)
... and 2 more

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

even-wei and others added 2 commits April 1, 2026 12:12
Add a new "Body Changes" mode to the lineage view Mode dropdown that uses
state:modified.body + state:modified.macros + state:modified.contract
selectors. This filters out config-only YAML changes (tags, descriptions,
deprecation dates, etc.) while keeping data-impacting changes visible.

The existing "Changed Models" and "All" modes remain unchanged.

Resolves DRC-3047

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: even-wei <evenwei@infuseai.io>
- Add missing body_changes to LineageView.tsx and LineageDiffView.tsx types
- Handle body_changes in Cloud LineageView node filtering (was falling through to "all")
- Update stale comments in run.ts, LineageViewTopBar docstring, and 3 test files
- Add Body Changes radio to dropdown assertion in existing test
- Add interaction test for switching to body_changes mode

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: even-wei <evenwei@infuseai.io>
@even-wei even-wei force-pushed the feature/drc-3047-body-changes-mode branch from 062059a to 9fbdd18 Compare April 1, 2026 04:12
@even-wei
Copy link
Copy Markdown
Contributor Author

Code Review: PR #1268

Reviewer: Claude Code Review (adversarial)
Files reviewed: 21 (16 source/non-test, 5 test)
Categories: API endpoints, Business logic, Frontend, Tests
Passes run: A, B, C, D, E, F, G, H
Note: PR is in Draft status — review proceeded at user request.

Validation Results

Pass A — Correctness & Logic — FAIL

ISSUE — LineageView.tsx does not filter body_changes correctly when used by saved Lineage Diff checks

js/packages/ui/src/components/lineage/LineageView.tsx:204-210:

} else if (
  viewOptions?.view_mode === "changed_models" ||
  viewOptions?.view_mode === "body_changes"
) {
  // Changed models or body changes (filtering done server-side via selector)
  selectedNodeIds = lineageGraph.modifiedSet;
}

lineageGraph.modifiedSet is computed in js/packages/ui/src/contexts/lineage/utils.ts:111-134 as every node with any change_status (body changes and config-only changes). The inline comment "filtering done server-side via selector" is true only for LineageViewOss.tsx:514-538, which actually calls the /api/select endpoint. LineageView.tsx never makes that call — it's a presentation component used by LineageDiffView.tsx (saved Lineage Diff checks).

Consequence: a user who saves a Lineage Diff check while in Body Changes mode will see the same node set as a Changed Models check (config-only YAML edits included), defeating the feature's purpose for that surface. The selector value is preserved on the wire, but the rendered graph isn't.

Either:

  • Drop body_changes from this branch and let selectedNodeIds remain undefined, then rely on a server-resolved node list, or
  • Add a body-change indicator to each node in MergedLineageResponse and build a separate bodyChangedSet in buildLineageGraph for this branch to use, or
  • Confirm that saved checks always carry viewOptions.node_ids (line 201-203 short-circuits the branch). If that's the contract, the comment must say so and the new branch is unnecessary.

NOTE — Empty-state guard missing for body_changes in js/packages/ui/src/components/lineage/LineageViewOss.tsx:1378:

if (viewMode === "changed_models" && !lineageGraph.modifiedSet.length) {
  return <LineageViewNoChanges ... />;
}

For body_changes, this empty-state path is never taken. The narrower selector (state:modified.body|.macros|.contract) can return an empty set on a PR that only edits YAML config — even when modifiedSet is non-empty — so the user gets a blank canvas instead of the friendly "no changes" message. Considering the use case is precisely to triage config-only PRs, this is a UX gap, not a defect.

Pass B — Security — PASS

No new endpoints, no new authn/authz boundaries, no PII surfaces. The selector strings are static (not user-controlled).

Pass C — Cross-Reference Consistency — PASS

The Literal["all", "changed_models", "body_changes"] widening is fanned out coherently across every Python type carrier (recce/adapter/base.py, recce/server.py:SelectNodesInput, recce/tasks/{core,lineage,rowcount,schema}.py) and every TypeScript carrier (api/lineagecheck.ts, api/schemacheck.ts, api/select.ts, api/types/run.ts, LineageView.tsx, LineageDiffView.tsx, SchemaDiffView.tsx). MCP tool schemas in recce/mcp_server.py:241,362 updated in both lineage_diff and schema_diff handlers. LineageViewTopBarOss.tsx correctly left untouched — it's a thin context wrapper around the modified core.

Verified: existing LineageViewTopBar.tsx consumers don't rely on view_mode being a 2-valued union for narrowing.

Pass D — Error Handling — PASS

LineageViewOss.tsx:514-538 already wraps the /api/select call with a try/except that falls back to view_mode: "all". If the underlying dbt selector state:modified.body is unsupported on a customer's dbt version, the user sees the full graph instead of an exception. Acceptable.

Pass E — Test Coverage — PASS with one gap

  • tests/adapter/dbt_adapter/test_selector.py:193-195 exercises select_nodes(view_mode="body_changes") end-to-end through the dbt adapter — confirms wiring.
  • js/src/components/lineage/__tests__/LineageViewTopBar.test.tsx adds 45 lines of tests (verified locally: 56 tests passed) covering: label rendering for body_changes, dropdown radio option, click-to-select dispatching view_mode: "body_changes".
  • tests/tasks/{test_lineage,test_row_count,test_schema}.py validators pass body_changes through the schema validator.

Gap: No test exercises the case the feature is for — a project where some models have body changes and others have config-only changes — to assert that select_nodes(view_mode="body_changes") returns the body-changed subset and excludes the config-only subset. The existing test (test_select_with_pacakage_mode_include_exclude) only happens to compare equal because the fixture's only modification is a body change ("same result here since the change is a body change"). A negative case (config-only model excluded) would catch any future regression in dbt's state:modified.* semantics.

Pass F — Diff-Specific Checks — PASS

  • The if view_mode and view_mode == "changed_models":if view_mode == "changed_models": cleanup is safe (None == "changed_models" is False in Python).
  • All callers of the widened literals already accept Optional[str] at the JSON boundary; no breaking change to saved check files.
  • MCP tool descriptions updated to advertise the new option.

Pass G — Performance — PASS

The selector compiles once per select_nodes call. Three additional parse_difference calls in the new branch are negligible compared with the existing state:modified+ graph compilation.

Pass H — Async/Concurrency — N/A

No async surface change.

Verification Results

Check Result
make flake8 clean
uv run pytest tests/adapter/dbt_adapter/test_selector.py tests/tasks/test_lineage.py tests/tasks/test_row_count.py tests/tasks/test_schema.py 25 passed
pnpm vitest run src/components/lineage/__tests__/LineageViewTopBar.test.tsx 56 passed
pnpm lint clean
pnpm type:check 3 errors in packages/ui/src/utils/csv/excel.{ts,test.ts}pre-existing on main, not introduced by this PR
pnpm test (full) 3688 passed, 17 failed — all 17 failures are in pre-existing excel.test.ts, unrelated to this PR

Verdict: NO-GO

Issues

  1. LineageView.tsx:204-210body_changes not actually filtered when rendered through LineageDiffView. Saved Lineage Diff checks created in Body Changes mode will display the same superset of nodes as Changed Models. Either special-case the branch to a body-change subset, or remove body_changes from the branch and rely on viewOptions.node_ids (clarify which contract is intended).

Notes

  1. LineageViewOss.tsx:1378 — extend the empty-state guard to viewMode === "body_changes" so a config-only PR shows the friendly "no changes" placeholder instead of a blank canvas.
  2. Test gap in tests/adapter/dbt_adapter/test_selector.py — existing body_changes assertion is degenerate (same result as changed_models because the fixture's only mod is a body change). Add a fixture variant with a config-only mod and assert it is excluded by body_changes.

What I could not verify

  • Whether the saved Lineage Diff check's stored viewOptions typically includes node_ids — that would short-circuit the branch in LineageView.tsx:201-203 and make the Issue moot. Confirming this requires reading addLineageDiffCheck plumbing or a manual save-and-replay test in a real environment.
  • That state:modified.body / .macros / .contract are universally available in the minimum-supported dbt version — I trust the existing fallback at LineageViewOss.tsx:525-538 covers this, but didn't enumerate the dbt-version test matrix.

What I looked for and did not find

  • SQL/command/template injection: none (selector strings are static literals).
  • Schema drift between Python Literal and TypeScript union: clean.
  • Stale callers of select_nodes / SelectNodesInput / LineageDiffParams not updated: all paths consistent.
  • Mock drift in LineageViewTopBar.test.tsx: passes locally; mocks correctly target @datarecce/ui/contexts and @datarecce/ui/components/run, and use the real LineageViewTopBarOss (which delegates to the modified LineageViewTopBar).
  • N+1 / unbounded query introduced by selector composition: none.

PR is in draft. Mark ready-for-review only after addressing the LineageView filtering branch (or documenting why node_ids always short-circuits it).

- LineageView.tsx: drop body_changes from the modifiedSet fallback (it
  rendered config-only changes too); document that saved body_changes
  checks must carry resolved node_ids.
- LineageViewOss.tsx: persist filteredNodeIds on body_changes saved
  Lineage Diff checks so the presentational LineageView has the right
  set; add an empty-state guard for body_changes when the server-side
  selector returns nothing.
- test_selector.py: add a config-only model (same raw_code, differing
  unrendered_config.meta) and assert body_changes is a strict subset
  of changed_models.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: even-wei <evenwei@infuseai.io>
@even-wei even-wei marked this pull request as ready for review May 8, 2026 05:14
…-changes-mode

Signed-off-by: even-wei <evenwei@infuseai.io>
@even-wei even-wei force-pushed the feature/drc-3047-body-changes-mode branch from 2b29bd8 to 04dfdb2 Compare May 21, 2026 02:46
even-wei and others added 2 commits May 21, 2026 12:16
…(DRC-3047)

Regression coverage for the two NO-GO frontend fixes from the 2026-04-29
self-review of PR #1268, now hardened with explicit tests:

- saved-check replay: when viewOptions carries explicit node_ids in
  body_changes mode, LineageView must honor that subset and NOT fall
  back to lineageGraph.modifiedSet (which includes config-only nodes).
- no client-side body_changes fallback: body_changes without node_ids
  must leave selectedNodeIds undefined, since the filter is resolved
  server-side. Falling back to modifiedSet would defeat the feature.
- empty-state: body_changes with an empty filter result renders the
  friendly "No nodes match the current filter criteria." placeholder.
- regression guard: changed_models still falls back to modifiedSet.

Pairs with the code fixes already in 8cffa99 (round-2 review fix).

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: even-wei <evenwei@infuseai.io>
…C-3047)

Seven captured screenshots paired with the PR Verification section:

- SS-1 mode dropdown open showing Body Changes alongside Changed Models / All
- SS-2 Mode=All (5 modified models + upstream staging on jaffle_shop fixture)
- SS-3 Mode=Changed Models (default — both body and config-only mods present)
- SS-4 Mode=Body Changes (4 nodes, config-only `orders` excluded)
- SS-5 paired SS-3 / SS-4 with `orders` annotated as the excluded config-only model
- SS-6 saved Lineage Diff check titled "Lineage diff of 4 nodes" — replay
  renders the 4-node subset, NOT the 5-node modifiedSet (validates the
  saved-check filtering fix from commit 8cffa99)
- SS-7 Body Changes empty-state ("No change detected") for a branch with
  only config-only YAML edits — validates the LineageViewOss guard from
  commit 8cffa99

Fixture: integration_tests/dbt with a 1-line body change in customers.sql
plus a description/meta-only change in schema.yml `orders` model. Verified
via /api/select: changed_models returns 5 nodes, body_changes returns 4,
and with the body change reverted body_changes returns 0 → empty-state.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: even-wei <evenwei@infuseai.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants