Every DataPoint read endpoint — /node/{id}/data/series, /node/{id}/data/export
(CSV) and /node/{id}/data/plot (SVG) — could hand back rows whose stored value
did not match the DataPoint’s declared DataType: a non-numeric string in a
DOUBLE series, a DIGITAL sample that was neither 0 nor 1, an unparseable
COLOR, an empty TEXT/JSON. Nothing crashed, which is what made it hard to
notice. Snapshot.doubleValue() deliberately swallows NumberFormatException
and returns 0.0, so a corrupt row rendered as a real-looking zero in the
graph, the plot SVG and the CSV export rather than as an error or an absence.
DataProcessor.range() built a sanitized list and then returned the
unsanitized one:
1
2
logger.d { "... got sanitized: ${range.size} resudced to ${sanitized.size}" }
return range// sanitized
The whole sanitizing loop above it was dead code. It had been that way since the
commit that introduced it — 598a82ada "data processing" flipped
return sanitized to return range// sanitized in the same change that rewrote
GraphScreen.kt by 526 lines, i.e. it looks like a debugging revert that was
never restored, not a deliberate policy.
There was a good reason it got reverted, though, and that is the second half of
the story. The dead loop did not only validate data types — it also replayed the
DataPoint’s Filter children (DiscardAbove / DiscardBelow / Debounce /
Deadband) against stored history. Those filters are ingest-time policy,
already enforced by ServerDataPointProcessor.checkFilters() before a sample is
ever written, so anything in the store has passed them once already. Worse, the
replay had no correct baseline available: it seeded lastGood from
DataStore.last(node), which is the newest sample in the entire store, and
then compared it against the oldest sample in the requested window. With a
Debounce filter configured that makes snapshot.timestamp - previous.timestamp
strongly negative, so the head of every range would be discarded. Simply
restoring return sanitized would have traded a quiet data-integrity bug for a
loud “my graph lost its history” bug.
range() in
server/src/jvmMain/kotlin/krill/zone/server/krillapp/datapoint/DataProcessor.kt
now returns range.filter { validateDataType(node, it) } — read-side
sanitization scoped to data-type validity only, which is stateless, idempotent
and safe to apply on every read. The filter replay, the getFilters() call and
the lastGood seeding are gone from range() entirely, with a KDoc block
recording why they must not come back. last() and verifySnapshotFilters()
still use sanitizeNewSnapshot() unchanged — their previous values genuinely
are the preceding sample, so their filter evaluation is meaningful.
DataProcessorRangeTest in server/src/jvmTest/ covers both directions:
three cases that fail without the fix (corrupt DOUBLE / DIGITAL / TEXT
rows must be dropped) and two that guard against the naive fix (a Deadband of
100 and a 60 s Debounce must not erase valid stored history).
return is a bug report, not a note.
return range// sanitized names the variable that should have been returned.
Grep for the shape — return \w+\s*//\s*\w+ — and for locals that are built
and never read; a val assembled in a loop and then dropped is dead code with
a live intention behind it.git log -S on
the changed token found the introducing commit in seconds, and the 526-line
GraphScreen.kt rewrite sitting in the same commit explained the motive. The
original code was wrong in a second way that the obvious fix would have
reinstated.sanitizeNewSnapshot(filters, node, previous,
snapshot, execute) helper is what let a read path silently acquire
sequence-dependent semantics it had no correct input for.Snapshot.doubleValue() returning 0.0 on garbage is the right call for
downstream math, but it means nothing downstream will ever report the problem.
Anywhere a tolerant accessor is the only consumer, something upstream has to
be doing the rejecting.