Merge pull request #57 from Demonstrandum/cursor/smoothing-url-parameter-b9fd

Smoothing URL parameter

Author: Demonstrandum

tensorbored/webapp/routes/dashboard_deeplink_provider.ts

@@ -41,6 +41,7 @@ import {
   SMOOTHING_KEY,
   TAG_FILTER_KEY,
 } from './dashboard_deeplink_provider_types';
+// Note: SMOOTHING_KEY is only used for deserialization (backwards compat with old URLs).
 import {featureFlagsToSerializableQueryParams} from './feature_flag_serializer';
 
 const COLOR_GROUP_REGEX_VALUE_PREFIX = 'regex:';
@@ -49,17 +50,20 @@ const COLOR_GROUP_REGEX_BY_EXP_VALUE_PREFIX = 'regex_by_exp:';
 /**
  * Provides deeplinking for the core dashboards page.
  *
- * Note: Pinned cards are NO LONGER stored in the URL to avoid URL length
- * limitations. Pins are now stored in localStorage via the profile system.
+ * Note: Pinned cards and smoothing are NO LONGER stored in the URL.
+ * Pins are stored in localStorage via the profile system to avoid URL length
+ * limitations. Smoothing is persisted in localStorage only.
  * See https://github.com/tensorflow/tensorboard/issues/4242 for context.
  *
  * The URL still supports:
  * - tagFilter: for filtering displayed tags
- * - smoothing: scalar smoothing setting
  * - runColorGroup: run color grouping configuration
  * - runFilter: run selector regex filter
  * - Feature flags
  *
+ * For backwards compatibility, smoothing and pinnedCards are still
+ * deserialized from the URL if present (e.g. from old bookmarked URLs).
+ *
  * For sharing complete dashboard configurations including pins, use the
  * profile export/import functionality.
  */
@@ -91,19 +95,6 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
           );
         })
       ),
-      store.select(selectors.getMetricsSettingOverrides).pipe(
-        map((settingOverrides) => {
-          if (Number.isFinite(settingOverrides.scalarSmoothing)) {
-            return [
-              {
-                key: SMOOTHING_KEY,
-                value: String(settingOverrides.scalarSmoothing),
-              },
-            ];
-          }
-          return [];
-        })
-      ),
       store.select(selectors.getRunUserSetGroupBy).pipe(
         map((groupBy) => {
           if (!groupBy) return [];

tensorbored/webapp/routes/dashboard_deeplink_provider_test.ts

@@ -69,30 +69,19 @@ describe('core deeplink provider', () => {
 
   describe('time series', () => {
     describe('smoothing state', () => {
-      it('serializes the smoothing state to the URL', () => {
+      it('does NOT serialize smoothing to the URL (stored in localStorage instead)', () => {
         store.overrideSelector(selectors.getMetricsSettingOverrides, {
-          scalarSmoothing: 0,
+          scalarSmoothing: 0.6,
         });
+        store.overrideSelector(selectors.getMetricsTagFilter, 'trigger');
         store.refreshState();
 
-        expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
-          [
-            {
-              key: 'smoothing',
-              value: '0',
-            },
-          ]
-        );
-      });
-
-      it('does not reflect state when there is no override', () => {
-        store.overrideSelector(selectors.getMetricsSettingOverrides, {});
-
-        store.refreshState();
-
-        expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
-          []
-        );
+        const lastEmission =
+          queryParamsSerialized[queryParamsSerialized.length - 1];
+        expect(lastEmission).toBeDefined();
+        expect(
+          lastEmission.find((p: {key: string}) => p.key === 'smoothing')
+        ).toBeUndefined();
       });
 
       it('deserializes the state in the URL without much sanitization', () => {
@@ -176,23 +165,19 @@ describe('core deeplink provider', () => {
       it('serializes nothing when states are empty', () => {
         store.overrideSelector(selectors.getPinnedCardsWithMetadata, []);
         store.overrideSelector(selectors.getUnresolvedImportedPinnedCards, []);
-        // Trigger an emission by changing a serialized selector (smoothing)
-        store.overrideSelector(selectors.getMetricsSettingOverrides, {
-          scalarSmoothing: 0.5,
-        });
+        // Trigger an emission by changing a serialized selector (tag filter)
+        store.overrideSelector(selectors.getMetricsTagFilter, 'trigger');
         store.refreshState();
 
-        // Verify emission occurred and does NOT contain pinned cards
         const lastEmission =
           queryParamsSerialized[queryParamsSerialized.length - 1];
         expect(lastEmission).toBeDefined();
-        // Should only have smoothing, no pinnedCards
         expect(
           lastEmission.find((p: {key: string}) => p.key === 'pinnedCards')
         ).toBeUndefined();
         expect(
           lastEmission.find((p: {key: string}) => p.key === 'smoothing')
-        ).toBeDefined();
+        ).toBeUndefined();
       });
 
       // Deserialization still works for backwards compatibility with old URLs