fix(#407): commensurable term-fidelity byte meta + retain per-attempt artifacts

.gitignore

@@ -1,4 +1,5 @@
 node_modules/
+node_modules
 out/
 dist/
 bin/relayfile-mount

playwright.term-fidelity.config.ts

@@ -13,7 +13,18 @@ export default defineConfig({
     timeout: 30_000
   },
   outputDir: 'test-results/term-fidelity/playwright',
+  // Retain every attempt's output on a retry-then-pass. A flaky first attempt is
+  // exactly the REAL divergence event we need to examine; `failures-only` would
+  // delete the whole (eventually-passing) test's dirs, including the failed
+  // attempt's error-context.md and trace. The harness also segregates its own
+  // divergence/telemetry bundles under attempt-<retry>/ (see oracle.ts) so a
+  // retry never overwrites the first attempt's data.
+  preserveOutput: 'always',
   use: {
+    // Records a trace per attempt and keeps it for any attempt that failed
+    // (dropped only for clean passes). On retry-then-pass the failed first
+    // attempt's trace is retained; combined with preserveOutput:'always' its
+    // error-context survives too.
     trace: 'retain-on-failure'
   },
   reporter: [['list']]

tests/term-fidelity/README.md

@@ -109,16 +109,27 @@ reconciler subsequently repairs the visible grid.
 A mismatched checkpoint writes:
 
 ```text
-test-results/term-fidelity/<cli>/<workload>/
+test-results/term-fidelity/<cli>/<workload>/attempt-<retry>/
   renderer.txt
   broker.txt
   diff.txt
   screen.png
   meta.json
 ```
 
-`meta.json` includes dimensions, cursors, timestamps, broker offset, quiet-gate
-state, installed Relay package and broker versions, isolated instance details,
-and reconciler telemetry observed during the workload. A telemetry-only failure
-writes its screenshot and metadata under
-`test-results/term-fidelity/<cli>/reconciler-telemetry/`.
+Bundles are segregated by Playwright attempt (`attempt-0/` is the first run,
+`attempt-1/` the first retry, …) so a retry-then-pass never overwrites a real
+first-attempt divergence. The config also sets `preserveOutput: 'always'` +
+`trace: 'retain-on-failure'` so Playwright's own error-context and trace for a
+failed attempt survive even when a later attempt passes.
+
+`meta.json` includes dimensions, cursors, timestamps, quiet-gate state, installed
+Relay package and broker versions, isolated instance details, and reconciler
+telemetry observed during the workload. Byte delivery is recorded under
+`byteAccounting`, which pairs the client-received IPC bytes with the broker's raw
+PTY snapshot offset **on a shared agent-start baseline** and states each figure's
+baseline + unit. Its `clientToBrokerByteRatio` is ~1.0 on a faithful one-to-one
+pipeline; the embedded `note` warns that a near-integer ratio (e.g. the historic
+exact-2.0) is a derivation artifact, never proof of double delivery. A
+telemetry-only failure writes its screenshot and metadata under
+`test-results/term-fidelity/<cli>/reconciler-telemetry/attempt-<retry>/`.

tests/term-fidelity/byte-accounting.test.ts

@@ -0,0 +1,66 @@
+import { describe, expect, it } from 'vitest'
+import { BYTE_ACCOUNTING_NOTE, deriveByteAccounting } from './byte-accounting'
+
+// Pins the client-vs-broker byte derivation used in divergence-bundle meta.
+// The whole point of the module is that the exact-2.0 codex artifact can no
+// longer reach meta as an unlabeled, misreadable pair — so these tests assert
+// the labels, the shared-baseline ratio, and the guard rails.
+describe('deriveByteAccounting', () => {
+  it('reports a ~1.0 ratio for one-to-one delivery on a shared baseline', () => {
+    const acc = deriveByteAccounting({
+      clientBytesReceived: 4096,
+      clientChunks: 12,
+      snapshotOffset: 4096
+    })
+    expect(acc.clientToBrokerByteRatio).toBe(1)
+    expect(acc.commensurable).toBe(true)
+    expect(acc.snapshotOffset).toBe(4096)
+  })
+
+  it('rounds the ratio to 4 dp', () => {
+    const acc = deriveByteAccounting({
+      clientBytesReceived: 1000,
+      clientChunks: 3,
+      snapshotOffset: 3000
+    })
+    expect(acc.clientToBrokerByteRatio).toBe(0.3333)
+  })
+
+  it('surfaces (does not hide) the historic exact-2.0 reading, with the do-not-misread note', () => {
+    const acc = deriveByteAccounting({
+      clientBytesReceived: 8192,
+      clientChunks: 20,
+      snapshotOffset: 4096
+    })
+    // The number is preserved for forensics — but it is explicitly labeled and
+    // carries the note so it can never again be read as "double delivery".
+    expect(acc.clientToBrokerByteRatio).toBe(2)
+    expect(acc.note).toBe(BYTE_ACCOUNTING_NOTE)
+    expect(acc.note).toMatch(/NOT proof of double PTY delivery/)
+    expect(acc.clientUnit).not.toBe(acc.brokerUnit)
+    expect(acc.clientBaseline).toMatch(/probe-install/)
+    expect(acc.brokerBaseline).toMatch(/worker-start/)
+  })
+
+  it('yields a null ratio and non-commensurable flag when the broker predates offsets', () => {
+    const acc = deriveByteAccounting({
+      clientBytesReceived: 500,
+      clientChunks: 4,
+      snapshotOffset: undefined
+    })
+    expect(acc.snapshotOffset).toBeNull()
+    expect(acc.clientToBrokerByteRatio).toBeNull()
+    expect(acc.commensurable).toBe(false)
+  })
+
+  it('yields a null ratio when the offset is zero (avoids divide-by-zero)', () => {
+    const acc = deriveByteAccounting({
+      clientBytesReceived: 0,
+      clientChunks: 0,
+      snapshotOffset: 0
+    })
+    expect(acc.clientToBrokerByteRatio).toBeNull()
+    // The offset was present (0) so the baselines are still comparable.
+    expect(acc.commensurable).toBe(true)
+  })
+})

tests/term-fidelity/byte-accounting.ts

@@ -0,0 +1,83 @@
+// Pure derivation of the client-vs-broker byte accounting recorded in each
+// divergence-bundle `meta.json`. Kept dependency-free (no Playwright, no xterm)
+// so it is unit-testable in isolation — and so the two byte figures can never
+// again be dropped into meta as a bare, unlabeled pair that reads as an
+// "exact-2.0 double-delivery" signal.
+//
+// Background (term-fidelity program, 2026-07-17): `quiet.activity.bytes` and
+// `brokerOffset` read exactly 2.0 apart in 5/5 codex bundles. That was PROVEN
+// NOT to be double delivery — a headless probe measured client-bytes /
+// snapshot-offset = 1.0000, and BrokerManager does exactly one IPC send per
+// worker_stream event. The 2.0 was a derivation artifact of pairing two figures
+// that measure different things on (historically) unstated baselines. This
+// module makes what each figure measures explicit and computes the ratio on a
+// SHARED baseline so the number is meaningful.
+
+export interface ByteAccountingInput {
+  // Client side: cumulative UTF-8 byte length of every `broker:pty-chunk` STRING
+  // this renderer received for the agent since the activity probe was installed.
+  // The probe is installed at harness launch, BEFORE the agent is spawned, so
+  // this series starts at the agent's very first byte (baseline = agent start).
+  clientBytesReceived: number
+  clientChunks: number
+  // Broker side: the attach snapshot's cumulative per-worker byte `offset` — raw
+  // PTY bytes the broker grid had consumed at capture, counted from worker start
+  // (offset 0). `undefined` on brokers that predate stream-offset support.
+  snapshotOffset: number | undefined
+}
+
+export interface ByteAccounting {
+  clientBytesReceived: number
+  clientChunks: number
+  snapshotOffset: number | null
+  // clientBytesReceived / snapshotOffset, rounded to 4 dp. `null` when the
+  // offset is absent or zero (no meaningful ratio). ~1.0 on a faithful
+  // one-IPC-send-per-chunk pipeline.
+  clientToBrokerByteRatio: number | null
+  // True when both figures share the agent-start baseline and are therefore
+  // directly comparable. (Always true when snapshotOffset is present, because
+  // the probe is installed before spawn; recorded explicitly so a future change
+  // that installs the probe mid-stream can flip it to false rather than silently
+  // producing an incomparable ratio.)
+  commensurable: boolean
+  clientBaseline: string
+  brokerBaseline: string
+  clientUnit: string
+  brokerUnit: string
+  note: string
+}
+
+export const BYTE_ACCOUNTING_NOTE =
+  'clientToBrokerByteRatio compares client-received IPC bytes to the broker raw-PTY ' +
+  'offset on a shared agent-start baseline. ~1.0 means one-to-one delivery. A ratio ' +
+  'near an integer such as 2.0 is NOT proof of double PTY delivery — historically it ' +
+  'was a derivation artifact (UTF-8 re-encoding of the decoded IPC string vs raw PTY ' +
+  'bytes, and differing baselines). Never infer a delivery mechanism from this number; ' +
+  'confirm duplicate delivery by counting BrokerManager IPC sends per worker_stream event.'
+
+function round4(value: number): number {
+  return Math.round(value * 10_000) / 10_000
+}
+
+export function deriveByteAccounting(input: ByteAccountingInput): ByteAccounting {
+  const hasOffset =
+    typeof input.snapshotOffset === 'number' &&
+    Number.isFinite(input.snapshotOffset)
+  const snapshotOffset = hasOffset ? (input.snapshotOffset as number) : null
+  const ratio =
+    snapshotOffset !== null && snapshotOffset > 0
+      ? round4(input.clientBytesReceived / snapshotOffset)
+      : null
+  return {
+    clientBytesReceived: input.clientBytesReceived,
+    clientChunks: input.clientChunks,
+    snapshotOffset,
+    clientToBrokerByteRatio: ratio,
+    commensurable: snapshotOffset !== null,
+    clientBaseline: 'activity-probe-install (installed pre-spawn ⇒ agent first byte)',
+    brokerBaseline: 'worker-start (snapshot offset 0)',
+    clientUnit: 'utf8-bytes-of-decoded-broker:pty-chunk-string',
+    brokerUnit: 'raw-pty-bytes-consumed-by-broker-grid',
+    note: BYTE_ACCOUNTING_NOTE
+  }
+}

tests/term-fidelity/harness.ts

@@ -35,6 +35,11 @@ export interface FidelityHarness {
   telemetry: TelemetryRecord[]
   mainLogs: string[]
   currentWorkload: string | null
+  // Playwright retry index (0 = first attempt). Divergence + telemetry bundles
+  // are written under `attempt-<n>/` so a retry never overwrites the prior
+  // attempt's artifacts (retry-then-pass used to erase real first-attempt
+  // divergence data).
+  attempt: number
   close(): Promise<void>
 }
 
@@ -169,7 +174,8 @@ async function validateConnection(
 
 export async function launchFidelityHarness(
   cli: FidelityCli,
-  repoRoot = resolve(__dirname, '../..')
+  repoRoot = resolve(__dirname, '../..'),
+  attempt = 0
 ): Promise<FidelityHarness> {
   // Keep the isolated tree outside the OS temp root. Agent sandboxes commonly
   // grant broad writes beneath TMPDIR, which would make the sibling userData
@@ -319,6 +325,7 @@ export async function launchFidelityHarness(
       relayVersions,
       telemetry,
       mainLogs,
+      attempt,
       get currentWorkload() {
         return harnessState.currentWorkload
       },

tests/term-fidelity/oracle.ts

@@ -3,6 +3,7 @@ import { mkdir, readFile, writeFile } from 'node:fs/promises'
 import { join } from 'node:path'
 import type { Page } from 'playwright'
 import { getActivity, type FidelityHarness } from './harness'
+import { deriveByteAccounting } from './byte-accounting'
 
 export const QUIET_WINDOW_MS = 1_500
 const QUIET_TIMEOUT_MS = 90_000
@@ -384,7 +385,18 @@ async function writeDivergenceBundle(
   quiet: QuietResult,
   options: CheckpointOptions
 ): Promise<string> {
-  const artifactDir = join(harness.repoRoot, 'test-results', 'term-fidelity', harness.cli, workload)
+  // Segregate by Playwright attempt so a retry never overwrites the first
+  // attempt's bundle. A retry-then-pass previously clobbered the diverging
+  // first-attempt data (which twice turned out to be a REAL divergence event),
+  // leaving nothing to examine afterward.
+  const artifactDir = join(
+    harness.repoRoot,
+    'test-results',
+    'term-fidelity',
+    harness.cli,
+    workload,
+    `attempt-${harness.attempt}`
+  )
   await mkdir(artifactDir, { recursive: true })
   const telemetry = harness.telemetry.slice(options.telemetryAtStart)
   const meta = {
@@ -407,7 +419,17 @@ async function writeDivergenceBundle(
       renderer: renderer.cursor,
       broker: broker.cursor
     },
-    brokerOffset: broker.offset,
+    // Self-documenting client-vs-broker byte accounting. Replaces the former
+    // bare `brokerOffset` + `quiet.activity.bytes` pair, whose exact-2.0 reading
+    // in codex bundles was misread as double delivery (it was a derivation
+    // artifact — see byte-accounting.ts / the embedded note). Each figure now
+    // declares its baseline and unit, and the ratio is computed on a shared
+    // agent-start baseline so ~1.0 is the meaningful "one-to-one delivery" value.
+    byteAccounting: deriveByteAccounting({
+      clientBytesReceived: quiet.activity.bytes,
+      clientChunks: quiet.activity.chunks,
+      snapshotOffset: broker.offset
+    }),
     quiet: {
       reached: quiet.reached,
       waitedMs: quiet.waitedMs,
@@ -479,7 +501,8 @@ export async function writeTelemetryArtifact(
     'test-results',
     'term-fidelity',
     harness.cli,
-    'reconciler-telemetry'
+    'reconciler-telemetry',
+    `attempt-${harness.attempt}`
   )
   await mkdir(artifactDir, { recursive: true })
   const screenshot = await harness.page.screenshot({ animations: 'disabled', type: 'png' })

tests/term-fidelity/term-fidelity.spec.ts

@@ -1,4 +1,4 @@
-import { test } from '@playwright/test'
+import { test, type TestInfo } from '@playwright/test'
 import {
   launchFidelityHarness,
   SUPPORTED_CLIS,
@@ -18,12 +18,14 @@ function selectedClis(): FidelityCli[] {
 test.describe.configure({ mode: 'serial' })
 
 for (const cli of selectedClis()) {
-  test(`${cli}: real Electron renderer matches isolated broker for all canonical workloads`, async () => {
+  test(`${cli}: real Electron renderer matches isolated broker for all canonical workloads`, async ({}, testInfo: TestInfo) => {
     let harness: FidelityHarness | null = null
     let workloadError: unknown = null
     let agentName = `tf-${cli}`
     try {
-      harness = await launchFidelityHarness(cli)
+      // Thread the retry index so divergence/telemetry bundles land under
+      // attempt-<n>/ and a retry can't overwrite a prior attempt's artifacts.
+      harness = await launchFidelityHarness(cli, undefined, testInfo.retry)
       console.log(
         `[term-fidelity] ${cli}: instance=${harness.instanceName} port=${harness.apiPort} ` +
         `userData=${harness.userDataDir}`

vitest.config.mjs

@@ -22,7 +22,9 @@ export default {
           include: [
             'src/main/**/*.test.ts',
             'src/renderer/src/**/*.test.ts',
-            'packages/**/*.test.ts'
+            'packages/**/*.test.ts',
+            // Dependency-free term-fidelity harness units (e.g. byte-accounting).
+            'tests/term-fidelity/**/*.test.ts'
           ],
           exclude: [
             '**/node_modules/**',