mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 16:03:20 +00:00
test(diff): calibrate STA-3420 burst assertions against an idle baseline
The burst window's peak lag is dominated by a one-off stall from opening 8x15k-line Monaco editors, which reproduces identically with invalidation disabled. Measure an equal-length idle window first and assert p95, sample coverage, and lag relative to that floor. Adds unit coverage for isUnchangedDiffSectionReload. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -1,5 +1,46 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { shouldRequestCombinedDiffSectionLoad } from './combined-diff-section-load-state'
|
||||
import type { GitDiffResult } from '../../../../shared/types'
|
||||
import type { LargeDiffRenderLimit } from './large-diff-render-limit'
|
||||
import {
|
||||
isUnchangedDiffSectionReload,
|
||||
shouldRequestCombinedDiffSectionLoad
|
||||
} from './combined-diff-section-load-state'
|
||||
|
||||
function textDiff(originalContent: string, modifiedContent: string): GitDiffResult {
|
||||
return {
|
||||
kind: 'text',
|
||||
originalContent,
|
||||
modifiedContent,
|
||||
originalIsBinary: false,
|
||||
modifiedIsBinary: false
|
||||
}
|
||||
}
|
||||
|
||||
function renderLimit(limited: boolean): LargeDiffRenderLimit {
|
||||
return limited
|
||||
? {
|
||||
limited: true,
|
||||
reason: 'line-count',
|
||||
lineCounts: null,
|
||||
characterCount: 0,
|
||||
limits: { maxLinesPerSide: 1, maxCombinedCharacters: 1 }
|
||||
}
|
||||
: { limited: false, lineCounts: { original: 1, modified: 1 }, characterCount: 2 }
|
||||
}
|
||||
|
||||
function loaded(
|
||||
originalContent: string,
|
||||
modifiedContent: string,
|
||||
overrides: { error?: string; limited?: boolean; diffResult?: GitDiffResult } = {}
|
||||
): Parameters<typeof isUnchangedDiffSectionReload>[0] {
|
||||
return {
|
||||
diffResult: overrides.diffResult ?? textDiff(originalContent, modifiedContent),
|
||||
error: overrides.error,
|
||||
largeDiffRenderLimit: renderLimit(overrides.limited ?? false),
|
||||
originalContent,
|
||||
modifiedContent
|
||||
}
|
||||
}
|
||||
|
||||
describe('combined diff section load state', () => {
|
||||
it('requests content when a stale loaded marker has no diff result', () => {
|
||||
@@ -29,3 +70,51 @@ describe('combined diff section load state', () => {
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('isUnchangedDiffSectionReload', () => {
|
||||
it('skips a revalidation that refetched the same text', () => {
|
||||
expect(isUnchangedDiffSectionReload(loaded('before', 'after'), loaded('before', 'after'))).toBe(
|
||||
true
|
||||
)
|
||||
})
|
||||
|
||||
it('commits when either side of the content moved', () => {
|
||||
expect(
|
||||
isUnchangedDiffSectionReload(loaded('before', 'after'), loaded('before', 'rebased'))
|
||||
).toBe(false)
|
||||
expect(
|
||||
isUnchangedDiffSectionReload(loaded('before', 'after'), loaded('rebased', 'after'))
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('commits when the error state changes in either direction', () => {
|
||||
expect(
|
||||
isUnchangedDiffSectionReload(loaded('a', 'b'), loaded('a', 'b', { error: 'boom' }))
|
||||
).toBe(false)
|
||||
expect(
|
||||
isUnchangedDiffSectionReload(loaded('a', 'b', { error: 'boom' }), loaded('a', 'b'))
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('commits when the render limit crosses the fallback boundary', () => {
|
||||
expect(
|
||||
isUnchangedDiffSectionReload(loaded('a', 'b'), loaded('a', 'b', { limited: true }))
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('never skips binary results, whose payload it cannot compare', () => {
|
||||
const binary: GitDiffResult = {
|
||||
kind: 'binary',
|
||||
originalContent: '',
|
||||
modifiedContent: '',
|
||||
originalIsBinary: true,
|
||||
modifiedIsBinary: true
|
||||
}
|
||||
expect(
|
||||
isUnchangedDiffSectionReload(
|
||||
loaded('', '', { diffResult: binary }),
|
||||
loaded('', '', { diffResult: binary })
|
||||
)
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -207,7 +207,7 @@ test.describe('Combined diff invalidation freeze repro (STA-3420)', () => {
|
||||
expect(opened.editorCount).toBeGreaterThan(0)
|
||||
|
||||
const measurement = await orcaPage.evaluate(
|
||||
async ({ wId, repoPath, relativePaths }) => {
|
||||
async ({ wId, repoPath, relativePaths, burstDurationMs }) => {
|
||||
const intervalMs = 50
|
||||
type LagWindow = { maxLagMs: number; p95LagMs: number; sampleCount: number }
|
||||
const startLagMeter = (): (() => LagWindow) => {
|
||||
@@ -245,27 +245,30 @@ test.describe('Combined diff invalidation freeze repro (STA-3420)', () => {
|
||||
}
|
||||
const settle = { ...stopSettle(), settleWindows }
|
||||
|
||||
// Why: settling still leaves occasional multi-hundred-ms stalls from the 8 mounted
|
||||
// 15k-line Monaco editors. Measure an identical idle window so the burst is judged
|
||||
// against this machine's floor rather than a fixed number.
|
||||
const stopBaseline = startLagMeter()
|
||||
await new Promise((resolve) => window.setTimeout(resolve, burstDurationMs))
|
||||
const baseline = stopBaseline()
|
||||
|
||||
const stopBurst = startLagMeter()
|
||||
const startedAt = performance.now()
|
||||
try {
|
||||
// Why: a rebase rewrites the worktree in bursts. The watcher debounces per
|
||||
// path, so each notification lands in its OWN task — never batched together.
|
||||
for (let round = 0; round < 3; round += 1) {
|
||||
for (const relativePath of relativePaths) {
|
||||
window.setTimeout(() => {
|
||||
window.dispatchEvent(
|
||||
new CustomEvent('orca:editor-external-file-change', {
|
||||
detail: { worktreeId: wId, worktreePath: repoPath, relativePath }
|
||||
})
|
||||
)
|
||||
}, 0)
|
||||
}
|
||||
await new Promise((resolve) => window.setTimeout(resolve, 1_000))
|
||||
// Why: a rebase rewrites the worktree in bursts. The watcher debounces per
|
||||
// path, so each notification lands in its OWN task — never batched together.
|
||||
for (let round = 0; round < 3; round += 1) {
|
||||
for (const relativePath of relativePaths) {
|
||||
window.setTimeout(() => {
|
||||
window.dispatchEvent(
|
||||
new CustomEvent('orca:editor-external-file-change', {
|
||||
detail: { worktreeId: wId, worktreePath: repoPath, relativePath }
|
||||
})
|
||||
)
|
||||
}, 0)
|
||||
}
|
||||
await new Promise((resolve) => window.setTimeout(resolve, 15_000))
|
||||
} finally {
|
||||
// no-op: burst meter stopped below so its window covers the awaits above
|
||||
await new Promise((resolve) => window.setTimeout(resolve, 1_000))
|
||||
}
|
||||
await new Promise((resolve) => window.setTimeout(resolve, burstDurationMs - 3_000))
|
||||
const burst = stopBurst()
|
||||
|
||||
const rows = Array.from(
|
||||
@@ -274,7 +277,9 @@ test.describe('Combined diff invalidation freeze repro (STA-3420)', () => {
|
||||
return {
|
||||
elapsedMs: performance.now() - startedAt,
|
||||
settle,
|
||||
baseline,
|
||||
burst,
|
||||
expectedSampleCount: Math.floor(burstDurationMs / intervalMs),
|
||||
editorCount: document.querySelectorAll('.monaco-diff-editor').length,
|
||||
sectionRowCount: rows.length,
|
||||
stuckLoadingRowCount: rows.filter((row) => row.textContent?.includes('Loading diff'))
|
||||
@@ -284,14 +289,22 @@ test.describe('Combined diff invalidation freeze repro (STA-3420)', () => {
|
||||
{
|
||||
wId: worktreeId,
|
||||
repoPath: fixture.repoPath,
|
||||
relativePaths: fixture.relativePaths
|
||||
relativePaths: fixture.relativePaths,
|
||||
burstDurationMs: 18_000
|
||||
}
|
||||
)
|
||||
|
||||
console.log(`external-change burst measurement ${JSON.stringify(measurement)}`)
|
||||
expect(measurement.stuckLoadingRowCount).toBe(0)
|
||||
expect(measurement.editorCount).toBeGreaterThan(0)
|
||||
expect(measurement.burst.maxLagMs).toBeLessThan(1_000)
|
||||
// Why: before the fix this window blocked continuously — p95 3963ms, 16 samples in 23s.
|
||||
expect(measurement.burst.p95LagMs).toBeLessThan(100)
|
||||
expect(measurement.burst.sampleCount).toBeGreaterThan(measurement.expectedSampleCount * 0.85)
|
||||
// Why: peak lag tracks the idle floor of this fixture, not invalidation; only a regression
|
||||
// that adds a full extra second of blocking on top of that floor is this bug returning.
|
||||
expect(measurement.burst.maxLagMs).toBeLessThan(
|
||||
Math.max(measurement.baseline.maxLagMs, 100) + 1_000
|
||||
)
|
||||
} finally {
|
||||
rmSync(fixture.repoPath, { recursive: true, force: true })
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user