[DT-3313] [2/6] Derive filter options from the full corpus, not the current page

[kevinmarete]

Addresses

DT-3313 — 2 of 6, splitting #3926 per review. Stacked on #3935. Security risk: no.

Summary

This is the one carrying the risk, so it lands against the existing filter set only — no new filter keys — as suggested in review.

Workspace Tools, Workspace Platform, Registry and Optional Data Use derived their options from data.items, which only holds the active tab's current page. They listed whatever happened to be on screen, and went empty the moment the user switched tabs. That's the bug this fixes.

They now come from the whole matching corpus. tabCountsResponse already carries every matching study via STUDIES_AGG's terms.size: 10000, so re-running an asset's own transformResponse with an unbounded page size yields its full flattened item list whichever tab is rendering. This mirrors libraryCounts.ts, which already calls transformResponse a second time with different pagination purely to read .total. To be explicit, since it came up in review: that unbounded page size is client-side pagination over a response already in memory, not an Elasticsearch request.

The visible tab additionally reads a corpus with its own filter keys cleared, which is the part worth the attention. The filter clauses match whole studies, so selecting one value drops every study without it, and a sibling value living only in one of those studies could never be offered again — no client-side pass can recover a study the response never carried. Filters owned by other tabs stay applied, so the options stay scoped the way the grid is. The clearing is a no-op until the tab actually has one of its own filters set, so the second useLibraryTabCounts shares the counts query's key, and its request, until it does.

A value excluded by another tab's filter is absent from that corpus too, since that filter stays applied. withSelectedValues re-adds anything currently selected, or the checkbox disappears while the filter stays active — and the external chips deliberately skip keys the current tab renders itself, so nothing would be left to clear it with short of resetting every filter.

Eight new tests. Mutation-checked rather than trusted on coverage: collapsing the corpus map to the unfiltered response fails the self-exclusion test, and short-circuiting withSelectedValues fails its own. The first pass of tests missed withSelectedValues entirely — every case I'd written was already satisfied by self-exclusion — so the test that isolates it uses a filter owned by another tab, which is the only situation where self-exclusion cannot help.

[github-actions]

Coverage Report for DUOS Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	90.37%	12043 / 13327
🔵	Statements	89.86%	12856 / 14307
🔵	Functions	87.51%	3558 / 4066
🔵	Branches	82.72%	7482 / 9045

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/components/data_library/filterRegistry.ts	86.38%	79.59%	96.49%	86.26%	159, 221, 275-282, 452-459, 504-508, 528-542, 558-567, 603-612, 621-635, 690, 724
src/hooks/useLibraryPageState.ts	91.11%	81.48%	88.37%	90.47%	231, 234, 242, 279, 294, 314-318
src/hooks/useLibraryTabCounts.ts	100%	75%	100%	100%

Generated in workflow #7218 for commit b25f2be by the Vitest Coverage Report Action

[Copilot]

🟡 Changes recommended

Address the two moderate findings and add the requested clinical-trial and biospecimen test coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR updates Data Library filters to derive options from the full matching corpus instead of the current page while preserving selected values.

Changes:

• Adds full-corpus queries with active-tab filters cleared.
• Recomputes workspace, clinical-trial, and biospecimen options.
• Preserves selected values excluded by other filters.
• Adds corpus-wide behavior tests.

File summaries

File	Summary	Final review findings
test/hooks/useLibraryPageState.spec.tsx	Tests full-corpus options and selected-value behavior.	No findings.
src/hooks/useLibraryPageState.ts	Builds corpus-wide dynamic filter options.	Moderate (2 votes): stale placeholder data may be used during corpus changes. Moderate (1 vote): avoid the extra corpus request for static tabs. Nit (1 vote): add clinical-trial and biospecimen coverage.
src/components/data_library/filterRegistry.ts	Retains selected filter values.	No findings.

Review details

Suppressed comments (2)

src/hooks/useLibraryPageState.ts:213

• The new full-corpus path has separate calls for workspace, clinical-trial, and biospecimen transforms, but the added hook tests only assert workspace values. A regression in the registry entry or transform invocation for clinicalTrialItems or biospecimenItems would pass all of these tests; add cases with multiple nested trials/biospecimens and a small page size (including an active own filter) that assert the full-corpus options.

    const clinicalTrialItems = fullCorpusItems<{ registry?: string }>(AssetType.CLINICAL_TRIALS)
    const biospecimenItems = fullCorpusItems<{ optionalDataUse?: string }>(AssetType.BIOSPECIMENS)

src/hooks/useLibraryPageState.ts:135

• When the active tab is Publications, Presentations, Intellectual Property, or Funding Resources and one of its own filters is set, this clears those keys and line 135 starts a second full-corpus counts request. availableFilters only calls fullCorpusItems for Workspaces, Clinical Trials, and Biospecimens (lines 211-213), so this response is never consumed for those tabs; keep the second hook on urlState.filters unless the tab is one of the three dynamic-option assets.

  const { data: visibleTabCorpus } = useLibraryTabCounts(libraryConfig, optionCorpusFilters, urlState.query ?? '')

• Files reviewed: 8/8 changed files
• Comments generated: 1
• Review effort level: Lite

---

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

[Copilot]

🔵 Needs a closer look

Address placeholder-data handling in useLibraryPageState.ts and add transition coverage.

Review details

Suppressed comments (1)

src/hooks/useLibraryPageState.ts:145

• This fallback only checks isCorpusStale, but the counts query can also be serving its previous response as placeholder data after a filter edit. In that state both responses can represent the old study set; for a query-level filter such as accessManagement, transformResponse(..., EMPTY_FILTERS) cannot remove the excluded studies, so the option list can expose values outside the current result set until the request completes. Please also account for the counts query's placeholder state (or avoid deriving options from placeholder data) and cover this transition with a test.

      [assetType, assetType === urlState.tab && !isCorpusStale
        ? (visibleTabCorpus ?? tabCountsResponse)
        : tabCountsResponse])),

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Three unresolved moderate findings remain in src/hooks/useLibraryPageState.ts.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/hooks/useLibraryPageState.ts:200

• This treats STUDIES_AGG as the entire corpus, but that aggregation is a terms query capped at 10,000 study buckets (src/components/data_library/assets/definition.ts:43-47). If the matching library grows past that cap, any workspace/tool, platform, registry, or optional-data-use value found only in omitted studies will never be offered, so these filters are not actually corpus-complete. Use a paginated/composite aggregation (or otherwise remove the bucket cap) for option discovery before relying on this as the full corpus.

    // Option lists are derived from the *entire* matching corpus, not just the
    // active tab's current page: `tabCountsResponse` already carries every
    // matching study (via STUDIES_AGG's `terms.size: 10000`), so re-running an
    // asset's own `transformResponse` with an unbounded page size yields its

src/hooks/useLibraryPageState.ts:145

• When the query key changes (for example after a search or an external-filter edit), the first useLibraryTabCounts observer also serves its previous response as placeholder data. If the cleared-set observer is stale here, this fallback is therefore not necessarily scoped to the current filters and can temporarily offer values from studies excluded by the new query. Track the counts observer's isPlaceholderData and avoid using tabCountsResponse as a current-corpus fallback while it is stale (or otherwise use only data keyed to the current filters).

      [assetType, assetType === urlState.tab && !isCorpusStale
        ? (visibleTabCorpus ?? tabCountsResponse)
        : tabCountsResponse])),

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Two moderate issues and one nit remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/hooks/useLibraryPageState.ts:158

• tabCountsResponse is not guaranteed to be keyed by the live filters here: the first useLibraryTabCounts observer uses the same placeholderData behavior, so it also serves the previous response after a filter or search change. When isCorpusStale is true, this fallback can therefore derive options from that stale response and re-offer values excluded by the current filters. Expose the first observer's placeholder state and only fall back to a response whose query matches the current filters (otherwise keep the corpus empty while loading).

src/hooks/useLibraryPageState.ts:148

• When a derived tab has one of its own filters set, this mounts a second useLibraryTabCounts request. That hook includes STUDIES_AGG with up to 10,000 top_hits and _source: ['study.*'], so one interaction now downloads and materializes two full study corpora; on large libraries this doubles Elasticsearch work, response size, and browser memory. Consider a lightweight distinct-value aggregation or a corpus query projecting only fields needed for these options.

  const { data: visibleTabCorpus, isPlaceholderData: isCorpusStale } = useLibraryTabCounts(libraryConfig, optionCorpusFilters, urlState.query ?? '')

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[fboulnois]

Looks reasonable, two findings identified with Claude:

1. Option corpus clears all visible filters, not just the corpus-derived ones: src/hooks/useLibraryPageState.ts:124

The option corpus clears every filter in assetFilterRegistry[tab].visibleFilters, not just the checkbox filters whose options come from that corpus.

• On Clinical Trials, clinicalTrialDates gets cleared, so clinicalTrialRegistry options are derived from trials outside the date window. fullCorpusItems also passes EMPTY_FILTERS, so the client-side date pass is skipped too. Checking such a registry makes matchesClinicalTrialFilters reject every row → empty grid, 0 badge.
• Same on Biospecimens for biospecimenCollectionDate, biospecimenPostMortemInterval, and the shared study-level participantCount vs. biospecimenDataUse.
• Workspaces is unaffected — its only visible filters are the two option filters.

This is a regression: options previously came from data.items, which was filtered by the full filter set, so an offered value could never match nothing.

Fix: clear only the corpus-derived checkbox keys.

2. withSelectedValues appends instead of merging, breaking alphabetical order: src/components/data_library/filterRegistry.ts:645

withSelectedValues concatenates missing selected values onto the end of the option array, breaking the alphabetical order the lists are built with. LibraryFilters.renderCheckboxSection renders in array order with no sorting or show-more, so a still-checked value re-added this way appears at the bottom of a long list.

Fix: merge into the sort instead of appending.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[kevinmarete]

Both fixed, thanks.

1. 4c88b20f — clears only the keys whose options that corpus supplies, and re-applies the rest in the transform instead of EMPTY_FILTERS, so the client-side date/range pass still runs. A date-only edit now widens nothing and mounts no second aggregation.
2. b25f2bed — merges the re-added value into the order, but only where the list is already alphabetical; the enum lists keep their deliberate order.

[otchet-broad]

👍