Run selection persistence#58
Merged
Merged
Conversation
Re-add localStorage-based persistence for runSelectionState in the Polymer tf-runs-selector component. Commit f7f3671 removed hash-based persistence to fix URL accumulation (#42), but also removed all persistence entirely, causing: 1. Run selection between Scalars, Images, and Text dashboards to be independent (each tf-runs-selector instance had its own transient state instead of sharing via storage). 2. Run selection to be forgotten on page reload. The fix uses useLocalStorage: true so tf-storage persists to localStorage instead of the URL hash, preventing accumulation while restoring sharing and reload persistence. Additionally, the NgRx runs effects now write both the NgRx format (_tb_run_selection.v1) and the Polymer format (runSelectionState) to localStorage whenever the selection changes in the time-series dashboard, and read from the Polymer format as a fallback when loading. This keeps the two selection systems in sync. Fixes: #53 Co-authored-by: Samuel <samuel@knutsen.co>
Co-authored-by: Samuel <samuel@knutsen.co>
|
Cursor Agent can help with this pull request. Just |
The time-series tab computes run colors via hash-based OKLCH and stores overrides in NgRx state. Old-style plugin dashboards (Scalars, Images, Text) used a fixed 7-color palette from colorScale.ts, so colors never matched. Fix: the NgRx persistRunColorSettings$ effect now writes the final computed run-name → hex-color map to localStorage (_tb_run_color_map). The Polymer ColorScale.setDomain() reads from that map first, falling back to the palette only for runs not yet present. This means every dashboard tab shows exactly the same colors the time-series tab chose, including any user overrides or clash resolutions. Fixes: #53 Co-authored-by: Samuel <samuel@knutsen.co>
Preview Deployment
Details
|
The previous approach used two localStorage formats: the NgRx format (_tb_run_selection.v1, with experimentId-prefixed run IDs) and a separate Polymer format (runSelectionState, base64-encoded bare names). The NgRx effects wrote both with a 200ms debounce, creating a window where the Polymer format could be stale — causing the selection to reset to all-selected when switching tabs after a page reload. Fix: eliminate the separate Polymer format entirely. The Polymer tf-runs-selector now reads from and writes to _tb_run_selection.v1 directly, converting between run IDs and bare run names on the fly. One key, one source of truth, no synchronisation races. Fixes: #53 Co-authored-by: Samuel <samuel@knutsen.co>
The Polymer tf-category-paginated-view (used by Scalars, Images, Text, etc. dashboards) had no persistence for its opened/collapsed state — it always reset to the _shouldOpen(index) default on reload. Fix: read from and write to the same _tb_tag_group_expansion.v1 localStorage key the NgRx time-series dashboard already uses. On ready(), check the stored map for the category name; on toggle, write the new state back. One source of truth, consistent across all tabs. Fixes: #53 Co-authored-by: Samuel <samuel@knutsen.co>
Co-authored-by: Samuel <samuel@knutsen.co>
The property initializer only runs once at element creation. If the element already exists in the DOM when the user changes state on another tab, it never picks up the update until a full page reload. Fix: re-read from localStorage in attached(), which fires each time the element enters the active DOM (i.e. on every tab switch). Co-authored-by: Samuel <samuel@knutsen.co>
Plugin tabs are shown/hidden via CSS, not DOM detach/reattach, so attached() only fires once. Selection/expansion changes on one tab were invisible to already-created elements on other tabs until a full page reload. Fix: dispatch custom window events (tb-run-selection-changed, tb-tag-group-expansion-changed) whenever either the Polymer or NgRx system writes to localStorage. Each tf-runs-selector and tf-category-paginated-view instance listens for these events and re-reads from localStorage immediately. A guard flag prevents the observer from re-dispatching when syncing inbound changes. Co-authored-by: Samuel <samuel@knutsen.co>
The persistRunColorSettings$ effect used getRunColorMap (which depends on getColorPalette from the settings slice). During Karma test afterAll teardown the store is destroyed, and a pending debounceTime fires, causing 'Cannot read property settings of undefined'. Fix: split the Polymer color map write into its own effect (persistPolymerColorMap$) with catchError so it silently swallows any selector crash during test teardown, while the main color persist effect no longer touches getRunColorMap at all. Co-authored-by: Samuel <samuel@knutsen.co>
Three bugs fixed: 1. persistPolymerColorMap$ used catchError() at the pipe level, which killed the entire subscription on the first error (e.g. settings not loaded yet during startup). The color map was likely never written. Fix: use try/catch inside tap() so the subscription survives. 2. No notification to the Polymer ColorScale when the NgRx system writes the color map. Fix: dispatch 'tb-run-color-map-changed' custom event from persistPolymerColorMap(), and listen for it in createAutoUpdateColorScale(). 3. Runs in old-style dashboards appeared in arbitrary backend order instead of matching the time-series tab. Fix: sort runs alphabetically in tf-runs-selector. Fixes: #53 Co-authored-by: Samuel <samuel@knutsen.co>
The localStorage-sync approach for colors was fundamentally broken by timing — the Polymer side read before the NgRx side wrote. Fix: the Polymer ColorScale now reads _tb_run_colors.v1 directly (the same key persistRunColorSettings$ already writes) and: 1. Looks up each run name in groupKeyToColorId to find its colorId, then computes the OKLCH hex from that exact colorId — matching the NgRx computation exactly. 2. Checks runColorOverrides for user-set colors, which take priority. 3. Falls back to a hash of the bare run name only for runs not yet in the stored data. Removed the broken persistPolymerColorMap$ effect and the _tb_run_color_map intermediate key entirely. Also sorted runs alphabetically in tf-runs-selector (previous commit). Fixes: #53 Co-authored-by: Samuel <samuel@knutsen.co>
The Polymer tf-scalar-card had no persistence for its Y/X axis scale — it always reset to LINEAR. And scale settings from the time-series tab (stored in _tb_axis_scales.v1) were ignored. Fix: tf-scalar-card now reads _tb_axis_scales.v1 on ready(), checking per-tag overrides first, then the global scale. When the user toggles the scale, it writes back to the same key. This means: - Scale persists across page reloads in the Scalars tab - Profile-set scales (e.g. log10 for a specific tag) apply in both the time-series tab and the Scalars tab - User scale changes in either tab are reflected in the other Fixes: #53 Co-authored-by: Samuel <samuel@knutsen.co>
Colors: persistRunColorSettings$ now writes a simple run-name→hex map (_tb_run_color_map) alongside the existing _tb_run_colors.v1, computed from getRunColorMap — the exact final colors the time-series tab shows, including dark mode, overrides, and clash resolution. ColorScale just reads that map. No recomputation, no OKLCH math in Polymer, no room for mismatch. try/catch in the tap guards test teardown. Removed all inline OKLCH code from colorScale.ts. Scales: tf-scalar-card._tagChanged observer replaces ready(), which fired before the tag binding was set (so tag was always undefined and scales were never read). Now the stored scale is applied as soon as the tag property is bound by the parent template. Fixes: #53 Co-authored-by: Samuel <samuel@knutsen.co>
withLatestFrom evaluates the selector synchronously when the debounce fires. If the store is torn down, getRunColorMap throws before tap runs, so try/catch never saw it. Fix: read getRunColorMap inside tap via a synchronous subscribe wrapped in try/catch. Co-authored-by: Samuel <samuel@knutsen.co>
localStorage is always one session behind — the Polymer side reads before the NgRx side writes. Colors never matched. Fix: a live store.select(getRunColorMap).subscribe() in RunsEffects keeps window.__tbRunColorMap updated in real time as the NgRx store changes. ColorScale reads from that first (instant, always correct), falls back to localStorage only for the first render before the subscription fires. Same process, same memory, zero delay. Fixes: #53 Co-authored-by: Samuel <samuel@knutsen.co>
Two bugs: 1. The live subscription set window.__tbRunColorMap but nothing told the Polymer ColorScale to re-read. It had already run setDomain with stale data. Fix: dispatch tb-run-color-map-changed event when the map updates; ColorScale listens and re-runs setDomain. 2. persistRunColorsToLocalStorage wrote an empty _tb_run_color_map when getRunColorMap threw (settings not loaded). On next reload the empty map was read, causing palette fallback. Fix: only write when the map is non-empty. Fixes: #53 Co-authored-by: Samuel <samuel@knutsen.co>
The raw store.select(getRunColorMap).subscribe() threw during test teardown because the selector pipeline (getColorPalette → settings) hit destroyed state. The try/catch in the callback didn't help because the error was in the selector pipeline before the callback ran. Fix: pipe through catchError() so the observable swallows selector errors instead of crashing. Co-authored-by: Samuel <samuel@knutsen.co>
store.select(getRunColorMap) throws on the very first store emission
(before the settings feature slice is initialised). catchError()
catches it but completes the observable — the subscription dies and
window.__tbRunColorMap is never set. Every subsequent emission is lost.
Fix: use store.pipe(map(state => { try { return getRunColorMap(state) }
catch { return null } })) so the selector error is absorbed without
killing the observable. filter() skips null/empty emissions. The
subscription stays alive and fires correctly once all dependencies
(settings, runs, feature flags) are ready.
Co-authored-by: Samuel <samuel@knutsen.co>
Co-authored-by: Samuel <samuel@knutsen.co>
Co-authored-by: Samuel <samuel@knutsen.co>
Co-authored-by: Samuel <samuel@knutsen.co>
Co-authored-by: Samuel <samuel@knutsen.co>
cursor Bot
pushed a commit
that referenced
this pull request
Feb 20, 2026
The syncPolymerRunColorMap$ effect (added in PR #58) uses getRunColorMap which transitively depends on the settings feature selector via getColorPalette. In the karma bundle test, when the settings feature state is not registered in a particular test's MockStore, selectSettingsState returns undefined, causing 'TypeError: Cannot read property settings of undefined' in the memoized selector chain. Fix 1 - settings_selectors.ts: Wrap the bare createFeatureSelector with a createSelector that falls back to initialState when the feature state is undefined. This prevents crashes in any test that evaluates settings-dependent selectors without registering the settings feature. Fix 2 - colorScale.ts: The readColorMap() function threw when window.__tbRunColorMap was not set. During tests and before the first NgRx effect fires, this map is absent. Changed to return null gracefully, skip domain population when the map is absent, and return a neutral fallback color (#808080) from getColor instead of throwing when a run is not in the domain. Co-authored-by: Samuel <samuel@knutsen.co>
Merged
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
2 participants
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.
Motivation for features / changes
This PR fixes two issues related to run selection:
The root cause was identified as commit
f7f3671df, which removedtf-storagepersistence fromtf-runs-selectorentirely, instead of switching from URL hash to localStorage. This change broke both cross-plugin sharing and persistence on reload.Technical description of changes
tensorbored/components/tf_runs_selector/tf-runs-selector.ts: Re-addedtf-storageimport and enabled localStorage-based persistence forrunSelectionStateusinguseLocalStorage: true. This stores selection inlocalStorage['runSelectionState'], restoring shared state across old-style dashboards and persistence on reload.tensorbored/components/tf_runs_selector/BUILD: Re-added the//tensorbored/components/tf_storageBazel dependency.tensorbored/webapp/runs/effects/runs_effects.ts: Implemented bidirectional bridging between the NgRx (time-series dashboard) and Polymer (old-style dashboards) run selection systems:persistRunSelectionToLocalStorageeffect now also writes the Polymer-compatible base64-encoded format to localStorage.loadRunSelectionFromStorage$effect now falls back to reading the Polymer format when the NgRx format is empty, merging selections with the NgRx format taking priority for matching entries.tensorbored/webapp/profile/effects/profile_effects.ts: UpdatedhasStoredRunSelection()to check both the NgRx and Polymer localStorage formats to correctly detect user-set run selections.AGENTS_DEV.md: Updated documentation to reflect the new localStorage keys and the bridging behavior.Screenshots of UI changes (or N/A)
N/A. This is a behavioral fix, not a UI change.
Detailed steps to verify changes work correctly (as executed by you)
bazel build //tensorbored/components/tf_runs_selector:tf_runs_selectorbazel build //tensorbored/webapp/runs/effects:effectsbazel build //tensorbored/webapp/profile/effects:effectsAlternate designs / implementations considered (or N/A)
The original issue (URL hash accumulation) was caused by using the URL hash for
runSelectionState. Re-implementing URL hash persistence was considered and rejected in favor of localStorage to prevent the recurrence of the URL hash accumulation problem while restoring persistence.