feat(stats): add StatsParams for Meilisearch v1.44 showInternalDatabaseSizes and sizeFormat#2186
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 (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR implements support for Meilisearch v1.44.0 stats API enhancements. It adds a new ChangesStats API: Support human-formatted sizes and internal database size options
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/types/types.ts (2)
757-767: ⚡ Quick winType widening requires careful handling in consuming code.
The widening of
rawDocumentDbSizeandavgDocumentSizefromnumbertonumber | stringis correct for supporting human-formatted sizes, but it's a breaking change for TypeScript consumers who perform arithmetic or numeric operations on these fields without type guards. While runtime behavior is preserved when params are omitted (defaults to numeric "raw" format), stricter TypeScript checking will now require users to handle both types.This is the right approach for type safety, but consider documenting this in a migration guide or changelog so users understand they may need to add type guards when working with size fields.
🤖 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 `@src/types/types.ts` around lines 757 - 767, The IndexStats type widened rawDocumentDbSize and avgDocumentSize from number to number | string which is correct but is a breaking typing change; update the project migration docs/changelog and any consumer-facing documentation to clearly state this change and show recommended fixes (e.g., use type guards, Number() conversion, or a provided parse/format helper) and include example patterns for safely performing arithmetic on IndexStats.rawDocumentDbSize and IndexStats.avgDocumentSize; reference the IndexStats type and the two fields by name so maintainers and consumers can find and update call sites accordingly.
769-776: ⚡ Quick winSame type-widening concern applies here.
Similar to
IndexStats, theStatstype now hasdatabaseSizeandusedDatabaseSizewidened tonumber | string. This correctly reflects the API behavior whensizeFormat: "human"is used, but consumers performing numeric operations on these fields will need to add type guards.🤖 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 `@src/types/types.ts` around lines 769 - 776, The widening of databaseSize and usedDatabaseSize to number | string can surprise numeric consumers; make the type explicit by converting Stats into a generic type parameterized by size (e.g., type Stats<TSize = number | string>) and change databaseSize and usedDatabaseSize to TSize so callers can opt into number-only or human-string variants, and mirror the same approach for any related IndexStats fields that were widened so consumers can pick Stats<number> or Stats<string> as needed.
🤖 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.
Nitpick comments:
In `@src/types/types.ts`:
- Around line 757-767: The IndexStats type widened rawDocumentDbSize and
avgDocumentSize from number to number | string which is correct but is a
breaking typing change; update the project migration docs/changelog and any
consumer-facing documentation to clearly state this change and show recommended
fixes (e.g., use type guards, Number() conversion, or a provided parse/format
helper) and include example patterns for safely performing arithmetic on
IndexStats.rawDocumentDbSize and IndexStats.avgDocumentSize; reference the
IndexStats type and the two fields by name so maintainers and consumers can find
and update call sites accordingly.
- Around line 769-776: The widening of databaseSize and usedDatabaseSize to
number | string can surprise numeric consumers; make the type explicit by
converting Stats into a generic type parameterized by size (e.g., type
Stats<TSize = number | string>) and change databaseSize and usedDatabaseSize to
TSize so callers can opt into number-only or human-string variants, and mirror
the same approach for any related IndexStats fields that were widened so
consumers can pick Stats<number> or Stats<string> as needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3561847c-ce76-473f-a41f-927c84f4cb35
📒 Files selected for processing (4)
.code-samples.meilisearch.yamlsrc/indexes.tssrc/meilisearch.tssrc/types/types.ts
32357b5 to
4980fef
Compare
Strift
left a comment
Strift
left a comment
There was a problem hiding this comment.
Hello @arieleli01212 and thanks for your PR!
The CI is not passing. I suggest running linter + formatter + tests locally before requesting another review 🙏
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2186 +/- ##
=======================================
Coverage 98.07% 98.07%
=======================================
Files 14 14
Lines 674 674
Branches 110 110
=======================================
Hits 661 661
Misses 12 12
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Closes #2185.
Adds support for the two new query parameters introduced in Meilisearch v1.44.0 for the stats endpoints.
Changes
src/types/types.tsStatsParamstype withshowInternalDatabaseSizes?: booleanandsizeFormat?: "raw" | "human"IndexStats.internalDatabaseSizes?: Record<string, number | string>— present only whenshowInternalDatabaseSizes: true; typed asRecord<string, number | string>per the spec note that keys are subject to changerawDocumentDbSize,avgDocumentSize,databaseSize,usedDatabaseSize) widened tonumber | stringto accommodatesizeFormat: "human"responsessrc/indexes.ts—getStats(params?: StatsParams)passes params as query stringsrc/meilisearch.ts—getStats(params?: StatsParams)passes params as query string.code-samples.meilisearch.yaml— updatedget_index_stats_1andget_indexes_stats_1to show both new paramsSummary
This PR adds support for two new query parameters introduced in Meilisearch v1.44.0 for the stats endpoints: showInternalDatabaseSizes and sizeFormat.
Changes
Type Definitions (src/types/types.ts)
StatsParams:showInternalDatabaseSizes?: boolean— include internal database size breakdown when truesizeFormat?: "raw" | "human"— return sizes as bytes or human-readable stringsIndexStats:rawDocumentDbSizeandavgDocumentSizewidened fromnumbertonumber | stringinternalDatabaseSizes?: Record<string, number | string>(present whenshowInternalDatabaseSizesis true)Stats:databaseSizeandusedDatabaseSizewidened fromnumbertonumber | stringMethod Signatures (src/indexes.ts, src/meilisearch.ts)
Index.getStats(params?: StatsParams)now accepts optional params and forwards them as query string parameters toGET /indexes/{uid}/stats.Meilisearch.getStats(params?: StatsParams)now accepts optional params and forwards them as query string parameters toGET /stats.Code Samples (.code-samples.meilisearch.yaml)
get_index_stats_1andget_indexes_stats_1to callgetStats()with{ showInternalDatabaseSizes: true, sizeFormat: 'human' }.References
#2185