Stats response changes #443
Conversation
WalkthroughThe changes update several components to use numerical representations instead of strings for sizes and counts. In the stream type definitions, properties like Changes
Sequence Diagram(s)sequenceDiagram
participant Home as HomePage Component
participant Sanitize as sanitizeBytes
participant Format as formatBytes
Home->>Sanitize: call sanitizeBytes(rawByteString)
Sanitize->>Format: call formatBytes(rawByteString)
Format-->>Sanitize: return formatted bytes
Sanitize-->>Home: return formatted bytes or '–'
sequenceDiagram
participant Stats as Stats Component
participant Humanize as HumanizeNumber
participant Format as formatBytes
Stats->>Humanize: call HumanizeNumber(rawCount)
Humanize-->>Stats: return formatted count
Stats->>Format: call formatBytes(rawSize)
Format-->>Stats: return formatted size
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/@types/parseable/api/stream.ts (2)
48-55: Consider updating HotTierConfig to use number types for consistency.While the current PR focuses on
LogStreamStat, for consistency you might want to consider updating theHotTierConfigtype to also use number types for size-related fields in a future update.export type HotTierConfig = | { - size: string; - used_size: string; - available_size: string; + size: number; + used_size: number; + available_size: number; oldest_date_time_entry: string; } | object;
57-59: Consider updating UpdateHotTierConfig to use number type for consistency.For consistency with the changes made to
LogStreamStat, consider updating thesizeproperty inUpdateHotTierConfigto use a number type in a future update.export type UpdateHotTierConfig = { - size: string; + size: number; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/@types/parseable/api/stream.ts(1 hunks)src/pages/Home/index.tsx(1 hunks)src/pages/Stream/Views/Manage/Stats.tsx(6 hunks)src/pages/Systems/StorageSection.tsx(1 hunks)src/utils/formatBytes.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (16)
src/pages/Home/index.tsx (1)
228-230: LGTM! Function simplified properly.The
sanitizeBytesfunction has been simplified to directly useformatBytes, which aligns with the type changes in theLogStreamStatinterface where size properties are now numbers instead of strings. This eliminates the unnecessary string-to-integer conversion step.src/@types/parseable/api/stream.ts (2)
10-19: Type changes appropriately reflect numerical representation.The change from
stringtonumberforsize,lifetime_size, anddeleted_sizein the ingestion object improves type safety and aligns with how these values are used in calculations and formatting.
20-25: Type changes consistently applied to storage object.The storage object properties have been consistently updated to use
numbertypes for size-related fields, which matches the changes in the ingestion object and creates a coherent data model.src/utils/formatBytes.ts (2)
50-58: Function signature and implementation updated correctly.The
calcCompressionRatefunction has been properly updated to:
- Accept numeric parameters instead of strings
- Remove string parsing logic
- Improve output formatting with
toFixed(2)for consistent decimal placesThis change aligns well with the updated type definitions in
LogStreamStat.
38-48:❌ Incorrect review comment
Verify if the
bytesStringToIntegerandsanitizeBytesfunctions can be safely removed.These functions appear to be unused after the changes in this PR, since the API now returns numeric values for sizes rather than strings. Consider removing them if they're no longer needed throughout the codebase.
🏁 Script executed:
#!/bin/bash # Check if bytesStringToInteger and sanitizeBytes are used elsewhere in the codebase echo "Checking for bytesStringToInteger usage..." rg "bytesStringToInteger" --type ts --type tsx echo "Checking for sanitizeBytes usage..." rg "sanitizeBytes" --type ts --type tsxLength of output: 318
I'm generating a new verification script using glob patterns to ensure we search all relevant TypeScript files (including TSX files):
Once this script runs and produces the output, we can confirm whether these functions are indeed unused and safely removable. Please run this updated script so we can verify the usage before finalizing the removal suggestion.
🏁 Script executed:
#!/bin/bash echo "Searching for bytesStringToInteger usage in .ts and .tsx files..." rg "bytesStringToInteger" --glob "*.ts" --glob "*.tsx" echo "Searching for sanitizeBytes usage in .ts and .tsx files..." rg "sanitizeBytes" --glob "*.ts" --glob "*.tsx"Length of output: 1367
The removal recommendation for these functions is not valid.
Verification shows that:
bytesStringToIntegeris defined insrc/utils/formatBytes.tsand is used within the file as part of thesanitizeBytesimplementation.sanitizeBytesis actively referenced in multiple locations (e.g., insrc/pages/Home/index.tsxandsrc/pages/Stream/Views/Manage/Settings.tsx).Since these functions remain in use, they should not be removed at this time. If the API now returns numeric values, consider refactoring the implementation to directly use those numbers with
formatBytes, but outright removal isn’t possible without further changes elsewhere in the codebase.Likely an incorrect or invalid review comment.
src/pages/Stream/Views/Manage/Stats.tsx (8)
5-5: Import statement updated correctly.The import statement has been updated to use
formatBytesandHumanizeNumberinstead of the removed functions, which aligns with the changes in the utility functions.
43-47: Default values updated to match new type expectations.The default values for event counts have been properly updated from string placeholders to numeric zeros, which matches the type expectations of the
LogStreamStatinterface.
63-63: Function usage updated appropriately.Changed from
sanitizeEventsCounttoHumanizeNumberto directly work with numeric values from the API.
80-84: Default values for ingested size updated correctly.The default values for ingested sizes have been properly updated from string placeholders to numeric zeros, which matches the type expectations of the
LogStreamStatinterface.
100-100: Function usage updated appropriately.Changed from
sanitizeBytestoformatBytesto directly work with numeric values from the API.
117-121: Default values for storage data updated correctly.The default values for storage sizes have been properly updated from string placeholders to numeric zeros, which matches the type expectations of the
LogStreamStatinterface.
141-141: Function usage updated appropriately.Changed from
sanitizeBytestoformatBytesto directly work with numeric values from the API.
133-133:❓ Verification inconclusive
Verify compression rate calculation with the new data types.
The
compressionSizecalculation is now using the raw numeric values directly without any string conversion. Ensure this is accurate in all edge cases, especially when dealing with very large or very small numbers.
🏁 Script executed:
#!/bin/bash # Check the implementation of calcCompressionRate function echo "Checking for calcCompressionRate implementation..." rg -A 10 "calcCompressionRate" --type ts --type tsxLength of output: 190
Below is a revised verification step. The initial shell command failed to search within TSX files because the file type wasn’t recognized. Please run the following shell script that explicitly searches both *.ts and *.tsx files for the definition of
calcCompressionRate. This will help confirm whether the calculation properly handles the new data types and edge cases.
#!/bin/bash # Search for calcCompressionRate implementation in .ts and .tsx files echo "Searching for calcCompressionRate implementation in .ts files..." rg -A 10 "calcCompressionRate" -g "*.ts" echo "Searching for calcCompressionRate implementation in .tsx files..." rg -A 10 "calcCompressionRate" -g "*.tsx"
Action Required:
- Verify that the
calcCompressionRatefunction correctly processes raw numeric values.- Confirm that it handles edge cases (e.g., very large and very small numbers) accurately.
Once you obtain the script output, please review the results manually to ensure the new data types are correctly accounted for.
src/pages/Systems/StorageSection.tsx (3)
155-156:✅ Verification successful
Verify compatibility with the updated
calcCompressionRatefunction.According to the PR summary, the
calcCompressionRatefunction was modified to accept numeric parameters instead of strings. Ensure that this function correctly handles the raw numeric values now being passed to it.
🏁 Script executed:
#!/bin/bash # Examine the implementation of calcCompressionRate to verify it properly handles numeric parameters echo "Checking calcCompressionRate implementation..." rg -A 10 -B 2 "calcCompressionRate" --type ts --type tsx # Look for the function definition in formatBytes.ts cat src/utils/formatBytes.ts || echo "File not found"Length of output: 1873
Compatibility Verified –
calcCompressionRateCorrectly Processes Numeric InputsThe updated
calcCompressionRatefunction insrc/utils/formatBytes.tsnow accepts numeric parameters and computes the compression rate as expected. The call sites insrc/pages/Systems/StorageSection.tsxpassinglifetimeStorageSize,lifetimeIngestedSize,storageSize, andingestedSizeare consistent with this update. No further modifications are required.
151-152:❓ Verification inconclusive
Confirm data structure for nested properties.
The change from underscore notation to dot notation for
parseable_lifetime_storage_size.dataandparseable_deleted_storage_size.datasuggests a change in the underlying data structure. Make sure this structure is correctly represented in your type definitions.
🏁 Script executed:
#!/bin/bash # Check type definitions for these properties echo "Checking IngestorQueryRecord type definition..." rg -A 15 "IngestorQueryRecord" --type ts --type tsx echo "Checking related type definitions in stream.ts..." cat src/@types/parseable/api/stream.ts || echo "File not found"Length of output: 1433
Action: Confirm Nested Property Types Alignment
Please verify that the nested structure for the properties
parseable_lifetime_storage_sizeandparseable_deleted_storage_sizein therecordobject is accurately reflected in your type definitions. While the change in access from underscore to dot notation implies these fields are now expected to be objects (with a nesteddataproperty), the current type definitions (e.g., insrc/@types/parseable/api/stream.ts) only show flat numeric values for similar storage fields. Ensure that the type (such as inIngestorQueryRecord, if applicable) correctly describes these nested structures.
150-154:❓ Verification inconclusive
Double-check fallback values for undefined properties.
All the
_.get()calls now use0as the default value. Ensure that zero is the appropriate fallback for these properties, especially if they might be used in calculations like division where zero could cause errors.
🏁 Script executed:
#!/bin/bash # Check how these values are used in calculations or display echo "Checking usage of storageSize in calculations..." rg "\bstorageSize\b.*calcCompressionRate" --type ts --type tsx echo "Checking if division operations are performed with these values..." rg "dividedBy|\/\s*\(?storageSize|\/\s*\(?ingestedSize" --type ts --type tsxLength of output: 548
Action: Verify Fallback Value Safety in Storage Calculations
The original regex checks produced errors because of unrecognized escape sequences and file type issues. To ensure that using 0 as the fallback value won’t inadvertently lead to division‐by‐zero or other calculation errors, please run the following updated shell script. It searches for:
- Any usage of
storageSizein conjunction withcalcCompressionRate- Division operations involving
storageSizeoringestedSizeOnce you verify that these properties are not part of calculations that might result in division by zero (or any unsafe operation), you can confirm that the fallback value of 0 is appropriate.
🏁 Script executed:
#!/bin/bash echo "Searching for calcCompressionRate usage with 'storageSize'..." rg -n "storageSize.*calcCompressionRate" . echo "Searching for division operations involving 'storageSize'..." rg -n "storageSize\s*\/" . echo "Searching for division operations involving 'ingestedSize'..." rg -n "ingestedSize\s*\/" .Length of output: 836
Action: Manually Verify Fallback Safety for Storage Calculations
The updated searches indicate that our previous regex issues prevented a conclusive automated verification. To ensure that using zero as the fallback is safe (i.e., that these values aren’t used in division or other calculations that could lead to division-by-zero errors), please run the following revised shell script manually:
#!/bin/bash echo "Searching for calcCompressionRate usage with 'storageSize'..." rg -n "storageSize.*calcCompressionRate" -g '*.tsx' -g '*.ts' . echo "Searching for division operations involving 'storageSize'..." rg -n "storageSize\s*/" -g '*.tsx' -g '*.ts' . echo "Searching for division operations involving 'ingestedSize'..." rg -n "ingestedSize\s*/" -g '*.tsx' -g '*.ts' .Points to verify:
- Ensure that the search for
calcCompressionRateusage withstorageSizereturns no unexpected results.- Confirm that neither
storageSizenoringestedSize(or other related properties) are being used directly as divisors in any arithmetic operations.Once you’ve manually verified that no unsafe calculations exist, you can confirm that the fallback value of zero is appropriate.
| const storageSize = _.get(record, 'parseable_storage_size_data', 0); | ||
| const lifetimeStorageSize = _.get(record, 'parseable_lifetime_storage_size.data', 0); | ||
| const deletedStorageSize = _.get(record, 'parseable_deleted_storage_size.data', 0); | ||
| const ingestedSize = _.get(record, 'parseable_events_ingested_size', 0); | ||
| const lifetimeIngestedSize = _.get(record, 'parseable_lifetime_events_ingested_size', 0); |
There was a problem hiding this comment.
Values need to be formatted before display in the UI.
The function now returns raw numeric values instead of pre-formatted byte strings, but the UI components (lines 86, 98, 106, 115, 123) still display these values directly without formatting. This will result in plain numbers being shown to users instead of human-readable formats like "1.2 MB".
Apply the following changes to ensure proper formatting:
return {
- storageSize,
- lifetimeIngestedSize,
- lifetimeStorageSize,
- deletedStorageSize,
- ingestedSize,
+ storageSize: formatBytes(storageSize),
+ lifetimeIngestedSize: formatBytes(lifetimeIngestedSize),
+ lifetimeStorageSize: formatBytes(lifetimeStorageSize),
+ deletedStorageSize: formatBytes(deletedStorageSize),
+ ingestedSize: formatBytes(ingestedSize),
lifetimeCompressionRate,
activeCompressionRate,
};Alternatively, update all display components to format these values at render time.
Also applies to: 158-166
💡 Verification agent
🧩 Analysis chain
Ensure consistent data property path access notation.
There's an inconsistency in how the data property paths are accessed:
- Lines 150, 153, 154 use underscore notation (
parseable_storage_size_data,parseable_events_ingested_size) - Lines 151-152 use dot notation (
parseable_lifetime_storage_size.data,parseable_deleted_storage_size.data)
This inconsistency could lead to access errors if the data structure is uniform across all properties.
🏁 Script executed:
#!/bin/bash
# Search for usage patterns of these properties to understand the correct notation pattern
echo "Searching for storage size data path patterns..."
rg -A 2 -B 2 "parseable_storage_size" --type ts --type tsx
echo "Searching for lifetime storage size path patterns..."
rg -A 2 -B 2 "parseable_lifetime_storage_size" --type ts --type tsx
echo "Searching for deleted storage size path patterns..."
rg -A 2 -B 2 "parseable_deleted_storage_size" --type ts --type tsxLength of output: 612
The previous script’s use of the --type tsx flag produced errors because “tsx” isn’t a recognized file type for ripgrep. This revised script uses glob patterns to include both .ts and .tsx files, which should yield the expected output for verifying property path usage across the codebase. Please run this script and review the results to determine whether the inconsistent property access (dot notation vs. underscore notation) is indeed an issue that needs addressing.
🏁 Script executed:
#!/bin/bash
# Re-verify property notation usage across TypeScript and TSX files using glob patterns
echo "Searching for 'parseable_storage_size' usage patterns..."
rg -g '*.ts' -g '*.tsx' -A 2 -B 2 "parseable_storage_size" .
echo "Searching for 'parseable_lifetime_storage_size' usage patterns..."
rg -g '*.ts' -g '*.tsx' -A 2 -B 2 "parseable_lifetime_storage_size" .
echo "Searching for 'parseable_deleted_storage_size' usage patterns..."
rg -g '*.ts' -g '*.tsx' -A 2 -B 2 "parseable_deleted_storage_size" .Length of output: 6687
Consistent Property Access Required: Update Dot Notation to Underscore Notation
In src/pages/Systems/StorageSection.tsx, the property access for lifetime and deleted storage sizes is inconsistent with the type definitions. While other properties use the underscore naming convention (e.g., parseable_storage_size_data), the code currently uses dot notation for these two properties:
- Change from
_.get(record, 'parseable_lifetime_storage_size.data', 0)to_.get(record, 'parseable_lifetime_storage_size_data', 0) - Change from
_.get(record, 'parseable_deleted_storage_size.data', 0)to_.get(record, 'parseable_deleted_storage_size_data', 0)
This adjustment aligns with the definitions in src/@types/parseable/api/clusterInfo.ts and ensures uniform data access.
Summary by CodeRabbit