LearnNewsExamplesServices
Frontmatter
id17172
titleA p95 over five samples is the maximum: the sustained-liveness helper''s percentile is degenerate at its own call sites
stateClosed
labels
bugaitestingai-generated
assigneesneo-opus-vega
createdAtAug 15, 2026, 2:15 PM
updatedAtAug 15, 2026, 3:33 PM
githubUrlhttps://github.com/neomjs/neo/issues/17172
authorneo-opus-vega
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtAug 15, 2026, 3:33 PM

A p95 over five samples is the maximum: the sustained-liveness helper's percentile is degenerate at its own call sites

Closed Backlog/active-chunk-16 bugaitestingai-generated
neo-opus-vega
neo-opus-vega commented on Aug 15, 2026, 2:15 PM

Context

CI reddened PR #17166 on integration-unified with p95 latency should be <= 500ms / Received: 501. The PR touches ai/services/github-workflow/** and nothing else; the failing spec probes the Dockerized KB/MC plane. The diff and the failure share no surface.

The Problem

assertSustainedHealth computes its p95 as an index into the sorted latency array:

const p95Index = Math.max(0, Math.ceil(latencies.length * 0.95) - 1);
actualP95 = latencies[p95Index];

The formula is correct. ceil(p·n) - 1 is the textbook nearest-rank percentile, and the original author implemented it faithfully. The defect is not the arithmetic but where it is applied: at the sample counts actually in use, the nearest-rank p95 coincides with the maximum.

samples p95Index rank selected samples allowed above it effective percentile
5 4 5 of 5 0 100%
10 9 10 of 10 0 100%
20 18 19 of 20 1 95%
30 28 29 of 30 1 96.7%

Below n = 20, nearest-rank p95 IS the maximum. ceil(0.95n) - 1 === n - 1 whenever ceil(0.95n) === n, which holds for every n ≤ 19. So the assertion labelled "p95 latency should be ≤ 500ms" means "every probe must be ≤ 500ms" — a percentile whose entire purpose is tolerating the slow tail, tolerating none of it.

Interpolation does not rescue it: a linearly-interpolated p95 of 5 samples still sits between the 4th and 5th value, i.e. essentially the max. No percentile definition extracts a meaningful 95th from five samples — the sample is too small for the statistic, and that is the thing the helper must say rather than paper over.

healthcheck.spec.mjs:43-44 passes windowMs: 5000, intervalMs: 1000 → 5 samples. One 501ms probe — a GC pause, a container scheduling hiccup, a noisy runner — fails the build for the whole team. That is what happened.

Why this call site makes it worse

The test is named Sustained liveness composability check (Lane B helper) — 5s/1s. Its stated job is proving the helper composes — that two probes can run concurrently under Promise.all and return. It is not a latency benchmark. But it inherits the helper's full default latency budget, so a composability check gates the merge queue on a 500ms tail it never meant to measure.

Two distinct defects, and they are separable:

  1. The helper's percentile is degenerate below n≈20 and says nothing about it. A caller cannot tell from the signature that windowMs: 5000 silently converts p95 into max.
  2. A composability call site inherits a latency gate it did not ask for.

Precedent — this is the second flake of this class in the same helper

#10918 (closed): HeartbeatPropagation asserted toBeGreaterThan on consecutive process.uptime() samples; on a fast CI runner two probes returned identical fractional seconds and the strict inequality failed. Fixed in PR #10920 by loosening to toBeGreaterThanOrEqual.

Same helper family, same root class: an assertion tuned to a developer's intuition about a real environment, violated by CI's environment without the system being unhealthy. One instance is a fix; two is the template. That is what raises this above "re-run the job".

Honest sizing

This is not currently crippling anything, and the ticket should not be read as claiming so. Measured over the last 60 Tests workflow runs: 7 failures, and all 7 failed on unit. Mine is the only integration-unified failure in that window — roughly 1 spurious red in 61 runs (~1.6%).

What justifies the lane at that rate is not frequency but cost and correctness:

  • The defect is structural, not statistical noise — nearest-rank p95 of 5 samples is the max, provable without running anything. It does not depend on how often it fires.
  • A spurious red costs a full CI cycle, and CI wall-clock is the team's stated throughput constraint right now.
  • It fails in the most expensive direction: an unrelated author debugging a plane they did not touch. I spent this turn on it.

The Fix

Make the helper honest about what its sample count can support:

  • Refuse to assert a percentile it cannot resolve. Below n = 20, fail loudly with cannot assert p95 from N samples and point the caller at a max-latency budget instead — never silently degrade to max under a p95 label. Interpolating is not the alternative; at n = 5 it lands on the max too. A helper that quietly means something other than its parameter name is the same defect class as a workflow header claiming enforcement it lacks (#17171, same session).
  • Give the composability call site what it actually wants. It needs "the helper composes and the plane answers", not a tail-latency gate. Either pass an explicit generous budget with a comment saying why, or let the latency assertion be opt-in.
  • Do not simply raise 500 → 600. That buys silence until the next noisy runner and leaves the label lying.

Acceptance Criteria

  • p95Ms at any sample count either resolves a genuine 95th percentile or refuses, with the refusal naming the sample count. No configuration silently yields max-under-a-p95-label.
  • A unit spec pins the index-selection arithmetic across n = 5, 10, 19, 20, 30, 60 — 19/20 being the boundary where p95 first becomes distinguishable from max — so a future refactor cannot reintroduce the degeneracy unnoticed. Red-proved: restoring the current expression fails it.
  • The 5s/1s composability check no longer gates on a tail-latency budget it did not intend, and the reason is stated at the call site rather than in this ticket.
  • The two HeartbeatPropagation call sites (n=30, tolerating exactly 1 outlier) are reviewed under the same lens and either kept deliberately or adjusted — a decision, not an omission.
  • NEO_INTEGRATION_SUSTAINED_P95_MS keeps working as an escape hatch.

Out of Scope

  • The 500ms budget's value. Whether a healthy plane should answer in 500ms is a separate question from whether the assertion measures what it claims; changing the number here would mask the arithmetic defect.
  • The unit job's own failure rate (7 of 60), which is the larger CI signal and belongs to whoever owns those specs.
  • Docker/Compose plane performance.

Avoided Traps

  • Re-running until green. The job re-run is how I unblocked #17166, and it is not a fix — it converts a wrong assertion into a coin flip and teaches the team to treat integration reds as noise, which is exactly how a real regression gets waved through.
  • Raising the threshold. Buys quiet, keeps the lie.
  • Claiming this is a major flake source. It is one red in 61 runs. Overstating the rate is how a real cause gets dismissed later when someone checks.

Evidence class

L2 — CI artifact from run 31883347460 (results.json: 49 expected, 1 unexpected, Received: 501), plus arithmetic on the index expression reproducible with node -e. The failure-rate figure is a live gh run list census over 60 Tests runs.

Related

#10896 (helper origin) · #10918 / PR #10920 (the first flake of this class in this helper) · PR #17166 (the red that surfaced it) · #17171 (same session, same shape: an artifact whose label claims more than its mechanism delivers)

Live latest-open sweep: latest 20 open issues checked at 2026-08-15T12:14Z, plus a targeted search for p95 / assertSustainedHealth / latency budget across all states — the only hits are #10896 and #10918, both closed. No competing A2A [lane-claim] on integration-suite or CI-flake scope.

Origin Session ID: 5cd926fa-77e1-4309-8bbf-ca563ab07403

Retrieval Hint: query_raw_memories("p95 of five samples is the max assertSustainedHealth degenerate percentile") · falsification anchor: Math.ceil(5*0.95)-1 === 4, the last index of a 5-element array; n = 20 is the smallest sample where nearest-rank p95 differs from max.

tobiu referenced in commit f672ccb - "fix(test): a p95 over five samples is the maximum — refuse the statistic the sample cannot support (#17172) (#17174) on Aug 15, 2026, 3:33 PM
tobiu closed this issue on Aug 15, 2026, 3:33 PM