Handle None latency gracefully in PerformanceCostMetric - #286
Merged
Conversation
kwargs.get("latency_ms", 0.0) only falls back to the default when the
key is missing, not when it's explicitly None. The pipeline always
passes latency_ms as a kwarg (None when no LLM is configured or
generation fails), so `None < 0` crashed on every such evaluation -
reproduced in every report checked into reports/*.json.
Fixes #285
|
🎉 Congratulations @himanshu231204! Your pull request has been successfully merged into main. 🚀 Thank you for contributing to OpenAgentHQ and helping improve the project. We truly appreciate your contribution and hope to see you back with more amazing PRs! Happy Open Sourcing! ❤️ |
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
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.
Description
This PR improves the robustness of the
PerformanceCostMetric.evaluate()method to gracefully handle cases where latency measurement is unavailable (e.g., when no LLM is configured or generation fails).Previously, the method defaulted missing
latency_msto0.0, which could mask scenarios where latency data was genuinely unavailable. Now it explicitly checks forNonevalues and returns a score of0.0with appropriate metadata indicating that no measurement was available.Type of Change
How Has This Been Tested?
Added two new unit tests:
test_none_latency_does_not_raise: Verifies that explicitly passinglatency_ms=Noneis handled gracefullytest_missing_latency_kwarg_does_not_raise: Verifies that omittinglatency_msentirely behaves the same as passingNoneBoth tests confirm that the metric returns a score of
0.0and includes appropriate metadata.Checklist
Additional Notes
This change improves observability by distinguishing between "no latency measurement available" (returns
0.0withlatency_ms: Nonein metadata) and actual latency values, making it easier to debug scenarios where latency data is missing.https://claude.ai/code/session_01WBiH9ViQkKjAr6knTz4AWA