Skip to content

DEVSU-3003 Do not display variant in TGR moves to VUS - #512

Open
bnguyen-bcgsc wants to merge 3 commits into
developfrom
bugfix/DEVSU-3003-do-not-display-variant-moves-variant-to-vus
Open

DEVSU-3003 Do not display variant in TGR moves to VUS#512
bnguyen-bcgsc wants to merge 3 commits into
developfrom
bugfix/DEVSU-3003-do-not-display-variant-moves-variant-to-vus

Conversation

@bnguyen-bcgsc

Copy link
Copy Markdown
Contributor
  • DEVSU-3003
  • Fix bug where variants with noTable tag gets included in table of unknown significance variants instead
  • Update unit test

- Fix bug where variants with noTable tag gets included in table of unknown significance variants instead
- Update unit test
@bnguyen-bcgsc
bnguyen-bcgsc requested review from elewis2, kttkjl and sshugsc and a lite review from Copilot August 25, 2026 22:54
@bnguyen-bcgsc bnguyen-bcgsc self-assigned this Aug 25, 2026
@bnguyen-bcgsc bnguyen-bcgsc added the bug Something isn't working label Aug 25, 2026
@github-actions

This comment has been minimized.

@codecov-commenter

codecov-commenter commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.86%. Comparing base (1a3e0a2) to head (a6bac45).

Files with missing lines Patch % Lines
app/routes/report/variants.js 77.77% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #512      +/-   ##
===========================================
+ Coverage    76.83%   76.86%   +0.03%     
===========================================
  Files          190      190              
  Lines         7029     7034       +5     
  Branches       920      921       +1     
===========================================
+ Hits          5401     5407       +6     
+ Misses        1530     1529       -1     
  Partials        98       98              

☔ View full report in Codecov by Harness.
📢 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.

Copilot AI left a comment

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.

Pull request overview

This PR addresses DEVSU-3003 by ensuring variants explicitly tagged noTable are not included in the Rapid Report “unknown significance” (table 3) output when they would otherwise be pulled in via gene-property qualification (especially when there are no KB matches). It also adds a regression test to cover the scenario.

Changes:

  • Exclude noTable-tagged variants from the unknown-significance results even when gene properties qualify them.
  • Track noTable-tagged variant idents to prevent re-adding them during gene-property backfill and to filter them out at return.
  • Add a unit/integration test covering “no KB matches + gene qualifies + noTable tag” behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
app/routes/report/variants.js Adds noTable ident tracking to prevent gene-property-based inclusion into unknown significance, and filters out noTable results.
test/routes/report/variants.test.js Adds a regression test ensuring a noTable-tagged variant with no KB matches does not appear in table 3.
Suppressed comments (2)

app/routes/report/variants.js:192

  • Avoid using console.log in the API route; the rest of this module uses the shared Winston logger. Also, doNotReport is not used, so pushing into it here is dead code.
        } else if (tableTag === 'noTable') {
          console.log(`Variant ${variant.ident} is tagged as noTable, will not be reported`);
          doNotReport.push(variant);
          doNotReportIdents.add(variant.ident);
        }

app/routes/report/variants.js:130

  • doNotReport is never read, so this push is dead code and can be removed (the ident Set is sufficient for filtering).
      if (variant?.observedVariantAnnotation?.annotations?.rapidReportTableTag === 'noTable') {
        doNotReport.push(variant);
        doNotReportIdents.add(variant.ident);
        continue;
      }

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread app/routes/report/variants.js Outdated
bnguyen-bcgsc and others added 2 commits August 25, 2026 15:58
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

Unit Test Results

    1 files  ±0    64 suites  ±0   4m 5s ⏱️ -8s
659 tests +1  658 ✔️ +1  1 💤 ±0  0 ❌ ±0 
656 runs  +1  655 ✔️ +1  1 💤 ±0  0 ❌ ±0 

Results for commit a6bac45. ± Comparison against base commit 1a3e0a2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants