SCIX-840 fix(vis): prevent author network crash on non-year-prefixed bibcodes#819
Open
thostetler wants to merge 1 commit intoadsabs:masterfrom
Open
SCIX-840 fix(vis): prevent author network crash on non-year-prefixed bibcodes#819thostetler wants to merge 1 commit intoadsabs:masterfrom
thostetler wants to merge 1 commit intoadsabs:masterfrom
Conversation
Bibcodes that don't start with a 4-digit year (e.g. searches like
"potato AND citation_count:20") caused parseInt to return NaN, which
propagated through d3.extent into range(NaN, NaN), throwing a runtime
error that crashed the author network visualization.
- Filter NaN values from years array after parseInt
- Guard against undefined d3.extent result (skip group with no valid years)
- Wrap getAuthorNetworkSummaryGraph body in try/catch, returning { error }
instead of throwing on unexpected data shapes
- Return data: [] (not undefined) on all error paths to satisfy ILineGraph type
- Fix container useMemo fallback: return { data: [] } when authorNetworkData
is not yet loaded, preventing undefined being cast to ILineGraph
- Fix SummaryPane: render error message OR graph, not both (error state was
already shown but LineGraph still rendered with undefined/empty data)
thostetler
force-pushed
the
fix/author-network-error-handling
branch
from
8570444 to
da84fcc
Compare
thostetler
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #819 +/- ##
========================================
- Coverage 62.0% 61.9% -0.0%
========================================
Files 317 317
Lines 36521 36530 +9
Branches 1642 1642
========================================
- Hits 22608 22600 -8
- Misses 13876 13893 +17
Partials 37 37
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the Author Network “Group Activity Over Time” summary graph generation and rendering to avoid crashes when bibcodes don’t start with a 4-digit year, and to surface errors in the UI instead of throwing.
Changes:
- Filter invalid years before computing year extents and skip groups with no valid year range.
- Wrap
getAuthorNetworkSummaryGraphintry/catchand return{ data: [], error }on failures. - Ensure containers/panes render either an error message or the graph (and provide a
{ data: [] }fallback).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/components/Visualizations/utils/graphUtils.ts |
Adds NaN filtering + guards for year range and wraps summary graph generation in try/catch. |
src/components/Visualizations/Panes/NetworkDetails/SummaryPane.tsx |
Renders error state or summary line graph (mutually exclusive). |
src/components/Visualizations/Containers/AuthorNetworkPageContainer.tsx |
Ensures the summary graph memo returns a { data: [] } fallback instead of undefined. |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+448
to
+464
| // years range — filter NaN in case bibcodes don't start with a 4-digit year | ||
| const years = uniq(bibcodes.map((bibcode) => parseInt(bibcode.slice(0, 4))).filter((y) => !isNaN(y))); | ||
| const yearsRange = d3.extent(years); | ||
| if (yearsRange[0] === undefined || yearsRange[1] === undefined) { | ||
| return; | ||
| } | ||
| const allYears = range(yearsRange[0], yearsRange[1]); // fill in the years gap | ||
|
|
||
| // prefill all years with 0 count values { year: count} | ||
| const skeleton: { [year in string]: number } = {}; | ||
| allYears.forEach((year) => (skeleton[year.toString()] = 0)); | ||
| // prefill all years with 0 count values { year: count} | ||
| const skeleton: { [year in string]: number } = {}; | ||
| allYears.forEach((year) => (skeleton[year.toString()] = 0)); | ||
|
|
||
| // into year and paper count array [ ... {year: count} ] | ||
| const yearPaperCount = { | ||
| ...skeleton, | ||
| ...countBy((bibcode) => bibcode.slice(0, 4), bibcodes), | ||
| }; | ||
| // build year→count map, merging skeleton with actual bibcode counts | ||
| const yearPaperCount = { | ||
| ...skeleton, | ||
| ...countBy((bibcode) => bibcode.slice(0, 4), bibcodes), | ||
| }; |
There was a problem hiding this comment.
The years filtering only removes NaN results from parseInt, but parseInt can still return a number for non-4-digit prefixes (e.g. '202A' → 202). Also, yearPaperCount is built from all bibcodes, so non-year-prefixed bibcodes can still end up as keys (x-values) in graphData, which can break the line graph (especially with xScaleType="linear"). Filter bibcodes up front using a strict 4-digit year check (e.g. /^\d{4}/) and then use that filtered list consistently for both the years/extent computation and the countBy map.
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.
Uh oh!
There was an error while loading. Please reload this page.