modify functions to allow frontend to pass percentiles #434
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis change introduces support for customizable percentile values across multiple components. A new constant Changes
Sequence Diagram(s)sequenceDiagram
participant FE as Frontend API
participant DH as DriftHandler
participant DS as DriftStore
participant CT as Constants
FE->>DH: Call getColumnSummary({ ..., percentiles })
DH->>DH: Extract percentiles (use Default if empty)
DH->>DS: getSummarySeries(..., percentiles)
alt Percentiles missing
DS->>CT: Retrieve DefaultPercentiles
end
Possibly related PRs
Suggested reviewers
Poem
🪧 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 (
|
…frontend # Conflicts: # api/src/main/scala/ai/chronon/api/Constants.scala
sean-zlai
reviewed
Feb 26, 2025
sean-zlai
approved these changes
Feb 26, 2025
|
@ken-zlai I see there is a failing test for
|
# Conflicts: # frontend/package-lock.json
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
hub/src/test/scala/ai/chronon/hub/handlers/DriftHandlerTest.scala (1)
128-151: Consider enhancing test coverage for percentiles functionality.The test doesn't verify actual percentiles behavior. Add test cases that check:
- Default percentiles used when not specified
- Custom percentiles correctly passed to the store
This would align with the PR objectives of allowing frontend to pass percentiles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
hub/src/test/scala/ai/chronon/hub/handlers/DriftHandlerTest.scala(1 hunks)
🔇 Additional comments (2)
hub/src/test/scala/ai/chronon/hub/handlers/DriftHandlerTest.scala (2)
142-143: Mock updated for new method signature.Mock now properly accommodates the new percentiles parameter in
getSummarySeries.
52-62: Ensure mock data aligns with supported percentiles.
createMockSummarySeriescreates fixed percentiles [0.1, 0.5, 0.9] but PR supports configurable values. Consider parameterizing this method to test various percentile configurations.
# Conflicts: # frontend/package-lock.json
Adds @tailwindcss/oxide-linux-x64-gnu as optional dependency to fix build failures in GitHub Actions. Required for Tailwind CSS 4.x Oxide engine when building on Linux environments.
Adds lightningcss-linux-x64-gnu as optional dependency to fix build failures in GitHub Actions. Required by Tailwind CSS 4.x's LightningCSS dependency when building on Linux environments.
Temporarily disables npm caching in GitHub Actions to ensure proper installation of Tailwind CSS 4.x native dependencies. This avoids adding optional dependencies with security vulnerabilities.
This reverts commit b8b8af9.
kumar-zlai
pushed a commit
that referenced
this pull request
Feb 26, 2025
## Summary This PR allows the frontend to specify which percentiles it retrieves from the backend. The percentiles can be passed as a query parameter: ``` percentiles=p0,p10,p90 ``` If omitted, the default percentiles are used: ``` percentiles=p5,p50,p95 ``` ### Example Requests *(App must be running)* #### Default (uses `p5,p50,p95`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000" ``` #### Equivalent Explicit Default ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p5,p50,p95" ``` #### Custom Percentiles (`p0,p10,p90`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p0,p10,p90" ``` ### Notes - Omitting the `percentiles` parameter is the same as explicitly setting `percentiles=p5,p50,p95`. - You can test using `curl` or Postman. - We need to let users change these percentiles via checkboxes or another UI control. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for customizable percentile parameters in summary data requests, with a default setting of "p5, p50, p95". - Enhanced the ability to retrieve detailed statistical summaries by allowing users to specify percentile values when querying data. - Introduced two new optional dependencies for improved functionality. - **Bug Fixes** - Adjusted method signatures to ensure compatibility with the new percentile parameters in various components. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
kumar-zlai
pushed a commit
that referenced
this pull request
Apr 25, 2025
## Summary This PR allows the frontend to specify which percentiles it retrieves from the backend. The percentiles can be passed as a query parameter: ``` percentiles=p0,p10,p90 ``` If omitted, the default percentiles are used: ``` percentiles=p5,p50,p95 ``` ### Example Requests *(App must be running)* #### Default (uses `p5,p50,p95`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000" ``` #### Equivalent Explicit Default ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p5,p50,p95" ``` #### Custom Percentiles (`p0,p10,p90`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p0,p10,p90" ``` ### Notes - Omitting the `percentiles` parameter is the same as explicitly setting `percentiles=p5,p50,p95`. - You can test using `curl` or Postman. - We need to let users change these percentiles via checkboxes or another UI control. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for customizable percentile parameters in summary data requests, with a default setting of "p5, p50, p95". - Enhanced the ability to retrieve detailed statistical summaries by allowing users to specify percentile values when querying data. - Introduced two new optional dependencies for improved functionality. - **Bug Fixes** - Adjusted method signatures to ensure compatibility with the new percentile parameters in various components. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
kumar-zlai
pushed a commit
that referenced
this pull request
Apr 29, 2025
## Summary This PR allows the frontend to specify which percentiles it retrieves from the backend. The percentiles can be passed as a query parameter: ``` percentiles=p0,p10,p90 ``` If omitted, the default percentiles are used: ``` percentiles=p5,p50,p95 ``` ### Example Requests *(App must be running)* #### Default (uses `p5,p50,p95`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000" ``` #### Equivalent Explicit Default ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p5,p50,p95" ``` #### Custom Percentiles (`p0,p10,p90`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p0,p10,p90" ``` ### Notes - Omitting the `percentiles` parameter is the same as explicitly setting `percentiles=p5,p50,p95`. - You can test using `curl` or Postman. - We need to let users change these percentiles via checkboxes or another UI control. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for customizable percentile parameters in summary data requests, with a default setting of "p5, p50, p95". - Enhanced the ability to retrieve detailed statistical summaries by allowing users to specify percentile values when querying data. - Introduced two new optional dependencies for improved functionality. - **Bug Fixes** - Adjusted method signatures to ensure compatibility with the new percentile parameters in various components. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
chewy-zlai
pushed a commit
that referenced
this pull request
May 15, 2025
## Summary This PR allows the frontend to specify which percentiles it retrieves from the backend. The percentiles can be passed as a query parameter: ``` percentiles=p0,p10,p90 ``` If omitted, the default percentiles are used: ``` percentiles=p5,p50,p95 ``` ### Example Requests *(App must be running)* #### Default (uses `p5,p50,p95`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000" ``` #### Equivalent Explicit Default ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p5,p50,p95" ``` #### Custom Percentiles (`p0,p10,p90`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p0,p10,p90" ``` ### Notes - Omitting the `percentiles` parameter is the same as explicitly setting `percentiles=p5,p50,p95`. - You can test using `curl` or Postman. - We need to let users change these percentiles via checkboxes or another UI control. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for customizable percentile parameters in summary data requests, with a default setting of "p5, p50, p95". - Enhanced the ability to retrieve detailed statistical summaries by allowing users to specify percentile values when querying data. - Introduced two new optional dependencies for improved functionality. - **Bug Fixes** - Adjusted method signatures to ensure compatibility with the new percentile parameters in various components. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
chewy-zlai
pushed a commit
that referenced
this pull request
May 15, 2025
## Summary This PR allows the frontend to specify which percentiles it retrieves from the backend. The percentiles can be passed as a query parameter: ``` percentiles=p0,p10,p90 ``` If omitted, the default percentiles are used: ``` percentiles=p5,p50,p95 ``` ### Example Requests *(App must be running)* #### Default (uses `p5,p50,p95`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000" ``` #### Equivalent Explicit Default ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p5,p50,p95" ``` #### Custom Percentiles (`p0,p10,p90`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p0,p10,p90" ``` ### Notes - Omitting the `percentiles` parameter is the same as explicitly setting `percentiles=p5,p50,p95`. - You can test using `curl` or Postman. - We need to let users change these percentiles via checkboxes or another UI control. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for customizable percentile parameters in summary data requests, with a default setting of "p5, p50, p95". - Enhanced the ability to retrieve detailed statistical summaries by allowing users to specify percentile values when querying data. - Introduced two new optional dependencies for improved functionality. - **Bug Fixes** - Adjusted method signatures to ensure compatibility with the new percentile parameters in various components. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
chewy-zlai
pushed a commit
that referenced
this pull request
May 16, 2025
## Summary This PR allows the frontend to specify which percentiles it retrieves from the baour clientsend. The percentiles can be passed as a query parameter: ``` percentiles=p0,p10,p90 ``` If omitted, the default percentiles are used: ``` percentiles=p5,p50,p95 ``` ### Example Requests *(App must be running)* #### Default (uses `p5,p50,p95`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000" ``` #### Equivalent Explicit Default ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p5,p50,p95" ``` #### Custom Percentiles (`p0,p10,p90`) ```sh curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p0,p10,p90" ``` ### Notes - Omitting the `percentiles` parameter is the same as explicitly setting `percentiles=p5,p50,p95`. - You can test using `curl` or Postman. - We need to let users change these percentiles via cheour clientsboxes or another UI control. ## Cheour clientslist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for customizable percentile parameters in summary data requests, with a default setting of "p5, p50, p95". - Enhanced the ability to retrieve detailed statistical summaries by allowing users to specify percentile values when querying data. - Introduced two new optional dependencies for improved functionality. - **Bug Fixes** - Adjusted method signatures to ensure compatibility with the new percentile parameters in various components. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR allows the frontend to specify which percentiles it retrieves from the backend. The percentiles can be passed as a query parameter:
If omitted, the default percentiles are used:
Example Requests (App must be running)
Default (uses
p5,p50,p95)curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000"Equivalent Explicit Default
curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p5,p50,p95"Custom Percentiles (
p0,p10,p90)curl "http://localhost:5173/api/v1/join/risk.user_transactions.txn_join/column/txn_by_user_transaction_amount_count_1h/summary?startTs=1672531200000&endTs=1677628800000&percentiles=p0,p10,p90"Notes
percentilesparameter is the same as explicitly settingpercentiles=p5,p50,p95.curlor Postman.Checklist
Summary by CodeRabbit
New Features
Bug Fixes