Add TODO documenting label distinctness assumption in environment comparison sheet#133
Conversation
Co-authored-by: prosenjitdhole <239307697+prosenjitdhole@users.noreply.github.com>
prosenjitdhole
merged commit 581d68d
into
prosenj_cli_hq_eval_report_phase_4
There was a problem hiding this comment.
Pull request overview
Adds an in-code note documenting the assumption that baseline_label and test_label are distinct when building the environment comparison DataFrame, since identical labels would cause dict key collisions and silently lose one column.
Changes:
- Add a TODO comment in
_build_environment_comparison_sheetwarning aboutbaseline_label/test_labeldict-key collision and potential overwrite.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # TODO: baseline_label and test_label are assumed to be distinct (enforced by the caller); | ||
| # if they were ever equal, the dict keys would collide and the baseline column would be | ||
| # overwritten. Consider enforcing distinct labels or using stable column headers if needed. |
There was a problem hiding this comment.
The new TODO says distinctness is "enforced by the caller", but in this codepath generate_comparison_excel() passes through baseline_label/test_label without checking for equality, and hwqueue_pipeline.py derives labels from config/dir names without enforcing distinctness. Please either remove/soften the "enforced by the caller" claim, or add a local guard (e.g., raise/adjust labels) to actually enforce the invariant and avoid silent column overwrites.
_build_environment_comparison_sheetusesbaseline_labelandtest_labelas DataFrame column keys. If both labels were identical, the dict keys would collide and the baseline column would be silently overwritten.Per reviewer confirmation, the caller enforces distinct labels — added a TODO to document this assumption and the associated risk.
Changes
hwqueue_excel.py: Added a TODO comment above therowslist in_build_environment_comparison_sheetnoting that labels are assumed distinct and documenting the dict key collision consequence if that invariant is ever violated.💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.