DEV-793 Robust slow-sensor gap window: accumulated median + symmetric limits (review follow-up for #285)

[jyong15]

Follow-up to the review on #285 (jyong15, 2026-08-20). Targets DEV-793_slow_sensor_gap_window so it can be merged into that PR before it lands on master. No version change in ShimmerDriver/build.gradle.

Findings addressed

1. MAJOR — self-referential gap window

refineSlowSensorSamplingRateFromBlockTicks re-derived the window on every payload from the same per-sample periods it then had to judge (perSamplePeriodsS[i] = (deltaTicks/32768.0)/sampleCount, and the checker computes exactly 1/period per boundary). With only 2-3 slow-sensor blocks per payload, a dropped block's 2x spacing was absorbed into the window that was supposed to catch it — and the healthy boundary back to the previous payload got flagged instead.

Fix: the measured per-sample periods are now accumulated per sensor across the payloads of a parse run in UtilCsvSplitting.SLOW_SENSOR_OBSERVED_PERIODS_PER_SENSOR, and the window is derived from a true median over that history rather than from the latest payload's values. A single 2x outlier in hundreds of measurements barely moves the median, so the real gap stays outside the window.

2. MAJOR — fast side of the window

limits[1] used FILE_GAP_TOLERANCE_MULTIPLIER.UPPER / minObservedPeriod — a single-sample extremum, so one fast spacing widened the fast edge for the whole recording. Both sides now come from the same robust median, in the new UtilCsvSplitting.calculateSlowSensorSamplingRateLimits(double):

limits[0] = medianRateHz / SLOW_SENSOR_MAX_INTER_BLOCK_GAP_RATIO
limits[1] = medianRateHz * FILE_GAP_TOLERANCE_MULTIPLIER.UPPER

3. MINOR — upper-median

perSamplePeriodsS.get(size/2) is the upper median for even counts. The surviving median computation is now UtilCsvSplitting.calculateMedian(List<Double>), which averages the two middle values for an even-sized input (and sorts a copy, so the caller's list is not re-ordered).

4. MINOR — stale javadoc

SLOW_SENSOR_MAX_INTER_BLOCK_GAP_RATIO's javadoc no longer claims the window is "seeded from the first payload that carries >= 2 blocks" (behaviour removed by #285) and no longer claims 1.5x "keeps comfortable margin on both sides" (it only ever applied to the gap side). It now describes the accumulated-median basis and points at the method that builds the limits.

5. MINOR — tests

New ShimmerDriver/src/test/java/com/shimmerresearch/verisense/payloaddesign/API_00009_UtilCsvSplittingSlowSensorGapWindow.java — JUnit 4, matching the existing API_000NN_* naming/style in ShimmerDriver/src/test. No hardware data or binary fixtures needed: the tests drive the window through per-sample periods exactly as the parser measures them, and the boundary rate the CSV splitter judges is simply 1/period.

6. NIT

FILE_GAP_TOLERANCE_MULTIPLIER is now a static nested class.

Design decisions

• Where the accumulator lives. A protected static HashMap<SENSORS, List<Double>> in UtilCsvSplitting, right next to SAMPLING_RATE_LIMITS_PER_SENSOR, keyed the same way. clearMapOfSamplingRateLimitsPerSensor() clears both, so measurements cannot leak across a CSV-set boundary (that method is the parser's per-CSV-set reset and has no in-repo caller — it is invoked from ASM_PC, which I could not inspect from here; the contract is kept and is asserted by a test). Keying by SENSORS rather than DATABLOCK_SENSOR_ID keeps the two maps' lifecycles literally identical; every sensor class key of a data block is fed the same measurements and therefore returns the same median.
• Bounded history. SLOW_SENSOR_PERIOD_HISTORY_MAX = 1024, oldest dropped first. Unbounded growth over a multi-day recording is avoided, and a sliding window (rather than a hard stop after N) means the median still follows genuine long-term drift in the sensor's cadence while remaining ~500 payloads deep — far too deep for individual dropped blocks to shift.
• The unconditional put is preserved. DEV-793 Widen + always apply the slow-sensor CSV gap window #285's fix for the original "guard skips forever" bug still stands: populateExpectedPayloadTsDiffLimitMapIfNeeded seeds a configured-rate ±10% band for a sensor that has not been measured yet (e.g. a first payload carrying <2 blocks of it), and the measured window overwrites it as soon as the first measurement exists. A containsKey guard would lock the too-tight header estimate in for the whole file. Two tests pin both directions of this.
• setSamplingRate also uses the accumulated median (review point 3's open question). The block sampling rate is what the block start times — and hence the CSV timestamps — are back-filled with. Using this payload's own median would stretch the sample spacing of exactly the payload that contains a dropped block, i.e. distort the data by the very artefact the window is meant to report. Keeping the timestamps and the continuity check on one estimate also stops them disagreeing about what the achieved cadence is. For the first payload of a recording the accumulated median equals the per-payload median, so nothing changes at the start of a file; later payloads simply get a steadier rate.
• Testability. The window computation is now three pure/near-pure public statics on UtilCsvSplitting (calculateMedian, calculateSlowSensorSamplingRateLimits, accumulateSlowSensorPeriodsAndGetMedianPeriodS) plus refineSlowSensorSamplingRateLimits which accumulates and applies. All additive — no existing signature changed or removed.

Test coverage added (12 cases)

Case	Test
(a) even-count median averages the middles	testMedianAveragesTheTwoMiddleValuesForAnEvenCount, testAccumulatedMedianAveragesTheTwoMiddleValuesForAnEvenCount
(b) healthy jitter (+12.5% spacing) judged continuous	testHealthyJitterStaysInsideTheWindow, testHealthyJitterStaysInsideTheWindowFromTheVeryFirstPayload
(c) dropped block (2x spacing) flagged even though its own payload fed the estimate	testDroppedBlockIsDetectedEvenThoughItsPayloadFedTheEstimate, plus testPerPayloadWindowWouldHaveAbsorbedTheDroppedBlock which pins the old per-payload behaviour as the baseline it must not regress to
(d) measured window engages even when the fallback seeded the map first	testMeasuredWindowWinsOverAFallbackSeededBand, testFallbackSeedingDoesNotClobberAnExistingMeasuredWindow
symmetric limits / fast-side extremum no longer widens the window	testBothLimitsAreDerivedFromTheSameMedian
lifecycle + hygiene	testClearingTheLimitsMapAlsoClearsTheAccumulatedPeriods, testPeriodHistoryIsBounded, testNonPositiveAndMissingMeasurementsAreIgnored

Validation

No Gradle wrapper in the repo, so the environment's Gradle 8.14.3 was used from the ShimmerDriver directory (there is no root settings.gradle):

gradle compileJava test --console=plain
...
BUILD SUCCESSFUL in 31s
4 actionable tasks: 4 executed

Test results from ShimmerDriver/build/test-results/test: 63 tests, 0 failures, 0 errors, 0 skipped, of which 12 are the new API_00009_UtilCsvSplittingSlowSensorGapWindow cases; the pre-existing suites (API_00003, API_00004, API_00006, API_00007, API_00008, API_XXXXX_UtilParseData) all still pass.

Not run here: any end-to-end parse against real DEV-927/SR68 recordings — those live in ASM_PC (ASM_PC_00005_VerisenseFileParserPC) and neither the repository nor the binary fixtures were available in this environment. Re-running that suite before merge is still worthwhile, particularly to confirm the CSV timestamps produced by the setSamplingRate change on multi-payload slow-sensor recordings.

🤖 Generated with Claude Code

https://claude.ai/code/session_014EmM3GD2F6zPqXmWHS94dh

---

Generated by Claude Code

[Copilot]

Pull request overview

This PR improves slow-sensor (VD6283 / MLX90632) CSV gap detection by replacing per-payload window derivation with an accumulated, robust median-based window across a parse run, and adds targeted unit tests to prevent regressions.

Changes:

• Add per-sensor accumulated slow-sensor period history and robust median / symmetric-limit helpers in UtilCsvSplitting.
• Update PayloadContentsDetailsV8orAbove.refineSlowSensorSamplingRateFromBlockTicks to feed per-payload measurements into the accumulator and apply the accumulated median to both timestamps (setSamplingRate) and the gap window.
• Add a new JUnit4 test suite covering median behavior, window geometry, fallback interplay, and lifecycle/limits hygiene.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
ShimmerDriver/src/main/java/com/shimmerresearch/verisense/payloaddesign/UtilCsvSplitting.java	Adds accumulated-median history + window/median helpers and clears accumulated history alongside existing limits map.
ShimmerDriver/src/main/java/com/shimmerresearch/verisense/payloaddesign/PayloadContentsDetailsV8orAbove.java	Switches slow-sensor refinement to use the accumulated median for both sampling-rate limits and per-block sampling rate.
ShimmerDriver/src/test/java/com/shimmerresearch/verisense/payloaddesign/API_00009_UtilCsvSplittingSlowSensorGapWindow.java	Introduces regression tests for the new accumulated-median gap window behavior and related edge cases.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.