-
Notifications
You must be signed in to change notification settings - Fork 20
fix: legacy evaluation reporting to backend with Strategy Pattern #1040
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Chibionos
wants to merge
5
commits into
main
Choose a base branch
from
fix/legacy-eval-request-wrapper
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
This PR fixes legacy evaluation reporting to the backend that was returning HTTP 400 errors and implements the Strategy Pattern for cleaner code separation. ## Changes ### Strategy Pattern Implementation - Created `EvalReportingStrategy` Protocol defining the interface for evaluation reporting strategies - Implemented `LegacyEvalReportingStrategy` for legacy evaluations: - Converts string IDs to deterministic GUIDs using uuid5 - Uses endpoints without /coded/ prefix - Uses assertionRuns format with assertionSnapshot - Implemented `CodedEvalReportingStrategy` for coded evaluations: - Keeps IDs as strings - Uses /coded/ endpoint prefix - Uses evaluatorRuns format with evaluationCriterias ### Bug Fixes - Fixed legacy eval API payload structure for backend compatibility - Added type assertion for project_id to fix mypy errors - Removed unused ABC, abstractmethod imports after Protocol migration ### Test Results - All 27 unit tests passing - All linting checks (ruff, mypy) passing - Integration testing with calculator sample: all API calls returning HTTP 200 OK 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ab72b22 to
c6cd5c3
Compare
- Create _reporting/ package with focused modules - Split strategies, utils, and reporter into separate files - Maintain backward compatibility via re-exports - Split tests to match new structure (48 tests, up from 27) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add INFO-level logging to show inputSchema and outputSchema when creating eval set runs for better debugging - Add DEBUG-level logging for full payloads on all eval reporting operations - Add warning when entrypoint is not provided, falling back to empty schemas - Add tests for agent snapshot extraction behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
mjnovice
reviewed
Dec 20, 2025
| @@ -0,0 +1,418 @@ | |||
| """Evaluation reporting strategies for legacy and coded evaluations. | |||
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we split the strategies into separate files ?
mjnovice
approved these changes
Dec 20, 2025
Split the monolithic _strategies.py into separate files for better code organization: - _strategy_protocol.py: Protocol definition - _legacy_strategy.py: Legacy evaluation reporting strategy - _coded_strategy.py: Coded evaluation reporting strategy - _strategies.py: Re-exports for backward compatibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
test:uipath-langchain
Triggers tests in the uipath-langchain-python repository
test:uipath-llamaindex
Triggers tests in the uipath-llamaindex-python repository
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
_reporting/packageChanges
_reporting/_strategies.py- Protocol + strategy implementations_reporting/_reporter.py- Main StudioWebProgressReporter class with logging_reporting/_utils.py- Error handling decoratorLogging Added
inputSchemaandoutputSchemaTests
🤖 Generated with Claude Code