Repository: apache/superset
File: superset-frontend/packages/superset-ui-chart-controls/src/utils/metricColumnFilter.ts (TypeScript)

Code before:
 * Determines if a column should be skipped based on metric filtering logic.
 *
 * This function implements the logic to skip unprefixed percent metric columns
 * if a prefixed version exists, but doesn't skip if it's also a regular metric.
 *
 * @param params - The parameters for metric column filtering
 * @returns true if the column should be skipped, false otherwise
 */
export function shouldSkipMetricColumn({
  colname,
  colnames,
  formData,
}: MetricColumnFilterParams): boolean {
  // Skip unprefixed percent metric columns if a prefixed version exists
  // But don't skip if it's also a regular metric
  return !!(
    colname &&
    !colname.startsWith('%') &&
    colnames.includes(`%${colname}`) &&
    formData.percent_metrics?.some(
      (metric: QueryFormMetric) => getMetricLabel(metric) === colname,
    ) &&
    !formData.metrics?.some(
      (metric: QueryFormMetric) => getMetricLabel(metric) === colname,
    )
  );
}

/**
 * Determines if a column is a regular metric.
 *
 * @param colname - The column name to check
 * @param formData - The form data containing metrics
 * @returns true if the column is a regular metric, false otherwise
 */
export function isRegularMetric(
  colname: string,
  formData: SqlaFormData,
): boolean {
  return !!formData.metrics?.some(metric => getMetricLabel(metric) === colname);
}

/**
 * Determines if a column is a percentage metric.
 *
 * @param colname: string,
 * @param formData - The form data containing percent_metrics
 * @returns true if the column is a percentage metric, false otherwise
 */
export function isPercentMetric(
  colname: string,

Review comment (suggestion):
It might be clearer here if you rewrite the logic following the comment you left, something like (untested):

```js
// Skip unprefixed percent metric columns if a prefixed version exists
// But don't skip if it's also a regular metric
if (!colname || colname.startsWith('%')) {
  return false; // Don't skip prefixed columns or invalid names
}

const hasPrefixedVersion = colnames.includes(`%${colname}`);
const isPercent = isPercentMetric(`%${colname}`, formData);
const isRegular = isRegularMetric(colname, formData);

// Skip if: has prefixed version AND is percent metric AND is NOT regular metric
return hasPrefixedVersion && isPercent && !isRegular;
```

But in general it would be better to use the information from the `form_data`, in order to know if there any percent metrics, instead of parsing the format looking for percent symbols.

Code after:
 * Determines if a column should be skipped based on metric filtering logic.
 *
 * This function implements the logic to skip unprefixed percent metric columns
 * if a prefixed version exists, but doesn't skip if it's also a regular metric.
 *
 * @param params - The parameters for metric column filtering
 * @returns true if the column should be skipped, false otherwise
 */
export function shouldSkipMetricColumn({
  colname,
  colnames,
  formData,
}: MetricColumnFilterParams): boolean {
  if (!colname) {
    return false;
  }

  // Check if this column name exists as a percent metric in form data
  const isPercentMetric = formData.percent_metrics?.some(
    (metric: QueryFormMetric) => getMetricLabel(metric) === colname,
  );

  // Check if this column name exists as a regular metric in form data
  const isRegularMetric = formData.metrics?.some(
    (metric: QueryFormMetric) => getMetricLabel(metric) === colname,
  );

  // Check if there's a prefixed version of this column in the column list
  const hasPrefixedVersion = colnames.includes(`%${colname}`);

  // Skip if: has prefixed version AND is percent metric AND is NOT regular metric
  return hasPrefixedVersion && isPercentMetric && !isRegularMetric;
}

/**
 * Determines if a column is a regular metric.
 *
 * @param colname - The column name to check
 * @param formData - The form data containing metrics
 * @returns true if the column is a regular metric, false otherwise
 */
export function isRegularMetric(
  colname: string,
  formData: SqlaFormData,
): boolean {
  return !!formData.metrics?.some(metric => getMetricLabel(metric) === colname);
}

/**
 * Determines if a column is a percentage metric.
 *