Skip to content

fix(performance): round alert values and label cache hit-rate scope - #270

Open
docdyhr wants to merge 4 commits into
mainfrom
fix/performance-output-scope
Open

docdyhr wants to merge 4 commits into
mainfrom
fix/performance-output-scope

Conversation

@docdyhr

@docdyhr docdyhr commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Pull Request

Description

Performance output polish from the v4.0.9 re-test results:

  • Alert messages printed raw floats: High response time: 2096.285714285714ms. formattedMessage did the same with actualValue.

  • In the same minute the cache hit rate read 6.3% in stats, 2% in alerts and 16.7% for site1. Each figure has a different scope, and nothing said which:

    • stats: every site's cache counters summed since server start;
    • alerts: a recorded alert's message, frozen at the value when the alert was raised;
    • site1: one site's cache only.

    Cache counters accumulate from server start; clearing entries doesn't reset them.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)

Changes Made

  • evaluateAlertConditions(): the response-time message is rounded to whole ms. actualValue keeps the raw number.
  • formatAlertMessage(): actualValue and threshold rounded to 2 decimals (2096.29, 0.02)
  • scope labels:
    • wp_performance_stats → cache ("all sites combined, since the server started") and siteSpecific ("site site1 only, …"); siteSpecific.cache.hitRate is now a percentage string like every other hit rate
    • wp_performance_alerts → data.scope.alerts (recorded history: the value when each alert was raised) and data.scope.activeAlerts (what is breaching now)
    • wp_performance_history → summary.scope (average of N snapshots in the timeframe)
    • wp_cache_stats → cache_stats.scope; wp_cache_info → current_stats.scope ("this site only, since the server started")
  • Review follow-up (994f7c9): wp_performance_benchmark/wp_performance_optimize get metadata.scope; wp_performance_export gets a scope map in its outer metadata (so CSV and summary carry it too); wp_performance_alerts gets scope.anomalies
  • wp_performance_history totalRequests was wrong: it summed cumulative totals across snapshots (100 + 150 + 230 = 480). It now reports the requests between the first and last snapshot (130), and the summary scope says each snapshot holds totals since the server started.
  • src/performance/AGENTS.md: the three scopes, where each is reported, and the no-summing rule; docs/PERFORMANCE_MONITORING.md: a "Scope of the figures" section covering the siteSpecific.cache.hitRate string
  • Related: history was always empty in production (the window compared uptime with epoch times). That is fixed in fix(performance): keep history snapshots by when they were taken #272, which merges cleanly with this PR
  • New tests/performance/OutputScope.test.js (14 tests). In tests/tools/cache.test.js, the two exact-shape assertions now include scope.

Testing

  • All 8 new tests were red before the change
  • Combined run with the other three branches from this pass: 93 files / 2740 tests, lint, typecheck, format:check, security:scan
  • Manual: built server over stdio against the live sites. wp_cache_stats, wp_performance_stats category=cache site=site1 and wp_performance_alerts each returned their scope text.

Notes for reviewers

  • Output-shape changes: one new scope field in each response listed above, and siteSpecific.cache.hitRate changes from a 0–1 number to e.g. "16.7%".

Documentation

  • Documentation updated (src/performance/AGENTS.md)
  • CHANGELOG.md updated — generated by semantic-release

Security

  • No sensitive information committed

Independent review follow-up

  • formattedMessage no longer prints a breach as equal to its threshold. Rates keep 3 significant digits (1/19 → 0.0526 vs 0.05), and more where needed (0.7996 vs 0.8).
  • History labels describe the real window: an unsupported timeframe falls back to 24h, and 7d/30d cover the 24 hours history keeps (describeHistoryWindow()).
  • History trends, export analytics and optimize predictions are labelled as coming from the analytics' 24-hour history. The export scope map lists only the sections it includes. History's scope is now data.scope {summary, trends}.
  • wp_cache_stats/wp_cache_info show hit rates with one decimal, like the performance tools.
  • Merge after fix(performance): keep history snapshots by when they were taken #272: totalRequests (last − first) needs fix(performance): keep history snapshots by when they were taken #272's deep-copied snapshots. On main, snapshots share the live counters, so the difference would always be 0. Verified on the combined build: 3 real snapshots of 100, 150 and 230 requests give 130.

🤖 Generated with Claude Code

Summary by Sourcery

Clarify performance output scopes and improve alert and cache metric formatting while correcting historical request totals.

Bug Fixes:

  • Round response-time and formatted alert values while preserving enough precision to show breaches accurately.
  • Correct history request totals by calculating activity between the first and last snapshots instead of summing cumulative counters.

Enhancements:

  • Add explicit scope metadata across performance, cache, alert, history, benchmark, optimization, and export outputs to clarify whether figures cover all sites, one site, recorded alerts, or analytics history.
  • Standardize cache hit-rate formatting to one decimal place and expose per-site hit rates as percentage strings.
  • Document metric scopes, history retention, timeframe behavior, and cumulative-counter interpretation.

Documentation:

  • Document the scope of performance and cache figures, including history windows and hit-rate formatting.

Tests:

  • Add coverage for alert rounding, scope labels, cache formatting, export metadata, history windows, and cumulative request calculations.

Alert messages printed the raw average ("High response time:
2096.285714285714ms") and formattedMessage the raw actualValue; both are
rounded now, and actualValue keeps the exact figure.

The cache hit rate read differently across tools in the same minute
(stats 6.3%, alerts 2%, site1 16.7%) because each has its own scope: all
sites combined since server start, the value when an alert was raised,
or a single site. Nothing said which. wp_performance_stats (cache,
siteSpecific), wp_performance_alerts, wp_performance_history,
wp_cache_stats and wp_cache_info now carry a scope label, and the
siteSpecific hit rate is a percentage like everywhere else.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 9, 2026 09:50
@sourcery-ai

sourcery-ai Bot commented Oct 9, 2026

Copy link
Copy Markdown

Sorry @docdyhr, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 5 days and 6 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T09:53:45.547264Z e6bea5a PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@sourcery-ai

sourcery-ai Bot commented Oct 9, 2026

Copy link
Copy Markdown

Reviewer's Guide

Polishes performance alert numbers and makes cache hit-rate scope explicit across aggregate, per-site, alert, historical, and cache-tool responses, with documentation and regression coverage for the changed output shapes.

Sequence diagram for scoped performance output

sequenceDiagram
    participant Caller
    participant PerformanceTools
    participant CacheTools
    participant AlertTools
    participant Metrics

    Caller->>PerformanceTools: getPerformanceStats()
    PerformanceTools->>Metrics: collect aggregate metrics
    Metrics-->>PerformanceTools: all-site cache hit rate
    PerformanceTools-->>Caller: cache with all-sites scope

    Caller->>PerformanceTools: getPerformanceHistory()
    PerformanceTools->>Metrics: read snapshots
    Metrics-->>PerformanceTools: timeframe snapshots
    PerformanceTools-->>Caller: summary with average scope

    Caller->>CacheTools: getCacheStats()
    CacheTools->>Metrics: read site cache counters
    Metrics-->>CacheTools: one-site hit rate
    CacheTools-->>Caller: cache_stats with site scope

    Caller->>AlertTools: getPerformanceAlerts()
    AlertTools-->>Caller: recorded alerts and activeAlerts scopes
Loading

File-Level Changes

Change Details Files
Rounds alert output for readability while preserving raw values for programmatic use.
  • Rounds response-time condition messages to whole milliseconds.
  • Rounds formatted actual values and thresholds to two decimals without changing stored actualValue.
src/performance/PerformanceMonitor.ts
src/tools/performance/PerformanceHelpers.ts
tests/performance/OutputScope.test.js
Adds explicit scope metadata to cache and performance outputs so differing hit-rate populations and time windows are distinguishable.
  • Labels aggregate, per-site, recorded-alert, active-alert, historical, and cache-tool scopes.
  • Formats per-site cache hit rates as percentage strings consistently with aggregate output.
  • Documents server-start counter behavior and the requirement for scope labels.
src/tools/performance/PerformanceTools.ts
src/tools/cache.ts
src/performance/AGENTS.md
tests/tools/cache.test.js
tests/performance/OutputScope.test.js
Adds regression coverage for numeric formatting and every updated output scope.
  • Verifies rounded alert messages and preservation of raw values.
  • Checks scope text and hit-rate formatting across performance and cache tools.
  • Updates exact cache response-shape expectations for the new scope fields.
tests/performance/OutputScope.test.js
tests/tools/cache.test.js

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e6bea5ade7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/tools/performance/PerformanceTools.ts
Comment thread src/tools/performance/PerformanceTools.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Some hit-rate outputs remain unlabeled, and history scope conflates the selection window with session-cumulative metrics.

2 open findings
What changed in this PR

Polishes cache/performance output by clarifying metric scope and rounding alert values.

Changes:

  • Adds scope labels to cache, performance, history, and alert responses.
  • Rounds displayed alert values while preserving raw values.
  • Adds regression tests and performance documentation.
File Description
src/​performance/​AGENTS.md Documents scope and rounding contracts.
src/​performance/​PerformanceMonitor.ts Rounds response-time alert messages.
src/​tools/​cache.ts Labels site-specific cache statistics.
src/​tools/​performance/​PerformanceHelpers.ts Rounds formatted alert values.
src/​tools/​performance/​PerformanceTools.ts Adds scope labels and formats per-site rates.
tests/​performance/​OutputScope.test.js Tests scope labels and rounding.
tests/​tools/​cache.test.js Updates cache response assertions.

🧠 Review effort: Balanced


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

Comment thread src/performance/AGENTS.md Outdated
Comment thread src/tools/performance/PerformanceTools.ts Outdated
…tals

Review follow-up. wp_performance_benchmark, wp_performance_optimize and
wp_performance_export (all three formats, via the outer metadata) now
carry a scope too, and wp_performance_alerts labels its anomalies.

wp_performance_history summed requests.total over its snapshots, but
every snapshot holds a total since server start, so three snapshots of
100, 150 and 230 reported 480 requests. It now reports the 130 made
between the first and last snapshot, and the summary scope says the
averages are over cumulative figures.

docs/PERFORMANCE_MONITORING.md documents the scopes and the
siteSpecific.cache.hitRate percentage string.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the docs Documentation changes label Oct 9, 2026
docdyhr and others added 2 commits October 9, 2026 13:37
…window

Review follow-up:
- formattedMessage rounded rates to 2 decimals, so an error rate of
  1/19 printed as "0.05 vs threshold: 0.05". Values below 1 keep 3
  significant digits, and more if they would still equal the threshold.
- History labels repeated any timeframe, but an unknown one falls back
  to 24h and 7d/30d still cover only the 24 hours history is kept for;
  describeHistoryWindow() says what the window really is.
- History trends, export analytics and optimize predictions come from
  the analytics' own 24-hour history and are now labelled as such; the
  export scope map lists only the sections it includes. History's scope
  moves to data.scope {summary, trends}, like the alerts tool.
- wp_cache_stats / wp_cache_info show hit rates with one decimal, like
  the performance tools ("16.7%", not "17%").

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants