Skip to content

fix(log-viewer): show governor usage only where the log measured it - #1071

Merged
lcottercertinia merged 3 commits into
certinia:mainfrom
lukecotter:bug-governor-trend-steps
Sep 18, 2026
Merged

lcottercertinia merged 3 commits into
certinia:mainfrom
lukecotter:bug-governor-trend-steps

Conversation

@lukecotter

Copy link
Copy Markdown
Collaborator

📝 PR Overview

The Inspector's Governor usage over time charts and the Timeline's governor strip read the same series, but drew it differently. The charts ran a straight line between two readings, which puts usage at instants the log never measured — plainest on CPU, whose only readings are the LIMIT_USAGE_FOR_NS and LIMIT_USAGE lines, so the invented climb between them spans most of the chart.

Each chart now steps like the strip: a reading stands until the next one. Two smaller disagreements go with it — the charts measured x against duration.total while the Timeline runs 0..exitStamp, so every reading sat right of where the Timeline drew it and a click seeked short of the instant picked; and the spans the log recorded nothing in, blanked on the strip in 1.22.0, were still filled straight across here.

🛠️ Changes made

  • Step the trend line, and hold the hover readout to the reading before the cursor — no value is reported for an instant the log did not measure
  • Measure x by exitStamp, the Timeline's own domain, so a reading sits where the Timeline draws it and click-to-seek lands on it
  • Carry gaps on the shared series rather than adding them in the Timeline, so the charts leave unrecorded spans unfilled; the line still carries across one, because a governor total cannot fall
  • Share recordedSegmentEnd between the strip and the charts, so the gap rule has one statement
  • Memoise the marker extraction per log, which the series and the Timeline now both ask for
  • Reframe inspector.png: the Analysis tab with a row selected, cropped to the panel, so the shot shows the Inspector on its own instead of repeating the Database tab

🧩 Type of change (check all applicable)

  • 🐛 Bug fix - something not working as expected
  • ✨ New feature – adds new functionality
  • ♻️ Refactor - internal changes with no user impact
  • ⚡ Performance Improvement
  • 📝 Documentation - README or documentation site changes
  • 🔧 Chore - dev tooling, CI, config
  • 💥 Breaking change

📷 Screenshots / gifs / video [optional]

The 1.22 screenshot set is refreshed in the second commit. The chart change itself reads best live: open a log, Timeline tab, nothing selected, and compare the CPU chart against the strip below the flame chart.

🔗 Related Issues

N/A

✅ Tests added?

  • 👍 yes

pnpm lint and pnpm exec jest --selectProjects log-viewer (2092 tests). New coverage: the step path, one fill shape per recorded stretch against an unbroken line, and the series carrying the log's gaps. The GovernorTrends fixture now sets exitStamp apart from duration.total, so reading the wrong one fails.

📚 Docs updated?

  • 🔖 README.md
  • 🔖 CHANGELOG.md
  • 📖 help site
  • 🧪 Marked any pre-release-only features
  • 🙅 not needed

Anything else we need to know? [optional]

Two commits: the fix, then the screenshot work. Start at governorTrendData.ts (pointAt) and GovernorTrends.ts (trendPaths) — the rest follows from those.

The charts drew a straight line between readings, so usage read at instants
the log never measured. Each chart now holds a reading until the next one,
runs on the Timeline's own timescale, and leaves the spans the log recorded
nothing in unfilled - matching the governor limits strip it shares its
figures with.
Capture the inspector on its own: the Analysis tab with a row selected, so
Findings names it, which no other tab's panel can do. An area drag round the
panel, not the whole window, so the shot no longer repeats the Database tab.
Crop to the panel on the Analysis tab, reading the whole log. Findings and
Self time spread are that tab's own sections, and no whole-window shot shows
them large enough to read.
@lcottercertinia
lcottercertinia merged commit 5471f8d into certinia:main Sep 18, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants