fix: restore original performance comment layout

scripts/performance/report-markdown.test.ts

@@ -1,5 +1,9 @@
 import { expect, test } from 'bun:test'
-import type { MetricComparison, PlatformComparison } from './comparison'
+import {
+  REPORTING_THRESHOLD_PERCENT,
+  type MetricComparison,
+  type PlatformComparison,
+} from './comparison'
 import { renderPerformanceReportMarkdown } from './report-markdown'
 
 const options = {
@@ -33,7 +37,80 @@ function report(
 ): PlatformComparison {
   return { platform: 'ios', ...options, suiteComparable, comparisons: metrics }
 }
-test('large Promise changes and disagreeing process pairs remain visible', () => {
+
+test('restores the original table, emphasis, colors, disclosure, and footer', () => {
+  const text = renderPerformanceReportMarkdown(
+    [
+      report([
+        metric('nitro-cpp/primitive/add-numbers', 20),
+        metric('nitro-platform/promise/immediate', -12.5),
+        metric('javascript/primitive/add-numbers', 2),
+      ]),
+    ],
+    options
+  )
+  expect(text).toMatchInlineSnapshot(`
+    "## Performance Report
+
+    > ⚠️ **Advisory:** Results do not fail this PR.
+
+    ### iOS
+
+    <table>
+      <thead>
+        <tr>
+          <th align="left">Benchmark</th>
+          <th align="right">Before</th>
+          <th align="right">After</th>
+          <th align="left">Difference</th>
+        </tr>
+      </thead>
+      <tbody>
+        <tr>
+          <td><strong>C++</strong> <code>addNumbers()</code></td>
+          <td align="right"><strong>100.0 ns</strong></td>
+          <td align="right">120.0 ns</td>
+          <td>🔴 +20% slower</td>
+        </tr>
+        <tr>
+          <td><strong>Swift</strong> <code>immediatePromise()</code></td>
+          <td align="right">100.0 ns</td>
+          <td align="right"><strong>87.5 ns</strong></td>
+          <td>🟢 -12.5% faster</td>
+        </tr>
+      </tbody>
+    </table>
+
+    <details>
+      <summary>All Benchmarks</summary>
+      <table>
+        <thead>
+          <tr>
+            <th align="left">Benchmark</th>
+            <th align="right">Before</th>
+            <th align="right">After</th>
+            <th align="left">Difference</th>
+          </tr>
+        </thead>
+        <tbody>
+          <tr>
+            <td><strong>JavaScript</strong> <code>addNumbers()</code></td>
+            <td align="right"><strong>100.0 ns</strong></td>
+            <td align="right">102.0 ns</td>
+            <td>🔴 +2% slower</td>
+          </tr>
+        </tbody>
+      </table>
+    </details>
+
+    Benchmarking Code Diff [\`aaaaaaaa\`...\`bbbbbbbb\`](https://github.com/margelo/nitro/compare/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa..bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb) ([view raw output](https://github.com/margelo/nitro/actions/runs/123))
+
+    Raw measurements: [performance-report-2 (JSON artifact)](https://github.com/margelo/nitro/actions/runs/123/artifacts/987). Run 123, attempt 2. Download requires GitHub access.
+    "
+  `)
+})
+
+test('large Promise changes remain visible without duplicate diagnostic tables', () => {
   const text = renderPerformanceReportMarkdown(
     [
       report([
@@ -43,32 +120,130 @@ test('large Promise changes and disagreeing process pairs remain visible', () =>
     ],
     options
   )
-  const main = text.split('<details>')[0]!
+  const [main, collapsed] = text.split('<details>')
   expect(main).toContain('immediatePromise()')
-  expect(main).toContain('+20% slower; process pairs disagree')
+  expect(main).toContain('🔴 +20% slower')
   expect(main).not.toContain('addNumbers()')
-  expect(text).toContain('90.0 ns, 110.0 ns')
-  expect(text).toContain('10.0%, 12.0%')
-  expect(text).toContain('performance-report-2 (JSON artifact)')
-  expect(text).toContain('/actions/runs/123/artifacts/987')
-  expect(text).not.toMatch(/95%|unchanged!|calibrated.*budget/)
+  expect(collapsed).toContain('addNumbers()')
+  expect(collapsed).not.toContain('immediatePromise()')
+  expect(text.match(/<table>/g)).toHaveLength(2)
+  expect(text).not.toMatch(/process pairs|Sample MAD|Base process|95%|noisy/)
+})
+
+test.each([
+  REPORTING_THRESHOLD_PERCENT,
+  -REPORTING_THRESHOLD_PERCENT,
+  REPORTING_THRESHOLD_PERCENT - 0.01,
+  -REPORTING_THRESHOLD_PERCENT + 0.01,
+])('preserves the reporting threshold for a %s%% change', (delta) => {
+  const text = renderPerformanceReportMarkdown(
+    [report([metric('nitro-cpp/primitive/add-numbers', delta)])],
+    options
+  )
+  const [main, collapsed] = text.split('<details>')
+  const visible = Math.abs(delta) >= REPORTING_THRESHOLD_PERCENT
+  expect(main!.includes('addNumbers()')).toBe(visible)
+  expect(collapsed!.includes('addNumbers()')).toBe(!visible)
+  expect(text.match(/<code>addNumbers\(\)<\/code>/g)).toHaveLength(1)
+  expect(text).not.toMatch(/unchanged|equal performance|decisive|calibrat/)
+  if (visible) {
+    expect(collapsed).toContain(
+      `Every benchmark reached the ${REPORTING_THRESHOLD_PERCENT}% reporting threshold.`
+    )
+  } else {
+    expect(main).toContain(
+      `No observed change reached the ${REPORTING_THRESHOLD_PERCENT}% reporting threshold.`
+    )
+  }
 })
-test('small observed changes do not claim equality', () => {
+
+test('equal measurements show zero observed change without bolding either time', () => {
   const text = renderPerformanceReportMarkdown(
     [report([metric('nitro-cpp/primitive/add-numbers', 0, [-30, 30])])],
     options
   )
-  expect(text).toContain('This does not establish equal performance.')
-  expect(text).toContain('process pairs disagree')
+  expect(text).toContain('⚪ ~0% observed change')
+  expect(text.match(/<td align="right">100.0 ns<\/td>/g)).toHaveLength(2)
+  expect(text).not.toMatch(/unchanged|equal performance/)
 })
+
+test('keeps iOS before Android and preserves names, escaping, and time units', () => {
+  const comparison = report([
+    {
+      ...metric('nitro-platform/buffer/copy-1-mib', -6.25),
+      baseMedianNsPerOp: 1_600_000,
+      headMedianNsPerOp: 1_500_000,
+    },
+    {
+      ...metric('turbo-module/primitive/add-numbers', 10),
+      baseMedianNsPerOp: 1_000,
+      headMedianNsPerOp: 1_100,
+    },
+    metric('nitro-platform/custom/a-<b>&"\'', 20),
+  ])
+  const platforms: PlatformComparison[] = [
+    { ...comparison, platform: 'android' },
+    comparison,
+  ]
+  const text = renderPerformanceReportMarkdown(platforms, options)
+  expect(text.indexOf('### iOS')).toBeLessThan(text.indexOf('### Android'))
+  expect(platforms[0]!.platform).toBe('android')
+  expect(text).toContain('<strong>Swift</strong> <code>copy(1 MiB)</code>')
+  expect(text).toContain('<strong>Kotlin</strong> <code>copy(1 MiB)</code>')
+  expect(text).toContain(
+    '<strong>TurboModule</strong> <code>addNumbers()</code>'
+  )
+  expect(text).toContain('1.60 ms')
+  expect(text).toContain('<strong>1.50 ms</strong>')
+  expect(text).toContain('<strong>1.00 µs</strong>')
+  expect(text).toContain('1.10 µs')
+  expect(text).toContain('🟢 -6.25% faster')
+  expect(text).toContain('<code>a-&lt;b&gt;&amp;&quot;&#39;()</code>')
+})
+
 test('same revision and changed suites are explicit', () => {
-  expect(
-    renderPerformanceReportMarkdown([report([], false)], options)
-  ).toContain('require a new baseline')
+  const changedSuite = renderPerformanceReportMarkdown(
+    [report([], false)],
+    options
+  )
+  expect(changedSuite).toContain(
+    '> Benchmark definitions changed in this PR. Results require a new baseline and are not compared.'
+  )
+  expect(changedSuite).not.toContain('<table>')
   expect(
     renderPerformanceReportMarkdown([], {
       ...options,
       headSha: options.baseSha,
     })
   ).toContain('Same-revision baseline run')
 })
+
+test('keeps exact run, raw JSON, and platform artifact provenance', () => {
+  const text = renderPerformanceReportMarkdown([], {
+    ...options,
+    artifacts: {
+      android: {
+        measurementId: 654,
+        measurementAttempt: 2,
+        buildId: 321,
+        buildAttempt: 1,
+      },
+      ios: {
+        measurementId: 765,
+        measurementAttempt: 2,
+        buildId: 432,
+        buildAttempt: 1,
+      },
+    },
+  })
+  expect(text).toContain(`([view raw output](${options.workflowRunUrl}))`)
+  expect(text).toContain(
+    `Raw measurements: [performance-report-2 (JSON artifact)](${options.workflowRunUrl}/artifacts/987). Run 123, attempt 2. Download requires GitHub access.`
+  )
+  expect(text).toContain(
+    `Android: [measurements, attempt 2](${options.workflowRunUrl}/artifacts/654), [apps, attempt 1](${options.workflowRunUrl}/artifacts/321).`
+  )
+  expect(text).toContain(
+    `iOS: [measurements, attempt 2](${options.workflowRunUrl}/artifacts/765), [apps, attempt 1](${options.workflowRunUrl}/artifacts/432).`
+  )
+})

scripts/performance/report-markdown.ts

@@ -89,24 +89,59 @@ function directionalChange(deltaPercent: number): string {
   return '~0% observed change'
 }
 
-function table(
+function difference(metric: MetricComparison): string {
+  if (metric.deltaPercent > 0)
+    return `🔴 ${directionalChange(metric.deltaPercent)}`
+  if (metric.deltaPercent < 0)
+    return `🟢 ${directionalChange(metric.deltaPercent)}`
+  return `⚪ ${directionalChange(metric.deltaPercent)}`
+}
+
+function measurement(
+  metric: MetricComparison,
+  revision: 'base' | 'head'
+): string {
+  const before = metric.baseMedianNsPerOp
+  const after = metric.headMedianNsPerOp
+  const value = revision === 'base' ? before : after
+  const isFaster = revision === 'base' ? before < after : after < before
+  const formatted = formatNumber(value)
+  return isFaster ? `<strong>${formatted}</strong>` : formatted
+}
+
+function renderMetricTable(
   metrics: readonly MetricComparison[],
-  platform: PlatformComparison['platform']
+  platform: PlatformComparison['platform'],
+  indentation = 0
 ): string {
-  return [
-    '| Benchmark | Base p50 | Head p50 | Observed change |',
-    '| --- | ---: | ---: | --- |',
-    ...metrics.map((metric) => {
-      const pairMin = Math.min(...metric.pairChangesPercent)
-      const pairMax = Math.max(...metric.pairChangesPercent)
-      const quality =
-        pairMin <= -REPORTING_THRESHOLD_PERCENT &&
-        pairMax >= REPORTING_THRESHOLD_PERCENT
-          ? '; process pairs disagree'
-          : ''
-      return `| ${benchmarkName(metric.id, platform)} | ${formatNumber(metric.baseMedianNsPerOp)} | ${formatNumber(metric.headMedianNsPerOp)} | ${directionalChange(metric.deltaPercent)}${quality} |`
-    }),
-  ].join('\n')
+  const indent = ' '.repeat(indentation)
+  const level1 = ' '.repeat(indentation + 2)
+  const level2 = ' '.repeat(indentation + 4)
+  const level3 = ' '.repeat(indentation + 6)
+  const lines = [
+    `${indent}<table>`,
+    `${level1}<thead>`,
+    `${level2}<tr>`,
+    `${level3}<th align="left">Benchmark</th>`,
+    `${level3}<th align="right">Before</th>`,
+    `${level3}<th align="right">After</th>`,
+    `${level3}<th align="left">Difference</th>`,
+    `${level2}</tr>`,
+    `${level1}</thead>`,
+    `${level1}<tbody>`,
+  ]
+  for (const metric of metrics) {
+    lines.push(
+      `${level2}<tr>`,
+      `${level3}<td>${benchmarkName(metric.id, platform)}</td>`,
+      `${level3}<td align="right">${measurement(metric, 'base')}</td>`,
+      `${level3}<td align="right">${measurement(metric, 'head')}</td>`,
+      `${level3}<td>${difference(metric)}</td>`,
+      `${level2}</tr>`
+    )
+  }
+  lines.push(`${level1}</tbody>`, `${indent}</table>`)
+  return lines.join('\n')
 }
 
 export function renderPerformanceReportMarkdown(
@@ -124,7 +159,7 @@ export function renderPerformanceReportMarkdown(
   const lines = [
     '## Performance Report',
     '',
-    '> **Report only:** Measurements do not fail this PR. Each benchmark has one base/head process pair. Samples describe within-process variation; they do not establish repeatability between launches or statistical confidence.',
+    '> ⚠️ **Advisory:** Results do not fail this PR.',
   ]
   if (options.baseSha === options.headSha) {
     lines.push(
@@ -138,43 +173,32 @@ export function renderPerformanceReportMarkdown(
     lines.push('', `### ${platformName(platform.platform)}`, '')
     if (!platform.suiteComparable) {
       lines.push(
-        'Benchmark definitions changed. Results require a new baseline and are not compared.'
+        '> Benchmark definitions changed in this PR. Results require a new baseline and are not compared.'
       )
       continue
     }
     const changed = platform.comparisons.filter(
       (metric) => Math.abs(metric.deltaPercent) >= REPORTING_THRESHOLD_PERCENT
     )
-    lines.push(
-      changed.length === 0
-        ? `No observed change reached the ${REPORTING_THRESHOLD_PERCENT}% reporting threshold. This does not establish equal performance.`
-        : table(changed, platform.platform)
+    const other = platform.comparisons.filter(
+      (metric) => Math.abs(metric.deltaPercent) < REPORTING_THRESHOLD_PERCENT
     )
     lines.push(
+      changed.length === 0
+        ? `No observed change reached the ${REPORTING_THRESHOLD_PERCENT}% reporting threshold.`
+        : renderMetricTable(changed, platform.platform),
       '',
       '<details>',
-      '<summary>All benchmarks and sample variation</summary>',
-      '',
-      table(platform.comparisons, platform.platform),
-      '',
-      '| Benchmark | Base process p50 | Head process p50 | Paired changes | Sample MAD / p50 (base, head) |',
-      '| --- | --- | --- | --- | --- |'
-    )
-    for (const metric of platform.comparisons) {
-      lines.push(
-        `| ${benchmarkName(metric.id, platform.platform)} | ${metric.baseProcessMedians.map(formatNumber).join(', ')} | ${metric.headProcessMedians.map(formatNumber).join(', ')} | ${metric.pairChangesPercent.map(directionalChange).join(', ')} | ${metric.baseMadPercent.toFixed(1)}%, ${metric.headMadPercent.toFixed(1)}% |`
-      )
-    }
-    lines.push(
-      '',
-      'p50 is the median of timed batch averages in ns/op, not individual-call latency. MAD describes sample spread; ordered raw samples retain within-process drift.',
-      '',
+      '  <summary>All Benchmarks</summary>',
+      other.length === 0
+        ? `  <p>Every benchmark reached the ${REPORTING_THRESHOLD_PERCENT}% reporting threshold.</p>`
+        : renderMetricTable(other, platform.platform, 2),
       '</details>'
     )
   }
   lines.push(
     '',
-    `Benchmarking Code Diff [\`${options.baseSha.slice(0, 8)}\`...\`${options.headSha.slice(0, 8)}\`](https://github.com/${options.repository}/compare/${options.baseSha}..${options.headSha})${options.workflowRunUrl == null ? '' : ` ([view CI run](${options.workflowRunUrl}))`}`,
+    `Benchmarking Code Diff [\`${options.baseSha.slice(0, 8)}\`...\`${options.headSha.slice(0, 8)}\`](https://github.com/${options.repository}/compare/${options.baseSha}..${options.headSha})${options.workflowRunUrl == null ? '' : ` ([view raw output](${options.workflowRunUrl}))`}`,
     ''
   )
   if (options.artifactId != null && options.workflowRunUrl != null) {

scripts/performance/report-validation.test.ts

@@ -212,11 +212,9 @@ describe('trusted performance report validation', () => {
       expect(markdown).toContain(
         '<strong>C++</strong> <code>addNumbers()</code>'
       )
+      expect(markdown).toContain('<summary>All Benchmarks</summary>')
       expect(markdown).toContain(
-        '<summary>All benchmarks and sample variation</summary>'
-      )
-      expect(markdown).toContain(
-        `Benchmarking Code Diff [\`${BASE_SHA.slice(0, 8)}\`...\`${HEAD_SHA.slice(0, 8)}\`](https://github.com/margelo/nitro/compare/${BASE_SHA}..${HEAD_SHA}) ([view CI run](https://github.com/margelo/nitro/actions/runs/123456789))`
+        `Benchmarking Code Diff [\`${BASE_SHA.slice(0, 8)}\`...\`${HEAD_SHA.slice(0, 8)}\`](https://github.com/margelo/nitro/compare/${BASE_SHA}..${HEAD_SHA}) ([view raw output](https://github.com/margelo/nitro/actions/runs/123456789))`
       )
       const bmf = JSON.parse(
         await readFile(path.join(fixture.output, 'bencher-ios.json'), 'utf8')