Skip to content

fix(performance): keep history snapshots by when they were taken - #272

Merged
docdyhr merged 7 commits into
mainfrom
fix/performance-history-window
Oct 10, 2026
Merged

docdyhr merged 7 commits into
mainfrom
fix/performance-history-window

Conversation

@docdyhr

@docdyhr docdyhr commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Pull Request

Description

Performance history was always empty in production. I found this while checking a Copilot comment on #270 about how the history summary is labelled.

Snapshots were selected and pruned by system.uptime as if it were a timestamp. But uptime is a duration since start (about 3.6e6 ms after an hour), while the cut-offs are epoch times (about 1.8e12):

  • PerformanceMonitor.getHistoricalData(start) kept uptime >= Date.now() - timeframe, which is never true, so it always returned [];
  • cleanupOldData() kept uptime > Date.now() - retention, which is never true, so every collection tick deleted all history, including the snapshot it had just stored;
  • PerformanceAnalytics.addDataPoint() pruned the same way, so analyzeTrends() (needs 5 points), anomaly detection and predictions never had data.

Result: wp_performance_history always returned dataPoints: 0, the export's historicalData was empty, and trends were never computed.

Snapshots were also shallow copies. recordRequest() updates metrics.requests in place, so every stored snapshot shared the live request counters.

Type of Change

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

Changes Made

  • PerformanceMetrics.timestamp (epoch ms). getMetrics() returns a deep copy (structuredClone) stamped with the time it was read.
  • getHistoricalData(), cleanupOldData() and PerformanceAnalytics.addDataPoint() select and prune by timestamp
  • processHistoricalDataForChart() uses timestamp for each chart point (it used uptime ms)
  • The collection timer's work is now recordSnapshot(), so it can be tested
  • src/performance/AGENTS.md: select and prune history by timestamp, never by uptime
  • New tests/performance/HistoryWindow.test.js (6 tests): timestamp, snapshot independence, time-window selection, 24h retention, chart timestamps, and trend analysis now getting its samples
  • tests/performance/PerformanceAnalytics.test.js: the fixtures set system.uptime: Date.now() (an epoch value posing as uptime), which is why they survived the broken filter. They now carry timestamp, and the lookback test asserts what is kept instead of expect(true).toBe(true).

Testing

  • The 6 new tests were red before the change (expected 2 to be +0 for the shared counters; analyzeTrends() returned [])
  • Full npm test: 90 files / 2717 tests; lint, typecheck, prettier
  • Manual: ran the built server over stdio in production mode in a sandbox (dummy single-site, no site contacted). After 68 s, wp_performance_history timeframe=1h returned 2 data points with epoch timestamps; before the fix it returned 0.
  • A dry-run merge with fix(performance): round alert values and label cache hit-rate scope #270, which also touches src/performance/ and PerformanceHelpers.ts, is conflict-free, and the combined performance tests pass (247)

Notes for reviewers

  • Output changes: metrics readings (e.g. the export's currentMetrics) gain a timestamp field; history chart points carry epoch ms instead of uptime ms.
  • Trend/anomaly analysis will start producing results in long-running sessions, since it now gets data.

Documentation

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

Security

  • No sensitive information committed

Independent review follow-up

Turning history on made dead code live; three paths misbehaved on real data and are fixed here:

  • Size: export and history sample at most 100 points (downsample(), newest kept) and report the full count. A day used to export 2880 snapshots (5.7–12.9 MB). Stored snapshots drop the per-tool maps (PerformanceSnapshot).
  • False anomalies: a point is judged against the points before it (it was part of its own baseline), the spread has a per-metric noise floor (320→321 ms scored as an anomaly), a zero mean gives a finite deviation, and anomalies expire after the 24h lookback.
  • NaN/Infinity: flat series and zero previous values are guarded ("declining at Infinity% rate").
  • 10 more tests in tests/performance/HistoryWindow.test.js. Merge before fix(performance): round alert values and label cache hit-rate scope #270.

🤖 Generated with Claude Code

Summary by Sourcery

Fix performance history and analytics so collected snapshots remain available, accurate, bounded, and actionable in long-running sessions.

Bug Fixes:

  • Restore performance history collection, time-window filtering, retention, charting, trend analysis, and anomaly detection by tracking snapshots with epoch timestamps instead of uptime durations.
  • Prevent historical snapshots from sharing mutable live metrics and reduce stored and exported history payloads through compact snapshots and bounded sampling.
  • Correct anomaly and trend reporting to ignore improvements and inactive caches, suppress noise, expire stale anomalies, and avoid non-finite results.

Enhancements:

  • Expose the number of full historical data points alongside sampled history output.
  • Make snapshot recording independently testable and document timestamp-based history handling.

Documentation:

  • Document that anomaly detection only reports changes in the worse direction and ignores caches without lookups.

Tests:

  • Add comprehensive coverage for timestamped snapshots, retention and lookback windows, chart and export sampling, trend analysis, and anomaly behavior.

docdyhr and others added 2 commits October 9, 2026 12:15
Performance history was always empty in production. Snapshots were
selected and pruned by system.uptime as if it were a timestamp, but
uptime is a duration since start (~3.6e6 ms after an hour) and the
cut-offs are epoch times (~1.8e12). getHistoricalData(start) matched
nothing, and cleanupOldData() and PerformanceAnalytics.addDataPoint()
deleted every snapshot as soon as it was stored, so wp_performance_history
had no data points and trend/anomaly analysis never got its 5 samples.
Snapshots were also shallow copies that shared the live request counters.

getMetrics() now returns a deep copy stamped with `timestamp` (epoch ms),
and history is selected, pruned and charted by it. The timer's work is
recordSnapshot(), so it can be tested.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keeps it apart from the scope bullet the cache hit-rate PR edits.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 9, 2026 10:20
@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-09T10:22:09.450642Z b797c41 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

Fix performance history by stamping deep-copied metric snapshots with epoch timestamps and using those timestamps consistently for selection, retention, analytics lookback pruning, and chart output; add focused regression tests and documentation.

Sequence diagram for timestamped performance history collection

sequenceDiagram
    participant Timer
    participant Monitor as PerformanceMonitor
    participant Metrics as LiveMetrics
    participant History as HistoricalData

    Timer->>Monitor: recordSnapshot()
    Monitor->>Metrics: getMetrics()
    Metrics-->>Monitor: structuredClone(metrics, timestamp: Date.now())
    Monitor->>History: push(snapshot)
    Monitor->>History: cleanupOldData()
    History-->>Monitor: retain snapshots where timestamp > cutoff
Loading

Flow diagram for performance history time-window processing

flowchart LR
    A[Metric snapshot] --> B[Epoch timestamp]
    B --> C[getHistoricalData]
    B --> D[Analytics lookback pruning]
    B --> E[Chart point timestamp]
    U[system.uptime duration] -. not used for history filtering .-> C
    U -. not used for retention .-> D
Loading

File-Level Changes

Change Details Files
Make performance history use immutable, epoch-timestamped snapshots.
  • Add a timestamp field to metrics readings.
  • Return deep-cloned snapshots so later in-place metric updates cannot mutate history.
  • Expose snapshot recording as a testable recordSnapshot() operation used by the collection timer.
src/performance/PerformanceMonitor.ts
Correct history selection, retention, analytics pruning, and chart time values.
  • Filter monitor history and retention windows by timestamp instead of duration-based system.uptime.
  • Prune analytics data using epoch timestamps so trend, anomaly, and prediction inputs survive the configured lookback period.
  • Emit epoch timestamps for chart points.
src/performance/PerformanceMonitor.ts
src/performance/PerformanceAnalytics.ts
src/tools/performance/PerformanceHelpers.ts
Add guidance and regression coverage for timestamp-based history behavior.
  • Document that history must be selected and pruned by timestamp, never system.uptime.
  • Test timestamp stamping, snapshot independence, time-window selection, retention, chart output, and trend analysis.
  • Update analytics fixtures and assertions to use timestamps and verify old points are removed.
src/performance/AGENTS.md
tests/performance/HistoryWindow.test.js
tests/performance/PerformanceAnalytics.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

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.

🟢 Approval recommended

The implementation consistently corrects timestamp handling and snapshot isolation with focused regression coverage.

0 open findings

What changed in this PR

Fixes performance history retention by using epoch timestamps and immutable snapshots.

Changes:

  • Adds timestamped, deep-cloned metric snapshots.
  • Uses timestamps for retention, filtering, analytics, and charts.
  • Adds regression tests and documents the history contract.
File Description
src/​performance/​PerformanceMonitor.ts Records and retains timestamped snapshots.
src/​performance/​PerformanceAnalytics.ts Prunes analytics data by timestamp.
src/​tools/​performance/​PerformanceHelpers.ts Charts points using snapshot timestamps.
src/​performance/​AGENTS.md Documents history semantics.
tests/​performance/​HistoryWindow.test.js Covers history regressions.
tests/​performance/​PerformanceAnalytics.test.js Corrects timestamp-based fixtures and assertions.

🧠 Review effort: Balanced


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

docdyhr and others added 4 commits October 9, 2026 13:32
Review follow-up. With history collected, code paths that never ran
before became live, and three misbehaved on real data:

- Size: wp_performance_export returned every snapshot (a day is 2880,
  measured 5.7-12.9 MB) and wp_performance_history charted all of them.
  Both now carry an even sample of at most 100 points (first and newest
  kept) plus the full count; stored snapshots drop the per-tool maps
  (PerformanceSnapshot), which nothing reading history uses.
- False anomalies: each point was part of its own baseline, and a flat
  baseline has zero spread, so 320 -> 321 ms scored as an anomaly; they
  never expired and pushed wp_performance_alerts to "Performance
  Warnings" on a healthy server. A point is now judged against the
  points before it, the spread has a per-metric noise floor, a zero
  mean gives a finite deviation, and anomalies expire after the
  lookback period.
- NaN/Infinity: a flat series made R-squared 0/0 and a zero previous
  value made the change rate Infinity ("declining at Infinity% rate").
  Both are guarded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Anomaly z-scores were absolute values, so an improvement (response time
2000 -> 1000 ms, a cache warming up from no lookups) scored as a "spike"
or "drop" -- critical at z > 4 -- and wp_performance_alerts reported
"Critical Issues Detected" for the 24 h lookback. This only became
reachable now that history is no longer empty.

- Only a move in the worse direction counts: response time and error
  rate up, hit rate down.
- A cache with no lookups is neutral: such points are neither judged
  nor part of the hit-rate baseline (idle zeros also hid real drops).
- Prune to the lookback period before judging a new point, so it is
  never compared with readings older than 24 h.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the docs Documentation changes label Oct 10, 2026
…ry-window

# Conflicts:
#	src/tools/performance/PerformanceHelpers.ts
@docdyhr
docdyhr merged commit bcdbb2a into main Oct 10, 2026
24 checks passed
@docdyhr
docdyhr deleted the fix/performance-history-window branch October 10, 2026 23:12
docdyhr pushed a commit that referenced this pull request Oct 10, 2026
## [4.1.4](v4.1.3...v4.1.4) (2026-10-10)

### 🐛 Bug Fixes

* **performance:** keep history snapshots by when they were taken ([#272](#272)) ([bcdbb2a](bcdbb2a))
@docdyhr

docdyhr commented Oct 10, 2026

Copy link
Copy Markdown
Owner Author

🎉 This PR is included in version 4.1.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

docdyhr added a commit that referenced this pull request Oct 12, 2026
… data (#274)

* fix(performance): fit trends against time and treat idle caches as no data

Follow-ups from the review of #270/#272, now that history is live:

- Trends regressed over the sample index, but snapshots are unevenly
  spaced (the analytics timer and every wp_performance_history call add
  one), and "one sample ahead" was labelled a 30-minute prediction;
  predictPerformance(timeframeMinutes) ignored its horizon. Trends now
  fit against timestamps and predict prediction.timeframe past the
  newest point; a direction needs a fitted change above the metric's
  noise floor (the fixed 0.1 slope meant ms for one metric and 10
  percentage points for another).
- A cache with no lookups is neutral, not 0%: idle points are left out
  of the hit-rate trend and of history's averageCacheHitRate (null if
  none had lookups), and hit-rate strings read "n/a (no lookups yet)"
  in wp_cache_stats, wp_cache_info and wp_performance_stats.
- History windows are inclusive at both ends: pruning used ">" while
  getHistoricalData(start) used ">=".
- History/export scope labels say the chart series are sampled to at
  most 100 points; downsample(items, 0) returns no items; the supported
  timeframes come from the parseTimeframe map instead of a second list.

Tests: 18 new; suite 2778 passed before, 2796 after, 0 failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(performance): no cache efficiency before the first lookup

Review follow-up (Codex on #274): wp_performance_stats showed the hit
rate as "n/a (no lookups yet)" next to efficiency "Poor", because
calculateCacheEfficiency() scored the undefined 0% hit rate.
calculateCacheEfficiency() now returns "n/a (no lookups yet)" when the
cache has had no lookups, like formatHitRate().

Tests: 1 new (failed before the fix); suite 2797 passed, 0 failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
docdyhr pushed a commit that referenced this pull request Oct 12, 2026
## [4.1.6](v4.1.5...v4.1.6) (2026-10-12)

### 🐛 Bug Fixes

* **performance:** fit trends against time and treat idle caches as no data ([#274](#274)) ([2996b29](2996b29)), closes [270/#272](https://github.com/270/mcp-wordpress/issues/272)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation changes released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants