Skip to content

fix(tui): tier events priority + decouple --time from events#42

Merged
m4r1k merged 3 commits intomainfrom
fix/tui-events-tiered-priority
Apr 29, 2026
Merged

fix(tui): tier events priority + decouple --time from events#42
m4r1k merged 3 commits intomainfrom
fix/tui-events-tiered-priority

Conversation

@m4r1k
Copy link
Copy Markdown
Owner

@m4r1k m4r1k commented Apr 29, 2026

Three real bugs in the unreleased v5.2.2 surfaced when testing against the maintainer's own data (65 daemon-lifecycle events vs 5 power events in the SQLite events table — the priority filter as it shipped was actually pushing power events off-screen).

Bugs fixed

  1. Events panel showed daemon noise instead of power events. PRIORITY_EVENTS lumped power transitions and daemon-lifecycle into one undifferentiated set, so the most-recent N rows in the cap were all DAEMON_RESTARTED / DAEMON_UPGRADED from package iteration. Power events from days ago got pushed off-screen.

    Fix: split into POWER_EVENTS (always preserved within cap) and LIFECYCLE_EVENTS (fill remaining slots). query_events_for_display trim logic preserves all power events first, then fills with most-recent lifecycle events. Daemon noise can no longer push power events off-screen.

  2. eneru tui --once --events-only silently fell back to log parsing for sparse-event installs. The default 1h time window was almost always empty, so SQLite returned [] and the code parsed /var/log/ups-monitor.log instead. Live TUI and --once showed structurally different content and the operator never noticed.

    Fix: events query no longer takes a time-range parameter at all. Always queries the full events table (events are sparse, this is fine).

  3. --time accidentally scoped both the graph window AND the events panel. Cause of fix: correct legacy Discord config detection in validate_config #2.

    Fix: --time and <T> are graph-only now. Anchor comments at tui.py (TIME_RANGES) and cli.py (--time flag def) name the bug they prevent so this can't be re-coupled by accident.

Knob change

  • Removed --full-history (only added in unreleased 5.2.2; replaced cleanly by --length 0).
  • Added --length N for --once: cap events output (default 30, 0 = no cap; argparse type validator rejects negatives + non-numerics).
  • Live TUI events cap now coupled to the visible-rows estimate (min(EVENTS_MAX_ROWS_NORMAL, visible_panel_rows)). Without this, the data cap was 30 but the panel only renders ~11 rows on a 24-row terminal, so the tier-trim's preserved power events sat above the visible window. <M> still expands to 500 rows.

Sequencing

This rides on the existing unreleased v5.2.2 in main. No version bump or changelog re-tag needed — the [5.2.2] entry is updated in this PR to reflect the final design (tier-trim, --length, no --full-history). Maintainer asked the changelog be kept tiny since this is a bugfix release.

Test plan

  • 981 unit/integration tests pass (pytest -q --timeout=30 in uv venv); 13 new tests covering tier preservation (incl. the maintainer's exact reproducer: 60 daemon + 3 power, asserts all 3 power events survive cap=20), pathological "more power events than cap", --length validation, and snapshot-path tail behavior
  • Manual on Rocky Linux 9 (rebuilt RPM, dnf reinstall): eneru tui (live, 24×80) shows the 5 power events at top of panel; --once --events-only --length 5 returns the 5 most-recent power events; --length -1 rejects with exit 2; --full-history rejected as unknown by argparse
  • Pre-push code review (agent-skills:code-reviewer, opus): one P3 (snapshot tail unbounded with --length 0) addressed before push
  • CI must pass: validate matrix (Python 3.9-3.14) + 5 E2E matrix jobs. E2E Stats exercises the new --length validation + priority-only default

🤖 Generated with Claude Code


Summary by cubic

Fixes the TUI events list with three-tier priority so power events never get pushed off-screen, and makes --time graph-only. Adds --length for --once, removes --full-history, and sizes live events to the visible rows.

  • Bug Fixes

    • Three-tier priority: power events are always kept; lifecycle fills next; chatter only when --verbose. Prevents chatter from evicting lifecycle rows in verbose mode.
    • Events read the full SQLite table (no time window). Prevents empty panels and log fallbacks. --time and <T> now affect only the graph.
    • Live TUI caps events to the visible rows (min(30, visible_panel_rows); <M> still expands to 500).
    • Snapshot tail always caps at 10 lines to keep the header readable.
  • Migration

    • Replace --full-history with --length 0.
    • If you used --time to size events, switch to --length.
    • If you grep for low‑priority types, pass --verbose.

Written for commit 7cdd1e7. Summary will update on new commits. Review in cubic

Summary by CodeRabbit

Release Notes

  • Documentation

    • Clarified --time controls only graph time range, not events query
  • Changed

    • Replaced --full-history with new --length N parameter to cap event output
    • Power events now prioritized in display; lifecycle events fill remaining slots
    • Increased events display cap from 20 to 30 rows in live mode
    • --verbose/-v now includes low-priority events
  • Bug Fixes

    • Events panel decoupled from graph time selection
    • --events-only now falls back to log parsing when default window is empty

m4r1k and others added 2 commits April 29, 2026 10:08
Three real bugs surfaced when testing 5.2.2 against the maintainer's
own data (65 daemon-lifecycle events vs 5 power events in the SQLite
events table):

1. Events panel showed daemon noise instead of power events.
   PRIORITY_EVENTS lumped power + daemon into one tier, so the most-
   recent N rows in the cap were all daemon-restarts. Power events
   from days ago got pushed off-screen.

   Fix: split into POWER_EVENTS (always preserved within cap) and
   LIFECYCLE_EVENTS (fill remaining slots). Daemon noise can no
   longer push power events off-screen.

2. `eneru tui --once --events-only` silently fell back to log
   parsing for sparse-event installs. The default 1h time window
   was almost always empty -- SQLite returned [], code parsed
   /var/log/ups-monitor.log instead. The two surfaces showed
   different content and the operator never noticed.

   Fix: events query no longer takes a time-range parameter.
   Always queries the full events table (sparse, so this is fine).

3. `--time` accidentally scoped both the graph window AND the
   events panel. Cause of #2.

   Fix: --time and <T> are graph-only now. Anchor comments at
   tui.py:23-37 (TIME_RANGES) and cli.py:_add_monitor_args
   (--time) name the bug they prevent so this can't be re-coupled
   by accident.

Knob change:
- Removed --full-history (only added in unreleased 5.2.2;
  --length 0 covers the same use case more cleanly)
- Added --length N (default 30, 0 = no cap; argparse type
  validator rejects negatives + non-numerics)
- Live TUI events cap default raised 20 -> 30 (matches CLI default)

Snapshot tail (run_once non-events-only path) caps at 10 rows
unconditionally so --length 0 doesn't drown the status block --
operators wanting "all events" use --events-only.

Test coverage: 13 new tests (981 total, was 968). Mirrors the
maintainer's reproducer in test_tiered_trim_preserves_power_events
(60 daemon + 3 power, asserts all 3 power events survive cap=20)
plus pathological "more power events than cap" + length=0 / --length
validation. E2E Test 31 swaps --full-history for --length assertions.

Pre-push reviewed by agent-skills:code-reviewer (opus); P3 finding
addressed (snapshot tail cap clamp).

Co-Authored-By: Claude Opus 4.7 <[email protected]>
Discovered while testing the tier-trim fix on the maintainer's
24-row terminal: the data layer correctly returned 30 events with
all 5 power events at the top of the chronological list, but the
events panel anchored at the bottom only renders ~11 rows visible
in that terminal size. Power events at positions 0-4 sat off-screen
until the operator scrolled up.

Fix: in run_tui's main loop, compute a visible-rows estimate from
the panel layout (logs_end - logs_start - 4) and cap the data
query to min(EVENTS_MAX_ROWS_NORMAL, visible_estimate). Tier-trim
then guarantees power events fit inside what the panel actually
shows. <M> still expands to EVENTS_MAX_ROWS_MORE (500) for full
scrollable history.

Verified manually on a 24x80 terminal: cap computes to 11; query
returns 5 power + 6 most-recent daemon; panel displays all 11 with
power events at the top.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

📝 Walkthrough

Walkthrough

The pull request replaces the --full-history CLI flag with a new --length N parameter that caps events output. Events querying is decoupled from the graph's time-window selection; events are always queried from the full table but trimmed using a tiered approach that preserves power events within the row cap. Documentation and tests are updated accordingly.

Changes

Cohort / File(s) Summary
Documentation
docs/changelog.md, docs/configuration.md
Updated changelog entries for 5.2.2 release and CLI option documentation to remove --full-history, document new --length N parameter, and clarify that --time applies only to graphs, not events.
CLI Implementation
src/eneru/cli.py
Removes --full-history option and validation logic; adds --length argument with non-negative integer validation (via new _non_negative_int helper); clarifies --time help text to apply only to graphs.
TUI Core Logic
src/eneru/tui.py
Decouples events from graph time selection by removing time_range_seconds parameter from query_events_for_display; implements tiered row-trimming logic that preserves all power events within cap while evicting non-power rows; changes run_once signature to accept length parameter (defaults to EVENTS_MAX_ROWS_NORMAL); increases events cap from 20 to 30 rows.
End-to-End Tests
tests/e2e/groups/stats.sh
Removes --time dependency for events tests; updates Test 31 to validate --length parameter behavior (accepts valid values, rejects negative ones) and removes prior --full-history checks.
Unit Tests
tests/test_cli.py, tests/test_tui.py
Replaces --full-history test coverage with --length validation (including edge cases like 0 and negative values); updates event-query assertions to remove time_range_seconds argument; adds new tiered-trimming test cases verifying power-event preservation under row-cap conditions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

Events now break free from time's tight leash,
Power events safe, with tiered trim's caress,
No more --full-history, just --length to confess—
A cleaner dance between graph and event mesh! 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.42% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly summarizes the main changes: fixing event tiering by priority and decoupling --time from events display, which aligns with the primary objectives of the PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/tui-events-tiered-priority

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@m4r1k
Copy link
Copy Markdown
Owner Author

m4r1k commented Apr 29, 2026

@coderabbitai full review

@m4r1k
Copy link
Copy Markdown
Owner Author

m4r1k commented Apr 29, 2026

@cubic-dev-ai review this pull request

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

✅ Actions performed

Full review triggered.

@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented Apr 29, 2026

@cubic-dev-ai review this pull request

@m4r1k I have started the AI code review. It will take a few minutes to complete.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

❌ Patch coverage is 88.23529% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.49%. Comparing base (c1290f4) to head (7cdd1e7).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/eneru/tui.py 83.33% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #42   +/-   ##
=======================================
  Coverage   79.49%   79.49%           
=======================================
  Files          25       25           
  Lines        4515     4530   +15     
  Branches      841      843    +2     
=======================================
+ Hits         3589     3601   +12     
- Misses        726      729    +3     
  Partials      200      200           
Files with missing lines Coverage Δ
src/eneru/cli.py 71.62% <100.00%> (+0.39%) ⬆️
src/eneru/tui.py 63.49% <83.33%> (+0.14%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
tests/e2e/groups/stats.sh (1)

264-273: Inline the exit-code check under set -e.

If eneru monitor --once --events-only --length 5 ... ever returns non-zero, the subshell exits before the $? branch runs, so the custom log dump never executes. Wrap the command directly in the if instead.

Suggested change
-eneru monitor --once --events-only --length 5 --config "$E2E_DIR/config-e2e-stats.yaml" > /tmp/test31-length.log 2>&1
-if [ $? -ne 0 ]; then
+if ! eneru monitor --once --events-only --length 5 \
+  --config "$E2E_DIR/config-e2e-stats.yaml" \
+  > /tmp/test31-length.log 2>&1; then
   echo "FAIL: --length 5 returned non-zero"
   cat /tmp/test31-length.log
   exit 1
 fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/e2e/groups/stats.sh` around lines 264 - 273, The test currently runs
`eneru monitor --once --events-only --length 5 --config
"$E2E_DIR/config-e2e-stats.yaml"` and then checks `$?`, but with `set -e` a
non-zero exit will abort before the check and log dump; change the test to run
the command inside an `if` conditional (e.g., `if eneru ... >
/tmp/test31-length.log 2>&1; then ... else ... fi`) so the failure branch can
print the log and exit with failure; update the pass/fail branches around that
`if` and keep the same log file `/tmp/test31-length.log` and messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/changelog.md`:
- Around line 36-38: Update the changelog bullet about the Live TUI events cap
to reflect that it's height-bounded rather than a flat 30 rows: change the line
mentioning "Live TUI events cap raised from 8 to 30 rows" to say "Live TUI
events cap raised (up to 30 rows, bounded by visible_panel_rows)" or explicitly
note it uses min(30, visible_panel_rows) in normal mode so shorter terminals
show fewer than 30 rows.

In `@docs/configuration.md`:
- Around line 439-446: The examples block mixes invocation styles (e.g., "sudo
eneru validate ..." vs "sudo /opt/ups-monitor/eneru.py monitor ..."); update
every example so package-user commands consistently use the packaged wrapper
form "/opt/ups-monitor/eneru.py" (e.g., replace "sudo eneru ..." and any "eneru
..." invocations with "sudo /opt/ups-monitor/eneru.py ..."), keeping the same
flags and arguments (validate, run, monitor, --dry-run, --once, --events-only,
--verbose, --length, --config) so the block uniformly shows package-user usage.

In `@src/eneru/tui.py`:
- Around line 473-489: When capping rows (when max_events is set) the current
logic treats all non-POWER_EVENTS equally so verbose chatter can evict lifecycle
events; change the trim in the block that references rows, POWER_EVENTS,
max_events, kept_non_power and kept so lifecycle rows are prioritized: split
non_power into lifecycle = [r for r in rows if r[2] in LIFECYCLE_EVENTS] and
chatter = [r for r in rows if r[2] not in LIFECYCLE_EVENTS and r[2] not in
POWER_EVENTS], then when computing remaining fill from the most recent lifecycle
rows first (up to remaining) and only then backfill with the most recent chatter
to produce kept_non_power, re-merge with power and sort by timestamp as before.

In `@tests/test_tui.py`:
- Around line 1240-1252: Rename the one-letter comprehension variable "l" to a
clearer name like "line" in the test assertions around the
query_events_for_display call: update the three asserts that use "l" in their
any() comprehensions and the daemon_lines list comprehension to use "line"
instead (e.g., any("ON_BATTERY: real outage" in line for line in lines) and
daemon_lines = [line for line in lines if "DAEMON_RESTARTED" in line]); this
will satisfy Ruff E741 and improve readability without changing test logic in
the test_tui.py tests that assert output from query_events_for_display.

---

Nitpick comments:
In `@tests/e2e/groups/stats.sh`:
- Around line 264-273: The test currently runs `eneru monitor --once
--events-only --length 5 --config "$E2E_DIR/config-e2e-stats.yaml"` and then
checks `$?`, but with `set -e` a non-zero exit will abort before the check and
log dump; change the test to run the command inside an `if` conditional (e.g.,
`if eneru ... > /tmp/test31-length.log 2>&1; then ... else ... fi`) so the
failure branch can print the log and exit with failure; update the pass/fail
branches around that `if` and keep the same log file `/tmp/test31-length.log`
and messages.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 175e8f5c-88c2-4eef-aa32-9f7d8c52c49e

📥 Commits

Reviewing files that changed from the base of the PR and between c1290f4 and c571b82.

📒 Files selected for processing (7)
  • docs/changelog.md
  • docs/configuration.md
  • src/eneru/cli.py
  • src/eneru/tui.py
  • tests/e2e/groups/stats.sh
  • tests/test_cli.py
  • tests/test_tui.py

Comment thread docs/changelog.md Outdated
Comment on lines +36 to +38
- `--time` and `<T>` apply to the graph only. Use `--length` for events.
- Events panel defaults to priority-only.
- Live TUI events cap raised from 8 to 30 rows.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

The live TUI cap is height-bounded, not a flat 30 rows.

Normal mode now uses min(30, visible_panel_rows), so shorter terminals still show fewer than 30 rows. This bullet should say "up to 30 rows" or mention the visible-panel bound explicitly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/changelog.md` around lines 36 - 38, Update the changelog bullet about
the Live TUI events cap to reflect that it's height-bounded rather than a flat
30 rows: change the line mentioning "Live TUI events cap raised from 8 to 30
rows" to say "Live TUI events cap raised (up to 30 rows, bounded by
visible_panel_rows)" or explicitly note it uses min(30, visible_panel_rows) in
normal mode so shorter terminals show fewer than 30 rows.

Comment thread docs/configuration.md
Comment on lines 439 to 446
Example package commands:

```bash
sudo eneru validate --config /etc/ups-monitor/config.yaml
sudo eneru run --dry-run --config /etc/ups-monitor/config.yaml
sudo eneru monitor --once --events-only --config /etc/ups-monitor/config.yaml
sudo /opt/ups-monitor/eneru.py monitor --once --events-only --verbose --full-history --config /etc/ups-monitor/config.yaml
sudo /opt/ups-monitor/eneru.py monitor --once --events-only --verbose --length 100 --config /etc/ups-monitor/config.yaml
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Keep the package-user examples on one invocation style.

This block is headed as package-user examples, but it now mixes eneru ... and /opt/ups-monitor/eneru.py .... Please make the whole block use the packaged wrapper consistently.

Suggested change
-sudo eneru validate --config /etc/ups-monitor/config.yaml
-sudo eneru run --dry-run --config /etc/ups-monitor/config.yaml
-sudo eneru monitor --once --events-only --config /etc/ups-monitor/config.yaml
+sudo /opt/ups-monitor/eneru.py validate --config /etc/ups-monitor/config.yaml
+sudo /opt/ups-monitor/eneru.py run --dry-run --config /etc/ups-monitor/config.yaml
+sudo /opt/ups-monitor/eneru.py monitor --once --events-only --config /etc/ups-monitor/config.yaml
 sudo /opt/ups-monitor/eneru.py monitor --once --events-only --verbose --length 100 --config /etc/ups-monitor/config.yaml

As per coding guidelines "When writing documentation, use /opt/ups-monitor/eneru.py invocation style for package users (README, troubleshooting), python -m eneru or eneru for developers (CONTRIBUTING, testing), and eneru for PyPI users".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Example package commands:
```bash
sudo eneru validate --config /etc/ups-monitor/config.yaml
sudo eneru run --dry-run --config /etc/ups-monitor/config.yaml
sudo eneru monitor --once --events-only --config /etc/ups-monitor/config.yaml
sudo /opt/ups-monitor/eneru.py monitor --once --events-only --verbose --full-history --config /etc/ups-monitor/config.yaml
sudo /opt/ups-monitor/eneru.py monitor --once --events-only --verbose --length 100 --config /etc/ups-monitor/config.yaml
```
Example package commands:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/configuration.md` around lines 439 - 446, The examples block mixes
invocation styles (e.g., "sudo eneru validate ..." vs "sudo
/opt/ups-monitor/eneru.py monitor ..."); update every example so package-user
commands consistently use the packaged wrapper form "/opt/ups-monitor/eneru.py"
(e.g., replace "sudo eneru ..." and any "eneru ..." invocations with "sudo
/opt/ups-monitor/eneru.py ..."), keeping the same flags and arguments (validate,
run, monitor, --dry-run, --once, --events-only, --verbose, --length, --config)
so the block uniformly shows package-user usage.

Comment thread src/eneru/tui.py Outdated
Comment on lines +473 to +489
# Tiered trim: power events always survive the cap. ``max_events``
# in (None, 0) means no cap.
if max_events and len(rows) > max_events:
power = [r for r in rows if r[2] in POWER_EVENTS]
non_power = [r for r in rows if r[2] not in POWER_EVENTS]
if len(power) >= max_events:
# Pathological: more power events than the cap. Show the
# most-recent N -- still better than evicting them.
kept = power[-max_events:]
else:
remaining = max_events - len(power)
kept_non_power = non_power[-remaining:]
# Re-sort by ts so the panel never displays out-of-time-order
# rows after the tier merge. Python's sort is stable, so
# equal-timestamp rows preserve insertion order (power first).
kept = sorted(power + kept_non_power, key=lambda r: r[0])
rows = kept
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Keep lifecycle events above verbose chatter when the list is capped.

In verbose mode, non_power currently mixes LIFECYCLE_EVENTS with low-priority chatter, and the [-remaining:] slice is purely recency-based. That means a noisy verbose tail can still push DAEMON_START/DAEMON_RESTARTED out of the capped list, even though the new contract says lifecycle rows fill the remaining slots after power events.

Suggested change
-    if max_events and len(rows) > max_events:
-        power = [r for r in rows if r[2] in POWER_EVENTS]
-        non_power = [r for r in rows if r[2] not in POWER_EVENTS]
+    if max_events and len(rows) > max_events:
+        power = [r for r in rows if r[2] in POWER_EVENTS]
+        lifecycle = [r for r in rows if r[2] in LIFECYCLE_EVENTS]
+        chatter = [
+            r for r in rows
+            if r[2] not in POWER_EVENTS and r[2] not in LIFECYCLE_EVENTS
+        ]
         if len(power) >= max_events:
             # Pathological: more power events than the cap. Show the
             # most-recent N -- still better than evicting them.
             kept = power[-max_events:]
         else:
             remaining = max_events - len(power)
-            kept_non_power = non_power[-remaining:]
+            kept_lifecycle = lifecycle[-remaining:]
+            remaining -= len(kept_lifecycle)
+            kept_chatter = chatter[-remaining:] if remaining > 0 else []
             # Re-sort by ts so the panel never displays out-of-time-order
             # rows after the tier merge. Python's sort is stable, so
             # equal-timestamp rows preserve insertion order (power first).
-            kept = sorted(power + kept_non_power, key=lambda r: r[0])
+            kept = sorted(power + kept_lifecycle + kept_chatter, key=lambda r: r[0])
         rows = kept
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/eneru/tui.py` around lines 473 - 489, When capping rows (when max_events
is set) the current logic treats all non-POWER_EVENTS equally so verbose chatter
can evict lifecycle events; change the trim in the block that references rows,
POWER_EVENTS, max_events, kept_non_power and kept so lifecycle rows are
prioritized: split non_power into lifecycle = [r for r in rows if r[2] in
LIFECYCLE_EVENTS] and chatter = [r for r in rows if r[2] not in LIFECYCLE_EVENTS
and r[2] not in POWER_EVENTS], then when computing remaining fill from the most
recent lifecycle rows first (up to remaining) and only then backfill with the
most recent chatter to produce kept_non_power, re-merge with power and sort by
timestamp as before.

Comment thread tests/test_tui.py
Comment on lines +1240 to +1252
lines = query_events_for_display(config, max_events=20)
# All 3 power-event rows MUST survive the cap.
assert any("ON_BATTERY: real outage" in l for l in lines), (
f"ON_BATTERY pushed off by daemon noise -- tiered trim regressed. "
f"Got: {lines}"
)
assert any("POWER_RESTORED: outage 10m" in l for l in lines)
assert any("EMERGENCY_SHUTDOWN_INITIATED" in l for l in lines)
# Result respects the cap.
assert len(lines) == 20
# Remaining 17 slots are most-recent daemon rows.
daemon_lines = [l for l in lines if "DAEMON_RESTARTED" in l]
assert len(daemon_lines) == 17
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find the test file
fd -type f -name "test_tui.py"

Repository: m4r1k/Eneru

Length of output: 226


🏁 Script executed:

# Check if the file exists and get line count
wc -l tests/test_tui.py

Repository: m4r1k/Eneru

Length of output: 78


🏁 Script executed:

# Read the specific line ranges mentioned in the review
sed -n '1240,1252p' tests/test_tui.py

Repository: m4r1k/Eneru

Length of output: 734


🏁 Script executed:

# Check the other mentioned ranges
sed -n '1271,1276p' tests/test_tui.py

Repository: m4r1k/Eneru

Length of output: 412


🏁 Script executed:

# Check the third range
sed -n '1525,1545p' tests/test_tui.py

Repository: m4r1k/Eneru

Length of output: 1021


Rename one-letter comprehension variables to improve readability.

The loop variable l in comprehensions violates Ruff's E741 rule (ambiguous variable name), as single lowercase l is easily confused with 1 or I. Rename to line across all affected test assertions to maintain PEP 8 compliance.

Suggested changes
-        assert any("ON_BATTERY: real outage" in l for l in lines), (
+        assert any("ON_BATTERY: real outage" in line for line in lines), (
             f"ON_BATTERY pushed off by daemon noise -- tiered trim regressed. "
             f"Got: {lines}"
         )
-        assert any("POWER_RESTORED: outage 10m" in l for l in lines)
-        assert any("EMERGENCY_SHUTDOWN_INITIATED" in l for l in lines)
+        assert any("POWER_RESTORED: outage 10m" in line for line in lines)
+        assert any("EMERGENCY_SHUTDOWN_INITIATED" in line for line in lines)
         # Result respects the cap.
         assert len(lines) == 20
         # Remaining 17 slots are most-recent daemon rows.
-        daemon_lines = [l for l in lines if "DAEMON_RESTARTED" in l]
+        daemon_lines = [line for line in lines if "DAEMON_RESTARTED" in line]
         assert len(daemon_lines) == 17
@@
-        assert all("ON_BATTERY" in l for l in lines)
-        assert any("outage-49" in l for l in lines)  # latest power event
-        assert any("outage-40" in l for l in lines)  # 10th-latest
+        assert all("ON_BATTERY" in line for line in lines)
+        assert any("outage-49" in line for line in lines)  # latest power event
+        assert any("outage-40" in line for line in lines)  # 10th-latest
@@
-        lines = [l for l in out.splitlines() if "DAEMON_START" in l]
+        lines = [line for line in out.splitlines() if "DAEMON_START" in line]
         assert len(lines) == 5
         assert "row-49" in lines[-1]
         assert "row-45" in lines[0]
@@
-        lines = [l for l in out.splitlines() if "DAEMON_START" in l]
+        lines = [line for line in out.splitlines() if "DAEMON_START" in line]
         assert len(lines) == 40, f"length=0 must show all rows; got {len(lines)}"
🧰 Tools
🪛 Ruff (0.15.12)

[error] 1242-1242: Ambiguous variable name: l

(E741)


[error] 1246-1246: Ambiguous variable name: l

(E741)


[error] 1247-1247: Ambiguous variable name: l

(E741)


[error] 1251-1251: Ambiguous variable name: l

(E741)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_tui.py` around lines 1240 - 1252, Rename the one-letter
comprehension variable "l" to a clearer name like "line" in the test assertions
around the query_events_for_display call: update the three asserts that use "l"
in their any() comprehensions and the daemon_lines list comprehension to use
"line" instead (e.g., any("ON_BATTERY: real outage" in line for line in lines)
and daemon_lines = [line for line in lines if "DAEMON_RESTARTED" in line]); this
will satisfy Ruff E741 and improve readability without changing test logic in
the test_tui.py tests that assert output from query_events_for_display.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 7 files

Addresses CodeRabbit's 5 findings (cubic.dev was clean):

1. docs/changelog.md — clarify the live TUI events cap is now
   min(30, visible_panel_rows), not a flat 30, since the second
   commit made it height-bound.

2. docs/configuration.md — make all four example commands use the
   same `sudo eneru ...` invocation form. Earlier draft mixed the
   wrapper-path form with the bare command; maintainer prefers
   the bare form (PATH-resolved) consistently.

3. src/eneru/tui.py — three-tier trim instead of two: POWER_EVENTS
   always survive; LIFECYCLE_EVENTS fill next; chatter rounds out
   only if room remains. In --verbose mode the previous code let
   chatter evict lifecycle rows when the cap was hit; the new tier
   matches the user-facing priority hierarchy.

4. tests/test_tui.py — rename single-letter loop var `l` to `line`
   in five spots (Ruff E741).

5. tests/e2e/groups/stats.sh — wrap the `--length 5` invocation in
   `if cmd; then ... fi`. Under `set -e` the previous bare
   `[ $? -ne 0 ]` branch would never run because the script aborts
   on the first non-zero exit, masking any regression with no log
   dump.

Plus one new unit test (`test_tiered_trim_lifecycle_outranks_chatter`)
covering the new 3-tier semantics: 1 power + 30 chatter + 4 lifecycle
at cap=7 produces 1 power + 4 lifecycle + 2 chatter, NOT 1 power +
6 chatter.

982 unit tests pass (was 981).

Co-Authored-By: Claude Opus 4.7 <[email protected]>
@m4r1k m4r1k merged commit 93a7fef into main Apr 29, 2026
40 checks passed
@m4r1k m4r1k deleted the fix/tui-events-tiered-priority branch April 29, 2026 08:53
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.

1 participant