Skip to content

feat(lineage): add conditional validation summary chip - #1540

Merged
gcko merged 4 commits into
feature/drc-4260-aggregate-compact-validation-summaries-by-lineage-nodefrom
feature/drc-4261-show-a-conditional-validation-summary-chip-on-lineage-nodes
Sep 8, 2026
Merged

gcko merged 4 commits into
feature/drc-4260-aggregate-compact-validation-summaries-by-lineage-nodefrom
feature/drc-4261-show-a-conditional-validation-summary-chip-on-lineage-nodes

Conversation

@gcko

@gcko gcko commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

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:

  • Shows a compact validation-summary chip only on lineage nodes with positive result evidence.
  • Lazily exposes readable per-type details with accessible hover, focus, keyboard, and Escape behavior.
  • Isolates chip and portal interactions from React Flow dragging, panning, selection, context menus, and keyboard handling.
  • Opens the selected node directly on the Analysis tab through a repeatable, acknowledged navigation request.
  • Re-exports the new public context types through all package entrypoints.

Which issue(s) this PR fixes:

Closes DRC-4261

Special notes for your reviewer:

  • Stack position: 2 of 2 in the lineage stack.
  • Base/parent: feature/drc-4260-aggregate-compact-validation-summaries-by-lineage-node
  • Parent PR: feat(lineage): add compact validation summaries #1539
  • GitHub stack: https://github.com/DataRecce/recce/stacks/1541
  • Final focused zero-result and React Flow interaction regressions: 29 passed.
  • Full frontend suite: 4,369 passed, 5 skipped.
  • Frontend lint/type checks, shared UI declaration build, OSS production build, diff checks, and hooks passed.
  • Tests exercise the real React Flow wrapper in normal and selecting modes.

Does this PR introduce a user-facing change?:

Yes.

Lineage nodes now show an accessible validation-summary chip for positive validation evidence, with per-type details and one-step navigation to the Analysis tab.

@gcko gcko changed the title feature/drc 4261 show a conditional validation summary chip on lineage nodes feat(lineage): add conditional validation summary chip Sep 4, 2026
@gcko gcko mentioned this pull request Sep 4, 2026
2 tasks done
@gcko gcko self-assigned this Sep 4, 2026
gcko and others added 3 commits September 8, 2026 08:05
DRC-4261

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
DRC-4261

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
DRC-4261

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
@gcko
gcko force-pushed the feature/drc-4261-show-a-conditional-validation-summary-chip-on-lineage-nodes branch from b5e58fb to 340da8d Compare September 8, 2026 00:05
@gcko
gcko requested a review from wcchang1115 September 8, 2026 00:05
@wcchang1115 wcchang1115 added reviewing code-review-loop: review in progress and removed reviewing code-review-loop: review in progress labels Sep 8, 2026
@wcchang1115

wcchang1115 commented Sep 8, 2026 •

Copy link
Copy Markdown
Collaborator

Code Review: PR #1540

SHA 340da8d · Verdict NO-GO

Blockers

None.

Issues

  1. js/packages/ui/src/components/lineage/nodes/LineageNode.tsx:681 — In action_result mode, a node with no action tag loses its change-category label.

    The new guard covers the label and the tag together. Before, only the tag sat behind it.

    To see it, start from a graph where the changed models show a category label. Select one changed model, then Actions, Row Count Diff. Every other changed model loses its label. The △ icon stays. The selected model keeps only its action tag, which is the old behavior.

    No test covers this state. LineageNode.test.tsx:613 asserts only that the tag is absent in that mode.

    Pass F.

Notes

  1. js/packages/ui/src/components/lineage/GraphNodeOss.tsx:559 — The runsAggregated read moves from the leaf tag to the memoized node root.

    GraphNode is memo(GraphNodeComponent) (:691), and a context change re-renders a consumer through memo. So each aggregate refresh now re-renders the whole node, not the tag subtree. The 1000-node test (GraphNodeOss.test.tsx:644) counts mounts, not re-renders. PR feat(lineage): add compact validation summaries #1539 widens the refetch to four more run types, so this path runs more often.

  2. js/packages/ui/src/components/lineage/GraphNodeOss.tsx:269,323 — role="dialog" promises a region that keyboard focus can never enter.

    The Paper holds only Stack, Box, and Typography, so it has no focusable content. The trigger also closes the popover on blur (:272). So focus never enters the region that both attributes describe. A disclosure pattern matches what the code does.

  3. js/packages/ui/src/contexts/lineage/types.ts:267 — openNodeDetails is a new required member of the exported LineageViewContextType (index.ts:124, contexts/index.ts:65, contexts-entry.ts:59). Code outside this repository that builds its own context value must add it.

@wcchang1115 wcchang1115 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 340da8d. NO-GO — 1 issue, 3 notes: #1540 (comment)

@wcchang1115 wcchang1115 added the fixing code-review-loop: fix in progress label Sep 8, 2026
Letting the change label and runsAggregatedTag show together put both
behind one selectMode check. A node with no action tag then lost its
change-category label while action results were on screen, and the
graph stopped naming which other models changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Wei-Chun, Chang <wcchang@infuseai.io>
@wcchang1115 wcchang1115 added reviewing code-review-loop: review in progress and removed fixing code-review-loop: fix in progress labels Sep 8, 2026
@wcchang1115

Copy link
Copy Markdown
Collaborator

Code Review: PR #1540

SHA 707e4f48 · Verdict GO · Incremental (since 340da8d)

Blockers

None.

Issues

None.

Notes

  1. Issue 1 of the 340da8d review is closed. LineageNode.tsx:701 applies the
    selectMode condition to runsAggregatedTag alone, so the change label renders again
    in every mode.
    Evidence: serverFlags.new_cll_experience is false by default (SchemaView.tsx:250),
    so the label returns for every category, not only unknown. When I revert the source
    change alone, LineageNode.test.tsx:627 fails.
    Pass F.

  2. Three Notes from the 340da8d review are still open. None of them blocks the merge.

    • GraphNodeOss.tsx:559 reads runsAggregated at the memoized node root.
    • GraphNodeOss.tsx:269 sets role="dialog" on a region that keyboard focus cannot enter.
    • contexts/lineage/types.ts:267 adds openNodeDetails as a required member of
      LineageViewContextType.

@wcchang1115 wcchang1115 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GO at 707e4f48. Full review: #1540 (comment)

@wcchang1115 wcchang1115 removed the reviewing code-review-loop: review in progress label Sep 8, 2026
@gcko
gcko merged commit bf29991 into main Sep 8, 2026
8 checks passed
@gcko
gcko deleted the feature/drc-4261-show-a-conditional-validation-summary-chip-on-lineage-nodes branch September 8, 2026 06:51
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