Skip to content

SCIX-856 feat(config): expose Solr tuning params as runtime env vars#834

Merged
thostetler merged 3 commits intoadsabs:masterfrom
thostetler:feat/SCIX-856-solr-runtime-config
Apr 3, 2026
Merged

SCIX-856 feat(config): expose Solr tuning params as runtime env vars#834
thostetler merged 3 commits intoadsabs:masterfrom
thostetler:feat/SCIX-856-solr-runtime-config

Conversation

@thostetler
Copy link
Copy Markdown
Member

Several Solr query parameters were hardcoded with no override path. The timeout env vars
declared in .env.local.sample were also never wired up.

  • Wire NEXT_PUBLIC_SEARCH_API_TIMEOUT_MS and NEXT_PUBLIC_SEARCH_SSR_API_TIMEOUT_MS into APP_DEFAULTS
  • Add NEXT_PUBLIC_SOLR_HL_MAX_ANALYZED_CHARS (default 1M, up from 150k) for highlight analyzer limit
  • Add NEXT_PUBLIC_SOLR_FACET_LIMIT (default 2000) used by years, citations, and reads facets
  • Add NEXT_PUBLIC_VIS_RESULTS_GRAPH_ROWS (default 1500) for the results graph Solr query
  • Update .env.local.sample with new vars

Timeout values declared in .env.local.sample were never wired up; hl.maxAnalyzedChars
was hardcoded at 150k with no override path.

- Wire NEXT_PUBLIC_SEARCH_API_TIMEOUT_MS and NEXT_PUBLIC_SEARCH_SSR_API_TIMEOUT_MS into APP_DEFAULTS
- Add NEXT_PUBLIC_SOLR_HL_MAX_ANALYZED_CHARS with 1M default, used in getHighlightParams
- Update .env.local.sample with new var
- Add NEXT_PUBLIC_SOLR_FACET_LIMIT with default 2000
- Use it in getSearchFacetYearsParams, getSearchFacetCitationsParams, getSearchFacetReadsParams
- Add NEXT_PUBLIC_VIS_RESULTS_GRAPH_ROWS with default 1500
- Use it in defaultResultsGraphParams
@thostetler thostetler requested a review from shinyichen April 1, 2026 19:54
@thostetler thostetler marked this pull request as ready for review April 3, 2026 17:36
Copilot AI review requested due to automatic review settings April 3, 2026 17:36
@thostetler thostetler merged commit b3ee2d6 into adsabs:master Apr 3, 2026
9 checks passed
@thostetler thostetler deleted the feat/SCIX-856-solr-runtime-config branch April 3, 2026 17:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 makes several Solr/search tuning parameters configurable via environment variables (instead of hardcoded constants), and wires previously-declared timeout env vars into the app’s default request configuration.

Changes:

  • Read NEXT_PUBLIC_SEARCH_API_TIMEOUT_MS / NEXT_PUBLIC_SEARCH_SSR_API_TIMEOUT_MS into APP_DEFAULTS timeouts.
  • Add env-configurable Solr tuning constants for highlight analyzer limit, facet limit, and results-graph row count, and use them in query model builders.
  • Document the new variables in .env.local.sample.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
src/config.ts Adds env-driven defaults for timeouts and new Solr tuning constants.
src/api/vis/models.ts Uses the new VIS_RESULTS_GRAPH_ROWS constant for results-graph queries.
src/api/search/models.ts Replaces hardcoded facet/highlight limits with env-driven constants.
.env.local.sample Adds sample values for the new env vars.

Comment on lines +26 to +27
API_TIMEOUT: parseInt(process.env.NEXT_PUBLIC_SEARCH_API_TIMEOUT_MS ?? '30000', 10),
SSR_API_TIMEOUT: parseInt(process.env.NEXT_PUBLIC_SEARCH_SSR_API_TIMEOUT_MS ?? '30000', 10),
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseInt() will return NaN when these env vars are set but empty (e.g. NEXT_PUBLIC_SEARCH_API_TIMEOUT_MS="") or non-numeric. That NaN flows into Axios timeout and can cause immediate timeouts or inconsistent behavior. Consider a small helper that coerces to a finite positive integer and falls back to the default when invalid (and optionally clamps to a sane max).

Copilot uses AI. Check for mistakes.
Comment on lines +26 to +27
API_TIMEOUT: parseInt(process.env.NEXT_PUBLIC_SEARCH_API_TIMEOUT_MS ?? '30000', 10),
SSR_API_TIMEOUT: parseInt(process.env.NEXT_PUBLIC_SEARCH_SSR_API_TIMEOUT_MS ?? '30000', 10),
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR title/description says these are "runtime env vars", but NEXT_PUBLIC_* values are typically embedded into the client bundle at build time in Next.js. If you need to tune Solr/timeout values without a rebuild (especially for client-side requests), this approach likely won’t work; consider a server-only env + API-provided config or another runtime injection mechanism.

Copilot uses AI. Check for mistakes.
Comment on lines +34 to +36
export const HL_MAX_ANALYZED_CHARS = parseInt(process.env.NEXT_PUBLIC_SOLR_HL_MAX_ANALYZED_CHARS ?? '1000000', 10);
export const SOLR_FACET_LIMIT = parseInt(process.env.NEXT_PUBLIC_SOLR_FACET_LIMIT ?? '2000', 10);
export const VIS_RESULTS_GRAPH_ROWS = parseInt(process.env.NEXT_PUBLIC_VIS_RESULTS_GRAPH_ROWS ?? '1500', 10);
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These Solr tuning values are parsed at module load with parseInt() and can become NaN (e.g. if the env var is set to an empty string). NaN will be sent to Solr for hl.maxAnalyzedChars/facet.limit/rows and can break queries. Recommend validating/coercing to a finite safe integer and falling back (or clamping) when invalid.

Copilot uses AI. Check for mistakes.
Comment on lines 203 to 206
stats: true,
'stats.field': 'citation_count',
'json.facet': `{"citation_count":{"type":"terms","field":"citation_count","sort":{"index":"desc"},"limit":2000}}`,
'json.facet': `{"citation_count":{"type":"terms","field":"citation_count","sort":{"index":"desc"},"limit":${SOLR_FACET_LIMIT}}}`,
});
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interpolating SOLR_FACET_LIMIT directly into a JSON string will produce invalid JSON if the value is NaN/Infinity (or unexpected if it’s negative). Prefer building the facet as an object and JSON.stringify-ing it (or otherwise ensure SOLR_FACET_LIMIT is a finite non-negative integer before interpolating).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants