Code Review: PR #25 — Wave A (Load Monitor + Priority Registry)

[mrtwebdesign]

Code Review for PR #25 (Wave A)

Reviewer: Claude Code (2026-05-07)
Scope: class-hcqg-load-monitor.php, class-hcqg-priority-registry.php, tests/bootstrap.php, composer.json, phpunit.xml.dist, and associated tests

Fix Now

• H1. APCu key collision (real bug) — class-hcqg-load-monitor.php:117 — apcu_fetch('throttle_state') uses no namespace prefix. On shared hosting with shared APCu memory, multiple WordPress sites read/write each other's throttle state. Fix: prefix with self::CACHE_GROUP . ':'. (Fixed in e4dc882)

Fix Before Production

• M6. get_thresholds() allows filter to inject unknown keys — class-hcqg-load-monitor.php:71 — array_merge(DEFAULT_THRESHOLDS, $thresholds) permits uncontrolled array growth. Fix: array_merge(self::DEFAULT_THRESHOLDS, array_intersect_key($thresholds, self::DEFAULT_THRESHOLDS)). (Fixed in 146e0ed)
• M7. Composer type mismatch — composer.json:4 — Type is wordpress-muplugin but plugin lives in wp-content/plugins/ and composer/installers is not required. Change to wordpress-plugin or document the intended deployment target. (Fixed in 146e0ed)

Fix Later (Test Infrastructure)

• H5. Zero test coverage for collect_metrics() / probe_* / run_mysqli_query() — The entire MySQL metrics subsystem that drives throttle decisions is untested. Add integration tests or at minimum test collect_metrics() with a stubbed $wpdb->dbh.
• M10. Zero test coverage for APCu backend paths — Combined with H1 (APCu collision bug), this gap is significant. Add APCu function stubs and test the APCu code path.
• H7. wp_cache_set stub drops TTL parameter — tests/bootstrap.php:163 — Production code passes TTL as 4th arg; stub silently ignores it. Add $expire = 0 to the stub signature.
• L9. add_filter/add_action stubs ignore priority and accepted_args — tests/bootstrap.php:127,178 — Add $priority = 10, $accepted_args = 1 parameters for forward compatibility.
• L11. add_option and update_option stubs have incomplete signatures — tests/bootstrap.php:140,149 — Add missing $deprecated, $autoload parameters.

Noted (No Action Required)

• L6. get_registry() rebuilds on every get_priority() call (no memoization). Minor perf concern; add static cache if it becomes measurable.
• L7. time() in evaluate_level makes it non-deterministic. An optional $now parameter would improve testability.
• L8. phpunit.xml.dist uses deprecated PHPUnit 9 attributes. Fine for now (pinned to ^9.6), remove when upgrading to PHPUnit 10.
• L12. Missing test edge cases: regex metacharacters in priority patterns, empty string hook name, catch-all * pattern, evaluate_level with previous=critical + no signals, add_option vs update_option branching.