Metric description mapping#66
Conversation
The old behavior collected descriptions per-tag from each run's event metadata. When the same tag had different summary_description strings across runs, _build_combined_description() generated a confusing '# Multiple descriptions / ## For run: ...' composite text. This contradicts TensorBored's model where metric descriptions are a global 1:1 mapping from metric name to description (set via the profile's metricDescriptions). Two fixes: 1. Profile descriptions now override event-metadata descriptions (previously they only filled gaps for tags with no event description). 2. When no profile description exists and event metadata has per-run variation, pick the description from the alphabetically first run instead of compositing. Remove _get_tag_description_info and _build_combined_description (dead code after this change). Co-authored-by: Samuel <samuel@knutsen.co>
|
Cursor Agent can help with this pull request. Just |
Preview Deployment
Details
|
When different runs provide different summary_description text for the same metric tag, the backend now returns them as tagRunDescriptions alongside the global tagDescriptions. The global description (from profile or first-run fallback) is always shown at the top. Backend: - Add _get_per_run_tag_descriptions() which collects per-run descriptions only for tags where descriptions actually differ. - Include tagRunDescriptions in the /tags response (omitted when empty). Frontend data flow: - Add TagToRunDescriptions type - Thread tagRunDescriptions through backend types, data source, store types, reducers, and test helpers. UI (scalar, histogram, image cards): - When per-run descriptions exist, replace the plain text matTooltip with a rich popover showing the global description at top and a tab strip below where each tab corresponds to a run with its own description text. - When no per-run descriptions exist, behavior is unchanged (simple matTooltip with tag + description). - Shared popover styles live in _common.scss. Co-authored-by: Samuel <samuel@knutsen.co>
- Don't assign undefined to optional properties (TS2375/TS2412); conditionally spread tagRunDescriptions only when defined. - Run Prettier on changed .ts/.ng.html files. - Run Black on metrics_plugin_test.py. Co-authored-by: Samuel <samuel@knutsen.co>
…tier 2.4.1 - Change tagRunDescriptions observable/input from nullable to empty-object default, avoiding pipe() overload resolution issues with null unions. - Reformat with project-pinned Prettier 2.4.1 (not latest 3.x). Co-authored-by: Samuel <samuel@knutsen.co>
Co-authored-by: Samuel <samuel@knutsen.co>
- Tag name shown in gray/secondary-text weight, description in primary text color with medium weight -- description is the useful info, tag name is just context. - Always use the rich popover when a description exists (not just for per-run descriptions). matTooltip only fires as a plain tag fallback when there is no description at all. - Remove now-unused getTagTooltip/buildTagTooltip from card components. Co-authored-by: Samuel <samuel@knutsen.co>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| <tb-truncated-path | ||
| class="tag" | ||
| [matTooltip]="getTagTooltip(tag, tagDescription)" | ||
| [matTooltip]="tagDescription ? '' : tag" |
There was a problem hiding this comment.
Dual tooltip appears when only run descriptions exist
Medium Severity
The matTooltip binding tagDescription ? '' : tag only suppresses the native tooltip when tagDescription is truthy, but the custom popover's *ngIf also activates when hasRunDescriptions is true. When a tag has per-run descriptions but no global tagDescription, both the matTooltip (showing the tag name) and the custom popover (also showing the tag name plus run descriptions) render simultaneously on hover. The condition needs to account for hasRunDescriptions as well, e.g. (tagDescription || hasRunDescriptions) ? '' : tag.
Additional Locations (2)
Write a diagnostics/convergence_rate scalar via the native writer with a description that varies per run (includes optimizer, lr, batch size, expected convergence step). This makes the per-run description tab view exercisable in the PR preview deployment. Co-authored-by: Samuel <samuel@knutsen.co>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| } | ||
|
|
||
| selectRunTab(run: string) { | ||
| this.selectedRunTab = run; |
There was a problem hiding this comment.
Identical popover logic duplicated across three card components
Low Severity
The properties descriptionTooltipVisible, selectedRunTab, and the methods hasRunDescriptions, runDescriptionEntries, selectedRunDescription, showDescriptionTooltip, hideDescriptionTooltip, and selectRunTab are copy-pasted identically across ScalarCardComponent, HistogramCardComponent, and ImageCardComponent. The same HTML popover template is also duplicated in all three .ng.html files. This triples the maintenance surface — any future fix (like the matTooltip condition bug) needs updating in three places.
Additional Locations (2)
2 participants
Motivation for features / changes
Previously, hovering over a run name in the dashboard often displayed "Multiple descriptions per run" for metrics. This was confusing because descriptions are intended to be a global 1:1 mapping from metric name to description, local to a profile. The issue arose because the system collected descriptions from individual run event metadata, which could vary, leading to a composite "Multiple descriptions" text. Additionally, profile-defined metric descriptions were only used as fallbacks and did not override event metadata.
This change ensures that metric descriptions are consistently 1:1 and that profile definitions take precedence, aligning with the intended behavior.
Technical description of changes
_get_tag_description_infoand_build_combined_descriptionwere removed frommetrics_plugin.py, eliminating the generation of composite descriptions._get_tag_to_descriptioninmetrics_plugin.pywas simplified to always produce a single description per tag. If multiple runs provide different descriptions for the same tag, the description from the alphabetically first run is deterministically chosen._merge_profile_tag_descriptionsinmetrics_plugin.pywas modified to ensure that profile-definedmetricDescriptionsalways take precedence over any descriptions found in event metadata.test_tags_conflicting_descriptionandtest_tags_unsafe_conflicting_descriptioninmetrics_plugin_test.pywere updated to expect a single, deterministic description instead of the composite.test_tags_profile_descriptions_override_event_descriptions, was added to verify that profile descriptions correctly override event metadata descriptions.Screenshots of UI changes (or N/A)
N/A (Backend logic change affecting tooltip text)
Detailed steps to verify changes work correctly (as executed by you)
summary_descriptionvalues in their event metadata. Observe that the dashboard tooltip for this metric now shows a single, deterministic description (from the alphabetically first run that provided one), not the "Multiple descriptions" composite.summary_descriptionin its event metadata, but also define a different description for the same metric in the user profile. Observe that the dashboard tooltip for this metric displays the description from the profile, overriding the event metadata description.metrics_plugin_test.pyto ensure all new and modified test cases pass. (This was done via a standalone test script during development due to environment constraints).Alternate designs / implementations considered (or N/A)
Considered attempting full integration tests, but opted for targeted unit tests and a standalone test script due to complexities with Bazel, TensorFlow, and proto compilation in the development environment. The core design choice to prioritize profile descriptions and enforce a 1:1 mapping was maintained.