feat(metrics): bound dimensional timeseries groups - #1842
Conversation
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
📝 WalkthroughWalkthroughThe metric-results API now exposes typed OpenAPI request and response schemas. Timeseries requests support validated group limits, shared ranking queries, capped SQL grouping, and response metadata for totals, ranks, remainder groups, and labels. ChangesMetric results API and timeseries flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant metric_results
participant ranking_queries
participant planned_queries
participant response_builder
Client->>metric_results: POST /v1/metric-results
metric_results->>ranking_queries: fetch ranking rows
ranking_queries-->>metric_results: ranked groups
metric_results->>planned_queries: plan and execute timeseries queries
planned_queries-->>response_builder: timeseries rows with metadata
response_builder-->>Client: MetricResultsResponse
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)docs/components/backend/analytics/openapi.jsonTraceback (most recent call last): 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/backend/services/analytics/src/domain/metric_results/dto.rs`:
- Around line 40-44: Update MetricGroupLimitRequest.count with a schema
constraint of minimum 1 and maximum 50 to match validate_group_limit’s enforced
range. Regenerate the OpenAPI artifact at
docs/components/backend/analytics/openapi.json; do not edit it manually.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1816b95-1520-4b39-aeaa-bcf48e75742d
📒 Files selected for processing (12)
docs/components/backend/analytics/openapi.jsonsrc/backend/services/analytics/src/api/metric_results.rssrc/backend/services/analytics/src/api/mod.rssrc/backend/services/analytics/src/api/openapi_tests.rssrc/backend/services/analytics/src/domain/metric_definitions/definition.rssrc/backend/services/analytics/src/domain/metric_results/batch.rssrc/backend/services/analytics/src/domain/metric_results/builder.rssrc/backend/services/analytics/src/domain/metric_results/compiler.rssrc/backend/services/analytics/src/domain/metric_results/dto.rssrc/backend/services/analytics/src/domain/metric_results/mod.rssrc/backend/services/analytics/src/domain/metric_results/validation.rssrc/backend/services/analytics/src/domain/metric_results/view.rs
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/backend/services/analytics/src/domain/metric_results/compiler.rs (1)
842-885: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd exact ratio-bind regression assertions.
Placeholder-count checks do not catch a numerator/denominator bind swap after splitting
grouped_value_paramsfrommetric_where_params. Assert exactparamsorder for ratio ranking and both capped and uncapped timeseries queries.🤖 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/backend/services/analytics/src/domain/metric_results/compiler.rs` around lines 842 - 885, Add regression assertions that compare the complete params vector, including exact ordering, for ratio ranking, capped timeseries, and uncapped timeseries queries. Cover numerator and denominator bindings explicitly so swaps are detected, while retaining existing placeholder-count checks and using the relevant query/compiler test symbols.
🤖 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/backend/services/analytics/src/domain/metric_results/compiler.rs`:
- Around line 842-885: Add regression assertions that compare the complete
params vector, including exact ordering, for ratio ranking, capped timeseries,
and uncapped timeseries queries. Cover numerator and denominator bindings
explicitly so swaps are detected, while retaining existing placeholder-count
checks and using the relevant query/compiler test symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e4c995a7-db12-43d3-8d50-d5a101dbfb74
📒 Files selected for processing (10)
docs/components/backend/analytics/openapi.jsonsrc/backend/services/analytics/src/api/metric_results.rssrc/backend/services/analytics/src/api/mod.rssrc/backend/services/analytics/src/domain/metric_definitions/definition.rssrc/backend/services/analytics/src/domain/metric_results/batch.rssrc/backend/services/analytics/src/domain/metric_results/builder.rssrc/backend/services/analytics/src/domain/metric_results/compiler.rssrc/backend/services/analytics/src/domain/metric_results/dto.rssrc/backend/services/analytics/src/domain/metric_results/mod.rssrc/backend/services/analytics/src/domain/metric_results/validation.rs
🚧 Files skipped from review as they are similar to previous changes (8)
- src/backend/services/analytics/src/domain/metric_results/mod.rs
- src/backend/services/analytics/src/domain/metric_definitions/definition.rs
- src/backend/services/analytics/src/api/mod.rs
- src/backend/services/analytics/src/api/metric_results.rs
- src/backend/services/analytics/src/domain/metric_results/batch.rs
- src/backend/services/analytics/src/domain/metric_results/dto.rs
- docs/components/backend/analytics/openapi.json
- src/backend/services/analytics/src/domain/metric_results/validation.rs
Adds bounded top-N dimension groups with stable Other remainder, ranked series totals, and typed metric-results schemas.
Summary by CodeRabbit
group_limitsupport for timeseries results, including ranking by another metric and optional remainder groups.total,rank,remainder, andlabelmetadata.