Skip to content

fix(search): author facet counts - #1164

Open
anouillz wants to merge 1 commit into
rero:stagingfrom
anouillz:goa-author-facet
Open

fix(search): author facet counts#1164
anouillz wants to merge 1 commit into
rero:stagingfrom
anouillz:goa-author-facet

Conversation

@anouillz

@anouillz anouillz commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

fix(search): author facet counts

Elasticsearch builds terms facet counts by merging candidate terms returned
by each shard. With a facet size of 50 and no explicit shard_size, each
shard returns only 85 candidates. Globally relevant terms can therefore be
omitted on some shards, causing their final counts to be underestimated.

Project facets use TermsFacet objects from invenio-records-resources
instead of the dictionaries defined in RECORDS_REST_FACETS. They therefore
cannot be configured by the recursive records-rest implementation.

  • Add configurable size and shard size defaults for terms aggregations.
  • Apply them recursively to RECORDS_REST_FACETS while preserving explicit
    values.
  • Build project TermsFacet definitions from the application configuration.
  • Allow deployments to override both values without redefining the facets.
  • Add tests for the records-rest aggregation configuration.ored.

Closes #926

Warning

Should be tested on the production data as well

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 131769e6-5ebc-41cd-803e-b34d8c96e770

📥 Commits

Reviewing files that changed from the base of the PR and between 9f06a64 and 31c6366.

📒 Files selected for processing (2)
  • sonar/config.py
  • tests/unit/elasticsearch/test_config.py

Important

Approval pending

CodeRabbit has no unresolved comments, but it has not reviewed the latest commit.

Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.

  • 🔍 Trigger review

Walkthrough

The change adds DEFAULT_AGGREGATION_SHARD_SIZE = 1000 and applies it to document, deposit, and user terms aggregations while preserving existing aggregation sizes. Unit tests recursively inspect RECORDS_REST_FACETS and verify the expected size and shard_size values for every discovered terms aggregation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes add shard_size 1000 to the documented terms facets and test that every terms aggregation uses it [#926].
Out of Scope Changes check ✅ Passed The configuration change and unit tests are directly scoped to correcting Elasticsearch facet counts [#926].
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The description directly explains the Elasticsearch terms aggregation issue and the changes to shard-size defaults, facet configuration, and tests.
Title check ✅ Passed The title clearly identifies the search facet-count fix and matches the primary objective of correcting inaccurate author facet counts.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@anouillz
anouillz requested review from PascalRepond and jma August 13, 2026 06:25

@jma jma 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.

Please add more details in your commit message.

Comment thread sonar/config.py Outdated
@anouillz
anouillz requested a review from jma August 17, 2026 08:31
Comment thread sonar/config.py Outdated
@anouillz
anouillz force-pushed the goa-author-facet branch 3 times, most recently from e41371b to 94108f2 Compare August 19, 2026 08:24
Comment thread sonar/resources/projects/service.py Outdated
* Add configurable size and shard size defaults for terms aggregations.
* Apply them recursively to RECORDS_REST_FACETS while preserving explicit
  values.
* Build project TermsFacet definitions from the application configuration.
* Allow deployments to override both values without redefining the facets.
* Add tests for the records-rest aggregation configuration.
* Closes rero#926

Co-Authored-by: Ana Gomes <anagoncalves7@icloud.com>
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.

facets: incorrect counts in Author facet

3 participants