fix(thold): correct counter delta, wrap modulus and percent denominator

[somethingwithproof]

Fixes #785.

Four defects in how a raw sample becomes the rate a threshold is compared against. Tests were written first and fail on develop.

A previous counter reading of zero was treated as no reading

if ($thold_data['oldvalue'] != 0 && is_numeric($thold_data['oldvalue'])) {

Zero is a legitimate previous reading. After a device reboot, or any interval where a counter genuinely sits at zero, the next poll reported a rate of zero — so a thold_low on interface utilisation fired spuriously and a thold_high under-reported the first interval of traffic.

The wrap modulus was off by one, and the 64-bit case lost precision

if ($thold_data['oldvalue'] > 4294967295) {
    $currentval = (18446744073709551615 - $thold_data['oldvalue']) + $item[...];
} else {
    $currentval = (4294967295 - $thold_data['oldvalue']) + $item[...];
}

A counter that wraps has advanced by (2^32 - old) + new. Using 2^32 - 1 loses exactly one count per wrap.

Worse, 18446744073709551615 is above PHP_INT_MAX and is parsed as a float:

$v = 18446744073709551615;  =>  1.8446744073709552E+19 (double)

which drops roughly eleven bits before the subtraction. The 64-bit case now goes through GMP, which Cacti already requires via ext-gmp. Extracted to thold_counter_wrap_delta() so it can be tested directly.

The percent denominator was cast to int

$t = (int) $rrd_reindexed[$thold['local_data_id']][$thold['percent_ds']];

A denominator below one truncated to zero, forcing the percentage to zero, so a percent threshold on a ratio with a sub-unit denominator reported 0% permanently and kept any configured low threshold in breach. A negative denominator gave zero rather than a negative percentage.

The daemon wrote a timestamp into oldvalue

if (isset($item[$thold_data['name']])) {
    $lasttime = $item[$thold_data['name']];              // a value
} else {
    $lasttime = $currenttime - $thold_data['rrd_step'];  // a timestamp
}
...
SET ... oldvalue = ?   [$lasttime]

When a data source produced no sample, a Unix timestamp was stored as the previous counter reading. The next poll computed a large negative delta, took the overflow branch above, and fabricated a rate in the billions — a bogus high alert on the poll after any missed sample.

includes/polling.php already carries the previous oldvalue forward and even documents why; this makes the daemon match.

Tests

17 tests, 100% of the changed lines covered, including exact 32-bit and 64-bit wrap arithmetic.

The harness commit matches #773, #788 and #790, with gmp added to the test image so the 64-bit case is verified rather than assumed. Whichever PR lands first, the others merge cleanly.

[somethingwithproof]

The four Integration Test failures are the upstream break, not this PR. The workflow checks out Cacti/cacti unpinned and Install Cacti via CLI dies in core:

PHP Fatal error: Uncaught Error: Call to undefined function __()
  in cacti/lib/functions.php:7973

format_cacti_version_text() calls __() before the translation layer loads. Nothing here touches Cacti core, and the PHPUnit job passes. #776 pins the checkout to release/1.2.31 and is green.

[Copilot]

Pull request overview

Fixes incorrect value acquisition for thold thresholds by correcting counter delta handling (including wraps), preventing percent calculations from truncating denominators, and ensuring daemon processing persists the correct previous raw reading (oldvalue). This fits into thold’s core evaluation pipeline (thold_get_currentval() / daemon thold_process.php) and adds a PHPUnit-based harness to lock the behavior down.

Changes:

• Correct COUNTER delta handling: treat previous reading 0 as valid; fix wrap modulus; avoid 64-bit precision loss by using GMP via thold_counter_wrap_delta().
• Fix percent-of calculations by no longer casting the denominator to int and handling non-numeric/zero denominators safely.
• Prevent daemon mode from writing timestamps into oldvalue, and add a PHPUnit test harness + CI workflow with changed-lines coverage enforcement.

Reviewed changes

Copilot reviewed 14 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
thold_process.php	Ensures daemon persists the correct raw previous reading into oldvalue instead of timestamps.
thold_functions.php	Fixes counter delta semantics (0 is valid), wrap modulus, and percent denominator handling; adds thold_counter_wrap_delta().
tests/Unit/TholdGetCurrentvalTest.php	Unit coverage for COUNTER wrap/zero handling and other data source types’ current-value semantics.
tests/Unit/TholdCalculatePercentTest.php	Unit coverage for percent denominator behavior (fractional/negative/zero/non-numeric).
tests/TestCase.php	Base test case providing per-test reset and plugin source loading helper.
tests/Support/CactiStub.php	Recording stub for Cacti framework/global functions used by plugin code.
tests/fixtures/optional-core-functions.php	Provides optional-core function shims for function-exists fallback testing.
tests/fixtures/cacti-lib/variables.php	Minimal fixture file so runtime include_once() paths resolve during tests.
tests/bootstrap.php	PHPUnit bootstrap stubbing Cacti globals/functions and providing global-scope loader.
tests/bin/patch-coverage.php	CI helper enforcing coverage on changed lines rather than whole-file coverage.
tests/docker/Dockerfile	Reproducible PHP 8.1 test runner image with GMP + pcov.
tests/docker/docker-compose.yml	Local runner mirroring CI’s Docker-based unit-test workflow.
phpunit.xml	Configures test suite and limits coverage scope to thold_functions.php.
composer.json	Adds PHPUnit dev dependency and scripts for local/CI test execution.
.github/workflows/php-unit-tests.yml	Adds CI workflow to run unit tests + lint + changed-line coverage enforcement.
.gitignore	Ignores composer/vendor, coverage artifacts, and phpunit caches.

---

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