fix(thold): make the unit suffix conversions inverses of each other

[somethingwithproof]

Fixes #786.

thold_display_to_raw() and thold_raw_to_display() move a threshold bound between what an operator types and what is stored, so they have to be inverses. They each carried their own suffix table and disagreed.

The failure that matters

Entering p scaled by 1e-9, but a stored value of 1e-12 rendered as 5f, which parsed back as 1e-15. A threshold is read out of the database, rendered into the form, and written back on every save — so opening a threshold and saving it without changing anything divided its bound by a thousand, and did so again on every visit.

Nano was absent from both, which is what put the rest of the small suffixes one place out of step:

stored	rendered before	re-parsed to	rendered now
1e-9	5p	1e-9	5n
1e-12	5f	1e-15	5p
1e-15	5	—	5f
1e-18	5	—	5a

Past the ends of the table the render indexed off the end of its pattern string, so the suffix came back empty and the magnitude was silently dropped — 5e-18 displayed as plain 5.

The change

Both functions now read one shared table, thold_unit_suffixes(), so they cannot drift apart again. The render walks it for the largest scale that still leaves a value of one or more, with the empty suffix standing for a scale of one, and a value beyond the table keeps its scale instead of losing it (5e27 renders 5000Y).

Verified across 45 orders of magnitude:

5.0e-18 -> 5a    -> 5.0e-18   round-trips
5.0e-15 -> 5f    -> 5.0e-15   round-trips
5.0e-12 -> 5p    -> 5.0e-12   round-trips
5.0e-9  -> 5n    -> 5.0e-9    round-trips
0.005   -> 5m    -> 0.005     round-trips
5       -> 5     -> 5         round-trips
5000    -> 5K    -> 5000      round-trips
5.0e+24 -> 5Y    -> 5.0e+24   round-trips
5.0e+27 -> 5000Y -> 5.0e+27   round-trips

What operators will notice

No migration is needed and no stored value is rewritten. Two things change:

• A bound stored at 1e-9 now displays as 5n where it used to say 5p. Same number, correct label.
• Typing p now means pico. Anyone who had learned that thold's p meant nano will get a thousand times less than before, which is the correction — but it is a change in what an existing habit produces, so it is worth a release note.

The affected fields are the ten in $thold_units_convert_array: the hi/low bounds, their warning and time-based counterparts, and the baseline percentages.

Tests

48 tests, 100% of the changed lines covered. The round-trip property is checked for every suffix rather than spot-checked, which is the assertion that would have caught this originally.

The harness commit matches the other open pull requests, so whichever lands first the rest merge cleanly.

[Copilot]

Pull request overview

This PR fixes a correctness issue in thold’s unit-suffix parsing/formatting by ensuring thold_display_to_raw() and thold_raw_to_display() use the same SI suffix table, so values round-trip without silently changing magnitude when a threshold is opened and saved.

Changes:

• Introduces a shared SI suffix map (thold_unit_suffixes()) and refactors both conversion functions to use it.
• Adds a PHPUnit unit-test harness validating SI scaling and round-trip behavior, plus regression coverage for out-of-range magnitudes.
• Adds a Docker-based CI job (lint + coverage) including a “changed-lines coverage” gate.

Reviewed changes

Copilot reviewed 12 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
thold_functions.php	Adds shared SI suffix table and refactors raw/display conversions to be inverse-safe.
tests/Unit/TholdUnitSuffixTest.php	Adds unit tests for suffix scaling, rendering, and round-tripping behavior.
tests/TestCase.php	Adds a shared base TestCase for plugin tests with common reset/setup behavior.
tests/Helpers/CactiStubs.php	Provides programmable stubs for Cacti core functions used by thold.
tests/fixtures/optional-core-functions.php	Supplies optional Cacti functions for tests that need to exercise function_exists() branches.
tests/fixtures/cacti-lib/variables.php	Fixture file to satisfy runtime include_once() paths during tests.
tests/docker/Dockerfile	Defines a pinned PHP 8.1 Docker image for running the unit tests + coverage.
tests/docker/docker-compose.yml	Provides a local docker compose runner matching CI behavior.
tests/bootstrap-unit.php	Test bootstrap wiring Cacti globals/functions to stubs and loading plugin code safely.
tests/bin/patch-coverage.php	Implements changed-lines coverage reporting and enforcement against a base ref.
phpunit.xml	Configures PHPUnit, bootstrap, strictness, and coverage source selection.
composer.json	Adds dev dependencies and scripts for linting/tests/coverage + docker test runner.
.gitignore	Ignores vendor, coverage, and PHPUnit cache artifacts.
.github/workflows/php-unit-tests.yml	Adds a GitHub Actions workflow to run Docker-based lint + unit tests + coverage gates.

Suppressed comments (1)

tests/Unit/TholdUnitSuffixTest.php:172

• testMagnitudesBelowTheSmallestSuffixKeepTheirScale() uses 5e-18, which is within the suffix table (atto), not below the smallest supported suffix (yocto, 1e-24). Using a value below 1e-24 exercises the intended edge case (no suffix available) and ensures scale is preserved rather than silently dropped.

	public function testMagnitudesBelowTheSmallestSuffixKeepTheirScale(): void {
		$rendered = thold_raw_to_display(5.0e-18);

		$this->assertNotSame('5', $rendered);
		$this->assertEqualsWithDelta(5.0e-18, (float) thold_display_to_raw($rendered, 'thold_hi'), 5.0e-27);
	}

---

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