Skip to content

Fix Results tab Metric display names containing replacement syntax - #607

Merged
isuttell merged 2 commits into
mainfrom
cursor/spl-587-metric-name-replace-3603
Sep 16, 2026
Merged

isuttell merged 2 commits into
mainfrom
cursor/spl-587-metric-name-replace-3603

Conversation

@isuttell

@isuttell isuttell commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Gate check details on the Results tab rewrite raw Metric ids through withMetricNames. String.replaceAll treats $&, $', and $ in authored Metric display names as JavaScript replacement patterns, so a name like Revenue $& rendered as Revenue metric_x. This change treats the authored display name as literal text.

Changes

  • Pass a replacer function to replaceAll so Metric display names are substituted byte-for-byte.
  • Add a focused Control Panel helper test for $&, $', $, an ordinary display name, and the deleted-Metric id fallback.

Risk: LOW

  • Areas touched: Control Panel Results Gate-check display helper only.
  • Security: Display-only. Names remain React text, not markup.
  • Performance: None.
  • Breaking: None. Gate decisions, stats, Run state, and API payloads are unchanged.

Test plan

Issue-required checks on head 05a8225a483176d2ecd732cd3448ccadf46fd6fc:

  • Focused Control Panel helper test: pnpm --filter @splitch/control-panel exec vitest run --config vitest.config.ts src/lib/experiments/metric-names.test.ts — 1 file / 7 tests passed (2026-09-16 17:24:15Z)
  • Cache-busted CI graph (pnpm verify:ci --force equivalent): pnpm audit --audit-level=high && pnpm exec turbo run //#depcruise //#knip //#format:check //#spec:lint //#docs:lint //#check:cli-mcp-parity //#test:scripts lint typecheck test test:connect-snippet stats:golden stats:property build --force --concurrency=4 — 106/106 successful, 0 cached, 8m42s. Audit: no high/critical findings.

Supporting evidence:

Issue: SPL-587

Linear Issue: SPL-587

Open in Web Open in Cursor 

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Gate check details were rewritten with String.replaceAll, which
interprets $&, $', and $ in authored Metric names as replacement
patterns. Use a replacer function so the display name stays
byte-for-byte.

Issue: SPL-587

Co-authored-by: Isaac Suttell <isaac@zaks.io>
@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. To trigger a review, include @coderabbitai review in the PR description. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: fc230f7f-248e-42a0-baee-98eec95579af

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@isuttell
isuttell marked this pull request as ready for review September 16, 2026 17:20
@isuttell
isuttell merged commit a2b835e into main Sep 16, 2026
4 checks passed
@isuttell
isuttell deleted the cursor/spl-587-metric-name-replace-3603 branch September 16, 2026 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants