feat(visualization): add variant track (Phase 6.4) - #30
Merged
Conversation
Reviewer's GuideAdds a dedicated, reusable variant track to the Genome Browser that fetches variants via the coordinate-search API, maps them into an enriched VariantFeature/Variant domain model, computes point-mark geometry with row stacking, and renders an accessible VariantTrack component with keyboard-selectable marks and a detail panel, alongside documentation for Phase 6.4 and an External Data & API master plan. Sequence diagram for the new VariantTrack data flowsequenceDiagram
actor User
participant GenomeBrowserDemo
participant GenomeBrowser
participant BrowserTrack
participant VariantTrack
participant useGenomeTrack
participant fetchVariants
participant requestCoordinateSearch
participant BackendSearchAPI
User ->> GenomeBrowserDemo: open /visualization
GenomeBrowserDemo ->> GenomeBrowser: render with tracks (kind variants)
GenomeBrowser ->> BrowserTrack: render track
BrowserTrack ->> VariantTrack: track kind variants
VariantTrack ->> useGenomeTrack: useGenomeTrack(track, debouncedViewport)
useGenomeTrack ->> fetchVariants: loader(interval, signal)
fetchVariants ->> requestCoordinateSearch: requestCoordinateSearch(variant, interval, signal, pageSize)
requestCoordinateSearch ->> BackendSearchAPI: POST /search/variant/coordinate
BackendSearchAPI -->> requestCoordinateSearch: items: RawSearchItem[]
requestCoordinateSearch -->> fetchVariants: RawSearchItem[]
fetchVariants -->> useGenomeTrack: Variant[]
useGenomeTrack -->> VariantTrack: data (status, label, data)
VariantTrack ->> VariantTrack: variantsInViewport / layoutVariantMarks
VariantTrack ->> User: render SVG marks + VariantDetail
User ->> VariantTrack: click / keypress on variant mark
VariantTrack ->> VariantTrack: handleSelect(variant)
VariantTrack ->> User: update selected mark + detail panel
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Warning Review limit reached
Next review available in: 115 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds Phase 6.4 variant visualization. The change introduces typed variant normalization, a coordinate-search adapter, pixel geometry and row stacking, an interactive ChangesVariant Visualization
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: ⚪ Minimal · up to The PR adds localized variant visualization behavior and documentation, with no actionable merge-blocking risk remaining beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant GenomeBrowserDemo
participant BrowserTrack
participant VariantTrack
participant fetchVariants
participant CoordinateSearchAPI
participant VisualizationContainer
GenomeBrowserDemo->>BrowserTrack: provide variant track and viewport
BrowserTrack->>VariantTrack: render variant track
VariantTrack->>fetchVariants: request viewport interval
fetchVariants->>CoordinateSearchAPI: post coordinate-overlap search
CoordinateSearchAPI-->>fetchVariants: return raw variant records
fetchVariants-->>VariantTrack: return normalized valid variants
VariantTrack->>VisualizationContainer: render marks and lifecycle state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The normalization logic in
toVariant(variant.ts) andtoVariantFeature(api.ts) is nearly identical; consider consolidating into a single shared helper to avoid drift if the variant schema changes. - In
VariantTrack,data.datais coerced toVariantFeature[]with a cast; if possible, thread a more specific type throughuseGenomeTrackor theloadertyping so you don’t need an unchecked cast at the adapter boundary.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The normalization logic in `toVariant` (variant.ts) and `toVariantFeature` (api.ts) is nearly identical; consider consolidating into a single shared helper to avoid drift if the variant schema changes.
- In `VariantTrack`, `data.data` is coerced to `VariantFeature[]` with a cast; if possible, thread a more specific type through `useGenomeTrack` or the `loader` typing so you don’t need an unchecked cast at the adapter boundary.
## Individual Comments
### Comment 1
<location path="apps/web/src/lib/genome/variant.ts" line_range="45-48" />
<code_context>
+/**
+ * Normalizes a raw variant record from the coordinate-search API.
+ *
+ * Mirrors `toVariantFeature` in `lib/genome/api.ts` so the model module is
+ * self-contained; the shared request pipeline remains in `api.ts`.
+ */
+export function toVariant(item: RawSearchItem): Variant {
+ const position = asNumber(item.position)
+ const chromosome = asString(item.chromosome)
</code_context>
<issue_to_address>
**suggestion:** Reduce duplication between `toVariant` and `toVariantFeature` to avoid divergent mapping logic.
This normalization closely duplicates `toVariantFeature` in `api.ts` (same coercions, name building, optional fields, etc.), which makes it easy for them to diverge (e.g., new or changed fields applied in one place but not the other). Please consider centralizing the mapping logic—either via a shared mapper that returns `VariantFeature` and is reused here, or by extracting the common helpers (`asString`, `asNumber`, `idOf`, object construction) into a shared module—so future API changes only need to be maintained in one location.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/web/src/components/genome/VariantTrack.test.tsx (1)
94-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExecute the keyboard-selection handlers.
This test checks ARIA attributes but does not execute the Enter or Space branches in
VariantTrack.tsxlines 105-110. Add keydown assertions for both keys. Verify that each key changesaria-pressedand the detail panel state.Proposed test extension
expect(control).toHaveAttribute('tabindex', '0') expect(control).toHaveAttribute('aria-pressed', 'false') + fireEvent.keyDown(control, { key: 'Enter' }) + expect(await screen.findByTestId('variant-detail-var-a')).toBeInTheDocument() + fireEvent.keyDown(control, { key: ' ' }) + await waitFor(() => + expect(screen.queryByTestId('variant-detail-var-a')).not.toBeInTheDocument(), + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/genome/VariantTrack.test.tsx` around lines 94 - 101, Extend the keyboard-accessibility test around the variant selection control returned by renderTrack to dispatch keydown events for both Enter and Space. After each key event, assert that aria-pressed changes and verify the detail panel opens or updates accordingly, covering both keyboard branches in VariantTrack.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/genome/GenomeBrowser.tsx`:
- Around line 229-231: Refactor BrowserTrack so the variants branch is rendered
by a separate child component, keeping useGenomeTrack called unconditionally
within the non-variant path. Add a rerender test that preserves the same track
id while changing track.kind between variant and non-variant values, and verify
rendering completes without a hook-order error.
In `@apps/web/src/lib/genome/variant.ts`:
- Around line 77-80: Update isValidVariant to require a non-empty variant
identity in addition to the existing chromosome and position checks, rejecting
records whose id is empty or missing. Ensure normalization provides a stable
unique identity when appropriate, and add a regression test covering API records
without id or variant_id so they cannot share selection or React-key identity.
In `@docs/external-data/MASTER_PLAN.md`:
- Around line 349-352: Update the Phase 6.4 Variant Viewer entry in the
governing plan to mark it implemented rather than “next,” and distinguish the
completed frontend work from the remaining live external-source integration
through the GenomeAI API. Preserve the existing external-source routing
requirement and leave Phase 6.5 unchanged.
- Around line 11-46: docs/external-data/MASTER_PLAN.md lines 11-46, 88-110,
133-144, 162-166, 175-185, 193-196, 204-213, 222-224, 232-236, 244-248, 262-275,
283-293, 301-313, 324-329, 415-430, and 442-456: add the text language tag to
each diagram or example fence; use python for the interface fence at lines
114-123. docs/external-data/README.md lines 9-11: add the text language tag to
the architecture fence. Ensure every listed fenced Markdown block has an
explicit appropriate language tag.
In `@docs/visualization/variant.md`:
- Around line 159-168: Update the Validation section in variant.md to visibly
document that live coordinate-search responses currently contain
non-serializable raw SQLAlchemy objects and therefore live variant data is
blocked. Add the relevant issue link if available, and explicitly state whether
the demo relies on fixtures or mocked responses.
---
Nitpick comments:
In `@apps/web/src/components/genome/VariantTrack.test.tsx`:
- Around line 94-101: Extend the keyboard-accessibility test around the variant
selection control returned by renderTrack to dispatch keydown events for both
Enter and Space. After each key event, assert that aria-pressed changes and
verify the detail panel opens or updates accordingly, covering both keyboard
branches in VariantTrack.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e1b482c-a9af-4720-811d-ee6e450ef6c8
📒 Files selected for processing (20)
README.mdapps/web/src/app/visualization/GenomeBrowserDemo.tsxapps/web/src/app/visualization/page.tsxapps/web/src/components/genome/GenomeBrowser.tsxapps/web/src/components/genome/VariantTrack.test.tsxapps/web/src/components/genome/VariantTrack.tsxapps/web/src/lib/genome/api.test.tsapps/web/src/lib/genome/api.tsapps/web/src/lib/genome/types.tsapps/web/src/lib/genome/variant.test.tsapps/web/src/lib/genome/variant.tsapps/web/src/lib/genome/variantApi.test.tsapps/web/src/lib/genome/variantApi.tsapps/web/src/lib/genome/variantGeometry.test.tsapps/web/src/lib/genome/variantGeometry.tsdocs/external-data/MASTER_PLAN.mddocs/external-data/README.mddocs/visualization/README.mddocs/visualization/roadmap.mddocs/visualization/variant.md
Comment on lines
+77
to
+80
| export function isValidVariant(variant: Variant): boolean { | ||
| return ( | ||
| variant.chromosome.length > 0 && Number.isSafeInteger(variant.position) && variant.position >= 1 | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require a non-empty variant identity.
Line 77 accepts variants with id === ''. VariantTrack uses id as both the React key and the selected-variant identity. If the API returns two records without id or variant_id, users cannot select them independently.
Reject records without an identity, or assign a stable unique identity during normalization. Add a regression test for this response shape.
Proposed fix
export function isValidVariant(variant: Variant): boolean {
return (
- variant.chromosome.length > 0 && Number.isSafeInteger(variant.position) && variant.position >= 1
+ variant.id.length > 0 &&
+ variant.chromosome.length > 0 &&
+ Number.isSafeInteger(variant.position) &&
+ variant.position >= 1
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function isValidVariant(variant: Variant): boolean { | |
| return ( | |
| variant.chromosome.length > 0 && Number.isSafeInteger(variant.position) && variant.position >= 1 | |
| ) | |
| export function isValidVariant(variant: Variant): boolean { | |
| return ( | |
| variant.id.length > 0 && | |
| variant.chromosome.length > 0 && | |
| Number.isSafeInteger(variant.position) && | |
| variant.position >= 1 | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/lib/genome/variant.ts` around lines 77 - 80, Update
isValidVariant to require a non-empty variant identity in addition to the
existing chromosome and position checks, rejecting records whose id is empty or
missing. Ensure normalization provides a stable unique identity when
appropriate, and add a regression test covering API records without id or
variant_id so they cannot share selection or React-key identity.
✅ Addressed in commit 4ddd0c0
10 tasks
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
1 participant
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.
Summary
Adds the Phase 6.4 Variant Visualization track to the Genome Browser. Variants render as point marks over the coordinate-search API, with row stacking, hover tooltips, keyboard-accessible selection, and a semantic detail panel.
Also adds the official External Data & API Master Plan (
docs/external-data/) as the governing reference for external-source integration across Phases 4–9.Changes
New
apps/web/src/lib/genome/variant.ts— variant model (toVariant, labels, detail lines)apps/web/src/lib/genome/variantGeometry.ts— pure point-mark geometry + row stackingapps/web/src/lib/genome/variantApi.ts—fetchVariantsadapter over the coordinate-search APIapps/web/src/components/genome/VariantTrack.tsx— point-mark track with selection + detail panelvariant.test.ts,variantGeometry.test.ts,variantApi.test.ts,VariantTrack.test.tsxdocs/visualization/variant.md,docs/external-data/MASTER_PLAN.md,docs/external-data/README.mdEnriched
lib/genome/types.ts+lib/genome/api.ts—VariantFeaturegainsvariantId,variantType,quality,filterStatus,geneId,descriptionGenomeBrowser.tsxdelegateskind: 'variants'tracks toVariantTrackGenomeBrowserDemo.tsxwiresfetchVariants;page.tsxmetadata updated to Phase 6.1–6.4Validation
tsc --noEmitclean · 186/186 web tests pass ·make buildsucceedsKnown issue (pre-existing, not from this PR)
The backend search endpoints return raw SQLAlchemy ORM objects in
items: list[Any], so any search returning rows fails JSON serialization withPydanticSerializationError(500), and/search/suggestionsfails on aSELECT DISTINCT ... ORDER BYerror. This blocks live-data gene/variant tracks end-to-end; the fixture-based gene/transcript viewer renders correctly. A separate backend fix is required.Summary by Sourcery
Add a reusable, coordinate-accurate variant track to the Genome Browser and wire it to the coordinate-search API, enriching the variant data model and documentation.
New Features:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
New Features
Documentation