feat(storybook): improve NodeView stories with real SchemaView#1385
Merged
Conversation
Render the NodeView sidebar against the real SchemaView (ag-grid) plus MockLineageProvider and the real run-registry icons, with light inline stubs for the RowCountDiffTag / RowCountTag / SandboxDialog injection slots. Adds Default, NoRowCountData, ViewMaterialization, SingleEnvMode, SchemaChanged, CodeChanged, and MaterializationChanged variants. Storybook infra updates that come along: - Alias @datarecce/ui/theme in .storybook/main.ts so preview can use the real CSS-variables theme (single theme drives both modes via .dark class) instead of ad-hoc createTheme calls. - MSW handler for /api/flag so useRecceServerFlag resolves under MSW (SchemaView reads server flags during render). - Add @tanstack/react-query to the storybook package so the story can wrap children in QueryClientProvider. - Mock catalogMetadata.base/current with realistic ArtifactMetadata stubs in MockProviders so SchemaView doesn't render the "catalog.json not found" warning in stories. - Additive type exports (NodeViewActionCallbacks, RunTypeIconMap) from @datarecce/ui/advanced so consumers can type their injected icons / callbacks without reaching into internal paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>
6 tasks
… story The previous version reimplemented the production row-count tag visuals (percentage calc, up/down arrows, success/error tone, N/A handling) as ~70 lines of factory code in the story file. That would drift the moment the real components changed. Plumb row-count fixture data through MockLineageProvider's runsAggregated instead, and pass the production RowCountDiffTag / RowCountTag to NodeView directly: - Re-export RowCountDiffTag and RowCountTag from @datarecce/ui/advanced (additive, matches existing NodeViewActionCallbacks / RunTypeIconMap pattern). - MockLineageProvider accepts an optional runsAggregated prop and forwards it to LineageGraphProvider. - buildFixture returns a RunsAggregated map keyed by node.id; makeStory splits it into parameters.mockRunsAggregated so the meta-level decorator can thread it into MockLineageProvider per story. - Delete makeRowCountDiffTag / makeRowCountTag factories and their Chip / Stack / icon imports. One small cast at the injection site bridges LineageGraphNode (real component's node type) and NodeViewNodeData (story's lighter fixture shape) — the real components only read node.id, so the contract is satisfied at runtime. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the Storybook Lineage/NodeView stories so they render the real SchemaView (ag-grid) and production run-type icons, by adding lightweight Storybook-only providers/stubs and updating Storybook theming + MSW mocks.
Changes:
- Wire
NodeViewstories to realSchemaView/SingleEnvSchemaView, real run registry icons, and real row-count tags (viarunsAggregatedfixtures). - Update Storybook preview to use the shared
@datarecce/ui/themeCSS-variables theme and add a new MSW handler for/api/flag. - Additive UI exports for typing/injection (
NodeViewActionCallbacks,RunTypeIconMap) and for story composition (RowCountDiffTag,RowCountTag).
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| js/pnpm-lock.yaml | Adds lock entry for @tanstack/react-query used by Storybook. |
| js/packages/ui/src/advanced.ts | Re-exports row-count tags and NodeView typing helpers via @datarecce/ui/advanced. |
| js/packages/storybook/stories/mocks/MockProviders.tsx | Enhances MockLineageProvider with runsAggregated and non-null catalog metadata to support real SchemaView. |
| js/packages/storybook/stories/lineage/NodeView.stories.tsx | Reworks stories to use real SchemaView, real registry icons, and row-count fixtures via a decorator. |
| js/packages/storybook/package.json | Adds @tanstack/react-query dependency for Storybook. |
| js/packages/storybook/.storybook/preview.tsx | Switches Storybook to the shared UI theme and toggles .dark for dark mode. |
| js/packages/storybook/.storybook/mocks/handlers.ts | Adds /api/flag MSW handler for useRecceServerFlag. |
| js/packages/storybook/.storybook/main.ts | Adds Vite alias for @datarecce/ui/theme. |
Files not reviewed (1)
- js/pnpm-lock.yaml: Language not supported
Contributor
Code Review: PR #1385SHA Storybook-only change. Replaces No production code paths change. Verified:
Verification
Reviewer should still complete the visual checklist in the PR description (open Storybook → Lineage → NodeView; confirm columns/legend/highlights render and no catalog warning fires) before merge. |
gcko
approved these changes
May 19, 2026
Contributor
gcko
left a comment
gcko
left a comment
There was a problem hiding this comment.
Code Review: PR #1385
SHA 4379afc7 · Verdict GO
Storybook-only change. Replaces StubSchemaView and hand-rolled tag stubs with the real SchemaView / SingleEnvSchemaView / RowCountDiffTag / RowCountTag, plumbed through a QueryClientProvider + extended MockLineageProvider. Adds an /api/flag MSW handler, swaps the ad-hoc createTheme() calls for the real @datarecce/ui/theme, and re-exports two existing types (NodeViewActionCallbacks, RunTypeIconMap) from @datarecce/ui/advanced so the story can name them without reaching into internal paths.
No production code paths change. Verified:
js/packages/storybook/stories/lineage/NodeView.stories.tsx:97-106— theas unknown as NodeTagComponentcast onRowCountDiffTag/RowCountTagis sound. Both components only readnode.idfrom theirnodeprop (js/packages/ui/src/components/lineage/NodeTag.tsx:159,:232), and the story fixtures setidon every node.js/packages/storybook/.storybook/mocks/handlers.ts:116-126—/api/flaghandler shape matchesApiResponse<RecceServerFlags>(js/packages/ui/src/api/flag.ts:6-12). Consumers likeSchemaViewuseserverFlags?.field ?? false, so flags added later won't silently break stories.js/packages/storybook/stories/mocks/MockProviders.tsx:143-162— non-nullcatalogMetadata.base/currentsuppresses the "catalog.json not found" warning thatSchemaView(js/packages/ui/src/components/schema/SchemaView.tsx:271-283) renders when either side is falsy. Truthiness is the only thing checked.js/packages/storybook/package.json:14+ lockfile —@tanstack/react-queryresolves to5.100.10across all three importers via the existingpnpm.overrides. No duplicate copies → no React Query context fragmentation.js/packages/ui/src/advanced.ts— type re-exports are additive; nothing removed.noopCallbacksinNodeView.stories.tsx:223-235covers all 11 fields onNodeViewActionCallbacks(js/packages/ui/src/components/lineage/NodeView.tsx:109-132).
Verification
pnpm lint— cleanpnpm --filter @datarecce/storybook build— clean (Storybook builds in 19s)pnpm type:check— pre-existing@datarecce/uierrors only (reproduced against themainmerge-basea544c9b0); no new errors introduced by this PRpnpm --filter @datarecce/storybook test— could not run locally; vitest requires a Playwright Chromium download that isn't present in the review environment. Not a code finding.
Reviewer should still complete the visual checklist in the PR description (open Storybook → Lineage → NodeView; confirm columns/legend/highlights render and no catalog warning fires) before merge.
gcko
approved these changes
May 19, 2026
Contributor
gcko
left a comment
gcko
left a comment
There was a problem hiding this comment.
Code Review: PR #1385
SHA 4379afc7 · Verdict GO
Storybook-only change. Replaces StubSchemaView and hand-rolled tag stubs with the real SchemaView / SingleEnvSchemaView / RowCountDiffTag / RowCountTag, plumbed through a QueryClientProvider + extended MockLineageProvider. Adds an /api/flag MSW handler, swaps the ad-hoc createTheme() calls for the real @datarecce/ui/theme, and re-exports two existing types (NodeViewActionCallbacks, RunTypeIconMap) from @datarecce/ui/advanced so the story can name them without reaching into internal paths.
No production code paths change. Verified:
js/packages/storybook/stories/lineage/NodeView.stories.tsx:97-106— theas unknown as NodeTagComponentcast onRowCountDiffTag/RowCountTagis sound. Both components only readnode.idfrom theirnodeprop (js/packages/ui/src/components/lineage/NodeTag.tsx:159,:232), and the story fixtures setidon every node.js/packages/storybook/.storybook/mocks/handlers.ts:116-126—/api/flaghandler shape matchesApiResponse<RecceServerFlags>(js/packages/ui/src/api/flag.ts:6-12). Consumers likeSchemaViewuseserverFlags?.field ?? false, so flags added later won't silently break stories.js/packages/storybook/stories/mocks/MockProviders.tsx:143-162— non-nullcatalogMetadata.base/currentsuppresses the "catalog.json not found" warning thatSchemaView(js/packages/ui/src/components/schema/SchemaView.tsx:271-283) renders when either side is falsy. Truthiness is the only thing checked.js/packages/storybook/package.json:14+ lockfile —@tanstack/react-queryresolves to5.100.10across all three importers via the existingpnpm.overrides. No duplicate copies → no React Query context fragmentation.js/packages/ui/src/advanced.ts— type re-exports are additive; nothing removed.noopCallbacksinNodeView.stories.tsx:223-235covers all 11 fields onNodeViewActionCallbacks(js/packages/ui/src/components/lineage/NodeView.tsx:109-132).
Verification
pnpm lint— cleanpnpm --filter @datarecce/storybook build— clean (Storybook builds in 19s)pnpm type:check— pre-existing@datarecce/uierrors only (reproduced against themainmerge-basea544c9b0); no new errors introduced by this PRpnpm --filter @datarecce/storybook test— could not run locally; vitest requires a Playwright Chromium download that isn't present in the review environment. Not a code finding.
Reviewer should still complete the visual checklist in the PR description (open Storybook → Lineage → NodeView; confirm columns/legend/highlights render and no catalog warning fires) before merge.
danyelf
added a commit
that referenced
this pull request
May 20, 2026
Resolves conflicts in two NodeView files where main and this branch touched overlapping areas. Key main changes pulled in: - feat(lineage): reorder NodeView tabs to Columns → Code → Lineage (DRC-3468 — Columns is now the default landing tab) - feat(lineage): enrich node-name tooltip with formatNodeTooltip - feat(lineage): float kebab on hover, keep status icons visible - AUTO-COMMIT version bumps to 1.50.0.dev0 Conflict resolutions: - NodeView.tsx header: kept this branch's compact layout (name + ResourceTypeTag + close, no Explore row), wrapped the name Typography in main's MuiTooltip+formatNodeTooltip so hover still surfaces resourceType and materialized. The kebab/ExploreHeaderButtons block from main was intentionally dropped — this branch removes the Sandbox slot and inlines row-count on the Row Count button via the new rowCountDisplay prop, which made the secondary header row obsolete. - NodeView.stories.tsx: took this branch's structure (createStoryArgs + rowCountDisplay slot) because main's makeStory / mockRunsAggregated plumbing was built around RowCountDiffTag/RowCountTag/SandboxDialog props that the redesign removes. Ported main's MaterializationChanged story (view → table fixture) into this branch's pattern so coverage is preserved. Follow-up filed: DRC-3512 — the MaterializationChanged storybook fixture revealed the sidebar has no visible affordance for materialization changes (changeStatus="modified" shows a dot but Code tab shows identical SQL). Story-side cleanup that fell out of the merge: - Exported RowCountDiffSummary / RowCountSummary from @datarecce/ui/advanced and replaced the storybook's inline RowCountDiffDisplay / RowCountDisplay helpers with the production components. This carries forward the spirit of main's "no story-side reimplementation of tag visuals" refactor (PR #1385) but targets the new inline summaries rather than the removed pill tags. Verified post-merge: - pnpm type:check passes - pnpm lint passes - All 459 lineage tests pass, including main's new DRC-3468 default-tab assertion - Storybook (localhost:6007) renders all 7 NodeView stories, including the ported MaterializationChanged Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Storybook only change. The Storybook story for Schema View was really bad -- it was hard to see what it did, since the formatting was dramatically off.
See, for example, this sample image:
The goal of this PR is to make the NodeView story accurate enough that we can actually do design against it. I had hoped this would just pull in some CSS (and it did!), but it also needs a smarter stub/mock machinery.
After this PR, and the stacked next one that makes actual changes to the layout, the storybook version now looks like this:
Summary
Improve the Storybook stories for
NodeViewso they render the realSchemaView(ag-grid) and the real run-registry icons, with light inline stubs for the dependency-injection slots that need app-level context. No changes to ship-to-appNodeViewbehavior — this only touchesjs/packages/storybook/**, an additive type re-export injs/packages/ui/src/advanced.ts, and the catalog-metadata stubs inMockProviders.SchemaView/SingleEnvSchemaViewwired throughQueryClientProvider+MockLineageProviderso column rendering, the schema legend, and added/removed row highlights all demo accurately.findByRunTypeso the action buttons match production.RowCountDiffTag,RowCountTag, and an emptySandboxDialog, so the layout works without wiring uprunsAggregated.Default,NoRowCountData,ViewMaterialization,SingleEnvMode,SchemaChanged,CodeChanged,MaterializationChanged.Storybook infra updates that come along:
@datarecce/ui/themeso the preview uses the real CSS-variables theme (single theme drives both modes via the.darkclass) instead of the ad-hoccreateThemecalls./api/flagsouseRecceServerFlagresolves under MSW (SchemaViewreads server flags during render).@tanstack/react-queryto the storybook package so the story can wrap children inQueryClientProvider.catalogMetadata.base/currentwith realisticArtifactMetadatastubs inMockProviderssoSchemaViewdoesn't render the "catalog.json not found" warning in stories.NodeViewActionCallbacks,RunTypeIconMap) from@datarecce/ui/advancedso consumers can type injected icons / callbacks without reaching into internal paths.This PR is intentionally independent of any
NodeViewredesign — it demonstrates the component as it exists on `main` today.Type of change
Test plan
User-facing changes
None — Storybook-only.
🤖 Generated with Claude Code