Skip to content

Catalog: render FCS Vega scatter from preview vegaLite spec#4860

Open
drernie wants to merge 37 commits into
preview-lambda-consumersfrom
260427-catalog-fcs-vega
Open

Catalog: render FCS Vega scatter from preview vegaLite spec#4860
drernie wants to merge 37 commits into
preview-lambda-consumersfrom
260427-catalog-fcs-vega

Conversation

@drernie
Copy link
Copy Markdown
Member

@drernie drernie commented Apr 27, 2026

Summary

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

PR Layer
#4858 Shared lambda parsing + vegaLite spec
#4859 Consumer lambdas (indexer, preview, …)
#4860 (this) Catalog Vega chart rendering

Changes

  • catalog/app/components/Preview/loaders/Fcs.js — thread vegaLite from preview info into PreviewData.Fcs
  • catalog/app/components/Preview/renderers/Fcs.jsx — wrap Vega in a spacing div and render when vegaLite is present (Greptile P2 fix: avoid passing className through Vega's renderer-style default export)
  • catalog/app/components/Preview/types.js — document optional vegaLite field
  • catalog/app/components/Preview/loaders/Fcs.spec.tsx — loader test
  • catalog/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.tsx
  • Manual: load an FCS file in catalog preview; chart appears only when backend emits vegaLite.

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.

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
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

❌ Patch coverage is 70.47757% with 204 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (preview-lambda-consumers@d1fa6c1). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...alog/app/components/Assistant/UI/Chat/DevTools.tsx 2.00% 45 Missing and 4 partials ⚠️
...g/app/components/Assistant/Model/Connectors/Mcp.ts 74.83% 37 Missing and 2 partials ⚠️
catalog/app/components/Assistant/UI/Chat/Chat.tsx 5.12% 31 Missing and 6 partials ⚠️
...talog/app/components/Assistant/Model/Assistant.tsx 11.53% 20 Missing and 3 partials ⚠️
...omponents/Assistant/Model/Connectors/Connectors.ts 92.57% 17 Missing and 2 partials ⚠️
...omponents/Assistant/Model/GlobalContext/preview.ts 13.33% 10 Missing and 3 partials ⚠️
...alog/app/containers/Admin/Settings/ThemeEditor.tsx 74.19% 5 Missing and 3 partials ⚠️
...log/app/components/Assistant/Model/Conversation.ts 88.23% 4 Missing ⚠️
catalog/app/components/Assistant/UI/Chat/Input.tsx 25.00% 2 Missing and 1 partial ⚠️
...app/components/Assistant/UI/Chat/MessageAction.tsx 25.00% 3 Missing ⚠️
... and 3 more
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           
Flag Coverage Δ
api-python 93.14% <ø> (?)
catalog 21.79% <69.95%> (?)
lambda 96.90% <100.00%> (?)
py-shared 98.18% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

<div className={className} {...props}>
{renderWarnings(warnings)}
{!!metadata && <JsonDisplay name="Metadata" value={metadata} />}
{!!vegaLite && <Vega className={classes.chart} spec={vegaLite} />}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 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>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@drernie
Copy link
Copy Markdown
Member Author

drernie commented Apr 28, 2026

@Austin-s-h heads-up on a new PR for code you originally wrote.

What this is
The catalog FCS frontend wiring you bundled into #4858 was extracted into this sibling PR so #4858 can stay scoped to the shared lambda layer. The 5 catalog files here (loaders/Fcs.js, renderers/Fcs.jsx, types.js, plus the two .spec tests) are unchanged from your branch except for one Copilot-driven fix.

One change from your version

  • Fcs.jsx now wraps <Vega> in a spacing div instead of passing className={classes.chart} to <Vega> directly. Reason: Vega's default export is a renderer-style function (data, props), not a standard React component, so JSX usage <Vega className=… spec=…/> collapses both into the first arg and silently drops className. The wrapper div carries the marginTop so the chart renders with the intended spacing. Commit 4c828bd4.

Status

  • CI: ✅ all 44 checks green
  • Copilot review errored (GitHub-side issue)
  • Independent of shared: improve FCS parsing and preview metadata #4858: renderer guards on !!vegaLite, so this PR is a no-op until backends emit the field; either PR may land first
  • I added you as a reviewer; let me know if anything looks off

nl0 and others added 5 commits April 29, 2026 18:30
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: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Sergey Fedoseev <fedoseev.sergey@quiltdata.io>
@drernie drernie changed the base branch from master to preview-lambda-consumers May 1, 2026 21:35
drernie and others added 14 commits May 7, 2026 01:40
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>
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>
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>
)

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

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
dependabot Bot and others added 6 commits May 12, 2026 10:41
…4890)

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>
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>
drernie and others added 10 commits May 15, 2026 11:00
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>
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.

5 participants