Conversation
Default monitor event output now shows only Power Events so outage transitions are not buried by daemon noise. Count-style verbosity exposes Diagnostics with -v / the first live <V> press and Lifecycle with -vv / the second <V> press. The live TUI groups enabled tiers as Power Events, Diagnostics, then Lifecycle, while --once remains a flat timestamp-sorted list for scripts. Event caps preserve Power first, fill with Diagnostics second, and only use Lifecycle rows last; crash/restart classifier rows remain Diagnostics. This also bumps the code version to 5.3.0-rc0, adds a light 5.3.0 changelog entry, and updates tests/docs for the new event-tier behavior. Co-Authored-By: opencode <noreply@opencode.ai>
Align the Stats E2E event assertions with the new 5.3.0 display contract: default output shows Power Events only, -v adds Diagnostics, and -vv adds Lifecycle. Seed a Power row alongside the existing Diagnostic marker so the default path proves it still surfaces operator-critical events while hiding diagnostics and lifecycle noise. Co-Authored-By: opencode <noreply@opencode.ai>
Count live TUI section headers inside the grouped event cap so preserved Power Events remain in the rendered viewport when Diagnostics or Lifecycle sections are enabled. Also seed E2E event rows in the past to avoid races with the events query end timestamp, and cover the grouped-cap behavior with a unit test. Co-Authored-By: opencode <noreply@opencode.ai>
Keep emitted safety-critical bypass and overload event names in the default Power Events tier so operators see active protection-loss conditions without increasing verbosity. Also keep the newest rows when a grouped section is truncated by live-panel header accounting, preserving the previous most-recent-within-tier cap behavior. Co-Authored-By: opencode <noreply@opencode.ai>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis change implements a three-tier event display system for version 5.3.0-rc0, transitioning from a boolean priority model to count-based verbosity levels (Power events at default, Diagnostics with Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant TUI
participant EventQuery as Event Filter<br/>(query_events_for_display)
participant Database as SQLite<br/>Events DB
participant Renderer as Log Panel<br/>Renderer
User->>CLI: eneru monitor [-v] [-vv]
CLI->>CLI: Parse verbosity level (0/1/2)
CLI->>TUI: run_tui(verbose=level) or run_once(verbose=level)
TUI->>EventQuery: query_events_for_display(verbosity=level)
EventQuery->>Database: Fetch all events from events table
Database-->>EventQuery: Event records
EventQuery->>EventQuery: Filter by verbosity tier<br/>(Power always,<br/>Diagnostics if ≥1,<br/>Lifecycle if ≥2)
EventQuery->>EventQuery: Trim cap: preserve Power > Diagnostics > Lifecycle
EventQuery->>EventQuery: Optionally group by tier
EventQuery-->>TUI: Filtered event list
TUI->>Renderer: render_logs_panel(..., verbosity=level)
Renderer->>Renderer: Render tier headers (bold)
Renderer->>Renderer: Display events grouped by tier
Renderer->>Renderer: Update footer hint with tier level
Renderer-->>User: Display filtered events in TUI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #43 +/- ##
==========================================
+ Coverage 79.31% 79.54% +0.23%
==========================================
Files 25 25
Lines 4530 4616 +86
Branches 843 861 +18
==========================================
+ Hits 3593 3672 +79
- Misses 736 738 +2
- Partials 201 206 +5
🚀 New features to boost your workflow:
|
Update the monitor CLI reference and TUI events documentation for the new Power, Diagnostics, and Lifecycle event verbosity model. Clarify that live TUI grouping is tiered while --once stays flat and timestamp-sorted, and that length caps preserve Power before Diagnostics before Lifecycle. Co-Authored-By: opencode <noreply@opencode.ai>
Owner
Author
|
@coderabbitai full review |
Owner
Author
|
@cubic-dev-ai review this pull request |
✅ Actions performedFull review triggered. |
@m4r1k I have started the AI code review. It will take a few minutes to complete. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/testing.md (1)
73-85:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRefresh the test-count sections on this page too.
This PR adds/updates coverage, but this edit only updates the area description. Please also refresh the pyramid/table counts and per-file breakdown here so the testing doc stays in sync with the current suite.
As per coding guidelines, "When adding or removing tests, update docs/testing.md with test counts in pyramid/table, per-file breakdown, and E2E test case table".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/testing.md` around lines 73 - 85, Update docs/testing.md to refresh the test-counts and breakdowns to match the changes: update the pyramid/table summary counts, per-file test-count breakdown, and the E2E test case table to include the newly added/updated coverage described in the "Test areas" section (e.g., Config loading and validation, Monitor state machine, Shutdown mixins, Multi-UPS coordinator, Redundancy runtime, Health monitoring, Notifications, Statistics and TUI, Packaging). Ensure each referenced area has its corresponding numeric test counts and any per-file entries updated to reflect added or removed tests so the overall totals and tables remain consistent with the current test suite.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/eneru/tui.py`:
- Around line 499-514: The current legacy shim maps priority_only=True to
EVENTS_VERBOSITY_POWER which incorrectly drops lifecycle events; change the
mapping so that when priority_only is True you set verbosity to PRIORITY_EVENTS
(the legacy "priority rows" set: power + lifecycle), otherwise keep the existing
mapping to EVENTS_VERBOSITY_ALL; then pass that value through _events_verbosity
as before (update the conditional handling around priority_only in the function
where priority_only, _events_verbosity, EVENTS_VERBOSITY_POWER,
EVENTS_VERBOSITY_ALL, and PRIORITY_EVENTS are referenced).
In `@tests/test_tui.py`:
- Around line 1705-1706: The comprehensions in tests/test_tui.py use the
ambiguous variable name "l" (e.g., in the expression that builds lines = [l for
l in out.splitlines() if "ON_BATTERY" in l]) which triggers Ruff E741; rename
the loop variable to a clearer identifier like "line" in both occurrences
(around the ON_BATTERY comprehension and the similar one at ~1722-1723) and
update any references inside the comprehension accordingly so the logic remains
identical but the variable name is unambiguous.
---
Outside diff comments:
In `@docs/testing.md`:
- Around line 73-85: Update docs/testing.md to refresh the test-counts and
breakdowns to match the changes: update the pyramid/table summary counts,
per-file test-count breakdown, and the E2E test case table to include the newly
added/updated coverage described in the "Test areas" section (e.g., Config
loading and validation, Monitor state machine, Shutdown mixins, Multi-UPS
coordinator, Redundancy runtime, Health monitoring, Notifications, Statistics
and TUI, Packaging). Ensure each referenced area has its corresponding numeric
test counts and any per-file entries updated to reflect added or removed tests
so the overall totals and tables remain consistent with the current test suite.
🪄 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: bab8a492-a969-4025-8fe2-d0dff8e7c139
📒 Files selected for processing (8)
docs/changelog.mddocs/testing.mdsrc/eneru/cli.pysrc/eneru/tui.pysrc/eneru/version.pytests/e2e/groups/stats.shtests/test_cli.pytests/test_tui.py
Comment on lines
+499
to
+514
| ``verbosity`` selects enabled tiers: ``0`` shows Power Events only, | ||
| ``1`` adds Diagnostics, and ``2`` adds Lifecycle. ``priority_only`` is | ||
| accepted as a legacy shim for tests / older internal callers. | ||
|
|
||
| ``max_events=None`` (or 0) disables the row cap entirely. | ||
|
|
||
| Trim is **3-tier**: POWER_EVENTS always survive the cap; remaining | ||
| slots are filled first with most-recent LIFECYCLE_EVENTS, then with | ||
| chatter (only reachable when ``priority_only=False`` widens the | ||
| upstream filter). Power events are never evicted; lifecycle context | ||
| outranks voltage-flap chatter when the cap is hit in verbose mode. | ||
| slots are filled first with most-recent Diagnostics, then with | ||
| Lifecycle. Power events are never evicted; diagnostics outrank daemon | ||
| lifecycle context when the cap is hit in verbose mode. | ||
| """ | ||
| if priority_only is not None: | ||
| # Legacy compatibility: the old boolean API had priority_only=False | ||
| # mean "show every event type". | ||
| verbosity = EVENTS_VERBOSITY_POWER if priority_only else EVENTS_VERBOSITY_ALL | ||
| verbosity = _events_verbosity(verbosity) |
There was a problem hiding this comment.
Preserve the old priority_only=True semantics or remove the shim.
priority_only=True used to mean “priority rows,” which in this module is still PRIORITY_EVENTS = power + lifecycle. Mapping it to EVENTS_VERBOSITY_POWER now hides lifecycle rows like DAEMON_START, so any stale caller using the documented compatibility arg gets a narrower result than before.
💡 One localized way to keep the shim honest
- if priority_only is not None:
- # Legacy compatibility: the old boolean API had priority_only=False
- # mean "show every event type".
- verbosity = EVENTS_VERBOSITY_POWER if priority_only else EVENTS_VERBOSITY_ALL
+ legacy_priority_only = priority_only
verbosity = _events_verbosity(verbosity)
@@
- if not _event_enabled(etype, verbosity):
- continue
+ if legacy_priority_only is True:
+ if etype not in PRIORITY_EVENTS:
+ continue
+ elif legacy_priority_only is None:
+ if not _event_enabled(etype, verbosity):
+ continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/eneru/tui.py` around lines 499 - 514, The current legacy shim maps
priority_only=True to EVENTS_VERBOSITY_POWER which incorrectly drops lifecycle
events; change the mapping so that when priority_only is True you set verbosity
to PRIORITY_EVENTS (the legacy "priority rows" set: power + lifecycle),
otherwise keep the existing mapping to EVENTS_VERBOSITY_ALL; then pass that
value through _events_verbosity as before (update the conditional handling
around priority_only in the function where priority_only, _events_verbosity,
EVENTS_VERBOSITY_POWER, EVENTS_VERBOSITY_ALL, and PRIORITY_EVENTS are
referenced).
Comment on lines
+1705
to
+1706
| # Should see exactly 5 lines -- the most-recent 5 ON_BATTERY rows. | ||
| lines = [l for l in out.splitlines() if "ON_BATTERY" in l] |
There was a problem hiding this comment.
Rename the comprehension variable to clear Ruff E741.
l is flagged as an ambiguous name in both comprehensions here, so this test file will keep failing lint until it's renamed to something like line.
♻️ Minimal fix
- lines = [l for l in out.splitlines() if "ON_BATTERY" in l]
+ lines = [line for line in out.splitlines() if "ON_BATTERY" in line]
...
- lines = [l for l in out.splitlines() if "ON_BATTERY" in l]
+ lines = [line for line in out.splitlines() if "ON_BATTERY" in line]Also applies to: 1722-1723
🧰 Tools
🪛 Ruff (0.15.12)
[error] 1706-1706: 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 1705 - 1706, The comprehensions in
tests/test_tui.py use the ambiguous variable name "l" (e.g., in the expression
that builds lines = [l for l in out.splitlines() if "ON_BATTERY" in l]) which
triggers Ruff E741; rename the loop variable to a clearer identifier like "line"
in both occurrences (around the ON_BATTERY comprehension and the similar one at
~1722-1723) and update any references inside the comprehension accordingly so
the logic remains identical but the variable name is unambiguous.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Drop the internal priority_only compatibility argument from query_events_for_display so the new v5.3 event tier API has one explicit path: verbosity 0, 1, or 2. Also address CodeRabbit follow-ups by renaming ambiguous test comprehension variables and documenting the updated Stats E2E event-tier check. Co-Authored-By: opencode <noreply@opencode.ai>
Owner
Author
|
CodeRabbit follow-up: we removed the internal |
Retry curses startup with xterm-256color when Ghostty advertises xterm-ghostty but the host terminfo database has not been updated yet. Refresh packaged shell completions for the new event verbosity flags and bump the release candidate to 5.3.0-rc1. Co-Authored-By: opencode <noreply@opencode.ai>
Make the non-events-only --once Recent Events block respect --length instead of applying an extra hard cap of 10 rows. This matches the documented flag semantics: --time remains graph-only, --verbose selects event tiers, and --length controls the event row cap in both --once output modes. Co-Authored-By: opencode <noreply@opencode.ai>
Grouped query_events_for_display previously returned an empty list when max_events=1 because the per-section guard `if remaining_lines < 2: break` exited before any header could render, silently violating the docstring contract that POWER_EVENTS are never evicted within the cap. --once is unaffected (it calls grouped=False), but --length 1 from the CLI is a valid value and the live-TUI panel-sizing path can in theory reach length=1 on a tiny terminal. Add an early return at max_events==1 that surfaces the single most-recent row from the trimmed `rows` list -- which the tier-priority trim block above has already reduced to the highest-priority survivor (Power before Diagnostics before Lifecycle). The bare row is rendered without a header because the cap is too small to honor both. Polish the existing `< 2` inline comment to spell out what 2 is (1 header + 1 row). Three regression tests cover the contract: grouped length=1 with a Power event renders bare ON_BATTERY, ungrouped length=1 keeps the Power event, and grouped length=1 with no Power falls back to the most-recent Diagnostic.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
-v/ first<V>, and Lifecycle with-vv/ second<V>.--onceflat and timestamp-sorted.Verification
bash -n tests/e2e/groups/stats.shpytest tests/test_tui.py tests/test_cli.py(134 passed)pytest(990 passed)agent-skills:code-reviewerpass: APPROVESummary by CodeRabbit
New Features
-v, and Lifecycle via-vv.--onceoutput remains timestamp-sorted.Documentation