feat(stats): add show_internal_database_sizes and size_format params for Meilisearch v1.44#701
feat(stats): add show_internal_database_sizes and size_format params for Meilisearch v1.44#701arieleli01212 wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughClient#stats and Index#stats now accept optional keyword arguments ChangesStats API query parameters support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
StantonMatt
left a comment
StantonMatt
left a comment
There was a problem hiding this comment.
I checked this locally. The implementation path looks right: the existing stats specs pass against the Docker-backed Meilisearch service, and direct calls to both Client#stats(show_internal_database_sizes: true, size_format: 'human') and Index#stats(show_internal_database_sizes: true, size_format: 'human') returned human-readable size values plus internalDatabaseSizes.
The main gap I see is test coverage. The linked issue explicitly includes "Add/update test cases for the new/updated methods", but this PR only changes the SDK methods and code samples. I tried a small scratch spec that asserts the new keyword args are sent as showInternalDatabaseSizes / sizeFormat for both /stats and /indexes/:uid/stats, and that the response shape includes the new human-size/internal-size behavior; that passed cleanly.
Validation I ran:
docker compose run --rm -e DISABLE_COVERAGE=1 package bash -c "bundle exec rspec spec/meilisearch/client/stats_spec.rb spec/meilisearch/index/stats_spec.rb"
docker compose run --rm package bash -c "bundle exec rubocop spec/meilisearch/client/stats_spec.rb spec/meilisearch/index/stats_spec.rb"Both passed after adding the local scratch coverage. I think adding equivalent regression specs to this PR would make it line up with the issue checklist and protect the query-param mapping from future regressions.
|
Thanks for checking it locally! I've added test coverage for both new parameters:
All 10 examples in the two stats specs pass against |
|
Rechecked the updated head The stats coverage itself passes, and the diff is whitespace-clean: docker compose run --rm -e DISABLE_COVERAGE=1 package bash -c "bundle install && bundle exec rspec spec/meilisearch/client/stats_spec.rb spec/meilisearch/index/stats_spec.rb"
git diff --check origin/main...HEADThat was 10 stats examples, 0 failures. One remaining issue: RuboCop fails when the changed library files are included in the scope: I checked |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
spec/meilisearch/client/stats_spec.rb (1)
17-21: 💤 Low valueConsider validating the human-readable format pattern.
The test verifies that
databaseSizeis returned as a String whensize_format: 'human'is used, which confirms basic functionality. For more thorough coverage, you could optionally assert that the string matches the expected human-readable pattern (e.g., contains units like "MiB", "GiB").🧪 Example of stronger assertion
it 'gets stats with human-readable size format' do response = client.stats(size_format: 'human') expect(response).to have_key('databaseSize') expect(response['databaseSize']).to be_a(String) + expect(response['databaseSize']).to match(/\d+(\.\d+)?\s+(B|KiB|MiB|GiB|TiB)/) end🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/meilisearch/client/stats_spec.rb` around lines 17 - 21, The test for client.stats(size_format: 'human') should also assert the returned databaseSize string matches a human-readable size pattern; update the example spec (the it block calling client.stats and inspecting response['databaseSize']) to include a regex-based expectation that the string contains a numeric value and a unit like B, KiB, MiB, GiB (e.g. /\A\d+(?:\.\d+)?\s?(B|KiB|MiB|GiB|TiB)\z/ or a variant) so the spec verifies both type and format.spec/meilisearch/index/stats_spec.rb (1)
12-16: 💤 Low valueConsider validating the human-readable format pattern.
Similar to the client spec, this test confirms that
rawDocumentDbSizeis a String whensize_format: 'human'is used. For additional confidence, you could optionally assert that the string matches the expected human-readable size pattern.🧪 Example of stronger assertion
it 'returns stats with human-readable size format' do response = index.stats(size_format: 'human') expect(response).to have_key('rawDocumentDbSize') expect(response['rawDocumentDbSize']).to be_a(String) + expect(response['rawDocumentDbSize']).to match(/\d+(\.\d+)?\s+(B|KiB|MiB|GiB|TiB)/) end🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spec/meilisearch/index/stats_spec.rb` around lines 12 - 16, Enhance the test that calls index.stats(size_format: 'human') by adding an assertion that response['rawDocumentDbSize'] matches a human-readable size pattern; locate the spec example with it 'returns stats with human-readable size format' and after confirming it is a String, add a regex/assertion that validates formats like "123 B", "1.2 KB", "10MB" (e.g., numeric with optional decimal followed by optional space and unit B/KB/MB/GB/TB, case-insensitive) to ensure index.stats returns properly formatted human-readable sizes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spec/meilisearch/client/stats_spec.rb`:
- Around line 23-31: The shared cleanup helper clear_all_indexes in
spec/support/indexes_helpers.rb issues client.delete_index(uid) without awaiting
the returned Models::Task, which allows deletions to run asynchronously and
causes race conditions; update clear_all_indexes to await each deletion (e.g.,
call client.delete_index(uid).await or otherwise wait for task completion) so
all index deletions complete before the next example runs, ensuring tests like
stats_spec that rely on a clean state no longer race.
---
Nitpick comments:
In `@spec/meilisearch/client/stats_spec.rb`:
- Around line 17-21: The test for client.stats(size_format: 'human') should also
assert the returned databaseSize string matches a human-readable size pattern;
update the example spec (the it block calling client.stats and inspecting
response['databaseSize']) to include a regex-based expectation that the string
contains a numeric value and a unit like B, KiB, MiB, GiB (e.g.
/\A\d+(?:\.\d+)?\s?(B|KiB|MiB|GiB|TiB)\z/ or a variant) so the spec verifies
both type and format.
In `@spec/meilisearch/index/stats_spec.rb`:
- Around line 12-16: Enhance the test that calls index.stats(size_format:
'human') by adding an assertion that response['rawDocumentDbSize'] matches a
human-readable size pattern; locate the spec example with it 'returns stats with
human-readable size format' and after confirming it is a String, add a
regex/assertion that validates formats like "123 B", "1.2 KB", "10MB" (e.g.,
numeric with optional decimal followed by optional space and unit B/KB/MB/GB/TB,
case-insensitive) to ensure index.stats returns properly formatted
human-readable sizes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ced3757-d8b3-4b59-afa7-0776f9e0b86f
📒 Files selected for processing (2)
spec/meilisearch/client/stats_spec.rbspec/meilisearch/index/stats_spec.rb
| it 'gets stats with internal database sizes' do | ||
| client.create_index('books').await | ||
| response = client.stats(show_internal_database_sizes: true) | ||
|
|
||
| expect(response).to have_key('indexes') | ||
| response['indexes'].each_value do |index_stats| | ||
| expect(index_stats).to have_key('internalDatabaseSizes') | ||
| end | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Look for RSpec cleanup hooks that remove test indexes
# Check for after hooks in this spec file
rg -n -A 3 -B 1 'after\(:(each|all)\)' spec/meilisearch/client/stats_spec.rb
# Check for shared cleanup contexts
rg -n -A 5 'RSpec\.configure.*after' spec/spec_helper.rb spec/support/
# Check for index deletion patterns in spec helpers
rg -n 'delete.*index|clear.*index' spec/spec_helper.rb spec/support/Repository: meilisearch/meilisearch-ruby
Length of output: 274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) See where clear_all_indexes is used
rg -n "clear_all_indexes\(" spec | head -n 50
# 2) Inspect stats_spec.rb for hooks / shared contexts
rg -n "shared_context|default_shared_context|include.*default|after\(|before\(" spec/meilisearch/client/stats_spec.rb
# 3) Print stats_spec.rb with line numbers (only the relevant region if short)
wc -l spec/meilisearch/client/stats_spec.rb
cat -n spec/meilisearch/client/stats_spec.rb
# 4) Inspect helper implementations referenced by the initial search
wc -l spec/support/indexes_helpers.rb spec/support/default_shared_context.rb
cat -n spec/support/indexes_helpers.rb
cat -n spec/support/default_shared_context.rbRepository: meilisearch/meilisearch-ruby
Length of output: 209
🏁 Script executed:
#!/bin/bash
set -u
# Don't fail on missing matches; we need absence info for verification.
set +e
# A) Confirm how stats_spec.rb sets up/tears down the client/indexes
echo "== stats_spec.rb =="
wc -l spec/meilisearch/client/stats_spec.rb
sed -n '1,160p' spec/meilisearch/client/stats_spec.rb
echo
echo "== usages of clear_all_indexes in spec/ =="
rg -n "clear_all_indexes\(" spec | head -n 50
echo
echo "== inclusion of default_shared_context in specs =="
rg -n "default_shared_context" spec | head -n 50
rg -n "include.*default_shared_context|include.*indexes_helpers|include.*shared_context" spec | head -n 50
echo
echo "== after/before hooks in stats_spec.rb =="
rg -n "before\(|after\(|around\(|shared_context" spec/meilisearch/client/stats_spec.rb
echo
echo "== helper implementations =="
wc -l spec/support/indexes_helpers.rb spec/support/default_shared_context.rb
cat -n spec/support/indexes_helpers.rb
cat -n spec/support/default_shared_context.rb
echo
echo "== RSpec configure after hooks =="
rg -n "RSpec\.configure|config\.after|config\.before" spec/spec_helper.rb spec/spec_helper.rb spec/support/ | head -n 80Repository: meilisearch/meilisearch-ruby
Length of output: 2637
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== find where shared context 'test defaults' is included =="
rg -n "include_context\s+['\"]test defaults['\"]|shared_context\s+['\"]test defaults['\"]" spec | head -n 100
echo
echo "== inspect spec/spec_helper.rb around RSpec.configure =="
sed -n '1,140p' spec/spec_helper.rb
sed -n '140,260p' spec/spec_helper.rb
echo
echo "== confirm whether stats_spec.rb includes default shared context =="
rg -n "include_context|shared_context|default_shared_context" spec/meilisearch/client/stats_spec.rb
echo
echo "== check for global metadata including shared context =="
rg -n "use.*shared_context|before.*include_context|RSpec\.configure" spec/spec_helper.rb spec/support | head -n 200Repository: meilisearch/meilisearch-ruby
Length of output: 3314
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate delete_index implementation and any usage patterns
rg -n "def delete_index\b" lib spec | head -n 50
rg -n "delete_index\(" lib spec | head -n 50
rg -n "delete_index\(.+\)\.await" lib spec | head -n 50
# Print the delete_index method (first match) with surrounding context
file="$(rg -l "def delete_index\b" lib spec | head -n 1)"
echo "== delete_index file: $file =="
if [ -n "${file:-}" ]; then
# show a window around the method
line="$(rg -n "def delete_index\b" "$file" | head -n 1 | cut -d: -f1)"
start=$((line-20))
end=$((line+60))
sed -n "${start},${end}p" "$file"
fiRepository: meilisearch/meilisearch-ruby
Length of output: 349
🏁 Script executed:
#!/bin/bash
set +e
echo "== delete_index implementation =="
file="lib/meilisearch/client.rb"
line="$(rg -n "def delete_index\b" "$file" | head -n 1 | cut -d: -f1)"
echo "found at line: $line"
start=$((line-20))
end=$((line+80))
sed -n "${start},${end}p" "$file"
echo
echo "== usages of delete_index(...).await =="
rg -n "delete_index\([^)]+\)\.await" lib spec | head -n 50
echo
echo "== usages of delete_index =="
rg -n "delete_index\(" lib spec | head -n 80
echo
echo "== inspect indexes_spec.rb around delete_index usage =="
spec_file="spec/meilisearch/client/indexes_spec.rb"
line2="$(rg -n "task = client\.delete_index" "$spec_file" | head -n 1 | cut -d: -f1)"
echo "found at line: $line2"
start2=$((line2-40))
end2=$((line2+60))
sed -n "${start2},${end2}p" "$spec_file"Repository: meilisearch/meilisearch-ruby
Length of output: 7628
🏁 Script executed:
#!/bin/bash
set +euo pipefail
# Find clear_all_keys implementation and how it's called
rg -n "def clear_all_keys|clear_all_keys\(" spec/support spec | head -n 50
# Inspect the helper file that defines clear_all_keys (if found)
file="$(rg -l "def clear_all_keys\b" spec/support spec | head -n 1)"
echo "== clear_all_keys file: ${file:-<none>} =="
if [ -n "${file:-}" ]; then
sed -n '1,200p' "$file"
fi
# Inspect if any cleanup helpers await deletion tasks
rg -n "clear_all_indexes|delete_index\(.+?\)\.await|.delete_index\(.+?\)" spec/support | head -n 80Repository: meilisearch/meilisearch-ruby
Length of output: 783
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check delete_key implementation
file="$(rg -l "def delete_key\b" lib/meilisearch/client.rb lib spec | head -n 1)"
echo "== delete_key file: ${file:-<none>} =="
if [ -n "${file:-}" ]; then
line="$(rg -n "def delete_key\b" "$file" | head -n 1 | cut -d: -f1)"
start=$((line-20))
end=$((line+70))
sed -n "${start},${end}p" "$file"
fi
# Check usage patterns of delete_key(...).await
rg -n "delete_key\([^)]+\)\.await" lib spec | head -n 50
rg -n "delete_key\(" lib spec | head -n 50
# Also check whether delete_index returns an async task (already saw return type in comment)
# but let's inspect the Index#delete used underneath
file2="$(rg -l "def delete\b" lib/meilisearch/index.rb lib/meilisearch -g'*.rb' | head -n 20)"
echo
echo "== candidate delete methods files (showing first few): =="
echo "$file2" | head -n 5Repository: meilisearch/meilisearch-ruby
Length of output: 3890
Await async index deletion in shared test cleanup to prevent race conditions
spec/meilisearch/client/stats_spec.rb doesn’t clean up the 'books' index itself, but the suite includes the shared context test defaults (via config.include_context 'test defaults') that calls clear_all_indexes(client) before each example. However, clear_all_indexes calls client.delete_index(uid) without awaiting the returned Models::Task, so index deletions may still be in progress when the next example starts. Update spec/support/indexes_helpers.rb to await each delete_index task (e.g., client.delete_index(uid).await) or otherwise ensure cleanup completes before the example runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@spec/meilisearch/client/stats_spec.rb` around lines 23 - 31, The shared
cleanup helper clear_all_indexes in spec/support/indexes_helpers.rb issues
client.delete_index(uid) without awaiting the returned Models::Task, which
allows deletions to run asynchronously and causes race conditions; update
clear_all_indexes to await each deletion (e.g., call
client.delete_index(uid).await or otherwise wait for task completion) so all
index deletions complete before the next example runs, ensuring tests like
stats_spec that rely on a clean state no longer race.
|
Good catch on the RuboCop
|
|
Rechecked head |
2 participants
Closes #697.
Adds support for the two new query parameters introduced in Meilisearch v1.44.0 for the stats endpoints.
Changes
lib/meilisearch/index.rb—statsnow accepts optional keyword args:show_internal_database_sizes:— whentrue, the response includesinternalDatabaseSizes(keys subject to change)size_format:—'human'returns sizes as strings like"2.3 MiB",'raw'(default) returns byte countslib/meilisearch/client.rb— same optional keyword args onClient#stats.code-samples.meilisearch.yaml— updatedget_index_stats_1andget_indexes_stats_1Both methods build the params hash and call
.compactto dropnilvalues, so existing callers with no arguments continue to work unchanged.Summary
This PR adds support for two new optional query parameters to the stats endpoints, aligning with Meilisearch v1.44.0:
show_internal_database_sizes(boolean): when true, responses include aninternalDatabaseSizesmapping (keys subject to change; should not be strongly typed).size_format(string:'human'or'raw'): controls size formatting —'human'returns human-readable strings (e.g., "2.3 MiB");'raw'returns numeric byte counts (default).Changes
Index#statsnow accepts keyword argsshow_internal_database_sizes: nil, size_format: nil; builds a params hash with camel-cased keys (showInternalDatabaseSizes,sizeFormat), compacts nils, and passes them to the/indexes/:uid/statsHTTP GET.Client#statsupdated similarly to accept the same keyword args and pass them as query params toGET /stats.get_index_stats_1andget_indexes_stats_1to call the new methods withshow_internal_database_sizes: true, size_format: 'human'.size_format: 'human'returns size fields as Strings.show_internal_database_sizes: trueincludesinternalDatabaseSizesin responses.Metrics/ClassLengthMax from 541 to 542 to address a RuboCop offense introduced by the change (class length baseline bump).All changes preserve backward compatibility — omitting the new args leaves behavior unchanged.
Related issue
Closes
#697