Skip to content

fix(ash): sync sampling rate to zoom level; show actual data window (#763)#763

Merged
NikolayS merged 1 commit intomainfrom
fix/763-ash-zoom-window
Mar 28, 2026
Merged

fix(ash): sync sampling rate to zoom level; show actual data window (#763)#763
NikolayS merged 1 commit intomainfrom
fix/763-ash-zoom-window

Conversation

@NikolayS
Copy link
Copy Markdown
Owner

Summary

Two UX fixes for /ash found during demo recording:

Fix 1: Zoom changes sampling rate

Zooming out (→) now increases refresh_interval_secs to match bucket_secs (capped at 60s). Previously zoom only changed how existing 1s samples were aggregated for display — at 5min-bucket zoom you still had 1s live samples, so the ring buffer held only ~10min of real data regardless of zoom level. Now zoom-out = coarser sampling + wider real window.

Fix 2: Window label shows actual data span

Status bar window: now shows actual elapsed data (samples × bucket_secs) rather than ring-buffer capacity. Previously showed 10min from the first second of launch, which was confusing. Now shows 5s, 30s, growing honestly as data accumulates.

Removes now-unused window_label() from AshState.

Tests

  • Added zoom_cycle_syncs_refresh_to_bucket unit test
  • 1865 passing, 0 failed
  • clippy clean, fmt clean

Fixes issue found by Nik during demo recording.

…763)

Two UX fixes for /ash:

1. Zoom out now also increases refresh_interval_secs to match bucket_secs
   (capped at 60s). Previously zoom only changed display aggregation — you
   could be at 5min-bucket zoom with 1s sampling, which made no sense.
   Now zoom out = coarser but wider real data.

2. Status bar window label now shows actual data span (samples × bucket_secs)
   rather than ring-buffer capacity. Previously showed '10min' from the
   first second of launch. Now shows '5s', '30s', etc. growing as data
   accumulates — honest about what you're actually seeing.

Remove now-unused window_label() from AshState.
Add test: zoom_cycle_syncs_refresh_to_bucket.
@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 83.33333% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.60%. Comparing base (cfc9f4e) to head (e527357).

Files with missing lines Patch % Lines
src/ash/renderer.rs 0.00% 6 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #763      +/-   ##
==========================================
+ Coverage   67.52%   67.60%   +0.08%     
==========================================
  Files          52       52              
  Lines       33792    33819      +27     
==========================================
+ Hits        22817    22863      +46     
+ Misses      10975    10956      -19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@NikolayS
Copy link
Copy Markdown
Owner Author

REV — PR #763 (zoom/window UX fixes)

Branch: fix/763-ash-zoom-window | Head: e527357
Files: src/ash/state.rs, src/ash/renderer.rs


Fix 1: zoom_cycle_forward/back now call sync_refresh_to_zoom()

Correctness: ✅

The logic is right. bucket_secs().min(60) means:

  • zoom=1 (1s buckets) → refresh=1s
  • zoom=2 (15s) → refresh=15s
  • zoom=3 (30s) → refresh=30s
  • zoom=4 (60s) → refresh=60s
  • zoom=5 (300s) → refresh=60s (capped)
  • zoom=6 (600s) → refresh=60s (capped)

The 60s cap is a reasonable UX choice — poll at least once per minute so the TUI doesn't feel frozen.

One question worth considering: when the user zooms out to 5min buckets (300s), each raw sample still covers 1s of data, but the display bucket groups 300 of them. Now with refresh=60s, only 1 raw sample is taken every 60 seconds — so the bucket effectively shows 1 active-session sample per minute rather than an average of 300 1-second samples. The label says bucket: 5min but the underlying data is now 1 sample/minute. This is a conscious simplification (the old behavior of 1s sampling at 5min zoom was also wrong), but worth documenting.

No blocking issues.


Fix 2: Window label shows actual data span

Correctness: ✅

snapshots.len() as u64 * state.bucket_secs() correctly reflects what's in the ring buffer. On startup with 0 samples it shows 0s; after 10 frames at 1s/bucket it shows 10s. Grows monotonically until ring buffer fills.

Minor: at zoom=5 (refresh=60s) with 3 samples, the window shows 3min (3×60=180s... wait, bucket_secs=300 not 60). Actually: snapshots.len() × bucket_secs. With refresh=60s and bucket_secs=300: each raw snapshot is taken every 60s but labeled as covering 300s? No — bucket_secs is the display aggregation unit, not the sampling interval. After this PR, refresh_interval_secs = bucket_secs.min(60). So at zoom=5: refresh=60s, bucket_secs=300, meaning 5 raw samples (taken 1/min over 5min) get aggregated into 1 display bucket. The window label would then show snapshots.len() × 300 which could read as 5min after 1 raw sample... That's off. This is the same conceptual question as above — acceptable for now, worth a TODO.

No blocking issues.


Test coverage

zoom_cycle_syncs_refresh_to_bucket — covers all 6 zoom levels forward and back, verifies 60s cap at levels 5 and 6. Solid.


Summary

APPROVE — 0 blocking findings.

The fixes are correct and well-scoped. The conceptual question about bucket_secs vs refresh_interval_secs mismatch at coarse zoom levels is a known simplification, not a bug — worth a follow-up TODO comment.

Test run: 1865 passed, 0 failed ✓
Clippy: clean ✓
fmt: clean ✓

@NikolayS
Copy link
Copy Markdown
Owner Author

Testing evidence

$ cargo test -- zoom_cycle
test ash::state::tests::zoom_cycle_syncs_refresh_to_bucket ... ok
test result: ok. 1 passed; 0 failed; 0 ignored

$ cargo test
test result: ok. 1865 passed; 0 failed; 0 ignored; 0 measured; finished in 1.46s

$ cargo clippy --all-targets --all-features -- -D warnings
Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.2s  (clean)

$ cargo fmt -- --check
(no output — clean)

Verified behaviors:

  • zoom_cycle_forward() level 1→2→3→4: refresh syncs to 1→15→30→60s ✓
  • zoom_cycle_forward() level 4→5→6: refresh stays capped at 60s ✓
  • zoom_cycle_back() reverses correctly ✓
  • Window label actual_secs = snapshots.len() × bucket_secs — shows 0s on startup, grows as data accumulates ✓

@NikolayS
Copy link
Copy Markdown
Owner Author

PR #763 — Ready for merge

Branch: fix/763-ash-zoom-window | Head: e527357

CI green (all checks pass). REV approved — 0 blocking findings. Testing evidence posted (zoom cycle + 1865 tests + clippy clean).

Awaiting Nik merge.

@NikolayS NikolayS merged commit 6be5618 into main Mar 28, 2026
27 of 28 checks passed
@NikolayS NikolayS deleted the fix/763-ash-zoom-window branch March 28, 2026 03:45
NikolayS added a commit that referenced this pull request Mar 28, 2026
…om ash.samples (#761)

* feat(ash): pre-populate ring buffer from ash.samples when pg_ash is installed (#761)

When pg_ash is detected on the server, query ash.wait_timeline() for the
configured window (default 10 min) and pre-populate the ring buffer before
the live polling loop starts. Falls back to live-only when pg_ash is absent.

- sampler.rs: add query_ash_history() using ash.wait_timeline(interval, '1s')
- mod.rs: pre-populate ring buffer on startup when pg_ash.installed
- state.rs: document pg_ash_installed field
- 26 new unit tests covering history parsing, ring buffer capacity, graceful
  degradation when pg_ash absent

Fixes #761

* test(ai): add slash-ash-pgash AI test for pg_ash history integration (#761)

* chore(demos): move GIFs to demos/, rename to match AI test filenames; add 1s pause after commands

- docs/ash-demo.gif -> demos/slash-ash-general.gif
- docs/ash-demo.cast -> demos/slash-ash-general.cast
- AI test recording paths updated accordingly
- Added sleep 1 after /ash command in expect scripts (gives user time to read output)
- slash-ash-pgash.md: same conventions applied

* fix(ash): address REV findings — parameterize SQL, remove dead code, use configured window; add CI integration tests (#761)

- BUG-1: parameterize ash.wait_timeline interval via $1 (no more format! injection)
- BUG-2: remove dead history_snapshots() fn; wire History mode through query_ash_history()
- WARN-1: use state.bucket_secs()*600 instead of hardcoded 600s window
- Add 2 #[ignore] integration tests: test_pg_ash_history_live, test_pg_ash_history_graceful_degradation

* fix(ash): address REV findings M1/M2 — parameterize interval $1, use int8 for samples; doc cleanups (#761)

- M1: replace format!() string interpolation with $1 parameterized query
  in query_ash_history_inner; rename helper to _inner (no longer interval-keyed)
- M2: use samples::int8 (was int4) to avoid silent overflow on busy servers;
  update row.get() to i64; use u32::MAX as saturating fallback
- L3: fix double-sentence doc on pg_ash_installed in state.rs
- L2: clarify retention_seconds is reserved, not yet wired
- L4: add cpu_count comment in history snapshot construction
- M3: add TODO comment for history-mode per-frame query caching

* test(ash): add CI integration tests for pg_ash sampler (#761)

Three tests in integration_repl.rs (run with --features integration):
- ash_pg_extension_absent_in_test_db: verifies detection SQL returns false
  when pg_ash is not installed (CI baseline)
- ash_wait_timeline_missing_returns_error: confirms the parameterized
  ash.wait_timeline query errors gracefully when schema is absent, validating
  the sampler's Ok(rows) fallback guard
- ash_live_snapshot_query_shape: executes the exact live_snapshot() SQL
  against the test DB, validates row shape (non-empty wtype, non-negative cnt)

* style(ash): cargo fmt — fix integration test formatting (#761)

* fix(ash): sync refresh_interval_secs to bucket_secs on zoom; show actual data window (#763)

- zoom_cycle_forward/back now call sync_refresh_to_zoom(), keeping
  refresh_interval_secs = bucket_secs (capped at 60s) so live sampling
  rate matches display granularity — zoom out = coarser but wider real data
- Status bar window label now shows actual data span (samples × bucket_secs)
  rather than ring-buffer capacity — no more '10min' when you've been running
  for 5 seconds
- Remove now-unused window_label() from AshState
- Add test: zoom_cycle_syncs_refresh_to_bucket

Fixes the misleading window label and zoom/sampling mismatch Nik found.

* docs(ash): re-record general demo GIF with drill-down navigation (#756)

- Show full drill-down: top level → wait type events → query level
- Demonstrate b key navigation back up the stack
- Show legend overlay toggle
- Updated AI test file with drill-down steps and pass criteria
- Resize to 800px wide for Telegram inline playback

* docs(ash): record pg_ash history integration demo GIF (#761)

Shows: history bars pre-populated from ash.wait_timeline on startup
(left side full before live data arrives), drill-down navigation,
legend overlay. 800px wide, Dracula theme.

* docs(ash): native resolution GIFs (951px, no resize)

* feat(ash): add X-axis timestamp labels to timeline

Show HH:MM anchors at left (oldest visible bucket), right (newest/now),
and midpoint when area >= 20 cols wide.  Pure UTC arithmetic — no extra
deps.  Carves a 1-row strip inside the timeline inner area so bar height
is preserved (same layout, one row taller overall inner area used for axis).

Closes the UX gap where bars had no time context at all.

* docs(ash): re-record GIFs with X-axis timestamp labels

Both slash-ash-general.gif and slash-ash-pgash.gif re-recorded
after feat(ash): add X-axis timestamp labels to timeline (feb6fab).

HH:MM anchors now visible at left/mid/right of timeline bottom row.

* docs(ash): re-record pgash GIF with actual historical data pre-populated

Previous recording had only ~90s of history; sampler had just restarted.
This recording has 1000+ samples (~10min) in ash.sample before launch,
so bars are fully pre-populated from the left on first frame.

* fix(ash): detect pg_ash via ash.wait_timeline in pg_proc, not pg_extension

pg_ash installed via SQL (not CREATE EXTENSION) doesn't appear in
pg_extension, causing detect_pg_ash() to return installed=false and
silently skip history pre-population on startup.

Fix: check for ash.wait_timeline in pg_proc/pg_namespace instead.
This handles both install paths (extension + manual SQL) and is the
only capability we actually need to verify.

Update integration test name/query to match.

* fix(ash): use format! for interval literal in wait_timeline query

tokio-postgres cannot serialize a Rust String as a Postgres interval
parameter ($1::interval) without a server-side type annotation that
requires an explicit type OID. The client-side serialization fails
with 'error serializing parameter 0', silently returning an empty
vec and causing history pre-population to be skipped on every launch.

Fix: embed the interval as a literal using format! on the u64 window_secs
value (not user input — no injection risk). Also convert the Err arm from
silent Ok(vec![]) to a proper Err return so callers can log if needed.

* docs(ash): re-record pgash GIF — history bars visible from first frame

Previous recordings failed because wait_timeline query silently returned
empty (tokio-postgres interval serialization bug). Now fixed: bars
pre-populated immediately on /ash launch (57s of history on first frame).

* test(ash): fix ash_wait_timeline_missing_returns_error to use real query

The sampler now uses a literal interval (format!) not a $1 parameter.
Update the integration test to use the same SQL so it tests actual
production behavior: ash.wait_timeline absent → query returns Err.

* docs(ash): re-record both demo GIFs with all fixes applied

- General /ash: live streaming, drill-down, X-axis timestamps, legend
- pg_ash history: bars pre-populated from first frame (1min window)

All three bugs fixed before recording:
- detect_pg_ash uses pg_proc not pg_extension
- wait_timeline interval serialization fixed
- integration test matches production SQL

* docs: add /ash section and GIFs to README

- Add slash-ash-general.gif at top (after intro, before Features)
- Add Active Session History to feature bullet list
- Add ## Active Session History section with usage, keybindings,
  zoom levels, pg_ash history pre-population, and pgash GIF
- Both GIFs already committed in demos/ on this branch

* docs(ash): re-record GIFs using pgbench load per tests/ai spec

- slash-ash-general: pgbench -c 8 load, drill-down, legend, zoom
- slash-ash-pgash: history pre-populated (1min, active=22) on first frame
Both recorded per exact expect scripts in tests/ai/

* docs: replace 2.8MB pspg_screenshot.png with 147K JPEG

PNG was oversized for inline README rendering.
Resized to 1200px wide, converted to JPEG (quality 80) — 147K vs 2.8MB.

* docs(ash): re-record ASH GIFs with fresh pgbench load

pgash GIF: 5min history pre-populated (active=26) on first frame
general GIF: live pgbench load, drill-down, legend

* docs: move /ash GIF to very top of README

* docs: collapse secondary GIFs and pspg screenshot behind <details>

* docs: add quickstart hero demo GIF (connect/version/fix/ash flow)

* chore: bump version to 0.9.0

* docs: re-record quickstart demo with hostname 'demo' in status bar

* docs: re-record quickstart demo v5 — hostname 'demo' in status bar

* refactor(ash): extract group_timeline_rows and split_wait_event helpers

Eliminates duplicated snapshot-grouping logic between
query_ash_history_inner and the test suite.  Tests now call the
production helper directly instead of maintaining a copy.

No behaviour change — all 82 ash tests pass.

* fix(ash): show HH:MM:SS on X axis at zoom 1-2 (bucket ≤ 15 s)

At zoom 1 (1-second buckets) all labels within the same minute were
identical (13:40 / 13:40 / 13:40), making the axis appear static.
Show seconds when bucket_secs ≤ 15 so the right label visibly ticks
forward every second.  Coarser zoom levels keep HH:MM as before.

* docs: re-record quickstart demo (1.4s pause before /ash); add X-axis labels demo GIF

- quickstart-demo: 1.4s deliberate pause after typing /ash so the
  command is visually distinct before the TUI opens; 265K
- slash-ash-xaxis.gif: dedicated demo showing HH:MM:SS labels shifting
  every second at zoom 1 (645K, behind <details>)
- README: update X-axis bullet to mention HH:MM:SS vs HH:MM behaviour;
  add <details> block with slash-ash-xaxis.gif

* docs: quickstart demo v6 — clean prompt, fix timing, hostname fix

- Shell prompt set to '$ ' via --command env override (no openclaw-server)
- Wait for hint text before typing /fix (no more overlap)
- 1.4s pause after typing /ash before TUI opens
- 257K
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