Catalog: render FCS Vega scatter from preview vegaLite spec#4860
Catalog: render FCS Vega scatter from preview vegaLite spec#4860drernie wants to merge 37 commits into
Conversation
Wire the optional vegaLite field from preview info responses through PreviewData.Fcs and render it as a Vega chart below the metadata block. The renderer guards on !!vegaLite, so the field is a soft contract: backends that don't emit it produce no chart, and this catalog change is a no-op until the lambda shared layer (quilt#4858) lands. This is the catalog half of a 2-PR split previously bundled in quilt#4858. The two PRs are independent — either may land first without regression. Files: - catalog/app/components/Preview/loaders/Fcs.js — thread vegaLite - catalog/app/components/Preview/loaders/Fcs.spec.tsx — loader test - catalog/app/components/Preview/renderers/Fcs.jsx — Vega render - catalog/app/components/Preview/renderers/Fcs.spec.tsx — render test - catalog/app/components/Preview/types.js — Fcs type doc Co-Authored-By: Austin Hovland <austin.hovland@alexion.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## preview-lambda-consumers #4860 +/- ##
===========================================================
Coverage ? 47.07%
===========================================================
Files ? 834
Lines ? 34412
Branches ? 5846
===========================================================
Hits ? 16200
Misses ? 16211
Partials ? 2001
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| <div className={className} {...props}> | ||
| {renderWarnings(warnings)} | ||
| {!!metadata && <JsonDisplay name="Metadata" value={metadata} />} | ||
| {!!vegaLite && <Vega className={classes.chart} spec={vegaLite} />} |
There was a problem hiding this comment.
className silently dropped — Vega renderer called as a JSX component
Vega's default export (Vega.tsx line 54) follows the two-argument renderer pattern (data, props) => <Vega ... />, not a standard single-props React component. When used as <Vega className={classes.chart} spec={vegaLite} />, React calls it with a single props object { className, spec } as the first argument. Only { spec } is destructured from that first arg; the second parameter props receives undefined (React's legacy second arg for functional components). The className={classes.chart} (marginTop: t.spacing(2)) is therefore silently dropped, so the chart renders flush against the metadata block without the intended spacing.
To correctly forward className and any other HTML props, call it as a renderer function instead:
{!!vegaLite && Vega({ spec: vegaLite }, { className: classes.chart })}Or, alternatively, import the internal Vega component directly if it becomes exported separately.
Vega's default export is a renderer function (data, props), not a
standard React component. JSX usage <Vega className={...} spec={...}/>
collapses both into the first arg, so className was silently dropped
and the chart rendered flush against the metadata block.
Wrap Vega in a div that carries the marginTop class, so the spec
renders with the intended spacing without depending on Vega's prop
forwarding behavior.
Addresses Greptile P2 on quilt#4860.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@Austin-s-h heads-up on a new PR for code you originally wrote. What this is One change from your version
Status
|
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Dr. Ernie Prabhakar <ernest@quilt.bio>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Maksim Chervonnyi <mail@redmax.dev>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Sergey Fedoseev <fedoseev.sergey@quiltdata.io>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…view (#4898) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…4895) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…dexer (#4894) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…rts (#4892) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…4890) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Alexei Mochalov <nl_0@quiltdata.io>
…4899) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…4900) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…60427-catalog-fcs-vega
…859-fcs-consumers
…27-catalog-fcs-vega
…-4859-fcs-consumers
…nto 260427-catalog-fcs-vega
The h5ad handler intermittently fails with h5py's "truncated file" OSError when the underlying HTTP read tears mid-transfer. The condition is non-deterministic — the same file/URL succeeds on a fresh attempt — so a single bounded retry hides the flap. Structural errors (ValueError / KeyError from a genuinely malformed h5ad) still go straight to the error envelope. Also include the (query-stripped) URL in the warning log so post-hoc debugging can distinguish a transient transport hiccup from a file that is actually broken. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Wire the optional
vegaLitefield from preview info responses throughPreviewData.Fcsand render it as a Vega chart below the metadata block.The renderer guards on
!!vegaLite, so the field is a soft contract: backends that don't emit it produce no chart, and this PR is a no-op until the lambda shared layer (#4858) lands.Sibling PRs
This is one of three independent PRs split from the original FCS preview work — any landing order works:
vegaLitespecindexer,preview, …)Changes
catalog/app/components/Preview/loaders/Fcs.js— threadvegaLitefrom preview info intoPreviewData.Fcscatalog/app/components/Preview/renderers/Fcs.jsx— wrapVegain a spacing div and render whenvegaLiteis present (Greptile P2 fix: avoid passingclassNamethrough Vega's renderer-style default export)catalog/app/components/Preview/types.js— document optionalvegaLitefieldcatalog/app/components/Preview/loaders/Fcs.spec.tsx— loader testcatalog/app/components/Preview/renderers/Fcs.spec.tsx— renderer test (chart present/absent conditional)Validation
cd catalog && npx vitest run app/components/Preview/loaders/Fcs.spec.tsx app/components/Preview/renderers/Fcs.spec.tsxvegaLite.Deployment context
Tracked in deployment#2395; catalog image bumped via deployment#2394.
Co-authors
Original work by @Austin-s-h (Alexion); split out from #4858 for review clarity.