Repository navigation
fix(live): keep a child's results veiled when a fetch fails, and let one bad row cost one card - #345
Merged
Merged
Conversation
…unknown A finding page whose state.json or frames.json did not answer, a person page whose state came without plugins, and a sample that no readable family lists while another family's pedigree cannot be read were all read as a standalone adult, so a child's result to act on showed without the "Show the result" step. Each is now an unknown role and stays closed; opening one finding keeps the unknown role for the rest of that page. The person page redraws when the family lookup starts, and the header search leaves out the hits and suggestions of a card the page keeps closed. A member summary of an unexpected shape costs its own column of the family page, not the page. Epic: pgp-5bja
…s page A ClinVar hit whose stars was not a number made finding_rows raise, and results.json fell back to a note with no card at all. Each rule now reads its rows one at a time: a row it cannot read becomes a finding worth a look on its own card that names no gene, and every other card and finding renders. The finding route test now logs in with the finding as next and checks the login lands back on it. Epic: pgp-5bja
The parsed-trace cache keyed on mtime and size alone, so a trace file replaced by another of the same size and mtime kept its old rows. The key now holds the device, inode and ctime too. A duration cell such as 1.2.3s raised and lost the whole trace file; it now costs that row its end time only. A trace removed between the listing and the read is skipped. Epic: pgp-5bja
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 49 minutes. View limit details
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. |
…hild's card stays veiled A rule that can give a finding to act on (secondary findings, CPSR, two hits, AnnotSV, expansions, VNtyper, carriers) now turns a row it cannot read into a placeholder at that level. The card adapter may still draw the row in the card's table, and a child's page veils a card only when it holds a finding to act on. The placeholder names no gene, so an adult's page loses nothing.
# Conflicts: # docs/live-site.md # docs/multi-sample.md # tests/test_live_demo.py
GeiserX
pushed a commit
that referenced
this pull request
Nov 9, 2026
… 10-minute limit (#351) Guard's `unit-tests` job runs every `tests/test_*` file in one job with a 10-minute limit. On main it now takes 9m17s to 9m54s, and it was cancelled at the limit on 97aa0f1 and on #345's head, so `ci-ok` failed for reasons unrelated to the change under test. The job now runs as three shards. File *i* in the sorted glob runs in shard *i* mod 3, so every file runs in exactly one shard, and a new test file still needs no workflow edit. Each shard keeps the two planted-failure controls and prints how many of the files it ran. `ci-ok` still lists `unit-tests` in `needs`; a matrix job reports failure there if any shard fails, and the ruleset requires only `ci-ok`. Checked locally by running the loop with stubbed interpreters: the three shards hold 37 files each, their union is the full list of 111 with no duplicates, and a mutated shard expression fails the same check. The first sharded run also hit a known flake: in `tests/test_owngenome_live_family.py`, a headless Chrome never answered its first DevTools call (`Target.createTarget: no answer in 30 s`). The same error failed Guard runs 42609845049 and 42082898427 on earlier PRs. The test helper now kills that Chrome and starts a fresh one, up to three times. Checked locally with a fake Chrome that hangs on its first start and runs the real one after that: the helper on main raises `no answer in 30 s`, while the new one retries once and evaluates `1+1` to 2. E2E evidence: https://github.com/geiresx/OwnGenome/actions/runs/42652467614
# Conflicts: # bin/owngenome_live_family.py # docs/multi-sample.md # tests/test_live_family_depth.py
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.
The problem
A child's result to act on is supposed to stay closed behind "Show the result" until someone opens it on purpose. Three paths read an unknown family role as a standalone adult and showed it anyway:
state.json(orframes.jsonin a replay) did not answer: the title, gene and variant were on the page;pluginsobject;The header search also listed a closed card's genes and variants, and suggested its gene names as you typed.
Separately, one malformed row could blank a page. A ClinVar hit whose
starswas not a number madelive_results.finding_rowsraise, andresults.jsonfell back to a note with no card at all. A membersummary.jsonof an unexpected shape madefamily.jsonfail. Inbin/run_state.py, a duration cell such as1.2.3slost the whole trace file, and a trace replaced by another file with the same size and mtime kept its old rows in the parsed-trace cache.The fix
assets/live/finding.js: no state, or a state with no plugins, or a family plugin that saysunknown, is an unknown role: the finding stays closed. Opening the finding keeps that unknown role, so the next link and the related list do not open other results to act on.assets/live/person.js: the same rule for a state with no plugins or anunknownfamily. The redraw key now includes the family lookup status, so the page closes results the moment the lookup starts. The page exposesPGP.searchHiddenfor the search.assets/live/plugins/search.js: leaves out hits from cards the page keeps closed, and suggested terms only those cards name. On a closed page it says so whether or not anything was left out, so the note itself names nothing.bin/owngenome_live_family.py:member_stateanswers{"kind": "member", "families": [], "unknown": true}when no readable family lists the sample and some family cannot be read. This matches what the home page already does. The family card says why.side_by_sidemarks a member whose summary cannot be read as "summary not readable" in that member's column only.bin/live_results.py: each rule reads its rows one at a time. A row it cannot read becomes a finding on its own card, "A row of this section could not be read", which names no gene. Every other card and finding stays. The card's adapter already said "could not be drawn" for a table it cannot draw. The placeholder takes the highest level its rule can give: "to act on" for secondary findings, CPSR, two hits, AnnotSV, repeat expansions, VNtyper and carriers, "worth a look" for the rest. The card's table can still draw a row the rule could not read (an AnnotSV class-5 row whose gene list holds a null draws its first three genes), and a child's page veils a card only when it holds a finding to act on. So the placeholder keeps that card closed on a child's page and costs nothing on an adult's.bin/run_state.py: the trace cache key adds device, inode and ctime.parse_durationreads only well-formed numbers.trace_filesskips a file that vanishes between the listing and the read.The parsed-trace cache for
/runs.jsonthat the audit asked for was already on main (TRACE_CACHE, keyed on path, mtime and size). This PR only widens its key.Output changes:
state.jsonplugins.familycan now carry"unknown": true.results.jsoncan hold a finding with an idunreadable-<card>-<n>, andfamily.jsonside-by-side values can read "summary not readable". These are new values in existing fields. Nothing is renamed or removed.Decision: when a family's pedigree cannot be read, a sample that a readable family lists keeps that family's role. Only unlisted samples become unknown. This is the smallest change that stays consistent with the home page's existing rule.
How it was checked
Every new check failed on main before the fix and passes after it. Each also has an in-test control that plants the old code and must fail.
tests/test_live_demo.py(headless Chrome, demo son DEMO-003 with two planted CPSR results to act on):family.jsonunavailable.frames.jsonunavailable). Red on main: the finding page showed its title, gene and variant.tests/test_live_journal.pycheck 5: a badstarsvalue, a CPSR row that is a string and a carrier row that is a list. Red on main:build raised AttributeError: 'list' object has no attribute 'get'. The same check now adds an AnnotSV class-5 row whose gene list holds a null, and requires each act-capable rule's unreadable row to come out "to act on". Its control hard-codes the guard's level to "worth a look" and fails (the AnnotSV card with an unreadable class-5 row holds no finding to act on: ['review']). Check 3 now posts the login with the finding URL asnextand asserts thatLocationpoints back to it.tests/test_live_family_depth.pycheck 7: three malformed member summaries. Red on main: AttributeError, AttributeError, TypeError.tests/test_owngenome_live_family.py: an unreadable family makes an unlisted sampleunknown. Red on main:None.tests/test_run_state_trace_cache.pychecks 6 and 7: a same-size, same-mtime replacement and a garbled duration. Red on main: the stale statuses, andcould not convert string to float: '.'.The related live, run-state and report-chart test files all pass with the fix.
CI on head b67fa02:
tests/test_*.py,test_live_demo.pyin headless Chrome included), Lint, Nextflow, Container Test, Personal data and Docs, green: https://github.com/GeiserX/OwnGenome/actions/runs/42612868223Beads
Epic pgp-5bja. The audit follow-ups from #333 and #335 that this package carries.