fix: update metrics table printing to handle backend response format #160
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
- Add typed models for MetricDetail, DatapointResult, DatapointMetric, MetricDatapoints - Update AggregatedMetrics to use details array instead of dynamic keys - Update print_table() to use typed MetricDetail objects - Update get_run_result() to parse datapoints into DatapointResult objects - Fix datapoint rendering to use typed attributes instead of hasattr checks This fixes the issue where the metrics table was printing without values because the SDK expected dynamic metric keys but the backend returns metrics in a 'details' array format per the OpenAPI spec. Co-Authored-By: Dhruv <dhruv@honeyhive.ai>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Contributor
📚 Documentation Preview Built✅ Documentation preview is ready! 📦 Download PreviewDownload documentation artifact 🔍 How to Review
✅ Validation Status
Preview generated for PR #160 |
- Add pylint disable=no-member comments for Pydantic field access - Add unit tests for new typed models (MetricDetail, DatapointResult, etc.) - Add unit tests for print_table() with details array format - Add integration test to validate typed models against real API Co-Authored-By: Dhruv <dhruv@honeyhive.ai>
Contributor
📚 Documentation Preview Built✅ Documentation preview is ready! 📦 Download PreviewDownload documentation artifact 🔍 How to Review
✅ Validation Status
Preview generated for PR #160 |
Co-Authored-By: Dhruv <dhruv@honeyhive.ai>
Contributor
📚 Documentation Preview Built✅ Documentation preview is ready! 📦 Download PreviewDownload documentation artifact 🔍 How to Review
✅ Validation Status
Preview generated for PR #160 |
Co-Authored-By: Dhruv <dhruv@honeyhive.ai>
Contributor
📚 Documentation Preview Built✅ Documentation preview is ready! 📦 Download PreviewDownload documentation artifact 🔍 How to Review
✅ Validation Status
Preview generated for PR #160 |
… AggregatedMetrics Co-Authored-By: Dhruv <dhruv@honeyhive.ai>
Contributor
📚 Documentation Preview Built✅ Documentation preview is ready! 📦 Download PreviewDownload documentation artifact 🔍 How to Review
✅ Validation Status
Preview generated for PR #160 |
…dels Co-Authored-By: Dhruv <dhruv@honeyhive.ai>
Contributor
📚 Documentation Preview Built✅ Documentation preview is ready! 📦 Download PreviewDownload documentation artifact 🔍 How to Review
✅ Validation Status
Preview generated for PR #160 |
3 tasks
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.
fix: update metrics table printing to handle backend response format
Summary
Fixes the issue where the datapoint & aggregate metrics table printed after running
evaluate()was showing empty values.Root cause: The SDK expected metrics as dynamic top-level keys (e.g.,
{"accuracy": {...}}), but the backend returns them in adetailsarray format per the OpenAPI spec:{ "metrics": { "aggregation_function": "average", "details": [{"metric_name": "accuracy", "aggregate": 0.85, ...}] } }Changes:
MetricDetail,DatapointResult,DatapointMetric,MetricDatapointsAggregatedMetricsto usedetailsarray instead of dynamic keysprint_table()to use typed attributes instead ofhasattrchecksget_run_result()to parse datapoints intoDatapointResultobjectsUpdates since last revision
CHANGELOG.mdanddocs/changelog.rstdocs/reference/experiments/models.rstwith API reference for all new typed modelsget_metric(),list_metrics(), andget_all_metrics()support BOTH the newdetailsarray format AND the legacymodel_extraformatAll unit tests pass locally (64 tests in models + results files). Pre-commit checks pass.
Review & Testing Checklist for Human
get_metric()now returnsUnion[MetricDetail, Dict]instead of justDict- check if any downstream code does strict type checking on the return valueAggregatedMetricswithmodel_extraformat should still workDatapointMetric.namevs potentialmetric_name)Recommended test plan:
Notes
detailsarray first, then falls back tomodel_extra- this dual-path logic should be verified--real-apiflag and is skipped in CI