Refactor result collection to use nested objects for metrics and configuration - #132
Conversation
…ed objects Co-authored-by: haoranpb <27280733+haoranpb@users.noreply.github.com>
Co-authored-by: haoranpb <27280733+haoranpb@users.noreply.github.com>
…ilot/update-evaluation-result-structure
|
Copilot I see there are many addtional code (e.g. in the result/base.py file) to ensure Backward compatibility. This is NOT needed, do the breaking change and keep the code simple. This might have downstream impact in the code, do the uptake accordingly |
- Remove @Property methods from BaseEvaluationResult for flattened access - Remove backward compatibility in create_result_from_json - Update all downstream code to access nested metrics and experiment fields directly - Update all tests to use nested structure (e.g., result.metrics.execution_time) - Keep code simple by removing unnecessary compatibility layer Co-authored-by: haoranpb <27280733+haoranpb@users.noreply.github.com>
Done! Removed all backward compatibility code:
The code is now much simpler - all access goes through the nested |
There was a problem hiding this comment.
Pull request overview
This PR refactors the result collection system to use nested Pydantic models (AgentMetrics and ExperimentConfiguration) instead of flat fields in BaseEvaluationResult, improving structure and maintainability. It also splits the leaderboard into category-specific files (bug-fix.json and test-generation.json) to better support multiple evaluation categories.
Key changes include:
- Converting
AgentMetrics,ExperimentConfiguration, andEvaluationResultSummaryfrom dataclasses to Pydantic BaseModel - Updating all result access patterns to use nested field access (e.g.,
result.metrics.execution_timeinstead ofresult.agent_execution_time) - Modifying
bcbench result updateCLI command to use--leaderboard-dirand auto-detect category from summaries
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_result_writer.py | Updates test fixtures to create AgentMetrics objects instead of passing flat fields |
| tests/test_result_serialization.py | Updates tests to use nested metrics structure and model_validate() for Pydantic models |
| tests/test_mini_agent_metrics.py | Updates assertions to access metrics through nested result.metrics object |
| tests/test_metrics_to_result_flow.py | Updates all assertions to use result.metrics.execution_time pattern throughout |
| tests/test_evaluation_summary.py | Updates test fixtures to use AgentMetrics objects in result creation |
| tests/test_cli_commands.py | Updates CLI tests for new leaderboard directory structure with category-specific files |
| src/bcbench/types.py | Converts AgentMetrics and ExperimentConfiguration from dataclasses to Pydantic models; moves EvaluationCategory enum earlier in file |
| src/bcbench/results/result_writer.py | Updates to access metrics through nested result.metrics with proper null checking |
| src/bcbench/results/evaluation_result.py | Converts EvaluationResultSummary to Pydantic model with model_dump() for serialization |
| src/bcbench/results/display.py | Updates display functions to access experiment configuration via result.experiment |
| src/bcbench/results/base.py | Refactors result creation to use nested metrics and experiment objects directly |
| src/bcbench/config.py | Changes leaderboard_path to leaderboard_dir to support multiple category files |
| src/bcbench/commands/result.py | Updates CLI to determine leaderboard filename from category and create files on-demand |
| docs/_data/test-generation.json | Creates new empty leaderboard for test-generation category |
| docs/_data/bug-fix.json | Creates new leaderboard with migrated bug-fix entries |
|
Copilot Looks good. Two things left:
|
…mprove code quality - Replace flat fields (mcp_servers, custom_instructions, custom_agent) with nested experiment: ExperimentConfiguration - Add model_validator to handle legacy flat format during deserialization - Simplify nested conditionals in result_writer.py using more concise syntax - Fix redundant elif in base.py (change to else) - Maintain backward compatibility by flattening experiment fields in to_dict() for leaderboard format - All 198 tests passing Co-authored-by: haoranpb <27280733+haoranpb@users.noreply.github.com>
Done! Changes in commit f595ffa:
Also addressed bot review comments:
|
|
Copilot Good, but you are still doing backward compatibility in |
- Remove model_validator that converted legacy flat format - Remove flattening in to_dict() - keep nested experiment structure - Update comparison logic in commands/result.py to compare nested experiment objects - Update all test fixtures to use nested experiment format - Update test assertions to check nested experiment.mcp_servers etc. - All 198 tests passing Co-authored-by: haoranpb <27280733+haoranpb@users.noreply.github.com>
Done in commit 0047632! Removed all backward compatibility:
The code is now fully consistent - nested experiment structure everywhere with no flattening or conversion. |
…onResult from the base module
…improve leaderboard handling
…ilot/update-evaluation-result-structure
After introducing the test-generation category, result storage needed restructuring. The flat field approach (
agent_execution_time,prompt_tokens, etc.) became unwieldy and made category-specific leaderboards difficult to manage.Core Changes
Result Structure
BaseEvaluationResultnow storesmetrics: AgentMetricsandexperiment: ExperimentConfigurationas nested Pydantic models instead of flat fieldsSerialization
create_result_from_json()expects the new nested formatSummary and Types
EvaluationResultSummary,AgentMetrics, andExperimentConfigurationfrom dataclasses to Pydantic modelsfrom_json()methods in favor ofmodel_validate()mcp_servers,custom_instructions,custom_agent) with nestedexperiment: ExperimentConfiguration | Nonefieldto_dict()preserves nested structure - no flattening for backward compatibilityCLI and Leaderboards
leaderboard.jsoninto category-specific files:bug-fix.jsonandtest-generation.jsonbcbench result updatenow uses--leaderboard-dirand auto-detects category from summaryCode Simplification
Breaking Changes
This is a breaking change. Old result files and leaderboard files with flat structure will not be compatible. All code and data must be updated to use the nested structure:
result.metrics.execution_timeinstead ofresult.agent_execution_timesummary.experiment.mcp_serversinstead ofsummary.mcp_serversexperimentobject instead of flatmcp_servers,custom_instructions,custom_agentfieldsOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.