findings: a registry, because the codes had already drifted

A finding code is the stable half of a result - what a dashboard groups by and
what someone greps a year of archived runs for. That only holds if a code means
exactly one thing forever, which fifteen ad-hoc string literals cannot promise.

By the time this was written the failure had happened twice:

  - Two emitters independently produced connectivity.downstream_loss and
    connectivity.loss_downstream for the same claim. Nothing objected. Anyone
    aggregating either would have silently seen half their data.
  - Two codes sat under nat.* while being declared Category.CONNECTIVITY.
    nat.udp_unreachable is not about NAT, and the prefix decides the category,
    which decides which verdict light the finding rolls up into. Renamed while
    that is still cheap.

Codes are now typed FindingSpecs carrying category and default severity;
emitters reference the spec rather than retyping the string, so a typo is a
compile error and two call sites cannot disagree about a finding's category.

docs/findings-registry.md is the contract and a test reads it, failing when the
document and the code disagree on which codes exist or how severe they are.
Documentation that drifts from its implementation is worse than none, because it
still looks authoritative. The check reads table rows only, so the prose can go
on explaining which codes were retired and why.

Closes open item 1 of measurement-schema.md section 9.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
mrambossek
2026-08-01 14:25:54 +02:00
co-authored by Claude Fable 5
parent f7701c2d2f
commit e7afc2210f
8 changed files with 475 additions and 22 deletions
@@ -118,7 +118,7 @@ class DownstreamMeasurement(private val ids: IdSource) {
if (!inOrder) {
findings.add(
finding(
"mtu.fragments_blocked", Category.MTU, Severity.MEDIUM, testId,
FindingRegistry.FRAGMENTS_BLOCKED, testId,
"IP fragments do not reach this device",
"A fragmented datagram sent in the normal order never arrived. Anything that " +
"relies on fragmentation — large DNS answers over UDP, some VPN traffic — " +
@@ -133,7 +133,7 @@ class DownstreamMeasurement(private val ids: IdSource) {
}.joinToString(" or ")
findings.add(
finding(
"mtu.fragment_reorder_sensitive", Category.MTU, Severity.LOW, testId,
FindingRegistry.FRAGMENT_REORDER_SENSITIVE, testId,
"Fragments are dropped when they arrive $which",
"In-order fragments are delivered, but the same datagram sent $which is not. " +
"Something on the path only reassembles when the first fragment (the one " +
@@ -198,7 +198,7 @@ class DownstreamMeasurement(private val ids: IdSource) {
if (ipMtu < 1500) {
findings.add(
finding(
"mtu.reduced_downstream", Category.MTU, Severity.LOW, df.test.id,
FindingRegistry.MTU_REDUCED_DOWNSTREAM, df.test.id,
"Downstream path MTU is $ipMtu bytes, below 1500",
"The largest datagram that reached this device without fragmenting was " +
"$pathMtu bytes of payload ($ipMtu on the wire). Tunnels (PPPoE, VPN, " +
@@ -213,7 +213,7 @@ class DownstreamMeasurement(private val ids: IdSource) {
if (fragLargest <= pathMtu && sizes.any { it > pathMtu }) {
findings.add(
finding(
"mtu.downstream_blackhole", Category.MTU, Severity.MEDIUM, frag.test.id,
FindingRegistry.MTU_DOWNSTREAM_BLACKHOLE, frag.test.id,
"Datagrams above $pathMtu bytes are dropped downstream, fragmented or not",
"Nothing larger than $pathMtu bytes arrived, even when the network was " +
"free to fragment it. Traffic that relies on large responses will " +
@@ -226,7 +226,7 @@ class DownstreamMeasurement(private val ids: IdSource) {
if (train.received == 0) {
findings.add(
finding(
"connectivity.downstream_blocked", Category.CONNECTIVITY, Severity.HIGH, train.test.id,
FindingRegistry.DOWNSTREAM_BLOCKED, train.test.id,
"No server-initiated packets arrived",
"The server sent ${train.sent} packets toward this device and none arrived, " +
"while the round-trip echo worked. Something on the path forwards replies " +
@@ -236,7 +236,7 @@ class DownstreamMeasurement(private val ids: IdSource) {
} else if (train.lossPct >= 5.0) {
findings.add(
finding(
"connectivity.downstream_loss", Category.CONNECTIVITY, Severity.MEDIUM, train.test.id,
FindingRegistry.LOSS_DOWNSTREAM, train.test.id,
"Downstream loss of ${round1(train.lossPct)}%",
"${train.sent - train.received} of ${train.sent} packets sent toward this " +
"device were lost. Downstream loss is invisible to a round-trip test, " +
@@ -247,7 +247,7 @@ class DownstreamMeasurement(private val ids: IdSource) {
if (train.reordered > 0) {
findings.add(
finding(
"connectivity.downstream_reorder", Category.CONNECTIVITY, Severity.LOW, train.test.id,
FindingRegistry.DOWNSTREAM_REORDER, train.test.id,
"${train.reordered} downstream packet(s) arrived out of order",
"Packets arrived in a different order than they were sent. Usually per-packet " +
"load balancing across links; harmless for most traffic, not for all of it.",
@@ -414,9 +414,17 @@ class DownstreamMeasurement(private val ids: IdSource) {
// ---- helpers ----------------------------------------------------------------------
private fun finding(code: String, cat: Category, sev: Severity, testId: String, title: String, desc: String) =
/**
* Builds a finding from a registry entry, which supplies the code, category and severity.
*
* Taking a [FindingSpec] rather than three loose values is the point: a typo becomes a
* compile error, and two call sites cannot disagree about which category a finding belongs
* to - a disagreement that would split one fault across two verdict lights.
*/
private fun finding(spec: FindingSpec, testId: String, title: String, desc: String) =
Finding(
id = ids.uuid(), code = code, category = cat, severity = sev, confidence = Confidence.HIGH,
id = ids.uuid(), code = spec.code, category = spec.category, severity = spec.severity,
confidence = Confidence.HIGH,
title = title, description = desc, evidenceRefs = listOf(EvidenceRef(testId)),
)
@@ -208,11 +208,11 @@ class ServerMeasurement(
val findings = ArrayList<Finding>()
if (received == 0) {
findings.add(finding("nat.udp_unreachable", Category.CONNECTIVITY, Severity.HIGH, testId,
findings.add(finding(FindingRegistry.UDP_UNREACHABLE, testId,
"No UDP echo replies from the server",
"Every ECHO probe to the server's UDP data plane was lost — the path blocks or drops the session's UDP traffic."))
} else if (lossPct >= 20.0) {
findings.add(finding("connectivity.udp_loss", Category.CONNECTIVITY, Severity.MEDIUM, testId,
findings.add(finding(FindingRegistry.UDP_LOSS, testId,
"High UDP loss to the server (${round1(lossPct)}%)",
"A large fraction of ECHO probes were lost, indicating an unreliable UDP path."))
}
@@ -220,14 +220,14 @@ class ServerMeasurement(
directional?.let { d ->
when {
d.noneReachedServer && received == 0 -> findings.add(
finding("nat.udp_unreachable_upstream", Category.CONNECTIVITY, Severity.HIGH, testId,
finding(FindingRegistry.UDP_UNREACHABLE_UPSTREAM, testId,
"Nothing reached the server",
"The server received none of the ${d.sent} probes, so the traffic is being " +
"dropped on the way out, not on the way back. A firewall or NAT on " +
"this side of the path is the place to look."),
)
d.lossUpstreamPct >= 2.0 -> findings.add(
finding("connectivity.loss_upstream", Category.CONNECTIVITY, Severity.MEDIUM, testId,
finding(FindingRegistry.LOSS_UPSTREAM, testId,
"${d.lossUpstreamPct} % of probes were lost on the way to the server",
"${d.lostUpstream} of ${d.sent} probes never reached the server. The " +
"return path is not implicated: replies came back for everything that " +
@@ -236,7 +236,7 @@ class ServerMeasurement(
}
if (d.lossDownstreamPct >= 2.0) {
findings.add(
finding("connectivity.loss_downstream", Category.CONNECTIVITY, Severity.MEDIUM, testId,
finding(FindingRegistry.LOSS_DOWNSTREAM, testId,
"${d.lossDownstreamPct} % of replies were lost on the way back",
"The server received ${d.seenByServer} probes and answered them, but " +
"${d.lostDownstream} of those replies never arrived. The outbound path " +
@@ -246,7 +246,7 @@ class ServerMeasurement(
}
if (natRebinding) {
findings.add(finding("nat.udp_rebinding", Category.NAT, Severity.MEDIUM, testId,
findings.add(finding(FindingRegistry.NAT_UDP_REBINDING, testId,
"NAT remapped the UDP source port mid-flow",
"The server observed more than one source port for this session (${observedPorts.joinToString()}), i.e. a NAT with a short UDP mapping or per-packet remapping."))
}
@@ -276,9 +276,17 @@ class ServerMeasurement(
}
}
private fun finding(code: String, cat: Category, sev: Severity, testId: String, title: String, desc: String) =
/**
* Builds a finding from a registry entry, which supplies the code, category and severity.
*
* Taking a [FindingSpec] rather than three loose values is the point: a typo becomes a
* compile error, and two call sites cannot disagree about which category a finding belongs
* to - a disagreement that would split one fault across two verdict lights.
*/
private fun finding(spec: FindingSpec, testId: String, title: String, desc: String) =
Finding(
id = ids.uuid(), code = code, category = cat, severity = sev, confidence = Confidence.HIGH,
id = ids.uuid(), code = spec.code, category = spec.category, severity = spec.severity,
confidence = Confidence.HIGH,
title = title, description = desc, evidenceRefs = listOf(EvidenceRef(testId)),
)
@@ -120,7 +120,7 @@ class ThroughputMeasurement(private val ids: IdSource) {
sender == null -> Unit // no sender report: nothing can be concluded, so nothing is
received.isEmpty() -> findings.add(
finding(
"perf.throughput_no_delivery", Category.PERFORMANCE, Severity.HIGH, testId,
FindingRegistry.THROUGHPUT_NO_DELIVERY, testId,
"No throughput traffic arrived",
"The server sent ${sender.packets} packets and none arrived. This is a " +
"connectivity fault rather than a slow link.",
@@ -128,7 +128,7 @@ class ThroughputMeasurement(private val ids: IdSource) {
)
networkLimited -> findings.add(
finding(
"perf.throughput_below_offered", Category.PERFORMANCE, Severity.LOW, testId,
FindingRegistry.THROUGHPUT_BELOW_OFFERED, testId,
"Downstream throughput ${receivedKbps / 1000} Mbit/s, below the " +
"${sender.kbps / 1000} Mbit/s offered",
"The server sent at ${sender.kbps / 1000} Mbit/s for the full run and " +
@@ -169,9 +169,17 @@ class ThroughputMeasurement(private val ids: IdSource) {
private fun parseInt(body: String?, key: String): Int? =
body?.let { Regex("\"$key\"\\s*:\\s*(-?\\d+)").find(it)?.groupValues?.get(1)?.toIntOrNull() }
private fun finding(code: String, cat: Category, sev: Severity, testId: String, title: String, desc: String) =
/**
* Builds a finding from a registry entry, which supplies the code, category and severity.
*
* Taking a [FindingSpec] rather than three loose values is the point: a typo becomes a
* compile error, and two call sites cannot disagree about which category a finding belongs
* to - a disagreement that would split one fault across two verdict lights.
*/
private fun finding(spec: FindingSpec, testId: String, title: String, desc: String) =
Finding(
id = ids.uuid(), code = code, category = cat, severity = sev, confidence = Confidence.HIGH,
id = ids.uuid(), code = spec.code, category = spec.category, severity = spec.severity,
confidence = Confidence.HIGH,
title = title, description = desc, evidenceRefs = listOf(EvidenceRef(testId)),
)
@@ -0,0 +1,175 @@
// SPDX-FileCopyrightText: 2026 Echolot contributors
// SPDX-License-Identifier: GPL-3.0-or-later
package app.echo_lot.measurement
/**
* The registry of finding codes (measurement-schema.md §9, open item 1).
*
* A finding code is the stable, machine-readable half of a result: the prose changes, the code is
* what a dashboard groups by and what someone greps a year of archived runs for. That only holds
* if a code means exactly one thing forever — which is not something ad-hoc string literals at
* fifteen call sites can promise.
*
* The failure this exists to prevent had already happened by the time it was written. Two
* independently-added emitters produced `connectivity.downstream_loss` and
* `connectivity.loss_downstream` for the same concept, and nothing anywhere objected. Anyone
* aggregating either one would have silently seen half their data.
*
* So codes are declared here as typed specs, each carrying its category and default severity, and
* emitters reference the spec rather than retyping the string. That makes a typo a compile error,
* and makes it impossible for two call sites to disagree about which category a finding belongs
* to — a disagreement that would otherwise split one fault across two verdict lights.
*/
data class FindingSpec(
val code: String,
val category: Category,
/** Severity when nothing about the specific run argues otherwise; emitters may escalate. */
val severity: Severity,
/** One line: what this finding asserts. Present tense, no hedging. */
val meaning: String,
/**
* What the finding rules *out*, where that is the useful half. "Loss upstream" is worth much
* more when it also says the return path is fine, because that halves where to look next.
*/
val rulesOut: String? = null,
)
object FindingRegistry {
// ---- connectivity ----------------------------------------------------------------
// Renamed from nat.* before anything shipped: neither of these is about NAT, and the
// prefix is what decides which category - and therefore which verdict light - a finding
// rolls up into. A nat.* code landing under connectivity would be a permanent puzzle.
val UDP_UNREACHABLE = FindingSpec(
"connectivity.udp_unreachable", Category.CONNECTIVITY, Severity.HIGH,
"No UDP echo replies came back from the server at all.",
)
val UDP_UNREACHABLE_UPSTREAM = FindingSpec(
"connectivity.udp_unreachable_upstream", Category.CONNECTIVITY, Severity.HIGH,
"The server received none of the probes, so traffic is dropped on the way out.",
rulesOut = "The return path: nothing arrived to be replied to.",
)
val UDP_LOSS = FindingSpec(
"connectivity.udp_loss", Category.CONNECTIVITY, Severity.MEDIUM,
"A large fraction of round-trip probes were lost, direction unknown.",
)
val LOSS_UPSTREAM = FindingSpec(
"connectivity.loss_upstream", Category.CONNECTIVITY, Severity.MEDIUM,
"Probes were lost on the way to the server.",
rulesOut = "The return path: replies came back for everything that arrived.",
)
/**
* The single code for "lost on the return path", whichever measurement found it.
*
* Two emitters had independently invented `connectivity.downstream_loss` and
* `connectivity.loss_downstream` for this, and nothing objected. Anyone aggregating either
* one would have silently seen half their data. Paired with [LOSS_UPSTREAM] so the two
* directions read as a set.
*/
val LOSS_DOWNSTREAM = FindingSpec(
"connectivity.loss_downstream", Category.CONNECTIVITY, Severity.MEDIUM,
"Packets were lost on the way back from the server.",
rulesOut = "The outbound path: the server received what it was answering.",
)
val DOWNSTREAM_BLOCKED = FindingSpec(
"connectivity.downstream_blocked", Category.CONNECTIVITY, Severity.HIGH,
"Server-initiated packets never arrive, although round trips work.",
rulesOut = "Basic reachability: the path forwards replies, just not unsolicited traffic.",
)
val DOWNSTREAM_REORDER = FindingSpec(
"connectivity.downstream_reorder", Category.CONNECTIVITY, Severity.LOW,
"Downstream packets arrive in a different order than they were sent.",
)
val CAPTIVE_PORTAL = FindingSpec(
"connectivity.captive_portal", Category.CONNECTIVITY, Severity.HIGH,
"A captive portal is intercepting connectivity checks.",
)
val NO_INTERNET = FindingSpec(
"connectivity.no_internet", Category.CONNECTIVITY, Severity.HIGH,
"Android's own connectivity checks fail on this network.",
)
// ---- mtu -------------------------------------------------------------------------
val MTU_REDUCED_DOWNSTREAM = FindingSpec(
"mtu.reduced_downstream", Category.MTU, Severity.LOW,
"The downstream path MTU is below the usual 1500 bytes.",
)
val MTU_DOWNSTREAM_BLACKHOLE = FindingSpec(
"mtu.downstream_blackhole", Category.MTU, Severity.MEDIUM,
"Datagrams above the path MTU are dropped downstream, fragmented or not.",
)
val FRAGMENTS_BLOCKED = FindingSpec(
"mtu.fragments_blocked", Category.MTU, Severity.MEDIUM,
"IP fragments do not reach this device even when sent in order.",
)
val FRAGMENT_REORDER_SENSITIVE = FindingSpec(
"mtu.fragment_reorder_sensitive", Category.MTU, Severity.LOW,
"Fragments are delivered in order but dropped when reordered or delayed.",
rulesOut = "Fragmentation itself: in-order fragments arrive fine.",
)
// ---- nat -------------------------------------------------------------------------
val NAT_UDP_REBINDING = FindingSpec(
"nat.udp_rebinding", Category.NAT, Severity.MEDIUM,
"A NAT remapped the UDP source port mid-flow.",
)
val NAT_SYMMETRIC = FindingSpec(
"nat.symmetric", Category.NAT, Severity.MEDIUM,
"The NAT assigns a different external port per destination.",
)
// ---- perf ------------------------------------------------------------------------
val THROUGHPUT_NO_DELIVERY = FindingSpec(
"perf.throughput_no_delivery", Category.PERFORMANCE, Severity.HIGH,
"No throughput traffic arrived, although the server sent it.",
)
val THROUGHPUT_BELOW_OFFERED = FindingSpec(
"perf.throughput_below_offered", Category.PERFORMANCE, Severity.LOW,
"Less throughput arrived than the server sent for the whole run.",
)
// ---- dns -------------------------------------------------------------------------
val DNS_ANSWER_REWRITTEN = FindingSpec(
"dns.answer_rewritten", Category.DNS, Severity.HIGH,
"A resolver returned an answer that differs from the authoritative record.",
)
val DNS_AUTHORITATIVE_UNREACHABLE = FindingSpec(
"dns.authoritative_unreachable", Category.DNS, Severity.MEDIUM,
"The canary zone's authoritative server could not be reached.",
)
/** Every registered finding, in declaration order. */
val all: List<FindingSpec> = listOf(
UDP_UNREACHABLE, UDP_UNREACHABLE_UPSTREAM, UDP_LOSS, LOSS_UPSTREAM, LOSS_DOWNSTREAM,
DOWNSTREAM_BLOCKED, DOWNSTREAM_REORDER, CAPTIVE_PORTAL, NO_INTERNET,
MTU_REDUCED_DOWNSTREAM, MTU_DOWNSTREAM_BLACKHOLE, FRAGMENTS_BLOCKED,
FRAGMENT_REORDER_SENSITIVE,
NAT_UDP_REBINDING, NAT_SYMMETRIC,
THROUGHPUT_NO_DELIVERY, THROUGHPUT_BELOW_OFFERED,
DNS_ANSWER_REWRITTEN, DNS_AUTHORITATIVE_UNREACHABLE,
)
private val byCode: Map<String, FindingSpec> = all.associateBy { it.code }
fun byCode(code: String): FindingSpec? = byCode[code]
}
@@ -0,0 +1,133 @@
// SPDX-FileCopyrightText: 2026 Echolot contributors
// SPDX-License-Identifier: GPL-3.0-or-later
package app.echo_lot.measurement
import java.io.File
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertTrue
import kotlin.test.fail
/**
* Keeps the finding registry honest.
*
* The interesting test is the last one: it reads `docs/findings-registry.md` and fails when the
* document and the code disagree. Documentation that drifts from its implementation is worse than
* none, because it still looks authoritative — and a finding registry is precisely the artifact
* other people build tooling against.
*/
class FindingRegistryTest {
@Test
fun codesAreUnique() {
val dupes = FindingRegistry.all.groupBy { it.code }.filterValues { it.size > 1 }.keys
assertTrue(dupes.isEmpty(), "duplicate finding codes: $dupes")
}
@Test
fun everyDeclaredSpecIsInTheAllList() {
// Reflection over the object's properties: a spec that is declared but left out of `all`
// is invisible to the doc check and to any consumer enumerating the registry.
val declared = FindingRegistry::class.java.declaredMethods
.filter { it.parameterCount == 0 && it.returnType == FindingSpec::class.java }
.mapNotNull { runCatching { it.invoke(FindingRegistry) as FindingSpec }.getOrNull() }
.map { it.code }
.toSet()
val listed = FindingRegistry.all.map { it.code }.toSet()
assertEquals(declared, listed, "declared specs and the `all` list disagree")
}
// The prefix decides the category, and the category decides which verdict light the finding
// rolls up into. A code whose prefix disagrees with its category silently moves a fault to a
// different light — the exact bug that got two codes renamed out of nat.*.
@Test
fun everyPrefixMatchesItsCategory() {
for (spec in FindingRegistry.all) {
val fromPrefix = TestType.category(spec.code)
assertEquals(
fromPrefix, spec.category,
"${spec.code} is declared as ${spec.category} but its prefix maps to $fromPrefix",
)
}
}
@Test
fun codesFollowTheNamingConvention() {
val shape = Regex("^[a-z0-9]+\\.[a-z0-9_]+$")
for (spec in FindingRegistry.all) {
assertTrue(shape.matches(spec.code), "malformed code: ${spec.code}")
assertTrue(spec.meaning.isNotBlank(), "${spec.code} has no meaning")
assertTrue(
spec.meaning.trimEnd().endsWith("."),
"${spec.code}'s meaning should be a sentence: '${spec.meaning}'",
)
}
}
// Two near-identical codes are how one fault ends up split across two dashboards. This is a
// blunt check — it will not catch every synonym — but it catches the shape that already
// happened: the same words in a different order.
@Test
fun noTwoCodesAreAnagramsOfEachOther() {
val normalised = FindingRegistry.all.associate { spec ->
spec.code to spec.code.substringAfter('.').split('_').sorted().joinToString("_")
}
val clashes = normalised.entries.groupBy { it.value }.filterValues { it.size > 1 }
if (clashes.isNotEmpty()) {
fail("codes differing only in word order: ${clashes.values.map { g -> g.map { it.key } }}")
}
}
@Test
fun theDocumentAndTheRegistryAgree() {
val doc = findDoc() ?: run {
println("findings-registry.md not found from ${File(".").absolutePath} — skipping")
return
}
val text = doc.readText()
// Only table rows count as "documented". Prose may legitimately mention a code that no
// longer exists — the rules section explains why two were merged — and treating that as
// a registry entry would force the document to forget its own history.
val documented = text.lines()
.filter { it.trimStart().startsWith("|") }
.flatMap { row -> Regex("`([a-z0-9]+\\.[a-z0-9_]+)`").findAll(row).map { it.groupValues[1] } }
.toSet()
val registered = FindingRegistry.all.map { it.code }.toSet()
val missingFromDoc = registered - documented
val missingFromCode = documented - registered
assertTrue(
missingFromDoc.isEmpty(),
"these codes exist in FindingRegistry but not in docs/findings-registry.md: $missingFromDoc",
)
assertTrue(
missingFromCode.isEmpty(),
"docs/findings-registry.md documents codes that no longer exist: $missingFromCode",
)
// And the severities must match, or the document is describing a different system.
for (spec in FindingRegistry.all) {
val row = text.lines().firstOrNull {
it.trimStart().startsWith("|") && it.contains("`${spec.code}`")
} ?: continue
val severity = spec.severity.name.lowercase()
assertTrue(
row.contains("| $severity |"),
"${spec.code} is ${severity} in code but the doc row says otherwise: $row",
)
}
}
/** Walks up from the test's working directory to find the repo's docs/ folder. */
private fun findDoc(): File? {
var dir: File? = File(".").absoluteFile
repeat(6) {
val candidate = File(dir, "docs/findings-registry.md")
if (candidate.isFile) return candidate
dir = dir?.parentFile
}
return null
}
}